Make default_notfound_view and default_forbidden_view expect an exception as

a context.

Cause append_slash_notfound_view to work in case it is registered via
set_notfound_view.

6 files changed, 86 insertions, 42 deletions

diff --git a/TODO.txt b/TODO.txt
index ff4a3373f..fd60e6523 100644
--- a/TODO.txt
+++ b/TODO.txt
@@ -10,9 +10,7 @@
 
 - Provide a webob.Response class facade for forward compat.
 
-- Replace default_notfound_view and default_forbidden_view with better
-  exception view candidates.
-
 - Fix message catalog extraction / compilation documentation.
 
 - Docs about creating a venusian decorator.
+
diff --git a/repoze/bfg/configuration.py b/repoze/bfg/configuration.py
index 9ebb62a01..fdc259934 100644
--- a/repoze/bfg/configuration.py
+++ b/repoze/bfg/configuration.py
@@ -314,8 +314,8 @@ class Configurator(object):
                                         authorization_policy)
         for name, renderer in renderers:
             self.add_renderer(name, renderer)
-        self.set_notfound_view(default_notfound_view)
-        self.set_forbidden_view(default_forbidden_view)
+        self.add_view(default_notfound_view, context=NotFound)
+        self.add_view(default_forbidden_view, context=Forbidden)
         if locale_negotiator:
             registry.registerUtility(locale_negotiator, ILocaleNegotiator)
 
@@ -1258,8 +1258,8 @@ class Configurator(object):
         method's ``wrapper`` argument for a description)."""
         view = self._derive_view(view, attr=attr, renderer_name=renderer)
         def bwcompat_view(context, request):
-            ctx = getattr(request, 'context', None)
-            return view(ctx, request)
+            context = getattr(request, 'context', None)
+            return view(context, request)
         return self.add_view(bwcompat_view, context=Forbidden,
                              wrapper=wrapper, _info=_info)
 
@@ -1304,8 +1304,8 @@ class Configurator(object):
         """
         view = self._derive_view(view, attr=attr, renderer_name=renderer)
         def bwcompat_view(context, request):
-            ctx = getattr(request, 'context', None)
-            return view(ctx, request)
+            context = getattr(request, 'context', None)
+            return view(context, request)
         return self.add_view(bwcompat_view, context=NotFound,
                              wrapper=wrapper, _info=_info)
 
@@ -1749,11 +1749,12 @@ def rendered_response(renderer, response, view, context, request,
         return response
     result = renderer(response, {'view':view, 'renderer_name':renderer_name,
                                  'context':context, 'request':request})
-    response_factory = Response
-    reg = getattr(request, 'registry', None)
-    if reg is not None:
-        # be kind to old unit tests
-        response_factory = reg.queryUtility(IResponseFactory, default=Response)
+    try:
+        registry = request.registry
+    except AttributeError:
+        registry = get_current_registry()
+    response_factory = registry.queryUtility(IResponseFactory,
+                                             default=Response)
     response = response_factory(result)
     if request is not None: # in tests, it may be None
         attrs = request.__dict__
diff --git a/repoze/bfg/exceptions.py b/repoze/bfg/exceptions.py
index a50da36b2..e7b6dd570 100644
--- a/repoze/bfg/exceptions.py
+++ b/repoze/bfg/exceptions.py
@@ -9,8 +9,9 @@ class Forbidden(Exception):
 
     This exception's constructor accepts a single positional argument,
     which should be a string.  The value of this string will be placed
-    into the WSGI environment under the ``repoze.bfg.message`` key,
-    for availability to the Forbidden view."""
+    into the WSGI environment by the router under the
+    ``repoze.bfg.message`` key, for availability to the
+    :term:`Forbidden View`."""
 
 class NotFound(Exception):
     """\
@@ -21,8 +22,9 @@ class NotFound(Exception):
 
     This exception's constructor accepts a single positional argument,
     which should be a string.  The value of this string will be placed
-    into the WSGI environment under the ``repoze.bfg.message`` key,
-    for availability to the Not Found view."""
+    into the WSGI environment by the router under the
+    ``repoze.bfg.message`` key, for availability to the :term:`Not Found
+    View`."""
 
 class ConfigurationError(ZCE):
     """ Raised when inappropriate input values are supplied to an API
diff --git a/repoze/bfg/tests/test_configuration.py b/repoze/bfg/tests/test_configuration.py
index 90e074e9a..fa18b75cb 100644
--- a/repoze/bfg/tests/test_configuration.py
+++ b/repoze/bfg/tests/test_configuration.py
@@ -184,17 +184,34 @@ class ConfiguratorTests(unittest.TestCase):
             def subscribers(self, events, name):
                 self.events = events
                 return events
-            def registerUtility(self, impl, iface, name=None, info=None):
+            def registerUtility(self, *arg, **kw):
                 pass
         reg = DummyRegistry()
         config = self._makeOne(reg)
-        config.set_notfound_view = lambda *arg, **kw: None
-        config.set_forbidden_view = lambda *arg, **kw: None
+        config.add_view = lambda *arg, **kw: False
         config.setup_registry()
         self.assertEqual(reg.has_listeners, True)
         self.assertEqual(reg.notify(1), None)
         self.assertEqual(reg.events, (1,))
 
+    def test_setup_registry_registers_default_exception_views(self):
+        from repoze.bfg.exceptions import NotFound
+        from repoze.bfg.exceptions import Forbidden
+        from repoze.bfg.view import default_notfound_view
+        from repoze.bfg.view import default_forbidden_view
+        class DummyRegistry(object):
+            def registerUtility(self, *arg, **kw):
+                pass
+        reg = DummyRegistry()
+        config = self._makeOne(reg)
+        views = []
+        config.add_view = lambda *arg, **kw: views.append((arg, kw))
+        config.setup_registry()
+        self.assertEqual(views[0], ((default_notfound_view,),
+                                    {'context':NotFound}))
+        self.assertEqual(views[1], ((default_forbidden_view,),
+                                    {'context':Forbidden}))
+
     def test_setup_registry_custom_settings(self):
         from repoze.bfg.registry import Registry
         from repoze.bfg.interfaces import ISettings
diff --git a/repoze/bfg/tests/test_view.py b/repoze/bfg/tests/test_view.py
index 76a218cf6..94366ecbe 100644
--- a/repoze/bfg/tests/test_view.py
+++ b/repoze/bfg/tests/test_view.py
@@ -413,17 +413,23 @@ class TestDefaultForbiddenView(BaseTest, unittest.TestCase):
         from repoze.bfg.view import default_forbidden_view
         return default_forbidden_view(context, request)
 
+    def test_no_registry_on_request(self):
+        request = None
+        context = Exception()
+        response = self._callFUT(context, request)
+        self.assertEqual(response.status, '401 Unauthorized')
+        self.failUnless('<code></code>' in response.body)
+
     def test_nomessage(self):
         request = self._makeRequest()
-        context = self._makeContext()
+        context = Exception()
         response = self._callFUT(context, request)
         self.assertEqual(response.status, '401 Unauthorized')
         self.failUnless('<code></code>' in response.body)
 
     def test_withmessage(self):
         request = self._makeRequest()
-        request.environ['repoze.bfg.message'] = 'abc&123'
-        context = self._makeContext()
+        context = Exception('abc&123')
         response = self._callFUT(context, request)
         self.assertEqual(response.status, '401 Unauthorized')
         self.failUnless('<code>abc&amp;123</code>' in response.body)
@@ -433,17 +439,23 @@ class TestDefaultNotFoundView(BaseTest, unittest.TestCase):
         from repoze.bfg.view import default_notfound_view
         return default_notfound_view(context, request)
 
+    def test_no_registry_on_request(self):
+        request = None
+        context = Exception()
+        response = self._callFUT(context, request)
+        self.assertEqual(response.status, '404 Not Found')
+        self.failUnless('<code></code>' in response.body)
+
     def test_nomessage(self):
         request = self._makeRequest()
-        context = self._makeContext()
+        context = Exception()
         response = self._callFUT(context, request)
         self.assertEqual(response.status, '404 Not Found')
         self.failUnless('<code></code>' in response.body)
 
     def test_withmessage(self):
         request = self._makeRequest()
-        request.environ['repoze.bfg.message'] = 'abc&123'
-        context = self._makeContext()
+        context = Exception('abc&123')
         response = self._callFUT(context, request)
         self.assertEqual(response.status, '404 Not Found')
         self.failUnless('<code>abc&amp;123</code>' in response.body)
@@ -469,29 +481,37 @@ class AppendSlashNotFoundView(BaseTest, unittest.TestCase):
         reg.registerUtility(mapper, IRoutesMapper)
         return mapper
 
-    def test_no_mapper(self):
+    def test_context_is_not_exception(self):
         request = self._makeRequest(PATH_INFO='/abc')
+        request.exception = Exception('halloo')
         context = DummyContext()
         response = self._callFUT(context, request)
         self.assertEqual(response.status, '404 Not Found')
+        self.failUnless('halloo' in response.body)
+
+    def test_no_mapper(self):
+        request = self._makeRequest(PATH_INFO='/abc')
+        context = Exception()
+        response = self._callFUT(context, request)
+        self.assertEqual(response.status, '404 Not Found')
 
     def test_no_path(self):
         request = self._makeRequest()
-        context = self._makeContext()
+        context = Exception()
         self._registerMapper(request.registry, True)
         response = self._callFUT(context, request)
         self.assertEqual(response.status, '404 Not Found')
 
     def test_mapper_path_already_slash_ending(self):
         request = self._makeRequest(PATH_INFO='/abc/')
-        context = DummyContext()
+        context = Exception()
         self._registerMapper(request.registry, True)
         response = self._callFUT(context, request)
         self.assertEqual(response.status, '404 Not Found')
 
     def test_matches(self):
         request = self._makeRequest(PATH_INFO='/abc')
-        context = DummyContext()
+        context = Exception()
         self._registerMapper(request.registry, True)
         response = self._callFUT(context, request)
         self.assertEqual(response.status, '302 Found')
diff --git a/repoze/bfg/view.py b/repoze/bfg/view.py
index f88d8e973..d794f74d3 100644
--- a/repoze/bfg/view.py
+++ b/repoze/bfg/view.py
@@ -512,8 +512,8 @@ class bfg_view(object):
 
 def default_view(context, request, status):
     try:
-        msg = cgi.escape(request.environ['repoze.bfg.message'])
-    except KeyError:
+        msg = cgi.escape('%s' % context.args[0])
+    except Exception:
         msg = ''
     html = """
     <html>
@@ -526,12 +526,12 @@ def default_view(context, request, status):
     """ % (status, status, msg)
     headers = [('Content-Length', str(len(html))),
                ('Content-Type', 'text/html')]
-    response_factory = Response
-    registry = getattr(request, 'registry', None)
-    if registry is not None:
-        # be kind to old tests
-        response_factory = registry.queryUtility(IResponseFactory,
-                                                 default=Response)
+    try:
+        registry = request.registry
+    except AttributeError:
+        registry = get_current_registry()
+    response_factory = registry.queryUtility(IResponseFactory,
+                                             default=Response)
     return response_factory(status = status,
                             headerlist = headers,
                             app_iter = [html])
@@ -558,21 +558,27 @@ def append_slash_notfound_view(context, request):
     If you use :term:`ZCML`, add the following to your application's
     ``configure.zcml`` to use this view as the Not Found view::
 
-      <notfound
+      <view
+         context="repoze.bfg.exceptions.NotFound"
          view="repoze.bfg.view.append_slash_notfound_view"/>
 
     Or use the
-    :meth:`repoze.bfg.configuration.Configurator.set_notfound_view`
+    :meth:`repoze.bfg.configuration.Configurator.add_view`
     method if you don't use ZCML::
 
+      from repoze.bfg.exceptions import NotFound
       from repoze.bfg.view import append_slash_notfound_view
-      config.set_notfound_view(append_slash_notfound_view)
+      config.add_view(append_slash_notfound_view, context=NotFound)
 
     See also :ref:`changing_the_notfound_view`.
 
     .. note:: This function is new as of :mod:`repoze.bfg` version 1.1.
 
     """
+    if not isinstance(context, Exception):
+        # backwards compat for an append_notslash_view registered via
+        # config.set_notfound_view instead of as a proper exception view
+        context = getattr(request, 'exception', None)
     path = request.environ.get('PATH_INFO', '/')
     registry = request.registry
     mapper = registry.queryUtility(IRoutesMapper)