Require that tween_factory argument to add_tween be a dotted name.

3 files changed, 136 insertions, 59 deletions

diff --git a/docs/narr/hooks.rst b/docs/narr/hooks.rst
index 97bee479b..50758f327 100644
--- a/docs/narr/hooks.rst
+++ b/docs/narr/hooks.rst
@@ -846,25 +846,26 @@ behave a bit like :term:`WSGI` middleware but they have the benefit of
 running in a context in which they have access to the Pyramid
 :term:`application registry` as well as the Pyramid rendering machinery.
 
+Creating a Tween Factory
+~~~~~~~~~~~~~~~~~~~~~~~~
+
 To make use of tweens, you must construct a "tween factory".  A tween factory
-must be a globally importable callable (or a :term:`dotted Python name` to
-such a callable) which accepts two arguments: ``handler`` and ``registry``.
-``handler`` will be the either the main Pyramid request handling function or
-another tween.  ``registry`` will be the Pyramid :term:`application registry`
-represented by this Configurator.  A tween factory must return a tween when
-it is called.
+must be a globally importable callable which accepts two arguments:
+``handler`` and ``registry``.  ``handler`` will be the either the main
+Pyramid request handling function or another tween.  ``registry`` will be the
+Pyramid :term:`application registry` represented by this Configurator.  A
+tween factory must return a tween when it is called.
 
 A tween is a callable which accepts a :term:`request` object and returns a
 two-tuple a :term:`response` object.
 
-Once you've created a tween factory, you can register it into the implicit
-tween chain using the :meth:`pyramid.config.Configurator.add_tween` method.
-
-Here's an example creating a tween factory and registering it:
+Here's an example of a tween factory:
 
 .. code-block:: python
    :linenos:
 
+    # in a module named myapp.tweens
+
     import time
     from pyramid.settings import asbool
     import logging
@@ -888,13 +889,58 @@ Here's an example creating a tween factory and registering it:
         # handler
         return handler
 
-    from pyramid.config import Configurator
+If you remember, a tween is an object which accepts a :term:`request` object
+and which returns a :term:`response` argument.  The ``request`` argument to a
+tween will be the request created by Pyramid's router when it receives a WSGI
+request.  The response object will be generated by the downstream Pyramid
+application and it should be returned by the tween.
+
+In the above example, the tween factory defines a ``timing_tween`` tween and
+returns it if ``asbool(registry.settings.get('do_timing'))`` is true.  It
+otherwise simply returns the handler it was given.  The ``registry.settings``
+attribute is a handle to the deployment settings provided by the user
+(usually in an ``.ini`` file).  In this case, if the user has defined a
+``do_timing`` setting, and that setting is ``True``, the user has said she
+wants to do timing, so the tween factory returns the timing tween; it
+otherwise just returns the handler it has been provided, preventing any
+timing.
+
+The example timing tween simply records the start time, calls the downstream
+handler, logs the number of seconds consumed by the downstream handler, and
+returns the response.
+
+Registering an Implicit Tween Factory
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-    config = Configurator()
-    config.add_tween(timing_tween_factory)
+Once you've created a tween factory, you can register it into the implicit
+tween chain using the :meth:`pyramid.config.Configurator.add_tween` method
+using its :term:`dotted Python name`.
 
-The ``request`` argument to a tween will be the request created by Pyramid's
-router when it receives a WSGI request.
+Here's an example of registering the a tween factory as an "implicit"
+tween in a Pyramid application:
+
+.. code-block:: python
+   :linenos:
+
+    from pyramid.config import Configurator
+    config = Configurator()
+    config.add_tween('myapp.tweens.timing_tween_factory')
+
+Note that you must use a :term:`dotted Python name` as the first argument to
+:meth:`pyramid.config.Configurator.add_tween`; this must point at a tween
+factory.  You cannot pass the tween factory object itself to the method: it
+must be a globally importable object.  In the above example, we assume that a
+``timing_tween_factory`` tween factory was defined in a module named
+``myapp.tweens``, so the tween factory is importable as
+``myapp.tweens.timing_tween_factory``.
+
+When you use :meth:`pyramid.config.Configurator.add_tween`, you're
+instructing the system to use your tween factory at startup time unless the
+user has provided an explicit tween list in his configuration.  This is
+what's meant by an "implicit" tween.  A user can always elect to supply an
+explicit tween list, reordering or disincluding implicitly added tweens.  See
+:ref:`explicit_tween_ordering` for more information about explicit tween
+ordering.
 
 If more than one call to :meth:`pyramid.config.Configurator.add_tween` is
 made within a single application configuration, the tweens will be chained
@@ -925,6 +971,9 @@ this::
     pyramid.tweens.excview_tween_factory (implicit)
     MAIN (implicit)
 
+Suggesting Implicit Tween Ordering
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
 By default, as described above, the ordering of the chain is controlled
 entirely by the relative ordering of calls to
 :meth:`pyramid.config.Configurator.add_tween`.  However, the caller of
diff --git a/pyramid/config.py b/pyramid/config.py
index 468b9441b..c68648a5f 100644
--- a/pyramid/config.py
+++ b/pyramid/config.py
@@ -956,8 +956,8 @@ class Configurator(object):
         Add a 'tween factory'.  A :term:`tween` (a contraction of 'between')
         is a bit of code that sits between the Pyramid router's main request
         handling function and the upstream WSGI component that uses
-        :app:`Pyramid` as its 'app'.  This is a feature that may be used by
-        Pyramid framework extensions, to provide, for example,
+        :app:`Pyramid` as its 'app'.  Tweens are a feature that may be used
+        by Pyramid framework extensions, to provide, for example,
         Pyramid-specific view timing support, bookkeeping code that examines
         exceptions before they are returned to the upstream WSGI application,
         or a variety of other features.  Tweens behave a bit like
@@ -969,14 +969,13 @@ class Configurator(object):
                   Pyramid application by using the ``paster ptweens``
                   command.  See :ref:`displaying_tweens`.
 
-        The ``tween_factory`` argument must be a globally importable function
-        or class or a :term:`dotted Python name` to a global object
-        representing the tween factory.
+        The ``tween_factory`` argument must be a :term:`dotted Python name`
+        to a global object representing the tween factory.
 
         The ``alias`` argument, if it is not ``None``, should be a string.
         The string will represent a value that other callers of ``add_tween``
-        may pass as an ``under`` and ``over`` argument instead of a dotted
-        name to a tween factory.
+        may pass as an ``under`` and ``over`` argument instead of this
+        tween's factory name.
 
         The ``under`` and ``over`` arguments allow the caller of
         ``add_tween`` to provide a hint about where in the tween chain this
@@ -988,8 +987,8 @@ class Configurator(object):
         - ``None`` (the default).
 
         - A :term:`dotted Python name` to a tween factory: a string
-          representing the predicted dotted name of a tween factory added in
-          a call to ``add_tween`` in the same configuration session.
+          representing the dotted name of a tween factory added in a call to
+          ``add_tween`` in the same configuration session.
 
         - A tween alias: a string representing the predicted value of
           ``alias`` in a separate call to ``add_tween`` in the same
@@ -1005,14 +1004,15 @@ class Configurator(object):
         ``under`` means 'closer to the main Pyramid application than',
         ``over`` means 'closer to the request ingress than'.
 
-        For example, calling ``add_tween(factory, over=pyramid.tweens.MAIN)``
-        will attempt to place the tween factory represented by ``factory``
-        directly 'above' (in ``paster ptweens`` order) the main Pyramid
-        request handler.  Likewise, calling ``add_tween(factory,
+        For example, calling ``add_tween('myapp.tfactory',
+        over=pyramid.tweens.MAIN)`` will attempt to place the tween factory
+        represented by the dotted name ``myapp.tfactory`` directly 'above' (in
+        ``paster ptweens`` order) the main Pyramid request handler.
+        Likewise, calling ``add_tween('myapp.tfactory',
         over=pyramid.tweens.MAIN, under='someothertween')`` will attempt to
         place this tween factory 'above' the main handler but 'below' (a
         fictional) 'someothertween' tween factory (which was presumably added
-        via ``add_tween(factory, alias='someothertween')``).
+        via ``add_tween('myapp.tfactory', alias='someothertween')``).
 
         If all options for ``under`` (or ``over``) cannot be found in the
         current configuration, it is an error. If some options are specified
@@ -1047,20 +1047,27 @@ class Configurator(object):
 
     def _add_tween(self, tween_factory, alias=None, under=None, over=None,
                    explicit=False):
-        if isinstance(tween_factory, basestring):
-            name = tween_factory
-            tween_factory = self.maybe_dotted(tween_factory)
-        else:
-            if (hasattr(tween_factory, '__name__') and
-                hasattr(tween_factory, '__module__')):
-                name = '.'.join([tween_factory.__module__,
-                                 tween_factory.__name__])
-            else:
-                raise ConfigurationError(
-                    'If it is provided as an object, a tween factory must be a '
-                    'globally importable object; %s is not a suitable tween '
-                    'factory (maybe pass tween_factory as a dotted name '
-                    'string to your instance instead)' % tween_factory)
+
+        if not isinstance(tween_factory, basestring):
+            raise ConfigurationError(
+                'The "tween_factory" argument to add_tween must be a '
+                'dotted name to a globally importable object, not %r' %
+                tween_factory)
+
+        name = tween_factory
+        tween_factory = self.maybe_dotted(tween_factory)
+
+        def is_string_or_iterable(v):
+            if isinstance(v, basestring):
+                return True
+            if hasattr(v, '__iter__'):
+                return True
+
+        for t, p in [('over', over), ('under', under)]:
+            if p is not None:
+                if not is_string_or_iterable(p):
+                    raise ConfigurationError(
+                        '"%s" must be a string or iterable, not %s' % (t, p))
 
         if alias in (MAIN, INGRESS):
             raise ConfigurationError('%s is a reserved tween name' % alias)
diff --git a/pyramid/tests/test_config.py b/pyramid/tests/test_config.py
index 7181d18b7..921f5ec84 100644
--- a/pyramid/tests/test_config.py
+++ b/pyramid/tests/test_config.py
@@ -647,17 +647,20 @@ pyramid.tests.test_config.dummy_include2""",
         def factory1(handler, registry): return handler
         def factory2(handler, registry): return handler
         config = self._makeOne()
-        config.add_tween(factory1)
-        config.add_tween(factory2)
+        config.add_tween('pyramid.tests.test_config.dummy_tween_factory')
+        config.add_tween('pyramid.tests.test_config.dummy_tween_factory2')
         config.commit()
         tweens = config.registry.queryUtility(ITweens)
         implicit = tweens.implicit()
         self.assertEqual(
             implicit,
             [
-                ('pyramid.tests.test_config.factory2', factory2),
-                ('pyramid.tests.test_config.factory1', factory1),
-                ('pyramid.tweens.excview_tween_factory', excview_tween_factory),
+                ('pyramid.tests.test_config.dummy_tween_factory2',
+                 dummy_tween_factory2),
+                ('pyramid.tests.test_config.dummy_tween_factory',
+                 dummy_tween_factory),
+                ('pyramid.tweens.excview_tween_factory',
+                 excview_tween_factory),
                 ]
             )
 
@@ -665,12 +668,12 @@ pyramid.tests.test_config.dummy_include2""",
         from pyramid.interfaces import ITweens
         from pyramid.tweens import excview_tween_factory
         from pyramid.tweens import MAIN
-        def factory1(handler, registry): return handler
-        def factory2(handler, registry): return handler
         config = self._makeOne()
-        config.add_tween(factory1, over=MAIN)
-        config.add_tween(factory2, over=MAIN,
-                         under='pyramid.tests.test_config.factory1')
+        config.add_tween('pyramid.tests.test_config.dummy_tween_factory',
+                         over=MAIN)
+        config.add_tween('pyramid.tests.test_config.dummy_tween_factory2',
+                         over=MAIN,
+                         under='pyramid.tests.test_config.dummy_tween_factory')
         config.commit()
         tweens = config.registry.queryUtility(ITweens)
         implicit = tweens.implicit()
@@ -678,10 +681,26 @@ pyramid.tests.test_config.dummy_include2""",
             implicit,
             [
                 ('pyramid.tweens.excview_tween_factory', excview_tween_factory),
-                ('pyramid.tests.test_config.factory1', factory1),
-                ('pyramid.tests.test_config.factory2', factory2),
+                ('pyramid.tests.test_config.dummy_tween_factory',
+                 dummy_tween_factory),
+                ('pyramid.tests.test_config.dummy_tween_factory2',
+                 dummy_tween_factory2),
              ])
 
+    def test_add_tweens_names_with_under_nonstringoriter(self):
+        from pyramid.exceptions import ConfigurationError
+        config = self._makeOne()
+        self.assertRaises(ConfigurationError, config.add_tween,
+                          'pyramid.tests.test_config.dummy_tween_factory',
+                          under=False)
+
+    def test_add_tweens_names_with_over_nonstringoriter(self):
+        from pyramid.exceptions import ConfigurationError
+        config = self._makeOne()
+        self.assertRaises(ConfigurationError, config.add_tween,
+                          'pyramid.tests.test_config.dummy_tween_factory',
+                          over=False)
+
     def test_add_tween_dottedname(self):
         from pyramid.interfaces import ITweens
         from pyramid.tweens import excview_tween_factory
@@ -735,11 +754,11 @@ pyramid.tests.test_config.dummy_include2""",
 
     def test_add_tweens_conflict_same_alias(self):
         from zope.configuration.config import ConfigurationConflictError
-        def atween1(): pass
-        def atween2(): pass
         config = self._makeOne()
-        config.add_tween(atween1, alias='a')
-        config.add_tween(atween2, alias='a')
+        config.add_tween('pyramid.tests.test_config.dummy_tween_factory',
+                         alias='a')
+        config.add_tween('pyramid.tests.test_config.dummy_tween_factory2',
+                         alias='a')
         self.assertRaises(ConfigurationConflictError, config.commit)
 
     def test_add_tween_over_ingress(self):
@@ -5805,3 +5824,5 @@ class DummyResponse(object):
     implements(IResponse)
     
 def dummy_tween_factory(handler, registry): pass
+
+def dummy_tween_factory2(handler, registry): pass