Author: lukeplant
Date: 2010-02-09 09:02:39 -0600 (Tue, 09 Feb 2010)
New Revision: 12399
Modified:
django/trunk/django/contrib/admin/options.py
django/trunk/django/contrib/auth/admin.py
django/trunk/django/contrib/auth/decorators.py
django/trunk/django/contrib/formtools/wizard.py
django/trunk/django/utils/decorators.py
django/trunk/django/views/decorators/cache.py
django/trunk/docs/releases/1.2.txt
django/trunk/tests/modeltests/test_client/views.py
django/trunk/tests/regressiontests/decorators/tests.py
Log:
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.
Modified: django/trunk/django/contrib/admin/options.py
===================================================================
--- django/trunk/django/contrib/admin/options.py 2010-02-07 15:55:40 UTC
(rev 12398)
+++ django/trunk/django/contrib/admin/options.py 2010-02-09 15:02:39 UTC
(rev 12399)
@@ -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 @@
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 @@
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 @@
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 @@
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 @@
'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
Modified: django/trunk/django/contrib/auth/admin.py
===================================================================
--- django/trunk/django/contrib/auth/admin.py 2010-02-07 15:55:40 UTC (rev
12398)
+++ django/trunk/django/contrib/auth/admin.py 2010-02-09 15:02:39 UTC (rev
12399)
@@ -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 @@
(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
Modified: django/trunk/django/contrib/auth/decorators.py
===================================================================
--- django/trunk/django/contrib/auth/decorators.py 2010-02-07 15:55:40 UTC
(rev 12398)
+++ django/trunk/django/contrib/auth/decorators.py 2010-02-09 15:02:39 UTC
(rev 12399)
@@ -6,8 +6,8 @@
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):
"""
Decorator for views that checks that the user passes the given test,
@@ -26,8 +26,9 @@
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):
"""
Decorator for views that checks that the user is logged in, redirecting
@@ -41,6 +42,7 @@
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
Modified: django/trunk/django/contrib/formtools/wizard.py
===================================================================
--- django/trunk/django/contrib/formtools/wizard.py 2010-02-07 15:55:40 UTC
(rev 12398)
+++ django/trunk/django/contrib/formtools/wizard.py 2010-02-09 15:02:39 UTC
(rev 12399)
@@ -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 @@
# 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
Modified: django/trunk/django/utils/decorators.py
===================================================================
--- django/trunk/django/utils/decorators.py 2010-02-07 15:55:40 UTC (rev
12398)
+++ django/trunk/django/utils/decorators.py 2010-02-09 15:02:39 UTC (rev
12399)
@@ -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 @@
"""
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 @@
"""
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 @@
return result
return response
return wraps(view_func)(_wrapped_view)
- return auto_adapt_to_methods(_decorator)
+ return _decorator
return _make_decorator
Modified: django/trunk/django/views/decorators/cache.py
===================================================================
--- django/trunk/django/views/decorators/cache.py 2010-02-07 15:55:40 UTC
(rev 12398)
+++ django/trunk/django/views/decorators/cache.py 2010-02-09 15:02:39 UTC
(rev 12399)
@@ -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 @@
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 @@
add_never_cache_headers(response)
return response
return wraps(view_func)(_wrapped_view_func)
-never_cache = auto_adapt_to_methods(never_cache)
Modified: django/trunk/docs/releases/1.2.txt
===================================================================
--- django/trunk/docs/releases/1.2.txt 2010-02-07 15:55:40 UTC (rev 12398)
+++ django/trunk/docs/releases/1.2.txt 2010-02-09 15:02:39 UTC (rev 12399)
@@ -251,6 +251,55 @@
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
Modified: django/trunk/tests/modeltests/test_client/views.py
===================================================================
--- django/trunk/tests/modeltests/test_client/views.py 2010-02-07 15:55:40 UTC
(rev 12398)
+++ django/trunk/tests/modeltests/test_client/views.py 2010-02-09 15:02:39 UTC
(rev 12399)
@@ -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 @@
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 @@
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
Modified: django/trunk/tests/regressiontests/decorators/tests.py
===================================================================
--- django/trunk/tests/regressiontests/decorators/tests.py 2010-02-07
15:55:40 UTC (rev 12398)
+++ django/trunk/tests/regressiontests/decorators/tests.py 2010-02-09
15:02:39 UTC (rev 12399)
@@ -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 @@
fully_decorated = allow_lazy(fully_decorated)
fully_decorated = lazy(fully_decorated)
+
class DecoratorsTest(TestCase):
def test_attributes(self):
@@ -112,42 +113,25 @@
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)
+# 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)
- 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)
+simple_dec_m = method_decorator(simple_dec)
- 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)
+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"))
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en.
