From a5a2ceeb453b86caaf01687342984f372428beb8 Mon Sep 17 00:00:00 2001 From: "Stefan R. Filipek" Date: Thu, 10 May 2018 20:42:44 -0400 Subject: [PATCH] Fixed #27629 -- Added router.allow_relation() calls for assignments between unsaved model instances. --- AUTHORS | 1 + .../db/models/fields/related_descriptors.py | 14 +++++------ docs/releases/2.1.txt | 3 +++ tests/gis_tests/layermap/tests.py | 5 +++- tests/multiple_database/tests.py | 25 +++++++++++++++++++ 5 files changed, 39 insertions(+), 9 deletions(-) diff --git a/AUTHORS b/AUTHORS index 59420932d5..3224be14a5 100644 --- a/AUTHORS +++ b/AUTHORS @@ -763,6 +763,7 @@ answer newbie questions, and generally made Django that much better: Stanislaus Madueke Stanislav Karpov starrynight + Stefan R. Filipek Stefane Fermgier Stefano Rivera Stéphane Raimbault diff --git a/django/db/models/fields/related_descriptors.py b/django/db/models/fields/related_descriptors.py index f9b770fade..850ea99fa1 100644 --- a/django/db/models/fields/related_descriptors.py +++ b/django/db/models/fields/related_descriptors.py @@ -205,11 +205,10 @@ class ForwardManyToOneDescriptor: elif value is not None: if instance._state.db is None: instance._state.db = router.db_for_write(instance.__class__, instance=value) - elif value._state.db is None: + if value._state.db is None: value._state.db = router.db_for_write(value.__class__, instance=instance) - elif value._state.db is not None and instance._state.db is not None: - if not router.allow_relation(value, instance): - raise ValueError('Cannot assign "%r": the current database router prevents this relation.' % value) + if not router.allow_relation(value, instance): + raise ValueError('Cannot assign "%r": the current database router prevents this relation.' % value) remote_field = self.field.remote_field # If we're setting the value of a OneToOneField to None, we need to clear @@ -451,11 +450,10 @@ class ReverseOneToOneDescriptor: else: if instance._state.db is None: instance._state.db = router.db_for_write(instance.__class__, instance=value) - elif value._state.db is None: + if value._state.db is None: value._state.db = router.db_for_write(value.__class__, instance=instance) - elif value._state.db is not None and instance._state.db is not None: - if not router.allow_relation(value, instance): - raise ValueError('Cannot assign "%r": the current database router prevents this relation.' % value) + if not router.allow_relation(value, instance): + raise ValueError('Cannot assign "%r": the current database router prevents this relation.' % value) related_pk = tuple(getattr(instance, field.attname) for field in self.related.field.foreign_related_fields) # Set the value of the related field to the value of the related object's related field diff --git a/docs/releases/2.1.txt b/docs/releases/2.1.txt index 101d42ee03..5f23d6d241 100644 --- a/docs/releases/2.1.txt +++ b/docs/releases/2.1.txt @@ -415,6 +415,9 @@ Miscellaneous * ``QuerySet.raw()`` now caches its results like regular querysets. Use ``iterator()`` if you don't want caching. +* The database router :meth:`allow_relation` method is called in more cases. + Improperly written routers may need to be updated accordingly. + .. _deprecated-features-2.1: Features deprecated in 2.1 diff --git a/tests/gis_tests/layermap/tests.py b/tests/gis_tests/layermap/tests.py index 1ebeeb31c6..2406533e66 100644 --- a/tests/gis_tests/layermap/tests.py +++ b/tests/gis_tests/layermap/tests.py @@ -319,7 +319,10 @@ class OtherRouter: return self.db_for_read(model, **hints) def allow_relation(self, obj1, obj2, **hints): - return None + # ContentType objects are created during a post-migrate signal while + # performing fixture teardown using the default database alias and + # don't abide by the database specified by this router. + return True def allow_migrate(self, db, app_label, **hints): return True diff --git a/tests/multiple_database/tests.py b/tests/multiple_database/tests.py index 6dd44f6475..e4cef8cdb9 100644 --- a/tests/multiple_database/tests.py +++ b/tests/multiple_database/tests.py @@ -2082,3 +2082,28 @@ class RouteForWriteTestCase(TestCase): self.assertEqual(e.mode, RouterUsed.WRITE) self.assertEqual(e.model, Book) self.assertEqual(e.hints, {'instance': auth}) + + +class NoRelationRouter: + """Disallow all relations.""" + def allow_relation(self, obj1, obj2, **hints): + return False + + +@override_settings(DATABASE_ROUTERS=[NoRelationRouter()]) +class RelationAssignmentTests(TestCase): + """allow_relation() is called with unsaved model instances.""" + multi_db = True + router_prevents_msg = 'the current database router prevents this relation' + + def test_foreign_key_relation(self): + person = Person(name='Someone') + pet = Pet() + with self.assertRaisesMessage(ValueError, self.router_prevents_msg): + pet.owner = person + + def test_reverse_one_to_one_relation(self): + user = User(username='Someone', password='fake_hash') + profile = UserProfile() + with self.assertRaisesMessage(ValueError, self.router_prevents_msg): + user.userprofile = profile