From f1cc22e13143cdb6c58c19a0b3f6e857a4ee6973 Mon Sep 17 00:00:00 2001 From: Antoine Wood Date: Sun, 14 Jul 2024 13:15:38 -0400 Subject: [PATCH 1/9] 708 - Add Natural Key Support to Tags model. --- taggit/managers.py | 15 +++++++ taggit/models.py | 35 ++++++++++++++++- tests/tests.py | 98 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 146 insertions(+), 2 deletions(-) diff --git a/taggit/managers.py b/taggit/managers.py index 0ff8e976..6d02eda5 100644 --- a/taggit/managers.py +++ b/taggit/managers.py @@ -1,5 +1,6 @@ import uuid from operator import attrgetter +from typing import List from django.conf import settings from django.contrib.contenttypes.fields import GenericRelation @@ -56,6 +57,20 @@ def clone(self): return type(self)(self.alias, self.col, self.content_types[:]) +class NaturalKeyManager(models.Manager): + def __init__(self, natural_key_fields: List[str], *args, **kwargs): + super().__init__(*args, **kwargs) + self.natural_key_fields = natural_key_fields + + def get_by_natural_key(self, *args): + if len(args) != len(self.model.natural_key_fields): + raise ValueError( + "Number of arguments does not match number of natural key fields." + ) + lookup_kwargs = dict(zip(self.model.natural_key_fields, args)) + return self.get(**lookup_kwargs) + + class _TaggableManager(models.Manager): # TODO investigate whether we can use a RelatedManager instead of all this stuff # to take advantage of all the Django goodness diff --git a/taggit/models.py b/taggit/models.py index 8d7f60bd..43064d43 100644 --- a/taggit/models.py +++ b/taggit/models.py @@ -1,3 +1,5 @@ +from typing import List + from django.conf import settings from django.contrib.contenttypes.fields import GenericForeignKey from django.contrib.contenttypes.models import ContentType @@ -15,7 +17,29 @@ def unidecode(tag): return tag -class TagBase(models.Model): +class NaturalKeyManager(models.Manager): + def __init__(self, natural_key_fields: List[str], *args, **kwargs): + super().__init__(*args, **kwargs) + self.natural_key_fields = natural_key_fields + + def get_by_natural_key(self, *args): + if len(args) != len(self.model.natural_key_fields): + raise ValueError( + "Number of arguments does not match number of natural key fields." + ) + lookup_kwargs = dict(zip(self.model.natural_key_fields, args)) + return self.get(**lookup_kwargs) + + +class NaturalKeyModel(models.Model): + def natural_key(self): + return (getattr(self, field) for field in self.natural_key_fields) + + class Meta: + abstract = True + + +class TagBase(NaturalKeyModel): name = models.CharField( verbose_name=pgettext_lazy("A tag name", "name"), unique=True, max_length=100 ) @@ -26,6 +50,9 @@ class TagBase(models.Model): allow_unicode=True, ) + natural_key_fields = ["name"] + objects = NaturalKeyManager(natural_key_fields) + def __str__(self): return self.name @@ -91,13 +118,15 @@ class Meta: app_label = "taggit" -class ItemBase(models.Model): +class ItemBase(NaturalKeyModel): def __str__(self): return gettext("%(object)s tagged with %(tag)s") % { "object": self.content_object, "tag": self.tag, } + objects = NaturalKeyManager(natural_key_fields=["name"]) + class Meta: abstract = True @@ -170,6 +199,7 @@ def tags_for(cls, model, instance=None, **extra_filters): class GenericTaggedItemBase(CommonGenericTaggedItemBase): object_id = models.IntegerField(verbose_name=_("object ID"), db_index=True) + natural_key_fields = ["object_id"] class Meta: abstract = True @@ -177,6 +207,7 @@ class Meta: class GenericUUIDTaggedItemBase(CommonGenericTaggedItemBase): object_id = models.UUIDField(verbose_name=_("object ID"), db_index=True) + natural_key_fields = ["object_id"] class Meta: abstract = True diff --git a/tests/tests.py b/tests/tests.py index 79489598..7b041db8 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -1,3 +1,4 @@ +import os from io import StringIO from unittest import mock @@ -1398,3 +1399,100 @@ def test_tests_have_no_pending_migrations(self): out = StringIO() call_command("makemigrations", "tests", dry_run=True, stdout=out) self.assertEqual(out.getvalue().strip(), "No changes detected in app 'tests'") + + +class NaturalKeyTests(TestCase): + @classmethod + def setUpTestData(cls): + cls.tag_names = ["circle", "square", "triangle", "rectangle", "pentagon"] + cls.filename = "test_data_dump.json" + cls.tag_count = len(cls.tag_names) + + def setUp(self): + self.tags = self._create_tags() + + def tearDown(self): + self._clear_existing_tags() + try: + os.remove(self.filename) + except FileNotFoundError: + pass + + @property + def _queryset(self): + return Tag.objects.filter(name__in=self.tag_names) + + def _create_tags(self): + return Tag.objects.bulk_create( + [Tag(name=shape, slug=shape) for shape in self.tag_names], + ignore_conflicts=True, + ) + + def _clear_existing_tags(self): + self._queryset.delete() + + def _dump_model(self, model): + model_label = model._meta.label + with open(self.filename, "w") as f: + call_command( + "dumpdata", + model_label, + natural_primary=True, + use_natural_foreign_keys=True, + stdout=f, + ) + + def _load_model(self): + call_command("loaddata", self.filename) + + def test_tag_natural_key(self): + """ + Test that tags can be dumped and loaded using natural keys. + """ + + # confirm count in the DB + self.assertEqual(self._queryset.count(), self.tag_count) + + # dump all tags to a file + self._dump_model(Tag) + + # Delete all tags + self._clear_existing_tags() + + # confirm all tags clear + self.assertEqual(self._queryset.count(), 0) + + # load the tags from the file + self._load_model() + + # confirm count in the DB + self.assertEqual(self._queryset.count(), self.tag_count) + + def test_tag_reloading_with_changed_pk(self): + """Test that tags are not reliant on the primary key of the tag model. + + Test that data is correctly loaded after database state has changed. + + """ + original_shape = self._queryset.first() + original_pk = original_shape.pk + original_shape_name = original_shape.name + new_shape_name = "hexagon" + + # dump all tags to a file + self._dump_model(Tag) + + # Delete the tag + self._clear_existing_tags() + + # create new tag with the same PK + Tag.objects.create(name=new_shape_name, slug=new_shape_name, pk=original_pk) + + # Load the tags from the file + self._load_model() + + # confirm that load did not overwrite the new_shape + self.assertEqual(Tag.objects.get(pk=original_pk).name, new_shape_name) + + # confirm that the original shape was reloaded with a different PK + self.assertNotEqual(Tag.objects.get(name=original_shape_name).pk, original_pk) From 743e9aef05e88eeaf8155b9b521a6d40639ce671 Mon Sep 17 00:00:00 2001 From: Antoine Wood Date: Sun, 14 Jul 2024 13:30:53 -0400 Subject: [PATCH 2/9] 708 - Update ChangeLog --- CHANGELOG.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 28f5cf50..874aa235 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -10,6 +10,7 @@ Changelog We believe that this should not cause a noticable performance change, and the number of queries involved should not change. * Add Django 5.0 support (no code changes were needed, but now we test this release). * Add Python 3.12 support +* Add Support for dumpdata/loaddata using Natural Keys 5.0.1 (2023-10-26) ~~~~~~~~~~~~~~~~~~ From 96394987f532b83e72ab69c5e96f3b32fa143d99 Mon Sep 17 00:00:00 2001 From: Antoine Wood Date: Mon, 15 Jul 2024 19:30:51 -0400 Subject: [PATCH 3/9] 708 - Remove Duplicate Manager Class --- taggit/models.py | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/taggit/models.py b/taggit/models.py index 43064d43..d30f2a23 100644 --- a/taggit/models.py +++ b/taggit/models.py @@ -9,6 +9,8 @@ from django.utils.translation import gettext_lazy as _ from django.utils.translation import pgettext_lazy +from taggit.managers import NaturalKeyManager + try: from unidecode import unidecode except ImportError: @@ -17,20 +19,6 @@ def unidecode(tag): return tag -class NaturalKeyManager(models.Manager): - def __init__(self, natural_key_fields: List[str], *args, **kwargs): - super().__init__(*args, **kwargs) - self.natural_key_fields = natural_key_fields - - def get_by_natural_key(self, *args): - if len(args) != len(self.model.natural_key_fields): - raise ValueError( - "Number of arguments does not match number of natural key fields." - ) - lookup_kwargs = dict(zip(self.model.natural_key_fields, args)) - return self.get(**lookup_kwargs) - - class NaturalKeyModel(models.Model): def natural_key(self): return (getattr(self, field) for field in self.natural_key_fields) From 1da39dc1876b49dea47f6eb0f2a64f3059063b6a Mon Sep 17 00:00:00 2001 From: Antoine Wood Date: Mon, 15 Jul 2024 19:34:07 -0400 Subject: [PATCH 4/9] 708 - Move Natural Key Manager back to models to avoid circular import --- taggit/managers.py | 14 -------------- taggit/models.py | 16 ++++++++++++++-- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/taggit/managers.py b/taggit/managers.py index 6d02eda5..3c89ad91 100644 --- a/taggit/managers.py +++ b/taggit/managers.py @@ -57,20 +57,6 @@ def clone(self): return type(self)(self.alias, self.col, self.content_types[:]) -class NaturalKeyManager(models.Manager): - def __init__(self, natural_key_fields: List[str], *args, **kwargs): - super().__init__(*args, **kwargs) - self.natural_key_fields = natural_key_fields - - def get_by_natural_key(self, *args): - if len(args) != len(self.model.natural_key_fields): - raise ValueError( - "Number of arguments does not match number of natural key fields." - ) - lookup_kwargs = dict(zip(self.model.natural_key_fields, args)) - return self.get(**lookup_kwargs) - - class _TaggableManager(models.Manager): # TODO investigate whether we can use a RelatedManager instead of all this stuff # to take advantage of all the Django goodness diff --git a/taggit/models.py b/taggit/models.py index d30f2a23..43064d43 100644 --- a/taggit/models.py +++ b/taggit/models.py @@ -9,8 +9,6 @@ from django.utils.translation import gettext_lazy as _ from django.utils.translation import pgettext_lazy -from taggit.managers import NaturalKeyManager - try: from unidecode import unidecode except ImportError: @@ -19,6 +17,20 @@ def unidecode(tag): return tag +class NaturalKeyManager(models.Manager): + def __init__(self, natural_key_fields: List[str], *args, **kwargs): + super().__init__(*args, **kwargs) + self.natural_key_fields = natural_key_fields + + def get_by_natural_key(self, *args): + if len(args) != len(self.model.natural_key_fields): + raise ValueError( + "Number of arguments does not match number of natural key fields." + ) + lookup_kwargs = dict(zip(self.model.natural_key_fields, args)) + return self.get(**lookup_kwargs) + + class NaturalKeyModel(models.Model): def natural_key(self): return (getattr(self, field) for field in self.natural_key_fields) From 0de8a3890182c589d9d7a147558359d4449e3548 Mon Sep 17 00:00:00 2001 From: Antoine Wood Date: Mon, 15 Jul 2024 19:40:38 -0400 Subject: [PATCH 5/9] 708 - Remove unused List import --- taggit/managers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/taggit/managers.py b/taggit/managers.py index 3c89ad91..0ff8e976 100644 --- a/taggit/managers.py +++ b/taggit/managers.py @@ -1,6 +1,5 @@ import uuid from operator import attrgetter -from typing import List from django.conf import settings from django.contrib.contenttypes.fields import GenericRelation From 46db2d6d83b8c0b2489c2dd6b26bf78c66c240a7 Mon Sep 17 00:00:00 2001 From: Antoine Wood Date: Wed, 17 Jul 2024 09:15:21 -0400 Subject: [PATCH 6/9] 708 - Clean up Natural Key Manager Implementation --- taggit/models.py | 8 ++------ tests/tests.py | 11 +++++++++++ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/taggit/models.py b/taggit/models.py index 43064d43..410d613e 100644 --- a/taggit/models.py +++ b/taggit/models.py @@ -18,10 +18,6 @@ def unidecode(tag): class NaturalKeyManager(models.Manager): - def __init__(self, natural_key_fields: List[str], *args, **kwargs): - super().__init__(*args, **kwargs) - self.natural_key_fields = natural_key_fields - def get_by_natural_key(self, *args): if len(args) != len(self.model.natural_key_fields): raise ValueError( @@ -51,7 +47,7 @@ class TagBase(NaturalKeyModel): ) natural_key_fields = ["name"] - objects = NaturalKeyManager(natural_key_fields) + objects = NaturalKeyManager() def __str__(self): return self.name @@ -125,7 +121,7 @@ def __str__(self): "tag": self.tag, } - objects = NaturalKeyManager(natural_key_fields=["name"]) + objects = NaturalKeyManager() class Meta: abstract = True diff --git a/tests/tests.py b/tests/tests.py index 7b041db8..38a14e40 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -1496,3 +1496,14 @@ def test_tag_reloading_with_changed_pk(self): # confirm that the original shape was reloaded with a different PK self.assertNotEqual(Tag.objects.get(name=original_shape_name).pk, original_pk) + + def test_get_by_natural_key(self): + # Test retrieval of tags by their natural key + for name in self.tag_names: + tag = Tag.objects.get_by_natural_key(name) + self.assertEqual(tag.name, name) + + def test_wrong_number_of_args(self): + # Test that get_by_natural_key raises an error when the wrong number of args is passed + with self.assertRaises(ValueError): + Tag.objects.get_by_natural_key() From 558ec17aafc27fdd30af1ed538810284244fea4a Mon Sep 17 00:00:00 2001 From: Antoine Wood Date: Wed, 17 Jul 2024 09:23:47 -0400 Subject: [PATCH 7/9] 708 - Remove unused import --- taggit/models.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/taggit/models.py b/taggit/models.py index 410d613e..17613c51 100644 --- a/taggit/models.py +++ b/taggit/models.py @@ -1,5 +1,3 @@ -from typing import List - from django.conf import settings from django.contrib.contenttypes.fields import GenericForeignKey from django.contrib.contenttypes.models import ContentType From 5ead6243419597eeb2f0c1e929dc9316ed7d2cec Mon Sep 17 00:00:00 2001 From: Antoine Wood Date: Sun, 21 Jul 2024 21:07:19 -0400 Subject: [PATCH 8/9] 708 - natural_key return type changed to string instead of generator to allow deserialization with natural-foreign key --- taggit/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taggit/models.py b/taggit/models.py index 17613c51..091d733b 100644 --- a/taggit/models.py +++ b/taggit/models.py @@ -27,7 +27,7 @@ def get_by_natural_key(self, *args): class NaturalKeyModel(models.Model): def natural_key(self): - return (getattr(self, field) for field in self.natural_key_fields) + return [getattr(self, field) for field in self.natural_key_fields] class Meta: abstract = True From c2147e8f6a6ac0507279a4fd2c171c6dae67beb4 Mon Sep 17 00:00:00 2001 From: Raphael Gaschignard Date: Thu, 25 Jul 2024 22:19:31 +1000 Subject: [PATCH 9/9] Add documentation details --- CHANGELOG.rst | 2 +- docs/index.rst | 1 + docs/testing.rst | 14 ++++++++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 docs/testing.rst diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 874aa235..24bc6c62 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -10,7 +10,7 @@ Changelog We believe that this should not cause a noticable performance change, and the number of queries involved should not change. * Add Django 5.0 support (no code changes were needed, but now we test this release). * Add Python 3.12 support -* Add Support for dumpdata/loaddata using Natural Keys +* Add support for dumpdata/loaddata using natural keys 5.0.1 (2023-10-26) ~~~~~~~~~~~~~~~~~~ diff --git a/docs/index.rst b/docs/index.rst index 611c28d3..db978980 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -13,6 +13,7 @@ tagging to your project easy and fun. forms admin serializers + testing api faq custom_tagging diff --git a/docs/testing.rst b/docs/testing.rst new file mode 100644 index 00000000..a18763e7 --- /dev/null +++ b/docs/testing.rst @@ -0,0 +1,14 @@ +Testing +======= + +Natural Key Support +------------------- +We have added `natural key support `_ to the Tag model in the Django taggit library. This allows you to identify objects by human-readable identifiers rather than by their database ID:: + + python manage.py dumpdata taggit.Tag --natural-foreign --natural-primary > tags.json + + python manage.py loaddata tags.json + +By default tags use the name field as the natural key. + +You can customize this in your own custom tag model by setting the ``natural_key_fields`` property on your model the required fields.