diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 9cc891d807..e8760c2931 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -1814,6 +1814,9 @@ class ModelAdmin(BaseModelAdmin): @csrf_protect_m def changeform_view(self, request, object_id=None, form_url="", extra_context=None): + if request.method in ("GET", "HEAD", "OPTIONS", "TRACE"): + return self._changeform_view(request, object_id, form_url, extra_context) + with transaction.atomic(using=router.db_for_write(self.model)): return self._changeform_view(request, object_id, form_url, extra_context) @@ -2175,6 +2178,9 @@ class ModelAdmin(BaseModelAdmin): @csrf_protect_m def delete_view(self, request, object_id, extra_context=None): + if request.method in ("GET", "HEAD", "OPTIONS", "TRACE"): + return self._delete_view(request, object_id, extra_context) + with transaction.atomic(using=router.db_for_write(self.model)): return self._delete_view(request, object_id, extra_context) diff --git a/django/contrib/auth/admin.py b/django/contrib/auth/admin.py index 90a53a142c..8e1d63ef07 100644 --- a/django/contrib/auth/admin.py +++ b/django/contrib/auth/admin.py @@ -117,6 +117,9 @@ class UserAdmin(admin.ModelAdmin): @sensitive_post_parameters_m @csrf_protect_m def add_view(self, request, form_url="", extra_context=None): + if request.method in ("GET", "HEAD", "OPTIONS", "TRACE"): + return self._add_view(request, form_url, extra_context) + with transaction.atomic(using=router.db_for_write(self.model)): return self._add_view(request, form_url, extra_context) diff --git a/tests/admin_views/test_multidb.py b/tests/admin_views/test_multidb.py index 654161e11d..0f18aeb315 100644 --- a/tests/admin_views/test_multidb.py +++ b/tests/admin_views/test_multidb.py @@ -40,6 +40,7 @@ urlpatterns = [ @override_settings(ROOT_URLCONF=__name__, DATABASE_ROUTERS=["%s.Router" % __name__]) class MultiDatabaseTests(TestCase): databases = {"default", "other"} + READ_ONLY_METHODS = {"get", "options", "head", "trace"} @classmethod def setUpTestData(cls): @@ -56,48 +57,116 @@ class MultiDatabaseTests(TestCase): b.save(using=db) cls.test_book_ids[db] = b.id + def tearDown(self): + # Reset the routers' state between each test. + Router.target_db = None + @mock.patch("django.contrib.admin.options.transaction") def test_add_view(self, mock): for db in self.databases: with self.subTest(db=db): + mock.mock_reset() Router.target_db = db self.client.force_login(self.superusers[db]) - self.client.post( + response = self.client.post( reverse("test_adminsite:admin_views_book_add"), {"name": "Foobar: 5th edition"}, ) + self.assertEqual(response.status_code, 302) + self.assertEqual( + response.url, reverse("test_adminsite:admin_views_book_changelist") + ) mock.atomic.assert_called_with(using=db) + @mock.patch("django.contrib.admin.options.transaction") + def test_read_only_methods_add_view(self, mock): + for db in self.databases: + for method in self.READ_ONLY_METHODS: + with self.subTest(db=db, method=method): + mock.mock_reset() + Router.target_db = db + self.client.force_login(self.superusers[db]) + response = getattr(self.client, method)( + reverse("test_adminsite:admin_views_book_add"), + ) + self.assertEqual(response.status_code, 200) + mock.atomic.assert_not_called() + @mock.patch("django.contrib.admin.options.transaction") def test_change_view(self, mock): for db in self.databases: with self.subTest(db=db): + mock.mock_reset() Router.target_db = db self.client.force_login(self.superusers[db]) - self.client.post( + response = self.client.post( reverse( "test_adminsite:admin_views_book_change", args=[self.test_book_ids[db]], ), {"name": "Test Book 2: Test more"}, ) + self.assertEqual(response.status_code, 302) + self.assertEqual( + response.url, reverse("test_adminsite:admin_views_book_changelist") + ) mock.atomic.assert_called_with(using=db) + @mock.patch("django.contrib.admin.options.transaction") + def test_read_only_methods_change_view(self, mock): + for db in self.databases: + for method in self.READ_ONLY_METHODS: + with self.subTest(db=db, method=method): + mock.mock_reset() + Router.target_db = db + self.client.force_login(self.superusers[db]) + response = getattr(self.client, method)( + reverse( + "test_adminsite:admin_views_book_change", + args=[self.test_book_ids[db]], + ), + data={"name": "Test Book 2: Test more"}, + ) + self.assertEqual(response.status_code, 200) + mock.atomic.assert_not_called() + @mock.patch("django.contrib.admin.options.transaction") def test_delete_view(self, mock): for db in self.databases: with self.subTest(db=db): + mock.mock_reset() Router.target_db = db self.client.force_login(self.superusers[db]) - self.client.post( + response = self.client.post( reverse( "test_adminsite:admin_views_book_delete", args=[self.test_book_ids[db]], ), {"post": "yes"}, ) + self.assertEqual(response.status_code, 302) + self.assertEqual( + response.url, reverse("test_adminsite:admin_views_book_changelist") + ) mock.atomic.assert_called_with(using=db) + @mock.patch("django.contrib.admin.options.transaction") + def test_read_only_methods_delete_view(self, mock): + for db in self.databases: + for method in self.READ_ONLY_METHODS: + with self.subTest(db=db, method=method): + mock.mock_reset() + Router.target_db = db + self.client.force_login(self.superusers[db]) + response = getattr(self.client, method)( + reverse( + "test_adminsite:admin_views_book_delete", + args=[self.test_book_ids[db]], + ) + ) + self.assertEqual(response.status_code, 200) + mock.atomic.assert_not_called() + class ViewOnSiteRouter: def db_for_read(self, model, instance=None, **hints): diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index 763fa44ce8..e0a4926b91 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -7385,7 +7385,7 @@ class UserAdminTest(TestCase): # Don't depend on a warm cache, see #17377. ContentType.objects.clear_cache() - expected_num_queries = 10 if connection.features.uses_savepoints else 8 + expected_num_queries = 8 if connection.features.uses_savepoints else 6 with self.assertNumQueries(expected_num_queries): response = self.client.get(reverse("admin:auth_user_change", args=(u.pk,))) self.assertEqual(response.status_code, 200) @@ -7433,7 +7433,7 @@ class GroupAdminTest(TestCase): # Ensure no queries are skipped due to cached content type for Group. ContentType.objects.clear_cache() - expected_num_queries = 8 if connection.features.uses_savepoints else 6 + expected_num_queries = 6 if connection.features.uses_savepoints else 4 with self.assertNumQueries(expected_num_queries): response = self.client.get(reverse("admin:auth_group_change", args=(g.pk,))) self.assertEqual(response.status_code, 200) diff --git a/tests/auth_tests/test_admin_multidb.py b/tests/auth_tests/test_admin_multidb.py index ce2ae6b103..17b04faa65 100644 --- a/tests/auth_tests/test_admin_multidb.py +++ b/tests/auth_tests/test_admin_multidb.py @@ -30,6 +30,7 @@ urlpatterns = [ @override_settings(ROOT_URLCONF=__name__, DATABASE_ROUTERS=["%s.Router" % __name__]) class MultiDatabaseTests(TestCase): databases = {"default", "other"} + READ_ONLY_METHODS = {"get", "options", "head", "trace"} @classmethod def setUpTestData(cls): @@ -42,13 +43,17 @@ class MultiDatabaseTests(TestCase): email="test@test.org", ) + def tearDown(self): + # Reset the routers' state between each test. + Router.target_db = None + @mock.patch("django.contrib.auth.admin.transaction") def test_add_view(self, mock): for db in self.databases: with self.subTest(db_connection=db): Router.target_db = db self.client.force_login(self.superusers[db]) - self.client.post( + response = self.client.post( reverse("test_adminsite:auth_user_add"), { "username": "some_user", @@ -56,4 +61,19 @@ class MultiDatabaseTests(TestCase): "password2": "helloworld", }, ) + self.assertEqual(response.status_code, 302) mock.atomic.assert_called_with(using=db) + + @mock.patch("django.contrib.auth.admin.transaction") + def test_read_only_methods_add_view(self, mock): + for db in self.databases: + for method in self.READ_ONLY_METHODS: + with self.subTest(db_connection=db, method=method): + mock.mock_reset() + Router.target_db = db + self.client.force_login(self.superusers[db]) + response = getattr(self.client, method)( + reverse("test_adminsite:auth_user_add") + ) + self.assertEqual(response.status_code, 200) + mock.atomic.assert_not_called()