Ticket #11811: 11811-patch-rev-5.diff

django/db/models/base.py

@@ -638 +638 @@
         return getattr(self, cachename)
 
     def prepare_database_save(self, unused):
+        if self.pk is None:
+            # This is reached if this instance is missing a primary key value
+            # but is passed as a parameter to a queryset's update() method,
+            # which needs some value to use as a foreign key.
+            raise ValueError('Cannot assign the instance "%r" to a foreign key'
+                ' field, because its primary key is not available.'
+                ' (For autonumbered keys, the instance would need to be saved'
+                ' first.)' % self)
         return self.pk
 
     def clean(self):

django/db/models/fields/related.py

@@ -807 +807 @@
         if isinstance(field_default, self.rel.to):
             return getattr(field_default, self.rel.get_related_field().attname)
         return field_default
+    
+    def pre_save(self, model_instance, add):
+        value = super(ForeignKey, self).pre_save(model_instance, add)
+        if (value is None and self.null and
+            getattr(model_instance, self.get_cache_name(), None) is not None):
+            # Error: we don't have a related key to save but do have a cached
+            # related instance. This indicates that a reference to the related
+            # instance was intended to make it into the database, but the lack
+            # of a related key value at the time of assignment makes this
+            # impossible.
+            # The situation is typically caused by assigning a new, unsaved
+            # instance to a foreign key field, leaving its autonumbered
+            # primary key empty.
+            # Non-null fields will raise an exception in this case anyway
+            # (an IntegrityError) so we only worry about nullable fields,
+            # to preserve previous behavior.
+            raise ValueError('Cannot save a foreign key referring to the'
+                ' instance "%r" into the field "%s.%s", because the key value'
+                ' for the instance was not available at the time of assignment'
+                ' to the field. (For autonumbered keys, the related instance'
+                ' may need to be saved prior to the assignment.)' %
+                (getattr(model_instance, self.get_cache_name()),
+                 model_instance._meta.object_name, self.name))
+        return value
 
     def get_db_prep_save(self, value, connection):
         if value == '' or value == None:

tests/regressiontests/model_regress/tests.py

@@ -1 @@
-from models import Worker
+from models import Department, Worker, Site, Flatpage
+from django.core.exceptions import ObjectDoesNotExist
 from django.test import TestCase
 
 class RelatedModelOrderedLookupTest(TestCase):
@@ -14 +15 @@
 
     def test_related_lte_lookup(self):
         Worker.objects.filter(department__lte=0)
+
+class AssignmentWithMissingForeignKeyTest(TestCase):
+    """
+    Regression test for #11811: setting a foreign key field to a model
+    instance with a null pk (i.e. an unsaved instance) should raise an
+    exception, which occurs when the model is saved to the database.
+    """
+    def test_attr_assignment_nullable(self):
+        s = Site(name='My Unsaved Site')
+        f = Flatpage(id=1, text='I will try to specify that site')
+        f.site = s
+        self.assertRaises(ValueError, f.save)
+        self.assertRaises(ObjectDoesNotExist, lambda: Flatpage.objects.get(
+            id=1))
+
+    def test_model_initializer_nullable(self):
+        s = Site(name='My Unsaved Site')
+        f = Flatpage(id=2, text='I will also try to specify that site', site=s)
+        self.assertRaises(ValueError, f.save)
+        self.assertRaises(ObjectDoesNotExist, lambda: Flatpage.objects.get(
+            id=2))
+    
+    def test_manager_create_nullable(self):
+        s = Site(name='My Unsaved Site')
+        self.assertRaises(ValueError, lambda: Flatpage.objects.create(id=3,
+            text='Never mind, I will try to specify that site', site=s))
+        self.assertRaises(ObjectDoesNotExist, lambda: Flatpage.objects.get(
+            id=3))
+    
+    def test_queryset_update_nullable(self):
+        s = Site(name='My Unsaved Site')
+        Flatpage.objects.create(id=4, text='Not specifying a site')
+        self.assertRaises(ValueError, lambda: Flatpage.objects.filter(
+            id=4).update(site=s))
+    
+    def test_attr_assignment_nonnull(self):
+        d = Department(name='An Unsaved Department')
+        w = Worker(id=1, name='Milton Waddams')
+        w.department = d
+        # the DB backend will define its own IntegrityError, so
+        # catch Exception to be backend-neutral
+        self.assertRaises(Exception, w.save)
+        self.assertRaises(ObjectDoesNotExist, lambda: Worker.objects.get(
+            id=1))
+
+    def test_model_initializer_nonnull(self):
+        d = Department(name='An Unsaved Department')
+        w = Worker(id=2, name='Milton Waddams', department=d)
+        # the DB backend will define its own IntegrityError, so
+        # catch Exception to be backend-neutral
+        self.assertRaises(Exception, w.save)
+        self.assertRaises(ObjectDoesNotExist, lambda: Worker.objects.get(
+            id=2))
+    
+    def test_manager_create_nonnull(self):
+        d = Department(name='An Unsaved Department')
+        # the DB backend will define its own IntegrityError, so
+        # catch Exception to be backend-neutral
+        self.assertRaises(Exception, lambda: Worker.objects.create(id=3,
+            name='Milton Waddams', department=d))
+        self.assertRaises(ObjectDoesNotExist, lambda: Worker.objects.get(
+            id=3))
+    
+    def test_queryset_update_nonnull(self):
+        d1 = Department(id=1, name='A Real Department')
+        d1.save()
+        d2 = Department(name='An Unsaved Department')
+        Worker.objects.create(id=4, name='Milton Waddams', department=d1)
+        self.assertRaises(ValueError, lambda: Worker.objects.filter(
+            id=4).update(department=d2))
+

tests/regressiontests/model_regress/models.py

@@ -50 +50 @@
     def __unicode__(self):
         return self.name
 
+# two models with a nullable foreign key relationship for testing #11811
+class Site(models.Model):
+    name = models.CharField(max_length=200)
+
+    def __unicode__(self):
+        return self.name
+
+class Flatpage(models.Model):
+    text = models.TextField()
+    site = models.ForeignKey(Site, null=True)
+
 class BrokenUnicodeMethod(models.Model):
     name = models.CharField(max_length=7)