- Add ``hook_zca`` and ``unhook_zca`` methods to the ``Configurator``

  API.

- Add roles to configurator API docs.

7 files changed, 165 insertions, 113 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index 03414e8c3..b0f7a614f 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -6,6 +6,9 @@ Features
 
 - Add a ``**kw`` arg to the ``Configurator.add_settings`` API.
 
+- Add ``hook_zca`` and ``unhook_zca`` methods to the ``Configurator``
+  API.
+
 Bug Fixes
 ---------
 
diff --git a/TODO.txt b/TODO.txt
index 6bbaf2d99..7d54da23a 100644
--- a/TODO.txt
+++ b/TODO.txt
@@ -5,5 +5,3 @@
 
 - Decide on ``unhook_zca`` argument to ``tearDown``.
 
-- Hook the ZCA in ``Configurator.begin``.
-
diff --git a/docs/api/configuration.rst b/docs/api/configuration.rst
index 49ad955f2..63f6eb44f 100644
--- a/docs/api/configuration.rst
+++ b/docs/api/configuration.rst
@@ -16,6 +16,10 @@
 
      .. automethod:: end
 
+     .. automethod:: hook_zca()
+
+     .. automethod:: unhook_zca()
+
      .. automethod:: add_renderer(name, factory)
 
      .. automethod:: add_route
@@ -47,3 +51,4 @@
      .. automethod:: testing_add_subscriber
 
      .. automethod:: testing_add_template
+
diff --git a/repoze/bfg/configuration.py b/repoze/bfg/configuration.py
index 8ea7cf09c..9028866a0 100644
--- a/repoze/bfg/configuration.py
+++ b/repoze/bfg/configuration.py
@@ -70,31 +70,34 @@ DEFAULT_RENDERERS = (
 class Configurator(object):
     """
     A Configurator is used to configure a :mod:`repoze.bfg`
-    :term:`application registry`.  The Configurator accepts a number
-    of arguments: ``registry``, ``package``, ``settings``,
-    ``root_factory``, ``zcml_file``, ``authentication_policy``,
-    ``authorization_policy``, ``renderers`` and ``debug_logger``.
+    :term:`application registry`.
+
+    The Configurator accepts a number of arguments: ``registry``,
+    ``package``, ``settings``, ``root_factory``, ``zcml_file``,
+    ``authentication_policy``, ``authorization_policy``, ``renderers``
+    and ``debug_logger``.
 
     If the ``registry`` argument is passed as a non-``None`` value, it
     must be an instance of the :class:`repoze.bfg.registry.Registry`
     class representing the registry to configure.  If ``registry`` is
-    ``None``, the configurator will create a ``Registry`` instance
-    itself; it will also perform some default configuration that would
-    not otherwise be done.  After construction, the configurator may
-    be used to add configuration to the registry.  The overall state
-    of a registry is called the 'configuration state'.
+    ``None``, the configurator will create a
+    :class`repoze.bfg.registry.Registry` instance itself; it will also
+    perform some default configuration that would not otherwise be
+    done.  After construction, the configurator may be used to add
+    configuration to the registry.  The overall state of a registry is
+    called the 'configuration state'.
 
     .. warning:: If a ``registry`` is passed to the Configurator
        constructor, all other constructor arguments except ``package``
        are ignored.
 
     If the ``package`` argument is passed, it must be a reference to a
-    Python package (e.g. ``sys.modules['thepackage']``).  This value
-    is used as a basis to convert relative paths passed to various
-    configuration methods, such as methods which accept a ``renderer``
-    argument, into absolute paths.  If ``None`` is passed (the
-    default), the package is assumed to be the Python package in which
-    the *caller* of the ``Configurator`` constructor lives.
+    Python :term:`package` (e.g. ``sys.modules['thepackage']``).  This
+    value is used as a basis to convert relative paths passed to
+    various configuration methods, such as methods which accept a
+    ``renderer`` argument, into absolute paths.  If ``None`` is passed
+    (the default), the package is assumed to be the Python package in
+    which the *caller* of the ``Configurator`` constructor lives.
 
     If the ``settings`` argument is passed, it should be a Python
     dictionary representing the deployment settings for this
@@ -123,10 +126,10 @@ class Configurator(object):
 
     If ``debug_logger`` is not passed, a default debug logger that
     logs to stderr will be used.  If it is passed, it should be an
-    instance of the ``logging.Logger`` (PEP 282) standard library
+    instance of the :class:`logging.Logger` (PEP 282) standard library
     class.  The debug logger is used by :mod:`repoze.bfg` itself to
-    log warnings and authorization debugging information.
-    """
+    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,
@@ -279,12 +282,40 @@ class Configurator(object):
 
     # API
 
+    # getSiteManager is a unit testing dep injection
+    def hook_zca(self, getSiteManager=None):
+        """ Call :func:`zope.component.getSiteManager.sethook` with
+        the argument
+        :data:`repoze.bfg.threadlocal.get_current_registry`, causing
+        the :term:`Zope Component Architecture` 'global' APIs such as
+        :func:`zope.component.getSiteManager`,
+        :func:`zope.component.getAdapter` and others to use the
+        :mod:`repoze.bfg` :term:`application registry` rather than the
+        Zope 'global' registry.  If :mod:`zope.component` cannot be
+        imported, this method will raise an :exc:`ImportError`."""
+        if getSiteManager is None:
+            from zope.component import getSiteManager
+        getSiteManager.sethook(get_current_registry)
+
+    # getSiteManager is a unit testing dep injection
+    def unhook_zca(self, getSiteManager=None):
+        """ Call :func:`zope.component.getSiteManager.reset` to undo
+        the action of
+        :meth:`repoze.bfg.configuration.Configurator.hook_zca`.  If
+        :mod:`zope.component` cannot be imported, this method will
+        raise an :exc:`ImportError`."""
+        if getSiteManager is None: # pragma: no cover
+            from zope.component import getSiteManager
+        getSiteManager.reset()
+
     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 :mod:`repoze.bfg.threadlocal` API functions."""
+        This pushes a dictionary containing the :term:`application
+        registry` implied by ``registry`` attribute of this
+        configurator and the :term:`request` implied by the
+        ``request`` argument on to the :term:`thread local` stack
+        consulted by various :mod:`repoze.bfg.threadlocal` API
+        functions."""
         self.manager.push({'registry':self.registry, 'request':request})
 
     def end(self):
@@ -313,13 +344,13 @@ class Configurator(object):
         return subscriber
 
     def add_settings(self, settings=None, **kw):
-        """ Add additional settings (beyond the ones passed in as
-        ``settings`` to the constructor of this object) to the
-        dictionarylike object returned from
-        :func:`repoze.bfg.settings.get_settings`.  The ``settings``
-        argument should be a dictionarylike object or ``None``.
-        Arbitrary ``kw`` arguments can be passed in to augment the
-        settings dict."""
+        """ Add additional settings beyond the ones passed in as
+        ``settings`` to the constructor of this object to the
+        dictionarylike object which is returned from
+        :func:`repoze.bfg.settings.get_settings` API.  The
+        ``settings`` argument should be a dictionarylike object or
+        ``None``.  Arbitrary ``kw`` arguments can be passed in to
+        augment the settings dict."""
         if settings is None:
             settings = {}
         utility = self.registry.queryUtility(ISettings)
@@ -435,9 +466,9 @@ class Configurator(object):
 
           When the renderer is a path, although a path is usually just
           a simple relative pathname (e.g. ``templates/foo.pt``,
-          implying that a template named"foo.pt" is in the "templates"
-          directory relative to the directory of the current
-          :term:`package` of the Configurator), a path can be
+          implying that a template named "foo.pt" is in the
+          "templates" directory relative to the directory of the
+          current :term:`package` of the Configurator), a path can be
           absolute, starting with a slash on UNIX or a drive letter
           prefix on Windows.  The path can alternately be a
           :term:`resource specification` in the form
@@ -494,8 +525,8 @@ class Configurator(object):
           the ``route`` configuration referred to by ``route_name``
           usually has a ``*traverse`` token in the value of its
           ``path``, representing a part of the path that will be used
-          by traversal against the result of the route's :term:`root
-          factory`.
+          by :term:`traversal` against the result of the route's
+          :term:`root factory`.
 
           .. warning:: Using this argument services an advanced
              feature that isn't often used unless you want to perform
@@ -508,17 +539,16 @@ class Configurator(object):
           This value should be an :term:`interface` that the
           :term:`request` must provide in order for this view to be
           found and called.  This value exists only for backwards
-          compatibility purposes: it's usually easier to use another
-          predicate.
+          compatibility purposes.
 
         request_method
 
-          This value can either be one of the strings 'GET', 'POST',
-          'PUT', 'DELETE', or 'HEAD' representing an HTTP
-          ``REQUEST_METHOD``.  A view declaration with this argument
-          ensures that the view will only be called when the request's
-          ``method`` (aka ``REQUEST_METHOD``) string matches the
-          supplied value.
+          This value can either be one of the strings ``GET``,
+          ``POST``, ``PUT``, ``DELETE``, or ``HEAD`` representing an
+          HTTP ``REQUEST_METHOD``.  A view declaration with this
+          argument ensures that the view will only be called when the
+          request's ``method`` attribute (aka the ``REQUEST_METHOD`` of
+          the WSGI environment) string matches the supplied value.
 
         request_param
 
@@ -538,15 +568,16 @@ class Configurator(object):
           This value should be a reference to a Python class or
           :term:`interface` that a parent object in the
           :term:`lineage` must provide in order for this view to be
-          found and called.  Your models must be "location-aware" to
-          use this feature.  See :ref:`location_aware` for more
-          information about location-awareness.
+          found and called.  The nodes in your object graph must be
+          "location-aware" to use this feature.  See
+          :ref:`location_aware` for more information about
+          location-awareness.
 
         xhr
 
           This value should be either ``True`` or ``False``.  If this
-          value is specified and is ``True``, the :term:`request` must
-          possess an ``HTTP_X_REQUESTED_WITH`` (aka
+          value is specified and is ``True``, the :term:`request`
+          must possess an ``HTTP_X_REQUESTED_WITH`` (aka
           ``X-Requested-With``) header that has the value
           ``XMLHttpRequest`` for this view to be found and called.
           This is useful for detecting AJAX requests issued from
@@ -991,8 +1022,8 @@ class Configurator(object):
         will influence the current configuration state.
 
         The ``package`` argument should be a reference to a Python
-        package or module object.  If ``package`` is ``None``, the
-        package of the *caller* is used.
+        :term:`package` or module object.  If ``package`` is ``None``,
+        the package of the *caller* is used.
         """
         if package is None: # pragma: no cover
             package = caller_package()
@@ -1024,8 +1055,12 @@ class Configurator(object):
         The ``factory`` argument is Python reference to an
         implementation of a :term:`renderer` factory.
 
-        Note that this function should be called *before* any
-        ``add_view`` invocation that names the renderer name as an argument.
+        Note that this function must be called *before* any
+        ``add_view`` invocation that names the renderer name as an
+        argument.  As a result, it's usually a better idea to pass
+        globally used renderers into the ``Configurator`` constructor
+        in the sequence of renderers passed as ``renderer`` than it is
+        to use this method.
         """
         self.registry.registerUtility(
             factory, IRendererFactory, name=name, info=_info)
@@ -1090,8 +1125,10 @@ class Configurator(object):
 
         The ``renderer`` argument should be the name of (or path to) a
         :term:`renderer` used to generate a response for this view
-        (see the ``add_view`` method's ``renderer`` argument for a
-        description).
+        (see the
+        :meth:`repoze.bfg.configuration.Configurator.add_view`
+        method's ``renderer`` argument for information about how a
+        configurator relates to a renderer).
 
         The ``wrapper`` argument should be the name of another view
         which will wrap this view when rendered (see the ``add_view``
@@ -1114,8 +1151,10 @@ class Configurator(object):
 
         The ``renderer`` argument should be the name of (or path to) a
         :term:`renderer` used to generate a response for this view
-        (see the ``add_view`` method's ``renderer`` argument for a
-        description).
+        (see the
+        :meth:`repoze.bfg.configuration.Configurator.add_view`
+        method's ``renderer`` argument for information about how a
+        configurator relates to a renderer).
 
         The ``wrapper`` argument should be the name of another view
         which will wrap this view when rendered (see the ``add_view``
@@ -1137,8 +1176,7 @@ class Configurator(object):
         files reside.  This can be an absolute path, a
         package-relative path, or a :term:`resource specification`.
 
-        See :ref:`static_resources_section` for more information (the
-        ZCML directive just calls this method).
+        See :ref:`static_resources_section` for more information.
         """
         spec = self._make_spec(path)
         view = static(spec, cache_max_age=cache_max_age)
@@ -1216,12 +1254,14 @@ class Configurator(object):
         return L
 
     def testing_add_template(self, path, renderer=None):
-        """ Register a template tenderer at ``path`` (usually a relative
-        filename ala ``templates/foo.pt``) and return the renderer object.
-        If the ``renderer`` argument is None, a 'dummy' renderer will be
-        used.  This function is useful when testing code that calls the
-        ``render_template_to_response`` or any other ``render_template*``
-        API of any of the built-in templating systems."""
+        """ Register a template tenderer at ``path`` (usually a
+        relative filename ala ``templates/foo.pt``) and return the
+        renderer object.  If the ``renderer`` argument is None, a
+        'dummy' renderer will be used.  This function is useful when
+        testing code that calls the ``render_template_to_response`` or
+        any other ``render_template*`` API of any of the built-in
+        templating systems (see :mod:`repoze.bfg.chameleon_zpt` and
+        :mod:`repoze.bfg.chameleon_text`)."""
         from repoze.bfg.testing import DummyTemplateRenderer
         if renderer is None:
             renderer = DummyTemplateRenderer()
@@ -1674,16 +1714,17 @@ def _accept_wrap(view, accept):
     return accept_view
 
 # note that ``options`` is a b/w compat alias for ``settings`` and
-# ``Configurator`` and getSiteManager is a testing dep inj
+# ``Configurator`` and ``getSiteManager`` are testing dep injs
 def make_app(root_factory, package=None, filename='configure.zcml',
-             settings=None, options=None, Configurator=Configurator,
-             getSiteManager=None):
+             settings=None, options=None, Configurator=Configurator):
     """ Return a Router object, representing a fully configured
-    ``repoze.bfg`` WSGI application.
+    :mod:`repoze.bfg` WSGI application.
 
     .. warning:: Use of this function is deprecated as of
        :mod:`repoze.bfg` 1.2.  You should instead use a
-       ``Configurator`` as shown in :ref:`configuration_narr`.
+       :class:`repoze.bfg.configuration.Configurator` instance to
+       perform startup configuration as shown in
+       :ref:`configuration_narr`.
 
     ``root_factory`` must be a callable that accepts a :term:`request`
     object and which returns a traversal root object.  The traversal
@@ -1692,21 +1733,21 @@ def make_app(root_factory, package=None, filename='configure.zcml',
     ``None``, in which case a 'default default' traversal root is
     used.
 
-    ``package`` is a Python module representing the application's
-    package.  It is optional, defaulting to ``None``.  ``package`` may
-    be ``None``.  If ``package`` is ``None``, the ``filename`` passed
-    or the value in the ``options`` dictionary named
-    ``configure_zcml`` must be a) absolute pathname to a ZCML file
-    that represents the application's configuration *or* b) a
-    'specification' in the form
-    ``dotted.package.name:relative/file/path.zcml``.
+    ``package`` is a Python :term:`package` or module representing the
+    application's package.  It is optional, defaulting to ``None``.
+    ``package`` may be ``None``.  If ``package`` is ``None``, the
+    ``filename`` passed or the value in the ``options`` dictionary
+    named ``configure_zcml`` must be a) absolute pathname to a
+    :term:`ZCML` file that represents the application's configuration
+    *or* b) a :term:`resource specification` to a :term:`ZCML` file in
+    the form ``dotted.package.name:relative/file/path.zcml``.
 
     ``filename`` is the filesystem path to a ZCML file (optionally
     relative to the package path) that should be parsed to create the
     application registry.  It defaults to ``configure.zcml``.  It can
-    also be a 'specification' in the form
+    also be a ;term:`resource specification` in the form
     ``dotted_package_name:relatve/file/path.zcml``. Note that if any
-    value for ``configure_zcml`` is passed within the ``options``
+    value for ``configure_zcml`` is passed within the ``settings``
     dictionary, the value passed as ``filename`` will be ignored,
     replaced with the ``configure_zcml`` value.
 
@@ -1722,9 +1763,7 @@ def make_app(root_factory, package=None, filename='configure.zcml',
     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)
+    config.hook_zca()
     config.begin()
     config.load_zcml(zcml_file)
     config.end()
diff --git a/repoze/bfg/testing.py b/repoze/bfg/testing.py
index 1884fe7f6..11127a65c 100644
--- a/repoze/bfg/testing.py
+++ b/repoze/bfg/testing.py
@@ -628,9 +628,8 @@ def setUp(registry=None, request=None, hook_zca=True):
     if registry is None:
         registry = Registry('testing')
     config = Configurator(registry=registry)
+    hook_zca and config.hook_zca()
     config.begin(request=request)
-    if hook_zca:
-        hook_zca_api()
 
 def tearDown(unhook_zca=True):
     """Undo the effects ``repoze.bfg.testing.setUp``.  Use this
@@ -657,7 +656,11 @@ def tearDown(unhook_zca=True):
 
     """
     if unhook_zca:
-        unhook_zca_api()
+        try:
+            from zope.component import getSiteManager
+            getSiteManager.reset()
+        except ImportError: # pragma: no cover
+            pass
     info = manager.pop()
     manager.clear()
     if info is not None:
@@ -680,17 +683,3 @@ 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/test_configuration.py b/repoze/bfg/tests/test_configuration.py
index 3b7c50f60..594f8e8f4 100644
--- a/repoze/bfg/tests/test_configuration.py
+++ b/repoze/bfg/tests/test_configuration.py
@@ -1908,6 +1908,21 @@ class ConfiguratorTests(unittest.TestCase):
         config.registry.notify(event2)
         self.assertEqual(L[-1], event2)
 
+    def test_hook_zca(self):
+        from repoze.bfg.threadlocal import get_current_registry
+        gsm = DummyGetSiteManager()
+        config = self._makeOne()
+        config.hook_zca(getSiteManager=gsm)
+        self.assertEqual(gsm.hook, get_current_registry)
+        
+    def test_unhook_zca(self):
+        gsm = DummyGetSiteManager()
+        config = self._makeOne()
+        config.unhook_zca(getSiteManager=gsm)
+        self.assertEqual(gsm.unhooked, True)
+
+
+
 class Test__map_view(unittest.TestCase):
     def setUp(self):
         from repoze.bfg.registry import Registry
@@ -2744,17 +2759,14 @@ class TestMakeApp(unittest.TestCase):
         return make_app(*arg, **kw)
 
     def test_it(self):
-        from repoze.bfg.threadlocal import get_current_registry
         settings = {'a':1}
         rootfactory = object()
-        gsm = DummyGetSiteManager()
         app = self._callFUT(rootfactory, settings=settings,
-                            Configurator=DummyConfigurator,
-                            getSiteManager=gsm)
+                            Configurator=DummyConfigurator)
         self.assertEqual(app.root_factory, rootfactory)
         self.assertEqual(app.settings, settings)
         self.assertEqual(app.zcml_file, 'configure.zcml')
-        self.assertEqual(gsm.hook, get_current_registry)
+        self.assertEqual(app.zca_hooked, True)
 
     def test_it_options_means_settings(self):
         settings = {'a':1}
@@ -2779,7 +2791,6 @@ class TestMakeApp(unittest.TestCase):
                             Configurator=DummyConfigurator)
         self.assertEqual(app.zcml_file, '2.zcml')
 
-
 class DummyRequest:
     subpath = ()
     def __init__(self):
@@ -2857,8 +2868,9 @@ class DummyConfigurator(object):
         self.package = package
         self.settings = settings
 
-    def begin(self):
+    def begin(self, request=None):
         self.begun = True
+        self.request = request
 
     def end(self):
         self.ended = True
@@ -2868,6 +2880,9 @@ class DummyConfigurator(object):
     
     def make_wsgi_app(self):
         return self
+
+    def hook_zca(self):
+        self.zca_hooked = True
     
 class DummyAccept(object):
     def __init__(self, *matches):
@@ -2899,6 +2914,8 @@ class DummyMultiView:
 class DummyGetSiteManager(object):
     def sethook(self, hook):
         self.hook = hook
+    def reset(self):
+        self.unhooked = True
     
 class DummyThreadLocalManager(object):
     pushed = None
diff --git a/repoze/bfg/tests/test_settings.py b/repoze/bfg/tests/test_settings.py
index 258571dda..c86c21de2 100644
--- a/repoze/bfg/tests/test_settings.py
+++ b/repoze/bfg/tests/test_settings.py
@@ -1,5 +1,4 @@
 import unittest
-from repoze.bfg.testing import cleanUp
 
 class TestSettings(unittest.TestCase):
     def _getTargetClass(self):
@@ -148,10 +147,14 @@ class TestSettings(unittest.TestCase):
 
 class TestGetSettings(unittest.TestCase):
     def setUp(self):
-        cleanUp()
+        from repoze.bfg.configuration import Configurator
+        from repoze.bfg.registry import Registry
+        registry = Registry('testing')
+        self.config = Configurator(registry=registry)
+        self.config.begin()
 
     def tearDown(self):
-        cleanUp()
+        self.config.end()
         
     def _callFUT(self):
         from repoze.bfg.settings import get_settings
@@ -162,9 +165,7 @@ class TestGetSettings(unittest.TestCase):
 
     def test_it_withsettings(self):
         from repoze.bfg.interfaces import ISettings
-        from repoze.bfg.threadlocal import get_current_registry
-        reg = get_current_registry()
         settings = {'a':1}
-        reg.registerUtility(settings, ISettings)
+        self.config.registry.registerUtility(settings, ISettings)
         self.assertEqual(self._callFUT(), settings)