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

This mostly reverts 5643a3b51be338196d0b292d5626ad43648448d3 and
81e1a35c364e5353d2bf99368ad30a4184fbb653.

Thanks Carl Meyer for review.

3 files changed, 25 insertions, 15 deletions

diff --git a/django/contrib/contenttypes/fields.py b/django/contrib/contenttypes/fields.py
index 0980ed21d1..837a9d5bca 100644
--- a/django/contrib/contenttypes/fields.py
+++ b/django/contrib/contenttypes/fields.py
@@ -41,8 +41,6 @@ class GenericForeignKey(object):
     related_model = None
     remote_field = None
 
-    allow_unsaved_instance_assignment = False
-
     def __init__(self, ct_field='content_type', fk_field='object_id', for_concrete_model=True):
         self.ct_field = ct_field
         self.fk_field = fk_field
@@ -253,11 +251,6 @@ class GenericForeignKey(object):
         if value is not None:
             ct = self.get_content_type(obj=value)
             fk = value._get_pk_val()
-            if not self.allow_unsaved_instance_assignment and fk is None:
-                raise ValueError(
-                    'Cannot assign "%r": "%s" instance isn\'t saved in the database.' %
-                    (value, value._meta.object_name)
-                )
 
         setattr(instance, self.ct_field, ct)
         setattr(instance, self.fk_field, fk)
diff --git a/django/db/models/base.py b/django/db/models/base.py
index 2a8e5fd2c8..e1d2165e3d 100644
--- a/django/db/models/base.py
+++ b/django/db/models/base.py
@@ -630,6 +630,30 @@ class Model(six.with_metaclass(ModelBase)):
         that the "save" must be an SQL insert or update (or equivalent for
         non-SQL backends), respectively. Normally, they should not be set.
         """
+        # Ensure that a model instance without a PK hasn't been assigned to
+        # a ForeignKey or OneToOneField on this model. If the field is
+        # nullable, allowing the save() would result in silent data loss.
+        for field in self._meta.concrete_fields:
+            if field.is_relation:
+                # If the related field isn't cached, then an instance hasn't
+                # been assigned and there's no need to worry about this check.
+                try:
+                    getattr(self, field.get_cache_name())
+                except AttributeError:
+                    continue
+                obj = getattr(self, field.name, None)
+                # A pk may have been assigned manually to a model instance not
+                # saved to the database (or auto-generated in a case like
+                # UUIDField), but we allow the save to proceed and rely on the
+                # database to raise an IntegrityError if applicable. If
+                # constraints aren't supported by the database, there's the
+                # unavoidable risk of data corruption.
+                if obj and obj.pk is None:
+                    raise ValueError(
+                        "save() prohibited to prevent data loss due to "
+                        "unsaved related object '%s'." % field.name
+                    )
+
         using = using or router.db_for_write(self.__class__, instance=self)
         if force_insert and (force_update or update_fields):
             raise ValueError("Cannot force both insert and updating in model saving.")
diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py
index 77b7bcb8d3..f61e557aac 100644
--- a/django/db/models/fields/related.py
+++ b/django/db/models/fields/related.py
@@ -515,7 +515,7 @@ class SingleRelatedObjectDescriptor(object):
                     raise ValueError('Cannot assign "%r": the current database router prevents this relation.' % value)
 
         related_pk = tuple(getattr(instance, field.attname) for field in self.related.field.foreign_related_fields)
-        if not self.related.field.allow_unsaved_instance_assignment and None in related_pk:
+        if None in related_pk:
             raise ValueError(
                 'Cannot assign "%r": "%s" instance isn\'t saved in the database.' %
                 (value, instance._meta.object_name)
@@ -685,12 +685,6 @@ class ReverseSingleRelatedObjectDescriptor(object):
         # Set the values of the related field.
         else:
             for lh_field, rh_field in self.field.related_fields:
-                pk = value._get_pk_val()
-                if not self.field.allow_unsaved_instance_assignment and pk is None:
-                    raise ValueError(
-                        'Cannot assign "%r": "%s" instance isn\'t saved in the database.' %
-                        (value, self.field.remote_field.model._meta.object_name)
-                    )
                 setattr(instance, lh_field.attname, getattr(value, rh_field.attname))
 
         # Since we already know what the related object is, seed the related
@@ -1601,7 +1595,6 @@ class ForeignObject(RelatedField):
     one_to_many = False
     one_to_one = False
 
-    allow_unsaved_instance_assignment = False
     requires_unique_target = True
     related_accessor_class = ForeignRelatedObjectsDescriptor
     rel_class = ForeignObjectRel