Merge pull request #139 from maennel/DJANGO_1.8

Added lastet changes to master

.travis.yml

@@ -1,11 +1,11 @@
 language: python
-python: "2.7"
+python:
+  - "2.7"
+  - "3.6"
 
 env:
-  - TOX_ENV=py27-django18-pg
-  - TOX_ENV=py27-django18-sqlite
-  - TOX_ENV=py34-django18-pg
-  - TOX_ENV=py34-django18-sqlite
+  - TOX_ENV=django18-pg
+  - TOX_ENV=django18-sqlite
 
 # Enable PostgreSQL usage
 addons:
@@ -14,6 +14,7 @@ addons:
 # Dependencies
 install:
   - pip install tox
+  - pip install tox-travis
   - pip install coveralls
 
 # Ensure PostgreSQL-DB to be configured correctly
@@ -25,7 +26,7 @@ before_script:
 
 # Run tests
 script:
-  tox -e $TOX_ENV
+  tox
 
 # Run coveralls
 after_success:

docs/doc/historization_with_cleanerversion.rst

@@ -518,6 +518,12 @@ Versioning an object in a ManyToMany relationship requires 3 steps to be done, i
   .. image:: ../images/clone_m2m_item_3.png
         :align: center
 
+The records in ManyToMany intermediary tables are versioned: they have ``version_birth_date``,
+``version_start_date`` and ``version_end_date`` columns.  The ForeignKey columns in ManyToMany
+Intermediary tables store the ``id`` of the referenced records.  Note that this is different than
+the VersionedForeignKeys in Versionable models, which store the ``identity`` of the referenced objects.
+This is transparent in normal usage, but can be important to keep in mind when you need to write a query
+that directly references the ForeignKey columns.
 
 Removing objects from a versioned M2M relationship
 --------------------------------------------------
@@ -542,29 +548,40 @@ Notes about using prefetch_related
 simple sting lookups or `Prefetch <https://docs.djangoproject.com/en/1.8/ref/models/querysets/#django.db.models.Prefetch>`_
 objects.
 
-When using ``prefetch_related`` with CleanerVersion, be aware that when using a ``Prefetch`` object **that
-specifies a queryset**, that you need to explicitly specify the ``as_of`` value, or use ``current``.
-A ``Prefetch`` queryset will not be automatically time-restricted based on the base queryset.
+When using ``prefetch_related`` with CleanerVersion, the generated query that fetches the related  objects will
+be time-restricted based on the base queryset.  If you provide a ``Prefetch`` object that specifies a queryset, the
+queryset must either not be time-limited (using ``.as_of()`` or ``.current``), or be time-limited with the same
+``.as_of`` or ``.current`` as the base queryset.  If the ``Prefetch`` queryset is not time-limited, but the base
+queryset is, the ``Prefetch`` queryset will adopt the same time limitation as the base queryset.
 
-For example, assuming you want everything at the time ``end_of_last_month``, do this::
+For example, assuming you want everything at the time ``end_of_last_month``, you can do this::
 
+    # Prefetch queryset is not explicitly time-restricted, and will adopt the base queryset's time-restriction.
+    disciplines_prefetch = Prefetch(
+        'sportsclubs__discipline_set',
+        queryset=Discipline.objects.filter('name__startswith'='B'))
+    people_last_month = Person.objects.as_of(end_of_last_month).prefetch_related(disciplines_prefetch)
+
+or this::
+
+    # Prefetch queryset is explicitly time-restricted with the same time restriction as the base queryset.
     disciplines_prefetch = Prefetch(
         'sportsclubs__discipline_set',
         queryset=Discipline.objects.as_of(end_of_last_month).filter('name__startswith'='B'))
     people_last_month = Person.objects.as_of(end_of_last_month).prefetch_related(disciplines_prefetch)
 
-On the other hand, the following ``Prefetch``, without an ``as_of`` in the queryset, would result in all
-matching ``Discipline`` objects being returned, regardless whether they existed at ``end_of_last_month``
-or not::
+However, the following ``Prefetch``, without a time restriction that differs from the base queryset, will
+raise a ``ValueError`` when evaluated::
 
-    # Don't do this, the Prefetch queryset is missing an as_of():
+    # Don't do this, the Prefetch queryset's time restriction doesn't match it's parent's:
     disciplines_prefetch = Prefetch(
         'sportsclubs__discipline_set',
-        queryset=Discipline.objects.filter('name__startswith'='B'))
+        queryset=Discipline.objects.current.filter('name__startswith'='B'))
     people_last_month = Person.objects.as_of(end_of_last_month).prefetch_related(disciplines_prefetch)
 
+
 If a ``Prefetch`` without an explicit queryset is used, or a simple string lookup, the generated queryset will
-be appropriately time-restricted.  The following statements will propagate the base queries'
+be appropriately time-restricted.  The following statements will propagate the base query's
 ``as_of`` value to the generated related-objects queryset::
 
     people1 = Person.objects.as_of(end_of_last_month).prefetch_related(Prefetch('sportsclubs__discipline_set'))

setup.py

@@ -21,7 +21,7 @@
 """
 
 setup(name='CleanerVersion',
-      version='1.6.1',
+      version='1.6.2',
       description='A versioning solution for relational data models using the Django ORM',
       long_description='CleanerVersion is a solution that allows you to read and write multiple versions of an entry '
                        'to and from your relational database. It allows to keep track of modifications on an object '

tox.ini

@@ -5,7 +5,7 @@
 
 [tox]
 envlist =
-	py{27,34}-django{18}-{sqlite,pg}
+	django{18}-{sqlite,pg}
 
 [testenv]
 deps =

versions/models.py

@@ -784,6 +784,61 @@ def __get__(self, instance, instance_type=None):
         else:
             return current_elt.__class__.objects.current.get(identity=current_elt.identity)
 
+    def get_prefetch_queryset(self, instances, queryset=None):
+        """
+        Overrides the parent method to:
+        - force queryset to use the querytime of the parent objects
+        - ensure that the join is done on identity, not id
+        - make the cache key identity, not id.
+        """
+        if queryset is None:
+            queryset = self.get_queryset()
+        queryset._add_hints(instance=instances[0])
+
+        # CleanerVersion change 1: force the querytime to be the same as the prefetched-for instance.
+        # This is necessary to have reliable results and avoid extra queries for cache misses when
+        # accessing the child objects from their parents (e.g. choice.poll).
+        instance_querytime = instances[0]._querytime
+        if instance_querytime.active:
+            if queryset.querytime.active and queryset.querytime.time != instance_querytime.time:
+                raise ValueError("A Prefetch queryset that specifies an as_of time must match "
+                                 "the as_of of the base queryset.")
+            else:
+                queryset.querytime = instance_querytime
+
+        # CleanerVersion change 2: make rel_obj_attr return a tuple with the object's identity.
+        # rel_obj_attr = self.field.get_foreign_related_value
+        def versioned_fk_rel_obj_attr(versioned_rel_obj):
+            return versioned_rel_obj.identity,
+        rel_obj_attr = versioned_fk_rel_obj_attr
+        instance_attr = self.field.get_local_related_value
+        instances_dict = {instance_attr(inst): inst for inst in instances}
+        # CleanerVersion change 3: fake the related field so that it provides a name of 'identity'.
+        # related_field = self.field.foreign_related_fields[0]
+        related_field = namedtuple('VersionedRelatedFieldTuple', 'name')('identity')
+
+        # FIXME: This will need to be revisited when we introduce support for
+        # composite fields. In the meantime we take this practical approach to
+        # solve a regression on 1.6 when the reverse manager in hidden
+        # (related_name ends with a '+'). Refs #21410.
+        # The check for len(...) == 1 is a special case that allows the query
+        # to be join-less and smaller. Refs #21760.
+        if self.field.rel.is_hidden() or len(self.field.foreign_related_fields) == 1:
+            query = {'%s__in' % related_field.name: set(instance_attr(inst)[0] for inst in instances)}
+            # query = {'identity__in': set(instance_attr(inst)[0] for inst in instances)}
+        else:
+            query = {'%s__in' % self.field.related_query_name(): instances}
+        queryset = queryset.filter(**query)
+
+        # Since we're going to assign directly in the cache,
+        # we must manage the reverse relation cache manually.
+        if not self.field.rel.multiple:
+            rel_obj_cache_name = self.field.rel.get_cache_name()
+            for rel_obj in queryset:
+                instance = instances_dict[rel_obj_attr(rel_obj)]
+                setattr(rel_obj, rel_obj_cache_name, instance)
+        return queryset, rel_obj_attr, instance_attr, True, self.cache_name
+
 
 class VersionedForeignRelatedObjectsDescriptor(ForeignRelatedObjectsDescriptor):
     """
@@ -830,6 +885,13 @@ def get_prefetch_queryset(self, instances, queryset=None):
 
                 queryset._add_hints(instance=instances[0])
                 queryset = queryset.using(queryset._db or self._db)
+                instance_querytime = instances[0]._querytime
+                if instance_querytime.active:
+                    if queryset.querytime.active and queryset.querytime.time != instance_querytime.time:
+                        raise ValueError("A Prefetch queryset that specifies an as_of time must match "
+                                         "the as_of of the base queryset.")
+                    else:
+                        queryset.querytime = instance_querytime
 
                 rel_obj_attr = rel_field.get_local_related_value
                 instance_attr = rel_field.get_foreign_related_value

versions_tests/tests/test_models.py

@@ -2313,6 +2313,21 @@ def test_reverse_fk_prefetch_queryset_with_historic_versions(self):
             team = [t for t in current_city.prefetched_teams if t.name == 'team1.v2'][0]
             self.assertSetEqual({'pl1.v2', 'pl2.v1'}, {p.name for p in team.prefetched_players})
 
+        # When a different time is specified for the prefetch queryset than for the base queryset:
+        with self.assertRaises(ValueError):
+            _ = City.objects.current.filter(name='city.v2').prefetch_related(
+                Prefetch(
+                    'team_set',
+                    queryset=Team.objects.as_of(self.time1),
+                    to_attr='prefetched_teams'
+                ),
+                Prefetch(
+                    'prefetched_teams__player_set',
+                    queryset=Player.objects.as_of(self.time1),
+                    to_attr='prefetched_players'
+                ),
+            )[0]
+
     def test_reverse_fk_simple_prefetch_with_historic_versions(self):
         """
         prefetch_related with simple lookup.
@@ -2367,6 +2382,75 @@ def test_reverse_fk_simple_prefetch_with_historic_versions(self):
             team = [t for t in current_city.team_set.all() if t.name == 'team1.v2'][0]
             self.assertSetEqual({'pl1.v2', 'pl2.v1'}, {p.name for p in team.player_set.all()})
 
+    def test_foreign_key_prefetch_with_historic_version(self):
+        self.modify_objects()
+        historic_city = City.objects.as_of(self.time1).get(identity=self.c1.identity)
+
+        # Test with a simple prefetch.
+        with self.assertNumQueries(2):
+            team = Team.objects.as_of(self.time1).filter(
+                identity=self.t1.identity
+            ).prefetch_related(
+                'city'
+            )[0]
+            self.assertIsNotNone(team.city)
+            self.assertEquals(team.city.id, historic_city.id)
+
+        # Test with a Prefetch object without a queryset.
+        with self.assertNumQueries(2):
+            team = Team.objects.as_of(self.time1).filter(
+                identity=self.t1.identity
+            ).prefetch_related(Prefetch(
+                'city',
+            ))[0]
+            self.assertIsNotNone(team.city)
+            self.assertEquals(team.city.id, historic_city.id)
+
+        # Test with a Prefetch object with a queryset with an explicit as_of.
+        with self.assertNumQueries(2):
+            team = Team.objects.as_of(self.time1).filter(
+                identity=self.t1.identity
+            ).prefetch_related(Prefetch(
+                'city',
+                queryset=City.objects.as_of(self.time1)
+            ))[0]
+            self.assertIsNotNone(team.city)
+            self.assertEquals(team.city.id, historic_city.id)
+
+        # Test with a Prefetch object with a queryset with no as_of.
+        with self.assertNumQueries(2):
+            team = Team.objects.as_of(self.time1).filter(
+                identity=self.t1.identity
+            ).prefetch_related(Prefetch(
+                'city',
+                queryset=City.objects.all()
+            ))[0]
+            self.assertIsNotNone(team.city)
+            self.assertEquals(team.city.id, historic_city.id)
+
+        # Test with a Prefetch object with a queryset with an as_of that differs from the parents.
+        # If permitted, it would lead to possibly incorrect results and definitely cache misses,
+        # which would defeat the purpose of using prefetch_related.  So a ValueError should be raised.
+        with self.assertRaises(ValueError):
+            team = Team.objects.as_of(self.time1).filter(
+                identity=self.t1.identity
+            ).prefetch_related(Prefetch(
+                'city',
+                queryset=City.objects.current
+            ))[0]
+
+        # Test with a Prefetch object with a queryset with an as_of, when the parent has no as_of.
+        # This is a bit of an odd thing to do, but possible.
+        with self.assertNumQueries(2):
+            team = Team.objects.filter(
+                identity=self.t1.identity
+            ).prefetch_related(Prefetch(
+                'city',
+                queryset=City.objects.as_of(self.time1)
+            ))[0]
+            self.assertIsNotNone(team.city)
+            self.assertEquals(team.city.id, historic_city.id)
+
 
 class IntegrationNonVersionableModelsTests(TestCase):
     def setUp(self):