Ticket #14512: ticket14512_draft.diff

django/utils/decorators.py

@@ -39 +39 @@
     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 class it's called upon, so *MUST NOT* do:
+
+       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. This is potentially 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!
+    """
+
+    from django.views.generic.base import classonlymethod
+    def decorator(cls):
+        if subclass:
+            cls = type("%sWithDecorator(%s)" % (cls.__name__, fdec.__name__), (cls,), {})
+        @wraps(cls.as_view.__func__)
+        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

@@ -5 +5 @@
 from django.utils.translation import ugettext_lazy as _
 from django.utils.functional import update_wrapper
 from django.utils.log import getLogger
+from itertools import chain
 
 logger = getLogger('django.request')
 
@@ -21 +22 @@
     """
 
     http_method_names = ['get', 'post', 'put', 'delete', 'head', 'options', 'trace']
+    decorators = ()
 
     def __init__(self, **kwargs):
         """
@@ -32 +34 @@
         for key, value in kwargs.iteritems():
             setattr(self, key, value)
 
+    @classmethod
+    def get_decorators(cls):
+        decorators = []
+        # decorators in the subclass should be applied after the ones in base
+        for klass in reversed(cls.__mro__):
+            # decorators defined in the class should be applied in 
+            # the reverse order they're specified 
+            decorators.extend(reversed(klass.__dict__.get('decorators', ())))
+        return decorators
+
     @classonlymethod
     def as_view(cls, **initkwargs):
         """
@@ -57 +69 @@
         # and possible attributes set by decorators
         # like csrf_exempt from dispatch
         update_wrapper(view, cls.dispatch, assigned=())
+
+        # apply all defined decorators
+        for decorator in cls.get_decorators():
+            view = decorator(view)
+
         return view
 
     def dispatch(self, request, *args, **kwargs):

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.
+
+Simple decoration
+------------------
+
+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, 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
+decorator. You don't need to worry about ``classonlymethod`` decorator.
+It's not required for your view to work, but prevents from invoking
+``as_view()`` on the view's instance by accident.
+
+
+.. note::
+
+    This way of decorating (as the previous one) always applies the decorators
+    at creation time, before any code in `dispatch()` has any chance to run.
+
+Class decorator
+--------------------
+
+Python 2.6 introduces new syntax that allows you to decorate classes. Django
+provides such a decorator for class-based views. The example above can
+be simplified to::
+
+    from django.utils.decorators import view_decorator
+
+    @view_decorator(login_required)
+    class ProtectedView(TemplateView)
+        pass
+
+.. note::
+
+    The 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 absolutly sure you know how super() and MRO works in Python, you
+    can pass ``subclass=True`` to the decorator and it will create a sublass
+    for you. You have been warned!

tests/regressiontests/generic_views/base.py

@@ -6 +6 @@
 from django.utils import simplejson
 from django.views.generic import View, TemplateView, RedirectView
 
+def prepend_string(text):
+    "Decorator that prepends strings to responses"
+    from django.utils.functional import wraps
+    def decorator(view):
+        @wraps(view)
+        def decorated_view(request, *args, **kwargs):
+            return text + ':' + view(request, *args, **kwargs)
+        return decorated_view
+    return decorator
 
 class SimpleView(View):
     """
@@ -149 +158 @@
         """
         self.assertTrue(DecoratedDispatchView.as_view().is_decorated)
 
+    def test_defining_decorators(self):
+        """
+        Test an alternate method of decoration.
+        """
+        class Base(View):
+            def get(self, request, *args, **kwargs):
+                return "Base"
+
+        class B(Base):
+            decorators = (prepend_string('B'),)
+            
+        class AB(B):
+            decorators = (prepend_string('A'),)
+
+        class CD(Base):
+            # decorators are applied in reverse order 
+            decorators = (prepend_string('C'), prepend_string('D'))
+
+        class ABCD(AB, CD):
+            pass
+
+        self.assertEqual(Base.as_view()(self.rf.get('/')), "Base")
+        self.assertEqual(B.as_view()(self.rf.get('/')), "B:Base")
+        self.assertEqual(AB.as_view()(self.rf.get('/')), "A:B:Base")
+        self.assertEqual(CD.as_view()(self.rf.get('/')), "C:D:Base")
+        self.assertEqual(ABCD.as_view()(self.rf.get('/')), "A:B:C:D:Base")
 
 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, subclassing=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])