Fixed #21174 -- transaction control in related manager methods

4 files changed, 25 insertions, 17 deletions

diff --git a/django/contrib/contenttypes/generic.py b/django/contrib/contenttypes/generic.py
index 1fdb1f1894..51eed3f28b 100644
--- a/django/contrib/contenttypes/generic.py
+++ b/django/contrib/contenttypes/generic.py
@@ -372,14 +372,12 @@ def create_generic_related_manager(superclass):
 
         def remove(self, *objs):
             db = router.db_for_write(self.model, instance=self.instance)
-            for obj in objs:
-                obj.delete(using=db)
+            self.using(db).filter(pk__in=[o.pk for o in objs]).delete()
         remove.alters_data = True
 
         def clear(self):
             db = router.db_for_write(self.model, instance=self.instance)
-            for obj in self.all():
-                obj.delete(using=db)
+            self.using(db).delete()
         clear.alters_data = True
 
         def create(self, **kwargs):
diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py
index aa2a83bd97..8252cce672 100644
--- a/django/db/models/fields/related.py
+++ b/django/db/models/fields/related.py
@@ -1,6 +1,6 @@
 from operator import attrgetter
 
-from django.db import connection, connections, router
+from django.db import connection, connections, router, transaction
 from django.db.backends import utils
 from django.db.models import signals
 from django.db.models.fields import (AutoField, Field, IntegerField,
@@ -18,7 +18,6 @@ from django import forms
 
 RECURSIVE_RELATIONSHIP_CONSTANT = 'self'
 
-
 def add_lazy_relation(cls, field, relation, operation):
     """
     Adds a lookup on ``cls`` when a related field is defined using a string,
@@ -416,11 +415,16 @@ def create_foreign_related_manager(superclass, rel_field, rel_model):
             return qs, rel_obj_attr, instance_attr, False, cache_name
 
         def add(self, *objs):
-            for obj in objs:
-                if not isinstance(obj, self.model):
-                    raise TypeError("'%s' instance expected, got %r" % (self.model._meta.object_name, obj))
-                setattr(obj, rel_field.name, self.instance)
-                obj.save()
+            objs = list(objs)
+            db = router.db_for_write(self.model, instance=self.instance)
+            with transaction.commit_on_success_unless_managed(
+                    using=db, savepoint=False):
+                for obj in objs:
+                    if not isinstance(obj, self.model):
+                        raise TypeError("'%s' instance expected, got %r" %
+                                        (self.model._meta.object_name, obj))
+                    setattr(obj, rel_field.name, self.instance)
+                    obj.save()
         add.alters_data = True
 
         def create(self, **kwargs):
diff --git a/tests/many_to_one/tests.py b/tests/many_to_one/tests.py
index ae629288b1..6b3a043d68 100644
--- a/tests/many_to_one/tests.py
+++ b/tests/many_to_one/tests.py
@@ -2,6 +2,7 @@ from copy import deepcopy
 import datetime
 
 from django.core.exceptions import MultipleObjectsReturned, FieldError
+from django.db import transaction
 from django.test import TestCase
 from django.utils import six
 from django.utils.translation import ugettext_lazy
@@ -68,8 +69,10 @@ class ManyToOneTests(TestCase):
         self.assertQuerysetEqual(self.r2.article_set.all(), ["<Article: Paul's story>"])
 
         # Adding an object of the wrong type raises TypeError.
-        with six.assertRaisesRegex(self, TypeError, "'Article' instance expected, got <Reporter.*"):
-            self.r.article_set.add(self.r2)
+        with transaction.atomic():
+            with six.assertRaisesRegex(self, TypeError,
+                                       "'Article' instance expected, got <Reporter.*"):
+                self.r.article_set.add(self.r2)
         self.assertQuerysetEqual(self.r.article_set.all(),
             [
                 "<Article: John's second story>",
diff --git a/tests/multiple_database/tests.py b/tests/multiple_database/tests.py
index 191cbaa154..665de2b604 100644
--- a/tests/multiple_database/tests.py
+++ b/tests/multiple_database/tests.py
@@ -7,7 +7,7 @@ from operator import attrgetter
 from django.contrib.auth.models import User
 from django.contrib.contenttypes.models import ContentType
 from django.core import management
-from django.db import connections, router, DEFAULT_DB_ALIAS
+from django.db import connections, router, DEFAULT_DB_ALIAS, transaction
 from django.db.models import signals
 from django.db.utils import ConnectionRouter
 from django.test import TestCase
@@ -490,21 +490,24 @@ class QueryTestCase(TestCase):
 
         # Set a foreign key with an object from a different database
         try:
-            dive.editor = marty
+            with transaction.atomic(using='default'):
+                dive.editor = marty
             self.fail("Shouldn't be able to assign across databases")
         except ValueError:
             pass
 
         # Set a foreign key set with an object from a different database
         try:
-            marty.edited = [pro, dive]
+            with transaction.atomic(using='default'):
+                marty.edited = [pro, dive]
             self.fail("Shouldn't be able to assign across databases")
         except ValueError:
             pass
 
         # Add to a foreign key set with an object from a different database
         try:
-            marty.edited.add(dive)
+            with transaction.atomic(using='default'):
+                marty.edited.add(dive)
             self.fail("Shouldn't be able to assign across databases")
         except ValueError:
             pass