diff --git a/django/core/serializers/base.py b/django/core/serializers/base.py index 0e2e25d946..165c7fa7ff 100644 --- a/django/core/serializers/base.py +++ b/django/core/serializers/base.py @@ -201,7 +201,7 @@ class DeserializedObject(object): models.Model.save_base(self.object, using=using, raw=True, **kwargs) if self.m2m_data and save_m2m: for accessor_name, object_list in self.m2m_data.items(): - setattr(self.object, accessor_name, object_list) + getattr(self.object, accessor_name).set(object_list) # prevent a second (possibly accidental) call to save() from saving # the m2m data twice. diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index 67bf3b88dc..1fcd2dd721 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -1600,7 +1600,7 @@ class ManyToManyField(RelatedField): return getattr(obj, self.attname).all() def save_form_data(self, instance, data): - setattr(instance, self.attname, data) + getattr(instance, self.attname).set(data) def formfield(self, **kwargs): db = kwargs.pop('using', None) diff --git a/django/db/models/fields/related_descriptors.py b/django/db/models/fields/related_descriptors.py index b27b92e405..050866efce 100644 --- a/django/db/models/fields/related_descriptors.py +++ b/django/db/models/fields/related_descriptors.py @@ -62,11 +62,13 @@ and two directions (forward and reverse) for a total of six combinations. from __future__ import unicode_literals +import warnings from operator import attrgetter from django.db import connections, router, transaction from django.db.models import Q, signals from django.db.models.query import QuerySet +from django.utils.deprecation import RemovedInDjango20Warning from django.utils.functional import cached_property @@ -477,6 +479,11 @@ class ReverseManyToOneDescriptor(object): - ``instance`` is the ``parent`` instance - ``value`` in the ``children`` sequence on the right of the equal sign """ + warnings.warn( + 'Direct assignment to the reverse side of a related set is ' + 'deprecated due to the implicit save() that happens. Use %s.set() ' + 'instead.' % self.rel.get_accessor_name(), RemovedInDjango20Warning, stacklevel=2, + ) manager = self.__get__(instance) manager.set(value) diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 39daa0c002..6b5eb95ad0 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -109,6 +109,9 @@ details on these changes. * The ``makemigrations --exit`` option will be removed. +* Support for direct assignment to a reverse foreign key or many-to-many + relation will be removed. + .. _deprecation-removed-in-1.10: 1.10 diff --git a/docs/ref/contrib/auth.txt b/docs/ref/contrib/auth.txt index 217bd432b5..6cbfd88ac6 100644 --- a/docs/ref/contrib/auth.txt +++ b/docs/ref/contrib/auth.txt @@ -337,7 +337,7 @@ Fields Many-to-many field to :class:`~django.contrib.auth.models.Permission`:: - group.permissions = [permission_list] + group.permissions.set([permission_list]) group.permissions.add(permission, permission, ...) group.permissions.remove(permission, permission, ...) group.permissions.clear() diff --git a/docs/ref/models/relations.txt b/docs/ref/models/relations.txt index e89355679a..8671d83d51 100644 --- a/docs/ref/models/relations.txt +++ b/docs/ref/models/relations.txt @@ -179,8 +179,6 @@ Related objects reference ` for a many-to-many relationship, some of the related manager's methods are disabled. -.. _direct-assignment: - Direct Assignment ----------------- @@ -200,3 +198,12 @@ added to the existing related object set. In earlier versions, direct assignment used to perform ``clear()`` followed by ``add()``. It now performs a ``set()`` with the keyword argument ``clear=False``. + +.. deprecated:: 1.10 + + Direct assignment is deprecated in favor of the + :meth:`~django.db.models.fields.related.RelatedManager.set` method:: + + >>> e.related_set.set([obj1, obj2, obj3]) + + This prevents confusion about an assignment resulting in an implicit save. diff --git a/docs/releases/1.10.txt b/docs/releases/1.10.txt index 16e7d2ea28..602e34a6f9 100644 --- a/docs/releases/1.10.txt +++ b/docs/releases/1.10.txt @@ -269,6 +269,21 @@ Miscellaneous Features deprecated in 1.10 =========================== +Direct assignment to a reverse foreign key or many-to-many relation +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Instead of assigning related objects using direct assignment:: + + >>> new_list = [obj1, obj2, obj3] + >>> e.related_set = new_list + +Use the :meth:`~django.db.models.fields.related.RelatedManager.set` method +added in Django 1.9:: + + >>> e.related_set.set([obj1, obj2, obj3]) + +This prevents confusion about an assignment resulting in an implicit save. + Miscellaneous ~~~~~~~~~~~~~ diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt index b73106a670..eb2260aa91 100644 --- a/docs/releases/1.8.txt +++ b/docs/releases/1.8.txt @@ -673,13 +673,12 @@ Related object operations are run in a transaction ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Some operations on related objects such as -:meth:`~django.db.models.fields.related.RelatedManager.add()` or -:ref:`direct assignment` ran multiple data modifying -queries without wrapping them in transactions. To reduce the risk of data -corruption, all data modifying methods that affect multiple related objects -(i.e. ``add()``, ``remove()``, ``clear()``, and :ref:`direct assignment -`) now perform their data modifying queries from within a -transaction, provided your database supports transactions. +:meth:`~django.db.models.fields.related.RelatedManager.add()` or direct +assignment ran multiple data modifying queries without wrapping them in +transactions. To reduce the risk of data corruption, all data modifying methods +that affect multiple related objects (i.e. ``add()``, ``remove()``, +``clear()``, and direct assignment) now perform their data modifying queries +from within a transaction, provided your database supports transactions. This has one backwards incompatible side effect, signal handlers triggered from these methods are now executed within the method's transaction and any diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt index 22b55dd9bc..c526825174 100644 --- a/docs/releases/1.9.txt +++ b/docs/releases/1.9.txt @@ -747,11 +747,10 @@ setuptools is not installed. Related set direct assignment ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -:ref:`Direct assignment ` of related objects in the ORM used -to perform a ``clear()`` followed by a call to ``add()``. This caused -needlessly large data changes and prevented using the -:data:`~django.db.models.signals.m2m_changed` signal to track individual -changes in many-to-many relations. +Direct assignment of related objects in the ORM used to perform a ``clear()`` +followed by a call to ``add()``. This caused needlessly large data changes and +prevented using the :data:`~django.db.models.signals.m2m_changed` signal to +track individual changes in many-to-many relations. Direct assignment now relies on the the new :meth:`~django.db.models.fields.related.RelatedManager.set` method on related diff --git a/docs/topics/auth/default.txt b/docs/topics/auth/default.txt index 15098f6517..5cef47589d 100644 --- a/docs/topics/auth/default.txt +++ b/docs/topics/auth/default.txt @@ -181,11 +181,11 @@ fields: ``groups`` and ``user_permissions``. objects in the same way as any other :doc:`Django model `:: - myuser.groups = [group_list] + myuser.groups.set([group_list]) myuser.groups.add(group, group, ...) myuser.groups.remove(group, group, ...) myuser.groups.clear() - myuser.user_permissions = [permission_list] + myuser.user_permissions.set([permission_list]) myuser.user_permissions.add(permission, permission, ...) myuser.user_permissions.remove(permission, permission, ...) myuser.user_permissions.clear() diff --git a/docs/topics/db/examples/many_to_many.txt b/docs/topics/db/examples/many_to_many.txt index cbf8da99e2..4966ff9272 100644 --- a/docs/topics/db/examples/many_to_many.txt +++ b/docs/topics/db/examples/many_to_many.txt @@ -221,11 +221,11 @@ And from the other end:: >>> a5.publications.all() -Relation sets can be assigned. Assignment clears any existing set members:: +Relation sets can be set:: >>> a4.publications.all() ]> - >>> a4.publications = [p3] + >>> a4.publications.set([p3]) >>> a4.publications.all() ]> @@ -282,18 +282,3 @@ referenced objects should be gone:: >>> p1.article_set.all() ]> - -An alternate to calling -:meth:`~django.db.models.fields.related.RelatedManager.clear` is to assign the -empty set:: - - >>> p1.article_set = [] - >>> p1.article_set.all() - - - >>> a2.publications = [p1, new_publication] - >>> a2.publications.all() - , ]> - >>> a2.publications = [] - >>> a2.publications.all() - diff --git a/docs/topics/db/models.txt b/docs/topics/db/models.txt index 586ec66927..ab6268b485 100644 --- a/docs/topics/db/models.txt +++ b/docs/topics/db/models.txt @@ -510,15 +510,15 @@ the intermediate model:: >>> beatles.members.all() , ]> -Unlike normal many-to-many fields, you *can't* use ``add``, ``create``, -or assignment (i.e., ``beatles.members = [...]``) to create relationships:: +Unlike normal many-to-many fields, you *can't* use ``add()``, ``create()``, +or ``set()`` to create relationships:: # THIS WILL NOT WORK >>> beatles.members.add(john) # NEITHER WILL THIS >>> beatles.members.create(name="George Harrison") # AND NEITHER WILL THIS - >>> beatles.members = [john, paul, ringo, george] + >>> beatles.members.set([john, paul, ringo, george]) Why? You can't just create a relationship between a ``Person`` and a ``Group`` - you need to specify all the detail for the relationship required by the @@ -575,7 +575,6 @@ Another way to access the same information is by querying the >>> ringos_membership.invite_reason 'Needed a new drummer.' - One-to-one relationships ~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/topics/db/queries.txt b/docs/topics/db/queries.txt index 956baa7975..ba026c8c58 100644 --- a/docs/topics/db/queries.txt +++ b/docs/topics/db/queries.txt @@ -1223,12 +1223,11 @@ be found in the :doc:`related objects reference `. ``set(objs)`` Replace the set of related objects. -To assign the members of a related set in one fell swoop, just assign to it -from any iterable object. The iterable can contain object instances, or just -a list of primary key values. For example:: +To assign the members of a related set, use the ``set()`` method with an +iterable of object instances or a list of primary key values. For example:: b = Blog.objects.get(id=1) - b.entry_set = [e1, e2] + b.entry_set.set([e1, e2] In this example, ``e1`` and ``e2`` can be full Entry instances, or integer primary key values. diff --git a/tests/admin_filters/tests.py b/tests/admin_filters/tests.py index 8363a44192..f1756a3c64 100644 --- a/tests/admin_filters/tests.py +++ b/tests/admin_filters/tests.py @@ -263,8 +263,7 @@ class ListFiltersTests(TestCase): title='Gipsy guitar for dummies', year=2002, is_best_seller=True, date_registered=self.one_week_ago, ) - self.gipsy_book.contributors = [self.bob, self.lisa] - self.gipsy_book.save() + self.gipsy_book.contributors.set([self.bob, self.lisa]) # Departments self.dev = Department.objects.create(code='DEV', description='Development') diff --git a/tests/custom_columns/tests.py b/tests/custom_columns/tests.py index dccb400daf..1826eb8db1 100644 --- a/tests/custom_columns/tests.py +++ b/tests/custom_columns/tests.py @@ -15,7 +15,7 @@ class CustomColumnsTests(TestCase): self.authors = [self.a1, self.a2] self.article = Article.objects.create(headline="Django lets you build Web apps easily", primary_author=self.a1) - self.article.authors = self.authors + self.article.authors.set(self.authors) def test_query_all_available_authors(self): self.assertQuerysetEqual( diff --git a/tests/forms_tests/tests/tests.py b/tests/forms_tests/tests/tests.py index a633b2da8a..b3a1cd1f1d 100644 --- a/tests/forms_tests/tests/tests.py +++ b/tests/forms_tests/tests/tests.py @@ -284,7 +284,8 @@ class ManyToManyExclusionTestCase(TestCase): 'multi_choice_int': [opt1.pk], } instance = ChoiceFieldModel.objects.create(**initial) - instance.multi_choice = instance.multi_choice_int = [opt2, opt3] + instance.multi_choice.set([opt2, opt3]) + instance.multi_choice_int.set([opt2, opt3]) form = ChoiceFieldExclusionForm(data=data, instance=instance) self.assertTrue(form.is_valid()) self.assertEqual(form.cleaned_data['multi_choice'], data['multi_choice']) diff --git a/tests/generic_relations/tests.py b/tests/generic_relations/tests.py index 8beee7242a..3d3e2acfe5 100644 --- a/tests/generic_relations/tests.py +++ b/tests/generic_relations/tests.py @@ -313,31 +313,31 @@ class GenericRelationsTests(TestCase): fatty = bacon.tags.create(tag="fatty") salty = bacon.tags.create(tag="salty") - bacon.tags = [fatty, salty] + bacon.tags.set([fatty, salty]) self.assertQuerysetEqual(bacon.tags.all(), [ "", "", ]) - bacon.tags = [fatty] + bacon.tags.set([fatty]) self.assertQuerysetEqual(bacon.tags.all(), [ "", ]) - bacon.tags = [] + bacon.tags.set([]) self.assertQuerysetEqual(bacon.tags.all(), []) 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 - # ManyToManyDescriptor.__set__. Refs #19816. + # ManyRelatedManager.set() (#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 + bacon.tags.set(qs) self.assertEqual(1, bacon.tags.count()) self.assertEqual(1, qs.count()) @@ -705,7 +705,7 @@ class ProxyRelatedModelTest(TestCase): base.save() newrel = ConcreteRelatedModel.objects.create() - newrel.bases = [base] + newrel.bases.set([base]) newrel = ConcreteRelatedModel.objects.get(pk=newrel.pk) self.assertEqual(base, newrel.bases.get()) diff --git a/tests/get_object_or_404/tests.py b/tests/get_object_or_404/tests.py index a5e3d2d8b8..05bea661ed 100644 --- a/tests/get_object_or_404/tests.py +++ b/tests/get_object_or_404/tests.py @@ -16,7 +16,7 @@ class GetObjectOr404Tests(TestCase): self.assertRaises(Http404, get_object_or_404, Article, title="Foo") article = Article.objects.create(title="Run away!") - article.authors = [a1, a2] + article.authors.set([a1, a2]) # get_object_or_404 can be passed a Model to query. self.assertEqual( get_object_or_404(Article, title__contains="Run"), diff --git a/tests/lookup/tests.py b/tests/lookup/tests.py index 49c6f7101e..70a94c96cb 100644 --- a/tests/lookup/tests.py +++ b/tests/lookup/tests.py @@ -677,13 +677,13 @@ class LookupTests(TestCase): season_2011.games.create(home="Houston Astros", away="St. Louis Cardinals") season_2011.games.create(home="Houston Astros", away="Milwaukee Brewers") hunter_pence = Player.objects.create(name="Hunter Pence") - hunter_pence.games = Game.objects.filter(season__year__in=[2009, 2010]) + hunter_pence.games.set(Game.objects.filter(season__year__in=[2009, 2010])) pudge = Player.objects.create(name="Ivan Rodriquez") - pudge.games = Game.objects.filter(season__year=2009) + pudge.games.set(Game.objects.filter(season__year=2009)) pedro_feliz = Player.objects.create(name="Pedro Feliz") - pedro_feliz.games = Game.objects.filter(season__year__in=[2011]) + pedro_feliz.games.set(Game.objects.filter(season__year__in=[2011])) johnson = Player.objects.create(name="Johnson") - johnson.games = Game.objects.filter(season__year__in=[2011]) + johnson.games.set(Game.objects.filter(season__year__in=[2011])) # Games in 2010 self.assertEqual(Game.objects.filter(season__year=2010).count(), 3) diff --git a/tests/m2m_regress/tests.py b/tests/m2m_regress/tests.py index 5e2891b0b4..88b7ebb952 100644 --- a/tests/m2m_regress/tests.py +++ b/tests/m2m_regress/tests.py @@ -74,7 +74,7 @@ class M2MRegressionTests(TestCase): t2 = Tag.objects.create(name='t2') c1 = TagCollection.objects.create(name='c1') - c1.tags = [t1, t2] + c1.tags.set([t1, t2]) c1 = TagCollection.objects.get(name='c1') self.assertQuerysetEqual(c1.tags.all(), ["", ""], ordered=False) @@ -104,10 +104,10 @@ class M2MRegressionTests(TestCase): t1 = Tag.objects.create(name='t1') t2 = Tag.objects.create(name='t2') c1 = TagCollection.objects.create(name='c1') - c1.tags = [t1, t2] + c1.tags.set([t1, t2]) with self.assertRaises(TypeError): - c1.tags = 7 + c1.tags.set(7) c1.refresh_from_db() self.assertQuerysetEqual(c1.tags.order_by('name'), ["", ""]) diff --git a/tests/m2m_signals/tests.py b/tests/m2m_signals/tests.py index da4120e609..daaa5c751c 100644 --- a/tests/m2m_signals/tests.py +++ b/tests/m2m_signals/tests.py @@ -270,7 +270,7 @@ class ManyToManySignalsTest(TestCase): self.assertEqual(self.m2m_changed_messages, expected_messages) # direct assignment clears the set first, then adds - self.vw.default_parts = [self.wheelset, self.doors, self.engine] + self.vw.default_parts.set([self.wheelset, self.doors, self.engine]) expected_messages.append({ 'instance': self.vw, 'action': 'pre_remove', @@ -362,7 +362,7 @@ class ManyToManySignalsTest(TestCase): # Check that signals still work when model inheritance is involved c4 = SportsCar.objects.create(name='Bugatti', price='1000000') c4b = Car.objects.get(name='Bugatti') - c4.default_parts = [self.doors] + c4.default_parts.set([self.doors]) expected_messages.append({ 'instance': c4, 'action': 'pre_add', @@ -407,7 +407,7 @@ class ManyToManySignalsTest(TestCase): def test_m2m_relations_with_self_add_friends(self): self._initialize_signal_person() - self.alice.friends = [self.bob, self.chuck] + self.alice.friends.set([self.bob, self.chuck]) self.assertEqual(self.m2m_changed_messages, [ { 'instance': self.alice, @@ -426,7 +426,7 @@ class ManyToManySignalsTest(TestCase): def test_m2m_relations_with_self_add_fan(self): self._initialize_signal_person() - self.alice.fans = [self.daisy] + self.alice.fans.set([self.daisy]) self.assertEqual(self.m2m_changed_messages, [ { 'instance': self.alice, @@ -445,7 +445,7 @@ class ManyToManySignalsTest(TestCase): def test_m2m_relations_with_self_add_idols(self): self._initialize_signal_person() - self.chuck.idols = [self.alice, self.bob] + self.chuck.idols.set([self.alice, self.bob]) self.assertEqual(self.m2m_changed_messages, [ { 'instance': self.chuck, diff --git a/tests/m2m_through/tests.py b/tests/m2m_through/tests.py index f8149b7c1f..497270eb7a 100644 --- a/tests/m2m_through/tests.py +++ b/tests/m2m_through/tests.py @@ -97,7 +97,7 @@ class M2mThroughTests(TestCase): members = list(Person.objects.filter(name__in=['Bob', 'Jim'])) with self.assertRaisesMessage(AttributeError, msg): - setattr(self.rock, 'members', members) + self.rock.members.set(members) self.assertQuerysetEqual( self.rock.members.all(), @@ -166,7 +166,7 @@ class M2mThroughTests(TestCase): members = list(Group.objects.filter(name__in=['Rock', 'Roll'])) with self.assertRaisesMessage(AttributeError, msg): - setattr(self.bob, 'group_set', members) + self.bob.group_set.set(members) self.assertQuerysetEqual( self.bob.group_set.all(), diff --git a/tests/m2m_through_regress/tests.py b/tests/m2m_through_regress/tests.py index 025ce11e63..82d88a85dc 100644 --- a/tests/m2m_through_regress/tests.py +++ b/tests/m2m_through_regress/tests.py @@ -49,10 +49,22 @@ class M2MThroughTestCase(TestCase): ) def test_cannot_use_setattr_on_reverse_m2m_with_intermediary_model(self): - self.assertRaises(AttributeError, setattr, self.bob, "group_set", []) + msg = ( + "Cannot set values on a ManyToManyField which specifies an " + "intermediary model. Use m2m_through_regress.Membership's Manager " + "instead." + ) + with self.assertRaisesMessage(AttributeError, msg): + self.bob.group_set.set([]) def test_cannot_use_setattr_on_forward_m2m_with_intermediary_model(self): - self.assertRaises(AttributeError, setattr, self.roll, "members", []) + msg = ( + "Cannot set values on a ManyToManyField which specifies an " + "intermediary model. Use m2m_through_regress.Membership's Manager " + "instead." + ) + with self.assertRaisesMessage(AttributeError, msg): + self.roll.members.set([]) def test_cannot_use_create_on_m2m_with_intermediary_model(self): self.assertRaises(AttributeError, self.rock.members.create, name="Anne") diff --git a/tests/many_to_many/tests.py b/tests/many_to_many/tests.py index be18dcedb0..9e10ae4b84 100644 --- a/tests/many_to_many/tests.py +++ b/tests/many_to_many/tests.py @@ -1,8 +1,9 @@ from __future__ import unicode_literals from django.db import transaction -from django.test import TestCase +from django.test import TestCase, ignore_warnings from django.utils import six +from django.utils.deprecation import RemovedInDjango20Warning from .models import Article, InheritedArticleA, InheritedArticleB, Publication @@ -371,8 +372,17 @@ class ManyToManyTests(TestCase): self.a4.publications.set([], clear=True) self.assertQuerysetEqual(self.a4.publications.all(), []) - def test_assign(self): - # Relation sets can be assigned. Assignment clears any existing set members + def test_assign_deprecation(self): + msg = ( + "Direct assignment to the reverse side of a related set is " + "deprecated due to the implicit save() that happens. Use " + "article_set.set() instead." + ) + with self.assertRaisesMessage(RemovedInDjango20Warning, msg): + self.p2.article_set = [self.a4, self.a3] + + @ignore_warnings(category=RemovedInDjango20Warning) + def test_assign_deprecated(self): self.p2.article_set = [self.a4, self.a3] self.assertQuerysetEqual(self.p2.article_set.all(), [ @@ -393,9 +403,29 @@ class ManyToManyTests(TestCase): self.a4.publications = [] self.assertQuerysetEqual(self.a4.publications.all(), []) + def test_assign(self): + # Relation sets can be assigned using set(). + self.p2.article_set.set([self.a4, self.a3]) + self.assertQuerysetEqual( + self.p2.article_set.all(), [ + '', + '', + ] + ) + self.assertQuerysetEqual(self.a4.publications.all(), ['']) + self.a4.publications.set([self.p3.id]) + self.assertQuerysetEqual(self.p2.article_set.all(), ['']) + self.assertQuerysetEqual(self.a4.publications.all(), ['']) + + # An alternate to calling clear() is to set an empty set. + self.p2.article_set.set([]) + self.assertQuerysetEqual(self.p2.article_set.all(), []) + self.a4.publications.set([]) + self.assertQuerysetEqual(self.a4.publications.all(), []) + def test_assign_ids(self): # Relation sets can also be set using primary key values - self.p2.article_set = [self.a4.id, self.a3.id] + self.p2.article_set.set([self.a4.id, self.a3.id]) self.assertQuerysetEqual(self.p2.article_set.all(), [ '', @@ -403,7 +433,7 @@ class ManyToManyTests(TestCase): ]) self.assertQuerysetEqual(self.a4.publications.all(), ['']) - self.a4.publications = [self.p3.id] + self.a4.publications.set([self.p3.id]) self.assertQuerysetEqual(self.p2.article_set.all(), ['']) self.assertQuerysetEqual(self.a4.publications.all(), @@ -412,11 +442,11 @@ class ManyToManyTests(TestCase): 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 - # ManyToManyDescriptor.__set__. Refs #19816. - self.a1.publications = [self.p1, self.p2] + # ManyRelatedManager.set() (#19816). + self.a1.publications.set([self.p1, self.p2]) qs = self.a1.publications.filter(title='The Python Journal') - self.a1.publications = qs + self.a1.publications.set(qs) self.assertEqual(1, self.a1.publications.count()) self.assertEqual(1, qs.count()) @@ -424,11 +454,11 @@ class ManyToManyTests(TestCase): 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 - # ManyToManyDescriptor.__set__. Refs #19816. - self.p1.article_set = [self.a1, self.a2] + # ManyRelatedManager.set() (#19816). + self.p1.article_set.set([self.a1, self.a2]) qs = self.p1.article_set.filter(headline='Django lets you build Web apps easily') - self.p1.article_set = qs + self.p1.article_set.set(qs) self.assertEqual(1, self.p1.article_set.count()) self.assertEqual(1, qs.count()) diff --git a/tests/many_to_one/tests.py b/tests/many_to_one/tests.py index 283baae433..69bba0993d 100644 --- a/tests/many_to_one/tests.py +++ b/tests/many_to_one/tests.py @@ -5,6 +5,7 @@ from django.core.exceptions import FieldError, MultipleObjectsReturned from django.db import models, transaction from django.test import TestCase from django.utils import six +from django.utils.deprecation import RemovedInDjango20Warning from django.utils.translation import ugettext_lazy from .models import ( @@ -123,6 +124,15 @@ class ManyToOneTests(TestCase): ]) self.assertQuerysetEqual(self.r2.article_set.all(), [""]) + def test_reverse_assignment_deprecation(self): + msg = ( + "Direct assignment to the reverse side of a related set is " + "deprecated due to the implicit save() that happens. Use " + "article_set.set() instead." + ) + with self.assertRaisesMessage(RemovedInDjango20Warning, msg): + self.r2.article_set = [] + def test_assign(self): new_article = self.r.article_set.create(headline="John's second story", pub_date=datetime.date(2005, 7, 29)) @@ -141,8 +151,8 @@ class ManyToOneTests(TestCase): ]) self.assertQuerysetEqual(self.r2.article_set.all(), []) - # Set the article back again using set descriptor. - self.r2.article_set = [new_article, new_article2] + # Set the article back again using set() method. + self.r2.article_set.set([new_article, new_article2]) self.assertQuerysetEqual(self.r.article_set.all(), [""]) self.assertQuerysetEqual(self.r2.article_set.all(), [ @@ -150,9 +160,9 @@ class ManyToOneTests(TestCase): "", ]) - # Funny case - assignment notation can only go so far; because the - # ForeignKey cannot be null, existing members of the set must remain. - self.r.article_set = [new_article] + # Because the ForeignKey cannot be null, existing members of the set + # must remain. + self.r.article_set.set([new_article]) self.assertQuerysetEqual(self.r.article_set.all(), [ "", diff --git a/tests/many_to_one_null/tests.py b/tests/many_to_one_null/tests.py index ae5e21e1cf..91c2e13cd7 100644 --- a/tests/many_to_one_null/tests.py +++ b/tests/many_to_one_null/tests.py @@ -94,7 +94,7 @@ class ManyToOneNullTests(TestCase): # Use descriptor assignment to allocate ForeignKey. Null is legal, so # existing members of the set that are not in the assignment set are # set to null. - self.r2.article_set = [self.a2, self.a3] + self.r2.article_set.set([self.a2, self.a3]) self.assertQuerysetEqual(self.r2.article_set.all(), ['', '']) # Clear the rest of the set @@ -106,11 +106,11 @@ class ManyToOneNullTests(TestCase): def test_assign_with_queryset(self): # Ensure that querysets used in reverse FK assignments are pre-evaluated # so their value isn't affected by the clearing operation in - # ReverseManyToOneDescriptor.__set__. Refs #19816. - self.r2.article_set = [self.a2, self.a3] + # RelatedManager.set() (#19816). + self.r2.article_set.set([self.a2, self.a3]) qs = self.r2.article_set.filter(headline="Second") - self.r2.article_set = qs + self.r2.article_set.set(qs) self.assertEqual(1, self.r2.article_set.count()) self.assertEqual(1, qs.count()) diff --git a/tests/model_formsets/tests.py b/tests/model_formsets/tests.py index b39054c7e4..cb93b2fce3 100644 --- a/tests/model_formsets/tests.py +++ b/tests/model_formsets/tests.py @@ -344,7 +344,7 @@ class ModelFormsetTest(TestCase): author3 = Author.objects.create(name='Walt Whitman') meeting = AuthorMeeting.objects.create(created=date.today()) - meeting.authors = Author.objects.all() + meeting.authors.set(Author.objects.all()) # create an Author instance to add to the meeting. diff --git a/tests/model_inheritance/tests.py b/tests/model_inheritance/tests.py index 3c4f71d934..173166a754 100644 --- a/tests/model_inheritance/tests.py +++ b/tests/model_inheritance/tests.py @@ -234,9 +234,9 @@ class ModelInheritanceDataTests(TestCase): def test_related_objects_for_inherited_models(self): # Related objects work just as they normally do. s1 = Supplier.objects.create(name="Joe's Chickens", address="123 Sesame St") - s1.customers = [self.restaurant, self.italian_restaurant] + s1.customers .set([self.restaurant, self.italian_restaurant]) s2 = Supplier.objects.create(name="Luigi's Pasta", address="456 Sesame St") - s2.customers = [self.italian_restaurant] + s2.customers.set([self.italian_restaurant]) # This won't work because the Place we select is not a Restaurant (it's # a Supplier). diff --git a/tests/model_inheritance_regress/tests.py b/tests/model_inheritance_regress/tests.py index 7dc73b6de5..7f6f872b42 100644 --- a/tests/model_inheritance_regress/tests.py +++ b/tests/model_inheritance_regress/tests.py @@ -350,10 +350,10 @@ class ModelInheritanceTest(TestCase): birthday = BirthdayParty.objects.create( name='Birthday party for Alice') - birthday.attendees = [p1, p3] + birthday.attendees.set([p1, p3]) bachelor = BachelorParty.objects.create(name='Bachelor party for Bob') - bachelor.attendees = [p2, p4] + bachelor.attendees.set([p2, p4]) parties = list(p1.birthdayparty_set.all()) self.assertEqual(parties, [birthday]) @@ -371,7 +371,7 @@ class ModelInheritanceTest(TestCase): # ... but it does inherit the m2m from its parent messy = MessyBachelorParty.objects.create( name='Bachelor party for Dave') - messy.attendees = [p4] + messy.attendees.set([p4]) messy_parent = messy.bachelorparty_ptr parties = list(p4.bachelorparty_set.all()) diff --git a/tests/multiple_database/tests.py b/tests/multiple_database/tests.py index f6b529adf7..befc6b0578 100644 --- a/tests/multiple_database/tests.py +++ b/tests/multiple_database/tests.py @@ -177,8 +177,8 @@ class QueryTestCase(TestCase): mark = Person.objects.using('other').create(name="Mark Pilgrim") # Save the author relations - pro.authors = [marty] - dive.authors = [mark] + pro.authors.set([marty]) + dive.authors.set([mark]) # Inspect the m2m tables directly. # There should be 1 entry in each database @@ -224,7 +224,7 @@ class QueryTestCase(TestCase): mark = Person.objects.using('other').create(name="Mark Pilgrim") # Save the author relations - dive.authors = [mark] + dive.authors.set([mark]) # Add a second author john = Person.objects.using('other').create(name="John Smith") @@ -285,7 +285,7 @@ class QueryTestCase(TestCase): mark = Person.objects.using('other').create(name="Mark Pilgrim") # Save the author relations - dive.authors = [mark] + dive.authors.set([mark]) # Create a second book on the other database grease = Book.objects.using('other').create(title="Greasemonkey Hacks", @@ -357,7 +357,7 @@ class QueryTestCase(TestCase): # Set a foreign key set with an object from a different database with self.assertRaises(ValueError): with transaction.atomic(using='default'): - marty.edited = [pro, dive] + marty.edited.set([pro, dive]) # Add to an m2m with an object from a different database with self.assertRaises(ValueError): @@ -367,7 +367,7 @@ class QueryTestCase(TestCase): # Set a m2m with an object from a different database with self.assertRaises(ValueError): with transaction.atomic(using='default'): - marty.book_set = [pro, dive] + marty.book_set.set([pro, dive]) # Add to a reverse m2m with an object from a different database with self.assertRaises(ValueError): @@ -377,7 +377,7 @@ class QueryTestCase(TestCase): # Set a reverse m2m with an object from a different database with self.assertRaises(ValueError): with transaction.atomic(using='other'): - dive.authors = [mark, marty] + dive.authors.set([mark, marty]) def test_m2m_deletion(self): "Cascaded deletions of m2m relations issue queries on the right database" @@ -386,7 +386,7 @@ class QueryTestCase(TestCase): published=datetime.date(2009, 5, 4)) mark = Person.objects.using('other').create(name="Mark Pilgrim") - dive.authors = [mark] + dive.authors.set([mark]) # Check the initial state self.assertEqual(Person.objects.using('default').count(), 0) @@ -414,7 +414,7 @@ class QueryTestCase(TestCase): # Now try deletion in the reverse direction. Set up the relation again dive = Book.objects.using('other').create(title="Dive into Python", published=datetime.date(2009, 5, 4)) - dive.authors = [mark] + dive.authors.set([mark]) # Check the initial state self.assertEqual(Person.objects.using('default').count(), 0) @@ -589,7 +589,7 @@ class QueryTestCase(TestCase): # Set a foreign key set with an object from a different database with self.assertRaises(ValueError): with transaction.atomic(using='default'): - marty.edited = [pro, dive] + marty.edited.set([pro, dive]) # Add to a foreign key set with an object from a different database with self.assertRaises(ValueError): @@ -1095,7 +1095,7 @@ class RouterTestCase(TestCase): pro = Book.objects.using('default').create(title="Pro Django", published=datetime.date(2008, 12, 16), editor=marty) - pro.authors = [marty] + pro.authors.set([marty]) # Create a book and author on the other database Book.objects.using('other').create(title="Dive into Python", @@ -1320,7 +1320,7 @@ class RouterTestCase(TestCase): # Set a m2m set with an object from a different database try: - marty.book_set = [pro, dive] + marty.book_set.set([pro, dive]) except ValueError: self.fail("Assignment across primary/replica databases with a common source should be ok") @@ -1358,7 +1358,7 @@ class RouterTestCase(TestCase): # Set a reverse m2m with an object from a different database try: - dive.authors = [mark, marty] + dive.authors.set([mark, marty]) except ValueError: self.fail("Assignment across primary/replica databases with a common source should be ok") @@ -1861,7 +1861,7 @@ class RouterAttributeErrorTestCase(TestCase): b = Book.objects.create(title="Pro Django", published=datetime.date(2008, 12, 16)) p = Person.objects.create(name="Marty Alchin") - b.authors = [p] + b.authors.set([p]) b.editor = p with self.override_router(): self.assertRaises(AttributeError, b.delete) @@ -1872,7 +1872,8 @@ class RouterAttributeErrorTestCase(TestCase): published=datetime.date(2008, 12, 16)) p = Person.objects.create(name="Marty Alchin") with self.override_router(): - self.assertRaises(AttributeError, setattr, b, 'authors', [p]) + with self.assertRaises(AttributeError): + b.authors.set([p]) class ModelMetaRouter(object): @@ -1898,7 +1899,7 @@ class RouterModelArgumentTestCase(TestCase): # test clear b.authors.clear() # test setattr - b.authors = [p] + b.authors.set([p]) # test M2M collection b.delete() diff --git a/tests/one_to_one/tests.py b/tests/one_to_one/tests.py index ee5d7415e3..f5629a5268 100644 --- a/tests/one_to_one/tests.py +++ b/tests/one_to_one/tests.py @@ -171,7 +171,7 @@ class OneToOneTests(TestCase): """ f = Favorites(name='Fred') f.save() - f.restaurants = [self.r1] + f.restaurants.set([self.r1]) self.assertQuerysetEqual( f.restaurants.all(), [''] diff --git a/tests/queries/tests.py b/tests/queries/tests.py index 05d9ca24a6..471507c5be 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -73,12 +73,12 @@ class Queries1Tests(BaseQuerysetTest): time3 = datetime.datetime(2007, 12, 20, 22, 25, 0) time4 = datetime.datetime(2007, 12, 20, 21, 0, 0) cls.i1 = Item.objects.create(name='one', created=cls.time1, modified=cls.time1, creator=cls.a1, note=cls.n3) - cls.i1.tags = [cls.t1, cls.t2] + cls.i1.tags.set([cls.t1, cls.t2]) cls.i2 = Item.objects.create(name='two', created=cls.time2, creator=cls.a2, note=n2) - cls.i2.tags = [cls.t1, cls.t3] + cls.i2.tags.set([cls.t1, cls.t3]) cls.i3 = Item.objects.create(name='three', created=time3, creator=cls.a2, note=cls.n3) i4 = Item.objects.create(name='four', created=time4, creator=cls.a4, note=cls.n3) - i4.tags = [t4] + i4.tags.set([t4]) cls.r1 = Report.objects.create(name='r1', creator=cls.a1) Report.objects.create(name='r2', creator=a3) @@ -1373,8 +1373,8 @@ class Queries4Tests(BaseQuerysetTest): math101 = tag.note_set.create(note='MATH', misc='101') s1 = tag.annotation_set.create(name='1') s2 = tag.annotation_set.create(name='2') - s1.notes = [math101, anth100] - s2.notes = [math101] + s1.notes.set([math101, anth100]) + s2.notes.set([math101]) result = math101.annotation_set.all() & tag.annotation_set.exclude(notes__in=[anth100]) self.assertEqual(list(result), [s2]) @@ -3348,9 +3348,9 @@ class ManyToManyExcludeTest(TestCase): pg2 = Page.objects.create(text='pg2') pg1 = Page.objects.create(text='pg1') pa1 = Paragraph.objects.create(text='pa1') - pa1.page = [pg1, pg2] + pa1.page.set([pg1, pg2]) pa2 = Paragraph.objects.create(text='pa2') - pa2.page = [pg2, pg3] + pa2.page.set([pg2, pg3]) pa3 = Paragraph.objects.create(text='pa3') ch1 = Chapter.objects.create(title='ch1', paragraph=pa1) ch2 = Chapter.objects.create(title='ch2', paragraph=pa2) diff --git a/tests/serializers/test_data.py b/tests/serializers/test_data.py index 5f0cd45643..350c07487c 100644 --- a/tests/serializers/test_data.py +++ b/tests/serializers/test_data.py @@ -66,7 +66,7 @@ def fk_create(pk, klass, data): def m2m_create(pk, klass, data): instance = klass(id=pk) models.Model.save_base(instance, raw=True) - instance.data = data + instance.data.set(data) return [instance] diff --git a/tests/serializers/tests.py b/tests/serializers/tests.py index 7dc2513fed..af275a8568 100644 --- a/tests/serializers/tests.py +++ b/tests/serializers/tests.py @@ -112,7 +112,7 @@ class SerializersTestBase(object): pub_date=datetime(2006, 6, 16, 11, 00) ) self.a1.save() - self.a1.categories = [sports, op_ed] + self.a1.categories.set([sports, op_ed]) self.a2 = Article( author=self.joe, @@ -120,7 +120,7 @@ class SerializersTestBase(object): pub_date=datetime(2006, 6, 16, 13, 00, 11, 345) ) self.a2.save() - self.a2.categories = [music, op_ed] + self.a2.categories.set([music, op_ed]) def test_serialize(self): """Tests that basic serialization works.""" diff --git a/tests/signals/tests.py b/tests/signals/tests.py index 560fd6e8d3..68fdb416f9 100644 --- a/tests/signals/tests.py +++ b/tests/signals/tests.py @@ -222,9 +222,9 @@ class SignalTests(BaseSignalTest): data[:] = [] # Assigning and removing to/from m2m shouldn't generate an m2m signal. - b1.authors = [a1] + b1.authors.set([a1]) self.assertEqual(data, []) - b1.authors = [] + b1.authors.set([]) self.assertEqual(data, []) finally: signals.pre_save.disconnect(pre_save_handler) diff --git a/tests/unmanaged_models/tests.py b/tests/unmanaged_models/tests.py index 4ed07bf0d2..50b123eef6 100644 --- a/tests/unmanaged_models/tests.py +++ b/tests/unmanaged_models/tests.py @@ -19,7 +19,7 @@ class SimpleTests(TestCase): a = A01.objects.create(f_a="foo", f_b=42) B01.objects.create(fk_a=a, f_a="fred", f_b=1729) c = C01.objects.create(f_a="barney", f_b=1) - c.mm_a = [a] + c.mm_a.set([a]) # ... and pull it out via the other set. a2 = A02.objects.all()[0] diff --git a/tests/update_only_fields/tests.py b/tests/update_only_fields/tests.py index 34519cce4c..c2ff8e1089 100644 --- a/tests/update_only_fields/tests.py +++ b/tests/update_only_fields/tests.py @@ -124,11 +124,9 @@ class UpdateOnlyFieldsTests(TestCase): profile_boss = Profile.objects.create(name='Boss', salary=3000) e1 = Employee.objects.create(name='Sara', gender='F', employee_num=1, profile=profile_boss) - a1 = Account.objects.create(num=1) a2 = Account.objects.create(num=2) - - e1.accounts = [a1, a2] + e1.accounts.set([a1, a2]) with self.assertRaises(ValueError): e1.save(update_fields=['accounts'])