Fixed #8070 -- Cache related objects passed to Model init as keyword arguments. Also:

* Model init no longer performs a database query to refetch the related objects it is passed.
 * Model init now caches unsaved related objects correctly, too. (Previously, accessing the field would raise `DoesNotExist` error for `null=False` fields.)
 * Added tests for assigning `None` to `null=True` `ForeignKey` fields (refs #6886).


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

django/db/models/base.py

@@ -200,6 +200,7 @@ class Model(object):
         # keywords, or default.
 
         for field in fields_iter:
+            rel_obj = None
             if kwargs:
                 if isinstance(field.rel, ManyToOneRel):
                     try:
@@ -216,17 +217,18 @@ class Model(object):
                         # pass in "None" for related objects if it's allowed.
                         if rel_obj is None and field.null:
                             val = None
-                        else:
-                            try:
-                                val = getattr(rel_obj, field.rel.get_related_field().attname)
-                            except AttributeError:
-                                raise TypeError("Invalid value: %r should be a %s instance, not a %s" %
-                                    (field.name, field.rel.to, type(rel_obj)))
                 else:
                     val = kwargs.pop(field.attname, field.get_default())
             else:
                 val = field.get_default()
-            setattr(self, field.attname, val)
+            # If we got passed a related instance, set it using the field.name
+            # instead of field.attname (e.g. "user" instead of "user_id") so
+            # that the object gets properly cached (and type checked) by the
+            # RelatedObjectDescriptor.
+            if rel_obj:
+                setattr(self, field.name, rel_obj)
+            else:
+                setattr(self, field.attname, val)
 
         if kwargs:
             for prop in kwargs.keys():

tests/modeltests/many_to_one/models.py

@@ -46,8 +46,12 @@ __test__ = {'API_TESTS':"""
 
 # Article objects have access to their related Reporter objects.
 >>> r = a.reporter
+
+# These are strings instead of unicode strings because that's what was used in
+# the creation of this reporter (and we haven't refreshed the data from the
+# database, which always returns unicode strings).
 >>> r.first_name, r.last_name
-(u'John', u'Smith')
+('John', 'Smith')
 
 # Create an Article via the Reporter object.
 >>> new_article = r.article_set.create(headline="John's second story", pub_date=datetime(2005, 7, 29))
@@ -176,7 +180,7 @@ False
 [<Article: John's second story>, <Article: Paul's story>, <Article: This is a test>]
 
 # You can also use a queryset instead of a literal list of instances.
-# The queryset must be reduced to a list of values using values(), 
+# The queryset must be reduced to a list of values using values(),
 # then converted into a query
 >>> Article.objects.filter(reporter__in=Reporter.objects.filter(first_name='John').values('pk').query).distinct()
 [<Article: John's second story>, <Article: This is a test>]

tests/regressiontests/many_to_one_regress/models.py

@@ -55,31 +55,36 @@ __test__ = {'API_TESTS':"""
 <Child: Child object>
 
 #
-# Tests of ForeignKey assignment and the related-object cache (see #6886)
+# 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
+# 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
+# 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
+# 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
+# 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
+# Assigning None succeeds if field is null=True.
+>>> p.bestchild = None
+>>> p.bestchild is None
+True
+
+# Assigning None fails: Child.parent is null=False.
 >>> c.parent = None
 Traceback (most recent call last):
     ...
@@ -91,8 +96,31 @@ Traceback (most recent call last):
     ...
 ValueError: Cannot assign "<First: First object>": "Child.parent" must be a "Parent" instance.
 
-# Test of multiple ForeignKeys to the same model (bug #7125)
+# Creation using keyword argument should cache the related object.
+>>> p = Parent.objects.get(name="Parent")
+>>> c = Child(parent=p)
+>>> c.parent is p
+True
 
+# Creation using keyword argument and unsaved related instance (#8070).
+>>> p = Parent()
+>>> c = Child(parent=p)
+>>> c.parent is p
+True
+
+# Creation using attname keyword argument and an id will cause the related
+# object to be fetched.
+>>> p = Parent.objects.get(name="Parent")
+>>> c = Child(parent_id=p.id)
+>>> c.parent is p
+False
+>>> c.parent == p
+True
+
+
+#
+# Test of multiple ForeignKeys to the same model (bug #7125).
+#
 >>> c1 = Category.objects.create(name='First')
 >>> c2 = Category.objects.create(name='Second')
 >>> c3 = Category.objects.create(name='Third')

tests/regressiontests/one_to_one_regress/models.py

@@ -22,6 +22,10 @@ class Bar(models.Model):
     def __unicode__(self):
         return u"%s the bar" % self.place.name
 
+class UndergroundBar(models.Model):
+    place = models.OneToOneField(Place, null=True)
+    serves_cocktails = models.BooleanField()
+
 class Favorites(models.Model):
     name = models.CharField(max_length = 50)
     restaurants = models.ManyToManyField(Restaurant)
@@ -42,7 +46,7 @@ __test__ = {'API_TESTS':"""
 >>> f.restaurants.all()
 [<Restaurant: Demon Dogs the restaurant>]
 
-# Regression test for #7173: Check that the name of the cache for the 
+# Regression test for #7173: Check that the name of the cache for the
 # reverse object is correct.
 >>> b = Bar(place=p1, serves_cocktails=False)
 >>> b.save()
@@ -53,7 +57,7 @@ __test__ = {'API_TESTS':"""
 
 #
 # 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")
@@ -76,6 +80,12 @@ False
 >>> p.restaurant is r2
 True
 
+# Assigning None succeeds if field is null=True.
+>>> ug_bar = UndergroundBar.objects.create(place=p, serves_cocktails=False)
+>>> ug_bar.place = None
+>>> ug_bar.place is None
+True
+
 # Assigning None fails: Place.restaurant is null=False
 >>> p.restaurant = None
 Traceback (most recent call last):
@@ -88,4 +98,25 @@ Traceback (most recent call last):
     ...
 ValueError: Cannot assign "<Place: Demon Dogs the place>": "Place.restaurant" must be a "Restaurant" instance.
 
+# Creation using keyword argument should cache the related object.
+>>> p = Place.objects.get(name="Demon Dogs")
+>>> r = Restaurant(place=p)
+>>> r.place is p
+True
+
+# Creation using keyword argument and unsaved related instance (#8070).
+>>> p = Place()
+>>> r = Restaurant(place=p)
+>>> r.place is p
+True
+
+# Creation using attname keyword argument and an id will cause the related
+# object to be fetched.
+>>> p = Place.objects.get(name="Demon Dogs")
+>>> r = Restaurant(place_id=p.id)
+>>> r.place is p
+False
+>>> r.place == p
+True
+
 """}