diff options
| -rw-r--r-- | django/db/models/fields/related_descriptors.py | 24 | ||||
| -rw-r--r-- | docs/releases/4.1.txt | 8 | ||||
| -rw-r--r-- | tests/many_to_one/tests.py | 12 | ||||
| -rw-r--r-- | tests/many_to_one_null/tests.py | 33 | ||||
| -rw-r--r-- | tests/null_queries/tests.py | 11 |
5 files changed, 80 insertions, 8 deletions
diff --git a/django/db/models/fields/related_descriptors.py b/django/db/models/fields/related_descriptors.py index 7e782630f0..f62a9170c0 100644 --- a/django/db/models/fields/related_descriptors.py +++ b/django/db/models/fields/related_descriptors.py @@ -627,6 +627,15 @@ def create_reverse_many_to_one_manager(superclass, rel): self.core_filters = {self.field.name: instance} + # Even if this relation is not to pk, we require still pk value. + # The wish is that the instance has been already saved to DB, + # although having a pk value isn't a guarantee of that. + if self.instance.pk is None: + raise ValueError( + f"{instance.__class__.__name__!r} instance needs to have a primary " + f"key value before this relationship can be used." + ) + def __call__(self, *, manager): manager = getattr(self.model, manager) manager_class = create_reverse_many_to_one_manager(manager.__class__, rel) @@ -634,6 +643,14 @@ def create_reverse_many_to_one_manager(superclass, rel): do_not_call_in_templates = True + def _check_fk_val(self): + for field in self.field.foreign_related_fields: + if getattr(self.instance, field.attname) is None: + raise ValueError( + f'"{self.instance!r}" needs to have a value for field ' + f'"{field.attname}" before this relationship can be used.' + ) + def _apply_rel_filters(self, queryset): """ Filter the queryset for the instance this manager is bound to. @@ -714,6 +731,7 @@ def create_reverse_many_to_one_manager(superclass, rel): return queryset, rel_obj_attr, instance_attr, False, cache_name, False def add(self, *objs, bulk=True): + self._check_fk_val() self._remove_prefetched_objects() db = router.db_for_write(self.model, instance=self.instance) @@ -752,6 +770,7 @@ def create_reverse_many_to_one_manager(superclass, rel): add.alters_data = True def create(self, **kwargs): + self._check_fk_val() kwargs[self.field.name] = self.instance db = router.db_for_write(self.model, instance=self.instance) return super(RelatedManager, self.db_manager(db)).create(**kwargs) @@ -759,6 +778,7 @@ def create_reverse_many_to_one_manager(superclass, rel): create.alters_data = True def get_or_create(self, **kwargs): + self._check_fk_val() kwargs[self.field.name] = self.instance db = router.db_for_write(self.model, instance=self.instance) return super(RelatedManager, self.db_manager(db)).get_or_create(**kwargs) @@ -766,6 +786,7 @@ def create_reverse_many_to_one_manager(superclass, rel): get_or_create.alters_data = True def update_or_create(self, **kwargs): + self._check_fk_val() kwargs[self.field.name] = self.instance db = router.db_for_write(self.model, instance=self.instance) return super(RelatedManager, self.db_manager(db)).update_or_create(**kwargs) @@ -779,6 +800,7 @@ def create_reverse_many_to_one_manager(superclass, rel): def remove(self, *objs, bulk=True): if not objs: return + self._check_fk_val() val = self.field.get_foreign_related_value(self.instance) old_ids = set() for obj in objs: @@ -802,6 +824,7 @@ def create_reverse_many_to_one_manager(superclass, rel): remove.alters_data = True def clear(self, *, bulk=True): + self._check_fk_val() self._clear(self, bulk) clear.alters_data = True @@ -822,6 +845,7 @@ def create_reverse_many_to_one_manager(superclass, rel): _clear.alters_data = True def set(self, objs, *, bulk=True, clear=False): + self._check_fk_val() # Force evaluation of `objs` in case it's a queryset whose value # could be affected by `manager.clear()`. Refs #19816. objs = tuple(objs) diff --git a/docs/releases/4.1.txt b/docs/releases/4.1.txt index 08e1103314..089cde29ec 100644 --- a/docs/releases/4.1.txt +++ b/docs/releases/4.1.txt @@ -375,6 +375,14 @@ this difference. In Django 4.0 and earlier, second example query, but this undocumented behavior led to queries with excessive joins. +Reverse foreign key changes for unsaved model instances +------------------------------------------------------- + +In order to unify the behavior with many-to-many relations for unsaved model +instances, a reverse foreign key now raises ``ValueError`` when calling +:class:`related managers <django.db.models.fields.related.RelatedManager>` for +unsaved objects. + Miscellaneous ------------- diff --git a/tests/many_to_one/tests.py b/tests/many_to_one/tests.py index b8b040eff6..2311834481 100644 --- a/tests/many_to_one/tests.py +++ b/tests/many_to_one/tests.py @@ -738,14 +738,16 @@ class ManyToOneTests(TestCase): self.assertEqual("id", cat.remote_field.get_related_field().name) def test_relation_unsaved(self): - # The <field>_set manager does not join on Null value fields (#17541) Third.objects.create(name="Third 1") Third.objects.create(name="Third 2") th = Third(name="testing") - # The object isn't saved and thus the relation field is null - we won't even - # execute a query in this case. - with self.assertNumQueries(0): - self.assertEqual(th.child_set.count(), 0) + # The object isn't saved and the relation cannot be used. + msg = ( + "'Third' instance needs to have a primary key value before this " + "relationship can be used." + ) + with self.assertRaisesMessage(ValueError, msg): + th.child_set.count() th.save() # Now the model is saved, so we will need to execute a query. with self.assertNumQueries(1): diff --git a/tests/many_to_one_null/tests.py b/tests/many_to_one_null/tests.py index 5bd06b1ac4..f92d49f0a9 100644 --- a/tests/many_to_one_null/tests.py +++ b/tests/many_to_one_null/tests.py @@ -146,3 +146,36 @@ class ManyToOneNullTests(TestCase): self.assertIs(d1.car, None) with self.assertNumQueries(0): self.assertEqual(list(c1.drivers.all()), []) + + def test_unsaved(self): + msg = ( + "'Car' instance needs to have a primary key value before this relationship " + "can be used." + ) + with self.assertRaisesMessage(ValueError, msg): + Car(make="Ford").drivers.all() + + def test_related_null_to_field_related_managers(self): + car = Car.objects.create(make=None) + driver = Driver.objects.create() + msg = ( + f'"{car!r}" needs to have a value for field "make" before this ' + f"relationship can be used." + ) + with self.assertRaisesMessage(ValueError, msg): + car.drivers.add(driver) + with self.assertRaisesMessage(ValueError, msg): + car.drivers.create() + with self.assertRaisesMessage(ValueError, msg): + car.drivers.get_or_create() + with self.assertRaisesMessage(ValueError, msg): + car.drivers.update_or_create() + with self.assertRaisesMessage(ValueError, msg): + car.drivers.remove(driver) + with self.assertRaisesMessage(ValueError, msg): + car.drivers.clear() + with self.assertRaisesMessage(ValueError, msg): + car.drivers.set([driver]) + + with self.assertNumQueries(0): + self.assertEqual(car.drivers.count(), 0) diff --git a/tests/null_queries/tests.py b/tests/null_queries/tests.py index 4c5c3bbe5c..828c68d921 100644 --- a/tests/null_queries/tests.py +++ b/tests/null_queries/tests.py @@ -44,9 +44,14 @@ class NullQueriesTests(TestCase): with self.assertRaisesMessage(ValueError, "Cannot use None as a query value"): Choice.objects.filter(id__gt=None) - # Related managers use __exact=None implicitly if the object hasn't been saved. - p2 = Poll(question="How?") - self.assertEqual(repr(p2.choice_set.all()), "<QuerySet []>") + def test_unsaved(self): + poll = Poll(question="How?") + msg = ( + "'Poll' instance needs to have a primary key value before this " + "relationship can be used." + ) + with self.assertRaisesMessage(ValueError, msg): + poll.choice_set.all() def test_reverse_relations(self): """ |