Fixed #25160 -- Moved unsaved model instance data loss check to Model.save()

This mostly reverts 5643a3b51be338196d0b292d5626ad43648448d3 and
81e1a35c364e5353d2bf99368ad30a4184fbb653.

Thanks Carl Meyer for review.

6 files changed, 47 insertions, 122 deletions

diff --git a/tests/admin_utils/tests.py b/tests/admin_utils/tests.py
index 18c95bf768..12254a983f 100644
--- a/tests/admin_utils/tests.py
+++ b/tests/admin_utils/tests.py
@@ -12,7 +12,7 @@ from django.contrib.admin.utils import (
     label_for_field, lookup_field, quote,
 )
 from django.db import DEFAULT_DB_ALIAS, models
-from django.test import TestCase, override_settings
+from django.test import SimpleTestCase, TestCase, override_settings
 from django.utils import six
 from django.utils.formats import localize
 from django.utils.safestring import mark_safe
@@ -94,7 +94,7 @@ class NestedObjectsTests(TestCase):
         n.collect([Vehicle.objects.first()])
 
 
-class UtilsTests(TestCase):
+class UtilsTests(SimpleTestCase):
 
     empty_value = '-empty-'
 
@@ -115,7 +115,7 @@ class UtilsTests(TestCase):
 
         simple_function = lambda obj: SIMPLE_FUNCTION
 
-        site_obj = Site.objects.create(domain=SITE_NAME)
+        site_obj = Site(domain=SITE_NAME)
         article = Article(
             site=site_obj,
             title=TITLE_TEXT,
diff --git a/tests/contenttypes_tests/tests.py b/tests/contenttypes_tests/tests.py
index 3ff613d9c4..0d0f9d30b8 100644
--- a/tests/contenttypes_tests/tests.py
+++ b/tests/contenttypes_tests/tests.py
@@ -236,54 +236,6 @@ class GenericForeignKeyTests(IsolatedModelsTestCase):
         errors = checks.run_checks()
         self.assertEqual(errors, ['performed!'])
 
-    def test_unsaved_instance_on_generic_foreign_key(self):
-        """
-        #10811 -- Assigning an unsaved object to GenericForeignKey
-        should raise an exception.
-        """
-        class Model(models.Model):
-            content_type = models.ForeignKey(ContentType, models.SET_NULL, null=True)
-            object_id = models.PositiveIntegerField(null=True)
-            content_object = GenericForeignKey('content_type', 'object_id')
-
-        author = Author(name='Author')
-        model = Model()
-        model.content_object = None   # no error here as content_type allows None
-        with self.assertRaisesMessage(ValueError,
-                                    'Cannot assign "%r": "%s" instance isn\'t saved in the database.'
-                                    % (author, author._meta.object_name)):
-            model.content_object = author   # raised ValueError here as author is unsaved
-
-        author.save()
-        model.content_object = author   # no error because the instance is saved
-
-    def test_unsaved_instance_on_generic_foreign_key_allowed_when_wanted(self):
-        """
-        #24495 - Assigning an unsaved object to a GenericForeignKey
-        should be allowed when the allow_unsaved_instance_assignment
-        attribute has been set to True.
-        """
-        class UnsavedGenericForeignKey(GenericForeignKey):
-            # A GenericForeignKey which can point to an unsaved object
-            allow_unsaved_instance_assignment = True
-
-        class Band(models.Model):
-            name = models.CharField(max_length=50)
-
-        class BandMember(models.Model):
-            band_ct = models.ForeignKey(ContentType, models.CASCADE)
-            band_id = models.PositiveIntegerField()
-            band = UnsavedGenericForeignKey('band_ct', 'band_id')
-            first_name = models.CharField(max_length=50)
-            last_name = models.CharField(max_length=50)
-
-        beatles = Band(name='The Beatles')
-        john = BandMember(first_name='John', last_name='Lennon')
-        # This should not raise an exception as the GenericForeignKey between
-        # member and band has allow_unsaved_instance_assignment=True.
-        john.band = beatles
-        self.assertEqual(john.band, beatles)
-
 
 class GenericRelationshipTests(IsolatedModelsTestCase):
 
diff --git a/tests/generic_relations/tests.py b/tests/generic_relations/tests.py
index 0162565969..c800bd77b0 100644
--- a/tests/generic_relations/tests.py
+++ b/tests/generic_relations/tests.py
@@ -4,6 +4,7 @@ from django import forms
 from django.contrib.contenttypes.forms import generic_inlineformset_factory
 from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import FieldError
+from django.db import IntegrityError
 from django.db.models import Q
 from django.test import SimpleTestCase, TestCase
 from django.utils import six
@@ -486,6 +487,15 @@ class GenericRelationsTests(TestCase):
         with self.assertRaisesMessage(FieldError, msg):
             TaggedItem.objects.get(content_object='')
 
+    def test_unsaved_instance_on_generic_foreign_key(self):
+        """
+        Assigning an unsaved object to GenericForeignKey should raise an
+        exception on model.save().
+        """
+        quartz = Mineral(name="Quartz", hardness=7)
+        with self.assertRaises(IntegrityError):
+            TaggedItem.objects.create(tag="shiny", content_object=quartz)
+
 
 class CustomWidget(forms.TextInput):
     pass
diff --git a/tests/many_to_one/tests.py b/tests/many_to_one/tests.py
index ba176c938c..610655beee 100644
--- a/tests/many_to_one/tests.py
+++ b/tests/many_to_one/tests.py
@@ -163,31 +163,6 @@ class ManyToOneTests(TestCase):
         self.assertFalse(hasattr(self.r2.article_set, 'remove'))
         self.assertFalse(hasattr(self.r2.article_set, 'clear'))
 
-    def test_assign_unsaved_check_override(self):
-        """
-        #24495 - Assigning an unsaved object to a ForeignKey
-        should be allowed when the allow_unsaved_instance_assignment
-        attribute has been set to True.
-        """
-        class UnsavedForeignKey(models.ForeignKey):
-            # A ForeignKey which can point to an unsaved object
-            allow_unsaved_instance_assignment = True
-
-        class Band(models.Model):
-            name = models.CharField(max_length=50)
-
-        class BandMember(models.Model):
-            band = UnsavedForeignKey(Band, models.CASCADE)
-            first_name = models.CharField(max_length=50)
-            last_name = models.CharField(max_length=50)
-
-        beatles = Band(name='The Beatles')
-        john = BandMember(first_name='John', last_name='Lennon')
-        # This should not raise an exception as the ForeignKey between member
-        # and band has allow_unsaved_instance_assignment=True.
-        john.band = beatles
-        self.assertEqual(john.band, beatles)
-
     def test_selects(self):
         self.r.article_set.create(headline="John's second story",
                                   pub_date=datetime.date(2005, 7, 29))
@@ -567,15 +542,13 @@ class ManyToOneTests(TestCase):
 
         # Creation using keyword argument and unsaved related instance (#8070).
         p = Parent()
-        with self.assertRaisesMessage(ValueError,
-                'Cannot assign "%r": "%s" instance isn\'t saved in the database.'
-                % (p, Child.parent.field.remote_field.model._meta.object_name)):
-            Child(parent=p)
+        msg = "save() prohibited to prevent data loss due to unsaved related object 'parent'."
+        with self.assertRaisesMessage(ValueError, msg):
+            Child.objects.create(parent=p)
 
-        with self.assertRaisesMessage(ValueError,
-                'Cannot assign "%r": "%s" instance isn\'t saved in the database.'
-                % (p, Child.parent.field.remote_field.model._meta.object_name)):
-            ToFieldChild(parent=p)
+        msg = "save() prohibited to prevent data loss due to unsaved related object 'parent'."
+        with self.assertRaisesMessage(ValueError, msg):
+            ToFieldChild.objects.create(parent=p)
 
         # Creation using attname keyword argument and an id will cause the
         # related object to be fetched.
diff --git a/tests/model_fields/test_uuid.py b/tests/model_fields/test_uuid.py
index bef1d54a74..343140c248 100644
--- a/tests/model_fields/test_uuid.py
+++ b/tests/model_fields/test_uuid.py
@@ -2,8 +2,10 @@ import json
 import uuid
 
 from django.core import exceptions, serializers
-from django.db import models
-from django.test import SimpleTestCase, TestCase
+from django.db import IntegrityError, models
+from django.test import (
+    SimpleTestCase, TestCase, TransactionTestCase, skipUnlessDBFeature,
+)
 
 from .models import (
     NullableUUIDModel, PrimaryKeyUUIDModel, RelatedToUUIDModel, UUIDGrandchild,
@@ -158,3 +160,14 @@ class TestAsPrimaryKey(TestCase):
     def test_two_level_foreign_keys(self):
         # exercises ForeignKey.get_db_prep_value()
         UUIDGrandchild().save()
+
+
+class TestAsPrimaryKeyTransactionTests(TransactionTestCase):
+    # Need a TransactionTestCase to avoid deferring FK constraint checking.
+    available_apps = ['model_fields']
+
+    @skipUnlessDBFeature('supports_foreign_keys')
+    def test_unsaved_fk(self):
+        u1 = PrimaryKeyUUIDModel()
+        with self.assertRaises(IntegrityError):
+            RelatedToUUIDModel.objects.create(uuid_fk=u1)
diff --git a/tests/one_to_one/tests.py b/tests/one_to_one/tests.py
index 807d504998..ee0536906c 100644
--- a/tests/one_to_one/tests.py
+++ b/tests/one_to_one/tests.py
@@ -1,6 +1,6 @@
 from __future__ import unicode_literals
 
-from django.db import IntegrityError, connection, models, transaction
+from django.db import IntegrityError, connection, transaction
 from django.test import TestCase
 
 from .models import (
@@ -134,41 +134,9 @@ class OneToOneTests(TestCase):
         should raise an exception.
         """
         place = Place(name='User', address='London')
-        with self.assertRaisesMessage(ValueError,
-                            'Cannot assign "%r": "%s" instance isn\'t saved in the database.'
-                            % (place, Restaurant.place.field.remote_field.model._meta.object_name)):
+        msg = "save() prohibited to prevent data loss due to unsaved related object 'place'."
+        with self.assertRaisesMessage(ValueError, msg):
             Restaurant.objects.create(place=place, serves_hot_dogs=True, serves_pizza=False)
-        bar = UndergroundBar()
-        p = Place(name='User', address='London')
-        with self.assertRaisesMessage(ValueError,
-                            'Cannot assign "%r": "%s" instance isn\'t saved in the database.'
-                            % (bar, p._meta.object_name)):
-            p.undergroundbar = bar
-
-    def test_unsaved_object_check_override(self):
-        """
-        #24495 - Assigning an unsaved object to a OneToOneField
-        should be allowed when the allow_unsaved_instance_assignment
-        attribute has been set to True.
-        """
-        class UnsavedOneToOneField(models.OneToOneField):
-            # A OneToOneField which can point to an unsaved object
-            allow_unsaved_instance_assignment = True
-
-        class Band(models.Model):
-            name = models.CharField(max_length=50)
-
-        class BandManager(models.Model):
-            band = UnsavedOneToOneField(Band, models.CASCADE)
-            first_name = models.CharField(max_length=50)
-            last_name = models.CharField(max_length=50)
-
-        band = Band(name='The Beatles')
-        manager = BandManager(first_name='Brian', last_name='Epstein')
-        # This should not raise an exception as the OneToOneField between
-        # manager and band has allow_unsaved_instance_assignment=True.
-        manager.band = band
-        self.assertEqual(manager.band, band)
 
     def test_reverse_relationship_cache_cascade(self):
         """
@@ -249,6 +217,11 @@ class OneToOneTests(TestCase):
         r = Restaurant(place=p)
         self.assertIs(r.place, p)
 
+        # Creation using keyword argument and unsaved related instance (#8070).
+        p = Place()
+        r = Restaurant(place=p)
+        self.assertTrue(r.place is p)
+
         # Creation using attname keyword argument and an id will cause the related
         # object to be fetched.
         p = Place.objects.get(name="Demon Dogs")
@@ -392,8 +365,12 @@ class OneToOneTests(TestCase):
         """
         p = Place()
         b = UndergroundBar.objects.create()
+        msg = (
+            'Cannot assign "<UndergroundBar: UndergroundBar object>": "Place" '
+            'instance isn\'t saved in the database.'
+        )
         with self.assertNumQueries(0):
-            with self.assertRaises(ValueError):
+            with self.assertRaisesMessage(ValueError, msg):
                 p.undergroundbar = b
 
     def test_nullable_o2o_delete(self):