14 update or create fix

.bumpversion.toml

@@ -1,5 +1,5 @@
 [tool.bumpversion]
-current_version = "0.4.1"
+current_version = "0.4.2"
 parse = "(?P<major>\\d+)\\.(?P<minor>\\d+)\\.(?P<patch>\\d+)"
 serialize = ["{major}.{minor}.{patch}"]
 search = "{current_version}"

README.md

@@ -269,18 +269,18 @@ This way the chronology is strictly kept and the infinite events loop is avoided
 
 The disadvantage is that each system will still consume its own message.
 
-#### There are 2 mixins for django Model and for QuerySet:
+#### There are 2 classes for django Model and for QuerySet:
 
-#### KafkaSkipMixin
-It adds new `kafka_skip` boolean field, which defaults to `False`. And overrides `Model.save` method and sets `kafka_skip=False`.
+#### KafkaSkipModel
+Adds the `kafka_skip` boolean field, defaulting to `False`. This also automatically resets `kafka_skip` to `False` when updating model instances (if not explicitly set).
 
 Usage:
 ```python
 from django.contrib.auth.base_user import AbstractBaseUser
 from django.contrib.auth.models import PermissionsMixin
-from django_kafka.models import KafkaSkipMixin
+from django_kafka.models import KafkaSkipModel
 
-class User(KafkaSkipMixin, PermissionsMixin, AbstractBaseUser):
+class User(KafkaSkipModel, PermissionsMixin, AbstractBaseUser):
     # ...
 ```
 
@@ -295,14 +295,14 @@ Usage:
 from django.contrib.auth.base_user import AbstractBaseUser
 from django.contrib.auth.base_user import BaseUserManager
 from django.contrib.auth.models import PermissionsMixin
-from django_kafka.models import KafkaSkipMixin, KafkaSkipQueryset
+from django_kafka.models import KafkaSkipModel, KafkaSkipQueryset
 
 
 class UserManager(BaseUserManager.from_queryset(KafkaSkipQueryset)):
     # ...
 
 
-class User(KafkaSkipMixin, PermissionsMixin, AbstractBaseUser):
+class User(KafkaSkipModel, PermissionsMixin, AbstractBaseUser):
     # ...
     objects = UserManager()
 ```

django_kafka/__init__.py

@@ -14,7 +14,7 @@
 
 logger = logging.getLogger(__name__)
 
-__version__ = "0.4.1"
+__version__ = "0.4.2"
 
 __all__ = [
     "autodiscover",

django_kafka/models.py

@@ -8,7 +8,7 @@ def update(self, **kwargs) -> int:
         return super().update(**kwargs)
 
 
-class KafkaSkipMixin(models.Model):
+class KafkaSkipModel(models.Model):
     """
     For models (tables) which are synced with other database(s) in both directions.
 
@@ -44,16 +44,41 @@ def save(
         using=None,
         update_fields=None,
     ):
-        if update_fields is None:
+        if self._reset_kafka_skip:
             self.kafka_skip = False
-
-        elif "kafka_skip" not in update_fields:
-            self.kafka_skip = False
-            update_fields = ["kafka_skip", *update_fields]
+            if update_fields and "kafka_skip" not in update_fields:
+                update_fields = ["kafka_skip", *update_fields]
 
         super().save(
             force_insert=force_insert,
             force_update=force_update,
             using=using,
             update_fields=update_fields,
         )
+        self._reset_kafka_skip = True
+
+    @classmethod
+    def from_db(cls, *args, **kwargs):
+        """
+        Used by Django's QuerySet to initialize instances fetched from db, and won't
+        be called by QuerySet.create or direct initialization.
+
+        The kafka_skip value stored in the DB should not be reused; instead it should be
+        reset to False before updating - unless explicitly set.
+        """
+        instance = super().from_db(*args, **kwargs)
+        instance._reset_kafka_skip = True
+        return instance
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self._reset_kafka_skip = False  # set True for DB fetched instances in from_db
+
+    def __setattr__(self, key, value):
+        super().__setattr__(key, value)
+        if key == "kafka_skip":
+            self._reset_kafka_skip = False
+
+    def refresh_from_db(self, *args, **kwargs):
+        super().refresh_from_db(*args, **kwargs)
+        self._reset_kafka_skip = True

django_kafka/tests/test_models.py

@@ -1,82 +1,117 @@
 from unittest.mock import patch
 
-from django_kafka.models import KafkaSkipMixin
+from django_kafka.models import KafkaSkipModel
 from django_kafka.tests.models import AbstractModelTestCase
 
 
 class KafkaSkipModelTestCase(AbstractModelTestCase):
-    abstract_model = KafkaSkipMixin
+    abstract_model = KafkaSkipModel
+    model: type[KafkaSkipModel]
 
-    @patch("django_kafka.models.super")
-    def test_save_update_fields_not_in_kwargs(self, super_mock):
-        """
-        kafka_skip should be set to False when update_fields is not provided
-        """
+    def test_save__direct_instance_respects_set_kafka_skip(self):
+        """test `save` on directly created instances will not ignore set kafka_skip"""
+        instance = self.model(kafka_skip=True)
+        instance_with_pk = self.model(pk=2, kafka_skip=True)
+
+        instance.save()
+        instance_with_pk.save()
+
+        self.assertTrue(instance.kafka_skip)
+        self.assertTrue(instance_with_pk.kafka_skip)
 
-        instance = self.model(pk=1, kafka_skip=True)
+    def test_save__second_save_resets_kafka_skip(self):
+        """test saving a second time will reset kafka_skip to False"""
+        instance = self.model(kafka_skip=True)
 
+        instance.save()
         instance.save()
 
-        # save sets kafka_skip value to False
         self.assertFalse(instance.kafka_skip)
-        # didn't forget to call super
-        super_mock.assert_called_once_with()
-        # no update_fields set
-        super_mock.return_value.save.assert_called_once_with(
+
+    def test_save__adds_kafka_skip_to_update_fields_when_reset(self):
+        """test update_fields has kafka_skip added when kafka_skip is reset"""
+        instance = self.model(kafka_skip=True)
+        instance.save()
+
+        with patch("django.db.models.Model.save") as mock_save:
+            instance.save(update_fields=["field"])
+
+        mock_save.assert_called_once_with(
             force_insert=False,
             force_update=False,
             using=None,
-            update_fields=None,
+            update_fields=["kafka_skip", "field"],
         )
 
-    @patch("django_kafka.models.super")
-    def test_save_update_fields_in_kwargs_without_kafka_skip(self, super_mock):
-        """
-        kafka_skip should be set to False when update_fields does not contain kafka_skip
-        """
-        instance = self.model(pk=1, kafka_skip=True)
+    def test__saving_retrieved_db_instance__resets_unset_kafka_skip(self):
+        """test saving a db-retrieved instance will reset kafka_skip if not set"""
+        instance = self.model(kafka_skip=True)
+        instance.save()
 
-        update_fields = ["some_fields"]
+        retrieved = self.model.objects.get(pk=instance.pk)
+        retrieved.save()
 
-        instance.save(update_fields=update_fields)
+        self.assertFalse(retrieved.kafka_skip)
 
-        # save sets kafka_skip value to False
-        self.assertFalse(instance.kafka_skip)
-        # didn't forget to call super
-        super_mock.assert_called_once_with()
-        # kafka_skip added to update_fields
-        super_mock.return_value.save.assert_called_once_with(
-            force_insert=False,
-            force_update=False,
-            using=None,
-            update_fields=["kafka_skip", *update_fields],
-        )
+    def test__saving_retrieved_db_instance__respects_set_kafka_skip(self):
+        """test saving a db-retrieved instance will not ignore newly set kafka skip"""
+        instance = self.model(kafka_skip=False)
+        instance.save()
 
-    @patch("django_kafka.models.super")
-    def test_save_update_fields_in_kwargs_with_kafka_skip(self, super_mock):
-        """
-        kafka_skip should not be changed if provided in update_fields
-        """
-        update_fields = ["some_field", "kafka_skip"]
+        retrieved = self.model.objects.get(pk=instance.pk)
+        retrieved.kafka_skip = True
+        retrieved.save()
+
+        self.assertTrue(retrieved.kafka_skip)
+
+    def test__refresh_from_db__makes_kafka_skip_be_reset(self):
+        """test refreshing from db will ignore newly set kafka_skip"""
+        instance = self.model(kafka_skip=False)
+        instance.save()
 
-        instance = self.model(pk=1, kafka_skip=True)
+        instance.kafka_skip = True
+        instance.refresh_from_db()
+        instance.save()
 
-        instance.save(update_fields=update_fields)
+        self.assertFalse(instance.kafka_skip)
+
+    def test_queryset_create__respects_kafka_skip(self):
+        """test kafka_skip=True is maintained for QuerySet.create"""
+        instance = self.model.objects.create(kafka_skip=True)
+        instance_with_pk = self.model.objects.create(pk=2, kafka_skip=True)
 
-        # save does not change kafka_skip value
         self.assertTrue(instance.kafka_skip)
-        # didn't forget to call super
-        super_mock.assert_called_once_with()
-        # update_fields are not changed
-        super_mock.return_value.save.assert_called_once_with(
-            force_insert=False,
-            force_update=False,
-            using=None,
-            update_fields=update_fields,
+        self.assertTrue(instance_with_pk.kafka_skip)
+
+    def test_queryset_get_or_create__respects_kafka_skip(self):
+        """test kafka_skip=True is maintained for QuerySet.get_or_create"""
+        instance, _ = self.model.objects.get_or_create(defaults={"kafka_skip": True})
+        instance_with_pk, _ = self.model.objects.get_or_create(
+            pk=2,
+            defaults={"kafka_skip": True},
         )
 
-    @patch("django_kafka.models.super")
-    def test_queryset_update_sets_kafka_skip(self, super_mock):
+        self.assertTrue(instance.kafka_skip)
+        self.assertTrue(instance_with_pk.kafka_skip)
+
+    def test_queryset_update_or_create_with_kafka_skip(self):
+        """test kafka_skip=True is maintained for QuerySet.update_or_create"""
+        instance = self.model.objects.create(kafka_skip=False)
+
+        existing_instance, _ = self.model.objects.update_or_create(
+            pk=instance.id,
+            defaults={"kafka_skip": True},
+        )
+        new_instance, _ = self.model.objects.update_or_create(
+            pk=2,
+            defaults={"kafka_skip": True},
+        )
+
+        self.assertTrue(existing_instance.kafka_skip)
+        self.assertTrue(new_instance.kafka_skip)
+
+    @patch("django.db.models.QuerySet.update")
+    def test_queryset_update_sets_kafka_skip(self, mock_update):
         """
         `update` method should automatically set `kafka_skip=False`
           if `kafka_skip` is not provided in kwargs.
@@ -87,12 +122,12 @@ def test_queryset_update_sets_kafka_skip(self, super_mock):
         self.model.objects.update(**update_kwargs)
 
         # kafka_skip=False was added to the update_kwargs
-        super_mock.return_value.update.assert_called_once_with(
+        mock_update.assert_called_once_with(
             **{"kafka_skip": False, **update_kwargs},
         )
 
-    @patch("django_kafka.models.super")
-    def test_queryset_update_does_not_override_kafka_skip(self, super_mock):
+    @patch("django.db.models.QuerySet.update")
+    def test_queryset_update_does_not_override_kafka_skip(self, mock_update):
         """
         `update` method should not change `kafka_skip`
           if `kafka_skip` is provided in kwargs
@@ -103,4 +138,4 @@ def test_queryset_update_does_not_override_kafka_skip(self, super_mock):
         self.model.objects.update(**update_kwargs)
 
         # kafka_skip is not changed, update_kwargs are not changed
-        super_mock.return_value.update.assert_called_once_with(**update_kwargs)
+        mock_update.assert_called_once_with(**update_kwargs)

django_kafka/tests/topic/test_model.py

@@ -4,7 +4,7 @@
 from django.test import TestCase
 
 from django_kafka.exceptions import DjangoKafkaError
-from django_kafka.models import KafkaSkipMixin
+from django_kafka.models import KafkaSkipModel
 from django_kafka.topic.model import ModelTopicConsumer
 
 
@@ -32,7 +32,7 @@ def test_get_defaults(self):
     def test_get_defaults__adds_kafka_skip(self):
         topic_consumer = self._get_model_topic_consumer()
 
-        class KafkaSkip(KafkaSkipMixin):
+        class KafkaSkip(KafkaSkipModel):
             pass
 
         defaults = topic_consumer.get_defaults(model=KafkaSkip, value={"name": 1})

django_kafka/topic/model.py

@@ -7,7 +7,7 @@
 from django.db.models import Model
 
 from django_kafka.exceptions import DjangoKafkaError
-from django_kafka.models import KafkaSkipMixin
+from django_kafka.models import KafkaSkipModel
 from django_kafka.topic import TopicConsumer
 
 
@@ -30,7 +30,7 @@ def get_defaults(self, model, value) -> dict:
             else:
                 defaults[attr] = attr_value
 
-        if issubclass(model, KafkaSkipMixin):
+        if issubclass(model, KafkaSkipModel):
             defaults["kafka_skip"] = True
 
         return defaults

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "django-kafka"
-version = "0.4.1"
+version = "0.4.2"
 dependencies = [
     "django>=4.0,<6.0",
     "confluent-kafka[avro, schema-registry]==2.4.0"