Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #6371 - several decorators don't work with bound methods.

This involved changing the way the internal function
decorator_from_middleware works slightly, breaking some code that relied on
the old behaviour.  As a result, it is much simpler, but cache_page has been
made slightly more complex to cope with the change.



git-svn-id: http://code.djangoproject.com/svn/django/trunk@11586 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit afeafcd492eec9f9cb333c8c55502c1c50b3b151 1 parent d56c1ab
@spookylukey spookylukey authored
View
126 django/utils/decorators.py
@@ -2,60 +2,88 @@
import types
try:
- from functools import wraps
+ from functools import wraps, update_wrapper
except ImportError:
- from django.utils.functional import wraps # Python 2.3, 2.4 fallback.
+ from django.utils.functional import wraps, update_wrapper # Python 2.3, 2.4 fallback.
+
+class MethodDecoratorAdaptor(object):
+ """
+ Generic way of creating decorators that adapt to being
+ used on methods
+ """
+ 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 _get_name(self):
+ return self.func.__name__
+ def _get_doc(self):
+ return self.func.__doc__
+
+def auto_adapt_to_methods(decorator):
+ """Allows you to use the same decorator on methods and functions,
+ hiding the self argument from the decorator."""
+ def adapt(func):
+ return MethodDecoratorAdaptor(decorator, func)
+ return wraps(decorator)(adapt)
+
+def decorator_from_middleware_with_args(middleware_class):
+ """
+ Like decorator_from_middleware, but returns a function
+ that accepts the arguments to be passed to the middleware_class.
+ Use like::
+
+ cache_page = decorator_from_middleware(CacheMiddleware)
+ # ...
+
+ @cache_page(3600)
+ 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
- lets you use middleware functionality on a per-view basis.
+ lets you use middleware functionality on a per-view basis. The middleware
+ is created with no params passed.
"""
- def _decorator_from_middleware(*args, **kwargs):
- # For historical reasons, these "decorators" are also called as
- # dec(func, *args) instead of dec(*args)(func). We handle both forms
- # for backwards compatibility.
- has_func = True
- try:
- view_func = kwargs.pop('view_func')
- except KeyError:
- if len(args):
- view_func, args = args[0], args[1:]
- else:
- has_func = False
- if not (has_func and isinstance(view_func, types.FunctionType)):
- # We are being called as a decorator.
- if has_func:
- args = (view_func,) + args
- middleware = middleware_class(*args, **kwargs)
-
- def decorator_func(fn):
- return _decorator_from_middleware(fn, *args, **kwargs)
- return decorator_func
-
- middleware = middleware_class(*args, **kwargs)
-
- def _wrapped_view(request, *args, **kwargs):
- if hasattr(middleware, 'process_request'):
- result = middleware.process_request(request)
- if result is not None:
- return result
- if hasattr(middleware, 'process_view'):
- result = middleware.process_view(request, view_func, args, kwargs)
- if result is not None:
- return result
- try:
- response = view_func(request, *args, **kwargs)
- except Exception, e:
- if hasattr(middleware, 'process_exception'):
- result = middleware.process_exception(request, e)
+ 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)
+ def _decorator(view_func):
+ def _wrapped_view(request, *args, **kwargs):
+ if hasattr(middleware, 'process_request'):
+ result = middleware.process_request(request)
+ if result is not None:
+ return result
+ if hasattr(middleware, 'process_view'):
+ result = middleware.process_view(request, view_func, args, kwargs)
+ if result is not None:
+ return result
+ try:
+ response = view_func(request, *args, **kwargs)
+ except Exception, e:
+ if hasattr(middleware, 'process_exception'):
+ result = middleware.process_exception(request, e)
+ if result is not None:
+ return result
+ raise
+ if hasattr(middleware, 'process_response'):
+ result = middleware.process_response(request, response)
if result is not None:
return result
- raise
- if hasattr(middleware, 'process_response'):
- result = middleware.process_response(request, response)
- if result is not None:
- return result
- return response
- return wraps(view_func)(_wrapped_view)
- return _decorator_from_middleware
+ return response
+ return wraps(view_func)(_wrapped_view)
+ return auto_adapt_to_methods(_decorator)
+ return _make_decorator
View
18 django/views/decorators/cache.py
@@ -16,11 +16,22 @@
except ImportError:
from django.utils.functional import wraps # Python 2.3, 2.4 fallback.
-from django.utils.decorators import decorator_from_middleware
+from django.utils.decorators import decorator_from_middleware_with_args, auto_adapt_to_methods
from django.utils.cache import patch_cache_control, add_never_cache_headers
from django.middleware.cache import CacheMiddleware
-cache_page = decorator_from_middleware(CacheMiddleware)
+def cache_page(*args, **kwargs):
+ # We need backwards compatibility with code which spells it this way:
+ # def my_view(): pass
+ # my_view = cache_page(123, my_view)
+ # and this way:
+ # my_view = cache_page(123)(my_view)
+ timeout = args[0]
+ if len(args) > 1:
+ fn = args[1]
+ return decorator_from_middleware_with_args(CacheMiddleware)(timeout)(fn)
+ else:
+ return decorator_from_middleware_with_args(CacheMiddleware)(timeout)
def cache_control(**kwargs):
@@ -33,7 +44,7 @@ def _cache_controlled(request, *args, **kw):
return wraps(viewfunc)(_cache_controlled)
- return _cache_controller
+ return auto_adapt_to_methods(_cache_controller)
def never_cache(view_func):
"""
@@ -45,3 +56,4 @@ 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
66 tests/regressiontests/decorators/tests.py
@@ -1,11 +1,16 @@
from unittest import TestCase
from sys import version_info
+try:
+ from functools import wraps
+except ImportError:
+ from django.utils.functional import wraps # Python 2.3, 2.4 fallback.
-from django.http import HttpResponse
+from django.http import HttpResponse, HttpRequest
from django.utils.functional import allow_lazy, lazy, memoize
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.contrib.auth.decorators import login_required, permission_required, user_passes_test
from django.contrib.admin.views.decorators import staff_member_required
@@ -84,4 +89,61 @@ class DummyRequest(object): pass
response = callback(request)
self.assertEqual(response, ['test2', 'test1'])
-
+
+ def test_cache_page_new_style(self):
+ """
+ Test that we can call cache_page the new way
+ """
+ def my_view(request):
+ return "response"
+ my_view_cached = cache_page(123)(my_view)
+ self.assertEqual(my_view_cached(HttpRequest()), "response")
+
+ def test_cache_page_old_style(self):
+ """
+ Test that we can call cache_page the old way
+ """
+ def my_view(request):
+ return "response"
+ my_view_cached = cache_page(123, my_view)
+ self.assertEqual(my_view_cached(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)
Please sign in to comment.
Something went wrong with that request. Please try again.