Skip to content

Commit 9459ec8

Browse files
Jibodeahtimgraham
authored andcommitted
Fixed #26170 -- Made ModelAdmin views run transactions on the correct database.
Thanks juntatalor for the initial patch.
1 parent 18c72d5 commit 9459ec8

File tree

4 files changed

+136
-4
lines changed

4 files changed

+136
-4
lines changed

django/contrib/admin/options.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1405,9 +1405,11 @@ def get_changeform_initial_data(self, request):
14051405
return initial
14061406

14071407
@csrf_protect_m
1408-
@transaction.atomic
14091408
def changeform_view(self, request, object_id=None, form_url='', extra_context=None):
1409+
with transaction.atomic(using=router.db_for_write(self.model)):
1410+
return self._changeform_view(request, object_id, form_url, extra_context)
14101411

1412+
def _changeform_view(self, request, object_id, form_url, extra_context):
14111413
to_field = request.POST.get(TO_FIELD_VAR, request.GET.get(TO_FIELD_VAR))
14121414
if to_field and not self.to_field_allowed(request, to_field):
14131415
raise DisallowedModelAdminToField("The field %s cannot be referenced." % to_field)
@@ -1681,8 +1683,11 @@ def changelist_view(self, request, extra_context=None):
16811683
], context)
16821684

16831685
@csrf_protect_m
1684-
@transaction.atomic
16851686
def delete_view(self, request, object_id, extra_context=None):
1687+
with transaction.atomic(using=router.db_for_write(self.model)):
1688+
return self._delete_view(request, object_id, extra_context)
1689+
1690+
def _delete_view(self, request, object_id, extra_context):
16861691
"The 'delete' admin view for this model."
16871692
opts = self.model._meta
16881693
app_label = opts.app_label

django/contrib/auth/admin.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
)
1010
from django.contrib.auth.models import Group, User
1111
from django.core.exceptions import PermissionDenied
12-
from django.db import transaction
12+
from django.db import router, transaction
1313
from django.http import Http404, HttpResponseRedirect
1414
from django.template.response import TemplateResponse
1515
from django.urls import reverse
@@ -98,8 +98,11 @@ def lookup_allowed(self, lookup, value):
9898

9999
@sensitive_post_parameters_m
100100
@csrf_protect_m
101-
@transaction.atomic
102101
def add_view(self, request, form_url='', extra_context=None):
102+
with transaction.atomic(using=router.db_for_write(self.model)):
103+
return self._add_view(request, form_url, extra_context)
104+
105+
def _add_view(self, request, form_url='', extra_context=None):
103106
# It's an error for a user to have add permission but NOT change
104107
# permission for users. If we allowed such users to add users, they
105108
# could create superusers, which would mean they would essentially have

tests/admin_views/test_multidb.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
from django.conf.urls import url
2+
from django.contrib import admin
3+
from django.contrib.auth.models import User
4+
from django.db import connections
5+
from django.test import TestCase, mock, override_settings
6+
from django.urls import reverse
7+
8+
from .models import Book
9+
10+
11+
class Router(object):
12+
target_db = None
13+
14+
def db_for_read(self, model, **hints):
15+
return self.target_db
16+
17+
db_for_write = db_for_read
18+
19+
site = admin.AdminSite(name='test_adminsite')
20+
site.register(Book)
21+
22+
urlpatterns = [
23+
url(r'^admin/', site.urls),
24+
]
25+
26+
27+
@override_settings(ROOT_URLCONF=__name__, DATABASE_ROUTERS=['%s.Router' % __name__])
28+
class MultiDatabaseTests(TestCase):
29+
multi_db = True
30+
31+
@classmethod
32+
def setUpTestData(cls):
33+
cls.superusers = {}
34+
cls.test_book_ids = {}
35+
for db in connections:
36+
Router.target_db = db
37+
cls.superusers[db] = User.objects.create_superuser(
38+
username='admin', password='something', email='[email protected]',
39+
)
40+
b = Book(name='Test Book')
41+
b.save(using=db)
42+
cls.test_book_ids[db] = b.id
43+
44+
@mock.patch('django.contrib.admin.options.transaction')
45+
def test_add_view(self, mock):
46+
for db in connections:
47+
Router.target_db = db
48+
self.client.force_login(self.superusers[db])
49+
self.client.post(
50+
reverse('test_adminsite:admin_views_book_add'),
51+
{'name': 'Foobar: 5th edition'},
52+
)
53+
mock.atomic.assert_called_with(using=db)
54+
55+
@mock.patch('django.contrib.admin.options.transaction')
56+
def test_change_view(self, mock):
57+
for db in connections:
58+
Router.target_db = db
59+
self.client.force_login(self.superusers[db])
60+
self.client.post(
61+
reverse('test_adminsite:admin_views_book_change', args=[self.test_book_ids[db]]),
62+
{'name': 'Test Book 2: Test more'},
63+
)
64+
mock.atomic.assert_called_with(using=db)
65+
66+
@mock.patch('django.contrib.admin.options.transaction')
67+
def test_delete_view(self, mock):
68+
for db in connections:
69+
Router.target_db = db
70+
self.client.force_login(self.superusers[db])
71+
self.client.post(
72+
reverse('test_adminsite:admin_views_book_delete', args=[self.test_book_ids[db]]),
73+
{'post': 'yes'},
74+
)
75+
mock.atomic.assert_called_with(using=db)
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
from django.conf.urls import url
2+
from django.contrib import admin
3+
from django.contrib.auth.admin import UserAdmin
4+
from django.contrib.auth.models import User
5+
from django.db import connections
6+
from django.test import TestCase, mock, override_settings
7+
from django.urls import reverse
8+
9+
10+
class Router(object):
11+
target_db = None
12+
13+
def db_for_read(self, model, **hints):
14+
return self.target_db
15+
16+
db_for_write = db_for_read
17+
18+
site = admin.AdminSite(name='test_adminsite')
19+
site.register(User, admin_class=UserAdmin)
20+
21+
urlpatterns = [
22+
url(r'^admin/', site.urls),
23+
]
24+
25+
26+
@override_settings(ROOT_URLCONF=__name__, DATABASE_ROUTERS=['%s.Router' % __name__])
27+
class MultiDatabaseTests(TestCase):
28+
multi_db = True
29+
30+
@classmethod
31+
def setUpTestData(cls):
32+
cls.superusers = {}
33+
for db in connections:
34+
Router.target_db = db
35+
cls.superusers[db] = User.objects.create_superuser(
36+
username='admin', password='something', email='[email protected]',
37+
)
38+
39+
@mock.patch('django.contrib.auth.admin.transaction')
40+
def test_add_view(self, mock):
41+
for db in connections:
42+
Router.target_db = db
43+
self.client.force_login(self.superusers[db])
44+
self.client.post(reverse('test_adminsite:auth_user_add'), {
45+
'username': 'some_user',
46+
'password1': 'helloworld',
47+
'password2': 'helloworld',
48+
})
49+
mock.atomic.assert_called_with(using=db)

0 commit comments

Comments
 (0)