Features

--------

- The ``Configurator`` object now has two new methods: ``begin`` and
  ``end``.  The ``begin`` method is meant to be called before any
  "configuration" begins (e.g. before ``add_view``, et. al are
  called).  The ``end`` method is meant to be called after all
  "configuration" is complete.

  Previously, before there was imperative configuration at all (1.1
  and prior), configuration begin and end was invariably implied by
  the process of loading a ZCML file.  When a ZCML load happened, the
  threadlocal data structure containing the request and registry was
  modified before the load, and torn down after the load, making sure
  that all framework code that needed ``get_current_registry`` for the
  duration of the ZCML load was satisfied.

  Some API methods called during imperative configuration, (such as
  ``Configurator.add_view`` when a renderer is involved) end up for
  historical reasons calling ``get_current_registry``.  However, in
  1.2a5 and below, the Configurator supplied no functionality that
  allowed people to make sure that ``get_current_registry`` returned
  the registry implied by the configurator being used.  ``begin`` now
  serves this purpose.  Inversely, ``end`` pops the thread local
  stack, undoing the actions of ``begin``.

  We make this boundary explicit to reduce the potential for confusion
  when the configurator is used in different circumstances (e.g. in
  unit tests and app code vs. just in initial app setup).

  Existing code written for 1.2a1-1.2a5 which does not call ``begin``
  or ``end`` continues to work in the same manner it did before.  It
  is however suggested that this code be changed to call ``begin`` and
  ``end`` to reduce the potential for confusion in the future.

- All ``paster`` templates which generate an application skeleton now
  make use of the new ``begin`` and ``end`` methods of the
  Configurator they use in their respective copies of ``run.py`` and
  ``tests.py``.

Documentation
-------------

- All documentation that makes use of a ``Configurator`` object to do
  application setup and test setup now makes use of the new ``begin``
  and ``end`` methods of the configurator.

Bug Fixes
---------

- When a ``repoze.bfg.exceptions.NotFound`` or
  ``repoze.bfg.exceptions.Forbidden`` *class* (as opposed to instance)
  was raised as an exception within a root factory (or route root
  factory), the exception would not be caught properly by the
  ``repoze.bfg.`` Router and it would propagate to up the call stack,
  as opposed to rendering the not found view or the forbidden view as
  would have been expected.

15 files changed, 199 insertions, 45 deletions

diff --git a/repoze/bfg/chameleon_text.py b/repoze/bfg/chameleon_text.py
index 9cb1c989a..6a9fb7c6c 100644
--- a/repoze/bfg/chameleon_text.py
+++ b/repoze/bfg/chameleon_text.py
@@ -43,7 +43,7 @@ class TextTemplateRenderer(object):
     def __init__(self, path):
         self.path = path
 
-    @reify
+    @reify # avoid looking up reload_templates before manager pushed
     def template(self):
         settings = get_settings()
         auto_reload = settings and settings['reload_templates']
diff --git a/repoze/bfg/configuration.py b/repoze/bfg/configuration.py
index 74d91a73b..a741c1564 100644
--- a/repoze/bfg/configuration.py
+++ b/repoze/bfg/configuration.py
@@ -124,6 +124,7 @@ class Configurator(object):
     class.  The debug logger is used by :mod:`repoze.bfg` itself to
     log warnings and authorization debugging information.
     """
+    manager = manager # for testing injection
     def __init__(self, registry=None, package=None, settings=None,
                  root_factory=None, authentication_policy=None,
                  authorization_policy=None, renderers=DEFAULT_RENDERERS,
@@ -274,6 +275,22 @@ class Configurator(object):
 
     # API
 
+    def begin(self, request=None):
+        """ Indicate that application or test configuration has begun.
+        This pushes a dictionary containing the registry implied by
+        this configurator and the :term:`request` implied by
+        ``request`` on to the :term:`thread local` stack consulted by
+        various ``repoze.bfg.threadlocal`` API functions."""
+        self.manager.push({'registry':self.registry, 'request':request})
+
+    def end(self):
+        """ Indicate that application or test configuration has ended.
+        This pops the last value pushed on to the :term:`thread local`
+        stack (usually by the ``begin`` method) and returns that
+        value.
+        """
+        return self.manager.pop()
+
     def add_subscriber(self, subscriber, iface=None):
         """Add an event :term:`subscriber` for the event stream
         implied by the supplied ``iface`` interface.  The
@@ -291,7 +308,7 @@ class Configurator(object):
         self.registry.registerHandler(subscriber, iface)
         return subscriber
 
-    def make_wsgi_app(self, manager=manager):
+    def make_wsgi_app(self):
         """ Returns a :mod:`repoze.bfg` WSGI application representing
         the current configuration state and sends a
         ``repoze.bfg.interfaces.WSGIApplicationCreatedEvent`` event to
@@ -302,11 +319,11 @@ class Configurator(object):
         # We push the registry on to the stack here in case any code
         # that depends on the registry threadlocal APIs used in
         # listeners subscribed to the WSGIApplicationCreatedEvent.
-        manager.push({'registry':self.registry, 'request':None})
+        self.manager.push({'registry':self.registry, 'request':None})
         try:
             self.registry.notify(WSGIApplicationCreatedEvent(app))
         finally:
-            manager.pop()
+            self.manager.pop()
         return app
 
     def load_zcml(self, spec='configure.zcml', lock=threading.Lock()):
@@ -323,12 +340,12 @@ class Configurator(object):
             package = sys.modules[package_name]
 
         lock.acquire()
-        manager.push({'registry':self.registry, 'request':None})
+        self.manager.push({'registry':self.registry, 'request':None})
         try:
             xmlconfig.file(filename, package, execute=True)
         finally:
             lock.release()
-            manager.pop()
+            self.manager.pop()
         return self.registry
 
     def add_view(self, view=None, name="", for_=None, permission=None, 
@@ -1560,12 +1577,14 @@ def make_app(root_factory, package=None, filename='configure.zcml',
     ``settings`` keyword parameter.
     """
     settings = settings or options or {}
+    zcml_file = settings.get('configure_zcml', filename)
     config = Configurator(package=package, settings=settings,
                           root_factory=root_factory)
     if getSiteManager is None:
         from zope.component import getSiteManager
     getSiteManager.sethook(get_current_registry)
-    zcml_file = settings.get('configure_zcml', filename)
+    config.begin()
     config.load_zcml(zcml_file)
+    config.end()
     return config.make_wsgi_app()
 
diff --git a/repoze/bfg/paster_templates/alchemy/+package+/run.py_tmpl b/repoze/bfg/paster_templates/alchemy/+package+/run.py_tmpl
index 462691048..0c8f31ec8 100644
--- a/repoze/bfg/paster_templates/alchemy/+package+/run.py_tmpl
+++ b/repoze/bfg/paster_templates/alchemy/+package+/run.py_tmpl
@@ -18,6 +18,7 @@ def app(global_config, **settings):
     
     It is usually called by the PasteDeploy framework during ``paster serve``.
     """
+    zcml_file = settings.get('configure_zcml', 'configure.zcml')
     db_string = settings.get('db_string')
     if db_string is None:
         raise ValueError("No 'db_string' in application configuration.")
@@ -26,7 +27,8 @@ def app(global_config, **settings):
         db_echo = True
     get_root = appmaker(db_string, db_echo)
     config = Configurator(settings=settings, root_factory=get_root)
-    zcml_file = settings.get('configure_zcml', 'configure.zcml')
+    config.begin()
     config.load_zcml(zcml_file)
+    config.end()
     return config.make_wsgi_app()
 
diff --git a/repoze/bfg/paster_templates/routesalchemy/+package+/run.py_tmpl b/repoze/bfg/paster_templates/routesalchemy/+package+/run.py_tmpl
index 3b392debd..066dbecc0 100644
--- a/repoze/bfg/paster_templates/routesalchemy/+package+/run.py_tmpl
+++ b/repoze/bfg/paster_templates/routesalchemy/+package+/run.py_tmpl
@@ -18,12 +18,14 @@ def app(global_config, **settings):
     
     It is usually called by the PasteDeploy framework during ``paster serve``.
     """
+    zcml_file = settings.get('configure_zcml', 'configure.zcml')
     db_string = settings.get('db_string')
     if db_string is None:
         raise ValueError("No 'db_string' value in application configuration.")
     initialize_sql(db_string)
     config = Configurator(settings=settings)
-    zcml_file = settings.get('configure_zcml', 'configure.zcml')
+    config.begin()
     config.load_zcml(zcml_file)
+    config.end()
     return config.make_wsgi_app()
 
diff --git a/repoze/bfg/paster_templates/routesalchemy/+package+/tests.py_tmpl b/repoze/bfg/paster_templates/routesalchemy/+package+/tests.py_tmpl
index 5c1477189..ed7f1280b 100644
--- a/repoze/bfg/paster_templates/routesalchemy/+package+/tests.py_tmpl
+++ b/repoze/bfg/paster_templates/routesalchemy/+package+/tests.py_tmpl
@@ -1,4 +1,5 @@
 import unittest
+from repoze.bfg.configuration import Configurator
 from repoze.bfg import testing
 
 def _initTestingDB():
@@ -8,11 +9,12 @@ def _initTestingDB():
 
 class TestMyView(unittest.TestCase):
     def setUp(self):
-        testing.setUp()
+        self.config = Configurator()
+        self.config.begin()
         _initTestingDB()
 
     def tearDown(self):
-        testing.tearDown()
+        self.config.end()
 
     def test_it(self):
         from {{package}}.views import my_view
diff --git a/repoze/bfg/paster_templates/starter/+package+/run.py_tmpl b/repoze/bfg/paster_templates/starter/+package+/run.py_tmpl
index de643f834..4505ed001 100644
--- a/repoze/bfg/paster_templates/starter/+package+/run.py_tmpl
+++ b/repoze/bfg/paster_templates/starter/+package+/run.py_tmpl
@@ -5,7 +5,9 @@ def app(global_config, **settings):
     """ This function returns a ``repoze.bfg`` application object.  It
     is usually called by the PasteDeploy framework during ``paster
     serve``"""
-    config = Configurator(root_factory=get_root, settings=settings)
     zcml_file = settings.get('configure_zcml', 'configure.zcml')
+    config = Configurator(root_factory=get_root, settings=settings)
+    config.begin()
     config.load_zcml(zcml_file)
+    config.end()
     return config.make_wsgi_app()
diff --git a/repoze/bfg/paster_templates/starter/+package+/tests.py_tmpl b/repoze/bfg/paster_templates/starter/+package+/tests.py_tmpl
index d4798bce7..578a58d35 100644
--- a/repoze/bfg/paster_templates/starter/+package+/tests.py_tmpl
+++ b/repoze/bfg/paster_templates/starter/+package+/tests.py_tmpl
@@ -1,13 +1,15 @@
 import unittest
 
+from repoze.bfg.configuration import Configurator
 from repoze.bfg import testing
 
 class ViewTests(unittest.TestCase):
     def setUp(self):
-        testing.setUp()
+        self.config = Configurator()
+        self.config.begin()
 
     def tearDown(self):
-        testing.tearDown()
+        self.config.end()
 
     def test_my_view(self):
         from {{package}}.views import my_view
diff --git a/repoze/bfg/paster_templates/zodb/+package+/run.py_tmpl b/repoze/bfg/paster_templates/zodb/+package+/run.py_tmpl
index 3ad6a8970..22e2048e0 100644
--- a/repoze/bfg/paster_templates/zodb/+package+/run.py_tmpl
+++ b/repoze/bfg/paster_templates/zodb/+package+/run.py_tmpl
@@ -8,6 +8,7 @@ def app(global_config, **settings):
     It is usually called by the PasteDeploy framework during ``paster serve``.
     """
     zodb_uri = settings.get('zodb_uri')
+    zcml_file = settings.get('configure_zcml', 'configure.zcml')
     if zodb_uri is None:
         raise ValueError("No 'zodb_uri' in application configuration.")
 
@@ -15,6 +16,7 @@ def app(global_config, **settings):
     def get_root(request):
         return finder(request.environ)
     config = Configurator(root_factory=get_root, settings=settings)
-    zcml_file = settings.get('configure_zcml', 'configure.zcml')
+    config.begin()
     config.load_zcml(zcml_file)
+    config.end()
     return config.make_wsgi_app()
diff --git a/repoze/bfg/paster_templates/zodb/+package+/tests.py_tmpl b/repoze/bfg/paster_templates/zodb/+package+/tests.py_tmpl
index 5c7d27a37..30da5c9b3 100644
--- a/repoze/bfg/paster_templates/zodb/+package+/tests.py_tmpl
+++ b/repoze/bfg/paster_templates/zodb/+package+/tests.py_tmpl
@@ -1,13 +1,15 @@
 import unittest
 
+from repoze.bfg.configuration import Configurator
 from repoze.bfg import testing
 
 class ViewTests(unittest.TestCase):
     def setUp(self):
-        testing.setUp()
+        self.config = Configurator()
+        self.config.begin()
 
     def tearDown(self):
-        testing.tearDown()
+        self.config.end()
 
     def test_my_view(self):
         from {{package}}.views import my_view
diff --git a/repoze/bfg/router.py b/repoze/bfg/router.py
index 630aa201c..caa56ffe2 100644
--- a/repoze/bfg/router.py
+++ b/repoze/bfg/router.py
@@ -125,11 +125,17 @@ class Router(object):
                     response = view_callable(context, request)
 
             except Forbidden, why:
-                msg = why[0]
+                try:
+                    msg = why[0]
+                except (IndexError, TypeError):
+                    msg = ''
                 environ['repoze.bfg.message'] = msg
                 response = self.forbidden_view(context, request)
             except NotFound, why:
-                msg = why[0]
+                try:
+                    msg = why[0]
+                except (IndexError, TypeError):
+                    msg = ''
                 environ['repoze.bfg.message'] = msg
                 response = self.notfound_view(context, request)
 
diff --git a/repoze/bfg/testing.py b/repoze/bfg/testing.py
index 428f1c11f..434131321 100644
--- a/repoze/bfg/testing.py
+++ b/repoze/bfg/testing.py
@@ -21,6 +21,7 @@ from repoze.bfg.interfaces import ITraverser
 from repoze.bfg.interfaces import IView
 from repoze.bfg.interfaces import IViewPermission
 
+from repoze.bfg.configuration import Configurator
 from repoze.bfg.exceptions import Forbidden
 from repoze.bfg.registry import Registry
 from repoze.bfg.security import Allowed
@@ -592,17 +593,22 @@ def setUp(registry=None, request=None, hook_zca=True):
 
     .. note:: The ``hook_zca`` argument is new as of :mod:`repoze.bfg`
        1.2.
+
+    .. warning:: Although this method of tearing a test setup down
+                 will never disappear, after :mod:`repoze.bfg` 1.2a6,
+                 using the ``begin`` and ``end`` methods of a
+                 ``Configurator`` are prefered to using
+                 ``repoze.bfg.testing.setUp`` and
+                 ``repoze.bfg.testing.tearDown``.  See
+                 :ref:`unittesting_chapter` for more information.
     """
     manager.clear()
     if registry is None:
         registry = Registry('testing')
-    manager.push({'registry':registry, 'request':request})
+    config = Configurator(registry=registry)
+    config.begin(request=request)
     if hook_zca:
-        try:
-            from zope.component import getSiteManager
-            getSiteManager.sethook(get_current_registry)
-        except ImportError: # pragma: no cover
-            pass
+        hook_zca_api()
 
 def tearDown(unhook_zca=True):
     """Undo the effects ``repoze.bfg.testing.setUp``.  Use this
@@ -619,20 +625,24 @@ def tearDown(unhook_zca=True):
     .. note:: The ``unhook_zca`` argument is new as of
        :mod:`repoze.bfg` 1.2.
 
+    .. warning:: Although this method of tearing a test setup down
+                 will never disappear, after :mod:`repoze.bfg` 1.2a6,
+                 using the ``begin`` and ``end`` methods of a
+                 ``Configurator`` are prefered to using
+                 ``repoze.bfg.testing.setUp`` and
+                 ``repoze.bfg.testing.tearDown``.  See
+                 :ref:`unittesting_chapter` for more information.
+
     """
     if unhook_zca:
-        try:
-            from zope.component import getSiteManager
-            getSiteManager.reset()
-        except ImportError: # pragma: no cover
-            pass
+        unhook_zca_api()
     info = manager.pop()
     manager.clear()
     if info is not None:
-        reg = info['registry']
-        if hasattr(reg, '__init__') and hasattr(reg, '__name__'):
+        registry = info['registry']
+        if hasattr(registry, '__init__') and hasattr(registry, '__name__'):
             try:
-                reg.__init__(reg.__name__)
+                registry.__init__(registry.__name__)
             except TypeError:
                 # calling __init__ is largely for the benefit of
                 # people who want to use the global ZCA registry;
@@ -648,3 +658,17 @@ def cleanUp(*arg, **kw):
     extensive production usage, it will never be removed."""
     setUp(*arg, **kw)
 
+def hook_zca_api():
+    try:
+        from zope.component import getSiteManager
+        getSiteManager.sethook(get_current_registry)
+    except ImportError: # pragma: no cover
+        pass
+    
+def unhook_zca_api():
+    try:
+        from zope.component import getSiteManager
+        getSiteManager.reset()
+    except ImportError: # pragma: no cover
+        pass
+    
diff --git a/repoze/bfg/tests/fixtureapp/views.py b/repoze/bfg/tests/fixtureapp/views.py
index 82e92d618..d9bc0bb6e 100644
--- a/repoze/bfg/tests/fixtureapp/views.py
+++ b/repoze/bfg/tests/fixtureapp/views.py
@@ -5,9 +5,6 @@ def fixture_view(context, request):
     """ """
     return Response('fixture')
 
-def renderer_view(request):
-    return {'a':1}
-
 class IDummy(Interface):
     pass
 
diff --git a/repoze/bfg/tests/test_configuration.py b/repoze/bfg/tests/test_configuration.py
index 2e2283ab4..a42e230f1 100644
--- a/repoze/bfg/tests/test_configuration.py
+++ b/repoze/bfg/tests/test_configuration.py
@@ -88,6 +88,36 @@ class ConfiguratorTests(unittest.TestCase):
         self.failUnless(config.registry.getUtility(IRendererFactory, '.pt'))
         self.failUnless(config.registry.getUtility(IRendererFactory, '.txt'))
 
+    def test_begin(self):
+        from repoze.bfg.configuration import Configurator
+        config = Configurator()
+        manager = DummyThreadLocalManager()
+        config.manager = manager
+        config.begin()
+        self.assertEqual(manager.pushed,
+                         {'registry':config.registry, 'request':None})
+        self.assertEqual(manager.popped, False)
+
+    def test_begin_with_request(self):
+        from repoze.bfg.configuration import Configurator
+        config = Configurator()
+        request = object()
+        manager = DummyThreadLocalManager()
+        config.manager = manager
+        config.begin(request=request)
+        self.assertEqual(manager.pushed,
+                         {'registry':config.registry, 'request':request})
+        self.assertEqual(manager.popped, False)
+
+    def test_end(self):
+        from repoze.bfg.configuration import Configurator
+        config = Configurator()
+        manager = DummyThreadLocalManager()
+        config.manager = manager
+        config.end()
+        self.assertEqual(manager.pushed, None)
+        self.assertEqual(manager.popped, True)
+
     def test_ctor_with_package_registry(self):
         import sys
         from repoze.bfg.configuration import Configurator
@@ -274,16 +304,12 @@ class ConfiguratorTests(unittest.TestCase):
     def test_make_wsgi_app(self):
         from repoze.bfg.router import Router
         from repoze.bfg.interfaces import IWSGIApplicationCreatedEvent
-        class ThreadLocalManager(object):
-            def push(self, d):
-                self.pushed = d
-            def pop(self):
-                self.popped = True
-        manager = ThreadLocalManager()
+        manager = DummyThreadLocalManager()
         config = self._makeOne()
         subscriber = self._registerEventListener(config,
                                                  IWSGIApplicationCreatedEvent)
-        app = config.make_wsgi_app(manager=manager)
+        config.manager = manager
+        app = config.make_wsgi_app()
         self.assertEqual(app.__class__, Router)
         self.assertEqual(manager.pushed['registry'], config.registry)
         self.assertEqual(manager.pushed['request'], None)
@@ -2702,6 +2728,12 @@ class DummyConfigurator(object):
         self.package = package
         self.settings = settings
 
+    def begin(self):
+        self.begun = True
+
+    def end(self):
+        self.ended = True
+
     def load_zcml(self, filename):
         self.zcml_file = filename
     
@@ -2738,5 +2770,11 @@ class DummyMultiView:
 class DummyGetSiteManager(object):
     def sethook(self, hook):
         self.hook = hook
-        
     
+class DummyThreadLocalManager(object):
+    pushed = None
+    popped = False
+    def push(self, d):
+        self.pushed = d
+    def pop(self):
+        self.popped = True
diff --git a/repoze/bfg/tests/test_integration.py b/repoze/bfg/tests/test_integration.py
index d210030af..da866f8d0 100644
--- a/repoze/bfg/tests/test_integration.py
+++ b/repoze/bfg/tests/test_integration.py
@@ -70,7 +70,7 @@ class TestFixtureApp(unittest.TestCase):
         config = Configurator()
         config.load_zcml('repoze.bfg.tests.fixtureapp:configure.zcml')
         twill.add_wsgi_intercept('localhost', 6543, config.make_wsgi_app)
-        if sys.platform is 'win32':
+        if sys.platform is 'win32': # pragma: no cover
             out = open('nul:', 'wb')
         else:
             out = open('/dev/null', 'wb')
diff --git a/repoze/bfg/tests/test_router.py b/repoze/bfg/tests/test_router.py
index 5352c6d79..c1d60ae9a 100644
--- a/repoze/bfg/tests/test_router.py
+++ b/repoze/bfg/tests/test_router.py
@@ -158,6 +158,62 @@ class TestRouter(unittest.TestCase):
         self.failIf('debug_notfound' in result[0])
         self.assertEqual(len(logger.messages), 0)
 
+    def test_traverser_raises_notfound_class(self):
+        from repoze.bfg.exceptions import NotFound
+        environ = self._makeEnviron()
+        context = DummyContext()
+        self._registerTraverserFactory(context, raise_error=NotFound)
+        router = self._makeOne()
+        start_response = DummyStartResponse()
+        result = router(environ, start_response)
+        headers = start_response.headers
+        self.assertEqual(len(headers), 2)
+        status = start_response.status
+        self.assertEqual(status, '404 Not Found')
+        self.failUnless('<code></code>' in result[0], result)
+
+    def test_traverser_raises_notfound_instance(self):
+        from repoze.bfg.exceptions import NotFound
+        environ = self._makeEnviron()
+        context = DummyContext()
+        self._registerTraverserFactory(context, raise_error=NotFound('foo'))
+        router = self._makeOne()
+        start_response = DummyStartResponse()
+        result = router(environ, start_response)
+        headers = start_response.headers
+        self.assertEqual(len(headers), 2)
+        status = start_response.status
+        self.assertEqual(status, '404 Not Found')
+        self.failUnless('<code>foo</code>' in result[0], result)
+
+    def test_traverser_raises_forbidden_class(self):
+        from repoze.bfg.exceptions import Forbidden
+        environ = self._makeEnviron()
+        context = DummyContext()
+        self._registerTraverserFactory(context, raise_error=Forbidden)
+        router = self._makeOne()
+        start_response = DummyStartResponse()
+        result = router(environ, start_response)
+        headers = start_response.headers
+        self.assertEqual(len(headers), 2)
+        status = start_response.status
+        self.assertEqual(status, '401 Unauthorized')
+        self.failUnless('<code></code>' in result[0], result)
+
+    def test_traverser_raises_forbidden_instance(self):
+        from repoze.bfg.exceptions import Forbidden
+        environ = self._makeEnviron()
+        context = DummyContext()
+        self._registerTraverserFactory(context, raise_error=Forbidden('foo'))
+        router = self._makeOne()
+        start_response = DummyStartResponse()
+        result = router(environ, start_response)
+        headers = start_response.headers
+        self.assertEqual(len(headers), 2)
+        status = start_response.status
+        self.assertEqual(status, '401 Unauthorized')
+        self.failUnless('<code>foo</code>' in result[0], result)
+
     def test_call_no_view_registered_no_isettings(self):
         environ = self._makeEnviron()
         context = DummyContext()