Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #12804 - regression with decorating admin views.

This is a BACKWARDS INCOMPATIBLE change, because it removes the flawed
'auto_adapt_to_methods' decorator, and replaces it with 'method_decorator'
which must be applied manually when necessary, as described in the 1.2
release notes.

For users of 1.1 and 1.0, this affects the decorators:

 * login_required
 * permission_required
 * user_passes_test

For those following trunk, this also affects:

 * csrf_protect
 * anything created with decorator_from_middleware 

If a decorator does not depend on the signature of the function it is
supposed to decorate (for example if it only does post-processing of the
result), it will not be affected.
 



git-svn-id: http://code.djangoproject.com/svn/django/trunk@12399 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 4bff19463361927ff752929da1bc04e9aa1c2e72 1 parent edb6d75
@spookylukey spookylukey authored
View
10 django/contrib/admin/options.py
@@ -13,6 +13,7 @@
from django.db.models.fields import BLANK_CHOICE_DASH
from django.http import Http404, HttpResponse, HttpResponseRedirect
from django.shortcuts import get_object_or_404, render_to_response
+from django.utils.decorators import method_decorator
from django.utils.datastructures import SortedDict
from django.utils.functional import update_wrapper
from django.utils.html import escape
@@ -53,6 +54,7 @@ class IncorrectLookupParameters(Exception):
models.FileField: {'widget': widgets.AdminFileWidget},
}
+csrf_protect_m = method_decorator(csrf_protect)
class BaseModelAdmin(object):
"""Functionality common to both ModelAdmin and InlineAdmin."""
@@ -754,7 +756,7 @@ def response_action(self, request, queryset):
msg = _("No action selected.")
self.message_user(request, msg)
- @csrf_protect
+ @csrf_protect_m
@transaction.commit_on_success
def add_view(self, request, form_url='', extra_context=None):
"The 'add' admin view for this model."
@@ -844,7 +846,7 @@ def add_view(self, request, form_url='', extra_context=None):
context.update(extra_context or {})
return self.render_change_form(request, context, form_url=form_url, add=True)
- @csrf_protect
+ @csrf_protect_m
@transaction.commit_on_success
def change_view(self, request, object_id, extra_context=None):
"The 'change' admin view for this model."
@@ -936,7 +938,7 @@ def change_view(self, request, object_id, extra_context=None):
context.update(extra_context or {})
return self.render_change_form(request, context, change=True, obj=obj)
- @csrf_protect
+ @csrf_protect_m
def changelist_view(self, request, extra_context=None):
"The 'change list' admin view for this model."
from django.contrib.admin.views.main import ERROR_FLAG
@@ -1057,7 +1059,7 @@ def changelist_view(self, request, extra_context=None):
'admin/change_list.html'
], context, context_instance=context_instance)
- @csrf_protect
+ @csrf_protect_m
def delete_view(self, request, object_id, extra_context=None):
"The 'delete' admin view for this model."
opts = self.model._meta
View
5 django/contrib/auth/admin.py
@@ -10,9 +10,12 @@
from django.shortcuts import render_to_response, get_object_or_404
from django.template import RequestContext
from django.utils.html import escape
+from django.utils.decorators import method_decorator
from django.utils.translation import ugettext, ugettext_lazy as _
from django.views.decorators.csrf import csrf_protect
+csrf_protect_m = method_decorator(csrf_protect)
+
class GroupAdmin(admin.ModelAdmin):
search_fields = ('name',)
ordering = ('name',)
@@ -76,7 +79,7 @@ def get_urls(self):
(r'^(\d+)/password/$', self.admin_site.admin_view(self.user_change_password))
) + super(UserAdmin, self).get_urls()
- @csrf_protect
+ @csrf_protect_m
@transaction.commit_on_success
def add_view(self, request, form_url='', extra_context=None):
# It's an error for a user to have add permission but NOT change
View
6 django/contrib/auth/decorators.py
@@ -6,7 +6,7 @@
from django.contrib.auth import REDIRECT_FIELD_NAME
from django.http import HttpResponseRedirect
from django.utils.http import urlquote
-from django.utils.decorators import auto_adapt_to_methods
+
def user_passes_test(test_func, login_url=None, redirect_field_name=REDIRECT_FIELD_NAME):
"""
@@ -26,7 +26,8 @@ def _wrapped_view(request, *args, **kwargs):
tup = login_url, redirect_field_name, path
return HttpResponseRedirect('%s?%s=%s' % tup)
return wraps(view_func)(_wrapped_view)
- return auto_adapt_to_methods(decorator)
+ return decorator
+
def login_required(function=None, redirect_field_name=REDIRECT_FIELD_NAME):
"""
@@ -41,6 +42,7 @@ def login_required(function=None, redirect_field_name=REDIRECT_FIELD_NAME):
return actual_decorator(function)
return actual_decorator
+
def permission_required(perm, login_url=None):
"""
Decorator for views that checks whether a user has a particular permission
View
4 django/contrib/formtools/wizard.py
@@ -14,8 +14,10 @@
from django.utils.hashcompat import md5_constructor
from django.utils.translation import ugettext_lazy as _
from django.contrib.formtools.utils import security_hash
+from django.utils.decorators import method_decorator
from django.views.decorators.csrf import csrf_protect
+
class FormWizard(object):
# Dictionary of extra template context variables.
extra_context = {}
@@ -45,7 +47,7 @@ def num_steps(self):
# hook methods might alter self.form_list.
return len(self.form_list)
- @csrf_protect
+ @method_decorator(csrf_protect)
def __call__(self, request, *args, **kwargs):
"""
Main method that does all the hard work, conforming to the Django view
View
54 django/utils/decorators.py
@@ -7,44 +7,24 @@
from django.utils.functional import wraps, update_wrapper # Python 2.3, 2.4 fallback.
-# Licence for MethodDecoratorAdaptor and auto_adapt_to_methods
-#
-# This code is taken from stackoverflow.com [1], the code being supplied by
-# users 'Ants Aasma' [2] and 'Silent Ghost' [3] with modifications. It is
-# legally included here under the terms of the Creative Commons
-# Attribution-Share Alike 2.5 Generic Licence [4]
-#
-# [1] http://stackoverflow.com/questions/1288498/using-the-same-decorator-with-arguments-with-functions-and-methods
-# [2] http://stackoverflow.com/users/107366/ants-aasma
-# [3] http://stackoverflow.com/users/12855/silentghost
-# [4] http://creativecommons.org/licenses/by-sa/2.5/
-
-class MethodDecoratorAdaptor(object):
+def method_decorator(decorator):
"""
- Generic way of creating decorators that adapt to being
- used on methods
+ Converts a function decorator into a method decorator
"""
- def __init__(self, decorator, func):
- update_wrapper(self, func)
- # NB: update the __dict__ first, *then* set
- # our own .func and .decorator, in case 'func' is actually
- # another MethodDecoratorAdaptor object, which has its
- # 'func' and 'decorator' attributes in its own __dict__
- self.decorator = decorator
- self.func = func
- def __call__(self, *args, **kwargs):
- return self.decorator(self.func)(*args, **kwargs)
- def __get__(self, instance, owner):
- return self.decorator(self.func.__get__(instance, owner))
+ def _dec(func):
+ def _wrapper(self, *args, **kwargs):
+ def bound_func(*args2, **kwargs2):
+ return func(self, *args2, **kwargs2)
+ # bound_func has the signature that 'decorator' expects i.e. no
+ # 'self' argument, but it is a closure over self so it can call
+ # 'func' correctly.
+ return decorator(bound_func)(*args, **kwargs)
+ return wraps(func)(_wrapper)
+ update_wrapper(_dec, decorator)
+ # Change the name to aid debugging.
+ _dec.__name__ = 'method_dec(%s)' % decorator.__name__
+ return _dec
-def auto_adapt_to_methods(decorator):
- """
- Takes a decorator function, and returns a decorator-like callable that can
- be used on methods as well as functions.
- """
- def adapt(func):
- return MethodDecoratorAdaptor(decorator, func)
- return wraps(decorator)(adapt)
def decorator_from_middleware_with_args(middleware_class):
"""
@@ -61,6 +41,7 @@ def my_view(request):
"""
return make_middleware_decorator(middleware_class)
+
def decorator_from_middleware(middleware_class):
"""
Given a middleware class (not an instance), returns a view decorator. This
@@ -69,6 +50,7 @@ def decorator_from_middleware(middleware_class):
"""
return make_middleware_decorator(middleware_class)()
+
def make_middleware_decorator(middleware_class):
def _make_decorator(*m_args, **m_kwargs):
middleware = middleware_class(*m_args, **m_kwargs)
@@ -96,5 +78,5 @@ def _wrapped_view(request, *args, **kwargs):
return result
return response
return wraps(view_func)(_wrapped_view)
- return auto_adapt_to_methods(_decorator)
+ return _decorator
return _make_decorator
View
10 django/views/decorators/cache.py
@@ -16,10 +16,11 @@
except ImportError:
from django.utils.functional import wraps # Python 2.3, 2.4 fallback.
-from django.utils.decorators import decorator_from_middleware_with_args, auto_adapt_to_methods
+from django.utils.decorators import decorator_from_middleware_with_args
from django.utils.cache import patch_cache_control, add_never_cache_headers
from django.middleware.cache import CacheMiddleware
+
def cache_page(*args, **kwargs):
# We need backwards compatibility with code which spells it this way:
# def my_view(): pass
@@ -48,18 +49,16 @@ def cache_page(*args, **kwargs):
else:
return decorator_from_middleware_with_args(CacheMiddleware)(cache_timeout=args[0], key_prefix=key_prefix)
-def cache_control(**kwargs):
+def cache_control(**kwargs):
def _cache_controller(viewfunc):
-
def _cache_controlled(request, *args, **kw):
response = viewfunc(request, *args, **kw)
patch_cache_control(response, **kwargs)
return response
-
return wraps(viewfunc)(_cache_controlled)
+ return _cache_controller
- return auto_adapt_to_methods(_cache_controller)
def never_cache(view_func):
"""
@@ -71,4 +70,3 @@ def _wrapped_view_func(request, *args, **kwargs):
add_never_cache_headers(response)
return response
return wraps(view_func)(_wrapped_view_func)
-never_cache = auto_adapt_to_methods(never_cache)
View
49 docs/releases/1.2.txt
@@ -251,6 +251,55 @@ incompatibilities, especially if you are storing comma or semi-colon in
cookies and have javascript code that parses and manipulates cookie values
client-side.
+``user_passes_test``, ``login_required`` and ``permission_required``
+--------------------------------------------------------------------
+
+``django.contrib.auth.decorators`` provides the decorators ``login_required``,
+``permission_required`` and ``user_passes_test``. Previously it was possible to
+use these decorators both on functions (where the first argument is 'request')
+and on methods (where the first argument is 'self', and the second argument is
+'request'). However, we have found that the trick which enabled this is
+flawed. It only works in limited circumstances, and produces errors that are
+very difficult to debug when it does not work.
+
+For this reason, the 'auto adapt' behaviour has been removed, and if you are
+using these decorators on methods, you will need to manually apply
+:func:`django.utils.decorators.method_decorator` to convert the decorator to one
+that works with methods. You would change code from this::
+
+ class MyClass(object):
+
+ @login_required
+ def my_view(self, request):
+ pass
+
+to this::
+
+ from django.utils.decorators import method_decorator
+
+ class MyClass(object):
+
+ @method_decorator(login_required)
+ def my_view(self, request):
+ pass
+
+or::
+
+ from django.utils.decorators import method_decorator
+
+ login_required_m = method_decorator(login_required)
+
+ class MyClass(object):
+
+ @login_required_m
+ def my_view(self, request):
+ pass
+
+For those following trunk, this change also applies to other decorators
+introduced since 1.1, including ``csrf_protect``, ``cache_control`` and anything
+created using ``decorator_from_middleware``.
+
+
.. _deprecated-features-1.2:
Features deprecated in 1.2
View
5 tests/modeltests/test_client/views.py
@@ -7,6 +7,7 @@
from django.forms.forms import Form
from django.forms import fields
from django.shortcuts import render_to_response
+from django.utils.decorators import method_decorator
def get_view(request):
"A simple view that expects a GET request, and returns a rendered template"
@@ -147,14 +148,15 @@ def permission_protected_view(request):
permission_protected_view = permission_required('modeltests.test_perm')(permission_protected_view)
class _ViewManager(object):
+ @method_decorator(login_required)
def login_protected_view(self, request):
t = Template('This is a login protected test using a method. '
'Username is {{ user.username }}.',
name='Login Method Template')
c = Context({'user': request.user})
return HttpResponse(t.render(c))
- login_protected_view = login_required(login_protected_view)
+ @method_decorator(permission_required('modeltests.test_perm'))
def permission_protected_view(self, request):
t = Template('This is a permission protected test using a method. '
'Username is {{ user.username }}. '
@@ -162,7 +164,6 @@ def permission_protected_view(self, request):
name='Permissions Template')
c = Context({'user': request.user})
return HttpResponse(t.render(c))
- permission_protected_view = permission_required('modeltests.test_perm')(permission_protected_view)
_view_manager = _ViewManager()
login_protected_method_view = _view_manager.login_protected_view
View
64 tests/regressiontests/decorators/tests.py
@@ -10,7 +10,7 @@
from django.views.decorators.http import require_http_methods, require_GET, require_POST
from django.views.decorators.vary import vary_on_headers, vary_on_cookie
from django.views.decorators.cache import cache_page, never_cache, cache_control
-from django.utils.decorators import auto_adapt_to_methods
+from django.utils.decorators import method_decorator
from django.contrib.auth.decorators import login_required, permission_required, user_passes_test
from django.contrib.admin.views.decorators import staff_member_required
@@ -47,6 +47,7 @@ def fully_decorated(request):
fully_decorated = allow_lazy(fully_decorated)
fully_decorated = lazy(fully_decorated)
+
class DecoratorsTest(TestCase):
def test_attributes(self):
@@ -112,42 +113,25 @@ def my_view(request):
my_view_cached2 = cache_page(my_view, 123, key_prefix="test")
self.assertEqual(my_view_cached2(HttpRequest()), "response")
-class MethodDecoratorAdapterTests(TestCase):
- def test_auto_adapt_to_methods(self):
- """
- Test that auto_adapt_to_methods actually works.
- """
- # Need 2 decorators with auto_adapt_to_methods,
- # to check it plays nicely with composing itself.
-
- def my_decorator(func):
- def wrapped(*args, **kwargs):
- # need to ensure that the first arg isn't 'self'
- self.assertEqual(args[0], "test")
- return "my_decorator:" + func(*args, **kwargs)
- wrapped.my_decorator_custom_attribute = True
- return wraps(func)(wrapped)
- my_decorator = auto_adapt_to_methods(my_decorator)
-
- def my_decorator2(func):
- def wrapped(*args, **kwargs):
- # need to ensure that the first arg isn't 'self'
- self.assertEqual(args[0], "test")
- return "my_decorator2:" + func(*args, **kwargs)
- wrapped.my_decorator2_custom_attribute = True
- return wraps(func)(wrapped)
- my_decorator2 = auto_adapt_to_methods(my_decorator2)
-
- class MyClass(object):
- def my_method(self, *args, **kwargs):
- return "my_method:%r %r" % (args, kwargs)
- my_method = my_decorator2(my_decorator(my_method))
-
- obj = MyClass()
- self.assertEqual(obj.my_method("test", 123, name='foo'),
- "my_decorator2:my_decorator:my_method:('test', 123) {'name': 'foo'}")
- self.assertEqual(obj.my_method.__name__, 'my_method')
- self.assertEqual(getattr(obj.my_method, 'my_decorator_custom_attribute', False),
- True)
- self.assertEqual(getattr(obj.my_method, 'my_decorator2_custom_attribute', False),
- True)
+
+# For testing method_decorator, a decorator that assumes a single argument.
+# We will get type arguments if there is a mismatch in the number of arguments.
+def simple_dec(func):
+ def wrapper(arg):
+ return func("test:" + arg)
+ return wraps(func)(wrapper)
+
+simple_dec_m = method_decorator(simple_dec)
+
+
+class MethodDecoratorTests(TestCase):
+ """
+ Tests for method_decorator
+ """
+ def test_method_decorator(self):
+ class Test(object):
+ @simple_dec_m
+ def say(self, arg):
+ return arg
+
+ self.assertEqual("test:hello", Test().say("hello"))
Please sign in to comment.
Something went wrong with that request. Please try again.