Fixed #6886: Tightened up ForeignKey and OneToOne field assignment. Specifically:

  * Raise a ValueError if you try to assign the wrong type of object.
  * Raise a ValueError if you try to assign None to a field not specified with null=True.
  * Cache the set value at set time instead of just at lookup time.

This is a slightly backwards-incompatible change; see BackwardsIncompatibleChanges for more details.


git-svn-id: http://code.djangoproject.com/svn/django/trunk@7574 bcc190cf-cafb-0310-a4f2-bffc1f526a37

1 files changed, 35 insertions, 10 deletions

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