diff --git a/versions/models.py b/versions/models.py index 0e7ea20..ad45208 100644 --- a/versions/models.py +++ b/versions/models.py @@ -318,7 +318,7 @@ def path_stack_to_tables(model_class, paths_stack, tables=None): # This is the counter part of one-to-many field if not direct: table_name = field_object.model._meta.db_table - queryset.related_table_in_filter.add(table_name) + tables.append(table_name) if m2m: if isinstance(field_object, VersionedManyToManyField): @@ -328,6 +328,10 @@ def path_stack_to_tables(model_class, paths_stack, tables=None): tables.append(table_name) + if isinstance(field_object, VersionedForeignKey): + table_name = field_object.rel.to._meta.db_table + tables.append(table_name) + except FieldDoesNotExist: # Of course in some occasion the filed might not be found, # that's accepted @@ -336,22 +340,47 @@ def path_stack_to_tables(model_class, paths_stack, tables=None): if not paths_stack: return tables else: - if isinstance(field_object, VersionedManyToManyField): + if isinstance(field_object, VersionedManyToManyField)\ + or isinstance(field_object, VersionedForeignKey): model_class = field_object.rel.to else: model_class = field_object.model + return path_stack_to_tables(model_class, paths_stack, tables) - for filter_expression in kwargs: + def flatten_Q(q, expressions): + """ + Recursive function that flattens the tree of Q nodes into a list + of filtering expressions. + + For each Q node we visit its children. If the children is a tuple + we have reach the bottom of the tree and we read the first element + of the tuple (which is the filtering expression). If the children + is a Q node we recursively walk down on it. + """ + for c in q.children: + if isinstance(c, tuple): + expressions.append(c[0]) + else: + e = flatten_Q(c, expressions) + expressions.extend(e) + return expressions + + return expressions + + filter_expression_list = [] + + for q in args: + filter_expression_list.extend(flatten_Q(q, [])) + + filter_expression_list.extend(list(kwargs)) + + for filter_expression in filter_expression_list: paths_stack = list(reversed(filter_expression.split(LOOKUP_SEP)[:-1])) if paths_stack: tables = path_stack_to_tables(model_class, paths_stack) queryset.related_table_in_filter = queryset.related_table_in_filter.union(tables) - # #TODO: take the necessary steps for reverse relationships - # relation_set = set(attr_obj.field.rel.through.objects.as_of(self.query_time).values_list('pk')) - # queryset = super(VersionedQuerySet, queryset)._filter_or_exclude(False, **{attr_obj.field._m2m_reverse_name_cache + '__in': list(relation_set)}) - # instance = attr_obj.field.rel.to return queryset diff --git a/versions/tests.py b/versions/tests.py index a6641c5..4232947 100644 --- a/versions/tests.py +++ b/versions/tests.py @@ -13,13 +13,13 @@ # limitations under the License. from django.core.exceptions import SuspiciousOperation, ObjectDoesNotExist, MultipleObjectsReturned, ValidationError +from django.db.models import Q from django.db.models.fields import CharField from django.test import TestCase import datetime from django.utils.timezone import utc from time import sleep import itertools -from unittest import skip from versions.models import Versionable, VersionedForeignKey, VersionedManyToManyField, get_utc_now @@ -58,6 +58,7 @@ class Student(Versionable): def __str__(self): return self.name + class MultiM2MTest(TestCase): """ Testing multiple ManyToMany-relationships on a same class; the following story was chosen: @@ -205,9 +206,9 @@ def test_number_of_queries_stay_constant(self): object was related with 10 others it will require 2 + 2x10 queries to get data. Obviously this is not something one would want and this problem is really difficult to find out as the behavior is correct. There is just too many queries - generated to carry on the work and therefor the system performances sink. + generated to carry on the work and therefore the system's performance sinks. This test is here to make sure we don't go back accidentally to such a situation - by making sure the number of queries stay the same. + by making sure the number of queries stays the same. """ annika = Student.objects.current.get(name='Annika') with self.assertNumQueries(1): @@ -264,14 +265,16 @@ def setUp(self): self.t3 = get_utc_now() def test_filtering_on_the_other_side_of_relation(self): - - language_pupils_count = Pupil.objects.as_of(self.t0).filter(language_teachers__name='Ms. Sue').propagate_querytime().count() + language_pupils_count = Pupil.objects.as_of(self.t0).filter( + language_teachers__name='Ms. Sue').propagate_querytime().count() self.assertEqual(0, language_pupils_count) - language_pupils_count = Pupil.objects.as_of(self.t1).filter(language_teachers__name='Ms. Sue').propagate_querytime().count() + language_pupils_count = Pupil.objects.as_of(self.t1).filter( + language_teachers__name='Ms. Sue').propagate_querytime().count() self.assertEqual(1, language_pupils_count) - language_pupils_count = Pupil.objects.as_of(self.t2).filter(language_teachers__name='Ms. Sue').propagate_querytime().count() + language_pupils_count = Pupil.objects.as_of(self.t2).filter( + language_teachers__name='Ms. Sue').propagate_querytime().count() self.assertEqual(0, language_pupils_count) def test_t0(self): @@ -838,21 +841,21 @@ def test_removing_and_then_adding_again_same_player_on_related_object(self): def test_filtering_on_the_other_side_of_the_relation(self): t1 = get_utc_now() - sleep(0.2) + sleep(0.1) p2 = Player.objects.get(name='p2.v1') self.team.player_set.remove(p2) t2 = get_utc_now() - sleep(0.2) + sleep(0.1) p1 = Player.objects.get(name='p1.v1') self.team.player_set.remove(p1) t3 = get_utc_now() - sleep(0.2) + sleep(0.1) - # self.team.player_set.clear() + # Let's get those players back into the game! p1 = Player.objects.current.get(name='p1.v1') p2 = Player.objects.current.get(name='p2.v1') self.team.player_set.add(p1) @@ -867,34 +870,55 @@ def test_filtering_on_the_other_side_of_the_relation(self): self.assertEqual(1, Player.objects.as_of(t1).filter(name='p1.v1').all().count()) self.assertEqual(1, Player.objects.as_of(t1).filter(name='p2.v1').all().count()) - # at t1 there should be no players in team + # at t1 there should be one team with two players p1 = Team.objects.as_of(t1).filter(player__name='p1.v1').propagate_querytime().first() self.assertIsNotNone(p1) p2 = Team.objects.as_of(t1).filter(player__name='p2.v1').propagate_querytime().first() self.assertIsNotNone(p2) - - p2 = Team.objects.as_of(t1).filter(player__name='p2.v1').propagate_querytime().first() - self.assertIsNotNone(p2) - - # at t2 there should be one player in team: p1.v1 + # at t2 there should be one team with one single player called 'p1.v1' p1 = Team.objects.as_of(t2).filter(player__name='p1.v1').propagate_querytime().first() p2 = Team.objects.as_of(t2).filter(player__name='p2.v1').propagate_querytime().first() self.assertIsNotNone(p1) self.assertIsNone(p2) - # at t3 there should be no players in team + # at t3 there should be one team with no players p1 = Team.objects.as_of(t3).filter(player__name='p1.v1').propagate_querytime().first() p2 = Team.objects.as_of(t3).filter(player__name='p2.v1').propagate_querytime().first() self.assertIsNone(p1) self.assertIsNone(p2) - # at t4 there should be no players in team + # at t4 there should be one team with two players again! p1 = Team.objects.as_of(t4).filter(player__name='p1.v1').first() p2 = Team.objects.as_of(t4).filter(player__name='p2.v1').first() self.assertIsNotNone(p1) self.assertIsNotNone(p2) + def test_simple_filter_using_q_objects(self): + """ + This tests explicitely the filtering of a versioned object using Q objects. + However, since this is done implicetly with every call to 'as_of', this test is redundant but is kept for + explicit test coverage + """ + t1 = get_utc_now() + sleep(0.1) + + p1 = Player.objects.current.get(name__startswith='p1') + self.team.player_set.remove(p1) + + p1 = Player.objects.current.get(name__startswith='p1') + p1.name = 'p1.v2' + p1.save() + + t2 = get_utc_now() + sleep(0.1) + + t1_players = list( + Player.objects.as_of(t1).filter(Q(name__startswith='p1') | Q(name__startswith='p2')).values_list('name', + flat=True)) + self.assertEqual(2, len(t1_players)) + self.assertItemsEqual(t1_players, ['p1.v1', 'p2.v1']) + class Directory(Versionable): name = CharField(max_length=100) @@ -1082,7 +1106,7 @@ def test_filtering_one_jump_with_version_at_t1(self): Test filtering m2m relations with 2 models with propagation of querytime information across all tables """ - should_be_c1 = C1.objects.as_of(self.t1)\ + should_be_c1 = C1.objects.as_of(self.t1) \ .filter(c2s__name__startswith='c2').propagate_querytime().first() self.assertIsNotNone(should_be_c1) @@ -1100,7 +1124,7 @@ def test_filtering_one_jump_reverse_with_version_at_t1(self): information across all tables and navigating the relation in the reverse direction """ - should_be_c3 = C3.objects.as_of(self.t1)\ + should_be_c3 = C3.objects.as_of(self.t1) \ .filter(c2s__name__startswith='c2').propagate_querytime().first() self.assertIsNotNone(should_be_c3) self.assertEqual(should_be_c3.name, 'c3.v1') @@ -1119,12 +1143,12 @@ def test_filtering_two_jumps_with_version_at_t1(self): information across all tables """ with self.assertNumQueries(2) as counter: - should_be_c1 = C1.objects.as_of(self.t1)\ + should_be_c1 = C1.objects.as_of(self.t1) \ .filter(c2s__c3s__name__startswith='c3').propagate_querytime().first() self.assertIsNotNone(should_be_c1) self.assertEqual(should_be_c1.name, 'c1.v1') - count = C1.objects.as_of(self.t1)\ + count = C1.objects.as_of(self.t1) \ .filter(c2s__c3s__name__startswith='c3').propagate_querytime().all().count() self.assertEqual(1, count) @@ -1134,11 +1158,11 @@ def test_filtering_two_jumps_with_version_at_t2(self): information across all tables but this time at point in time t2 """ with self.assertNumQueries(2) as counter: - should_be_c1 = C1.objects.as_of(self.t2)\ + should_be_c1 = C1.objects.as_of(self.t2) \ .filter(c2s__c3s__name__startswith='c3a').propagate_querytime().first() self.assertIsNotNone(should_be_c1) - count = C1.objects.as_of(self.t2)\ + count = C1.objects.as_of(self.t2) \ .filter(c2s__c3s__name__startswith='c3').propagate_querytime().all().count() self.assertEqual(2, count) @@ -1158,12 +1182,12 @@ def test_filtering_two_jumps_reverse_with_version_at_t1(self): direction """ with self.assertNumQueries(2) as counter: - should_be_c3 = C3.objects.as_of(self.t1).\ + should_be_c3 = C3.objects.as_of(self.t1). \ filter(c2s__c1s__name__startswith='c1').propagate_querytime().first() self.assertIsNotNone(should_be_c3) self.assertEqual(should_be_c3.name, 'c3.v1') - count = C3.objects.as_of(self.t1)\ + count = C3.objects.as_of(self.t1) \ .filter(c2s__c1s__name__startswith='c1').propagate_querytime().all().count() self.assertEqual(1, count) @@ -1174,11 +1198,11 @@ def test_filtering_two_jumps_reverse_with_version_at_t2(self): direction but this time at point in time t2 """ with self.assertNumQueries(2) as counter: - should_be_c3 = C3.objects.as_of(self.t2)\ + should_be_c3 = C3.objects.as_of(self.t2) \ .filter(c2s__c1s__name__startswith='c1').propagate_querytime().first() self.assertIsNotNone(should_be_c3) - count = C3.objects.as_of(self.t2)\ + count = C3.objects.as_of(self.t2) \ .filter(c2s__c1s__name__startswith='c1').propagate_querytime().all().count() self.assertEqual(2, count)