From 3190abcd75b1fcd660353da4001885ef82cbc596 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Tue, 9 Oct 2012 21:36:35 +0200 Subject: [PATCH] Fixed #18153 -- Reverse OneToOne lookups on unsaved instances. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thanks David Hatch and Anssi Kääriäinen for their inputs. --- django/db/models/fields/related.py | 21 +++++++--- .../one_to_one_regress/tests.py | 40 +++++++++++++++++++ 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index 157640c0e3..a7af237714 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -261,13 +261,17 @@ class SingleRelatedObjectDescriptor(object): try: rel_obj = getattr(instance, self.cache_name) except AttributeError: - params = {'%s__pk' % self.related.field.name: instance._get_pk_val()} - try: - rel_obj = self.get_query_set(instance=instance).get(**params) - except self.related.model.DoesNotExist: + related_pk = instance._get_pk_val() + if related_pk is None: rel_obj = None else: - setattr(rel_obj, self.related.field.get_cache_name(), instance) + params = {'%s__pk' % self.related.field.name: related_pk} + try: + rel_obj = self.get_query_set(instance=instance).get(**params) + except self.related.model.DoesNotExist: + rel_obj = None + else: + setattr(rel_obj, self.related.field.get_cache_name(), instance) setattr(instance, self.cache_name, rel_obj) if rel_obj is None: raise self.related.model.DoesNotExist @@ -301,8 +305,13 @@ class SingleRelatedObjectDescriptor(object): raise ValueError('Cannot assign "%r": instance is on database "%s", value is on database "%s"' % (value, instance._state.db, value._state.db)) + related_pk = getattr(instance, self.related.field.rel.get_related_field().attname) + if related_pk is None: + raise ValueError('Cannot assign "%r": "%s" instance isn\'t saved in the database.' % + (value, self.related.opts.object_name)) + # Set the value of the related field to the value of the related object's related field - setattr(value, self.related.field.attname, getattr(instance, self.related.field.rel.get_related_field().attname)) + setattr(value, self.related.field.attname, related_pk) # 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 diff --git a/tests/regressiontests/one_to_one_regress/tests.py b/tests/regressiontests/one_to_one_regress/tests.py index eced88598b..615536ba38 100644 --- a/tests/regressiontests/one_to_one_regress/tests.py +++ b/tests/regressiontests/one_to_one_regress/tests.py @@ -202,3 +202,43 @@ class OneToOneRegressionTests(TestCase): with self.assertNumQueries(0): with self.assertRaises(UndergroundBar.DoesNotExist): self.p1.undergroundbar + + def test_get_reverse_on_unsaved_object(self): + """ + Regression for #18153 and #19089. + + Accessing the reverse relation on an unsaved object + always raises an exception. + """ + p = Place() + + # When there's no instance of the origin of the one-to-one + with self.assertNumQueries(0): + with self.assertRaises(UndergroundBar.DoesNotExist): + p.undergroundbar + + UndergroundBar.objects.create() + + # When there's one instance of the origin + # (p.undergroundbar used to return that instance) + with self.assertNumQueries(0): + with self.assertRaises(UndergroundBar.DoesNotExist): + p.undergroundbar + + UndergroundBar.objects.create() + + # When there are several instances of the origin + with self.assertNumQueries(0): + with self.assertRaises(UndergroundBar.DoesNotExist): + p.undergroundbar + + def test_set_reverse_on_unsaved_object(self): + """ + Writing to the reverse relation on an unsaved object + is impossible too. + """ + p = Place() + b = UndergroundBar.objects.create() + with self.assertNumQueries(0): + with self.assertRaises(ValueError): + p.undergroundbar = b