From 2db4b134809e162a6aa142300a1df14638eeae94 Mon Sep 17 00:00:00 2001 From: Gary Wilson Jr Date: Fri, 1 Aug 2008 23:16:59 +0000 Subject: [PATCH] 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 | 16 +++---- tests/modeltests/many_to_one/models.py | 8 +++- .../many_to_one_regress/models.py | 42 +++++++++++++++---- .../one_to_one_regress/models.py | 35 +++++++++++++++- 4 files changed, 83 insertions(+), 18 deletions(-) diff --git a/django/db/models/base.py b/django/db/models/base.py index 53307dc14c..7d7def3bad 100644 --- a/django/db/models/base.py +++ b/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(): diff --git a/tests/modeltests/many_to_one/models.py b/tests/modeltests/many_to_one/models.py index 081cffb807..2dd1226e97 100644 --- a/tests/modeltests/many_to_one/models.py +++ b/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 [, , ] # 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() [, ] diff --git a/tests/regressiontests/many_to_one_regress/models.py b/tests/regressiontests/many_to_one_regress/models.py index 429bdd7558..57df8f3716 100644 --- a/tests/regressiontests/many_to_one_regress/models.py +++ b/tests/regressiontests/many_to_one_regress/models.py @@ -55,31 +55,36 @@ __test__ = {'API_TESTS':""" # -# 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 "": "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') diff --git a/tests/regressiontests/one_to_one_regress/models.py b/tests/regressiontests/one_to_one_regress/models.py index 99022882f2..66d00d7223 100644 --- a/tests/regressiontests/one_to_one_regress/models.py +++ b/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() [] -# 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.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 + """}