- The error presented when a view invoked by the router returns a

  non-response object now includes the view's name for troubleshooting
  purposes.

- A "new response" event is emitted for forbiden and notfound views.

3 files changed, 107 insertions, 40 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index a6a6abcaa..33518eb5e 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -15,6 +15,15 @@ Features
   basically).  See the Hooks narrative chapter of the BFG docs for
   more info.
 
+- The error presented when a view invoked by the router returns a
+  non-response object now includes the view's name for troubleshooting
+  purposes.
+
+Bug Fixes
+---------
+
+- A "new response" event is emitted for forbiden and notfound views.
+
 Deprecations
 ------------
 
@@ -28,7 +37,6 @@ Renames
 - Renamed ``repoze.bfg.interfaces.IForbiddenResponseFactory`` to
   ``repoze.bfg.interfaces.IForbiddenView``.
 
-
 0.9a7 (2009-05-30)
 ==================
 
diff --git a/repoze/bfg/router.py b/repoze/bfg/router.py
index 32b8c5b72..b243ec5b0 100644
--- a/repoze/bfg/router.py
+++ b/repoze/bfg/router.py
@@ -16,7 +16,6 @@ from repoze.bfg.events import WSGIApplicationCreatedEvent
 
 from repoze.bfg.interfaces import ILogger
 from repoze.bfg.interfaces import ISecurityPolicy
-from repoze.bfg.interfaces import INotFoundAppFactory
 from repoze.bfg.interfaces import IRequestFactory
 from repoze.bfg.interfaces import IResponseFactory
 from repoze.bfg.interfaces import IRootFactory
@@ -24,7 +23,9 @@ from repoze.bfg.interfaces import IRouter
 from repoze.bfg.interfaces import IRoutesMapper
 from repoze.bfg.interfaces import ISettings
 from repoze.bfg.interfaces import IForbiddenView
+from repoze.bfg.interfaces import INotFoundView
 from repoze.bfg.interfaces import IUnauthorizedAppFactory
+from repoze.bfg.interfaces import INotFoundAppFactory
 from repoze.bfg.interfaces import IView
 from repoze.bfg.interfaces import IViewPermission
 from repoze.bfg.interfaces import IAuthorizationPolicy
@@ -112,6 +113,9 @@ class Router(object):
                 app = notfound_app_factory()
                 response = request.get_response(app)
                 return response
+
+        notfound = registry.queryUtility(INotFoundView,
+                                         default=notfound)
                 
         self.notfound_view = notfound or default_notfound_view
         
@@ -134,11 +138,20 @@ class Router(object):
         iterable.
         """
         registry = self.registry
+        logger = self.logger
+
         threadlocals = {'registry':registry, 'request':None}
         self.threadlocal_manager.push(threadlocals)
 
-        logger = self.logger
-        request = None
+        def respond(response, view_name):
+            registry.has_listeners and registry.notify(NewResponse(response))
+            try:
+                start_response(response.status, response.headerlist)
+                return response.app_iter
+            except AttributeError:
+                raise ValueError(
+                    'Non-response object returned from view %s: %r' %
+                    (view_name, response))
 
         try:
             if self.request_factory is None:
@@ -221,9 +234,8 @@ class Router(object):
 
                 environ['repoze.bfg.message'] = msg
 
-                response = self.forbidden_view(context, request)
-                start_response(response.status, response.headerlist)
-                return response.app_iter
+                return respond(self.forbidden_view(context, request),
+                               '<IForbiddenView>')
 
             response = registry.queryMultiAdapter(
                 (context, request), IView, name=view_name)
@@ -241,18 +253,10 @@ class Router(object):
                 else:
                     msg = request.url
                 environ['repoze.bfg.message'] = msg
-                response = self.notfound_view(context, request)
-                start_response(response.status, response.headerlist)
-                return response.app_iter
-
-            registry.has_listeners and registry.notify(NewResponse(response))
+                return respond(self.notfound_view(context, request),
+                               '<INotFoundView>')
 
-            try:
-                start_response(response.status, response.headerlist)
-                return response.app_iter
-            except AttributeError:
-                raise ValueError(
-                    'Non-response object returned from view: %r' % response)
+            return respond(response, view_name)
 
         finally:
             self.threadlocal_manager.pop()
diff --git a/repoze/bfg/tests/test_router.py b/repoze/bfg/tests/test_router.py
index 3009e65dd..09c3f7c1f 100644
--- a/repoze/bfg/tests/test_router.py
+++ b/repoze/bfg/tests/test_router.py
@@ -127,26 +127,7 @@ class RouterTests(unittest.TestCase):
         router = self._makeOne()
         self.assertEqual(router.root_policy, rootfactory)
 
-    def test_secpol_with_inotfound_appfactory_BBB(self):
-        from repoze.bfg.interfaces import INotFoundAppFactory
-        environ = self._makeEnviron()
-        context = DummyContext()
-        self._registerTraverserFactory(context)
-        rootfactory = self._registerRootFactory(None)
-        logger = self._registerLogger()
-        def factory():
-            return 'yo'
-        self.registry.registerUtility(factory, INotFoundAppFactory)
-        router = self._makeOne()
-        self.assertEqual(len(logger.messages), 1)
-        self.failUnless('INotFoundView' in logger.messages[0])
-        class DummyRequest:
-            def get_response(self, app):
-                return app
-        req = DummyRequest()
-        self.assertEqual(router.notfound_view(None, req), 'yo')
-
-    def test_iforbidden_responsefactory_override(self):
+    def test_iforbiddenview_override(self):
         from repoze.bfg.interfaces import IForbiddenView
         def app():
             """ """
@@ -155,14 +136,30 @@ class RouterTests(unittest.TestCase):
         router = self._makeOne()
         self.assertEqual(router.forbidden_view, app)
 
-    def test_iforbidden_responsefactory_nooverride(self):
+    def test_iforbiddenview_nooverride(self):
         context = DummyContext()
         self._registerRootFactory(None)
         router = self._makeOne()
         from repoze.bfg.router import default_forbidden_view
         self.assertEqual(router.forbidden_view, default_forbidden_view)
 
-    def test_secpol_with_iunauthorized_appfactory_BBB(self):
+    def test_inotfoundview_override(self):
+        from repoze.bfg.interfaces import INotFoundView
+        def app():
+            """ """
+        self.registry.registerUtility(app, INotFoundView)
+        self._registerRootFactory(None)
+        router = self._makeOne()
+        self.assertEqual(router.notfound_view, app)
+
+    def test_inotfoundview_nooverride(self):
+        context = DummyContext()
+        self._registerRootFactory(None)
+        router = self._makeOne()
+        from repoze.bfg.router import default_notfound_view
+        self.assertEqual(router.notfound_view, default_notfound_view)
+
+    def test_iunauthorized_appfactory_BBB(self):
         from repoze.bfg.interfaces import IUnauthorizedAppFactory
         environ = self._makeEnviron()
         context = DummyContext()
@@ -181,6 +178,25 @@ class RouterTests(unittest.TestCase):
         req = DummyRequest()
         self.assertEqual(router.forbidden_view(None, req), 'yo')
 
+    def test_inotfound_appfactory_BBB(self):
+        from repoze.bfg.interfaces import INotFoundAppFactory
+        environ = self._makeEnviron()
+        context = DummyContext()
+        self._registerTraverserFactory(context)
+        rootfactory = self._registerRootFactory(None)
+        logger = self._registerLogger()
+        def factory():
+            return 'yo'
+        self.registry.registerUtility(factory, INotFoundAppFactory)
+        router = self._makeOne()
+        self.assertEqual(len(logger.messages), 1)
+        self.failUnless('INotFoundView' in logger.messages[0])
+        class DummyRequest:
+            def get_response(self, app):
+                return app
+        req = DummyRequest()
+        self.assertEqual(router.notfound_view(None, req), 'yo')
+
     def test_call_no_view_registered_no_isettings(self):
         environ = self._makeEnviron()
         context = DummyContext()
@@ -271,6 +287,45 @@ class RouterTests(unittest.TestCase):
         start_response = DummyStartResponse()
         self.assertRaises(ValueError, router, environ, start_response)
 
+    def test_inotfoundview_returns_nonresponse(self):
+        from repoze.bfg.interfaces import INotFoundView
+        context = DummyContext()
+        environ = self._makeEnviron()
+        self._registerTraverserFactory(context)
+        self._registerRootFactory(None)
+        def app(context, request):
+            """ """
+        self.registry.registerUtility(app, INotFoundView)
+        router = self._makeOne()
+        start_response = DummyStartResponse()
+        self.assertRaises(ValueError, router, environ, start_response)
+
+    def test_iforbiddenview_returns_nonresponse(self):
+        from repoze.bfg.interfaces import IForbiddenView
+        from zope.interface import Interface
+        from zope.interface import directlyProvides
+        class IContext(Interface):
+            pass
+        from repoze.bfg.interfaces import IRequest
+        context = DummyContext()
+        directlyProvides(context, IContext)
+        self._registerTraverserFactory(context)
+        self._registerAuthenticationPolicy()
+        response = DummyResponse()
+        view = make_view(response)
+        from repoze.bfg.security import ACLDenied
+        denied = ACLDenied('ace', 'acl', 'permission', ['principals'], context)
+        environ = self._makeEnviron()
+        self._registerView(view, '', IContext, IRequest)
+        checker = self._registerViewPermission('', denied)
+        self._registerRootFactory(None)
+        def app(context, request):
+            """ """
+        self.registry.registerUtility(app, IForbiddenView)
+        router = self._makeOne()
+        start_response = DummyStartResponse()
+        self.assertRaises(ValueError, router, environ, start_response)
+
     def test_call_view_registered_nonspecific_default_path(self):
         context = DummyContext()
         self._registerTraverserFactory(context)