Ticket #6707: 6707_m2m_set.diff

docs/ref/signals.txt

@@ -332 +332 @@
 ``using``       ``"default"`` (since the default router sends writes here)
 ==============  ============================================================
 
+We can also assign directly to the relation directly::
+
+    >>> p.toppings = [t]
+
+and the :data:`m2m_handler` will receive ``"pre_remove"`` and
+``"post_remove"`` actions, if there were ``Topping`` objects to
+remove. These are followed by ``"pre_add"`` and ``"post_add"`` actions.
+
+Assigning an empty iterable::
+
+   >>> p.toppings = []
+
+will send ``"pre_clear"`` and ``"post_clear"`` actions to the
+:data:`m2m_handler`.
+
+
 class_prepared
 --------------
 

tests/modeltests/m2m_signals/tests.py

@@ -252 +252 @@
         })
         self.assertEqual(self.m2m_changed_messages, expected_messages)
 
-        # direct assignment clears the set first, then adds
+        # direct assignment removes objects from the set first, then adds
         self.vw.default_parts = [self.wheelset,self.doors,self.engine]
         expected_messages.append({
             'instance': self.vw,
-            'action': 'pre_clear',
+            'action': 'pre_remove',
             'reverse': False,
             'model': Part,
+            'objects': [p6],
         })
         expected_messages.append({
             'instance': self.vw,
-            'action': 'post_clear',
+            'action': 'post_remove',
             'reverse': False,
             'model': Part,
+            'objects': [p6],
         })
         expected_messages.append({
             'instance': self.vw,
@@ -282 +284 @@
         })
         self.assertEqual(self.m2m_changed_messages, expected_messages)
 
-        # 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]
+        # direct assignment can clear objects, if iterable is empty
+        self.vw.default_parts = []
         expected_messages.append({
-            'instance': c4,
+            'instance': self.vw,
             'action': 'pre_clear',
             'reverse': False,
             'model': Part,
         })
         expected_messages.append({
-            'instance': c4,
+            'instance': self.vw,
             'action': 'post_clear',
             'reverse': False,
             'model': Part,
         })
+        self.assertEqual(self.m2m_changed_messages, expected_messages)
+
+        # 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]
         expected_messages.append({
             'instance': c4,
             'action': 'pre_add',
@@ -344 +350 @@
         self.alice.friends = [self.bob, self.chuck]
         expected_messages.append({
             'instance': self.alice,
-            'action': 'pre_clear',
-            'reverse': False,
-            'model': Person,
-        })
-        expected_messages.append({
-            'instance': self.alice,
-            'action': 'post_clear',
-            'reverse': False,
-            'model': Person,
-        })
-        expected_messages.append({
-            'instance': self.alice,
             'action': 'pre_add',
             'reverse': False,
             'model': Person,
@@ -373 +367 @@
         self.alice.fans = [self.daisy]
         expected_messages.append({
             'instance': self.alice,
-            'action': 'pre_clear',
-            'reverse': False,
-            'model': Person,
-        })
-        expected_messages.append({
-            'instance': self.alice,
-            'action': 'post_clear',
-            'reverse': False,
-            'model': Person,
-        })
-        expected_messages.append({
-            'instance': self.alice,
             'action': 'pre_add',
             'reverse': False,
             'model': Person,
@@ -402 +384 @@
         self.chuck.idols = [self.alice,self.bob]
         expected_messages.append({
             'instance': self.chuck,
-            'action': 'pre_clear',
-            'reverse': True,
-            'model': Person,
-        })
-        expected_messages.append({
-            'instance': self.chuck,
-            'action': 'post_clear',
-            'reverse': True,
-            'model': Person,
-        })
-        expected_messages.append({
-            'instance': self.chuck,
             'action': 'pre_add',
             'reverse': True,
             'model': Person,

django/db/models/fields/related.py

@@ -574 +574 @@
         # If the ManyToMany relation has an intermediary model,
         # the add and remove methods do not exist.
         if rel.through._meta.auto_created:
-            def add(self, *objs):
-                self._add_items(self.source_field_name, self.target_field_name, *objs)
+            def add(self, *objs, **kwargs):
+                check_duplicates = kwargs.get('check_duplicates', True)
+                self._add_items(self.source_field_name, self.target_field_name,
+                    check_duplicates, *objs)
 
                 # If this is a symmetrical m2m relation to self, add the mirror entry in the m2m table
                 if self.symmetrical:
-                    self._add_items(self.target_field_name, self.source_field_name, *objs)
+                    self._add_items(self.target_field_name, self.source_field_name,
+                        check_duplicates, *objs)
             add.alters_data = True
 
             def remove(self, *objs):
@@ -621 +624 @@
             return obj, created
         get_or_create.alters_data = True
 
-        def _add_items(self, source_field_name, target_field_name, *objs):
+        def _check_new_ids(self, objs):
+            from django.db.models import Model
+            new_ids = set()
+            for obj in objs:
+                if isinstance(obj, self.model):
+                    if not router.allow_relation(obj, self.instance):
+                       raise ValueError('Cannot add "%r": instance is on database "%s", value is on database "%s"' %
+                                           (obj, self.instance._state.db, obj._state.db))
+                    new_ids.add(obj.pk)
+                elif isinstance(obj, Model):
+                    raise TypeError("'%s' instance expected, got %r" % (self.model._meta.object_name, obj))
+                else:
+                    new_ids.add(obj)
+            return new_ids
+
+        def _add_items(self, source_field_name, target_field_name, check_duplicates=True, *objs):
             # source_field_name: the PK fieldname in join table for the source object
             # target_field_name: the PK fieldname in join table for the target object
+            # check_duplicates: Checks to avoid adding duplicate objects if True.
             # *objs - objects to add. Either object instances, or primary keys of object instances.
 
             # If there aren't any objects, there is nothing to do.
             from django.db.models import Model
             if objs:
-                new_ids = set()
-                for obj in objs:
-                    if isinstance(obj, self.model):
-                        if not router.allow_relation(obj, self.instance):
-                           raise ValueError('Cannot add "%r": instance is on database "%s", value is on database "%s"' %
-                                               (obj, self.instance._state.db, obj._state.db))
-                        new_ids.add(obj.pk)
-                    elif isinstance(obj, Model):
-                        raise TypeError("'%s' instance expected, got %r" % (self.model._meta.object_name, obj))
-                    else:
-                        new_ids.add(obj)
+                new_ids = self._check_new_ids(objs)
                 db = router.db_for_write(self.through, instance=self.instance)
-                vals = self.through._default_manager.using(db).values_list(target_field_name, flat=True)
-                vals = vals.filter(**{
-                    source_field_name: self._pk_val,
-                    '%s__in' % target_field_name: new_ids,
-                })
-                new_ids = new_ids - set(vals)
 
+                if check_duplicates:
+                    vals = self.through._default_manager.using(db).values_list(target_field_name, flat=True)
+                    vals = vals.filter(**{
+                        source_field_name: self._pk_val,
+                        '%s__in' % target_field_name: new_ids,
+                    })
+                    new_ids = new_ids - set(vals)
+
                 if self.reverse or source_field_name == self.source_field_name:
                     # Don't send the signal when we are inserting the
                     # duplicate data row for symmetrical reverse entries.
@@ -773 +784 @@
             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)
-        manager.clear()
-        manager.add(*value)
+        if value:
+            new_ids = manager._check_new_ids(value)
+            old_ids = set(manager.values_list('pk', flat=True))
 
+            manager.remove(*(old_ids - new_ids))
+            manager.add(*(new_ids - old_ids), check_duplicates=False)
+        else:
+            manager.clear()
 
+
 class ReverseManyRelatedObjectsDescriptor(object):
     # This class provides the functionality that makes the related-object
     # managers available as attributes on a model class, for fields that have
@@ -830 +847 @@
             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)
-        manager.clear()
-        manager.add(*value)
+        if value:
+            new_ids = manager._check_new_ids(value)
+            old_ids = set(manager.values_list('pk', flat=True))
 
+            manager.remove(*(old_ids - new_ids))
+            manager.add(*(new_ids - old_ids), check_duplicates=False)
+        else:
+            manager.clear()
+
 class ManyToOneRel(object):
     def __init__(self, to, field_name, related_name=None, limit_choices_to=None,
         parent_link=False, on_delete=None):