Index: docs/ref/signals.txt =================================================================== --- docs/ref/signals.txt (revision 17827) +++ docs/ref/signals.txt (working copy) @@ -332,6 +332,22 @@ ``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 -------------- Index: tests/modeltests/m2m_signals/tests.py =================================================================== --- tests/modeltests/m2m_signals/tests.py (revision 17827) +++ tests/modeltests/m2m_signals/tests.py (working copy) @@ -252,19 +252,21 @@ }) 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,22 +284,26 @@ }) 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,18 +350,6 @@ 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,18 +367,6 @@ 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,18 +384,6 @@ 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, Index: django/db/models/fields/related.py =================================================================== --- django/db/models/fields/related.py (revision 17827) +++ django/db/models/fields/related.py (working copy) @@ -574,12 +574,15 @@ # 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,33 +624,41 @@ 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,10 +784,16 @@ 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,9 +847,15 @@ 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):