GID improvements (#32)

* Better handling of corner cases for GID

* Up version

* Add future package

Author: dkarchmer

CHANGELOG.md

@@ -1,3 +1,8 @@
+### v0.8.8 (2018-03-02)
+
+  * Better testing of IOTileCloudSlug corner cases
+  * Add `future` package to make utils.gid python2 friendly
+
 ### v0.8.7 (2018-03-02)
 
   * Ensure IOTileDeviceSlug only considers 48 bits when converting formatted ids

iotile_cloud/utils/gid.py

@@ -1,3 +1,4 @@
+from builtins import int
 
 int16gid = lambda n: '-'.join(['{:04x}'.format(n >> (i << 4) & 0xFFFF) for i in range(0, 1)[::-1]])
 int32gid = lambda n: '-'.join(['{:04x}'.format(n >> (i << 4) & 0xFFFF) for i in range(0, 2)[::-1]])
@@ -33,7 +34,11 @@ def fix_gid(gid, num_terms):
 def gid2int(gid):
     elements = gid.split('-')
     hex_value = ''.join(elements)
-    return int(hex_value, 16)
+    try:
+        id = int(hex_value, 16)
+    except Exception:
+        raise ValueError('Expected HEX number. Got {}'.format(hex_value))
+    return id
 
 
 class IOTileCloudSlug(object):
@@ -47,8 +52,10 @@ def formatted_id(self):
         return gid_join(parts[1:])
 
     def set_from_single_id_slug(self, type, terms, id):
-        assert(type in ['p', 'd', 'b', 'g'])
-        assert (isinstance(id, str))
+        if type not in ['p', 'd', 'b', 'g']:
+            raise ValueError('Slugs must start with p/d/b/g')
+        if not isinstance(id, str):
+            raise ValueError('Slug must be a string')
         parts = gid_split(id)
         if parts[0] in ['p', 'd', 'b', 'g']:
             id = parts[1]
@@ -57,8 +64,10 @@ def set_from_single_id_slug(self, type, terms, id):
 
     def get_id(self):
         parts = gid_split(self._slug)
-        assert(len(parts) == 2)
-        assert(parts[0] in ['p', 'd', 'g'])
+        if len(parts) != 2:
+            raise ValueError('Cannot call get_id() for IDs with more than one term')
+        if parts[0] not in ['p', 'd', 'g']:
+            raise ValueError('Only Devices/DataBlocks/Fleets have single IDs')
         return gid2int(parts[1])
 
 
@@ -67,6 +76,8 @@ class IOTileProjectSlug(IOTileCloudSlug):
 
     def __init__(self, id):
         if isinstance(id, int):
+            if id < 0:
+                raise ValueError('IOTileProjectSlug: UUID should be greater or equal than zero')
             pid = int2pid(id)
         else:
             pid = id
@@ -82,17 +93,24 @@ def __init__(self, id):
             return
 
         if isinstance(id, int):
+            if id <= 0:
+                raise ValueError('IOTileDeviceSlug: UUID should be greater than zero')
             did = int2did(id)
         else:
-            assert isinstance(id, str)
+            if not isinstance(id, str):
+                raise ValueError('IOTileDeviceSlug: must be an int or str')
             parts = gid_split(id)
             if len(parts) == 1:
                 did = parts[0]
             else:
+                if parts[0] != 'd':
+                    raise ValueError('IOTileDeviceSlug: must start with a "d"')
                 did = gid_join(parts[1:])
 
             # Convert to int and back to get rid of anything above 48 bits
             id = gid2int(did)
+            if id <= 0:
+                raise ValueError('IOTileDeviceSlug: UUID should be greater than zero')
             did = int2did(id)
 
         self.set_from_single_id_slug('d', 4, did)
@@ -115,34 +133,45 @@ class IOTileBlockSlug(IOTileCloudSlug):
 
     def __init__(self, id, block=0):
         if isinstance(id, int):
+            if id <= 0:
+                raise ValueError('IOTileBlockSlug: UUID should be greater than zero')
             did = int2did(id)
         else:
             parts = gid_split(id)
             if(len(parts) == 1):
+                # gid2int will raise exception if not a proper HEX string
+                id = gid2int(parts[0])
+                if id <= 0:
+                    raise ValueError('IOTileBlockSlug: UUID should be greater than zero')
                 parts = ['d',] + parts
-            assert(parts[0] in ['d', 'b'])
+            if parts[0] not in ['d', 'b']:
+                raise ValueError('IOTileBlockSlug: Slug must start with "b" or "d"')
             id_parts = parts[1].split('-')
             if parts[0] == 'b':
-                assert(len(id_parts) == 4)
+                if len(id_parts) != 4:
+                    raise ValueError('IOTileBlockSlug: Expected format: b--xxxx-xxxx-xxxx-xxxx')
                 self._slug = id
                 self._block = id_parts[0]
                 return
             if len(id_parts) == 4:
                 self._block = id_parts[0]
                 did = '-'.join(id_parts[1:])
             else:
-                assert(parts[0] == 'd')
+                if parts[0] != 'd':
+                    raise ValueError('DataBlock Slug must start with "b" or "d"')
                 did = '-'.join(id_parts[0:])
         did = fix_gid(did, 3)
         if not self._block:
             self._block = int2bid(block)
         self.set_from_single_id_slug('b', 4, '-'.join([self._block, did]))
 
+
     def get_id(self):
         # DataBlocks should behave like Devices
         # get_id returns the device ID
         parts = gid_split(self._slug)
-        assert(len(parts) == 2)
+        if len(parts) != 2:
+            raise ValueError('IOTileBlockSlug: Cannot call get_id() for IDs with more than one term')
         id_parts = parts[1].split('-')
         hex_value = ''.join(id_parts[1:])
         return int(hex_value, 16)
@@ -167,7 +196,7 @@ def __init__(self, device, index):
         elif isinstance(device, str):
             device_id = IOTileDeviceSlug(device).get_id()
         else:
-            raise ValueError("Unknown device specifier, must be string, int or IOTileDeviceSlug")
+            raise ValueError("IOTileStreamerSlug: Unknown device specifier, must be string, int or IOTileDeviceSlug")
 
         index = int(index)
 
@@ -194,26 +223,33 @@ class IOTileVariableSlug(IOTileCloudSlug):
     # Store local variable ID on top of globally unique slug
     _local = None
 
-    def __init__(self, id, project=None):
+    def __init__(self, id, project=IOTileProjectSlug(0)):
         """
 
         :param id: Variable Local Id (string or int)
-        :param project: IOTileCProjectSlug instance
+        :param project: IOTileCProjectSlug instance. Defaults to zero, which represent a wildcard
         """
         if project:
             if not isinstance(project, IOTileProjectSlug):
                 project = IOTileProjectSlug(project)
-        if isinstance(id, int) and project != None:
+
+        if isinstance(id, int):
+            if id <= 0:
+                raise ValueError('IOTileVariableSlug: UUID should be greater than zero')
             vid = int2vid(id)
             self._slug = gid_join(['v', project.formatted_id(), vid])
         else:
-            assert (isinstance(id, str))
+            if not isinstance(id, str):
+                raise ValueError("IOTileVariableSlug: ID must be int or str")
+
             parts = gid_split(id)
-            if len(parts) == 1 and project != None:
+            if len(parts) == 1:
+                # gid2int will raise exception if not a proper HEX string
+                gid2int(id)
                 self._slug = gid_join(['v', project.formatted_id(), id])
             else:
-                assert(project == None)
-                assert(len(parts) == 3)
+                if len(parts) != 3:
+                    raise ValueError("IOTileVariableSlug: Expected format: v--xxxx-xxxx--xxxx")
                 self._slug = id
         self._local = gid_split(self._slug)[2]
 
@@ -225,8 +261,10 @@ class IOTileStreamSlug(IOTileCloudSlug):
 
     def __init__(self, id=None):
         if id:
-            assert(isinstance(id, str))
-            assert(len(gid_split(id)) == 4)
+            if not isinstance(id, str):
+                raise ValueError("Variable ID must be int or str")
+            if len(gid_split(id)) != 4:
+                raise ValueError("Stream slug must have three terms: s--<prj>--<dev>--<var>")
             self._slug = id
 
     def from_parts(self, project, device, variable):

requirements-test.txt

@@ -7,3 +7,4 @@ coverage==3.7.1
 coveralls==1.1
 mock==2.0.0
 requests-mock==1.3.0
+future==0.16.0

setup.py

@@ -18,6 +18,7 @@
         'pytest11': ['mock_cloud = iotile_cloud.utils.mock_cloud']
     },
     install_requires=[
+        'future',
         'requests',
         'python-dateutil'
     ],

tests/test_gid.py

@@ -50,13 +50,19 @@ def test_device_slug(self):
         self.assertEqual(str(id), 'd--0000-0000-0000-0001')
         self.assertEqual(id.get_id(), 1)
 
+        self.assertRaises(ValueError, IOTileDeviceSlug, 'string')
+        self.assertRaises(ValueError, IOTileDeviceSlug, 'x--0000-0000-0000-0001')
+        self.assertRaises(ValueError, IOTileDeviceSlug, '0000-0000-0000-0000')
+        self.assertRaises(ValueError, IOTileDeviceSlug, -5)
+        self.assertRaises(ValueError, IOTileDeviceSlug, 0)
+
     def test_block_slug(self):
         id = IOTileBlockSlug(5)
         self.assertEqual(str(id), 'b--0000-0000-0000-0005')
         id = IOTileBlockSlug(0xa)
         self.assertEqual(str(id), 'b--0000-0000-0000-000a')
 
-        self.assertRaises(AssertionError, IOTileBlockSlug, 'b--0000-0000-1234')
+        self.assertRaises(ValueError, IOTileBlockSlug, 'b--0000-0000-1234')
 
         id = IOTileBlockSlug('b--0001-0000-0000-1234')
         self.assertEqual(str(id), 'b--0001-0000-0000-1234')
@@ -74,13 +80,23 @@ def test_block_slug(self):
 
         self.assertEqual(id.formatted_id(), '0003-0000-0000-0005')
 
+        self.assertRaises(ValueError, IOTileBlockSlug, 'string')
+        self.assertRaises(ValueError, IOTileBlockSlug, 'x--0000-0000-0000-0001')
+        self.assertRaises(ValueError, IOTileBlockSlug, '0000-0000-0000-0000')
+        self.assertRaises(ValueError, IOTileBlockSlug, -5)
+        self.assertRaises(ValueError, IOTileBlockSlug, 0)
+
     def test_variable_slug(self):
-        self.assertRaises(AssertionError, IOTileVariableSlug, 5)
+        self.assertRaises(ValueError, IOTileVariableSlug, 'foo')
 
         id = IOTileVariableSlug(5, '1234')
         self.assertEqual(str(id), 'v--0000-1234--0005')
         self.assertEqual(id.formatted_local_id(), '0005')
 
+        id = IOTileVariableSlug(5)
+        self.assertEqual(str(id), 'v--0000-0000--0005')
+        self.assertEqual(id.formatted_local_id(), '0005')
+
         id = IOTileVariableSlug(5, IOTileProjectSlug('1234'))
         self.assertEqual(str(id), 'v--0000-1234--0005')
         self.assertEqual(id.formatted_local_id(), '0005')
@@ -91,9 +107,12 @@ def test_variable_slug(self):
         id = IOTileVariableSlug('v--0000-1234--0005')
         self.assertEqual(str(id), 'v--0000-1234--0005')
 
+        self.assertRaises(ValueError, IOTileVariableSlug, -5)
+        self.assertRaises(ValueError, IOTileVariableSlug, 0)
+
     def test_stream_slug(self):
-        self.assertRaises(AssertionError, IOTileStreamSlug, 5)
-        self.assertRaises(AssertionError, IOTileStreamSlug, 's--0001')
+        self.assertRaises(ValueError, IOTileStreamSlug, 5)
+        self.assertRaises(ValueError, IOTileStreamSlug, 's--0001')
 
         id = IOTileStreamSlug('s--0000-0001--0000-0000-0000-0002--0003')
         self.assertEqual(str(id), 's--0000-0001--0000-0000-0000-0002--0003')
@@ -135,6 +154,22 @@ def test_stream_from_parts(self):
         id.from_parts(project=7, device=1, variable=vslug)
         self.assertEqual(str(id), 's--0000-0007--0000-0000-0000-0001--5002')
 
+        id = IOTileStreamSlug()
+        id.from_parts(project=7, device=1, variable='5002')
+        self.assertEqual(str(id), 's--0000-0007--0000-0000-0000-0001--5002')
+
+        # Project is the only one that can be zero (wildcard)
+        id = IOTileStreamSlug()
+        id.from_parts(project=0, device=1, variable='5002')
+        self.assertEqual(str(id), 's--0000-0000--0000-0000-0000-0001--5002')
+
+        id = IOTileStreamSlug()
+        with pytest.raises(ValueError):
+            id.from_parts(project=-1, device=1, variable='5002')
+        with pytest.raises(ValueError):
+            id.from_parts(project=1, device=-1, variable='5002')
+        with pytest.raises(ValueError):
+            id.from_parts(project=1, device=1, variable=-1)
 
     def test_id_property(self):
         project = IOTileProjectSlug(5)
@@ -145,32 +180,30 @@ def test_id_property(self):
 
         id = IOTileStreamSlug()
         id.from_parts(project=project, device=device, variable=variable)
-        self.assertRaises(AssertionError, id.get_id)
-
-
-def test_streamer_gid():
-    """Ensure that IOTileStreamerSlug works."""
+        self.assertRaises(ValueError, id.get_id)
 
-    s_gid = IOTileStreamerSlug(1, 2)
-    assert str(s_gid) == "t--0000-0000-0000-0001--0002"
-    assert s_gid.get_device() == "d--0000-0000-0000-0001"
-    assert s_gid.get_index() == "0002"
+    def test_streamer_gid(self):
+        """Ensure that IOTileStreamerSlug works."""
 
-    s_gid = IOTileStreamerSlug("d--0000-0000-0000-1234", 1)
-    assert str(s_gid) == "t--0000-0000-0000-1234--0001"
+        s_gid = IOTileStreamerSlug(1, 2)
+        assert str(s_gid) == "t--0000-0000-0000-0001--0002"
+        assert s_gid.get_device() == "d--0000-0000-0000-0001"
+        assert s_gid.get_index() == "0002"
 
-    with pytest.raises(ValueError):
-        IOTileStreamerSlug([], 1)
+        s_gid = IOTileStreamerSlug("d--0000-0000-0000-1234", 1)
+        assert str(s_gid) == "t--0000-0000-0000-1234--0001"
 
-    d_gid = IOTileDeviceSlug(15)
-    s_gid = IOTileStreamerSlug(d_gid, 0)
-    assert str(s_gid) == "t--0000-0000-0000-000f--0000"
-    assert s_gid.get_device() == str(d_gid)
-    assert s_gid.get_index() == "0000"
+        with pytest.raises(ValueError):
+            IOTileStreamerSlug([], 1)
 
+        d_gid = IOTileDeviceSlug(15)
+        s_gid = IOTileStreamerSlug(d_gid, 0)
+        assert str(s_gid) == "t--0000-0000-0000-000f--0000"
+        assert s_gid.get_device() == str(d_gid)
+        assert s_gid.get_index() == "0000"
 
-def test_fleet_gid():
-    """Ensure that IOTileFleetSlug works."""
+    def test_fleet_gid(self):
+        """Ensure that IOTileFleetSlug works."""
 
-    f_gid = IOTileFleetSlug(1)
-    assert str(f_gid) == 'g--0000-0000-0001'
+        f_gid = IOTileFleetSlug(1)
+        assert str(f_gid) == 'g--0000-0000-0001'

version.py

@@ -1 +1 @@
-version = '0.8.7'
+version = '0.8.8'