Merge pull request #98 from CameronJHall/master

Guaranteed order in deduplication dictionary hashes

Author: James Bell

setup.py

@@ -4,7 +4,7 @@
 
 setup(
     name='tgt_grease',
-    version='2.4.1',
+    version='2.5.0',
     license="MIT",
     description='Modern distributed automation engine built with love by Target',
     long_description="""

tgt_grease/enterprise/Model/DeDuplication.py

@@ -234,39 +234,37 @@ def deduplicate_object(ioc, obj, expiry, expiry_max, threshold, source_name, con
                 }
             )
             return
-        else:
-            # T1 Not Found Protocol: We have a possibly unique object
-            ioc.getLogger().debug("Type1 Match not found; Beginning type 2 processing")
-            # Create a T1
-            T1ObjectId = DeDupCollection.insert_one({
-                'expiry': Deduplication.generate_expiry_time(int(expiry)),
-                'grease_internal_configuration': configuration_name,
-                'max_expiry': Deduplication.generate_max_expiry_time(int(expiry_max)),
-                'type': 1,
-                'score': 1,
-                'source': str(source_name),
-                'hash': Deduplication.generate_hash_from_obj(t1test)
-            }).inserted_id
-            # Begin T2 Deduplication
-            compositeScore = Deduplication.object_field_score(
-                collection, ioc, source_name, configuration_name, obj, str(T1ObjectId), expiry, expiry_max, field_set
+        # T1 Not Found Protocol: We have a possibly unique object
+        ioc.getLogger().debug("Type1 Match not found; Beginning type 2 processing")
+        # Create a T1
+        T1ObjectId = DeDupCollection.insert_one({
+            'expiry': Deduplication.generate_expiry_time(int(expiry)),
+            'grease_internal_configuration': configuration_name,
+            'max_expiry': Deduplication.generate_max_expiry_time(int(expiry_max)),
+            'type': 1,
+            'score': 1,
+            'source': str(source_name),
+            'hash': Deduplication.generate_hash_from_obj(t1test)
+        }).inserted_id
+        # Begin T2 Deduplication
+        compositeScore = Deduplication.object_field_score(
+            collection, ioc, source_name, configuration_name, obj, str(T1ObjectId), expiry, expiry_max, field_set
+        )
+        if compositeScore < threshold:
+            # unique obj
+            ioc.getLogger().trace(
+                "Unique object! Composite score was: [{0}] threashold: [{1}]".format(compositeScore, threshold),
+                verbose=True
             )
-            if compositeScore < threshold:
-                # unique obj
-                ioc.getLogger().trace(
-                    "Unique object! Composite score was: [{0}] threashold: [{1}]".format(compositeScore, threshold),
-                    verbose=True
-                )
-                final.append(obj)
-                return
-            else:
-                # likely duplicate value
-                ioc.getLogger().trace(
-                    "Object surpassed threshold, suspected to be duplicate! "
-                    "Composite score was: [{0}] threashold: [{1}]".format(compositeScore, threshold),
-                    verbose=True
-                )
-                return
+            final.append(obj)
+            return
+        # likely duplicate value
+        ioc.getLogger().trace(
+            "Object surpassed threshold, suspected to be duplicate! "
+            "Composite score was: [{0}] threashold: [{1}]".format(compositeScore, threshold),
+            verbose=True
+        )
+        return
 
     @staticmethod
     def object_field_score(collection, ioc, source_name, configuration_name, obj, objectId, expiry, max_expiry, field_set=None):
@@ -361,8 +359,46 @@ def object_field_score(collection, ioc, source_name, configuration_name, obj, ob
                 continue
         if len(field_scores) is 0:
             return 0.0
-        else:
-            return float(sum(field_scores) / float(len(field_scores)))
+        return float(sum(field_scores) / float(len(field_scores)))
+
+    @staticmethod
+    def make_hashable(obj):
+        """Takes a dictionary and makes a sorted tuple of strings representing flattened key value pairs
+        Args:
+            obj (dict): A dictionary
+        Returns:
+            tuple<str>: a sorted flattened tuple of the dictionary's key value pairs
+
+        Example:
+            {
+                "a": ["test1", "test2"],
+                "b": [{"test2": 21}, {"test1": 1}, {"test7": 3}],
+                "c": "test"
+            }
+            becomes...
+            (('a', ('test1', 'test2')),
+             ('b', ((('test1', 1),), (('test2', 21),), (('test7', 3),))),
+             ('c', 'test'))
+        """
+        final = []
+        sorted_tuples = Deduplication.make_hashable_helper(obj)
+        for pair in sorted_tuples:
+            final.append(pair)
+        return tuple(final)
+
+    @staticmethod
+    def make_hashable_helper(obj):
+        """Recursively turns iterables into sorted tuples"""
+        if isinstance(obj, (tuple, list)):
+            return tuple(sorted(Deduplication.make_hashable_helper(e) for e in obj))
+
+        if isinstance(obj, dict):
+            return tuple(sorted((k, Deduplication.make_hashable_helper(v)) for k, v in obj.items()))
+
+        if isinstance(obj, (set, frozenset)):
+            return tuple(sorted(Deduplication.make_hashable_helper(e) for e in obj))
+
+        return obj
 
     @staticmethod
     def generate_hash_from_obj(obj):
@@ -375,7 +411,7 @@ def generate_hash_from_obj(obj):
             str: Object Hash
 
         """
-        return hashlib.sha256(str(obj).encode('utf-8')).hexdigest()
+        return hashlib.sha256(repr(Deduplication.make_hashable(obj)).encode('utf-8')).hexdigest()
 
     @staticmethod
     def generate_expiry_time(hours):

tgt_grease/enterprise/Model/tests/test_deduplication.py

@@ -18,51 +18,70 @@ def test_generate_expiry_time(self):
         )
 
     def test_generate_max_expiry_time(self):
-            print(str(Deduplication.generate_max_expiry_time(7)))
-            print(str(datetime.datetime.utcnow() + datetime.timedelta(days=7)))
-            self.assertTrue(
-                Deduplication.generate_max_expiry_time(7).day == (datetime.datetime.utcnow() + datetime.timedelta(days=7)).day
-            )
+        print(str(Deduplication.generate_max_expiry_time(7)))
+        print(str(datetime.datetime.utcnow() + datetime.timedelta(days=7)))
+        self.assertTrue(
+            Deduplication.generate_max_expiry_time(7).day == (datetime.datetime.utcnow() + datetime.timedelta(days=7)).day
+        )
+
+    def test_make_hashable_helper(self):
+        obj1 = {'test1': 1, 'test2': 2, 'test3': 3}
+        obj2 = {'test3': 3, 'test1': 1, 'test2': 2}
+        self.assertEqual(
+            Deduplication.make_hashable_helper(obj1),
+            (('test1', 1), ('test2', 2), ('test3', 3))
+        )
+        self.assertEqual(
+            Deduplication.make_hashable_helper(obj1),
+            Deduplication.make_hashable_helper(obj2)
+        )
+
+    def test_make_hashable(self):
+        obj = {'test1': 'var', 'test2': 5, 'test3': ['1', '3', '2']}
+        self.assertEqual(
+            Deduplication.make_hashable_helper(obj),
+            (('test1', 'var'), ('test2', 5), ('test3', ('1', '2', '3')))
+        )
 
     def test_generate_hash(self):
         obj = {'test': 'var', 'test1': 5, 'test2': 7.89}
         self.assertEqual(
             Deduplication.generate_hash_from_obj(obj),
-            hashlib.sha256(str(obj).encode('utf-8')).hexdigest()
+            hashlib.sha256(repr(Deduplication.make_hashable(obj)).encode('utf-8')).hexdigest()
         )
 
     def test_generate_hash_multi_str_type(self):
         obj = {'test': u'var', 'test1': 5, 'test2': 7.89, 'test3': 'ver'}
         self.assertEqual(
             Deduplication.generate_hash_from_obj(obj),
-            hashlib.sha256(str(obj).encode('utf-8')).hexdigest()
+            hashlib.sha256(repr(Deduplication.make_hashable(obj)).encode('utf-8')).hexdigest()
         )
 
     def test_generate_hash_other_type(self):
-        obj = 7
+        obj = {'test': 7}
         self.assertEqual(
             Deduplication.generate_hash_from_obj(obj),
-            hashlib.sha256(str(obj).encode('utf-8')).hexdigest()
+            hashlib.sha256(repr(Deduplication.make_hashable(obj)).encode('utf-8')).hexdigest()
         )
-        obj = 'test'
+        obj = {'test': 'test'}
         self.assertEqual(
             Deduplication.generate_hash_from_obj(obj),
-            hashlib.sha256(str(obj).encode('utf-8')).hexdigest()
+            hashlib.sha256(repr(Deduplication.make_hashable(obj)).encode('utf-8')).hexdigest()
         )
-        obj = u'test'
+        obj = {'test': u'test'}
         self.assertEqual(
             Deduplication.generate_hash_from_obj(obj),
-            hashlib.sha256(str(obj).encode('utf-8')).hexdigest()
+            hashlib.sha256(repr(Deduplication.make_hashable(obj)).encode('utf-8')).hexdigest()
         )
-        obj = 7.8
+        obj = {'test': 7.8}
         self.assertEqual(
             Deduplication.generate_hash_from_obj(obj),
-            hashlib.sha256(str(obj).encode('utf-8')).hexdigest()
+            hashlib.sha256(repr(Deduplication.make_hashable(obj)).encode('utf-8')).hexdigest()
         )
-        obj = ['test', 'var', 8, 8.43]
+        obj = {'test': ['test', 'var', '8', '8.43']}
         self.assertEqual(
             Deduplication.generate_hash_from_obj(obj),
-            hashlib.sha256(str(obj).encode('utf-8')).hexdigest()
+            hashlib.sha256(repr(Deduplication.make_hashable(obj)).encode('utf-8')).hexdigest()
         )
 
     def test_object_score_low_duplication(self):

tgt_grease/management/Model/bridge.py

@@ -36,10 +36,9 @@ def action_register(self):
             print("Registration Complete!")
             self.ioc.getLogger().info("Registration Completed Successfully")
             return True
-        else:
-            print("Registration Failed!")
-            self.ioc.getLogger().info("Registration Failed")
-            return False
+        print("Registration Failed!")
+        self.ioc.getLogger().info("Registration Failed")
+        return False
 
     def action_info(self, node=None, jobs=None, prototypeJobs=None):
         """Gets Node Information
@@ -66,8 +65,8 @@ def action_info(self, node=None, jobs=None, prototypeJobs=None):
             return False
         valid, serverId = self.valid_server(node)
         if not valid:
-                print("Invalid ObjectID")
-                return False
+            print("Invalid ObjectID")
+            return False
         server = self.ioc.getCollection('JobServer').find_one({'_id': ObjectId(str(serverId))})
         if server:
             server = dict(server)
@@ -148,10 +147,9 @@ def action_info(self, node=None, jobs=None, prototypeJobs=None):
                         job['grease_data']['execution']['returnData'])
                     )
             return True
-        else:
-            print("Unable to locate server")
-            self.ioc.getLogger().error("Unable to load [{0}] server for information".format(serverId))
-            return False
+        print("Unable to locate server")
+        self.ioc.getLogger().error("Unable to load [{0}] server for information".format(serverId))
+        return False
 
     def action_assign(self, prototype=None, role=None, node=None):
         """Assign prototypes/roles to a node either local or remote
@@ -177,12 +175,12 @@ def action_assign(self, prototype=None, role=None, node=None):
             del job
             valid, serverId = self.valid_server(node)
             if not valid:
-                    print("Invalid ObjectID")
-                    return False
+                print("Invalid ObjectID")
+                return False
             updated = self.ioc.getCollection('JobServer').update_one(
                 {'_id': ObjectId(serverId)},
                 {
-                    '$push': {
+                    '$addToSet': {
                         'prototypes': prototype
                     }
                 }
@@ -198,8 +196,8 @@ def action_assign(self, prototype=None, role=None, node=None):
         if role:
             valid, serverId = self.valid_server(node)
             if not valid:
-                    print("Invalid ObjectID")
-                    return False
+                print("Invalid ObjectID")
+                return False
             updated = self.ioc.getCollection('JobServer').update_one(
                 {'_id': ObjectId(serverId)},
                 {
@@ -245,8 +243,8 @@ def action_unassign(self, prototype=None, role=None, node=None):
             del job
             valid, serverId = self.valid_server(node)
             if not valid:
-                    print("Invalid ObjectID")
-                    return False
+                print("Invalid ObjectID")
+                return False
             updated = self.ioc.getCollection('JobServer').update_one(
                 {'_id': ObjectId(serverId)},
                 {
@@ -266,8 +264,8 @@ def action_unassign(self, prototype=None, role=None, node=None):
         if role:
             valid, serverId = self.valid_server(node)
             if not valid:
-                    print("Invalid ObjectID")
-                    return False
+                print("Invalid ObjectID")
+                return False
             updated = self.ioc.getCollection('JobServer').update_one(
                 {'_id': ObjectId(serverId)},
                 {
@@ -302,8 +300,8 @@ def action_cull(self, node=None):
             return False
         valid, serverId = self.valid_server(node)
         if not valid:
-                print("Invalid ObjectID")
-                return False
+            print("Invalid ObjectID")
+            return False
         if not self.monitor.deactivateServer(serverId):
             self.ioc.getLogger().error(
                 "Failed deactivating server [{0}]".format(serverId)
@@ -356,8 +354,8 @@ def action_activate(self, node=None):
             return False
         valid, serverId = self.valid_server(node)
         if not valid:
-                print("Invalid ObjectID")
-                return False
+            print("Invalid ObjectID")
+            return False
         if self.ioc.getCollection('JobServer').update_one(
                 {'_id': ObjectId(serverId)},
                 {
@@ -369,9 +367,8 @@ def action_activate(self, node=None):
         ).modified_count < 1:
             self.ioc.getLogger().warning("Server [{0}] failed to be activated".format(serverId))
             return False
-        else:
-            self.ioc.getLogger().warning("Server [{0}] activated".format(serverId))
-            return True
+        self.ioc.getLogger().warning("Server [{0}] activated".format(serverId))
+        return True
 
     def valid_server(self, node=None):
         """Validates node is in the MongoDB instance connected to
@@ -391,8 +388,6 @@ def valid_server(self, node=None):
                 return False, ""
             if server:
                 return True, dict(server).get('_id')
-            else:
-                self.ioc.getLogger().error("Failed to find server [{0}] in the database".format(node))
-                return False, ""
-        else:
-            return True, self.ioc.getConfig().NodeIdentity
+            self.ioc.getLogger().error("Failed to find server [{0}] in the database".format(node))
+            return False, ""
+        return True, self.ioc.getConfig().NodeIdentity