Merge branch 'feature.re-entrant-config'

7 files changed, 286 insertions, 13 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index 596e5f506..f2bedbcc9 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -4,6 +4,14 @@ Next release
 Features
 --------
 
+- The ``pyramid.config.Configurator`` has grown the ability to allow
+  actions to call other actions during a commit-cycle. This enables much more
+  logic to be placed into actions, such as the ability to invoke other actions
+  or group them for improved conflict detection. We have also exposed and
+  documented the config phases that Pyramid uses in order to further assist
+  in building conforming addons.
+  See https://github.com/Pylons/pyramid/pull/1513
+
 - Add ``pyramid.request.apply_request_extensions`` function which can be
   used in testing to apply any request extensions configured via
   ``config.add_request_method``. Previously it was only possible to test
diff --git a/docs/api/config.rst b/docs/api/config.rst
index 48dd2f0b9..ae913d32c 100644
--- a/docs/api/config.rst
+++ b/docs/api/config.rst
@@ -132,3 +132,8 @@
    are being used.
 
 .. autoclass:: not_
+
+.. attribute:: PHASE0_CONFIG
+.. attribute:: PHASE1_CONFIG
+.. attribute:: PHASE2_CONFIG
+.. attribute:: PHASE3_CONFIG
diff --git a/docs/narr/extconfig.rst b/docs/narr/extconfig.rst
index 6587aef92..a61eca7b7 100644
--- a/docs/narr/extconfig.rst
+++ b/docs/narr/extconfig.rst
@@ -215,13 +215,115 @@ registers an action with a higher order than the
 passed to it, that a route by this name was already registered by
 ``add_route``, and if such a route has not already been registered, it's a
 configuration error (a view that names a nonexistent route via its
-``route_name`` parameter will never be called).
+``route_name`` parameter will never be called). As of Pyramid 1.6 it is
+possible for one action to invoke another. See :ref:`ordering_actions` for
+more information.
 
 ``introspectables`` is a sequence of :term:`introspectable` objects.  You can
 pass a sequence of introspectables to the
 :meth:`~pyramid.config.Configurator.action` method, which allows you to
 augment Pyramid's configuration introspection system.
 
+.. _ordering_actions:
+
+Ordering Actions
+----------------
+
+In Pyramid every :term:`action` has an inherent ordering relative to other
+actions. The logic within actions is deferred until a call to
+:meth:`pyramid.config.Configurator.commit` (which is automatically invoked by
+:meth:`pyramid.config.Configurator.make_wsgi_app`). This means you may call
+``config.add_view(route_name='foo')`` **before**
+``config.add_route('foo', '/foo')`` because nothing actually happens until
+commit-time. During a commit cycle conflicts are resolved, actions are ordered
+and executed.
+
+By default, almost every action in Pyramid has an ``order`` of
+:const:`pyramid.config.PHASE3_CONFIG`. Every action within the same order-level
+will be executed in the order it was called.
+This means that if an action must be reliably executed before or after another
+action, the ``order`` must be defined explicitly to make this work. For
+example, views are dependent on routes being defined. Thus the action created
+by :meth:`pyramid.config.Configurator.add_route` has an ``order`` of
+:const:`pyramid.config.PHASE2_CONFIG`.
+
+Pre-defined Phases
+~~~~~~~~~~~~~~~~~~
+
+:const:`pyramid.config.PHASE0_CONFIG`
+
+- This phase is reserved for developers who want to execute actions prior
+  to Pyramid's core directives.
+
+:const:`pyramid.config.PHASE1_CONFIG`
+
+- :meth:`pyramid.config.Configurator.add_renderer`
+- :meth:`pyramid.config.Configurator.add_route_predicate`
+- :meth:`pyramid.config.Configurator.add_subscriber_predicate`
+- :meth:`pyramid.config.Configurator.add_view_predicate`
+- :meth:`pyramid.config.Configurator.set_authorization_policy`
+- :meth:`pyramid.config.Configurator.set_default_permission`
+- :meth:`pyramid.config.Configurator.set_view_mapper`
+
+:const:`pyramid.config.PHASE2_CONFIG`
+
+- :meth:`pyramid.config.Configurator.add_route`
+- :meth:`pyramid.config.Configurator.set_authentication_policy`
+
+:const:`pyramid.config.PHASE3_CONFIG`
+
+- The default for all builtin or custom directives unless otherwise specified.
+
+Calling Actions From Actions
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. versionadded:: 1.6
+
+Pyramid's configurator allows actions to be added during a commit-cycle as
+long as they are added to the current or a later ``order`` phase. This means
+that your custom action can defer decisions until commit-time and then do
+things like invoke :meth:`pyramid.config.Configurator.add_route`. It can also
+provide better conflict detection if your addon needs to call more than one
+other action.
+
+For example, let's make an addon that invokes ``add_route`` and ``add_view``,
+but we want it to conflict with any other call to our addon:
+
+.. code-block:: python
+   :linenos:
+
+   from pyramid.config import PHASE0_CONFIG
+
+   def includeme(config):
+       config.add_directive('add_auto_route', add_auto_route)
+
+   def add_auto_route(config, name, view):
+       def register():
+           config.add_view(route_name=name, view=view)
+           config.add_route(name, '/' + name)
+       config.action(('auto route', name), register, order=PHASE0_CONFIG)
+
+Now someone else can use your addon and be informed if there is a conflict
+between this route and another, or two calls to ``add_auto_route``.
+Notice how we had to invoke our action **before** ``add_view`` or
+``add_route``. If we tried to invoke this afterward, the subsequent calls to
+``add_view`` and ``add_route`` would cause conflicts because that phase had
+already been executed, and the configurator cannot go back in time to add more
+views during that commit-cycle.
+
+.. code-block:: python
+   :linenos:
+
+   from pyramid.config import Configurator
+
+   def main(global_config, **settings):
+       config = Configurator()
+       config.include('auto_route_addon')
+       config.add_auto_route('foo', my_view)
+
+   def my_view(request):
+       return request.response
+
 .. _introspection:
 
 Adding Configuration Introspection
diff --git a/pyramid/compat.py b/pyramid/compat.py
index 3aa191968..a12790d82 100644
--- a/pyramid/compat.py
+++ b/pyramid/compat.py
@@ -258,6 +258,16 @@ else:
 def is_bound_method(ob):
     return inspect.ismethod(ob) and getattr(ob, im_self, None) is not None
 
+# support annotations and keyword-only arguments in PY3
+if PY3: # pragma: no cover
+    from inspect import getfullargspec as getargspec
+else:
+    from inspect import getargspec
+
+if PY3: # pragma: no cover
+    from itertools import zip_longest
+else:
+    from itertools import izip_longest as zip_longest
 
 def is_unbound_method(fn):
     """
diff --git a/pyramid/config/__init__.py b/pyramid/config/__init__.py
index 2ab654b9a..401def208 100644
--- a/pyramid/config/__init__.py
+++ b/pyramid/config/__init__.py
@@ -12,7 +12,10 @@ from pyramid.interfaces import (
     IDebugLogger,
     IExceptionResponse,
     IPredicateList,
+    PHASE0_CONFIG,
     PHASE1_CONFIG,
+    PHASE2_CONFIG,
+    PHASE3_CONFIG,
     )
 
 from pyramid.asset import resolve_asset_spec
@@ -23,6 +26,7 @@ from pyramid.compat import (
     text_,
     reraise,
     string_types,
+    zip_longest,
     )
 
 from pyramid.events import ApplicationCreated
@@ -54,7 +58,9 @@ from pyramid.settings import aslist
 from pyramid.threadlocal import manager
 
 from pyramid.util import (
+    ActionInfo,
     WeakOrderedSet,
+    action_method,
     object_description,
     )
 
@@ -74,11 +80,6 @@ from pyramid.config.zca import ZCAConfiguratorMixin
 
 from pyramid.path import DottedNameResolver
 
-from pyramid.util import (
-    action_method,
-    ActionInfo,
-    )
-
 empty = text_('')
 _marker = object()
 
@@ -86,6 +87,10 @@ ConfigurationError = ConfigurationError # pyflakes
 
 not_ = not_ # pyflakes, this is an API
 
+PHASE0_CONFIG = PHASE0_CONFIG  # api
+PHASE1_CONFIG = PHASE1_CONFIG  # api
+PHASE2_CONFIG = PHASE2_CONFIG  # api
+PHASE3_CONFIG = PHASE3_CONFIG  # api
 
 class Configurator(
     TestingConfiguratorMixin,
@@ -987,7 +992,7 @@ class Configurator(
 class ActionState(object):
     def __init__(self):
         # NB "actions" is an API, dep'd upon by pyramid_zcml's load_zcml func
-        self.actions = [] 
+        self.actions = []
         self._seen_files = set()
 
     def processSpec(self, spec):
@@ -1071,10 +1076,82 @@ class ActionState(object):
         >>> output
         [('f', (1,), {}), ('f', (2,), {})]
 
-        """
+        The execution is re-entrant such that actions may be added by other
+        actions with the one caveat that the order of any added actions must
+        be equal to or larger than the current action.
 
+        >>> output = []
+        >>> def f(*a, **k):
+        ...   output.append(('f', a, k))
+        ...   context.actions.append((3, g, (8,), {}))
+        >>> def g(*a, **k):
+        ...    output.append(('g', a, k))
+        >>> context.actions = [
+        ...   (1, f, (1,)),
+        ...   ]
+        >>> context.execute_actions()
+        >>> output
+        [('f', (1,), {}), ('g', (8,), {})]
+
+        """
         try:
-            for action in resolveConflicts(self.actions):
+            all_actions = []
+            executed_actions = []
+            pending_actions = iter([])
+
+            # resolve the new action list against what we have already
+            # executed -- if a new action appears intertwined in the list
+            # of already-executed actions then someone wrote a broken
+            # re-entrant action because it scheduled the action *after* it
+            # should have been executed (as defined by the action order)
+            def resume(actions):
+                for a, b in zip_longest(actions, executed_actions):
+                    if b is None and a is not None:
+                        # common case is that we are executing every action
+                        yield a
+                    elif b is not None and a != b:
+                        raise ConfigurationError(
+                            'During execution a re-entrant action was added '
+                            'that modified the planned execution order in a '
+                            'way that is incompatible with what has already '
+                            'been executed.')
+                    else:
+                        # resolved action is in the same location as before,
+                        # so we are in good shape, but the action is already
+                        # executed so we skip it
+                        assert b is not None and a == b
+
+            while True:
+                # We clear the actions list prior to execution so if there
+                # are some new actions then we add them to the mix and resolve
+                # conflicts again. This orders the new actions as well as
+                # ensures that the previously executed actions have no new
+                # conflicts.
+                if self.actions:
+                    # Only resolve the new actions against executed_actions
+                    # and pending_actions instead of everything to avoid
+                    # redundant checks.
+                    # Assume ``actions = resolveConflicts([A, B, C])`` which
+                    # after conflict checks, resulted in ``actions == [A]``
+                    # then we know action A won out or a conflict would have
+                    # been raised. Thus, when action D is added later, we only
+                    # need to check the new action against A.
+                    # ``actions = resolveConflicts([A, D]) should drop the
+                    # number of redundant checks down from O(n^2) closer to
+                    # O(n lg n).
+                    all_actions.extend(self.actions)
+                    pending_actions = resume(resolveConflicts(
+                        executed_actions
+                        + list(pending_actions)
+                        + self.actions
+                    ))
+                    self.actions = []
+
+                action = next(pending_actions, None)
+                if action is None:
+                    # we are done!
+                    break
+
                 callable = action['callable']
                 args = action['args']
                 kw = action['kw']
@@ -1095,15 +1172,19 @@ class ActionState(object):
                                 ConfigurationExecutionError(t, v, info),
                                 tb)
                     finally:
-                       del t, v, tb
+                        del t, v, tb
 
                 if introspector is not None:
                     for introspectable in introspectables:
                         introspectable.register(introspector, info)
-                
+
+                executed_actions.append(action)
+
         finally:
             if clear:
                 del self.actions[:]
+            else:
+                self.actions = all_actions
 
 # this function is licensed under the ZPL (stolen from Zope)
 def resolveConflicts(actions):
@@ -1224,4 +1305,3 @@ def expand_action(discriminator, callable=None, args=(), kw=None,
         )
 
 global_registries = WeakOrderedSet()
-
diff --git a/pyramid/interfaces.py b/pyramid/interfaces.py
index 1508f282e..4c171f9cc 100644
--- a/pyramid/interfaces.py
+++ b/pyramid/interfaces.py
@@ -1228,6 +1228,7 @@ class ICacheBuster(Interface):
 # with this phase will be executed earlier than those with later phase
 # numbers.  The default phase number is 0, FTR.
 
+PHASE0_CONFIG = -30
 PHASE1_CONFIG = -20
 PHASE2_CONFIG = -10
-
+PHASE3_CONFIG = 0
diff --git a/pyramid/tests/test_config/test_init.py b/pyramid/tests/test_config/test_init.py
index aeebe3c91..0ed04eb06 100644
--- a/pyramid/tests/test_config/test_init.py
+++ b/pyramid/tests/test_config/test_init.py
@@ -1515,6 +1515,73 @@ class TestActionState(unittest.TestCase):
         self.assertRaises(ConfigurationExecutionError, c.execute_actions)
         self.assertEqual(output, [('f', (1,), {}), ('f', (2,), {})])
 
+    def test_reentrant_action(self):
+        output = []
+        c = self._makeOne()
+        def f(*a, **k):
+            output.append(('f', a, k))
+            c.actions.append((3, g, (8,), {}))
+        def g(*a, **k):
+            output.append(('g', a, k))
+        c.actions = [
+            (1, f, (1,)),
+        ]
+        c.execute_actions()
+        self.assertEqual(output, [('f', (1,), {}), ('g', (8,), {})])
+
+    def test_reentrant_action_error(self):
+        from pyramid.exceptions import ConfigurationError
+        c = self._makeOne()
+        def f(*a, **k):
+            c.actions.append((3, g, (8,), {}, (), None, -1))
+        def g(*a, **k): pass
+        c.actions = [
+            (1, f, (1,)),
+        ]
+        self.assertRaises(ConfigurationError, c.execute_actions)
+
+    def test_reentrant_action_without_clear(self):
+        c = self._makeOne()
+        def f(*a, **k):
+            c.actions.append((3, g, (8,)))
+        def g(*a, **k): pass
+        c.actions = [
+            (1, f, (1,)),
+        ]
+        c.execute_actions(clear=False)
+        self.assertEqual(c.actions, [
+            (1, f, (1,)),
+            (3, g, (8,)),
+        ])
+
+class Test_reentrant_action_functional(unittest.TestCase):
+    def _makeConfigurator(self, *arg, **kw):
+        from pyramid.config import Configurator
+        config = Configurator(*arg, **kw)
+        return config
+
+    def test_functional(self):
+        def add_auto_route(config, name, view):
+               def register():
+                   config.add_view(route_name=name, view=view)
+                   config.add_route(name, '/' + name)
+               config.action(
+                   ('auto route', name), register, order=-30
+                   )
+        config = self._makeConfigurator()
+        config.add_directive('add_auto_route', add_auto_route)
+        def my_view(request): return request.response
+        config.add_auto_route('foo', my_view)
+        config.commit()
+        from pyramid.interfaces import IRoutesMapper
+        mapper = config.registry.getUtility(IRoutesMapper)
+        routes = mapper.get_routes()
+        route = routes[0]
+        self.assertEqual(len(routes), 1)
+        self.assertEqual(route.name, 'foo')
+        self.assertEqual(route.path, '/foo')
+
+
 class Test_resolveConflicts(unittest.TestCase):
     def _callFUT(self, actions):
         from pyramid.config import resolveConflicts