Merge branch 'filter-on-fk-rels'

Conflicts:
	versions_tests/tests/test_models.py

versions/models.py

@@ -309,7 +309,7 @@ def get_compiler(self, *args, **kwargs):
         (e.g. by adding a filter to the queryset) does not allow the caching of related
         object to work (they are attached to a queryset; filter() returns a new queryset).
         """
-        if self.querytime.active:
+        if self.querytime.active and (not hasattr(self, '_querytime_filter_added') or not self._querytime_filter_added):
             time = self.querytime.time
             if time is None:
                 self.add_q(Q(version_end_date__isnull=True))
@@ -318,6 +318,9 @@ def get_compiler(self, *args, **kwargs):
                     (Q(version_end_date__gt=time) | Q(version_end_date__isnull=True))
                     & Q(version_start_date__lte=time)
                 )
+            # Ensure applying these filters happens only a single time (even if it doesn't falsify the query, it's
+            # just not very comfortable to read)
+            self._querytime_filter_added = True
         return super(VersionedQuery, self).get_compiler(*args, **kwargs)
 
 
@@ -479,6 +482,30 @@ def get_extra_restriction(self, where_class, alias, remote_alias):
         return where_class([VersionedExtraWhere(historic_sql=historic_sql, current_sql=current_sql, alias=alias,
                                                 remote_alias=remote_alias)])
 
+    def get_joining_columns(self, reverse_join=False):
+        """
+        Get and return joining columns defined by this foreign key relationship
+
+        :return: A tuple containing the column names of the tables to be joined (<local_col_name>, <remote_col_name>)
+        :rtype: tuple
+        """
+        source = self.reverse_related_fields if reverse_join else self.related_fields
+        joining_columns = tuple()
+        for lhs_field, rhs_field in source:
+            lhs_col_name = lhs_field.column
+            rhs_col_name = rhs_field.column
+            # Test whether
+            # - self is the current ForeignKey relationship
+            # - self was not auto_created (e.g. is not part of a M2M relationship)
+            if self is lhs_field and not self.auto_created:
+                if rhs_col_name == Versionable.VERSION_IDENTIFIER_FIELD:
+                    rhs_col_name = Versionable.OBJECT_IDENTIFIER_FIELD
+            elif self is rhs_field and not self.auto_created:
+                if lhs_col_name == Versionable.VERSION_IDENTIFIER_FIELD:
+                    lhs_col_name = Versionable.OBJECT_IDENTIFIER_FIELD
+            joining_columns = joining_columns + ((lhs_col_name, rhs_col_name),)
+        return joining_columns
+
 
 class VersionedManyToManyField(ManyToManyField):
     def __init__(self, *args, **kwargs):
@@ -557,8 +584,8 @@ def create_versioned_many_to_many_intermediary_model(field, cls, field_name):
         return type(str(name), (Versionable,), {
             'Meta': meta,
             '__module__': cls.__module__,
-            from_: VersionedForeignKey(cls, related_name='%s+' % name),
-            to_field_name: VersionedForeignKey(to, related_name='%s+' % name),
+            from_: VersionedForeignKey(cls, related_name='%s+' % name, auto_created=name),
+            to_field_name: VersionedForeignKey(to, related_name='%s+' % name, auto_created=name),
         })
 
 
@@ -921,6 +948,9 @@ class Versionable(models.Model):
     This is pretty much the central point for versioning objects.
     """
 
+    VERSION_IDENTIFIER_FIELD = 'id'
+    OBJECT_IDENTIFIER_FIELD = 'identity'
+
     id = models.CharField(max_length=36, primary_key=True)
     """id stands for ID and is the primary key; sometimes also referenced as the surrogate key"""
 

versions_tests/tests/test_models.py

@@ -679,6 +679,10 @@ def setUp(self):
 
         self.t1 = get_utc_now()
         sleep(0.1)
+        # State at t1
+        # Players: [p1.v1, p2.v1]
+        # Teams: [t.v1]
+        # t.player_set = [p1, p2]
 
         team.player_set.remove(p2)
 
@@ -688,6 +692,10 @@ def setUp(self):
 
         self.t2 = get_utc_now()
         sleep(0.1)
+        # State at t2
+        # Players: [p1.v1, p2.v1, p2.v2]
+        # Teams: [t.v1]
+        # t.player_set = [p1]
 
         team.player_set.remove(p1)
 
@@ -697,6 +705,10 @@ def setUp(self):
 
         self.t3 = get_utc_now()
         sleep(0.1)
+        # State at t3
+        # Players: [p1.v1, p2.v1, p2.v2, p1.v2]
+        # Teams: [t.v1]
+        # t.player_set = []
 
         # Let's get those players back into the game!
         team.player_set.add(p1)
@@ -712,10 +724,18 @@ def setUp(self):
 
         self.t4 = get_utc_now()
         sleep(0.1)
+        # State at t4
+        # Players: [p1.v1, p2.v1, p2.v2, p1.v2, p2.v3, p1.v3]
+        # Teams: [t.v1]
+        # t.player_set = [p1, p2]
 
         p1.delete()
 
         self.t5 = get_utc_now()
+        # State at t4
+        # Players: [p1.v1, p2.v1, p2.v2, p1.v2, p2.v3, p1.v3]
+        # Teams: [t.v1]
+        # t.player_set = [p2]
 
     def test_filtering_on_the_other_side_of_the_relation(self):
         self.assertEqual(1, Team.objects.all().count())
@@ -775,6 +795,8 @@ def test_filtering_for_deleted_player_at_t5(self):
     @skipUnless(connection.vendor == 'sqlite', 'SQL is database specific, only sqlite is tested here.')
     def test_query_created_by_filtering_for_deleted_player_at_t5(self):
         team_none_queryset = Team.objects.as_of(self.t5).filter(player__name__startswith='p1')
+        # Validating the current query prior to analyzing the generated SQL
+        self.assertEqual([], list(team_none_queryset))
         team_none_query = str(team_none_queryset.query)
 
         team_table = Team._meta.db_table
@@ -794,7 +816,7 @@ def test_query_created_by_filtering_for_deleted_player_at_t5(self):
             FROM "{team_table}"
             INNER JOIN
                 "{player_table}" ON (
-                    "{team_table}"."id" = "{player_table}"."team_id"
+                    "{team_table}"."identity" = "{player_table}"."team_id"
                     AND ((
                         {player_table}.version_start_date <= {ts}
                         AND (
@@ -1623,7 +1645,10 @@ def test_select_related(self):
 
     @skipUnless(connection.vendor == 'sqlite', 'SQL is database specific, only sqlite is tested here.')
     def test_select_related_query_sqlite(self):
-        select_related_query = str(Player.objects.as_of(self.t1).select_related('team').all().query)
+        select_related_queryset = Player.objects.as_of(self.t1).select_related('team').all()
+        # Validating the query before verifying the SQL string
+        self.assertEqual(['pl1.v1', 'pl2.v1'], [player.name for player in select_related_queryset])
+        select_related_query = str(select_related_queryset.query)
 
         team_table = Team._meta.db_table
         player_table = Player._meta.db_table
@@ -1645,7 +1670,7 @@ def test_select_related_query_sqlite(self):
                    "{team_table}"."name",
                    "{team_table}"."city_id"
             FROM "{player_table}"
-            LEFT OUTER JOIN "{team_table}" ON ("{player_table}"."team_id" = "{team_table}"."id"
+            LEFT OUTER JOIN "{team_table}" ON ("{player_table}"."team_id" = "{team_table}"."identity"
                                                       AND (({team_table}.version_start_date <= {ts}
                                                             AND ({team_table}.version_end_date > {ts}
                                                                  OR {team_table}.version_end_date IS NULL))))
@@ -1682,7 +1707,7 @@ def test_select_related_query_postgresql(self):
                    "{team_table}"."name",
                    "{team_table}"."city_id"
             FROM "{player_table}"
-            LEFT OUTER JOIN "{team_table}" ON ("{player_table}"."team_id" = "{team_table}"."id"
+            LEFT OUTER JOIN "{team_table}" ON ("{player_table}"."team_id" = "{team_table}"."identity"
                                                       AND (({team_table}.version_start_date <= {ts}
                                                             AND ({team_table}.version_end_date > {ts}
                                                                  OR {team_table}.version_end_date IS NULL))))
@@ -1881,3 +1906,15 @@ def test_accessibility_of_versions_and_non_versionables_via_versioned_fk(self):
         # TODO: Issue #33 on Github aims for a more direct syntax to get to another version of the same object
         should_be_jacques_t1 = should_be_jacques.__class__.objects.as_of(self.t1).get(identity=should_be_jacques.identity)
         self.assertEqual(jacques_t1, should_be_jacques_t1)
+
+
+class FilterOnForeignKeyRelationTest(TestCase):
+    def test_filter_on_fk_relation(self):
+        team = Team.objects.create(name='team')
+        player = Player.objects.create(name='player', team=team)
+        t1 = get_utc_now()
+        sleep(0.1)
+        l1 = len(Player.objects.as_of(t1).filter(team__name='team'))
+        team.clone()
+        l2 = len(Player.objects.as_of(t1).filter(team__name='team'))
+        self.assertEqual(l1, l2)