From 4325591988617ead33d66039bea1a81bf732369d Mon Sep 17 00:00:00 2001 From: Jordan Hyatt <38536669+JordanHyatt@users.noreply.github.com> Date: Wed, 20 Nov 2024 13:36:58 -0500 Subject: [PATCH] HistoricOneToOneField (#1394) * Implemented HistoricOneToOneField with unit tests * Updated AUTHORS.rst * Added HistoricOneToOneFiled to querying_history.rst * removed call to field cache * - created HistoricDescriptorMixin to DRY out get_queryset method - Added suggested doc string to HistoricOneToOneField * Switch to lowest supported python version for pre-commit black --------- Co-authored-by: quadracik Co-authored-by: David Diamant Co-authored-by: Tim Schilling --- .pre-commit-config.yaml | 2 +- AUTHORS.rst | 3 +- CHANGES.rst | 1 + docs/querying_history.rst | 6 + simple_history/models.py | 76 +++++++++++-- simple_history/tests/models.py | 61 +++++++++- simple_history/tests/tests/test_models.py | 132 ++++++++++++++++++++++ 7 files changed, 269 insertions(+), 12 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index abba0c549..0e5536233 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -10,7 +10,7 @@ repos: rev: 24.8.0 hooks: - id: black - language_version: python3.8 + language_version: python3.9 - repo: https://github.com/pycqa/flake8 rev: 7.1.1 diff --git a/AUTHORS.rst b/AUTHORS.rst index 4c38bbbf9..082a4fca1 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -79,7 +79,8 @@ Authors - Jonathan Loo (`alpha1d3d `_) - Jonathan Sanchez - Jonathan Zvesper (`zvesp `_) -- Jordon Wing (`jordonwii `_) +- Jordon Wing (`jordonwii `_) - Josh Fyne - Josh Thomas (`joshuadavidthomas `_) - Keith Hackbarth diff --git a/CHANGES.rst b/CHANGES.rst index 9c44563c3..073d6c3ad 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -7,6 +7,7 @@ Unreleased - Made ``skip_history_when_saving`` work when creating an object - not just when updating an object (gh-1262) - Improved performance of the ``latest_of_each()`` history manager method (gh-1360) +- Added HistoricOneToOneField (gh-1394) 3.7.0 (2024-05-29) ------------------ diff --git a/docs/querying_history.rst b/docs/querying_history.rst index bdf339716..bddd80f63 100644 --- a/docs/querying_history.rst +++ b/docs/querying_history.rst @@ -149,6 +149,7 @@ historic point in time (even if it is the most recent version). You can use `to_historic` to return the historical model that was used to furnish the instance at hand, if it is actually historic. +.. _`HistoricForeignKey`: HistoricForeignKey ------------------ @@ -162,6 +163,11 @@ reverse relationships. See the `HistoricForeignKeyTest` code and models for an example. +HistoricOneToOneField +--------------------- + +Similar to :ref:`HistoricForeignKey`, but for OneToOneFields instead. + most_recent ----------- diff --git a/simple_history/models.py b/simple_history/models.py index 3ffe42a52..008eea9d8 100644 --- a/simple_history/models.py +++ b/simple_history/models.py @@ -18,7 +18,9 @@ from django.db.models.fields.related import ForeignKey from django.db.models.fields.related_descriptors import ( ForwardManyToOneDescriptor, + ForwardOneToOneDescriptor, ReverseManyToOneDescriptor, + ReverseOneToOneDescriptor, create_reverse_many_to_one_manager, ) from django.db.models.query import QuerySet @@ -833,21 +835,16 @@ def transform_field(field): field.serialize = True -class HistoricForwardManyToOneDescriptor(ForwardManyToOneDescriptor): - """ - Overrides get_queryset to provide historic query support, should the - instance be historic (and therefore was generated by a timepoint query) - and the other side of the relation also uses a history manager. - """ +class HistoricDescriptorMixin: - def get_queryset(self, **hints) -> QuerySet: + def get_queryset(self, **hints): instance = hints.get("instance") if instance: history = getattr(instance, SIMPLE_HISTORY_REVERSE_ATTR_NAME, None) histmgr = getattr( - self.field.remote_field.model, + self.get_related_model(), getattr( - self.field.remote_field.model._meta, + self.get_related_model()._meta, "simple_history_manager_attribute", "_notthere", ), @@ -858,6 +855,19 @@ def get_queryset(self, **hints) -> QuerySet: return super().get_queryset(**hints) +class HistoricForwardManyToOneDescriptor( + HistoricDescriptorMixin, ForwardManyToOneDescriptor +): + """ + Overrides get_queryset to provide historic query support, should the + instance be historic (and therefore was generated by a timepoint query) + and the other side of the relation also uses a history manager. + """ + + def get_related_model(self): + return self.field.remote_field.model + + class HistoricReverseManyToOneDescriptor(ReverseManyToOneDescriptor): """ Overrides get_queryset to provide historic query support, should the @@ -922,6 +932,54 @@ class HistoricForeignKey(ForeignKey): related_accessor_class = HistoricReverseManyToOneDescriptor +class HistoricForwardOneToOneDescriptor( + HistoricDescriptorMixin, ForwardOneToOneDescriptor +): + """ + Overrides get_queryset to provide historic query support, should the + instance be historic (and therefore was generated by a timepoint query) + and the other side of the relation also uses a history manager. + """ + + def get_related_model(self): + return self.field.remote_field.model + + +class HistoricReverseOneToOneDescriptor( + HistoricDescriptorMixin, ReverseOneToOneDescriptor +): + """ + Overrides get_queryset to provide historic query support, should the + instance be historic (and therefore was generated by a timepoint query) + and the other side of the relation also uses a history manager. + """ + + def get_related_model(self): + return self.related.related_model + + +class HistoricOneToOneField(models.OneToOneField): + """ + Allows one to one fields to work properly from a historic instance. + + If you use as_of queries to extract historical instances from + a model, and you have other models that are related by one to + one fields and also historic, changing them to a + HistoricOneToOneField field type will allow you to naturally + cross the relationship boundary at the same point in time as + the origin instance. + + A historic instance maintains an attribute ("_historic") when + it is historic, holding the historic record instance and the + timepoint used to query it ("_as_of"). HistoricOneToOneField + looks for this and uses an as_of query against the related + object so the relationship is assessed at the same timepoint. + """ + + forward_related_accessor_class = HistoricForwardOneToOneDescriptor + related_accessor_class = HistoricReverseOneToOneDescriptor + + def is_historic(instance): """ Returns True if the instance was acquired with an as_of timepoint. diff --git a/simple_history/tests/models.py b/simple_history/tests/models.py index f35b5cf6e..c6771302c 100644 --- a/simple_history/tests/models.py +++ b/simple_history/tests/models.py @@ -10,7 +10,11 @@ from simple_history import register from simple_history.manager import HistoricalQuerySet, HistoryManager -from simple_history.models import HistoricalRecords, HistoricForeignKey +from simple_history.models import ( + HistoricalRecords, + HistoricForeignKey, + HistoricOneToOneField, +) from .custom_user.models import CustomUser as User from .external.models import AbstractExternal, AbstractExternal2, AbstractExternal3 @@ -983,3 +987,58 @@ class TestHistoricParticipanToHistoricOrganization(models.Model): related_name="historic_participants", ) history = HistoricalRecords() + + +class TestParticipantToHistoricOrganizationOneToOne(models.Model): + """ + Non-historic table with one to one relationship to historic table. + + In this case it should simply behave like ForeignKey because + the origin model (this one) cannot be historic, so foreign key + lookups are always "current". + """ + + name = models.CharField(max_length=15, unique=True) + organization = HistoricOneToOneField( + TestOrganizationWithHistory, on_delete=CASCADE, related_name="participant" + ) + + +class TestHistoricParticipantToOrganizationOneToOne(models.Model): + """ + Historic table with one to one relationship to non-historic table. + + In this case it should simply behave like OneToOneField because + the origin model (this one) cannot be historic, so one to one field + lookups are always "current". + """ + + name = models.CharField(max_length=15, unique=True) + organization = HistoricOneToOneField( + TestOrganization, on_delete=CASCADE, related_name="participant" + ) + history = HistoricalRecords() + + +class TestHistoricParticipanToHistoricOrganizationOneToOne(models.Model): + """ + Historic table with one to one relationship to historic table. + + In this case as_of queries on the origin model (this one) + or on the target model (the other one) will traverse the + one to one field relationship honoring the timepoint of the + original query. This only happens when both tables involved + are historic. + + NOTE: related_name has to be different than the one used in + TestParticipantToHistoricOrganizationOneToOne as they are + sharing the same target table. + """ + + name = models.CharField(max_length=15, unique=True) + organization = HistoricOneToOneField( + TestOrganizationWithHistory, + on_delete=CASCADE, + related_name="historic_participant", + ) + history = HistoricalRecords() diff --git a/simple_history/tests/tests/test_models.py b/simple_history/tests/tests/test_models.py index d70ca3ce9..224bbabb7 100644 --- a/simple_history/tests/tests/test_models.py +++ b/simple_history/tests/tests/test_models.py @@ -110,10 +110,13 @@ Street, Temperature, TestHistoricParticipanToHistoricOrganization, + TestHistoricParticipanToHistoricOrganizationOneToOne, TestHistoricParticipantToOrganization, + TestHistoricParticipantToOrganizationOneToOne, TestOrganization, TestOrganizationWithHistory, TestParticipantToHistoricOrganization, + TestParticipantToHistoricOrganizationOneToOne, UnicodeVerboseName, UnicodeVerboseNamePlural, UserTextFieldChangeReasonModel, @@ -2841,3 +2844,132 @@ def test_historic_to_historic(self): )[0] pt1i = pt1h.instance self.assertEqual(pt1i.organization.name, "original") + + +class HistoricOneToOneFieldTest(TestCase): + """ + Tests chasing OneToOne foreign keys across time points naturally with + HistoricForeignKey. + """ + + def test_non_historic_to_historic(self): + """ + Non-historic table with one to one relationship to historic table. + + In this case it should simply behave like OneToOneField because + the origin model (this one) cannot be historic, so OneToOneField + lookups are always "current". + """ + org = TestOrganizationWithHistory.objects.create(name="original") + part = TestParticipantToHistoricOrganizationOneToOne.objects.create( + name="part", organization=org + ) + before_mod = timezone.now() + self.assertEqual(part.organization.id, org.id) + self.assertEqual(org.participant, part) + + historg = TestOrganizationWithHistory.history.as_of(before_mod).get( + name="original" + ) + self.assertEqual(historg.participant, part) + + self.assertEqual(org.history.count(), 1) + org.name = "modified" + org.save() + self.assertEqual(org.history.count(), 2) + + # drop internal caches, re-select + part = TestParticipantToHistoricOrganizationOneToOne.objects.get(name="part") + self.assertEqual(part.organization.name, "modified") + + def test_historic_to_non_historic(self): + """ + Historic table OneToOneField to non-historic table. + + In this case it should simply behave like OneToOneField because + the origin model (this one) can be historic but the target model + is not, so foreign key lookups are always "current". + """ + org = TestOrganization.objects.create(name="org") + part = TestHistoricParticipantToOrganizationOneToOne.objects.create( + name="original", organization=org + ) + self.assertEqual(part.organization.id, org.id) + self.assertEqual(org.participant, part) + + histpart = TestHistoricParticipantToOrganizationOneToOne.objects.get( + name="original" + ) + self.assertEqual(histpart.organization.id, org.id) + + def test_historic_to_historic(self): + """ + Historic table with one to one relationship to historic table. + + In this case as_of queries on the origin model (this one) + or on the target model (the other one) will traverse the + foreign key relationship honoring the timepoint of the + original query. This only happens when both tables involved + are historic. + + At t1 we have one org, one participant. + At t2 we have one org, one participant, however the org's name has changed. + """ + org = TestOrganizationWithHistory.objects.create(name="original") + + p1 = TestHistoricParticipanToHistoricOrganizationOneToOne.objects.create( + name="p1", organization=org + ) + t1 = timezone.now() + org.name = "modified" + org.save() + p1.name = "p1_modified" + p1.save() + t2 = timezone.now() + + # forward relationships - see how natural chasing timepoint relations is + p1t1 = TestHistoricParticipanToHistoricOrganizationOneToOne.history.as_of( + t1 + ).get(name="p1") + self.assertEqual(p1t1.organization, org) + self.assertEqual(p1t1.organization.name, "original") + p1t2 = TestHistoricParticipanToHistoricOrganizationOneToOne.history.as_of( + t2 + ).get(name="p1_modified") + self.assertEqual(p1t2.organization, org) + self.assertEqual(p1t2.organization.name, "modified") + + # reverse relationships + # at t1 + ot1 = TestOrganizationWithHistory.history.as_of(t1).all()[0] + self.assertEqual(ot1.historic_participant.name, "p1") + + # at t2 + ot2 = TestOrganizationWithHistory.history.as_of(t2).all()[0] + self.assertEqual(ot2.historic_participant.name, "p1_modified") + + # current + self.assertEqual(org.historic_participant.name, "p1_modified") + + self.assertTrue(is_historic(ot1)) + self.assertFalse(is_historic(org)) + + self.assertIsInstance( + to_historic(ot1), TestOrganizationWithHistory.history.model + ) + self.assertIsNone(to_historic(org)) + + # test querying directly from the history table and converting + # to an instance, it should chase the foreign key properly + # in this case if _as_of is not present we use the history_date + # https://github.com/jazzband/django-simple-history/issues/983 + pt1h = TestHistoricParticipanToHistoricOrganizationOneToOne.history.all()[0] + pt1i = pt1h.instance + self.assertEqual(pt1i.organization.name, "modified") + pt1h = ( + TestHistoricParticipanToHistoricOrganizationOneToOne.history.all().order_by( + "history_date" + )[0] + ) + pt1i = pt1h.instance + self.assertEqual(pt1i.organization.name, "original")