Documentation

-------------

- Add an API chapter for the ``repoze.bfg.request`` module, which
  includes documentation for the ``repoze.bfg.request.Request`` class
  (the "request object").

- Modify the "Request and Response" narrative chapter to reference the
  new ``repoze.bfg.request`` API chapter.  Some content was moved from
  this chapter into the API documentation itself.

Features
--------

- A new ``repoze.bfg.request.Request.add_response_callback`` API has
  been added.  This method is documented in the new
  ``repoze.bfg.request`` API chapter.  It can be used to influence
  response values before a concrete response object has been created.

Internal
--------

- The (internal) feature which made it possible to attach a
  ``global_response_headers`` attribute to the request (which was
  assumed to contain a sequence of header key/value pairs which would
  later be added to the response by the router), has been removed.
  The functionality of
  ``repoze.bfg.request.Request.add_response_callback`` takes its
  place.

11 files changed, 310 insertions, 97 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index a20c26c0e..6fc1a664a 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,3 +1,36 @@
+Next release
+============
+
+Documentation
+-------------
+
+- Add an API chapter for the ``repoze.bfg.request`` module, which
+  includes documentation for the ``repoze.bfg.request.Request`` class
+  (the "request object").
+
+- Modify the "Request and Response" narrative chapter to reference the
+  new ``repoze.bfg.request`` API chapter.  Some content was moved from
+  this chapter into the API documentation itself.
+
+Features
+--------
+
+- A new ``repoze.bfg.request.Request.add_response_callback`` API has
+  been added.  This method is documented in the new
+  ``repoze.bfg.request`` API chapter.  It can be used to influence
+  response values before a concrete response object has been created.
+
+Internal
+--------
+
+- The (internal) feature which made it possible to attach a
+  ``global_response_headers`` attribute to the request (which was
+  assumed to contain a sequence of header key/value pairs which would
+  later be added to the response by the router), has been removed.
+  The functionality of
+  ``repoze.bfg.request.Request.add_response_callback`` takes its
+  place.
+
 1.3a9 (2010-08-22)
 ==================
 
diff --git a/docs/api.rst b/docs/api.rst
index 3bdb323ca..050e8be13 100644
--- a/docs/api.rst
+++ b/docs/api.rst
@@ -20,6 +20,7 @@ documentation is organized alphabetically by module name.
    api/location
    api/paster
    api/renderers
+   api/request
    api/router
    api/scripting
    api/security
diff --git a/docs/api/request.rst b/docs/api/request.rst
new file mode 100644
index 000000000..86202b830
--- /dev/null
+++ b/docs/api/request.rst
@@ -0,0 +1,78 @@
+.. _request_module:
+
+:mod:`repoze.bfg.request`
+---------------------------
+
+.. module:: repoze.bfg.request
+
+.. autoclass:: Request
+   :members:
+   :inherited-members:
+
+   .. attribute:: context
+
+     The :term:`context` will be available as the ``context``
+     attribute of the :term:`request` object.  It will be the context
+     object implied by the current request.  See
+     :ref:`traversal_chapter` for information about context objects.
+
+   .. attribute:: registry
+
+     The :term:`application registry` will be available as the
+     ``registry`` attribute of the :term:`request` object.  See
+     :ref:`zca_chapter` for more information about the application
+     registry.
+
+   .. attribute:: root
+
+      The :term:`root` object will be available as the ``root``
+      attribute of the :term:`request` object.  It will be the model
+      object at which traversal started (the root).  See
+      :ref:`traversal_chapter` for information about root objects.
+
+   .. attribute:: subpath
+
+      The traversal :term:`subpath` will be available as the
+      ``subpath`` attribute of the :term:`request` object.  It will
+      be a sequence containing zero or more elements (which will be
+      Unicode objects).  See :ref:`traversal_chapter` for information
+      about the subpath.
+
+   .. attribute:: traversed
+
+      The "traversal path" will be available as the ``traversed``
+      attribute of the :term:`request` object.  It will be a sequence
+      representing the ordered set of names that were used to
+      traverse to the :term:`context`, not including the view name or
+      subpath.  If there is a virtual root associated with the
+      request, the virtual root path is included within the traversal
+      path.  See :ref:`traversal_chapter` for more information.
+
+   .. attribute:: view_name
+
+      The :term:`view name` will be available as the ``view_name``
+      attribute of the :term:`request` object.  It will be a single
+      string (possibly the empty string if we're rendering a default
+      view).  See :ref:`traversal_chapter` for information about view
+      names.
+
+   .. attribute:: virtual_root
+
+      The :term:`virtual root` will be available as the
+      ``virtual_root`` attribute of the :term:`request` object.  It
+      will be the virtual root object implied by the current request.
+      See :ref:`vhosting_chapter` for more information about virtual
+      roots.
+
+   .. attribute:: virtual_root_path
+
+      The  :term:`virtual  root`  *path*  will be  available  as  the
+      ``virtual_root_path`` attribute  of the :term:`request` object.
+      It will  be a  sequence representing the  ordered set  of names
+      that were  used to  traverse to the  virtual root  object.  See
+      :ref:`vhosting_chapter`  for  more  information  about  virtual
+      roots.
+
+.. autofunction:: make_request_ascii
+
+
diff --git a/docs/narr/webob.rst b/docs/narr/webob.rst
index 7ea55add0..5d69efe51 100644
--- a/docs/narr/webob.rst
+++ b/docs/narr/webob.rst
@@ -13,35 +13,29 @@ Request and Response Objects
 :mod:`repoze.bfg` uses the :term:`WebOb` package to supply
 :term:`request` and :term:`response` object implementations.  The
 :term:`request` object that is passed to a :mod:`repoze.bfg`
-:term:`view` is an instance of the :class:`repoze.bfg.Request` class,
-which is a subclass of :class:`webob.Request`.  The :term:`response`
-returned from a :mod:`repoze.bfg` :term:`view` :term:`renderer` is an
-instance of the :mod:`webob.Response` class.  Users can also return an
-instance of :mod:`webob.Response` directly from a view as necessary.
+:term:`view` is an instance of the :class:`repoze.bfg.request.Request`
+class, which is a subclass of :class:`webob.Request`.  The
+:term:`response` returned from a :mod:`repoze.bfg` :term:`view`
+:term:`renderer` is an instance of the :mod:`webob.Response` class.
+Users can also return an instance of :mod:`webob.Response` directly
+from a view as necessary.
 
 WebOb is a project separate from :mod:`repoze.bfg` with a separate set
 of authors and a fully separate `set of documentation
 <http://pythonpaste.org/webob/>`_.
 
-.. warning:: The following information is only an overview of the
-   request and response objects provided by :term:`WebOb`.  See the
-   `reference documentation
-   <http://pythonpaste.org/webob/reference.html>`_ for more detailed
-   API reference information.  All methods in the :term:`WebOb`
-   documentation work against :mod:`repoze.bfg` requests and
-   responses.
-
 WebOb provides objects for HTTP requests and responses.  Specifically
 it does this by wrapping the `WSGI <http://wsgi.org>`_ request
-environment and response status/headers/app_iter(body).
-
-The request and response objects provide many conveniences for parsing
-HTTP request and forming HTTP responses.  Both objects are read/write:
-as a result, WebOb is also a nice way to create HTTP requests and
-parse HTTP responses; however, we won't cover that use case in this
-document.  The `reference documentation
+environment and response status/headers/app_iter (body).
+
+WebOb request and response objects provide many conveniences for
+parsing WSGI requests and forming WSGI responses.  WebOb is a nice way
+to represent "raw" WSGI requests and responses; however, we won't
+cover that use case in this document, as users of :mod:`repoze.bfg`
+don't typically need to use the WSGI-related features of WebOb
+directly.  The `reference documentation
 <http://pythonpaste.org/webob/reference.html>`_ shows many examples of
-creating requests.
+creating requests and using response objects in this manner, however.
 
 .. index::
    single: request object
@@ -102,9 +96,7 @@ for instance: ``req.accept_language``, ``req.content_length``,
 *parsed* form of each header, for whatever parsing makes sense.  For
 instance, ``req.if_modified_since`` returns a `datetime
 <http://python.org/doc/current/lib/datetime-datetime.html>`_ object
-(or None if the header is was not provided).  Details are in the
-`Request reference
-<http://pythonpaste.org/webob/class-webob.Request.html>`_.
+(or None if the header is was not provided).
 
 .. index::
    single: request attributes (special)
@@ -115,60 +107,11 @@ Special Attributes Added to the Request by :mod:`repoze.bfg`
 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 
 In addition to the standard :term:`WebOb` attributes,
-:mod:`repoze.bfg` adds the following special attributes to every
-request.
-
-``req.context``
-  The :term:`context` will be available as the ``context`` attribute
-  of the :term:`request` object.  It will be the context object
-  implied by the current request.  See :ref:`traversal_chapter` for
-  information about context objects.
-
-``req.registry``
-  The :term:`application registry` will be available as
-  the ``registry`` attribute of the :term:`request` object.  See
-  :ref:`zca_chapter` for more information about the application
-  registry.
-
-``req.root``
-  The :term:`root` object will be available as the ``root`` attribute
-  of the :term:`request` object.  It will be the model object at which
-  traversal started (the root).  See :ref:`traversal_chapter` for
-  information about root objects.
-
-``req.subpath``
-  The traversal :term:`subpath` will be available as the ``subpath``
-  attribute of the :term:`request` object.  It will be a sequence
-  containing zero or more elements (which will be Unicode objects).
-  See :ref:`traversal_chapter` for information about the subpath.
-
-``req.traversed``
-  The "traversal path" will be available as the ``traversed`` attribute of the
-  :term:`request` object.  It will be a sequence representing the
-  ordered set of names that were used to traverse to the
-  :term:`context`, not including the view name or subpath.  If there
-  is a virtual root associated with the request, the virtual root path is
-  included within the traversal path.  See :ref:`traversal_chapter`
-  for more information.
-
-``req.view_name``
-  The :term:`view name` will be available as the ``view_name``
-  attribute of the :term:`request` object.  It will be a single string
-  (possibly the empty string if we're rendering a default view).
-  See :ref:`traversal_chapter` for information about view names.
-
-``req.virtual_root``
-  The :term:`virtual root` will be available as the ``virtual_root``
-  attribute of the :term:`request` object.  It will be the virtual
-  root object implied by the current request.  See
-  :ref:`vhosting_chapter` for more information about virtual roots.
-
-``req.virtual_root_path``
-  The :term:`virtual root` *path* will be available as the
-  ``virtual_root_path`` attribute of the :term:`request` object.  It
-  will be a sequence representing the ordered set of names that were
-  used to traverse to the virtual root object.  See
-  :ref:`vhosting_chapter` for more information about virtual roots.
+:mod:`repoze.bfg` adds special attributes to every request:
+``context``, ``registry``, ``root``, ``subpath``, ``traversed``,
+``view_name``, ``virtual_root`` , and ``virtual_root_path``.  These
+attributes are documented further within the
+:class:`repoze.bfg.request.Request` API documentation.
 
 .. index::
    single: request URLs
@@ -243,6 +186,18 @@ corresponding ``req.str_*`` (like ``req.str_POST``) that is always
 .. index::
    single: response object
 
+More Details
+++++++++++++
+
+More detail about the request object API is available in:
+
+- The :class:`repoze.bfg.request.Request` API documentation.
+
+- The `WebOb documentation <http://pythonpaste.org/webob>`_ .  All
+  methods and attributes of a ``webob.Request`` documented within the
+  WebOb documentation will work against request objects created by
+  :mod:`repoze.bfg`.
+
 Response
 ~~~~~~~~
 
@@ -395,6 +350,17 @@ objects.
 .. index::
    single: multidict (WebOb)
 
+More Details
+++++++++++++
+
+More details about the response object API are available in the `WebOb
+documentation <http://pythonpaste.org/webob>`_ .  All methods and
+attributes of a ``webob.Response`` documented within the WebOb
+documentation will work against response objects created by
+:mod:`repoze.bfg`.  :mod:`repoze.bfg` does not use a Webob Response
+object subclass to represent a response, it uses WebOb's Response
+class directly.
+
 Multidict
 ~~~~~~~~~
 
diff --git a/repoze/bfg/request.py b/repoze/bfg/request.py
index 8939e7b88..c9422e24d 100644
--- a/repoze/bfg/request.py
+++ b/repoze/bfg/request.py
@@ -16,9 +16,74 @@ def make_request_ascii(event):
     request.default_charset = None
 
 class Request(WebobRequest):
+    """
+    A subclass of the :term:`WebOb` Request class.  An instance of
+    this class is created by the :term:`router` and is provided to a
+    view callable (and to other subsystems) as the ``request``
+    argument.
+
+    The documentation below (save for the ``add_response_callback``
+    method, which is defined in this subclass itself, and the
+    attributes ``context``, ``registry``, ``root``, ``subpath``,
+    ``traversed``, ``view_name``, ``virtual_root`` , and
+    ``virtual_root_path``, each of which is added to the request at by
+    the :term:`router` at request ingress time) are autogenerated from
+    the WebOb source code used when this documentation was generated.
+
+    Due to technical constraints, we can't yet display the WebOb
+    version number from which this documentation is autogenerated, but
+    it will be the 'prevailing WebOb version' at the time of the
+    release of this :mod:`repoze.bfg` version.  See
+    `http://http://pythonpaste.org/webob/
+    <http://pythonpaste.org/webob/>`_ for further information.
+    """
     implements(IRequest)
+    response_callbacks = ()
     default_charset = 'utf-8'
 
+    def add_response_callback(self, callback):
+        """
+        Add a callback to the set of callbacks to be called by the
+        :term:`router` at a point after a :term:`response` object is
+        successfully created.  :mod:`repoze.bfg` does not have a
+        global response object: this functionality allows an
+        application to register an action to be performed against the
+        response once one is created.
+
+        A 'callback' is a callable which accepts two positional
+        parameters: ``request`` and ``response``.  For example:
+
+        .. code-block:: python
+           :linenos:
+
+           def cache_callback(request, response):
+               'Set the cache_control max_age for the response'
+               response.cache_control.max_age = 360
+           request.add_response_callback(cache_callback)
+
+        Response callbacks are called in the order they're added
+        (first-to-most-recently-added).  No response callback is
+        called if an exception happens in application code, or if the
+        response object returned by :term:`view` code is invalid.
+
+        All response callbacks are called *before* the
+        :class:`repoze.bfg.interfaces.INewResponse` event is sent.
+
+        Errors raised by callbacks are not handled specially.  They
+        will be propagated to the caller of the :mod:`repoze.bfg`
+        router application.  """
+
+        callbacks = self.response_callbacks
+        if not callbacks:
+            callbacks = []
+        callbacks.append(callback)
+        self.response_callbacks = callbacks
+
+    def _process_response_callbacks(self, response):
+        for callback in self.response_callbacks:
+            callback(self, response)
+        self.response_callbacks = ()
+
     # override default WebOb "environ['adhoc_attr']" mutation behavior
     __getattr__ = object.__getattribute__
     __setattr__ = object.__setattr__
@@ -87,9 +152,10 @@ def route_request_iface(name, bases=()):
     return iface
 
 def add_global_response_headers(request, headerlist):
-    attrs = request.__dict__
-    response_headers = attrs.setdefault('global_response_headers', [])
-    response_headers.extend(headerlist)
+    def add_headers(request, response):
+        for k, v in headerlist:
+            response.headers.add(k, v)
+    request.add_response_callback(add_headers)
 
 from repoze.bfg.threadlocal import get_current_request as get_request # b/c
 
diff --git a/repoze/bfg/router.py b/repoze/bfg/router.py
index ed187240c..8f96bb798 100644
--- a/repoze/bfg/router.py
+++ b/repoze/bfg/router.py
@@ -144,8 +144,6 @@ class Router(object):
                 attrs['exception'] = why
                 response = view_callable(why, request)
 
-            # process the response
-            has_listeners and registry.notify(NewResponse(response))
             try:
                 headers = response.headerlist
                 app_iter = response.app_iter
@@ -155,9 +153,11 @@ class Router(object):
                     'Non-response object returned from view named %s '
                     '(and no renderer): %r' % (view_name, response))
 
-            if 'global_response_headers' in attrs:
-                headers = list(headers)
-                headers.extend(attrs['global_response_headers'])
+            if request.response_callbacks:
+                request._process_response_callbacks(response)
+
+            # process the response
+            has_listeners and registry.notify(NewResponse(response))
 
             start_response(status, headers)
             return app_iter
diff --git a/repoze/bfg/testing.py b/repoze/bfg/testing.py
index d5ae7bd56..5502cb3d2 100644
--- a/repoze/bfg/testing.py
+++ b/repoze/bfg/testing.py
@@ -568,6 +568,7 @@ class DummyRequest(object):
     application_url = 'http://example.com'
     host = 'example.com:80'
     content_length = 0
+    response_callbacks = ()
     def __init__(self, params=None, environ=None, headers=None, path='/',
                  cookies=None, post=None, **kw):
         if environ is None:
@@ -608,6 +609,11 @@ class DummyRequest(object):
         self.registry = get_current_registry()
         self.__dict__.update(kw)
 
+    def add_response_callback(self, callback):
+        if not self.response_callbacks:
+            self.response_callbacks = []
+        self.response_callbacks.append(callback)
+
 def setUp(registry=None, request=None, hook_zca=True):
     """
     Set :mod:`repoze.bfg` registry and request thread locals for the
diff --git a/repoze/bfg/tests/test_authentication.py b/repoze/bfg/tests/test_authentication.py
index a6f34970f..bce80ca20 100644
--- a/repoze/bfg/tests/test_authentication.py
+++ b/repoze/bfg/tests/test_authentication.py
@@ -416,9 +416,11 @@ class TestAuthTktCookieHelper(unittest.TestCase):
         request = self._makeRequest({'HTTP_COOKIE':'auth_tkt=bogus'})
         result = plugin.identify(request)
         self.failUnless(result)
-        response_headers = request.global_response_headers
-        self.assertEqual(len(response_headers), 3)
-        self.assertEqual(response_headers[0][0], 'Set-Cookie')
+        self.assertEqual(len(request.callbacks), 1)
+        response = DummyResponse()
+        request.callbacks[0](None, response)
+        self.assertEqual(len(response.headers.added), 3)
+        self.assertEqual(response.headers.added[0][0], 'Set-Cookie')
 
     def test_remember(self):
         plugin = self._makeOne('secret')
@@ -595,6 +597,10 @@ class DummyContext:
 class DummyRequest:
     def __init__(self, environ):
         self.environ = environ
+        self.callbacks = []
+
+    def add_response_callback(self, callback):
+        self.callbacks.append(callback)
 
 class DummyWhoPlugin:
     def remember(self, environ, identity):
@@ -652,3 +658,14 @@ class DummyAuthTktModule(object):
     class BadTicket(Exception):
         pass
 
+class DummyHeaders:
+    def __init__(self):
+        self.added = []
+
+    def add(self, k, v):
+        self.added.append((k, v))
+
+class DummyResponse:
+    def __init__(self):
+        self.headers = DummyHeaders()
+        
diff --git a/repoze/bfg/tests/test_request.py b/repoze/bfg/tests/test_request.py
index 7b3d0ce7b..8d23e360f 100644
--- a/repoze/bfg/tests/test_request.py
+++ b/repoze/bfg/tests/test_request.py
@@ -157,6 +157,33 @@ class TestRequest(unittest.TestCase):
         result = inst.values()
         self.assertEqual(result, environ.values())
 
+    def test_add_response_callback(self):
+        inst = self._makeOne({})
+        self.assertEqual(inst.response_callbacks, ())
+        def callback(request, response):
+            """ """
+        inst.add_response_callback(callback)
+        self.assertEqual(inst.response_callbacks, [callback])
+        inst.add_response_callback(callback)
+        self.assertEqual(inst.response_callbacks, [callback, callback])
+
+    def test__process_response_callbacks(self):
+        inst = self._makeOne({})
+        def callback1(request, response):
+            request.called1 = True
+            response.called1 = True
+        def callback2(request, response):
+            request.called2  = True
+            response.called2 = True
+        inst.response_callbacks = [callback1, callback2]
+        response = DummyResponse()
+        inst._process_response_callbacks(response)
+        self.assertEqual(inst.called1, True)
+        self.assertEqual(inst.called2, True)
+        self.assertEqual(response.called1, True)
+        self.assertEqual(response.called2, True)
+        self.assertEqual(inst.response_callbacks, ())
+
 class Test_route_request_iface(unittest.TestCase):
     def _callFUT(self, name):
         from repoze.bfg.request import route_request_iface
@@ -175,10 +202,11 @@ class Test_add_global_response_headers(unittest.TestCase):
 
     def test_it(self):
         request = DummyRequest()
-        headers = [('a', 1), ('b', 2)]
-        request.global_response_headers = headers[:]
+        response = DummyResponse()
         self._callFUT(request, [('c', 1)])
-        self.assertEqual(request.global_response_headers, headers + [('c', 1)])
+        self.assertEqual(len(request.response_callbacks), 1)
+        request.response_callbacks[0](None, response)
+        self.assertEqual(response.headers.added,  [('c', 1)] )
 
 class DummyRequest:
     def __init__(self, environ=None):
@@ -186,10 +214,21 @@ class DummyRequest:
             environ = {}
         self.environ = environ
 
+    def add_response_callback(self, callback):
+        self.response_callbacks = [callback]
+
 class DummyNewRequestEvent:
     def __init__(self, request):
         self.request = request
         
-
+class DummyHeaders:
+    def __init__(self):
+        self.added = []
+    def add(self, k, v):
+        self.added.append((k, v))
+
+class DummyResponse:
+    def __init__(self):
+        self.headers = DummyHeaders()
 
 
diff --git a/repoze/bfg/tests/test_router.py b/repoze/bfg/tests/test_router.py
index 6e25b0584..1ce499e41 100644
--- a/repoze/bfg/tests/test_router.py
+++ b/repoze/bfg/tests/test_router.py
@@ -372,7 +372,7 @@ class TestRouter(unittest.TestCase):
         why = exc_raised(NotFound, router, environ, start_response)
         self.assertEqual(why[0], 'notfound')
 
-    def test_call_request_has_global_response_headers(self):
+    def test_call_request_has_response_callbacks(self):
         from zope.interface import Interface
         from zope.interface import directlyProvides
         class IContext(Interface):
@@ -385,15 +385,16 @@ class TestRouter(unittest.TestCase):
         response = DummyResponse('200 OK')
         response.headerlist = [('a', 1)]
         def view(context, request):
-            request.global_response_headers = [('b', 2)]
+            def callback(request, response):
+                response.called_back = True
+            request.response_callbacks = [callback]
             return response
         environ = self._makeEnviron()
         self._registerView(view, '', IViewClassifier, IRequest, IContext)
         router = self._makeOne()
         start_response = DummyStartResponse()
         router(environ, start_response)
-        self.assertEqual(start_response.status, '200 OK')
-        self.assertEqual(start_response.headers, [('a', 1), ('b', 2)])
+        self.assertEqual(response.called_back, True)
 
     def test_call_eventsends(self):
         from repoze.bfg.interfaces import INewRequest
diff --git a/repoze/bfg/tests/test_testing.py b/repoze/bfg/tests/test_testing.py
index b24843e77..b900a44a4 100644
--- a/repoze/bfg/tests/test_testing.py
+++ b/repoze/bfg/tests/test_testing.py
@@ -514,6 +514,12 @@ class TestDummyRequest(unittest.TestCase):
         request = self._makeOne(water = 1)
         self.assertEqual(request.water, 1)
 
+    def test_add_response_callback(self):
+        request = self._makeOne()
+        request.add_response_callback(1)
+        self.assertEqual(request.response_callbacks, [1])
+        
+
 class TestDummyTemplateRenderer(unittest.TestCase):
     def _getTargetClass(self, ):
         from repoze.bfg.testing import DummyTemplateRenderer