[1.5.x] Fixed #18823 -- Ensured m2m.clear() works when using through+to_field

There was a potential data-loss issue involved -- when clearing
instance's m2m assignments it was possible some other instance's
m2m data was deleted instead.

This commit also improved None handling for to_field cases.

Backpatch of 611c4d6f1c24763e5e6e331a5dcf9b610288aaa8

2 files changed, 92 insertions, 6 deletions

diff --git a/tests/regressiontests/m2m_through_regress/models.py b/tests/regressiontests/m2m_through_regress/models.py
index 47c24ed5b2..23d3366f22 100644
--- a/tests/regressiontests/m2m_through_regress/models.py
+++ b/tests/regressiontests/m2m_through_regress/models.py
@@ -62,18 +62,18 @@ class B(models.Model):
 # Using to_field on the through model
 @python_2_unicode_compatible
 class Car(models.Model):
-    make = models.CharField(max_length=20, unique=True)
+    make = models.CharField(max_length=20, unique=True, null=True)
     drivers = models.ManyToManyField('Driver', through='CarDriver')
 
     def __str__(self):
-        return self.make
+        return "%s" % self.make
 
 @python_2_unicode_compatible
 class Driver(models.Model):
-    name = models.CharField(max_length=20, unique=True)
+    name = models.CharField(max_length=20, unique=True, null=True)
 
     def __str__(self):
-        return self.name
+        return "%s" % self.name
 
 @python_2_unicode_compatible
 class CarDriver(models.Model):
diff --git a/tests/regressiontests/m2m_through_regress/tests.py b/tests/regressiontests/m2m_through_regress/tests.py
index 458c194f89..828ec3618c 100644
--- a/tests/regressiontests/m2m_through_regress/tests.py
+++ b/tests/regressiontests/m2m_through_regress/tests.py
@@ -123,18 +123,104 @@ class ToFieldThroughTests(TestCase):
         self.car = Car.objects.create(make="Toyota")
         self.driver = Driver.objects.create(name="Ryan Briscoe")
         CarDriver.objects.create(car=self.car, driver=self.driver)
+        # We are testing if wrong objects get deleted due to using wrong
+        # field value in m2m queries. So, it is essential that the pk
+        # numberings do not match.
+        # Create one intentionally unused driver to mix up the autonumbering
+        self.unused_driver = Driver.objects.create(name="Barney Gumble")
+        # And two intentionally unused cars.
+        self.unused_car1 = Car.objects.create(make="Trabant")
+        self.unused_car2 = Car.objects.create(make="Wartburg")
 
     def test_to_field(self):
         self.assertQuerysetEqual(
             self.car.drivers.all(),
             ["<Driver: Ryan Briscoe>"]
-            )
+        )
 
     def test_to_field_reverse(self):
         self.assertQuerysetEqual(
             self.driver.car_set.all(),
             ["<Car: Toyota>"]
-            )
+        )
+
+    def test_to_field_clear_reverse(self):
+        self.driver.car_set.clear()
+        self.assertQuerysetEqual(
+            self.driver.car_set.all(),[])
+
+    def test_to_field_clear(self):
+        self.car.drivers.clear()
+        self.assertQuerysetEqual(
+            self.car.drivers.all(),[])
+
+    # Low level tests for _add_items and _remove_items. We test these methods
+    # because .add/.remove aren't available for m2m fields with through, but
+    # through is the only way to set to_field currently. We do want to make
+    # sure these methods are ready if the ability to use .add or .remove with
+    # to_field relations is added some day.
+    def test_add(self):
+        self.assertQuerysetEqual(
+            self.car.drivers.all(),
+            ["<Driver: Ryan Briscoe>"]
+        )
+        # Yikes - barney is going to drive...
+        self.car.drivers._add_items('car', 'driver', self.unused_driver)
+        self.assertQuerysetEqual(
+            self.car.drivers.all(),
+            ["<Driver: Ryan Briscoe>", "<Driver: Barney Gumble>"]
+        )
+
+    def test_add_null(self):
+        nullcar = Car.objects.create(make=None)
+        with self.assertRaises(ValueError):
+            nullcar.drivers._add_items('car', 'driver', self.unused_driver)
+
+    def test_add_related_null(self):
+        nulldriver = Driver.objects.create(name=None)
+        with self.assertRaises(ValueError):
+            self.car.drivers._add_items('car', 'driver', nulldriver)
+
+    def test_add_reverse(self):
+        car2 = Car.objects.create(make="Honda")
+        self.assertQuerysetEqual(
+            self.driver.car_set.all(),
+            ["<Car: Toyota>"]
+        )
+        self.driver.car_set._add_items('driver', 'car', car2)
+        self.assertQuerysetEqual(
+            self.driver.car_set.all(),
+            ["<Car: Toyota>", "<Car: Honda>"]
+        )
+
+    def test_add_null_reverse(self):
+        nullcar = Car.objects.create(make=None)
+        with self.assertRaises(ValueError):
+            self.driver.car_set._add_items('driver', 'car', nullcar)
+
+    def test_add_null_reverse_related(self):
+        nulldriver = Driver.objects.create(name=None)
+        with self.assertRaises(ValueError):
+            nulldriver.car_set._add_items('driver', 'car', self.car)
+
+    def test_remove(self):
+        self.assertQuerysetEqual(
+            self.car.drivers.all(),
+            ["<Driver: Ryan Briscoe>"]
+        )
+        self.car.drivers._remove_items('car', 'driver', self.driver)
+        self.assertQuerysetEqual(
+            self.car.drivers.all(),[])
+
+    def test_remove_reverse(self):
+        self.assertQuerysetEqual(
+            self.driver.car_set.all(),
+            ["<Car: Toyota>"]
+        )
+        self.driver.car_set._remove_items('driver', 'car', self.car)
+        self.assertQuerysetEqual(
+            self.driver.car_set.all(),[])
+
 
 class ThroughLoadDataTestCase(TestCase):
     fixtures = ["m2m_through"]