Fixed #33004 -- Made saving objects with unsaved GenericForeignKey raise ValueError.

This aligns to the behaviour of OneToOneField and ForeignKey fields.

Thanks Jonny Park for the initial patch.
This commit is contained in:
sarahboyce 2022-04-19 13:21:23 +02:00 committed by Mariusz Felisiak
parent 1ed8ca43f6
commit cd4da34fc1
4 changed files with 38 additions and 26 deletions

View File

@ -1072,8 +1072,9 @@ class Model(metaclass=ModelBase):
def _prepare_related_fields_for_save(self, operation_name, fields=None): def _prepare_related_fields_for_save(self, operation_name, fields=None):
# Ensure that a model instance without a PK hasn't been assigned to # Ensure that a model instance without a PK hasn't been assigned to
# a ForeignKey or OneToOneField on this model. If the field is # a ForeignKey, GenericForeignKey or OneToOneField on this model. If
# nullable, allowing the save would result in silent data loss. # the field is nullable, allowing the save would result in silent data
# loss.
for field in self._meta.concrete_fields: for field in self._meta.concrete_fields:
if fields and field not in fields: if fields and field not in fields:
continue continue
@ -1107,6 +1108,21 @@ class Model(metaclass=ModelBase):
self, field.attname self, field.attname
): ):
field.delete_cached_value(self) field.delete_cached_value(self)
# GenericForeignKeys are private.
for field in self._meta.private_fields:
if fields and field not in fields:
continue
if (
field.is_relation
and field.is_cached(self)
and hasattr(field, "fk_field")
):
obj = field.get_cached_value(self, default=None)
if obj and obj.pk is None:
raise ValueError(
f"{operation_name}() prohibited to prevent data loss due to "
f"unsaved related object '{field.name}'."
)
def delete(self, using=None, keep_parents=False): def delete(self, using=None, keep_parents=False):
if self.pk is None: if self.pk is None:

View File

@ -1,8 +1,7 @@
from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.models import ContentType
from django.core.exceptions import FieldError from django.core.exceptions import FieldError
from django.db import IntegrityError
from django.db.models import Q from django.db.models import Q
from django.test import SimpleTestCase, TestCase from django.test import SimpleTestCase, TestCase, skipUnlessDBFeature
from .models import ( from .models import (
AllowsNullGFK, AllowsNullGFK,
@ -501,14 +500,26 @@ class GenericRelationsTests(TestCase):
with self.assertRaisesMessage(FieldError, msg): with self.assertRaisesMessage(FieldError, msg):
TaggedItem.objects.get(content_object="") TaggedItem.objects.get(content_object="")
def test_unsaved_instance_on_generic_foreign_key(self): def test_unsaved_generic_foreign_key_parent_save(self):
"""
Assigning an unsaved object to GenericForeignKey should raise an
exception on model.save().
"""
quartz = Mineral(name="Quartz", hardness=7) quartz = Mineral(name="Quartz", hardness=7)
with self.assertRaises(IntegrityError): tagged_item = TaggedItem(tag="shiny", content_object=quartz)
TaggedItem.objects.create(tag="shiny", content_object=quartz) msg = (
"save() prohibited to prevent data loss due to unsaved related object "
"'content_object'."
)
with self.assertRaisesMessage(ValueError, msg):
tagged_item.save()
@skipUnlessDBFeature("has_bulk_insert")
def test_unsaved_generic_foreign_key_parent_bulk_create(self):
quartz = Mineral(name="Quartz", hardness=7)
tagged_item = TaggedItem(tag="shiny", content_object=quartz)
msg = (
"bulk_create() prohibited to prevent data loss due to unsaved related "
"object 'content_object'."
)
with self.assertRaisesMessage(ValueError, msg):
TaggedItem.objects.bulk_create([tagged_item])
def test_cache_invalidation_for_content_type_id(self): def test_cache_invalidation_for_content_type_id(self):
# Create a Vegetable and Mineral with the same id. # Create a Vegetable and Mineral with the same id.

View File

@ -104,11 +104,6 @@ class Company(models.Model):
links = GenericRelation(Link) links = GenericRelation(Link)
# For testing #13085 fix, we also use Note model defined above
class Developer(models.Model):
name = models.CharField(max_length=15)
class Team(models.Model): class Team(models.Model):
name = models.CharField(max_length=15) name = models.CharField(max_length=15)

View File

@ -1,4 +1,3 @@
from django.db import IntegrityError
from django.db.models import ProtectedError, Q, Sum from django.db.models import ProtectedError, Q, Sum
from django.forms.models import modelform_factory from django.forms.models import modelform_factory
from django.test import TestCase, skipIfDBFeature from django.test import TestCase, skipIfDBFeature
@ -15,7 +14,6 @@ from .models import (
Contact, Contact,
Content, Content,
D, D,
Developer,
Guild, Guild,
HasLinkThing, HasLinkThing,
Link, Link,
@ -140,14 +138,6 @@ class GenericRelationTests(TestCase):
self.assertEqual(count_places(p1), 1) self.assertEqual(count_places(p1), 1)
self.assertEqual(count_places(p2), 1) self.assertEqual(count_places(p2), 1)
def test_target_model_is_unsaved(self):
"""Test related to #13085"""
# Fails with another, ORM-level error
dev1 = Developer(name="Joe")
note = Note(note="Deserves promotion", content_object=dev1)
with self.assertRaises(IntegrityError):
note.save()
def test_target_model_len_zero(self): def test_target_model_len_zero(self):
""" """
Saving a model with a GenericForeignKey to a model instance whose Saving a model with a GenericForeignKey to a model instance whose