diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py
index e3677c2f55..4ea0c3fb06 100644
--- a/django/db/models/fields/related.py
+++ b/django/db/models/fields/related.py
@@ -182,14 +182,29 @@ class SingleRelatedObjectDescriptor(object):
def __set__(self, instance, value):
if instance is None:
raise AttributeError, "%s must be accessed via instance" % self.related.opts.object_name
+
+ # The similarity of the code below to the code in
+ # ReverseSingleRelatedObjectDescriptor is annoying, but there's a bunch
+ # of small differences that would make a common base class convoluted.
+
+ # If null=True, we can assign null here, but otherwise the value needs
+ # to be an instance of the related class.
+ if value is None and self.related.field.null == False:
+ raise ValueError('Cannot assign None: "%s.%s" does not allow null values.' %
+ (instance._meta.object_name, self.related.get_accessor_name()))
+ elif value is not None and not isinstance(value, self.related.model):
+ raise ValueError('Cannot assign "%r": "%s.%s" must be a "%s" instance.' %
+ (value, instance._meta.object_name,
+ self.related.get_accessor_name(), self.related.opts.object_name))
+
# Set the value of the related field
setattr(value, self.related.field.rel.get_related_field().attname, instance)
- # Clear the cache, if it exists
- try:
- delattr(value, self.related.field.get_cache_name())
- except AttributeError:
- pass
+ # Since we already know what the related object is, seed the related
+ # object caches now, too. This avoids another db hit if you get the
+ # object you just set.
+ setattr(instance, self.cache_name, value)
+ setattr(value, self.related.field.get_cache_name(), instance)
class ReverseSingleRelatedObjectDescriptor(object):
# This class provides the functionality that makes the related-object
@@ -225,6 +240,17 @@ class ReverseSingleRelatedObjectDescriptor(object):
def __set__(self, instance, value):
if instance is None:
raise AttributeError, "%s must be accessed via instance" % self._field.name
+
+ # If null=True, we can assign null here, but otherwise the value needs
+ # to be an instance of the related class.
+ if value is None and self.field.null == False:
+ raise ValueError('Cannot assign None: "%s.%s" does not allow null values.' %
+ (instance._meta.object_name, self.field.name))
+ elif value is not None and not isinstance(value, self.field.rel.to):
+ raise ValueError('Cannot assign "%r": "%s.%s" must be a "%s" instance.' %
+ (value, instance._meta.object_name,
+ self.field.name, self.field.rel.to._meta.object_name))
+
# Set the value of the related field
try:
val = getattr(value, self.field.rel.get_related_field().attname)
@@ -232,11 +258,10 @@ class ReverseSingleRelatedObjectDescriptor(object):
val = None
setattr(instance, self.field.attname, val)
- # Clear the cache, if it exists
- try:
- delattr(instance, self.field.get_cache_name())
- except AttributeError:
- pass
+ # Since we already know what the related object is, seed the related
+ # object cache now, too. This avoids another db hit if you get the
+ # object you just set.
+ setattr(instance, self.field.get_cache_name(), value)
class ForeignRelatedObjectsDescriptor(object):
# This class provides the functionality that makes the related-object
diff --git a/tests/modeltests/one_to_one/models.py b/tests/modeltests/one_to_one/models.py
index 800ccddac2..6fa4dd8c18 100644
--- a/tests/modeltests/one_to_one/models.py
+++ b/tests/modeltests/one_to_one/models.py
@@ -80,11 +80,8 @@ DoesNotExist: Restaurant matching query does not exist.
>>> r.place
<Place: Ace Hardware the place>
-# Set the place back again, using assignment in the reverse direction. Need to
-# reload restaurant object first, because the reverse set can't update the
-# existing restaurant instance
+# Set the place back again, using assignment in the reverse direction.
>>> p1.restaurant = r
->>> r.save()
>>> p1.restaurant
<Restaurant: Demon Dogs the restaurant>
diff --git a/tests/regressiontests/many_to_one_regress/models.py b/tests/regressiontests/many_to_one_regress/models.py
index 57bbcd8489..4e49df1555 100644
--- a/tests/regressiontests/many_to_one_regress/models.py
+++ b/tests/regressiontests/many_to_one_regress/models.py
@@ -1,3 +1,7 @@
+"""
+Regression tests for a few FK bugs: #1578, #6886
+"""
+
from django.db import models
# If ticket #1578 ever slips back in, these models will not be able to be
@@ -25,10 +29,48 @@ class Child(models.Model):
__test__ = {'API_TESTS':"""
->>> Third.AddManipulator().save(dict(id='3', name='An example', another=None))
+>>> Third.objects.create(id='3', name='An example')
<Third: Third object>
>>> parent = Parent(name = 'fred')
>>> parent.save()
->>> Child.AddManipulator().save(dict(name='bam-bam', parent=parent.id))
+>>> Child.objects.create(name='bam-bam', parent=parent)
<Child: Child object>
+
+#
+# Tests of ForeignKey assignment and the related-object cache (see #6886)
+#
+>>> p = Parent.objects.create(name="Parent")
+>>> c = Child.objects.create(name="Child", parent=p)
+
+# Look up the object again so that we get a "fresh" object
+>>> c = Child.objects.get(name="Child")
+>>> p = c.parent
+
+# Accessing the related object again returns the exactly same object
+>>> c.parent is p
+True
+
+# But if we kill the cache, we get a new object
+>>> del c._parent_cache
+>>> c.parent is p
+False
+
+# Assigning a new object results in that object getting cached immediately
+>>> p2 = Parent.objects.create(name="Parent 2")
+>>> c.parent = p2
+>>> c.parent is p2
+True
+
+# Assigning None fails: Child.parent is null=False
+>>> c.parent = None
+Traceback (most recent call last):
+ ...
+ValueError: Cannot assign None: "Child.parent" does not allow null values.
+
+# You also can't assign an object of the wrong type here
+>>> c.parent = First(id=1, second=1)
+Traceback (most recent call last):
+ ...
+ValueError: Cannot assign "<First: First object>": "Child.parent" must be a "Parent" instance.
+
"""}
diff --git a/tests/regressiontests/one_to_one_regress/models.py b/tests/regressiontests/one_to_one_regress/models.py
index c68fdfc780..99022882f2 100644
--- a/tests/regressiontests/one_to_one_regress/models.py
+++ b/tests/regressiontests/one_to_one_regress/models.py
@@ -50,4 +50,42 @@ __test__ = {'API_TESTS':"""
<Restaurant: Demon Dogs the restaurant>
>>> p1.bar
<Bar: Demon Dogs the bar>
+
+#
+# Regression test for #6886 (the related-object cache)
+#
+
+# Look up the objects again so that we get "fresh" objects
+>>> p = Place.objects.get(name="Demon Dogs")
+>>> r = p.restaurant
+
+# Accessing the related object again returns the exactly same object
+>>> p.restaurant is r
+True
+
+# But if we kill the cache, we get a new object
+>>> del p._restaurant_cache
+>>> p.restaurant is r
+False
+
+# Reassigning the Restaurant object results in an immediate cache update
+# We can't use a new Restaurant because that'll violate one-to-one, but
+# with a new *instance* the is test below will fail if #6886 regresses.
+>>> r2 = Restaurant.objects.get(pk=r.pk)
+>>> p.restaurant = r2
+>>> p.restaurant is r2
+True
+
+# Assigning None fails: Place.restaurant is null=False
+>>> p.restaurant = None
+Traceback (most recent call last):
+ ...
+ValueError: Cannot assign None: "Place.restaurant" does not allow null values.
+
+# You also can't assign an object of the wrong type here
+>>> p.restaurant = p
+Traceback (most recent call last):
+ ...
+ValueError: Cannot assign "<Place: Demon Dogs the place>": "Place.restaurant" must be a "Restaurant" instance.
+
"""}