mirror of https://github.com/django/django.git
[1.8.x] Fixed #24495 -- Allowed unsaved model instance assignment check to be bypassed.
Backport of 81e1a35c36
from master
This commit is contained in:
parent
247251c2e1
commit
a8c53041f9
|
@ -32,6 +32,8 @@ class GenericForeignKey(object):
|
||||||
one_to_one = False
|
one_to_one = False
|
||||||
related_model = None
|
related_model = None
|
||||||
|
|
||||||
|
allow_unsaved_instance_assignment = False
|
||||||
|
|
||||||
def __init__(self, ct_field="content_type", fk_field="object_id", for_concrete_model=True):
|
def __init__(self, ct_field="content_type", fk_field="object_id", for_concrete_model=True):
|
||||||
self.ct_field = ct_field
|
self.ct_field = ct_field
|
||||||
self.fk_field = fk_field
|
self.fk_field = fk_field
|
||||||
|
@ -241,7 +243,7 @@ class GenericForeignKey(object):
|
||||||
if value is not None:
|
if value is not None:
|
||||||
ct = self.get_content_type(obj=value)
|
ct = self.get_content_type(obj=value)
|
||||||
fk = value._get_pk_val()
|
fk = value._get_pk_val()
|
||||||
if fk is None:
|
if not self.allow_unsaved_instance_assignment and fk is None:
|
||||||
raise ValueError(
|
raise ValueError(
|
||||||
'Cannot assign "%r": "%s" instance isn\'t saved in the database.' %
|
'Cannot assign "%r": "%s" instance isn\'t saved in the database.' %
|
||||||
(value, value._meta.object_name)
|
(value, value._meta.object_name)
|
||||||
|
|
|
@ -498,7 +498,7 @@ class SingleRelatedObjectDescriptor(object):
|
||||||
raise ValueError('Cannot assign "%r": the current database router prevents this relation.' % value)
|
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)
|
related_pk = tuple(getattr(instance, field.attname) for field in self.related.field.foreign_related_fields)
|
||||||
if None in related_pk:
|
if not self.related.field.allow_unsaved_instance_assignment and None in related_pk:
|
||||||
raise ValueError(
|
raise ValueError(
|
||||||
'Cannot assign "%r": "%s" instance isn\'t saved in the database.' %
|
'Cannot assign "%r": "%s" instance isn\'t saved in the database.' %
|
||||||
(value, instance._meta.object_name)
|
(value, instance._meta.object_name)
|
||||||
|
@ -662,7 +662,7 @@ class ReverseSingleRelatedObjectDescriptor(object):
|
||||||
else:
|
else:
|
||||||
for lh_field, rh_field in self.field.related_fields:
|
for lh_field, rh_field in self.field.related_fields:
|
||||||
pk = value._get_pk_val()
|
pk = value._get_pk_val()
|
||||||
if pk is None:
|
if not self.field.allow_unsaved_instance_assignment and pk is None:
|
||||||
raise ValueError(
|
raise ValueError(
|
||||||
'Cannot assign "%r": "%s" instance isn\'t saved in the database.' %
|
'Cannot assign "%r": "%s" instance isn\'t saved in the database.' %
|
||||||
(value, self.field.rel.to._meta.object_name)
|
(value, self.field.rel.to._meta.object_name)
|
||||||
|
@ -1475,6 +1475,7 @@ class ForeignObject(RelatedField):
|
||||||
one_to_many = False
|
one_to_many = False
|
||||||
one_to_one = False
|
one_to_one = False
|
||||||
|
|
||||||
|
allow_unsaved_instance_assignment = False
|
||||||
requires_unique_target = True
|
requires_unique_target = True
|
||||||
related_accessor_class = ForeignRelatedObjectsDescriptor
|
related_accessor_class = ForeignRelatedObjectsDescriptor
|
||||||
|
|
||||||
|
|
|
@ -299,6 +299,13 @@ model:
|
||||||
is ``True``. This mirrors the ``for_concrete_model`` argument to
|
is ``True``. This mirrors the ``for_concrete_model`` argument to
|
||||||
:meth:`~django.contrib.contenttypes.models.ContentTypeManager.get_for_model`.
|
: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
|
.. deprecated:: 1.7
|
||||||
|
|
||||||
This class used to be defined in ``django.contrib.contenttypes.generic``.
|
This class used to be defined in ``django.contrib.contenttypes.generic``.
|
||||||
|
|
|
@ -1330,6 +1330,30 @@ The possible values for :attr:`~ForeignKey.on_delete` are found in
|
||||||
|
|
||||||
If in doubt, leave it to its default of ``True``.
|
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:
|
.. _ref-manytomany:
|
||||||
|
|
||||||
``ManyToManyField``
|
``ManyToManyField``
|
||||||
|
@ -1532,6 +1556,12 @@ that control how the relationship functions.
|
||||||
|
|
||||||
If in doubt, leave it to its default of ``True``.
|
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`.
|
:class:`ManyToManyField` does not support :attr:`~Field.validators`.
|
||||||
|
|
||||||
:attr:`~Field.null` has no effect since there is no way to require a
|
:attr:`~Field.null` has no effect since there is no way to require a
|
||||||
|
|
|
@ -712,6 +712,12 @@ Now, an error will be raised to prevent data loss::
|
||||||
...
|
...
|
||||||
ValueError: Cannot assign "<Author: John>": "Author" instance isn't saved in the database.
|
ValueError: Cannot assign "<Author: John>": "Author" instance isn't saved in the database.
|
||||||
|
|
||||||
|
If you require allowing the assignment of unsaved instances (the old behavior)
|
||||||
|
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 using the
|
||||||
|
:attr:`~django.db.models.ForeignKey.allow_unsaved_instance_assignment`
|
||||||
|
attribute.
|
||||||
|
|
||||||
Management commands that only accept positional arguments
|
Management commands that only accept positional arguments
|
||||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||||
|
|
||||||
|
|
|
@ -60,6 +60,10 @@ raises ``ValueError``::
|
||||||
...
|
...
|
||||||
ValueError: 'Cannot assign "<Reporter: John Smith>": "Reporter" instance isn't saved in the database.'
|
ValueError: 'Cannot assign "<Reporter: John Smith>": "Reporter" instance isn't saved in the database.'
|
||||||
|
|
||||||
|
If you want to disable the unsaved instance check, you can use the
|
||||||
|
:attr:`~django.db.models.ForeignKey.allow_unsaved_instance_assignment`
|
||||||
|
attribute.
|
||||||
|
|
||||||
.. versionchanged:: 1.8
|
.. versionchanged:: 1.8
|
||||||
|
|
||||||
Previously, assigning unsaved objects did not raise an error and could
|
Previously, assigning unsaved objects did not raise an error and could
|
||||||
|
|
|
@ -101,6 +101,10 @@ raises ``ValueError``::
|
||||||
...
|
...
|
||||||
ValueError: 'Cannot assign "<Restaurant: Demon Dogs the restaurant>": "Restaurant" instance isn't saved in the database.'
|
ValueError: 'Cannot assign "<Restaurant: Demon Dogs the restaurant>": "Restaurant" instance isn't saved in the database.'
|
||||||
|
|
||||||
|
If you want to disable the unsaved instance check, you can use the
|
||||||
|
:attr:`~django.db.models.ForeignKey.allow_unsaved_instance_assignment`
|
||||||
|
attribute.
|
||||||
|
|
||||||
.. versionchanged:: 1.8
|
.. versionchanged:: 1.8
|
||||||
|
|
||||||
Previously, assigning unsaved objects did not raise an error and could
|
Previously, assigning unsaved objects did not raise an error and could
|
||||||
|
|
|
@ -233,6 +233,33 @@ class GenericForeignKeyTests(IsolatedModelsTestCase):
|
||||||
author.save()
|
author.save()
|
||||||
model.content_object = author # no error because the instance is saved
|
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)
|
||||||
|
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):
|
class GenericRelationshipTests(IsolatedModelsTestCase):
|
||||||
|
|
||||||
|
|
|
@ -120,6 +120,31 @@ class ManyToOneTests(TestCase):
|
||||||
self.assertFalse(hasattr(self.r2.article_set, 'remove'))
|
self.assertFalse(hasattr(self.r2.article_set, 'remove'))
|
||||||
self.assertFalse(hasattr(self.r2.article_set, 'clear'))
|
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)
|
||||||
|
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):
|
def test_selects(self):
|
||||||
self.r.article_set.create(headline="John's second story",
|
self.r.article_set.create(headline="John's second story",
|
||||||
pub_date=datetime.date(2005, 7, 29))
|
pub_date=datetime.date(2005, 7, 29))
|
||||||
|
|
|
@ -1,6 +1,6 @@
|
||||||
from __future__ import unicode_literals
|
from __future__ import unicode_literals
|
||||||
|
|
||||||
from django.db import IntegrityError, connection, transaction
|
from django.db import IntegrityError, connection, models, transaction
|
||||||
from django.test import TestCase
|
from django.test import TestCase
|
||||||
|
|
||||||
from .models import (
|
from .models import (
|
||||||
|
@ -145,6 +145,31 @@ class OneToOneTests(TestCase):
|
||||||
% (bar, p._meta.object_name)):
|
% (bar, p._meta.object_name)):
|
||||||
p.undergroundbar = bar
|
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)
|
||||||
|
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):
|
def test_reverse_relationship_cache_cascade(self):
|
||||||
"""
|
"""
|
||||||
Regression test for #9023: accessing the reverse relationship shouldn't
|
Regression test for #9023: accessing the reverse relationship shouldn't
|
||||||
|
|
Loading…
Reference in New Issue