Commit 20399083 authored by Loic Bistuer's avatar Loic Bistuer
Browse files

Fixed #19816 -- Pre-evaluate querysets used in direct relation assignments. 

Since assignments on M2M or reverse FK descriptors is composed of a `clear()`,
followed by an `add()`, `clear()` could potentially affect the value of the
assigned queryset before the `add()` step; pre-evaluating it solves the problem.

This patch fixes the issue for ForeignRelatedObjectsDescriptor,
ManyRelatedObjectsDescriptor, and ReverseGenericRelatedObjectsDescriptor.
It completes 6cb6e1 which addressed ReverseManyRelatedObjectsDescriptor.
parent 2f3e1fe3

django/contrib/contenttypes/fields.py

+4 −1
Original line number Diff line number Diff line
@@ -393,8 +393,11 @@ class ReverseGenericRelatedObjectsDescriptor(object): 
        return manager



    def __set__(self, instance, value):

        manager = self.__get__(instance)

        # Force evaluation of `value` in case it's a queryset whose

        # value could be affected by `manager.clear()`. Refs #19816.

        value = tuple(value)



        manager = self.__get__(instance)

        db = router.db_for_write(manager.model, instance=manager.instance)

        with transaction.atomic(using=db, savepoint=False):

            manager.clear()

django/db/models/fields/related.py

+11 −6
Original line number Diff line number Diff line
@@ -763,8 +763,11 @@ class ForeignRelatedObjectsDescriptor(object): 
        return self.related_manager_cls(instance)



    def __set__(self, instance, value):

        manager = self.__get__(instance)

        # Force evaluation of `value` in case it's a queryset whose

        # value could be affected by `manager.clear()`. Refs #19816.

        value = tuple(value)



        manager = self.__get__(instance)

        db = router.db_for_write(manager.model, instance=manager.instance)

        with transaction.atomic(using=db, savepoint=False):

            # If the foreign key can support nulls, then completely clear the related set.
@@ -1113,8 +1116,11 @@ class ManyRelatedObjectsDescriptor(object): 
            opts = self.related.field.rel.through._meta

            raise AttributeError("Cannot set values on a ManyToManyField which specifies an intermediary model. Use %s.%s's Manager instead." % (opts.app_label, opts.object_name))



        manager = self.__get__(instance)

        # Force evaluation of `value` in case it's a queryset whose

        # value could be affected by `manager.clear()`. Refs #19816.

        value = tuple(value)



        manager = self.__get__(instance)

        db = router.db_for_write(manager.through, instance=manager.instance)

        with transaction.atomic(using=db, savepoint=False):

            manager.clear()
@@ -1170,12 +1176,11 @@ class ReverseManyRelatedObjectsDescriptor(object): 
            opts = self.field.rel.through._meta

            raise AttributeError("Cannot set values on a ManyToManyField which specifies an intermediary model.  Use %s.%s's Manager instead." % (opts.app_label, opts.object_name))



        manager = self.__get__(instance)



        # clear() can change expected output of 'value' queryset, we force evaluation

        # of queryset before clear; ticket #19816

        # Force evaluation of `value` in case it's a queryset whose

        # value could be affected by `manager.clear()`. Refs #19816.

        value = tuple(value)



        manager = self.__get__(instance)

        db = router.db_for_write(manager.through, instance=manager.instance)

        with transaction.atomic(using=db, savepoint=False):

            manager.clear()

tests/generic_relations/tests.py

+15 −0
Original line number Diff line number Diff line
@@ -163,6 +163,21 @@ class GenericRelationsTests(TestCase): 
            "<Animal: Platypus>"

        ])



    def test_assign_with_queryset(self):

        # Ensure that querysets used in reverse GFK assignments are pre-evaluated

        # so their value isn't affected by the clearing operation in

        # ManyRelatedObjectsDescriptor.__set__. Refs #19816.

        bacon = Vegetable.objects.create(name="Bacon", is_yucky=False)

        bacon.tags.create(tag="fatty")

        bacon.tags.create(tag="salty")

        self.assertEqual(2, bacon.tags.count())



        qs = bacon.tags.filter(tag="fatty")

        bacon.tags = qs



        self.assertEqual(1, bacon.tags.count())

        self.assertEqual(1, qs.count())



    def test_generic_relation_related_name_default(self):

        # Test that GenericRelation by default isn't usable from

        # the reverse side.

tests/m2m_regress/tests.py

+0 −12
Original line number Diff line number Diff line
@@ -97,15 +97,3 @@ class M2MRegressionTests(TestCase): 
        # causes a TypeError in add_lazy_relation

        m1 = RegressionModelSplit(name='1')

        m1.save()
 


    def test_m2m_filter(self):

        worksheet = Worksheet.objects.create(id=1)

        line_hi = Line.objects.create(name="hi")

        line_bye = Line.objects.create(name="bye")



        worksheet.lines = [line_hi, line_bye]

        hi = worksheet.lines.filter(name="hi")



        worksheet.lines = hi

        self.assertEqual(1, worksheet.lines.count())

        self.assertEqual(1, hi.count())

tests/many_to_many/tests.py

+24 −0
Original line number Diff line number Diff line
@@ -370,6 +370,30 @@ class ManyToManyTests(TestCase): 
        self.assertQuerysetEqual(self.a4.publications.all(),

                                 ['<Publication: Science Weekly>'])



    def test_forward_assign_with_queryset(self):

        # Ensure that querysets used in m2m assignments are pre-evaluated

        # so their value isn't affected by the clearing operation in

        # ManyRelatedObjectsDescriptor.__set__. Refs #19816.

        self.a1.publications = [self.p1, self.p2]



        qs = self.a1.publications.filter(id=self.a1.id)

        self.a1.publications = qs



        self.assertEqual(1, self.a1.publications.count())

        self.assertEqual(1, qs.count())



    def test_reverse_assign_with_queryset(self):

        # Ensure that querysets used in M2M assignments are pre-evaluated

        # so their value isn't affected by the clearing operation in

        # ReverseManyRelatedObjectsDescriptor.__set__. Refs #19816.

        self.p1.article_set = [self.a1, self.a2]



        qs = self.p1.article_set.filter(id=self.p1.id)

        self.p1.article_set = qs



        self.assertEqual(1, self.p1.article_set.count())

        self.assertEqual(1, qs.count())



    def test_clear(self):

        # Relation sets can be cleared:

        self.p2.article_set.clear()