From 43e5eeddf648db6ee129eeeb24f4155b00318b77 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Mon, 2 Oct 2023 18:40:39 -0700 Subject: [PATCH 1/5] Add test for deletion and recreation across syncing. --- .../tests/integration/test_syncsession.py | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/testapp/tests/integration/test_syncsession.py b/tests/testapp/tests/integration/test_syncsession.py index 4b5d76a..9bef5cf 100644 --- a/tests/testapp/tests/integration/test_syncsession.py +++ b/tests/testapp/tests/integration/test_syncsession.py @@ -351,3 +351,46 @@ def test_resume(self): with second_environment(): self.assertEqual(5, SummaryLog.objects.filter(user=self.remote_user).count()) self.assertEqual(5, InteractionLog.objects.filter(user=self.remote_user).count()) + + def test_create_sync_delete_sync_recreate_sync(self): + with second_environment(): + SummaryLog.objects.create(user=self.remote_user) + summ_log = SummaryLog.objects.first() + summ_log_id = summ_log.id + content_id = summ_log.content_id + + self.assertEqual(0, SummaryLog.objects.filter(id=summ_log_id).count()) + + # first pull + pull_client = self.client.get_pull_client() + pull_client.initialize(self.filter) + transfer_session = pull_client.context.transfer_session + self.assertEqual(1, transfer_session.records_total) + self.assertEqual(0, transfer_session.records_transferred) + pull_client.run() + self.assertEqual(1, transfer_session.records_transferred) + pull_client.finalize() + + # sanity check pull worked + self.assertEqual(1, SummaryLog.objects.filter(id=summ_log_id).count()) + + with second_environment(): + SummaryLog.objects.get(id=summ_log_id).delete() + + # now do another pull, which should pull in the deletion + second_pull_client = self.client.get_pull_client() + second_pull_client.initialize(self.filter) + second_pull_client.run() + second_pull_client.finalize() + self.assertEqual(0, SummaryLog.objects.filter(id=summ_log_id).count()) + + with second_environment(): + sum_log = SummaryLog.objects.create(user=self.remote_user, content_id=content_id) + self.assertEqual(sum_log.id, summ_log_id) + + # now do another pull, which should pull in the recreation + third_pull_client = self.client.get_pull_client() + third_pull_client.initialize(self.filter) + third_pull_client.run() + third_pull_client.finalize() + self.assertEqual(1, SummaryLog.objects.filter(id=summ_log_id).count()) From 8d4a4af2e52c5e6646b01fbf3d9c5a722c994b11 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Mon, 2 Oct 2023 18:41:00 -0700 Subject: [PATCH 2/5] Prevent DeletedModel creation during model deserialization. --- morango/models/core.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/morango/models/core.py b/morango/models/core.py index 4037732..3f9222f 100644 --- a/morango/models/core.py +++ b/morango/models/core.py @@ -17,6 +17,7 @@ from django.db.models import Func from django.db.models import Max from django.db.models import Q +from django.db.models import signals from django.db.models import TextField from django.db.models import Value from django.db.models.deletion import Collector @@ -475,7 +476,11 @@ def _deserialize_store_model(self, fk_cache, defer_fks=False): # noqa: C901 except klass_model.DoesNotExist: pass else: - klass_model.objects.filter(id=self.id).delete() + # Import here to avoid circular import, as the utils module + # imports core models. + from morango.sync.utils import mute_signals + with mute_signals(signals.post_delete): + klass_model.objects.filter(id=self.id).delete() return None, deferred_fks else: # load model into memory From 318f86166dcc060c5cd60882e1d8a43a3f1974f1 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 4 Oct 2023 18:23:14 -0700 Subject: [PATCH 3/5] Don't create HardDeleted or Deleted models on deserialization. --- morango/models/core.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/morango/models/core.py b/morango/models/core.py index 3f9222f..ad80103 100644 --- a/morango/models/core.py +++ b/morango/models/core.py @@ -469,18 +469,14 @@ def _deserialize_store_model(self, fk_cache, defer_fks=False): # noqa: C901 klass_model = syncable_models.get_model(self.profile, self.model_name) # if store model marked as deleted, attempt to delete in app layer if self.deleted: - # if hard deleted, propagate to related models - if self.hard_deleted: - try: - klass_model.objects.get(id=self.id).delete(hard_delete=True) - except klass_model.DoesNotExist: - pass - else: - # Import here to avoid circular import, as the utils module - # imports core models. - from morango.sync.utils import mute_signals - with mute_signals(signals.post_delete): - klass_model.objects.filter(id=self.id).delete() + # Don't differentiate between deletion and hard deletion here, + # as we don't want to add additional tracking for models in either case, + # just to actually delete them. + # Import here to avoid circular import, as the utils module + # imports core models. + from morango.sync.utils import mute_signals + with mute_signals(signals.post_delete): + klass_model.objects.filter(id=self.id).delete() return None, deferred_fks else: # load model into memory From 057ffded9f11bf358ae1ad87ed3cf2e75e7dc7f2 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 4 Oct 2023 18:25:49 -0700 Subject: [PATCH 4/5] Update changelog and version for 0.6.18 --- CHANGELOG.md | 4 ++++ morango/__init__.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7e199e..2a438b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,10 @@ # Release Notes List of the most important changes for each release. + +## 0.6.18 +- Prevent creation of Deleted and HardDeleted models during deserialization to allow propagation of syncable objects that are recreated after a previous deletion without causing a merge conflict. + ## 0.6.17 - Added `client-instance-id`, `server-instance-id`, `sync-filter`, `push` and `pull` arguments to `cleanupsyncs` management command - Added option for resuming a sync to ignore the `SyncSession`'s existing `process_id` diff --git a/morango/__init__.py b/morango/__init__.py index f055a7e..9ee5615 100644 --- a/morango/__init__.py +++ b/morango/__init__.py @@ -3,4 +3,4 @@ from __future__ import unicode_literals default_app_config = "morango.apps.MorangoConfig" -__version__ = "0.6.17" +__version__ = "0.6.18" From ca558cc06b4f95dd71e1f02d9d61bbbc204073b8 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 5 Oct 2023 13:33:54 -0700 Subject: [PATCH 5/5] Assert deletion in fact, rather than in record, for FKs onto hard deleted models. --- tests/testapp/tests/sync/test_controller.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/testapp/tests/sync/test_controller.py b/tests/testapp/tests/sync/test_controller.py index b0c16dd..f009e21 100644 --- a/tests/testapp/tests/sync/test_controller.py +++ b/tests/testapp/tests/sync/test_controller.py @@ -18,7 +18,6 @@ from morango.constants import transfer_statuses from morango.models.certificates import Filter from morango.models.core import DeletedModels -from morango.models.core import HardDeletedModels from morango.models.core import InstanceIDModel from morango.models.core import RecordMaxCounter from morango.models.core import Store @@ -287,7 +286,7 @@ def test_store_hard_delete_propagates(self): ) # make sure hard_deleted propagates to related models even if they are not hard_deleted self.mc.deserialize_from_store() - self.assertTrue(HardDeletedModels.objects.filter(id=log.id).exists()) + self.assertFalse(SummaryLog.objects.filter(id=log.id).exists()) class RecordMaxCounterUpdatesDuringSerialization(TestCase):