Fixed #18153 -- Reverse OneToOne lookups on unsaved instances.

Thanks David Hatch and Anssi Kääriäinen for their inputs.

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