disallow adding a tween factory which is an instance without passing its globally importable name

6 files changed, 39 insertions, 53 deletions

diff --git a/TODO.txt b/TODO.txt
index 91bb528a8..d3609604d 100644
--- a/TODO.txt
+++ b/TODO.txt
@@ -13,8 +13,6 @@ Should-Have
 - Make it possible to use tween aliases in explicit tween config?  If not,
   the tween factories of all add-ons must be APIs.
 
-- Tween factories that are instances get a weird name.  Disallow?
-
 - BeforeRender event subclasses dict but implements a bunch of shit.  Its
   repr is currently broken (it always shows empty).  Decide what to do.
 
diff --git a/docs/narr/commandline.rst b/docs/narr/commandline.rst
index b1a646aec..97004d2b8 100644
--- a/docs/narr/commandline.rst
+++ b/docs/narr/commandline.rst
@@ -311,11 +311,9 @@ application request handler and the WSGI application which calls it.  A user
 can get a representation of both the implicit tween ordering (the ordering
 specified by calls to :meth:`pyramid.config.Configurator.add_tween`) and the
 explicit tween ordering (specified by the ``pyramid.tweens`` configuration
-setting) orderings using the ``paster ptweens`` command.  Handler factories
-which are functions or classes will show up as a standard Python dotted name
-in the ``paster ptweens`` output.  Tween factories which are *instances* will
-show their module and class name; the Python object id of the instance will
-be appended.
+setting) orderings using the ``paster ptweens`` command.  Tween factories
+will show up represented by their standard Python dotted name in the
+``paster ptweens`` output.
 
 For example, here's the ``paster pwteens`` command run against a system
 configured without any explicit tweens:
diff --git a/docs/narr/hooks.rst b/docs/narr/hooks.rst
index c8efc057c..97bee479b 100644
--- a/docs/narr/hooks.rst
+++ b/docs/narr/hooks.rst
@@ -847,12 +847,12 @@ running in a context in which they have access to the Pyramid
 :term:`application registry` as well as the Pyramid rendering machinery.
 
 To make use of tweens, you must construct a "tween factory".  A tween factory
-must be a 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 (if more
-than one tween is configured into the request handling chain).  ``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 (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.
 
 A tween is a callable which accepts a :term:`request` object and returns a
 two-tuple a :term:`response` object.
@@ -1089,11 +1089,12 @@ Tween Conflicts and Ordering Cycles
 Pyramid will prevent the same tween factory from being added to the tween
 chain more than once using configuration conflict detection.  If you wish to
 add the same tween factory more than once in a configuration, you should
-either: a) use a tween factory that is an instance rather than a function or
-class, b) use a function or class as a tween factory with the same logic as
-the other tween factory it conflicts with but with a different ``__name__``
-attribute or c) call :meth:`pyramid.config.Configurator.commit` between calls
-to :meth:`pyramid.config.Configurator.add_tween`.
+either: a) use a tween factory that is a separate globally importable
+instance object from the factory that it conflicts with b) use a function or
+class as a tween factory with the same logic as the other tween factory it
+conflicts with but with a different ``__name__`` attribute or c) call
+:meth:`pyramid.config.Configurator.commit` between calls to
+:meth:`pyramid.config.Configurator.add_tween`.
 
 If a cycle is detected in implicit tween ordering when ``over`` and ``under``
 are used in any call to "add_tween", an exception will be raised at startup
diff --git a/pyramid/config.py b/pyramid/config.py
index 920b70319..468b9441b 100644
--- a/pyramid/config.py
+++ b/pyramid/config.py
@@ -83,7 +83,6 @@ from pyramid.traversal import find_interface
 from pyramid.traversal import traversal_path
 from pyramid.tweens import excview_tween_factory
 from pyramid.tweens import Tweens
-from pyramid.tweens import tween_factory_name
 from pyramid.tweens import MAIN, INGRESS, EXCVIEW
 from pyramid.urldispatch import RoutesMapper
 from pyramid.util import DottedNameResolver
@@ -970,6 +969,10 @@ 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 ``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
@@ -1044,8 +1047,21 @@ class Configurator(object):
 
     def _add_tween(self, tween_factory, alias=None, under=None, over=None,
                    explicit=False):
-        tween_factory = self.maybe_dotted(tween_factory)
-        name = tween_factory_name(tween_factory)
+        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 alias in (MAIN, INGRESS):
             raise ConfigurationError('%s is a reserved tween name' % alias)
 
@@ -1060,7 +1076,7 @@ class Configurator(object):
         if tweens is None:
             tweens = Tweens()
             registry.registerUtility(tweens, ITweens)
-            tweens.add_implicit(tween_factory_name(excview_tween_factory),
+            tweens.add_implicit('pyramid.tweens.excview_tween_factory',
                                 excview_tween_factory, alias=EXCVIEW,
                                 over=MAIN)
         if explicit:
diff --git a/pyramid/tests/test_config.py b/pyramid/tests/test_config.py
index ec04f177b..7181d18b7 100644
--- a/pyramid/tests/test_config.py
+++ b/pyramid/tests/test_config.py
@@ -698,23 +698,11 @@ pyramid.tests.test_config.dummy_include2""",
                 ])
 
     def test_add_tween_instance(self):
-        from pyramid.interfaces import ITweens
-        from pyramid.tweens import excview_tween_factory
+        from pyramid.exceptions import ConfigurationError
         class ATween(object): pass
         atween = ATween()
         config = self._makeOne()
-        config.add_tween(atween)
-        config.commit()
-        tweens = config.registry.queryUtility(ITweens)
-        implicit = tweens.implicit()
-        self.assertEqual(len(implicit), 2)
-        self.assertTrue(
-          implicit[0][0].startswith(
-                'pyramid.tests.test_config.ATween.'))
-        self.assertEqual(implicit[0][1], atween)
-        self.assertEqual(
-            implicit[1],
-            ('pyramid.tweens.excview_tween_factory', excview_tween_factory))
+        self.assertRaises(ConfigurationError, config.add_tween, atween)
 
     def test_add_tween_unsuitable(self):
         from pyramid.exceptions import ConfigurationError
@@ -747,9 +735,8 @@ pyramid.tests.test_config.dummy_include2""",
 
     def test_add_tweens_conflict_same_alias(self):
         from zope.configuration.config import ConfigurationConflictError
-        class ATween(object): pass
-        atween1 = ATween()
-        atween2 = ATween()
+        def atween1(): pass
+        def atween2(): pass
         config = self._makeOne()
         config.add_tween(atween1, alias='a')
         config.add_tween(atween2, alias='a')
diff --git a/pyramid/tweens.py b/pyramid/tweens.py
index 73b96d375..b53942a36 100644
--- a/pyramid/tweens.py
+++ b/pyramid/tweens.py
@@ -181,20 +181,6 @@ class Tweens(object):
             handler = factory(handler, registry)
         return handler
     
-def tween_factory_name(factory):
-    if (hasattr(factory, '__name__') and hasattr(factory, '__module__')):
-        # function or class
-        name = '.'.join([factory.__module__, factory.__name__])
-    elif hasattr(factory, '__module__'):
-        # instance
-        name = '.'.join([factory.__module__, factory.__class__.__name__,
-                         str(id(factory))])
-    else:
-        raise ConfigurationError(
-            'A tween factory must be a class, an instance, or a function; '
-            '%s is not a suitable tween factory' % factory)
-    return name
-
 MAIN = 'MAIN'
 INGRESS = 'INGRESS'
 EXCVIEW = 'excview'