Merge pull request #269 from rtibbles/syncable_model_manager

Allow default model manager to be overridden without affecting syncing behaviour

Author: rtibbles

.github/workflows/check_migrations_sqlite.yml

@@ -29,6 +29,7 @@ jobs:
           apt-get -y -qq update
           apt-get install -y build-essential tcl git-lfs
       - uses: actions/checkout@v4
+        if: ${{ needs.pre_job.outputs.should_skip != 'true' }}
         with:
           lfs: true
       - name: Build SQLite 3.25.3
@@ -45,12 +46,14 @@ jobs:
           # Once we have confirmed that this works, set it for subsequent steps
           echo "LD_PRELOAD=$(realpath .libs/libsqlite3.so)" >> $GITHUB_ENV
       - uses: actions/cache@v4
+        if: ${{ needs.pre_job.outputs.should_skip != 'true' }}
         with:
           path: ~/.cache/pip
           key: ${{ runner.os }}-pip-${{ hashFiles('setup.py') }}
           restore-keys: |
             ${{ runner.os }}-pip-
       - name: Install dependencies
+        if: ${{ needs.pre_job.outputs.should_skip != 'true' }}
         run: pip install .
       - name: Run migrations
         if: ${{ needs.pre_job.outputs.should_skip != 'true' }}

morango/__init__.py

@@ -1 +1 @@
-__version__ = "0.8.3"
+__version__ = "0.8.4"

morango/models/core.py

@@ -473,7 +473,7 @@ def _deserialize_store_model(self, fk_cache, defer_fks=False):  # noqa: C901
             # imports core models.
             from morango.sync.utils import mute_signals
             with mute_signals(signals.post_delete):
-                klass_model.objects.filter(id=self.id).delete()
+                klass_model.syncing_objects.filter(id=self.id).delete()
             return None, deferred_fks
         else:
             # load model into memory
@@ -806,6 +806,11 @@ class SyncableModel(UUIDModelMixin):
 
     objects = SyncableModelManager()
 
+    # Add a special syncing_objects queryset to every SyncableModel for use in syncing operations.
+    # This means that we still deal with the entire set of objects when syncing, even if the default
+    # model manager has been overridden to filter the queryset.
+    syncing_objects = SyncableModelManager()
+
     class Meta:
         abstract = True
 

morango/registry.py

@@ -35,6 +35,24 @@ def _multiple_self_ref_fk_check(class_model):
     return False
 
 
+def _check_manager(name, objects):
+    from morango.models.manager import SyncableModelManager
+    from morango.models.query import SyncableModelQuerySet
+    # syncable model checks
+    if not isinstance(objects, SyncableModelManager):
+        raise InvalidMorangoModelConfiguration(
+            "Manager for {} must inherit from SyncableModelManager.".format(
+                name
+            )
+        )
+    if not isinstance(objects.none(), SyncableModelQuerySet):
+        raise InvalidMorangoModelConfiguration(
+            "Queryset for {} model must inherit from SyncableModelQuerySet.".format(
+                name
+            )
+        )
+
+
 class SyncableModelRegistry(object):
     def __init__(self):
         self.profile_models = {}
@@ -98,8 +116,6 @@ def populate(self):  # noqa: C901
 
         import django.apps
         from morango.models.core import SyncableModel
-        from morango.models.manager import SyncableModelManager
-        from morango.models.query import SyncableModelQuerySet
 
         model_list = []
         for model in django.apps.apps.get_models():
@@ -110,19 +126,10 @@ def populate(self):  # noqa: C901
                     raise InvalidMorangoModelConfiguration(
                         "Syncing models with more than 1 self referential ForeignKey is not supported."
                     )
-                # syncable model checks
-                if not isinstance(model.objects, SyncableModelManager):
-                    raise InvalidMorangoModelConfiguration(
-                        "Manager for {} must inherit from SyncableModelManager.".format(
-                            name
-                        )
-                    )
-                if not isinstance(model.objects.none(), SyncableModelQuerySet):
-                    raise InvalidMorangoModelConfiguration(
-                        "Queryset for {} model must inherit from SyncableModelQuerySet.".format(
-                            name
-                        )
-                    )
+                # Check both the objects and the syncing_objects querysets.
+                _check_manager(name, model.objects)
+                _check_manager(name, model.syncing_objects)
+
                 if model._meta.many_to_many:
                     raise UnsupportedFieldType(
                         "{} model with a ManyToManyField is not supported in morango."

morango/sync/operations.py

@@ -145,7 +145,7 @@ def _serialize_into_store(profile, filter=None):
         for model in syncable_models.get_models(profile):
             new_store_records = []
             new_rmc_records = []
-            klass_queryset = model.objects.filter(_morango_dirty_bit=True)
+            klass_queryset = model.syncing_objects.filter(_morango_dirty_bit=True)
             if prefix_condition:
                 klass_queryset = klass_queryset.filter(prefix_condition)
             store_records_dict = Store.objects.in_bulk(

tests/testapp/facility_profile/migrations/0004_testmodel.py

@@ -0,0 +1,29 @@
+# Generated by Django 3.2.25 on 2025-09-22 23:23
+from django.db import migrations
+from django.db import models
+
+import morango.models.fields.uuids
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('facility_profile', '0003_auto_20240129_2025'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='TestModel',
+            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)),
+                ('name', models.CharField(max_length=100)),
+                ('hidden', models.BooleanField(default=False)),
+            ],
+            options={
+                'abstract': False,
+            },
+        ),
+    ]

tests/testapp/facility_profile/models.py

@@ -98,3 +98,29 @@ def calculate_source_id(self, *args, **kwargs):
 
     def calculate_partition(self, *args, **kwargs):
         return '{user_id}:user:interaction'.format(user_id=self.user.id)
+
+
+class FilteredModelManager(SyncableModelManager):
+    """Custom manager that filters out models marked as 'hidden'"""
+
+    def get_queryset(self):
+        return super().get_queryset().filter(hidden=False)
+
+
+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)
+    hidden = models.BooleanField(default=False)
+
+    # Override the default manager to filter hidden objects
+    objects = FilteredModelManager()
+
+    def calculate_source_id(self, *args, **kwargs):
+        return self.name
+
+    def calculate_partition(self, *args, **kwargs):
+        return 'testmodel'

tests/testapp/tests/integration/test_syncing_models.py

@@ -1,8 +1,13 @@
+import json
+
 from django.test import TestCase
 from facility_profile.models import MyUser
+from facility_profile.models import TestModel
 
+from morango.models.core import Store
 from morango.models.manager import SyncableModelManager
 from morango.models.query import SyncableModelQuerySet
+from morango.sync.controller import MorangoProfileController
 
 
 class SyncingModelsTestCase(TestCase):
@@ -61,3 +66,79 @@ def test_syncable_save(self):
         self.assertTrue(MyUser.objects.first()._morango_dirty_bit)
         user.save(update_dirty_bit_to=None)
         self.assertTrue(MyUser.objects.first()._morango_dirty_bit)
+
+    def test_syncing_objects_manager_with_custom_default_manager(self):
+        """Test that syncing_objects manager includes all objects even when default manager filters them out"""
+        # Create some test objects
+        TestModel.objects.create(name="visible", hidden=False)
+        hidden_obj = TestModel(name="hidden", hidden=True)
+        # Use save() to bypass the manager filter during creation
+        hidden_obj.save()
+
+        # Verify that the default objects manager filters out hidden objects
+        self.assertEqual(TestModel.objects.count(), 1)
+        self.assertEqual(TestModel.objects.first().name, "visible")
+
+        # Verify that syncing_objects manager includes all objects for syncing
+        self.assertEqual(TestModel.syncing_objects.count(), 2)
+        syncing_names = set(TestModel.syncing_objects.values_list('name', flat=True))
+        self.assertEqual(syncing_names, {"visible", "hidden"})
+
+    def test_hidden_models_serialization_into_store(self):
+        """Test that hidden models (filtered by default manager) still get serialized into Store"""
+        controller = MorangoProfileController(TestModel.morango_profile)
+
+        # Create both visible and hidden objects
+        visible_obj = TestModel.objects.create(name="visible", hidden=False)
+        hidden_obj = TestModel(name="hidden", hidden=True)
+        # Use save() to bypass the manager filter during creation
+        hidden_obj.save()
+
+        # Serialize into store
+        controller.serialize_into_store()
+
+        # Verify both objects (visible and hidden) are serialized into Store
+        store_records = Store.objects.filter(model_name=TestModel.morango_model_name)
+        self.assertEqual(store_records.count(), 2)
+
+        # Verify both objects are present in store by checking serialized data
+        serialized_names = set()
+        for store_record in store_records:
+            serialized_data = json.loads(store_record.serialized)
+            serialized_names.add(serialized_data['name'])
+
+        self.assertEqual(serialized_names, {"visible", "hidden"})
+
+        # Verify the store records have the correct IDs
+        store_ids = set(store_records.values_list('id', flat=True))
+        expected_ids = {str(visible_obj.id), str(hidden_obj.id)}
+        self.assertEqual(store_ids, expected_ids)
+
+    def test_hidden_models_deletion_during_deserialization(self):
+        """Test that hidden models can be deleted during deserialization using syncing_objects"""
+        controller = MorangoProfileController(TestModel.morango_profile)
+
+        # Create and serialize a hidden object
+        hidden_obj = TestModel(name="hidden", hidden=True)
+        hidden_obj.save()
+        controller.serialize_into_store()
+
+        # Verify object exists in Store
+        store_record = Store.objects.get(id=hidden_obj.id)
+        self.assertFalse(store_record.deleted)
+
+        # Mark the store record as deleted (simulating deletion from another device)
+        # Also set dirty_bit=True so it gets processed during deserialization
+        store_record.deleted = True
+        store_record.dirty_bit = True
+        store_record.save()
+
+        # Deserialize from store - this should delete the hidden object
+        controller.deserialize_from_store()
+
+        # Verify the hidden object was deleted from the database
+        self.assertFalse(TestModel.syncing_objects.filter(id=hidden_obj.id).exists())
+        self.assertFalse(TestModel.objects.filter(id=hidden_obj.id).exists())
+
+        # The store record should still exist but marked as deleted
+        self.assertTrue(Store.objects.filter(id=hidden_obj.id, deleted=True).exists())