Ticket #14512: ticket14512.diff

django/utils/decorators.py

@@ -1 +1 @@
 "Functions that help with dynamically creating decorators for views."
 
-import types
 try:
     from functools import wraps, update_wrapper, WRAPPER_ASSIGNMENTS
 except ImportError:
     from django.utils.functional import wraps, update_wrapper, WRAPPER_ASSIGNMENTS  # Python 2.4 fallback.
 
+class classonlymethod(classmethod):
+    def __get__(self, instance, owner):
+        if instance is not None:
+            raise AttributeError("This method is available only on the view class.")
+        return super(classonlymethod, self).__get__(instance, owner)
 
 def method_decorator(decorator):
     """
@@ -39 +43 @@
     return _dec
 
 
+def view_decorator(fdec, subclass=False):
+    """
+    Change a function decorator into a view decorator.
+
+    This is a simplest approach possible. `as_view()` is replaced, so
+    that it applies the given decorator before returning.
+
+    In this approach, decorators are always put on top - that means it's not
+    possible to have functions called in this order:
+
+       B.dispatch, login_required, A.dispatch
+
+    NOTE: By default this modifies the given class, so be careful when doing this:
+
+       TemplateView = view_decorator(login_required)(TemplateView)
+
+    Because it will modify the TemplateView class. Instead create a fresh
+    class first and apply the decorator there. A shortcut for this is
+    specifying the ``subclass`` argument. But this is also dangerous. Consider:
+
+        @view_decorator(login_required, subclass=True)
+        class MyView(View):
+
+            def get_context_data(self):
+                data = super(MyView, self).get_context_data()
+                data["foo"] = "bar"
+                return data
+
+    This looks like a normal Python code, but there is a hidden infinite
+    recursion, because of how `super()` works in Python 2.x; By the time
+    `get_context_data()` is invoked, MyView refers to a subclass created in
+    the decorator. super() looks at the next class in the MRO of MyView,
+    which is the original MyView class we created, so it contains the
+    `get_context_data()` method. Which is exactly the method that was just
+    called. BOOM!
+    """
+    def decorator(cls):
+        if subclass:
+            cls = type("%sWithDecorator(%s)" % (cls.__name__, fdec.__name__), (cls,), {})
+        @wraps(cls.as_view)
+        def as_view(current, **initkwargs):
+            return fdec(super(cls, current).as_view(**initkwargs))
+        cls.as_view = classonlymethod(as_view)
+        return cls
+    return decorator
+
+
 def decorator_from_middleware_with_args(middleware_class):
     """
     Like decorator_from_middleware, but returns a function

django/views/generic/base.py

@@ -1 @@
-import copy
 from django import http
 from django.core.exceptions import ImproperlyConfigured
 from django.template import RequestContext, loader
-from django.utils.translation import ugettext_lazy as _
 from django.utils.functional import update_wrapper
 from django.utils.log import getLogger
+from django.utils.decorators import classonlymethod
 
 logger = getLogger('django.request')
 
-class classonlymethod(classmethod):
-    def __get__(self, instance, owner):
-        if instance is not None:
-            raise AttributeError("This method is available only on the view class.")
-        return super(classonlymethod, self).__get__(instance, owner)
-
 class View(object):
     """
     Intentionally simple parent class for all views. Only implements

docs/topics/class-based-views.txt

@@ -537 +537 @@
 :func:`render_to_response()` implementation will override the
 versions provided by :class:`JSONResponseMixin` and
 :class:`~django.views.generic.detail.SingleObjectTemplateResponseMixin`.
+
+Decorating
+===========
+
+.. highlightlang:: python
+
+While class-based views and mixins introduce a whole new way of extending
+view behaviour in a reusable way, you also use existing decorators.
+
+
+Decorating in URLconf
+-----------------------
+
+The simplest way of decorating class-based views is to decorate the result
+of the :meth:`~django.views.generic.base.View.as_view` method. This
+is most useful in the URLconf::
+
+    from django.views.generic import TemplateView
+    from django.contrib.auth.decorators import login_required
+
+    urlpatterns = patterns('',
+        (r'^about/',login_required(TemplateView.as_view(template_name="secret.html"))),
+    )
+
+The downside of this approach is that you can't define a class-based view with
+the decorator always applied to it.
+
+
+Overriding ``as_view`` class method
+--------------------------------------
+
+If you want your view to have a decorator applied and be able to subclass
+it later, you need to override one of it's methods. The most straightforward
+solution is to override :meth:`~django.views.generic.base.View.as_view`::
+
+    from django.views.generic import TemplateView
+    from django.utils.decorators import classonlymethod
+
+    class ProtectedView(TemplateView):
+
+        @classonlymethod
+        def as_view(cls, **initkwargs):
+            return login_required(super(ProtectedView, cls).as_view(**initkwargs))
+
+This will make the ``ProtectedView`` and any of it's subclasses always apply the
+``login_required`` decorator. The  ``classonlymethod`` decorator is a variation
+of the standard Python ``classmethod`` decorator that lets you invoke the
+method directly from the class, but not from an instance.
+
+
+Class decorators
+-----------------
+
+Django provides also provides a way to turn function decorators into
+class decorators::
+
+    from django.utils.decorators import view_decorator
+
+    class ProtectedView(TemplateView):
+        pass
+    ProtectedView = view_decorator(login_required)(ProtectedView)
+
+In Python 2.6 and above you can also use class decorator syntax for this::
+
+    from django.utils.decorators import view_decorator
+
+    @view_decorator(login_required)
+    class ProtectedView(TempalateView):
+        pass
+
+.. note::
+
+    The produced class decorator modifies the class it was given, so it is 
+    *not safe* to decorate generic views this way **without creating a subclass**.
+
+    If you are absolutely sure you know how super() and MRO works in Python, you
+    can pass a ``subclass`` keyword to make the decorator create an inline
+    subclass for you::
+
+        ProtectedView = view_decorator(login_required, subclass=True)(TemplateView)
+
+    You have been warned!

tests/regressiontests/generic_views/base.py

@@ -22 +22 @@
 class CustomizableView(SimpleView):
     parameter = {}
 
+
 def decorator(view):
     view.is_decorated = True
     return view
@@ -149 +150 @@
         """
         self.assertTrue(DecoratedDispatchView.as_view().is_decorated)
 
-
 class TemplateViewTest(TestCase):
     urls = 'regressiontests.generic_views.urls'
 

tests/regressiontests/utils/decorators.py

@@ -1 @@
-from django.test import TestCase
+from django.test import TestCase, RequestFactory
+from django.utils import unittest
+from django.utils.decorators import method_decorator, wraps, view_decorator
+from django.views.generic import View
+
 
 class DecoratorFromMiddlewareTests(TestCase):
     """
@@ -17 +21 @@
         Test a middleware that implements process_view, operating on a callable class.
         """
         self.client.get('/utils/class_xview/')
+
+
+def simple_dec(func):
+    """
+    Simple decotator for testing method_decorator and view_decorator.
+    It assumes a request argument and one extra argument.  Appends
+    string "decorator:" to the result. It also sets `is_decorated` attribute
+    on the wrappper.
+    """
+    def wrapper(request, arg):
+        return "decorator:" + func(request, arg)
+    wrapper = wraps(func)(wrapper)
+    wrapper.is_decorated = True
+    return wrapper
+
+
+class MethodDecoratorTests(TestCase):
+    """
+    Tests for method_decorator.
+    """
+
+    def test_method_decorator(self):
+        simple_dec_m = method_decorator(simple_dec)
+
+        class Test(object):
+            @simple_dec_m
+            def say(self, request, arg):
+                return arg
+
+        self.assertEqual("decorator:hello", Test().say(None, "hello"))
+        self.assertTrue(getattr(Test().say, "is_decorated", False),
+                "Method decorator didn't preserve attributes.")
+
+
+class ClassBasedViewDecorationTests(TestCase):
+    rf = RequestFactory()
+
+    def test_decorate_view(self):
+        class TextView(View):
+            "Docstring"
+            def get(self, request, text):
+                return "get:" + text
+            def post(self, request, text):
+                return "post:" + text
+        TextView = view_decorator(simple_dec)(TextView)
+
+        self.assertTrue(getattr(TextView.as_view(), "is_decorated", False),
+                    "Class based view decorator didn't preserve attributes.")
+        self.assertEqual(TextView.as_view().__doc__, "Docstring",
+                    "Class based view decorator didn't preserve docstring.")
+        self.assertEqual(TextView.as_view()(self.rf.get('/'), "hello"), "decorator:get:hello")
+        self.assertEqual(TextView.as_view()(self.rf.post('/'), "hello"), "decorator:post:hello")
+
+    def test_super_calls(self):
+        class TextView(View):
+            def dispatch(self, request, text):
+                return "view1:" + text
+
+        # NOTE: it's important for this test, that the definition
+        # and decoration of the class happens in the *same scope*.
+        class ViewWithSuper(TextView):
+
+            def __init__(self, **initargs):
+                self.recursion_count = 0
+                super(ViewWithSuper, self).__init__(**initargs)
+
+            def dispatch(self, *args, **kwargs):
+                self.recursion_count += 1
+                if self.recursion_count > 10:
+                    raise Exception("Decoration caused recursive super() calls.")
+                return "view2:" + super(ViewWithSuper, self).dispatch(*args, **kwargs)
+        ViewWithSuper = view_decorator(simple_dec)(ViewWithSuper)
+
+        self.assertEqual(ViewWithSuper.as_view()(self.rf.get('/'), "A"), "decorator:view2:view1:A")
+
+    @unittest.expectedFailure
+    def test_super_calls_with_subclassing(self):
+        class TextView(View):
+            def dispatch(self, request, text):
+                return "view1:" + text
+
+        # NOTE: it's important for this test, that the definition
+        # and decoration of the class happens in the *same scope*.
+        class ViewWithSuper(TextView):
+
+            def __init__(self, **initargs):
+                self.recursion_count = 0
+                super(ViewWithSuper, self).__init__(**initargs)
+
+            def dispatch(self, *args, **kwargs):
+                self.recursion_count += 1
+                if self.recursion_count > 10:
+                    raise RuntimeError("Decoration caused recursive super() calls.")
+                return "view2:" + super(ViewWithSuper, self).dispatch(*args, **kwargs)
+        ViewWithSuper = view_decorator(simple_dec, subclass=True)(ViewWithSuper)
+
+        self.assertEqual(ViewWithSuper.as_view()(self.rf.get('/'), "A"), "decorator:view2:view1:A")
+
+    def test_subclassing_decorated(self):
+        """
+        Test that decorators are always pushed to front.
+        """
+        class TextView(View):
+            def dispatch(self, request, text):
+                return "view1:" + text
+        TextView = view_decorator(simple_dec)(TextView)
+
+        class SubView(TextView):
+            def dispatch(self, *args, **kwargs):
+                return "view2:" + super(SubView, self).dispatch(*args, **kwargs)
+
+        self.assertEqual(SubView.as_view()(self.rf.get('/'), "A"), "decorator:view2:view1:A")
+
+    @unittest.expectedFailure
+    def test_base_unmodified(self):
+        class TextView(View):
+            attr = "OK"
+            def dispatch(self, request, text):
+                return "view1:" + text
+        DecoratedView = view_decorator(simple_dec)(TextView)
+        self.assertEqual(DecoratedView.as_view()(self.rf.get('/'), "A"), "decorator:view1:A")
+        self.assertEqual(TextView.as_view()(self.rf.get('/'), "A"), "view1:A")
+        self.assertFalse(DecoratedView is TextView)
+        self.assertEqual(DecoratedView.mro(), [DecoratedView, TextView, View, object])
+
+    def test_base_unmodified_with_subclassing(self):
+        class TextView(View):
+            attr = "OK"
+            def dispatch(self, request, text):
+                return "view1:" + text
+        DecoratedView = view_decorator(simple_dec, subclass=True)(TextView)
+
+        self.assertEqual(DecoratedView.as_view()(self.rf.get('/'), "A"), "decorator:view1:A")
+        self.assertEqual(TextView.as_view()(self.rf.get('/'), "A"), "view1:A")
+        self.assertFalse(DecoratedView is TextView)
+        self.assertEqual(DecoratedView.mro(), [DecoratedView, TextView, View, object])