summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAdam Donaghy <adamdonaghy1994@gmail.com>2018-05-03 23:41:04 +1000
committerTim Graham <timograham@gmail.com>2018-06-01 11:00:34 -0400
commit22c7c2db1dcc3d0ba2799441e7de025d502b3a4c (patch)
treee03e5e89e383d695ea0bef8e1db5d967ae76a324
parentb935f112e1a3db6919c9c7ad873e1451c79e31fe (diff)
[2.0.x] Fixed #28462 -- Decreased memory usage with ModelAdmin.list_editable.
Regression in 917cc288a38f3c114a5440f0749b7e5e1086eb36. Backport of b18650a2634890aa758abae2f33875daa13a9ba3 from master
-rw-r--r--AUTHORS1
-rw-r--r--django/contrib/admin/options.py25
-rw-r--r--docs/releases/1.11.14.txt3
-rw-r--r--docs/releases/2.0.6.txt3
-rw-r--r--tests/admin_changelist/models.py3
-rw-r--r--tests/admin_changelist/tests.py85
6 files changed, 116 insertions, 4 deletions
diff --git a/AUTHORS b/AUTHORS
index 6c099aefcd..32247127d8 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -10,6 +10,7 @@ answer newbie questions, and generally made Django that much better:
Aaron T. Myers <atmyers@gmail.com>
Abhishek Gautam <abhishekg1128@yahoo.com>
Adam BogdaƂ <adam@bogdal.pl>
+ Adam Donaghy
Adam Johnson <https://github.com/adamchainz>
Adam Malinowski <http://adammalinowski.co.uk>
Adam Vandenberg
diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py
index 1eee19fea4..8d8f263db1 100644
--- a/django/contrib/admin/options.py
+++ b/django/contrib/admin/options.py
@@ -1,6 +1,7 @@
import copy
import json
import operator
+import re
from collections import OrderedDict
from functools import partial, reduce, update_wrapper
from urllib.parse import quote as urlquote
@@ -1555,6 +1556,27 @@ class ModelAdmin(BaseModelAdmin):
def change_view(self, request, object_id, form_url='', extra_context=None):
return self.changeform_view(request, object_id, form_url, extra_context)
+ def _get_edited_object_pks(self, request, prefix):
+ """Return POST data values of list_editable primary keys."""
+ pk_pattern = re.compile('{}-\d+-{}$'.format(prefix, self.model._meta.pk.name))
+ return [value for key, value in request.POST.items() if pk_pattern.match(key)]
+
+ def _get_list_editable_queryset(self, request, prefix):
+ """
+ Based on POST data, return a queryset of the objects that were edited
+ via list_editable.
+ """
+ object_pks = self._get_edited_object_pks(request, prefix)
+ queryset = self.get_queryset(request)
+ validate = queryset.model._meta.pk.to_python
+ try:
+ for pk in object_pks:
+ validate(pk)
+ except ValidationError:
+ # Disable the optimization if the POST data was tampered with.
+ return queryset
+ return queryset.filter(pk__in=object_pks)
+
@csrf_protect_m
def changelist_view(self, request, extra_context=None):
"""
@@ -1629,7 +1651,8 @@ class ModelAdmin(BaseModelAdmin):
# Handle POSTed bulk-edit data.
if request.method == 'POST' and cl.list_editable and '_save' in request.POST:
FormSet = self.get_changelist_formset(request)
- formset = cl.formset = FormSet(request.POST, request.FILES, queryset=self.get_queryset(request))
+ modified_objects = self._get_list_editable_queryset(request, FormSet.get_default_prefix())
+ formset = cl.formset = FormSet(request.POST, request.FILES, queryset=modified_objects)
if formset.is_valid():
changecount = 0
for form in formset.forms:
diff --git a/docs/releases/1.11.14.txt b/docs/releases/1.11.14.txt
index 06fe74464a..c433673111 100644
--- a/docs/releases/1.11.14.txt
+++ b/docs/releases/1.11.14.txt
@@ -11,3 +11,6 @@ Bugfixes
* Fixed ``WKBWriter.write()`` and ``write_hex()`` for empty polygons on
GEOS 3.6.1+ (:ticket:`29460`).
+
+* Fixed a regression in Django 1.10 that could result in large memory usage
+ when making edits using ``ModelAdmin.list_editable`` (:ticket:`28462`).
diff --git a/docs/releases/2.0.6.txt b/docs/releases/2.0.6.txt
index 8b07cb2fd8..73943885dd 100644
--- a/docs/releases/2.0.6.txt
+++ b/docs/releases/2.0.6.txt
@@ -20,3 +20,6 @@ Bugfixes
* Fixed ``WKBWriter.write()`` and ``write_hex()`` for empty polygons on
GEOS 3.6.1+ (:ticket:`29460`).
+
+* Fixed a regression in Django 1.10 that could result in large memory usage
+ when making edits using ``ModelAdmin.list_editable`` (:ticket:`28462`).
diff --git a/tests/admin_changelist/models.py b/tests/admin_changelist/models.py
index 1cec7b8a82..72ba62d16a 100644
--- a/tests/admin_changelist/models.py
+++ b/tests/admin_changelist/models.py
@@ -1,3 +1,5 @@
+import uuid
+
from django.db import models
@@ -72,6 +74,7 @@ class Invitation(models.Model):
class Swallow(models.Model):
+ uuid = models.UUIDField(primary_key=True, default=uuid.uuid4)
origin = models.CharField(max_length=255)
load = models.FloatField()
speed = models.FloatField()
diff --git a/tests/admin_changelist/tests.py b/tests/admin_changelist/tests.py
index e884a8671d..fb83718ebd 100644
--- a/tests/admin_changelist/tests.py
+++ b/tests/admin_changelist/tests.py
@@ -8,11 +8,13 @@ from django.contrib.admin.tests import AdminSeleniumTestCase
from django.contrib.admin.views.main import ALL_VAR, SEARCH_VAR
from django.contrib.auth.models import User
from django.contrib.contenttypes.models import ContentType
+from django.db import connection
from django.db.models import F
from django.db.models.functions import Upper
from django.template import Context, Template
from django.test import TestCase, override_settings
from django.test.client import RequestFactory
+from django.test.utils import CaptureQueriesContext
from django.urls import reverse
from django.utils import formats
@@ -624,9 +626,9 @@ class ChangeListTests(TestCase):
'form-INITIAL_FORMS': '3',
'form-MIN_NUM_FORMS': '0',
'form-MAX_NUM_FORMS': '1000',
- 'form-0-id': str(d.pk),
- 'form-1-id': str(c.pk),
- 'form-2-id': str(a.pk),
+ 'form-0-uuid': str(d.pk),
+ 'form-1-uuid': str(c.pk),
+ 'form-2-uuid': str(a.pk),
'form-0-load': '9.0',
'form-0-speed': '9.0',
'form-1-load': '5.0',
@@ -656,6 +658,83 @@ class ChangeListTests(TestCase):
# No new swallows were created.
self.assertEqual(len(Swallow.objects.all()), 4)
+ def test_get_edited_object_ids(self):
+ a = Swallow.objects.create(origin='Swallow A', load=4, speed=1)
+ b = Swallow.objects.create(origin='Swallow B', load=2, speed=2)
+ c = Swallow.objects.create(origin='Swallow C', load=5, speed=5)
+ superuser = self._create_superuser('superuser')
+ self.client.force_login(superuser)
+ changelist_url = reverse('admin:admin_changelist_swallow_changelist')
+ m = SwallowAdmin(Swallow, custom_site)
+ data = {
+ 'form-TOTAL_FORMS': '3',
+ 'form-INITIAL_FORMS': '3',
+ 'form-MIN_NUM_FORMS': '0',
+ 'form-MAX_NUM_FORMS': '1000',
+ 'form-0-uuid': str(a.pk),
+ 'form-1-uuid': str(b.pk),
+ 'form-2-uuid': str(c.pk),
+ 'form-0-load': '9.0',
+ 'form-0-speed': '9.0',
+ 'form-1-load': '5.0',
+ 'form-1-speed': '5.0',
+ 'form-2-load': '5.0',
+ 'form-2-speed': '4.0',
+ '_save': 'Save',
+ }
+ request = self.factory.post(changelist_url, data=data)
+ pks = m._get_edited_object_pks(request, prefix='form')
+ self.assertEqual(sorted(pks), sorted([str(a.pk), str(b.pk), str(c.pk)]))
+
+ def test_get_list_editable_queryset(self):
+ a = Swallow.objects.create(origin='Swallow A', load=4, speed=1)
+ Swallow.objects.create(origin='Swallow B', load=2, speed=2)
+ data = {
+ 'form-TOTAL_FORMS': '2',
+ 'form-INITIAL_FORMS': '2',
+ 'form-MIN_NUM_FORMS': '0',
+ 'form-MAX_NUM_FORMS': '1000',
+ 'form-0-uuid': str(a.pk),
+ 'form-0-load': '10',
+ '_save': 'Save',
+ }
+ superuser = self._create_superuser('superuser')
+ self.client.force_login(superuser)
+ changelist_url = reverse('admin:admin_changelist_swallow_changelist')
+ m = SwallowAdmin(Swallow, custom_site)
+ request = self.factory.post(changelist_url, data=data)
+ queryset = m._get_list_editable_queryset(request, prefix='form')
+ self.assertEqual(queryset.count(), 1)
+ data['form-0-uuid'] = 'INVALD_PRIMARY_KEY'
+ # The unfiltered queryset is returned if there's invalid data.
+ request = self.factory.post(changelist_url, data=data)
+ queryset = m._get_list_editable_queryset(request, prefix='form')
+ self.assertEqual(queryset.count(), 2)
+
+ def test_changelist_view_list_editable_changed_objects_uses_filter(self):
+ """list_editable edits use a filtered queryset to limit memory usage."""
+ a = Swallow.objects.create(origin='Swallow A', load=4, speed=1)
+ Swallow.objects.create(origin='Swallow B', load=2, speed=2)
+ data = {
+ 'form-TOTAL_FORMS': '2',
+ 'form-INITIAL_FORMS': '2',
+ 'form-MIN_NUM_FORMS': '0',
+ 'form-MAX_NUM_FORMS': '1000',
+ 'form-0-uuid': str(a.pk),
+ 'form-0-load': '10',
+ '_save': 'Save',
+ }
+ superuser = self._create_superuser('superuser')
+ self.client.force_login(superuser)
+ changelist_url = reverse('admin:admin_changelist_swallow_changelist')
+ with CaptureQueriesContext(connection) as context:
+ response = self.client.post(changelist_url, data=data)
+ self.assertEqual(response.status_code, 200)
+ self.assertIn('WHERE', context.captured_queries[4]['sql'])
+ self.assertIn('IN', context.captured_queries[4]['sql'])
+ # Check only the first few characters since the UUID may have dashes.
+ self.assertIn(str(a.pk)[:8], context.captured_queries[4]['sql'])
+
def test_deterministic_order_for_unordered_model(self):
"""
The primary key is used in the ordering of the changelist's results to