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

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)
+    return make_middleware_decorator(middleware_class)()
 
-            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)
+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
-                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
+                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
+                return response
+            return wraps(view_func)(_wrapped_view)
+        return auto_adapt_to_methods(_decorator)
+    return _make_decorator

django/views/decorators/cache.py

@@ -16,11 +16,22 @@ try:
 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_control(**kwargs):
 
         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 never_cache(view_func):
         add_never_cache_headers(response)
         return response
     return wraps(view_func)(_wrapped_view_func)
+never_cache = auto_adapt_to_methods(never_cache)

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 DecoratorsTest(TestCase):
         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)