Merge pull request #6 from iotile/fix-malformed-slugs

Fix slug expansion

RELEASE.md

@@ -2,6 +2,10 @@
 
 All major changes in each released version of the archfx-cloud plugin are listed here.
 
+## 0.10.3
+
+- Fix bug where slug expansion produces malformed slugs
+
 ## 0.10.2
 
 - Remove mock_cloud until a proper ArchFX Cloud Mock is created

archfx_cloud/utils/convert.py

@@ -1,25 +1,23 @@
 # pylint: disable=invalid-name
 def int16gid(n):
-    return '-'.join(['{:04x}'.format(n >> (i << 4) & 0xFFFF) for i in range(0, 1)[::-1]])
+    return f"{n&0xFFFF:04x}"[-4:]
 
 
 def int32gid(n):
-    return '-'.join(['{:04x}'.format(n >> (i << 4) & 0xFFFF) for i in range(0, 2)[::-1]])
+    return f"{n&0xFFFFFFFF:09_x}".replace("_", "-")
 
 
 def int48gid(n):
-    return '-'.join(['{:04x}'.format(n >> (i << 4) & 0xFFFF) for i in range(0, 3)[::-1]])
+    return f"{n&0xFFFFFFFFFFFF:014_x}".replace("_", "-")
 
 
 def int64gid(n):
-    return '-'.join(['{:04x}'.format(n >> (i << 4) & 0xFFFF) for i in range(0, 4)[::-1]])
+    return f"{n&0xFFFFFFFFFFFFFFFF:019_x}".replace("_", "-")
 
 
-def int2bid(n):
-    return int16gid(n)
 # pylint: enable=invalid-name
 
-
+int2bid = int16gid
 int2did = int64gid
 int2did_short = int48gid
 int2fleet_id = int48gid
@@ -43,7 +41,7 @@ def fix_gid(gid, num_terms):
         # Only keep right most terms
         elements = elements[(len(elements) - num_terms):]
 
-    return'-'.join(elements)
+    return '-'.join(elements)
 
 
 def formatted_dbid(bid, did):

archfx_cloud/utils/slugs.py

@@ -14,6 +14,7 @@
     gid2int,
 )
 
+
 def slugify(value: str, allow_unicode: bool = False) -> str:
     """
     Convert to ASCII if 'allow_unicode' is False. Convert spaces to hyphens.
@@ -57,7 +58,7 @@ def get_id(self):
         parts = gid_split(self._slug)
         if len(parts) != 2:
             raise ValueError('Cannot call get_id() for IDs with more than one term')
-        if parts[0] not in ['ps', 'pa', 'pl', 'd',]:
+        if parts[0] not in ['ps', 'pa', 'pl', 'd', ]:
             raise ValueError('Only Devices/DataBlocks/Fleets have single IDs')
         return gid2int(parts[1])
 
@@ -113,13 +114,7 @@ def __init__(self, id, allow_64bits=True):
             self._slug = id._slug
             return
 
-        if isinstance(id, int):
-            if id < 0 or id >= pow(16, hex_count):
-                raise ValueError('ArchFxDeviceSlug: UUID should be greater or equal than zero and less than 16^12')
-            did = int2did(id)
-        else:
-            if not isinstance(id, str):
-                raise ValueError('ArchFxDeviceSlug: must be an int or str')
+        if isinstance(id, str):
             parts = gid_split(id)
             if len(parts) == 1:
                 did = parts[0]
@@ -128,12 +123,15 @@ def __init__(self, id, allow_64bits=True):
                     raise ValueError('ArchFxDeviceSlug: must start with a "d" or "m"')
                 did = gid_join(parts[1:])
 
-            # Convert to int and back to get rid of anything above 48 bits
-            id = gid2int(did)
-            if id < 0 or id >= pow(16, hex_count):
-                raise ValueError('ArchFxDeviceSlug: UUID should be greater or equal than zero and less than 16^12')
+            id = gid2int(did)  # Canonicalize to int
+
+        if not isinstance(id, int):
+            raise ValueError(f"ArchFxDeviceSlug: not convertible from {type(id)}")
+
+        if id < 0 or id >= pow(16, hex_count):
+            raise ValueError('ArchFxDeviceSlug: UUID should be greater or equal than zero and less than 16^12')
 
-        self.set_from_single_id_slug('d', 4, did)
+        self.set_from_single_id_slug('d', 4, int2did(id))
 
 
 class ArchFxVariableID(ArchFxCloudSlug):
@@ -167,28 +165,24 @@ def __init__(self, id_):
                     raise ValueError('ArchFxVariableID: Var should be greater or equal than zero and less than 16^4')
                 var = int16gid(var)
 
-            id_ = '-'.join([scope, var])
+            id_ = f"{scope}-{var}"
 
-        if isinstance(id_, int):
-            if id_ < 0 or id_ >= pow(16, 8):
-                raise ValueError('ArchFxVariableID: ID should be greater or equal than zero and less than 16^8')
-            vid = int2vid(id_)
-        else:
-            if not isinstance(id_, str):
-                raise ValueError('ArchFxVariableID: must be an int or str: {}'.format(type(id_)))
+        if isinstance(id_, str):
             parts = gid_split(id_)
             if len(parts) == 1:
                 vid = parts[0]
             else:
                 raise ValueError('ArchFxVariableID: must start with a digit')
 
-            # Convert to int and back to get rid of anything above 32 bits
-            int_id = gid2int(vid)
-            if int_id < 0 or int_id >= pow(16, 8):
-                raise ValueError('ArchFxVariableID: ID should be greater or equal than zero and less than 16^8')
-            vid = int2vid(int_id)
+            id_ = gid2int(vid)  # Canonicalize to int
+
+        if not isinstance(id_, int):
+            raise ValueError(f"ArchFxDeviceSlug: not convertible from {type(id_)}")
 
-        self.set_from_single_id_slug(None, None, vid)
+        if id_ < 0 or id_ >= pow(16, 8):
+            raise ValueError('ArchFxVariableID: ID should be greater or equal than zero and less than 16^8')
+
+        self.set_from_single_id_slug(None, None, int2vid(id_))
 
     def formatted_id(self):
         """Formatted ID is the same as a Slug for a VariableID"""
@@ -244,7 +238,7 @@ class ArchFxStreamSlug(ArchFxCloudSlug):
         'pa': 'sa',
         'ps': 'ss'
     }
-    
+
     def __init__(self, sid=None):
         if not sid:
             self.stype = 'sd'

tests/test_slugs.py

@@ -1,5 +1,6 @@
 import unittest2 as unittest
 import datetime
+import pytest
 
 from archfx_cloud.utils.slugs import (
     ArchFxParentSlug,
@@ -9,6 +10,29 @@
     ArchFxStreamerSlug
 )
 
+ArchFxVariableID_CASES = (
+    ("int-no-scope", 0x5051, '0000-5051', 0, 0x5051),
+    ("int-with-scope", 0x10000 | 0x5051, '0001-5051', 1, 0x5051),
+    ("string-no-scope", '5051', '0000-5051', 0, 0x5051),
+    ("scoped-string", '0001-5051', '0001-5051', 1, 0x5051),
+    ("scoped-string-no-delimiter", 'f5051', '000f-5051', 0x0f, 0x5051),
+    ("int-tuple", (1, 0x5051), '0001-5051', 1, 0x5051),
+    ("string-tuple", ("1", "5051"), '0001-5051', 1, 0x5051),
+)
+ArchFxVariableID_NAMES = [x[0] for x in ArchFxVariableID_CASES]
+
+
+@pytest.mark.parametrize("name, input, formatted, scope, var", ArchFxVariableID_CASES, ids=ArchFxVariableID_NAMES)
+def test_ArchFxVariableID(name, input, formatted, scope, var):
+    result = ArchFxVariableID(input)
+    assert str(result) == formatted
+    assert result.formatted_id() == formatted
+    assert result.get_id() == (scope << 16) + var
+    assert result.var_id == var
+    assert result.var_hex == f"{var:04x}"
+    assert result.scope == scope
+    assert result.scope_hex == f"{scope:04x}"
+
 
 class SlugTestCase(unittest.TestCase):
 
@@ -28,17 +52,19 @@ def test_parent_slug(self):
         id = ArchFxParentSlug('pl--1234')
         self.assertEqual(str(id), 'pl--0000-1234')
 
+        id = ArchFxParentSlug('pl--1')
+        self.assertEqual(str(id), 'pl--0000-0001')
+
         id = ArchFxParentSlug('0005')
         self.assertEqual(str(id), 'pl--0000-0005')
 
         self.assertEqual(id.formatted_id(), '0000-0005')
 
         self.assertRaises(ValueError, ArchFxParentSlug, 'string')
         self.assertRaises(ValueError, ArchFxParentSlug, 'x--0000-0001')
-        self.assertRaises(ValueError, ArchFxParentSlug, 'p--1234-0000-0000-0001') # > 16bts
+        self.assertRaises(ValueError, ArchFxParentSlug, 'p--1234-0000-0000-0001')  # > 16bts
         self.assertRaises(ValueError, ArchFxParentSlug, -5)
-        self.assertRaises(ValueError, ArchFxParentSlug, pow(16,8))
-
+        self.assertRaises(ValueError, ArchFxParentSlug, pow(16, 8))
 
     def test_device_slug(self):
         id = ArchFxDeviceSlug(0)
@@ -56,6 +82,9 @@ def test_device_slug(self):
         id = ArchFxDeviceSlug(id)
         self.assertEqual(str(id), 'd--0000-0000-0000-1234')
 
+        id = ArchFxDeviceSlug('d--1')
+        self.assertEqual('d--0000-0000-0000-0001', str(id))
+
         id = ArchFxDeviceSlug('d--1234')
         self.assertEqual(str(id), 'd--0000-0000-0000-1234')
 
@@ -74,51 +103,11 @@ def test_device_slug(self):
 
         self.assertRaises(ValueError, ArchFxDeviceSlug, 'string')
         self.assertRaises(ValueError, ArchFxDeviceSlug, 'x--0000-0000-0000-0001')
-        self.assertRaises(ValueError, ArchFxDeviceSlug, 'd--1234-0000-0000-0001', False) # > 48bts
+        self.assertRaises(ValueError, ArchFxDeviceSlug, 'd--1234-0000-0000-0001', False)  # > 48bts
+        self.assertRaises(ValueError, ArchFxDeviceSlug, b'binary string')
         self.assertRaises(ValueError, ArchFxDeviceSlug, -5)
-        self.assertRaises(ValueError, ArchFxDeviceSlug, pow(16,16))
-        self.assertRaises(ValueError, ArchFxDeviceSlug, pow(16,12), False)
-
-    def test_variable_slugs(self):
-
-        var1 = ArchFxVariableID('5051')
-        self.assertEqual(str(var1), '0000-5051')
-        self.assertEqual(var1.formatted_id(), '0000-5051')
-        self.assertEqual(var1.get_id(), 0x5051)
-        self.assertEqual(var1.var_id, 0x5051)
-        self.assertEqual(var1.var_hex, '5051')
-        self.assertEqual(var1.scope, 0)
-
-        var2 = ArchFxVariableID(0x5051)
-        self.assertEqual(str(var2), '0000-5051')
-        self.assertEqual(var2.formatted_id(), '0000-5051')
-
-        var3 = ArchFxVariableID(0x10000 | 0x5051)
-        self.assertEqual(str(var3), '0001-5051')
-        self.assertEqual(var3.formatted_id(), '0001-5051')
-        self.assertEqual(var3.get_id(), 0x15051)
-        self.assertEqual(var3.var_hex, '5051')
-        self.assertEqual(var3.scope_hex, '0001')
-        self.assertEqual(var3.var_id, 0x5051)
-        self.assertEqual(var3.scope, 1)
-
-        var4 = ArchFxVariableID('0001-5051')
-        self.assertEqual(str(var4), '0001-5051')
-        self.assertEqual(var4.formatted_id(), '0001-5051')
-        self.assertEqual(var4.get_id(), 0x15051)
-        self.assertEqual(var4.var_hex, '5051')
-        self.assertEqual(var4.scope_hex, '0001')
-        self.assertEqual(var4.var_id, 0x5051)
-        self.assertEqual(var4.scope, 1)
-
-        var5 = ArchFxVariableID('f5051')
-        self.assertEqual(str(var5), '000f-5051')
-        self.assertEqual(var5.formatted_id(), '000f-5051')
-        self.assertEqual(var5.get_id(), 0xF5051)
-        self.assertEqual(var5.var_hex, '5051')
-        self.assertEqual(var5.scope_hex, '000f')
-        self.assertEqual(var5.var_id, 0x5051)
-        self.assertEqual(var5.scope, 0xF)
+        self.assertRaises(ValueError, ArchFxDeviceSlug, pow(16, 16))
+        self.assertRaises(ValueError, ArchFxDeviceSlug, pow(16, 12), False)
 
     def test_stream_slug(self):
         slug1 = ArchFxStreamSlug('sl--0000-0001--0000-0000-0000-0002--5051')

version.py

@@ -1 +1 @@
-version = '0.10.2'
+version = '0.10.3'