From da74631d9f4a4ecf0267eb2f0cdaa979421d8466 Mon Sep 17 00:00:00 2001 From: Laurent Szyster Date: Sat, 7 Dec 2024 10:08:19 +0100 Subject: [PATCH] Fixed #35974 - test only _state.adding to assert that an object is unsaved in RelatedManager.add and GenericRelatedObjectManager.add --- django/contrib/contenttypes/fields.py | 2 +- .../db/models/fields/related_descriptors.py | 2 +- tests/multiple_database/fixtures/replica.json | 26 +++++ tests/multiple_database/tests.py | 107 ++++++++++++++++-- 4 files changed, 124 insertions(+), 13 deletions(-) create mode 100644 tests/multiple_database/fixtures/replica.json diff --git a/django/contrib/contenttypes/fields.py b/django/contrib/contenttypes/fields.py index a3e87f6ed4..9a9b11ea3c 100644 --- a/django/contrib/contenttypes/fields.py +++ b/django/contrib/contenttypes/fields.py @@ -692,7 +692,7 @@ def create_generic_related_manager(superclass, rel): if bulk: pks = [] for obj in objs: - if obj._state.adding or obj._state.db != db: + if obj._state.adding: raise ValueError( "%r instance isn't saved. Use bulk=False or save " "the object first." % obj diff --git a/django/db/models/fields/related_descriptors.py b/django/db/models/fields/related_descriptors.py index 2b23085fa8..daf8fff131 100644 --- a/django/db/models/fields/related_descriptors.py +++ b/django/db/models/fields/related_descriptors.py @@ -820,7 +820,7 @@ def create_reverse_many_to_one_manager(superclass, rel): pks = [] for obj in objs: check_and_update_obj(obj) - if obj._state.adding or obj._state.db != db: + if obj._state.adding: raise ValueError( "%r instance isn't saved. Use bulk=False or save " "the object first." % obj diff --git a/tests/multiple_database/fixtures/replica.json b/tests/multiple_database/fixtures/replica.json new file mode 100644 index 0000000000..c6beb0c111 --- /dev/null +++ b/tests/multiple_database/fixtures/replica.json @@ -0,0 +1,26 @@ +[ + { + "pk": 1, + "model": "multiple_database.book", + "fields": { + "title": "The Definitive Guide to Django", + "published": "2009-7-8" + } + }, + { + "pk": 1, + "model": "multiple_database.person", + "fields": { + "name": "Tim Graham" + } + }, + { + "pk": 1, + "model": "multiple_database.review", + "fields": { + "source": "Python Weekly", + "content_type": 1, + "object_id": 1 + } + } +] \ No newline at end of file diff --git a/tests/multiple_database/tests.py b/tests/multiple_database/tests.py index 9587030a46..2b9b043ce7 100644 --- a/tests/multiple_database/tests.py +++ b/tests/multiple_database/tests.py @@ -1125,10 +1125,6 @@ class QueryTestCase(TestCase): review1.content_object = dive # Add to a foreign key set with an object from a different database - msg = ( - " instance isn't saved. " - "Use bulk=False or save the object first." - ) with self.assertRaisesMessage(ValueError, msg): with transaction.atomic(using="other"): dive.reviews.add(review1) @@ -1502,18 +1498,15 @@ class RouterTestCase(TestCase): self.assertEqual(Book.objects.using("default").count(), 1) self.assertEqual(Book.objects.using("other").count(), 1) - def test_invalid_set_foreign_key_assignment(self): + def test_invalid_set_unsaved_assignment(self): marty = Person.objects.using("default").create(name="Marty Alchin") - dive = Book.objects.using("other").create( + dive = Book( title="Dive into Python", published=datetime.date(2009, 5, 4), ) # Set a foreign key set with an object from a different database - msg = ( - " instance isn't saved. Use bulk=False or save the " - "object first." - ) - with self.assertRaisesMessage(ValueError, msg): + msg = "Model instances without primary key value are unhashable" + with self.assertRaisesMessage(TypeError, msg): marty.edited.set([dive]) def test_foreign_key_cross_database_protection(self): @@ -2556,3 +2549,95 @@ class RelationAssignmentTests(SimpleTestCase): profile = UserProfile() with self.assertRaisesMessage(ValueError, self.router_prevents_msg): user.userprofile = profile + + +@override_settings(DATABASE_ROUTERS=[TestRouter()]) +class ReadWriteClusterTests(TestCase): + """see ticket #35974""" + + databases = {"default", "other"} + fixtures = ["replica"] + + def test_reverse_many_to_one_add(self): + person = Person.objects.get(name="Tim Graham") + book = Book.objects.get(title="The Definitive Guide to Django") + person.edited.add(book) + self.assertEqual( + list(Person.objects.using("default").get(name="Tim Graham").edited.all()), + [book], + ) + + def test_reverse_many_to_one_set(self): + person = Person.objects.get(name="Tim Graham") + book = Book.objects.get(title="The Definitive Guide to Django") + person.edited.set([book]) + self.assertEqual( + list(Person.objects.using("default").get(name="Tim Graham").edited.all()), + [book], + ) + + def test_reverse_many_to_one_add_unsaved(self): + person = Person.objects.get(name="Tim Graham") + book = Book( + title="The Definitive Guide to Django", + published=datetime.date(2009, 7, 8), + ) + msg = ( + " instance isn't saved." + " Use bulk=False or save the object first." + ) + with self.assertRaisesMessage(ValueError, msg): + person.edited.add(book) + + def test_reverse_many_to_one_set_unsaved(self): + person = Person.objects.get(name="Tim Graham") + book = Book( + title="The Definitive Guide to Django", + published=datetime.date(2009, 7, 8), + ) + msg = "Model instances without primary key value are unhashable" + with self.assertRaisesMessage(TypeError, msg): + person.edited.set([book]) + + def test_generic_related_add(self): + book = Book.objects.get(title="The Definitive Guide to Django") + review = Review.objects.get(source="Python Weekly") + book.reviews.add(review) + self.assertEqual( + list( + Book.objects.using("default") + .get(title="The Definitive Guide to Django") + .reviews.all() + ), + [review], + ) + + def test_generic_related_set(self): + book = Book.objects.get(title="The Definitive Guide to Django") + review = Review.objects.get(source="Python Weekly") + book.reviews.set([review]) + self.assertEqual( + list( + Book.objects.using("default") + .get(title="The Definitive Guide to Django") + .reviews.all() + ), + [review], + ) + + def test_generic_related_add_unsaved(self): + book = Book.objects.get(title="The Definitive Guide to Django") + review = Review(source="Python Weekly") + msg = ( + " instance isn't saved." + " Use bulk=False or save the object first." + ) + with self.assertRaisesMessage(ValueError, msg): + book.reviews.add(review) + + def test_generic_related_set_unsaved(self): + book = Book.objects.get(title="The Definitive Guide to Django") + review = Review(source="Python Weekly") + msg = "Model instances without primary key value are unhashable" + with self.assertRaisesMessage(TypeError, msg): + book.reviews.set([review])