Fixed #18556 -- Allowed RelatedManager.add() to execute 1 query where possible.

Thanks Loic Bistuer for review.

4 files changed, 55 insertions, 5 deletions

diff --git a/tests/generic_relations/tests.py b/tests/generic_relations/tests.py
index 47545bdef9..0162565969 100644
--- a/tests/generic_relations/tests.py
+++ b/tests/generic_relations/tests.py
@@ -247,6 +247,32 @@ class GenericRelationsTests(TestCase):
             self.comp_func
         )
 
+    def test_add_bulk(self):
+        bacon = Vegetable.objects.create(name="Bacon", is_yucky=False)
+        t1 = TaggedItem.objects.create(content_object=self.quartz, tag="shiny")
+        t2 = TaggedItem.objects.create(content_object=self.quartz, tag="clearish")
+        # One update() query.
+        with self.assertNumQueries(1):
+            bacon.tags.add(t1, t2)
+        self.assertEqual(t1.content_object, bacon)
+        self.assertEqual(t2.content_object, bacon)
+
+    def test_add_bulk_false(self):
+        bacon = Vegetable.objects.create(name="Bacon", is_yucky=False)
+        t1 = TaggedItem.objects.create(content_object=self.quartz, tag="shiny")
+        t2 = TaggedItem.objects.create(content_object=self.quartz, tag="clearish")
+        # One save() for each object.
+        with self.assertNumQueries(2):
+            bacon.tags.add(t1, t2, bulk=False)
+        self.assertEqual(t1.content_object, bacon)
+        self.assertEqual(t2.content_object, bacon)
+
+    def test_add_rejects_unsaved_objects(self):
+        t1 = TaggedItem(content_object=self.quartz, tag="shiny")
+        msg = "<TaggedItem: shiny> instance isn't saved. Use bulk=False or save the object first."
+        with self.assertRaisesMessage(ValueError, msg):
+            self.bacon.tags.add(t1)
+
     def test_set(self):
         bacon = Vegetable.objects.create(name="Bacon", is_yucky=False)
         fatty = bacon.tags.create(tag="fatty")
@@ -266,13 +292,13 @@ class GenericRelationsTests(TestCase):
         bacon.tags.set([])
         self.assertQuerysetEqual(bacon.tags.all(), [])
 
-        bacon.tags.set([fatty, salty], clear=True)
+        bacon.tags.set([fatty, salty], bulk=False, clear=True)
         self.assertQuerysetEqual(bacon.tags.all(), [
             "<TaggedItem: fatty>",
             "<TaggedItem: salty>",
         ])
 
-        bacon.tags.set([fatty], clear=True)
+        bacon.tags.set([fatty], bulk=False, clear=True)
         self.assertQuerysetEqual(bacon.tags.all(), [
             "<TaggedItem: fatty>",
         ])
diff --git a/tests/many_to_one/tests.py b/tests/many_to_one/tests.py
index 7b2a640b2b..ba176c938c 100644
--- a/tests/many_to_one/tests.py
+++ b/tests/many_to_one/tests.py
@@ -57,7 +57,11 @@ class ManyToOneTests(TestCase):
 
         # Create a new article, and add it to the article set.
         new_article2 = Article(headline="Paul's story", pub_date=datetime.date(2006, 1, 17))
-        self.r.article_set.add(new_article2)
+        msg = "<Article: Paul's story> instance isn't saved. Use bulk=False or save the object first."
+        with self.assertRaisesMessage(ValueError, msg):
+            self.r.article_set.add(new_article2)
+
+        self.r.article_set.add(new_article2, bulk=False)
         self.assertEqual(new_article2.reporter.id, self.r.id)
         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 e39f527abe..06f756e35b 100644
--- a/tests/many_to_one_null/tests.py
+++ b/tests/many_to_one_null/tests.py
@@ -115,6 +115,15 @@ class ManyToOneNullTests(TestCase):
         self.assertEqual(1, self.r2.article_set.count())
         self.assertEqual(1, qs.count())
 
+    def test_add_efficiency(self):
+        r = Reporter.objects.create()
+        articles = []
+        for _ in range(3):
+            articles.append(Article.objects.create())
+        with self.assertNumQueries(1):
+            r.article_set.add(*articles)
+        self.assertEqual(r.article_set.count(), 3)
+
     def test_clear_efficiency(self):
         r = Reporter.objects.create()
         for _ in range(3):
diff --git a/tests/multiple_database/tests.py b/tests/multiple_database/tests.py
index 08493ffaf2..4354217d57 100644
--- a/tests/multiple_database/tests.py
+++ b/tests/multiple_database/tests.py
@@ -1043,6 +1043,17 @@ class RouterTestCase(TestCase):
         self.assertEqual(Book.objects.using('default').count(), 1)
         self.assertEqual(Book.objects.using('other').count(), 1)
 
+    def test_invalid_set_foreign_key_assignment(self):
+        marty = Person.objects.using('default').create(name="Marty Alchin")
+        dive = Book.objects.using('other').create(
+            title="Dive into Python",
+            published=datetime.date(2009, 5, 4),
+        )
+        # Set a foreign key set with an object from a different database
+        msg = "<Book: Dive into Python> instance isn't saved. Use bulk=False or save the object first."
+        with self.assertRaisesMessage(ValueError, msg):
+            marty.edited.set([dive])
+
     def test_foreign_key_cross_database_protection(self):
         "Foreign keys can cross databases if they two databases have a common source"
         # Create a book and author on the default database
@@ -1085,7 +1096,7 @@ class RouterTestCase(TestCase):
 
         # Set a foreign key set with an object from a different database
         try:
-            marty.edited = [pro, dive]
+            marty.edited.set([pro, dive], bulk=False)
         except ValueError:
             self.fail("Assignment across primary/replica databases with a common source should be ok")
 
@@ -1107,7 +1118,7 @@ class RouterTestCase(TestCase):
 
         # Add to a foreign key set with an object from a different database
         try:
-            marty.edited.add(dive)
+            marty.edited.add(dive, bulk=False)
         except ValueError:
             self.fail("Assignment across primary/replica databases with a common source should be ok")