From 00428103d6127c3b04554dbe7710a751517d12e8 Mon Sep 17 00:00:00 2001 From: Blaine Jester Date: Wed, 21 Jan 2026 11:50:12 -0800 Subject: [PATCH] Allow passthrough of sync_filter during deserialization and model validation (clean) logic --- morango/models/core.py | 59 +++++++-- morango/sync/operations.py | 5 +- .../migrations/0005_conditionallog.py | 34 +++++ tests/testapp/facility_profile/models.py | 43 +++++-- tests/testapp/tests/models/test_core.py | 25 ++++ tests/testapp/tests/sync/test_operations.py | 119 ++++++++++++++++-- 6 files changed, 250 insertions(+), 35 deletions(-) create mode 100644 tests/testapp/facility_profile/migrations/0005_conditionallog.py diff --git a/morango/models/core.py b/morango/models/core.py index 448012c..73f175f 100644 --- a/morango/models/core.py +++ b/morango/models/core.py @@ -453,7 +453,7 @@ class Meta: models.Index(fields=["profile", "model_name", "partition", "dirty_bit"], condition=models.Q(dirty_bit=True), name="idx_morango_deserialize"), ] - def _deserialize_store_model(self, fk_cache, defer_fks=False): # noqa: C901 + def _deserialize_store_model(self, fk_cache, defer_fks=False, sync_filter=None): # noqa: C901 """ When deserializing a store model, we look at the deleted flags to know if we should delete the app model. Upon loading the app model in memory we validate the app models fields, if any errors occurs we follow @@ -461,6 +461,13 @@ def _deserialize_store_model(self, fk_cache, defer_fks=False): # noqa: C901 We return: None => if the model was deleted successfully model => if the model validates successfully + + :param fk_cache: A cache for foreign key lookups + :type fk_cache: dict + :param defer_fks: Whether to defer foreign key lookups + :type defer_fks: bool + :param sync_filter: The current sync's filter, if any + :type sync_filter: Filter|None """ deferred_fks = {} klass_model = syncable_models.get_model(self.profile, self.model_name) @@ -476,8 +483,10 @@ def _deserialize_store_model(self, fk_cache, defer_fks=False): # noqa: C901 klass_model.syncing_objects.filter(id=self.id).delete() return None, deferred_fks else: + if sync_filter: + print("Has filter", sync_filter) # load model into memory - app_model = klass_model.deserialize(json.loads(self.serialized)) + app_model = klass_model.deserialize(json.loads(self.serialized), sync_filter=sync_filter) app_model._morango_source_id = self.source_id app_model._morango_partition = self.partition app_model._morango_dirty_bit = False @@ -485,9 +494,9 @@ def _deserialize_store_model(self, fk_cache, defer_fks=False): # noqa: C901 try: # validate and return the model if defer_fks: - deferred_fks = app_model.deferred_clean_fields() + deferred_fks = app_model.deferred_clean_fields(sync_filter=sync_filter) else: - app_model.cached_clean_fields(fk_cache) + app_model.cached_clean_fields(fk_cache, sync_filter=sync_filter) return app_model, deferred_fks except (exceptions.ValidationError, exceptions.ObjectDoesNotExist) as e: @@ -853,15 +862,31 @@ def delete( obj._update_hard_deleted_models() return collector.delete() - def cached_clean_fields(self, fk_lookup_cache): + def clean_fields(self, exclude=None, sync_filter=None): + """ + Immediately validates all fields + + :param exclude: A list of field names to exclude from validation + :type exclude: list[str] + :param sync_filter: The current sync's filter, if any + :type sync_filter: Filter|None + """ + super(SyncableModel, self).clean_fields(exclude=exclude) + + def cached_clean_fields(self, fk_lookup_cache, exclude=None, sync_filter=None): """ Immediately validates all fields, but uses a cache for foreign key (FK) lookups to reduce repeated queries for many records with the same FK :param fk_lookup_cache: A dictionary to use as a cache to prevent querying the database if a FK exists in the cache, having already been validated + :type fk_lookup_cache: dict + :param exclude: A list of field names to exclude from validation + :type exclude: list[str] + :param sync_filter: The current sync's filter, if any + :type sync_filter: Filter|None """ - excluded_fields = [] + excluded_fields = exclude or [] fk_fields = [ field for field in self._meta.fields if isinstance(field, models.ForeignKey) ] @@ -883,7 +908,7 @@ def cached_clean_fields(self, fk_lookup_cache): fk_lookup_cache[key] = 1 excluded_fields.append(f.name) - self.clean_fields(exclude=excluded_fields) + self.clean_fields(exclude=excluded_fields, sync_filter=sync_filter) # after cleaning, we can confidently set ourselves in the fk_lookup_cache self_key = "{id}_{db_table}".format( @@ -892,15 +917,19 @@ def cached_clean_fields(self, fk_lookup_cache): ) fk_lookup_cache[self_key] = 1 - def deferred_clean_fields(self): + def deferred_clean_fields(self, exclude=None, sync_filter=None): """ Calls `.clean_fields()` but excludes all foreign key fields and instead returns them as a dictionary for deferred batch processing + :param exclude: A list of field names to exclude from validation + :type exclude: list[str] + :param sync_filter: The current sync's filter, if any + :type sync_filter: Filter|None :return: A dictionary containing lists of `ForeignKeyReference`s keyed by the name of the model being referenced by the FK """ - excluded_fields = [] + excluded_fields = exclude or [] deferred_fks = defaultdict(list) for field in self._meta.fields: if not isinstance(field, models.ForeignKey): @@ -918,7 +947,7 @@ def deferred_clean_fields(self): ) ) - self.clean_fields(exclude=excluded_fields) + self.clean_fields(exclude=excluded_fields, sync_filter=sync_filter) return deferred_fks def serialize(self): @@ -939,8 +968,14 @@ def serialize(self): return data @classmethod - def deserialize(cls, dict_model): - """Returns an unsaved class object based on the valid properties passed in.""" + def deserialize(cls, dict_model, sync_filter=None): + """Returns an unsaved class object based on the valid properties passed in. + + :param dict_model: The model data to deserialize + :type dict_model: dict + :param sync_filter: The current sync's filter, if any + :type sync_filter: Filter|None + """ kwargs = {} for f in cls._meta.concrete_fields: if f.attname in dict_model: diff --git a/morango/sync/operations.py b/morango/sync/operations.py index 3e3f386..a5fa0c7 100644 --- a/morango/sync/operations.py +++ b/morango/sync/operations.py @@ -465,6 +465,7 @@ def _deserialize_from_store(profile, skip_erroring=False, filter=None): lambda x, y: x | y, [Q(partition__startswith=prefix) for prefix in filter], ) + print("prefix_condition: ", prefix_condition) store_models = store_models.filter(prefix_condition) # if requested, skip any records that previously errored, to be faster @@ -485,7 +486,7 @@ def _deserialize_from_store(profile, skip_erroring=False, filter=None): for store_model in dirty_children: try: app_model, _ = store_model._deserialize_store_model( - fk_cache + fk_cache, sync_filter=filter ) if app_model: with mute_signals(signals.pre_save, signals.post_save): @@ -538,7 +539,7 @@ def _deserialize_from_store(profile, skip_erroring=False, filter=None): app_model, model_deferred_fks, ) = store_model._deserialize_store_model( - fk_cache, defer_fks=True + fk_cache, defer_fks=True, sync_filter=filter, ) if app_model: app_models.append(app_model) diff --git a/tests/testapp/facility_profile/migrations/0005_conditionallog.py b/tests/testapp/facility_profile/migrations/0005_conditionallog.py new file mode 100644 index 0000000..a82b6b4 --- /dev/null +++ b/tests/testapp/facility_profile/migrations/0005_conditionallog.py @@ -0,0 +1,34 @@ +# Generated by Django 3.2.25 on 2026-01-21 18:58 +import uuid + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations +from django.db import models + +import morango.models.fields.uuids + + +class Migration(migrations.Migration): + + dependencies = [ + ('facility_profile', '0004_testmodel'), + ] + + operations = [ + migrations.CreateModel( + name='ConditionalLog', + fields=[ + ('id', morango.models.fields.uuids.UUIDField(editable=False, primary_key=True, serialize=False)), + ('_morango_dirty_bit', models.BooleanField(default=True, editable=False)), + ('_morango_source_id', models.CharField(editable=False, max_length=96)), + ('_morango_partition', models.CharField(editable=False, max_length=128)), + ('content_id', morango.models.fields.uuids.UUIDField(db_index=True, default=uuid.uuid4)), + ('facility', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='facility_profile.facility')), + ('user', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + options={ + 'abstract': False, + }, + ), + ] diff --git a/tests/testapp/facility_profile/models.py b/tests/testapp/facility_profile/models.py index 5f29f99..7c717cb 100644 --- a/tests/testapp/facility_profile/models.py +++ b/tests/testapp/facility_profile/models.py @@ -22,8 +22,6 @@ class SyncableUserModelManager(SyncableModelManager, UserManager): class Facility(FacilityDataSyncableModel): - - # Morango syncing settings morango_model_name = "facility" name = models.CharField(max_length=100) @@ -34,7 +32,10 @@ def calculate_source_id(self, *args, **kwargs): return self.name def calculate_partition(self, *args, **kwargs): - return '' + if self.id: + return uuid.UUID(self.id).hex + else: + return '{id}'.format(id=self.ID_PLACEHOLDER) def clean_fields(self, *args, **kwargs): # reference parent here just to trigger a non-validation error to make sure we handle it @@ -43,7 +44,6 @@ def clean_fields(self, *args, **kwargs): class MyUser(AbstractBaseUser, FacilityDataSyncableModel): - # Morango syncing settings morango_model_name = "user" USERNAME_FIELD = "username" @@ -73,21 +73,19 @@ def compute_namespaced_id(partition_value, source_id_value, model_name): class SummaryLog(FacilityDataSyncableModel): - # Morango syncing settings morango_model_name = "contentsummarylog" user = models.ForeignKey(MyUser, on_delete=models.CASCADE) content_id = UUIDField(db_index=True, default=uuid.uuid4) def calculate_source_id(self, *args, **kwargs): - return '{}:{}'.format(self.user.id, self.content_id) + return '{}:{}'.format(self.user_id, self.content_id) def calculate_partition(self, *args, **kwargs): - return '{user_id}:user:summary'.format(user_id=self.user.id) + return '{user_id}:user:summary'.format(user_id=self.user_id) class InteractionLog(FacilityDataSyncableModel): - # Morango syncing settings morango_model_name = "contentinteractionlog" user = models.ForeignKey(MyUser, blank=True, null=True, on_delete=models.CASCADE) @@ -97,7 +95,33 @@ def calculate_source_id(self, *args, **kwargs): return None def calculate_partition(self, *args, **kwargs): - return '{user_id}:user:interaction'.format(user_id=self.user.id) + return '{user_id}:user:interaction'.format(user_id=self.user_id) + + +class ConditionalLog(FacilityDataSyncableModel): + morango_model_name = "conditionallog" + + facility = models.ForeignKey(Facility, blank=False, null=False, on_delete=models.CASCADE) + user = models.ForeignKey(MyUser, blank=True, null=True, on_delete=models.CASCADE) + content_id = UUIDField(db_index=True, default=uuid.uuid4) + + def calculate_source_id(self, *args, **kwargs): + return None + + def calculate_partition(self, *args, **kwargs): + return uuid.UUID(self.facility_id).hex + + def clean_fields(self, exclude=None, sync_filter=None): + exclude = exclude or [] + if sync_filter: + exclude.append("user_id") + super(ConditionalLog, self).clean_fields(exclude=exclude, sync_filter=sync_filter) + + @classmethod + def deserialize(cls, dict_model, sync_filter=None): + if sync_filter: + del dict_model["user_id"] + return super().deserialize(dict_model, sync_filter) class FilteredModelManager(SyncableModelManager): @@ -110,7 +134,6 @@ def get_queryset(self): class TestModel(FacilityDataSyncableModel): """Test model with a custom manager to test syncing_objects behavior""" - # Morango syncing settings morango_model_name = "testmodel" name = models.CharField(max_length=100) diff --git a/tests/testapp/tests/models/test_core.py b/tests/testapp/tests/models/test_core.py index cb7d893..de25201 100644 --- a/tests/testapp/tests/models/test_core.py +++ b/tests/testapp/tests/models/test_core.py @@ -1,9 +1,11 @@ import uuid import factory +import mock from django.test import override_settings from django.test import TestCase from django.utils import timezone +from facility_profile.models import Facility from facility_profile.models import MyUser from ..helpers import RecordMaxCounterFactory @@ -367,3 +369,26 @@ def test_get_touched_record_ids_for_model__string(self): ) ), ) + + +class SyncableModelTestCase(TestCase): + @mock.patch("morango.models.core.UUIDModelMixin.clean_fields") + def test_clean_fields(self, mock_super_clean_fields): + f = Facility(name="test") + sync_filter = Filter("test") + f.clean_fields(exclude=["test1"], sync_filter=sync_filter) + mock_super_clean_fields.assert_called_once_with(exclude=["test1"]) + + @mock.patch("morango.models.core.SyncableModel.clean_fields") + def test_cached_clean_fields(self, mock_clean_fields): + f = Facility(name="test") + sync_filter = Filter("test") + f.cached_clean_fields({}, exclude=["test1"], sync_filter=sync_filter) + mock_clean_fields.assert_called_once_with(exclude=["test1", "parent"], sync_filter=sync_filter) + + @mock.patch("morango.models.core.SyncableModel.clean_fields") + def test_deferred_clean_fields(self, mock_clean_fields): + f = Facility(name="test") + sync_filter = Filter("test") + f.deferred_clean_fields(exclude=["test1"], sync_filter=sync_filter) + mock_clean_fields.assert_called_once_with(exclude=["test1"], sync_filter=sync_filter) diff --git a/tests/testapp/tests/sync/test_operations.py b/tests/testapp/tests/sync/test_operations.py index 4edf905..6aae39e 100644 --- a/tests/testapp/tests/sync/test_operations.py +++ b/tests/testapp/tests/sync/test_operations.py @@ -12,6 +12,7 @@ from django.test import TestCase from django.test import TransactionTestCase from django.utils import timezone +from facility_profile.models import ConditionalLog from facility_profile.models import Facility from facility_profile.models import MyUser from facility_profile.models import SummaryLog @@ -1135,9 +1136,13 @@ def test_local_cleanup(self): class DeserializationTestCases(TestCase): def setUp(self): - self.profile = "facilitydata" + self.serialized_facility = { + "id": uuid.uuid4().hex, + "name": "test facility", + "now_date": timezone.now().isoformat() + } self.serialized_user = { "id": uuid.uuid4().hex, "username": "testuser", @@ -1153,9 +1158,16 @@ def setUp(self): "user_id": self.serialized_user["id"], "content_id": uuid.uuid4().hex, } + self.serialized_conditional = { + "id": uuid.uuid4().hex, + "facility_id": self.serialized_facility["id"], + "user_id": self.serialized_user["id"], + "content_id": uuid.uuid4().hex, + } def serialize_to_store(self, Model, data): instance = Model(**data) + instance.calculate_uuid() serialized = instance.serialize() Store.objects.create( id=serialized["id"], @@ -1170,17 +1182,70 @@ def serialize_to_store(self, Model, data): ) def serialize_all_to_store(self): + self.serialize_to_store(Facility, self.serialized_facility) self.serialize_to_store(MyUser, self.serialized_user) self.serialize_to_store(SummaryLog, self.serialized_log1) self.serialize_to_store(SummaryLog, self.serialized_log2) - - def assert_deserialization(self, user_deserialized=True, log1_deserialized=True, log2_deserialized=True): - assert MyUser.objects.filter(id=self.serialized_user["id"]).exists() == user_deserialized - assert SummaryLog.objects.filter(id=self.serialized_log1["id"]).exists() == log1_deserialized - assert SummaryLog.objects.filter(id=self.serialized_log2["id"]).exists() == log2_deserialized - assert Store.objects.get(id=self.serialized_user["id"]).dirty_bit == (not user_deserialized) - assert Store.objects.get(id=self.serialized_log1["id"]).dirty_bit == (not log1_deserialized) - assert Store.objects.get(id=self.serialized_log2["id"]).dirty_bit == (not log2_deserialized) + self.serialize_to_store(ConditionalLog, self.serialized_conditional) + + def assert_deserialization( + self, + facility_deserialized=True, + user_deserialized=True, + log1_deserialized=True, + log2_deserialized=True, + conditional_deserialized=True, + ): + self.assertEqual( + Facility.objects.filter(id=self.serialized_facility["id"]).exists(), + facility_deserialized, + msg="Facility was not deserialized" if facility_deserialized else "Facility was deserialized" + ) + self.assertEqual( + MyUser.objects.filter(id=self.serialized_user["id"]).exists(), + user_deserialized, + msg="User was not deserialized" if user_deserialized else "User was deserialized" + ) + self.assertEqual( + SummaryLog.objects.filter(id=self.serialized_log1["id"]).exists(), + log1_deserialized, + msg="Log1 was not deserialized" if log1_deserialized else "Log1 was deserialized" + ) + self.assertEqual( + SummaryLog.objects.filter(id=self.serialized_log2["id"]).exists(), + log2_deserialized, + msg="Log2 was not deserialized" if log2_deserialized else "Log2 was deserialized" + ) + self.assertEqual( + ConditionalLog.objects.filter(id=self.serialized_conditional["id"]).exists(), + conditional_deserialized, + msg="Conditional was not deserialized" if conditional_deserialized else "Conditional was deserialized" + ) + self.assertEqual( + Store.objects.get(id=self.serialized_facility["id"]).dirty_bit, + (not facility_deserialized), + msg="Facility store does not reflect deserialization" if not facility_deserialized else "Facility store reflects deserialization" + ) + self.assertEqual( + Store.objects.get(id=self.serialized_user["id"]).dirty_bit, + (not user_deserialized), + msg="User store does not reflect deserialization" if not user_deserialized else "User store reflects deserialization" + ) + self.assertEqual( + Store.objects.get(id=self.serialized_log1["id"]).dirty_bit, + (not log1_deserialized), + msg="Log1 store does not reflect deserialization" if not log1_deserialized else "Log1 store reflects deserialization" + ) + self.assertEqual( + Store.objects.get(id=self.serialized_log2["id"]).dirty_bit, + (not log2_deserialized), + msg="Log2 store does not reflect deserialization" if not log2_deserialized else "Log2 store reflects deserialization" + ) + self.assertEqual( + Store.objects.get(id=self.serialized_conditional["id"]).dirty_bit, + (not conditional_deserialized), + msg="Conditional store does not reflect deserialization" if not conditional_deserialized else "Conditional store reflects deserialization" + ) def test_successful_deserialization(self): @@ -1198,7 +1263,12 @@ def test_deserialization_with_missing_username(self): _deserialize_from_store(self.profile) - self.assert_deserialization(user_deserialized=False, log1_deserialized=False, log2_deserialized=False) + self.assert_deserialization( + user_deserialized=False, + log1_deserialized=False, + log2_deserialized=False, + conditional_deserialized=False + ) def test_deserialization_with_excessively_long_username(self): @@ -1208,7 +1278,12 @@ def test_deserialization_with_excessively_long_username(self): _deserialize_from_store(self.profile) - self.assert_deserialization(user_deserialized=False, log1_deserialized=False, log2_deserialized=False) + self.assert_deserialization( + user_deserialized=False, + log1_deserialized=False, + log2_deserialized=False, + conditional_deserialized=False + ) def test_deserialization_with_invalid_content_id(self): @@ -1229,3 +1304,25 @@ def test_deserialization_with_invalid_log_user_id(self): _deserialize_from_store(self.profile) self.assert_deserialization(log1_deserialized=False) + + def test_deserialization__conditional__no_filter(self): + self.serialize_all_to_store() + + _deserialize_from_store(self.profile) + + self.assert_deserialization() + conditional_log = ConditionalLog.objects.get(pk=self.serialized_conditional["id"]) + self.assertEqual(conditional_log.user_id, self.serialized_user["id"]) + + def test_deserialization__conditional__with_filter(self): + self.serialize_all_to_store() + + _deserialize_from_store(self.profile, filter=Filter(self.serialized_facility["id"])) + + self.assert_deserialization( + user_deserialized=False, + log1_deserialized=False, + log2_deserialized=False, + ) + conditional_log = ConditionalLog.objects.get(pk=self.serialized_conditional["id"]) + self.assertIsNone(conditional_log.user_id)