summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKarl Hobley <karlhobley10@gmail.com>2015-03-16 19:28:53 +0000
committerTim Graham <timograham@gmail.com>2015-03-18 19:12:46 -0400
commita8c53041f9645df9bfa744027efb2a36867422d8 (patch)
treee123dcd1f6b0e486144f2fae349f466087a403a5
parent247251c2e1702831b7ab80a69a3f7f5831be3b96 (diff)
[1.8.x] Fixed #24495 -- Allowed unsaved model instance assignment check to be bypassed.
Backport of 81e1a35c364e5353d2bf99368ad30a4184fbb653 from master
-rw-r--r--django/contrib/contenttypes/fields.py4
-rw-r--r--django/db/models/fields/related.py5
-rw-r--r--docs/ref/contrib/contenttypes.txt7
-rw-r--r--docs/ref/models/fields.txt32
-rw-r--r--docs/releases/1.8.txt6
-rw-r--r--docs/topics/db/examples/many_to_one.txt4
-rw-r--r--docs/topics/db/examples/one_to_one.txt4
-rw-r--r--tests/contenttypes_tests/tests.py27
-rw-r--r--tests/many_to_one/tests.py25
-rw-r--r--tests/one_to_one/tests.py27
10 files changed, 136 insertions, 5 deletions
diff --git a/django/contrib/contenttypes/fields.py b/django/contrib/contenttypes/fields.py
index 199abc165a..be19352fdb 100644
--- a/django/contrib/contenttypes/fields.py
+++ b/django/contrib/contenttypes/fields.py
@@ -32,6 +32,8 @@ class GenericForeignKey(object):
one_to_one = False
related_model = 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
@@ -241,7 +243,7 @@ class GenericForeignKey(object):
if value is not None:
ct = self.get_content_type(obj=value)
fk = value._get_pk_val()
- if fk is None:
+ 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)
diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py
index fd06223146..a499fef136 100644
--- a/django/db/models/fields/related.py
+++ b/django/db/models/fields/related.py
@@ -498,7 +498,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 None in related_pk:
+ if not self.related.field.allow_unsaved_instance_assignment and None in related_pk:
raise ValueError(
'Cannot assign "%r": "%s" instance isn\'t saved in the database.' %
(value, instance._meta.object_name)
@@ -662,7 +662,7 @@ class ReverseSingleRelatedObjectDescriptor(object):
else:
for lh_field, rh_field in self.field.related_fields:
pk = value._get_pk_val()
- if pk is None:
+ 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)
@@ -1475,6 +1475,7 @@ class ForeignObject(RelatedField):
one_to_many = False
one_to_one = False
+ allow_unsaved_instance_assignment = False
requires_unique_target = True
related_accessor_class = ForeignRelatedObjectsDescriptor
diff --git a/docs/ref/contrib/contenttypes.txt b/docs/ref/contrib/contenttypes.txt
index 474d0a6ff1..09ee14c0fe 100644
--- a/docs/ref/contrib/contenttypes.txt
+++ b/docs/ref/contrib/contenttypes.txt
@@ -299,6 +299,13 @@ 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``.
diff --git a/docs/ref/models/fields.txt b/docs/ref/models/fields.txt
index 372147c476..d764719705 100644
--- a/docs/ref/models/fields.txt
+++ b/docs/ref/models/fields.txt
@@ -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``.
+.. 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``
@@ -1433,7 +1457,7 @@ that control how the relationship functions.
* ``<other_model>_id``: the ``id`` of the model that the
``ManyToManyField`` points to.
- If the ``ManyToManyField`` points from and to the same model, the following
+ If the ``ManyToManyField`` points from and to the same model, the following
fields are generated:
* ``id``: the primary key of the relation.
@@ -1532,6 +1556,12 @@ 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
diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt
index b7593c85ce..006d37a1af 100644
--- a/docs/releases/1.8.txt
+++ b/docs/releases/1.8.txt
@@ -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.
+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
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/docs/topics/db/examples/many_to_one.txt b/docs/topics/db/examples/many_to_one.txt
index c5c3e10034..e43809fd1c 100644
--- a/docs/topics/db/examples/many_to_one.txt
+++ b/docs/topics/db/examples/many_to_one.txt
@@ -60,6 +60,10 @@ raises ``ValueError``::
...
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
Previously, assigning unsaved objects did not raise an error and could
diff --git a/docs/topics/db/examples/one_to_one.txt b/docs/topics/db/examples/one_to_one.txt
index f1bdf3de9c..f2a3f54508 100644
--- a/docs/topics/db/examples/one_to_one.txt
+++ b/docs/topics/db/examples/one_to_one.txt
@@ -101,6 +101,10 @@ raises ``ValueError``::
...
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
Previously, assigning unsaved objects did not raise an error and could
diff --git a/tests/contenttypes_tests/tests.py b/tests/contenttypes_tests/tests.py
index a791f8df3c..350dc83162 100644
--- a/tests/contenttypes_tests/tests.py
+++ b/tests/contenttypes_tests/tests.py
@@ -233,6 +233,33 @@ class GenericForeignKeyTests(IsolatedModelsTestCase):
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)
+ 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/many_to_one/tests.py b/tests/many_to_one/tests.py
index a5337b8702..9031d03e92 100644
--- a/tests/many_to_one/tests.py
+++ b/tests/many_to_one/tests.py
@@ -120,6 +120,31 @@ 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)
+ 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))
diff --git a/tests/one_to_one/tests.py b/tests/one_to_one/tests.py
index d7d5793b0a..2cda25b0aa 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, transaction
+from django.db import IntegrityError, connection, models, transaction
from django.test import TestCase
from .models import (
@@ -145,6 +145,31 @@ class OneToOneTests(TestCase):
% (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)
+ 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):
"""
Regression test for #9023: accessing the reverse relationship shouldn't