From e992e57d3e66708015899efd21bb0174377baeed Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Mon, 13 Jul 2009 13:46:31 +0000 Subject: [PATCH] Fixed #11416 -- Restored use of the never_cache decorator on admin views. Thanks to Ramiro Morales and Michael Newmann for their work on the patch. git-svn-id: http://code.djangoproject.com/svn/django/trunk@11229 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/contrib/admin/sites.py | 26 +++++--- docs/ref/contrib/admin/index.txt | 28 ++++++-- tests/regressiontests/admin_views/tests.py | 74 ++++++++++++++++++++++ 3 files changed, 111 insertions(+), 17 deletions(-) diff --git a/django/contrib/admin/sites.py b/django/contrib/admin/sites.py index 6e9ef1161f..16b254ed20 100644 --- a/django/contrib/admin/sites.py +++ b/django/contrib/admin/sites.py @@ -114,20 +114,20 @@ class AdminSite(object): name = name or action.__name__ self._actions[name] = action self._global_actions[name] = action - + def disable_action(self, name): """ Disable a globally-registered action. Raises KeyError for invalid names. """ del self._actions[name] - + def get_action(self, name): """ Explicitally get a registered global action wheather it's enabled or not. Raises KeyError for invalid names. """ return self._global_actions[name] - + def actions(self): """ Get all the enabled actions as an iterable of (name, func). @@ -159,9 +159,9 @@ class AdminSite(object): if 'django.core.context_processors.auth' not in settings.TEMPLATE_CONTEXT_PROCESSORS: raise ImproperlyConfigured("Put 'django.core.context_processors.auth' in your TEMPLATE_CONTEXT_PROCESSORS setting in order to use the admin application.") - def admin_view(self, view): + def admin_view(self, view, cacheable=False): """ - Decorator to create an "admin view attached to this ``AdminSite``. This + Decorator to create an admin view attached to this ``AdminSite``. This wraps the view and provides permission checking by calling ``self.has_permission``. @@ -177,19 +177,25 @@ class AdminSite(object): url(r'^my_view/$', self.admin_view(some_view)) ) return urls + + By default, admin_views are marked non-cacheable using the + ``never_cache`` decorator. If the view can be safely cached, set + cacheable=True. """ def inner(request, *args, **kwargs): if not self.has_permission(request): return self.login(request) return view(request, *args, **kwargs) + if not cacheable: + inner = never_cache(inner) return update_wrapper(inner, view) def get_urls(self): from django.conf.urls.defaults import patterns, url, include - def wrap(view): + def wrap(view, cacheable=False): def wrapper(*args, **kwargs): - return self.admin_view(view)(*args, **kwargs) + return self.admin_view(view, cacheable)(*args, **kwargs) return update_wrapper(wrapper, view) # Admin-site-wide views. @@ -201,13 +207,13 @@ class AdminSite(object): wrap(self.logout), name='%sadmin_logout'), url(r'^password_change/$', - wrap(self.password_change), + wrap(self.password_change, cacheable=True), name='%sadmin_password_change' % self.name), url(r'^password_change/done/$', - wrap(self.password_change_done), + wrap(self.password_change_done, cacheable=True), name='%sadmin_password_change_done' % self.name), url(r'^jsi18n/$', - wrap(self.i18n_javascript), + wrap(self.i18n_javascript, cacheable=True), name='%sadmin_jsi18n' % self.name), url(r'^r/(?P\d+)/(?P.+)/$', 'django.views.defaults.shortcut'), diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt index 394ebd1f24..f0f5621fe6 100644 --- a/docs/ref/contrib/admin/index.txt +++ b/docs/ref/contrib/admin/index.txt @@ -762,12 +762,19 @@ documented in :ref:`topics-http-urls`:: anything, so you'll usually want to prepend your custom URLs to the built-in ones. -Note, however, that the ``self.my_view`` function registered above will *not* -have any permission check done; it'll be accessible to the general public. Since -this is usually not what you want, Django provides a convience wrapper to check -permissions. This wrapper is :meth:`AdminSite.admin_view` (i.e. -``self.admin_site.admin_view`` inside a ``ModelAdmin`` instance); use it like -so:: +However, the ``self.my_view`` function registered above suffers from two +problems: + + * It will *not* perform and permission checks, so it will be accessible to + the general public. + * It will *not* provide any header details to prevent caching. This means if + the page retrieves data from the database, and caching middleware is + active, the page could show outdated information. + +Since this is usually not what you want, Django provides a convenience wrapper +to check permissions and mark the view as non-cacheable. This wrapper is +:meth:`AdminSite.admin_view` (i.e. ``self.admin_site.admin_view`` inside a +``ModelAdmin`` instance); use it like so: class MyModelAdmin(admin.ModelAdmin): def get_urls(self): @@ -781,7 +788,14 @@ Notice the wrapped view in the fifth line above:: (r'^my_view/$', self.admin_site.admin_view(self.my_view)) -This wrapping will protect ``self.my_view`` from unauthorized access. +This wrapping will protect ``self.my_view`` from unauthorized access and will +apply the ``django.views.decorators.cache.never_cache`` decorator to make sure +it is not cached if the cache middleware is active. + +If the page is cacheable, but you still want the permission check to be performed, +you can pass a ``cacheable=True`` argument to :meth:`AdminSite.admin_view`:: + + (r'^my_view/$', self.admin_site.admin_view(self.my_view, cacheable=True)) .. method:: ModelAdmin.formfield_for_foreignkey(self, db_field, request, **kwargs) diff --git a/tests/regressiontests/admin_views/tests.py b/tests/regressiontests/admin_views/tests.py index 99168fdeee..38fe5ccc9d 100644 --- a/tests/regressiontests/admin_views/tests.py +++ b/tests/regressiontests/admin_views/tests.py @@ -10,6 +10,7 @@ from django.contrib.admin.models import LogEntry, DELETION from django.contrib.admin.sites import LOGIN_FORM_KEY from django.contrib.admin.util import quote from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME +from django.utils.cache import get_max_age from django.utils.html import escape # local test models @@ -1527,3 +1528,76 @@ class AdminInlineTests(TestCase): self.failUnlessEqual(Category.objects.get(id=2).order, 13) self.failUnlessEqual(Category.objects.get(id=3).order, 1) self.failUnlessEqual(Category.objects.get(id=4).order, 0) + + +class NeverCacheTests(TestCase): + fixtures = ['admin-views-users.xml', 'admin-views-colors.xml', 'admin-views-fabrics.xml'] + + def setUp(self): + self.client.login(username='super', password='secret') + + def tearDown(self): + self.client.logout() + + def testAdminIndex(self): + "Check the never-cache status of the main index" + response = self.client.get('/test_admin/admin/') + self.failUnlessEqual(get_max_age(response), 0) + + def testAppIndex(self): + "Check the never-cache status of an application index" + response = self.client.get('/test_admin/admin/admin_views/') + self.failUnlessEqual(get_max_age(response), 0) + + def testModelIndex(self): + "Check the never-cache status of a model index" + response = self.client.get('/test_admin/admin/admin_views/fabric/') + self.failUnlessEqual(get_max_age(response), 0) + + def testModelAdd(self): + "Check the never-cache status of a model add page" + response = self.client.get('/test_admin/admin/admin_views/fabric/add/') + self.failUnlessEqual(get_max_age(response), 0) + + def testModelView(self): + "Check the never-cache status of a model edit page" + response = self.client.get('/test_admin/admin/admin_views/section/1/') + self.failUnlessEqual(get_max_age(response), 0) + + def testModelHistory(self): + "Check the never-cache status of a model history page" + response = self.client.get('/test_admin/admin/admin_views/section/1/history/') + self.failUnlessEqual(get_max_age(response), 0) + + def testModelDelete(self): + "Check the never-cache status of a model delete page" + response = self.client.get('/test_admin/admin/admin_views/section/1/delete/') + self.failUnlessEqual(get_max_age(response), 0) + + def testLogin(self): + "Check the never-cache status of login views" + self.client.logout() + response = self.client.get('/test_admin/admin/') + self.failUnlessEqual(get_max_age(response), 0) + + def testLogout(self): + "Check the never-cache status of logout view" + response = self.client.get('/test_admin/admin/logout/') + self.failUnlessEqual(get_max_age(response), 0) + + def testPasswordChange(self): + "Check the never-cache status of the password change view" + self.client.logout() + response = self.client.get('/test_admin/password_change/') + self.failUnlessEqual(get_max_age(response), None) + + def testPasswordChangeDone(self): + "Check the never-cache status of the password change done view" + response = self.client.get('/test_admin/admin/password_change/done/') + self.failUnlessEqual(get_max_age(response), None) + + def testJsi18n(self): + "Check the never-cache status of the Javascript i18n view" + response = self.client.get('/test_admin/jsi18n/') + self.failUnlessEqual(get_max_age(response), None) +