better conflict error reporting

2 files changed, 51 insertions, 52 deletions

diff --git a/pyramid/configuration.py b/pyramid/configuration.py
index a6b712d99..810f1775c 100644
--- a/pyramid/configuration.py
+++ b/pyramid/configuration.py
@@ -103,9 +103,25 @@ if chameleon_text:
 if chameleon_zpt:
     DEFAULT_RENDERERS += (('.txt', chameleon_text.renderer_factory),)
 
-def maybe_autocommit(wrapped):
+def config_method(wrapped):
     def wrapper(self, *arg, **kw):
+        context = self._ctx
+        if not context.autocommit:
+            if not context.info:
+                # try to provide more accurate info for conflict reports by
+                # wrapping the context in a decorator and attaching caller info
+                # to it, unless the context already has info (if it already has
+                # info, it's likely a context generated by a ZCML directive).
+                newctx = GroupingContextDecorator(context)
+                try:
+                    f = traceback.extract_stack(limit=3)
+                    info = f[-2]
+                except:
+                    info = ''
+                newctx.info = info
+                self._ctx = newctx
         result = wrapped(self, *arg, **kw)
+        self._ctx = context
         if self._ctx.autocommit:
             self.commit()
         return result
@@ -278,7 +294,7 @@ class Configurator(object):
         self.registry.settings = settings
         return settings
 
-    @maybe_autocommit
+    @config_method
     def _set_root_factory(self, factory):
         """ Add a :term:`root factory` to the current configuration
         state.  If the ``factory`` argument is ``None`` a default root
@@ -291,7 +307,7 @@ class Configurator(object):
             self.registry.registerUtility(factory, IDefaultRootFactory) # b/c
         self._action(IRootFactory, register)
 
-    @maybe_autocommit
+    @config_method
     def _set_authentication_policy(self, policy):
         """ Add a :app:`Pyramid` :term:`authentication policy` to
         the current configuration."""
@@ -299,7 +315,7 @@ class Configurator(object):
         self.registry.registerUtility(policy, IAuthenticationPolicy)
         self._action(IAuthenticationPolicy)
 
-    @maybe_autocommit
+    @config_method
     def _set_authorization_policy(self, policy):
         """ Add a :app:`Pyramid` :term:`authorization policy` to
         the current configuration state (also accepts a :term:`dotted
@@ -393,10 +409,13 @@ class Configurator(object):
     def commit(self):
         """ Commit pending configuration actions. """
         self._ctx.execute_actions()
+        # reset the context
         self._ctx = self._make_context(self._ctx.autocommit)
 
     @classmethod
     def with_context(cls, context):
+        """ Used by ZCML directives to obtain a configurator with 'the right'
+        context """
         configurator = cls(registry=context.registry, package=context.package)
         configurator._ctx = context
         return configurator
@@ -610,7 +629,7 @@ class Configurator(object):
             renderer = {'name':renderer, 'package':self.package}
         return self._derive_view(view, attr=attr, renderer=renderer)
 
-    @maybe_autocommit
+    @config_method
     def add_subscriber(self, subscriber, iface=None):
         """Add an event :term:`subscriber` for the event stream
         implied by the supplied ``iface`` interface.  The
@@ -754,7 +773,8 @@ class Configurator(object):
                 context.basepath = os.path.dirname(filename)
                 context.includepath = _context.includepath + (spec,)
                 context.package = package_of(module)
-                func(Configurator.with_context(context))
+                config = Configurator.with_context(context)
+                func(config)
 
     def add_handler(self, route_name, pattern, handler, action=None, **kw):
 
@@ -869,7 +889,7 @@ class Configurator(object):
 
         return route
 
-    @maybe_autocommit
+    @config_method
     def add_view(self, view=None, name="", for_=None, permission=None,
                  request_type=None, route_name=None, request_method=None,
                  request_param=None, containment=None, attr=None,
@@ -1297,7 +1317,7 @@ class Configurator(object):
         discriminator = tuple(discriminator)
         self._action(discriminator, register)
 
-    @maybe_autocommit
+    @config_method
     def add_route(self,
                   name,
                   pattern=None,
@@ -1694,7 +1714,7 @@ class Configurator(object):
         scanner = self.venusian.Scanner(config=self)
         scanner.scan(package, categories=categories)
 
-    @maybe_autocommit
+    @config_method
     def add_renderer(self, name, factory):
         """
         Add a :app:`Pyramid` :term:`renderer` factory to the
@@ -1726,7 +1746,7 @@ class Configurator(object):
         self.registry.registerUtility(factory, IRendererFactory, name=name)
         self._action((IRendererFactory, name), None)
 
-    @maybe_autocommit
+    @config_method
     def override_resource(self, to_override, override_with, _override=None):
         """ Add a :app:`Pyramid` resource override to the current
         configuration state.
@@ -1848,7 +1868,7 @@ class Configurator(object):
             return view(context, request)
         return self.add_view(bwcompat_view, context=NotFound, wrapper=wrapper)
 
-    @maybe_autocommit
+    @config_method
     def set_request_factory(self, factory):
         """ The object passed as ``factory`` should be an object (or a
         :term:`dotted Python name` which refers to an object) which
@@ -1867,7 +1887,7 @@ class Configurator(object):
             self.registry.registerUtility(factory, IRequestFactory)
         self._action(IRequestFactory, register)
 
-    @maybe_autocommit
+    @config_method
     def set_renderer_globals_factory(self, factory):
         """ The object passed as ``factory`` should be an callable (or
         a :term:`dotted Python name` which refers to an callable) that
@@ -1891,7 +1911,7 @@ class Configurator(object):
             self.registry.registerUtility(factory, IRendererGlobalsFactory)
         self._action(IRendererGlobalsFactory, register)
 
-    @maybe_autocommit
+    @config_method
     def set_locale_negotiator(self, negotiator):
         """
         Set the :term:`locale negotiator` for this application.  The
@@ -1915,7 +1935,7 @@ class Configurator(object):
             self.registry.registerUtility(negotiator, ILocaleNegotiator)
         self._action(ILocaleNegotiator, register)
 
-    @maybe_autocommit
+    @config_method
     def set_default_permission(self, permission):
         """
         Set the default permission to be used by all subsequent
@@ -1946,7 +1966,7 @@ class Configurator(object):
         self.registry.registerUtility(permission, IDefaultPermission)
         self._action(IDefaultPermission, None)
 
-    @maybe_autocommit
+    @config_method
     def set_session_factory(self, session_factory):
         """
         Configure the application with a :term:`session factory`.  If
@@ -1957,7 +1977,7 @@ class Configurator(object):
             self.registry.registerUtility(session_factory, ISessionFactory)
         self._action(ISessionFactory, register)
 
-    @maybe_autocommit
+    @config_method
     def add_translation_dirs(self, *specs):
         """ Add one or more :term:`translation directory` paths to the
         current configuration state.  The ``specs`` argument is a
@@ -2871,33 +2891,6 @@ class ActionPredicate(object):
 
 class PyramidConfigurationMachine(ConfigurationMachine):
     autocommit = False
-    def action(self, discriminator, callable, args=(), kw=None, order=0):
-        if kw is None:
-            kw = {}
-
-        includepath = getattr(self, 'includepath', ())
-        info = getattr(self, 'info', None)
-        if not info:
-            info = self._extract_stack()
-
-        action = (discriminator, callable, args, kw,
-                  includepath,
-                  info,
-                  order,
-                  )
-
-        # remove trailing false items
-        while (len(action) > 2) and not action[-1]:
-            action = action[:-1]
-
-        self.actions.append(action)
-
-    def _extract_stack(self):
-        try:
-            f = traceback.extract_stack(limit=5)
-            return f[0]
-        except:
-            return ''
 
     def processSpec(self, spec):
         """Check whether a callable needs to be processed.  The ``spec``
diff --git a/pyramid/tests/test_zcml.py b/pyramid/tests/test_zcml.py
index 73426f166..2ba6589b5 100644
--- a/pyramid/tests/test_zcml.py
+++ b/pyramid/tests/test_zcml.py
@@ -995,9 +995,12 @@ class TestSubscriberDirective(unittest.TestCase):
         subadapt = actions[0]
         self.assertEqual(subadapt['discriminator'], None)
         subadapt['callable'](*subadapt['args'], **subadapt['kw'])
+        registrations = self.config.registry._subscription_registrations
+        self.assertEqual(len(registrations), 1)
+        reg = registrations[0]
         self.assertEqual(
-            self.config.registry._subscription_registrations,
-            [((IDummy,), IFactory, None, factory, '')]
+            reg[:4],
+            ((IDummy,), IFactory, None, factory)
             )
 
     def test_register_with_handler(self):
@@ -1047,7 +1050,7 @@ class TestUtilityDirective(unittest.TestCase):
         self.assertEqual(utility['discriminator'], ('utility', IFactory, ''))
         self.assertEqual(utility['callable'].im_func,
                          Registry.registerUtility.im_func)
-        self.assertEqual(utility['args'], (None, IFactory, '', ''))
+        self.assertEqual(utility['args'][:3], (None, IFactory, ''))
         self.assertEqual(utility['kw'], {'factory':DummyFactory})
 
     def test_provides_from_component_provides(self):
@@ -1061,7 +1064,7 @@ class TestUtilityDirective(unittest.TestCase):
         self.assertEqual(utility['discriminator'], ('utility', IFactory, ''))
         self.assertEqual(utility['callable'].im_func,
                          Registry.registerUtility.im_func)
-        self.assertEqual(utility['args'], (component, IFactory, '', ''))
+        self.assertEqual(utility['args'][:3], (component, IFactory, ''))
         self.assertEqual(utility['kw'], {})
 
 class TestTranslationDirDirective(unittest.TestCase):
@@ -1261,14 +1264,17 @@ class DummyPackage(object):
         self.__file__ = '/__init__.py'
 
 def extract_actions(native):
+    from zope.configuration.config import expand_action
     L = []
     for action in native:
+        (discriminator, callable, args, kw, includepath, info, order
+         ) = expand_action(*action)
         d = {}
-        d['discriminator'] = action[0]
-        d['callable'] = action[1]
-        d['args'] = action[2]
-        d['kw'] = action[3]
-        d['order'] = action[4]
+        d['discriminator'] = discriminator
+        d['callable'] = callable
+        d['args'] = args
+        d['kw'] = kw
+        d['order'] = order
         L.append(d)
     return L