Admin sends out potentially harmful m2m_changed signals

[Gabriel Hurley]

I'd like to call attention to ticket #6707 [1], which deals with the
implementation of ReverseManyRelatedObjectsDescriptor.__set__.
Specifically, the problem is that the current implentation takes a
very efficient "clear everything and add all the keys back in"
approach to managing these relationships.

However, since 1.2 gained signals for ManyToMany relationships, this
is now a real problem. As reported in #13962 [2] and #14482 [3],
rather than sending the correct pre_add/post_add/pre_remove/
post_remove signals based on changes to a ManyToManyField in the admin
you instead get a set of pre_clear, post_clear, pre_add, post_add
signals containing *all* the keys.

This strikes me as having the potential for significant data loss,
corruption or duplication if the wrong kind of signals are hooked up.
Particularly to the pre/post-clear signals. It would be very easy to
see a user accidentally clearing a related table of data with a signal
not realizing that the admin application would be sending that clear
signal only to add everything back a moment later.

So, I want to get some discussion going on the right way to fix the
problem. The patch on the ticket uses an extra query and sets to
compute the added and removed keys; Malcolm suggested that the admin
might independently track the old values to compute what's changed as
per #4102. [4] The existing patch has the advantage of being much
simpler to implement even if less efficient.

What are other people's thoughts?

Thanks!

- Gabriel

[1] http://code.djangoproject.com/ticket/6707
[2] http://code.djangoproject.com/ticket/13962
[3] http://code.djangoproject.com/ticket/14482
[4] http://code.djangoproject.com/ticket/4102

[Gabriel Hurley]

I see that this issue was also discussed in #13022 [5], wherein Russ
argued that the current signals are technically correct but that
solving #6707 would be worthwhile *if* it could be done efficiently.

[5] http://code.djangoproject.com/ticket/13022