Fix stats.countby --by-name sort with mixed value types (SYN-9957)

changes/92730936fe2f60d077b3b1d9db81bb0f.yaml

@@ -0,0 +1,8 @@
+---
+desc: Fixed a bug where the ``stats.countby`` Storm command could raise a ``TypeError``
+  when using ``--by-name`` to sort a tally which contained a mix of numeric and non-numeric
+  values.
+desc:literal: false
+prs: []
+type: bug
+...

synapse/lib/stormlib/stats.py

@@ -1,3 +1,4 @@
+import decimal
 import collections
 
 import synapse.exc as s_exc
@@ -86,17 +87,20 @@ async def execStormCmd(self, runt, genr):
             return
 
         if byname:
-            # Try to sort numerically instead of lexicographically
-            def coerce(indx):
-                def wrapped(valu):
-                    valu = valu[indx]
-                    try:
-                        return int(valu)
-                    except ValueError:
-                        return valu
-                return wrapped
-
-            values = list(sorted(counts.items(), key=coerce(0)))
+            # Sort numeric names numerically, then non-numeric names lexicographically.
+            def sortkey(item):
+                try:
+                    huge = s_common.hugenum(item[0])
+                except (ValueError, decimal.DecimalException):
+                    return (1, decimal.Decimal(0), item[0])
+
+                # non-finite values such as nan cannot be ordered
+                if not huge.is_finite():
+                    return (1, decimal.Decimal(0), item[0])
+
+                return (0, huge, '')
+
+            values = list(sorted(counts.items(), key=sortkey))
             maxv = max(val[1] for val in values)
 
         else:

synapse/tests/test_lib_stormlib_stats.py

@@ -136,6 +136,44 @@
 13 | 4 | 26.67% | ########################################
 '''.strip()
 
+chartunset_byname = '''
+None | 1 | #########################
+   4 | 1 | #########################
+   1 | 2 | ##################################################
+   0 | 1 | #########################
+'''.strip()
+
+chartunset_rev_byname = '''
+   0 | 1 | #########################
+   1 | 2 | ##################################################
+   4 | 1 | #########################
+None | 1 | #########################
+'''.strip()
+
+chartmixed_byname = '''
+ nan | 1 | ##################################################
+ abc | 1 | ##################################################
+None | 1 | ##################################################
+0xzz | 1 | ##################################################
+0x20 | 1 | ##################################################
+  10 | 1 | ##################################################
+   2 | 1 | ##################################################
+ 1.5 | 1 | ##################################################
+   1 | 1 | ##################################################
+'''.strip()
+
+chartmixed_rev_byname = '''
+   1 | 1 | ##################################################
+ 1.5 | 1 | ##################################################
+   2 | 1 | ##################################################
+  10 | 1 | ##################################################
+0x20 | 1 | ##################################################
+0xzz | 1 | ##################################################
+None | 1 | ##################################################
+ abc | 1 | ##################################################
+ nan | 1 | ##################################################
+'''.strip()
+
 
 class StatsTest(s_test.SynTest):
 
@@ -230,6 +268,30 @@ async def test_stormlib_stats_countby(self):
             with self.raises(s_exc.BadArg):
                 self.len(15, await core.nodes('inet:ipv4 | stats.countby ({})'))
 
+            # a tally of numeric values where some nodes do not have the property
+            # set tallies the unset nodes under None. ( SYN-9957 )
+            q = '''[ (inet:ipv4=1.2.3.1 :asn=0) (inet:ipv4=1.2.3.2 :asn=1) (inet:ipv4=1.2.3.3 :asn=1)
+                     (inet:ipv4=1.2.3.4) (inet:ipv4=1.2.3.5 :asn=4) +#unset ]'''
+            self.len(5, await core.nodes(q))
+
+            msgs = await core.stormlist('inet:ipv4#unset | stats.countby :asn --by-name')
+            self.stormIsInPrint(chartunset_byname, msgs)
+
+            msgs = await core.stormlist('inet:ipv4#unset | stats.countby :asn --by-name --reverse')
+            self.stormIsInPrint(chartunset_rev_byname, msgs)
+
+            # numeric names sort numerically and non-numeric names sort
+            # lexicographically after them.
+            q = '''[ it:dev:str="1" it:dev:str="1.5" it:dev:str="2" it:dev:str="10" it:dev:str="nan"
+                     it:dev:str="0x20" it:dev:str="0xzz" it:dev:str="None" it:dev:str="abc" ]'''
+            self.len(9, await core.nodes(q))
+
+            msgs = await core.stormlist('it:dev:str | stats.countby --by-name')
+            self.stormIsInPrint(chartmixed_byname, msgs)
+
+            msgs = await core.stormlist('it:dev:str | stats.countby --by-name --reverse')
+            self.stormIsInPrint(chartmixed_rev_byname, msgs)
+
     async def test_stormlib_stats_tally(self):
 
         async with self.getTestCore() as core: