Commit e4b813c0 authored by Tim Graham's avatar Tim Graham
Browse files

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

This mostly reverts 5643a3b5 and
81e1a35c.

Thanks Carl Meyer for review.

Backport of 5980b05c from master
parent 90c7078f
Loading
Loading
Loading
Loading
+1 −5
Original line number Diff line number Diff line
@@ -32,6 +32,7 @@ class GenericForeignKey(object):
    one_to_one = False
    related_model = None

    # For backwards compatibility; ignored as of Django 1.8.4.
    allow_unsaved_instance_assignment = False

    def __init__(self, ct_field="content_type", fk_field="object_id", for_concrete_model=True):
@@ -243,11 +244,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)
+24 −0
Original line number Diff line number Diff line
@@ -661,6 +661,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.")
+2 −7
Original line number Diff line number Diff line
@@ -506,7 +506,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)
@@ -669,12 +669,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.rel.to._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
@@ -1491,6 +1485,7 @@ class ForeignObject(RelatedField):
    one_to_many = False
    one_to_one = False

    # For backwards compatibility; ignored as of Django 1.8.4.
    allow_unsaved_instance_assignment = False
    requires_unique_target = True
    related_accessor_class = ForeignRelatedObjectsDescriptor
+0 −7
Original line number Diff line number Diff line
@@ -299,13 +299,6 @@ model:
       is ``True``. This mirrors the ``for_concrete_model`` argument to
       :meth:`~django.contrib.contenttypes.models.ContentTypeManager.get_for_model`.

    .. attribute:: GenericForeignKey.allow_unsaved_instance_assignment

        .. versionadded:: 1.8

        Works analogously to :attr:`ForeignKey.allow_unsaved_instance_assignment
        <django.db.models.ForeignKey.allow_unsaved_instance_assignment>`.

    .. deprecated:: 1.7

        This class used to be defined in ``django.contrib.contenttypes.generic``.
+0 −30
Original line number Diff line number Diff line
@@ -1349,30 +1349,6 @@ The possible values for :attr:`~ForeignKey.on_delete` are found in

    If in doubt, leave it to its default of ``True``.

.. attribute:: ForeignKey.allow_unsaved_instance_assignment

    .. versionadded:: 1.8

        This flag was added for backwards compatibility as older versions of
        Django always allowed assigning unsaved model instances.

    Django prevents unsaved model instances from being assigned to a
    ``ForeignKey`` field to prevent accidental data loss (unsaved foreign keys
    are silently ignored when saving a model instance).

    If you require allowing the assignment of unsaved instances and aren't
    concerned about the data loss possibility (e.g. you never save the objects
    to the database), you can disable this check by creating a subclass of the
    field class and setting its ``allow_unsaved_instance_assignment`` attribute
    to ``True``. For example::

        class UnsavedForeignKey(models.ForeignKey):
            # A ForeignKey which can point to an unsaved object
            allow_unsaved_instance_assignment = True

        class Book(models.Model):
            author = UnsavedForeignKey(Author)

.. _ref-manytomany:

``ManyToManyField``
@@ -1575,12 +1551,6 @@ that control how the relationship functions.

    If in doubt, leave it to its default of ``True``.

.. attribute:: ManyToManyField.allow_unsaved_instance_assignment

    .. versionadded:: 1.8

    Works analogously to :attr:`ForeignKey.allow_unsaved_instance_assignment`.

:class:`ManyToManyField` does not support :attr:`~Field.validators`.

:attr:`~Field.null` has no effect since there is no way to require a
Loading