diff --git a/.bumpversion.toml b/.bumpversion.toml index 2aaec02..3584bba 100644 --- a/.bumpversion.toml +++ b/.bumpversion.toml @@ -1,5 +1,5 @@ [tool.bumpversion] -current_version = "0.4.1" +current_version = "0.4.2" parse = "(?P\\d+)\\.(?P\\d+)\\.(?P\\d+)" serialize = ["{major}.{minor}.{patch}"] search = "{current_version}" diff --git a/README.md b/README.md index 4d3e7e1..660b57c 100644 --- a/README.md +++ b/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() ``` diff --git a/django_kafka/__init__.py b/django_kafka/__init__.py index eb7dbf0..0463745 100644 --- a/django_kafka/__init__.py +++ b/django_kafka/__init__.py @@ -14,7 +14,7 @@ logger = logging.getLogger(__name__) -__version__ = "0.4.1" +__version__ = "0.4.2" __all__ = [ "autodiscover", diff --git a/django_kafka/models.py b/django_kafka/models.py index c460e2c..411e344 100644 --- a/django_kafka/models.py +++ b/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,12 +44,10 @@ 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, @@ -57,3 +55,30 @@ def save( 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 diff --git a/django_kafka/tests/test_models.py b/django_kafka/tests/test_models.py index 92ff303..bac0dcf 100644 --- a/django_kafka/tests/test_models.py +++ b/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) diff --git a/django_kafka/tests/topic/test_model.py b/django_kafka/tests/topic/test_model.py index 97ebf7b..3575094 100644 --- a/django_kafka/tests/topic/test_model.py +++ b/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}) diff --git a/django_kafka/topic/model.py b/django_kafka/topic/model.py index c6f0673..a5effd5 100644 --- a/django_kafka/topic/model.py +++ b/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 diff --git a/pyproject.toml b/pyproject.toml index 48335cf..f55a64f 100644 --- a/pyproject.toml +++ b/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"