Merge pull request #159 from ambitioninc/develop

5.0.1

Author: somewes

entity/sync.py

@@ -231,6 +231,28 @@ def sync(self):
         # IDs of sub/super entity relationships
         super_entities_by_ctype = _get_super_entities_by_ctype(model_objs_by_ctype, model_ids_to_sync, sync_all)
 
+        # Handle the case where accounts are created before _get_super_entities_by_ctype and
+        # the model_ids_to_sync do not match the model_objs_map
+        for ctype, model_ids in model_ids_to_sync.items():
+
+            # Build a set of ids of already fetched models
+            ids_of_fetched_models = {
+                model.id
+                for model in model_objs_by_ctype[ctype]
+            }
+
+            # Compute the set diff to see if any new records were created
+            created_model_ids = model_ids - ids_of_fetched_models
+
+            # Check if new records
+            if created_model_ids:
+
+                # Fetch the records and add them to the model_objs_map
+                new_records = ctype.model_class().objects.filter(id__in=created_model_ids)
+                for new_record in new_records:
+                    model_objs_by_ctype[ctype].append(new_record)
+                    model_objs_map[(ctype, new_record.id)] = new_record
+
         # Now that we have all models we need to sync, fetch them so that we can extract
         # metadata and entity kinds. If we are syncing all entities, we've already fetched
         # everything and can fill in this data struct without doing another DB hit
@@ -300,6 +322,7 @@ def sync(self):
             for sub_ctype, super_entities_by_sub_ctype in super_entities_by_ctype.items()
             for super_ctype, relationships in super_entities_by_sub_ctype.items()
             for sub_entity_id, super_entity_id in relationships
+            if (sub_ctype.id, sub_entity_id) in entities_map and (super_ctype.id, super_entity_id) in entities_map
         ]
 
         # Find the entities of the original model objects we were syncing. These
@@ -308,6 +331,7 @@ def sync(self):
             entities_map[ctype.id, model_obj.id].id
             for ctype, model_objs_for_ctype in model_objs_by_ctype.items()
             for model_obj in model_objs_for_ctype
+            if (ctype.id, model_obj.id) in entities_map
         ]
 
         if self.sync_all:

entity/tests/models.py

@@ -146,14 +146,19 @@ def get_super_entities(self, model_objs, sync_all):
         """
         Gets the super entities this entity belongs to.
         """
+        if sync_all:
+            accounts = list(self.queryset.all())
+        if not sync_all:
+            accounts = model_objs
+
         return {
             Team: [
-                (a.id, a.team_id) for a in model_objs if a.team_id
+                (a.id, a.team_id) for a in accounts if a.team_id
             ] + [
-                (a.id, a.team2_id) for a in model_objs if a.team2_id
+                (a.id, a.team2_id) for a in accounts if a.team2_id
             ],
-            TeamGroup: [(a.id, a.team_group_id) for a in model_objs if a.team_group_id],
-            Competitor: [(a.id, a.competitor_id) for a in model_objs if a.competitor_id]
+            TeamGroup: [(a.id, a.team_group_id) for a in accounts if a.team_group_id],
+            Competitor: [(a.id, a.competitor_id) for a in accounts if a.competitor_id]
         }
 
 

entity/tests/sync_tests.py

@@ -7,7 +7,7 @@
 from django_dynamic_fixture import G
 from entity.config import EntityRegistry
 from entity.models import Entity, EntityRelationship, EntityKind
-from entity.sync import sync_entities, defer_entity_syncing, transaction_atomic_with_retry
+from entity.sync import sync_entities, defer_entity_syncing, transaction_atomic_with_retry, _get_super_entities_by_ctype
 from entity.signal_handlers import turn_on_syncing, turn_off_syncing
 from mock import patch, MagicMock, call
 
@@ -250,9 +250,86 @@ def test_sync_all_accounts_teams(self):
         # There should be four entity relationships since four accounts have teams
         self.assertEquals(EntityRelationship.objects.all().count(), 4)
 
+    def test_sync_all_accounts_teams_new_account_during_sync(self):
+        """
+        Tests the scenario of a new account being created after account ids are fetched but before the super
+        entities are fetched
+        """
+        # Create five test accounts
+        accounts = [Account.objects.create() for i in range(5)]
+        # Create two teams to assign to some of the accounts
+        teams = [Team.objects.create() for i in range(2)]
+        accounts[0].team = teams[0]
+        accounts[0].save()
+        accounts[1].team = teams[0]
+        accounts[1].save()
+        accounts[2].team = teams[1]
+        accounts[2].save()
+        accounts[3].team = teams[1]
+        accounts[3].save()
+
+        def wrapped_super_entities(*args, **kwargs):
+            if not Account.objects.filter(email='[email protected]').exists():
+                Account.objects.create(
+                    email='[email protected]',
+                    team=Team.objects.order_by('id')[0],
+                    team2=Team.objects.order_by('id')[1],
+                )
+
+            return _get_super_entities_by_ctype(*args, **kwargs)
+
+        # Sync all the entities. There should be 8 (6 accounts 2 teams)
+        with patch('entity.sync._get_super_entities_by_ctype', wraps=wrapped_super_entities):
+            sync_entities()
+
+        self.assertEquals(Entity.objects.filter(entity_type=ContentType.objects.get_for_model(Account)).count(), 6)
+        self.assertEquals(Entity.objects.filter(entity_type=ContentType.objects.get_for_model(Team)).count(), 2)
+        self.assertEquals(Entity.objects.all().count(), 8)
+
+        # There should be six entity relationships
+        self.assertEquals(EntityRelationship.objects.all().count(), 6)
+
+    def test_sync_all_accounts_teams_deleted_account_during_sync(self):
+        """
+        Tests the scenario of an account being deleted after account ids are fetched but before the super
+        entities are fetched
+        """
+        # Create five test accounts
+        accounts = [Account.objects.create() for i in range(5)]
+        # Create two teams to assign to some of the accounts
+        teams = [Team.objects.create() for i in range(2)]
+        accounts[0].team = teams[0]
+        accounts[0].email = '[email protected]'
+        accounts[0].save()
+        accounts[1].team = teams[0]
+        accounts[1].save()
+        accounts[2].team = teams[1]
+        accounts[2].save()
+        accounts[3].team = teams[1]
+        accounts[3].save()
+
+        def wrapped_super_entities(*args, **kwargs):
+            if Account.objects.filter(email='[email protected]').exists():
+                Account.objects.filter(email='[email protected]').delete()
+
+            return _get_super_entities_by_ctype(*args, **kwargs)
+
+        # Sync all the entities. There should be 6 (4 accounts 2 teams)
+        with patch('entity.sync._get_super_entities_by_ctype', wraps=wrapped_super_entities):
+            sync_entities(*accounts)
+            # Sync again to hit other wrapped function branch and make sure it doesn't error
+            sync_entities(*accounts)
+
+        self.assertEquals(Entity.objects.filter(entity_type=ContentType.objects.get_for_model(Account)).count(), 4)
+        self.assertEquals(Entity.objects.filter(entity_type=ContentType.objects.get_for_model(Team)).count(), 2)
+        self.assertEquals(Entity.objects.all().count(), 6)
+
+        # There should be six entity relationships
+        self.assertEquals(EntityRelationship.objects.all().count(), 3)
+
     def test_sync_all_accounts_teams_inactive_entity_kind(self):
         """
-        Tests syncing of all accounts when they have super entities and the entiity kind is inactive
+        Tests syncing of all accounts when they have super entities and the entity kind is inactive
         """
         # Create five test accounts
         accounts = [Account.objects.create() for i in range(5)]
@@ -905,7 +982,7 @@ def test_sync_all_optimal_queries(self):
         with patch('entity.sync.entity_registry') as mock_entity_registry:
             mock_entity_registry.entity_registry = new_registry.entity_registry
             ContentType.objects.clear_cache()
-            with self.assertNumQueries(19):
+            with self.assertNumQueries(20):
                 sync_entities()
 
         self.assertEquals(Entity.objects.filter(entity_type=ContentType.objects.get_for_model(Account)).count(), 5)

entity/version.py

@@ -1 +1 @@
-__version__ = '5.0.0'
+__version__ = '5.0.1'

release_notes.md

@@ -1,5 +1,7 @@
 ## Release Notes
 
+- 5.0.1:
+    - Fix bug where an entity created or deleted during a sync would cause errors
 - 5.0.0:
     - Django 3.0, Django 3.1
     - Drop Django 2.0, 2.1 support