diff --git a/.github/workflows/check_migrations_sqlite.yml b/.github/workflows/check_migrations_sqlite.yml index 3674c15b..cbcbc690 100644 --- a/.github/workflows/check_migrations_sqlite.yml +++ b/.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' }} diff --git a/morango/__init__.py b/morango/__init__.py index 732155f8..fa3ddd8c 100644 --- a/morango/__init__.py +++ b/morango/__init__.py @@ -1 +1 @@ -__version__ = "0.8.3" +__version__ = "0.8.4" diff --git a/morango/models/core.py b/morango/models/core.py index 1f4456c7..448012c9 100644 --- a/morango/models/core.py +++ b/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 diff --git a/morango/registry.py b/morango/registry.py index ca9ed6a9..5614c814 100644 --- a/morango/registry.py +++ b/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." diff --git a/morango/sync/operations.py b/morango/sync/operations.py index a2fbca2b..3e3f3861 100644 --- a/morango/sync/operations.py +++ b/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( diff --git a/tests/testapp/facility_profile/migrations/0004_testmodel.py b/tests/testapp/facility_profile/migrations/0004_testmodel.py new file mode 100644 index 00000000..216ef9c7 --- /dev/null +++ b/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, + }, + ), + ] diff --git a/tests/testapp/facility_profile/models.py b/tests/testapp/facility_profile/models.py index ab7ccf56..5f29f991 100644 --- a/tests/testapp/facility_profile/models.py +++ b/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' diff --git a/tests/testapp/tests/integration/test_syncing_models.py b/tests/testapp/tests/integration/test_syncing_models.py index e427cd03..460b1029 100644 --- a/tests/testapp/tests/integration/test_syncing_models.py +++ b/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())