Merge pull request #2435 from mmerickel/feature/separate-viewderiver-module

separate viewderiver module

5 files changed, 206 insertions, 69 deletions

diff --git a/docs/api/viewderivers.rst b/docs/api/viewderivers.rst
new file mode 100644
index 000000000..2a141501e
--- /dev/null
+++ b/docs/api/viewderivers.rst
@@ -0,0 +1,17 @@
+.. _viewderivers_module:
+
+:mod:`pyramid.viewderivers`
+---------------------------
+
+.. automodule:: pyramid.viewderivers
+
+   .. attribute:: INGRESS
+
+      Constant representing the request ingress, for use in ``under``
+      arguments to :meth:`pyramid.config.Configurator.add_view_deriver`.
+
+   .. attribute:: VIEW
+
+      Constant representing the :term:`view callable` at the end of the view
+      pipeline, for use in ``over`` arguments to
+      :meth:`pyramid.config.Configurator.add_view_deriver`.
diff --git a/docs/narr/hooks.rst b/docs/narr/hooks.rst
index a32e94d1a..2c3782387 100644
--- a/docs/narr/hooks.rst
+++ b/docs/narr/hooks.rst
@@ -1580,12 +1580,6 @@ There are several built-in view derivers that :app:`Pyramid` will automatically
 apply to any view. Below they are defined in order from furthest to closest to
 the user-defined :term:`view callable`:
 
-``authdebug_view``
-
-  Used to output useful debugging information when
-  ``pyramid.debug_authorization`` is enabled. This element is a no-op
-  otherwise.
-
 ``secured_view``
 
   Enforce the ``permission`` defined on the view. This element is a no-op if no
@@ -1593,6 +1587,9 @@ the user-defined :term:`view callable`:
   default permission was assigned via
   :meth:`pyramid.config.Configurator.set_default_permission`.
 
+  This element will also output useful debugging information when
+  ``pyramid.debug_authorization`` is enabled.
+
 ``owrapped_view``
 
   Invokes the wrapped view defined by the ``wrapper`` option.
@@ -1620,6 +1617,14 @@ the user-defined :term:`view callable`:
   view pipeline interface to accept ``(context, request)`` from all previous
   view derivers.
 
+.. warning::
+
+   Any view derivers defined ``under`` the ``rendered_view`` are not
+   guaranteed to receive a valid response object. Rather they will receive the
+   result from the :term:`view mapper` which is likely the original response
+   returned from the view. This is possibly a dictionary for a renderer but it
+   may be any Python object that may be adapted into a response.
+
 Custom View Derivers
 ~~~~~~~~~~~~~~~~~~~~
 
@@ -1645,7 +1650,7 @@ view pipeline:
            response.headers['X-View-Performance'] = '%.3f' % (end - start,)
        return wrapper_view
 
-   config.add_view_deriver(timing_view, 'timing view')
+   config.add_view_deriver(timing_view)
 
 View derivers are unique in that they have access to most of the options
 passed to :meth:`pyramid.config.Configurator.add_view` in order to decide what
@@ -1671,7 +1676,7 @@ token unless ``disable_csrf=True`` is passed to the view:
 
    require_csrf_view.options = ('disable_csrf',)
 
-   config.add_view_deriver(require_csrf_view, 'require_csrf_view')
+   config.add_view_deriver(require_csrf_view)
 
    def protected_view(request):
        return Response('protected')
@@ -1694,13 +1699,19 @@ By default, every new view deriver is added between the ``decorated_view`` and
 ``rendered_view`` built-in derivers. It is possible to customize this ordering
 using the ``over`` and ``under`` options. Each option can use the names of
 other view derivers in order to specify an ordering. There should rarely be a
-reason to worry about the ordering of the derivers.
+reason to worry about the ordering of the derivers except when the deriver
+depends on other operations in the view pipeline.
 
 Both ``over`` and ``under`` may also be iterables of constraints. For either
 option, if one or more constraints was defined, at least one must be satisfied,
 else a :class:`pyramid.exceptions.ConfigurationError` will be raised. This may
 be used to define fallback constraints if another deriver is missing.
 
+Two sentinel values exist, :attr:`pyramid.viewderivers.INGRESS` and
+:attr:`pyramid.viewderivers.VIEW`, which may be used when specifying
+constraints at the edges of the view pipeline. For example, to add a deriver
+at the start of the pipeline you may use ``under=INGRESS``.
+
 It is not possible to add a view deriver under the ``mapped_view`` as the
 :term:`view mapper` is intimately tied to the signature of the user-defined
 :term:`view callable`. If you simply need to know what the original view
@@ -1710,8 +1721,12 @@ deriver.
 
 .. warning::
 
-   Any view derivers defined ``under`` the ``rendered_view`` are not
-   guaranteed to receive a valid response object. Rather they will receive the
-   result from the :term:`view mapper` which is likely the original response
-   returned from the view. This is possibly a dictionary for a renderer but it
-   may be any Python object that may be adapted into a response.
+   The default constraints for any view deriver are ``over='rendered_view'``
+   and ``under='decorated_view'``. When escaping these constraints you must
+   take care to avoid cyclic dependencies between derivers. For example, if
+   you want to add a new view deriver before ``secured_view`` then
+   simply specifying ``over='secured_view'`` is not enough, because the
+   default is also under ``decorated view`` there will be an unsatisfiable
+   cycle. You must specify a valid ``under`` constraint as well, such as
+   ``under=INGRESS`` to fall between INGRESS and ``secured_view`` at the
+   beginning of the view pipeline.
diff --git a/pyramid/config/views.py b/pyramid/config/views.py
index 1516743ad..3f6a9080d 100644
--- a/pyramid/config/views.py
+++ b/pyramid/config/views.py
@@ -76,9 +76,11 @@ from pyramid.util import (
     )
 
 import pyramid.config.predicates
-import pyramid.config.derivations
+import pyramid.viewderivers
 
-from pyramid.config.derivations import (
+from pyramid.viewderivers import (
+    INGRESS,
+    VIEW,
     preserve_view_attrs,
     view_description,
     requestonly,
@@ -89,6 +91,7 @@ from pyramid.config.derivations import (
 from pyramid.config.util import (
     DEFAULT_PHASH,
     MAX_ORDER,
+    as_sorted_tuple,
     )
 
 urljoin = urlparse.urljoin
@@ -1028,17 +1031,15 @@ class ViewsConfiguratorMixin(object):
             raise ConfigurationError('Unknown view options: %s' % (kw,))
 
     def _apply_view_derivers(self, info):
-        d = pyramid.config.derivations
-        # These derivations have fixed order
+        d = pyramid.viewderivers
+
+        # These derivers are not really derivers and so have fixed order
         outer_derivers = [('attr_wrapped_view', d.attr_wrapped_view),
                           ('predicated_view', d.predicated_view)]
-        inner_derivers = [('mapped_view', d.mapped_view)]
 
         view = info.original_view
         derivers = self.registry.getUtility(IViewDerivers)
-        for name, deriver in reversed(
-            outer_derivers + derivers.sorted() + inner_derivers
-        ):
+        for name, deriver in reversed(outer_derivers + derivers.sorted()):
             view = wraps_view(deriver)(view, info)
         return view
 
@@ -1090,7 +1091,7 @@ class ViewsConfiguratorMixin(object):
             self.add_view_predicate(name, factory)
 
     @action_method
-    def add_view_deriver(self, deriver, name, under=None, over=None):
+    def add_view_deriver(self, deriver, name=None, under=None, over=None):
         """
         .. versionadded:: 1.7
 
@@ -1105,16 +1106,21 @@ class ViewsConfiguratorMixin(object):
         restrictions on the name of a view deriver. If left unspecified, the
         name will be constructed from the name of the ``deriver``.
 
-        The ``under`` and ``over`` options may be used to control the ordering
+        The ``under`` and ``over`` options can be used to control the ordering
         of view derivers by providing hints about where in the view pipeline
-        the deriver is used.
+        the deriver is used. Each option may be a string or a list of strings.
+        At least one view deriver in each, the over and under directions, must
+        exist to fully satisfy the constraints.
 
         ``under`` means closer to the user-defined :term:`view callable`,
         and ``over`` means closer to view pipeline ingress.
 
-        Specifying neither ``under`` nor ``over`` is equivalent to specifying
-        ``over='rendered_view'`` and ``under='decorated_view'``, placing the
-        deriver somewhere between the ``decorated_view`` and ``rendered_view``
+        The default value for ``over`` is ``rendered_view`` and ``under`` is
+        ``decorated_view``. This places the deriver somewhere between the two
+        in the view pipeline. If the deriver should be placed elsewhere in the
+        pipeline, such as above ``decorated_view``, then you MUST also specify
+        ``under`` to something earlier in the order, or a
+        ``CyclicDependencyError`` will be raised when trying to sort the
         derivers.
 
         See :ref:`view_derivers` for more information.
@@ -1122,10 +1128,34 @@ class ViewsConfiguratorMixin(object):
         """
         deriver = self.maybe_dotted(deriver)
 
-        if under is None and over is None:
+        if name is None:
+            name = deriver.__name__
+
+        if name in (INGRESS, VIEW):
+            raise ConfigurationError('%s is a reserved view deriver name'
+                                     % name)
+
+        if under is None:
             under = 'decorated_view'
+
+        if over is None:
             over = 'rendered_view'
 
+        over = as_sorted_tuple(over)
+        under = as_sorted_tuple(under)
+
+        if INGRESS in over:
+            raise ConfigurationError('%s cannot be over INGRESS' % name)
+
+        # ensure everything is always over mapped_view
+        if VIEW in over and name != 'mapped_view':
+            over = as_sorted_tuple(over + ('mapped_view',))
+
+        if VIEW in under:
+            raise ConfigurationError('%s cannot be under VIEW' % name)
+        if 'mapped_view' in under:
+            raise ConfigurationError('%s cannot be under "mapped_view"' % name)
+
         discriminator = ('view deriver', name)
         intr = self.introspectable(
             'view derivers',
@@ -1139,34 +1169,37 @@ class ViewsConfiguratorMixin(object):
         def register():
             derivers = self.registry.queryUtility(IViewDerivers)
             if derivers is None:
-                derivers = TopologicalSorter()
+                derivers = TopologicalSorter(
+                    default_before=None,
+                    default_after=INGRESS,
+                    first=INGRESS,
+                    last=VIEW,
+                )
                 self.registry.registerUtility(derivers, IViewDerivers)
             derivers.add(name, deriver, before=over, after=under)
         self.action(discriminator, register, introspectables=(intr,),
                     order=PHASE1_CONFIG) # must be registered before add_view
 
     def add_default_view_derivers(self):
-        d = pyramid.config.derivations
+        d = pyramid.viewderivers
         derivers = [
-            ('authdebug_view', d.authdebug_view),
             ('secured_view', d.secured_view),
             ('owrapped_view', d.owrapped_view),
             ('http_cached_view', d.http_cached_view),
             ('decorated_view', d.decorated_view),
+            ('rendered_view', d.rendered_view),
+            ('mapped_view', d.mapped_view),
         ]
-        last = pyramid.util.FIRST
+        last = INGRESS
         for name, deriver in derivers:
-            self.add_view_deriver(deriver, name=name, under=last)
+            self.add_view_deriver(
+                deriver,
+                name=name,
+                under=last,
+                over=VIEW,
+            )
             last = name
 
-        # ensure rendered_view is over LAST
-        self.add_view_deriver(
-            d.rendered_view,
-            'rendered_view',
-            under=last,
-            over=pyramid.util.LAST,
-        )
-
     def derive_view(self, view, attr=None, renderer=None):
         """
         Create a :term:`view callable` using the function, instance,
diff --git a/pyramid/tests/test_config/test_derivations.py b/pyramid/tests/test_viewderivers.py
index d93b37f38..1823beb4d 100644
--- a/pyramid/tests/test_config/test_derivations.py
+++ b/pyramid/tests/test_viewderivers.py
@@ -46,7 +46,7 @@ class TestDeriveView(unittest.TestCase):
             self.assertEqual(
                 e.args[0],
                 'Could not convert return value of the view callable function '
-                'pyramid.tests.test_config.test_derivations.view into a response '
+                'pyramid.tests.test_viewderivers.view into a response '
                 'object. The value returned was None. You may have forgotten '
                 'to return a value from the view callable.'
                 )
@@ -64,7 +64,7 @@ class TestDeriveView(unittest.TestCase):
             self.assertEqual(
                 e.args[0],
                 "Could not convert return value of the view callable function "
-                "pyramid.tests.test_config.test_derivations.view into a response "
+                "pyramid.tests.test_viewderivers.view into a response "
                 "object. The value returned was {'a': 1}. You may have "
                 "forgotten to define a renderer in the view configuration."
                 )
@@ -84,7 +84,7 @@ class TestDeriveView(unittest.TestCase):
             msg = e.args[0]
             self.assertTrue(msg.startswith(
                 'Could not convert return value of the view callable object '
-                '<pyramid.tests.test_config.test_derivations.'))
+                '<pyramid.tests.test_viewderivers.'))
             self.assertTrue(msg.endswith(
                 '> into a response object. The value returned was None. You '
                 'may have forgotten to return a value from the view callable.'))
@@ -128,7 +128,7 @@ class TestDeriveView(unittest.TestCase):
                 e.args[0],
                 'Could not convert return value of the view callable '
                 'method __call__ of '
-                'class pyramid.tests.test_config.test_derivations.AView into a '
+                'class pyramid.tests.test_viewderivers.AView into a '
                 'response object. The value returned was None. You may have '
                 'forgotten to return a value from the view callable.'
                 )
@@ -151,7 +151,7 @@ class TestDeriveView(unittest.TestCase):
                 e.args[0],
                 'Could not convert return value of the view callable '
                 'method theviewmethod of '
-                'class pyramid.tests.test_config.test_derivations.AView into a '
+                'class pyramid.tests.test_viewderivers.AView into a '
                 'response object. The value returned was None. You may have '
                 'forgotten to return a value from the view callable.'
                 )
@@ -358,7 +358,7 @@ class TestDeriveView(unittest.TestCase):
         self.assertFalse(result is view)
         self.assertEqual(view.__module__, result.__module__)
         self.assertEqual(view.__doc__, result.__doc__)
-        self.assertTrue('test_derivations' in result.__name__)
+        self.assertTrue('test_viewderivers' in result.__name__)
         self.assertFalse(hasattr(result, '__call_permissive__'))
         self.assertEqual(result(None, None), response)
 
@@ -1103,14 +1103,13 @@ class TestDerivationOrder(unittest.TestCase):
         from pyramid.interfaces import IViewDerivers
 
         self.config.add_view_deriver(None, 'deriv1')
-        self.config.add_view_deriver(None, 'deriv2', over='deriv1')
-        self.config.add_view_deriver(None, 'deriv3', under='deriv2')
+        self.config.add_view_deriver(None, 'deriv2', 'decorated_view', 'deriv1')
+        self.config.add_view_deriver(None, 'deriv3', 'deriv2', 'deriv1')
 
         derivers = self.config.registry.getUtility(IViewDerivers)
         derivers_sorted = derivers.sorted()
         dlist = [d for (d, _) in derivers_sorted]
         self.assertEqual([
-            'authdebug_view',
             'secured_view',
             'owrapped_view',
             'http_cached_view',
@@ -1119,6 +1118,7 @@ class TestDerivationOrder(unittest.TestCase):
             'deriv3',
             'deriv1',
             'rendered_view',
+            'mapped_view',
             ], dlist)
 
     def test_right_order_implicit(self):
@@ -1132,7 +1132,6 @@ class TestDerivationOrder(unittest.TestCase):
         derivers_sorted = derivers.sorted()
         dlist = [d for (d, _) in derivers_sorted]
         self.assertEqual([
-            'authdebug_view',
             'secured_view',
             'owrapped_view',
             'http_cached_view',
@@ -1141,31 +1140,32 @@ class TestDerivationOrder(unittest.TestCase):
             'deriv2',
             'deriv1',
             'rendered_view',
+            'mapped_view',
             ], dlist)
 
     def test_right_order_under_rendered_view(self):
         from pyramid.interfaces import IViewDerivers
 
-        self.config.add_view_deriver(None, 'deriv1', under='rendered_view')
+        self.config.add_view_deriver(None, 'deriv1', 'rendered_view', 'mapped_view')
 
         derivers = self.config.registry.getUtility(IViewDerivers)
         derivers_sorted = derivers.sorted()
         dlist = [d for (d, _) in derivers_sorted]
         self.assertEqual([
-            'authdebug_view',
             'secured_view',
             'owrapped_view',
             'http_cached_view',
             'decorated_view',
             'rendered_view',
             'deriv1',
+            'mapped_view',
             ], dlist)
 
 
     def test_right_order_under_rendered_view_others(self):
         from pyramid.interfaces import IViewDerivers
 
-        self.config.add_view_deriver(None, 'deriv1', under='rendered_view')
+        self.config.add_view_deriver(None, 'deriv1', 'rendered_view', 'mapped_view')
         self.config.add_view_deriver(None, 'deriv2')
         self.config.add_view_deriver(None, 'deriv3')
 
@@ -1173,7 +1173,6 @@ class TestDerivationOrder(unittest.TestCase):
         derivers_sorted = derivers.sorted()
         dlist = [d for (d, _) in derivers_sorted]
         self.assertEqual([
-            'authdebug_view',
             'secured_view',
             'owrapped_view',
             'http_cached_view',
@@ -1182,6 +1181,7 @@ class TestDerivationOrder(unittest.TestCase):
             'deriv2',
             'rendered_view',
             'deriv1',
+            'mapped_view',
             ], dlist)
 
 
@@ -1218,11 +1218,11 @@ class TestAddDeriver(unittest.TestCase):
             def __init__(self):
                 self.response = DummyResponse()
 
-        def deriv1(view, value, **kw):
+        def deriv1(view, info):
             flags['deriv1'] = True
             return view
 
-        def deriv2(view, value, **kw):
+        def deriv2(view, info):
             flags['deriv2'] = True
             return view
 
@@ -1239,28 +1239,94 @@ class TestAddDeriver(unittest.TestCase):
         self.assertFalse(flags.get('deriv1'))
         self.assertTrue(flags.get('deriv2'))
 
+    def test_override_mapped_view(self):
+        from pyramid.viewderivers import VIEW
+        response = DummyResponse()
+        view = lambda *arg: response
+        flags = {}
+
+        def deriv1(view, info):
+            flags['deriv1'] = True
+            return view
+
+        result = self.config._derive_view(view)
+        self.assertFalse(flags.get('deriv1'))
+
+        flags.clear()
+        self.config.add_view_deriver(
+            deriv1, name='mapped_view', under='rendered_view', over=VIEW)
+        result = self.config._derive_view(view)
+        self.assertTrue(flags.get('deriv1'))
+
     def test_add_multi_derivers_ordered(self):
+        from pyramid.viewderivers import INGRESS
         response = DummyResponse()
         view = lambda *arg: response
         response.deriv = []
 
-        def deriv1(view, value, **kw):
+        def deriv1(view, info):
             response.deriv.append('deriv1')
             return view
 
-        def deriv2(view, value, **kw):
+        def deriv2(view, info):
             response.deriv.append('deriv2')
             return view
 
-        def deriv3(view, value, **kw):
+        def deriv3(view, info):
             response.deriv.append('deriv3')
             return view
 
         self.config.add_view_deriver(deriv1, 'deriv1')
-        self.config.add_view_deriver(deriv2, 'deriv2', under='deriv1')
-        self.config.add_view_deriver(deriv3, 'deriv3', over='deriv2')
+        self.config.add_view_deriver(deriv2, 'deriv2', INGRESS, 'deriv1')
+        self.config.add_view_deriver(deriv3, 'deriv3', 'deriv2', 'deriv1')
         result = self.config._derive_view(view)
-        self.assertEqual(response.deriv, ['deriv2', 'deriv3', 'deriv1'])
+        self.assertEqual(response.deriv, ['deriv1', 'deriv3', 'deriv2'])
+
+    def test_add_deriver_without_name(self):
+        from pyramid.interfaces import IViewDerivers
+        def deriv1(view, info): pass
+        self.config.add_view_deriver(deriv1)
+        derivers = self.config.registry.getUtility(IViewDerivers)
+        self.assertTrue('deriv1' in derivers.names)
+
+    def test_add_deriver_reserves_ingress(self):
+        from pyramid.exceptions import ConfigurationError
+        from pyramid.viewderivers import INGRESS
+        def deriv1(view, info): pass
+        self.assertRaises(
+            ConfigurationError, self.config.add_view_deriver, deriv1, INGRESS)
+
+    def test_add_deriver_enforces_ingress_is_first(self):
+        from pyramid.exceptions import ConfigurationError
+        from pyramid.viewderivers import INGRESS
+        def deriv1(view, info): pass
+        try:
+            self.config.add_view_deriver(deriv1, over=INGRESS)
+        except ConfigurationError as ex:
+            self.assertTrue('cannot be over INGRESS' in ex.args[0])
+        else: # pragma: no cover
+            raise AssertionError
+
+    def test_add_deriver_enforces_view_is_last(self):
+        from pyramid.exceptions import ConfigurationError
+        from pyramid.viewderivers import VIEW
+        def deriv1(view, info): pass
+        try:
+            self.config.add_view_deriver(deriv1, under=VIEW)
+        except ConfigurationError as ex:
+            self.assertTrue('cannot be under VIEW' in ex.args[0])
+        else: # pragma: no cover
+            raise AssertionError
+
+    def test_add_deriver_enforces_mapped_view_is_last(self):
+        from pyramid.exceptions import ConfigurationError
+        def deriv1(view, info): pass
+        try:
+            self.config.add_view_deriver(deriv1, 'deriv1', under='mapped_view')
+        except ConfigurationError as ex:
+            self.assertTrue('cannot be under "mapped_view"' in ex.args[0])
+        else: # pragma: no cover
+            raise AssertionError
 
 
 class TestDeriverIntegration(unittest.TestCase):
diff --git a/pyramid/config/derivations.py b/pyramid/viewderivers.py
index 99baf46f9..8061e5d4a 100644
--- a/pyramid/config/derivations.py
+++ b/pyramid/viewderivers.py
@@ -260,6 +260,13 @@ def http_cached_view(view, info):
 http_cached_view.options = ('http_cache',)
 
 def secured_view(view, info):
+    for wrapper in (_secured_view, _authdebug_view):
+        view = wraps_view(wrapper)(view, info)
+    return view
+
+secured_view.options = ('permission',)
+
+def _secured_view(view, info):
     permission = info.options.get('permission')
     if permission == NO_PERMISSION_REQUIRED:
         # allow views registered within configurations that have a
@@ -291,9 +298,7 @@ def secured_view(view, info):
 
     return wrapped_view
 
-secured_view.options = ('permission',)
-
-def authdebug_view(view, info):
+def _authdebug_view(view, info):
     wrapped_view = view
     settings = info.settings
     permission = info.options.get('permission')
@@ -330,8 +335,6 @@ def authdebug_view(view, info):
 
     return wrapped_view
 
-authdebug_view.options = ('permission',)
-
 def predicated_view(view, info):
     preds = info.predicates
     if not preds:
@@ -451,3 +454,6 @@ def decorated_view(view, info):
     return decorator(view)
 
 decorated_view.options = ('decorator',)
+
+VIEW = 'VIEW'
+INGRESS = 'INGRESS'