repoze.bfg.request.get_request -> repoze.bfg.threadlocal.get_current_request

repoze.bfg.registry.get_registry -> repoze.bfg.threadlocal.get_current_registry

Remove getSiteManager from registry module.

7 files changed, 172 insertions, 155 deletions

diff --git a/repoze/bfg/registry.py b/repoze/bfg/registry.py
index b2fa67442..bc52e297b 100644
--- a/repoze/bfg/registry.py
+++ b/repoze/bfg/registry.py
@@ -4,13 +4,12 @@ import zope.component
 
 from zope.component import getGlobalSiteManager
 from zope.component import getSiteManager as original_getSiteManager
-from zope.component.interfaces import ComponentLookupError
-from zope.component.interfaces import IComponentLookup
 from zope.component.registry import Components
 
 from zope.deprecation import deprecated
 
 from repoze.bfg.threadlocal import manager
+from repoze.bfg.threadlocal import get_current_registry
 
 from repoze.bfg.zcml import zcml_configure
 
@@ -36,10 +35,8 @@ class Registry(Components):
             for ignored in self.subscribers(events, None):
                 """ """
 
-def get_registry():
-    return manager.get()['registry']
-
-def populateRegistry(registry, filename, package, lock=threading.Lock()):
+def populateRegistry(registry, filename, package, lock=threading.Lock(),
+                     manager=manager): # lock and manager for testing
 
     """ We push our ZCML-defined configuration into an app-local
     component registry in order to allow more than one bfg app to live
@@ -58,23 +55,14 @@ def populateRegistry(registry, filename, package, lock=threading.Lock()):
     lock.acquire()
     manager.push({'registry':registry, 'request':None})
     try:
-        original_getSiteManager.sethook(getSiteManager)
-        zope.component.getGlobalSiteManager = get_registry
+        original_getSiteManager.sethook(get_current_registry)
+        zope.component.getGlobalSiteManager = get_current_registry
         zcml_configure(filename, package)
     finally:
         zope.component.getGlobalSiteManager = getGlobalSiteManager
         lock.release()
         manager.pop()
 
-def getSiteManager(context=None):
-    if context is None:
-        return manager.get()['registry']
-    else:
-        try:
-            return IComponentLookup(context)
-        except TypeError, error:
-            raise ComponentLookupError(*error.args)
-
 class FakeRegistryManager(object):
     manager = manager # for unit tests
     def push(self, registry):
@@ -103,3 +91,19 @@ deprecated('registry_manager',
            'within a "debug" script of your own making, use the ``bfgshell`` '
            'paster command instead. ``registry_manager`` will disappear in '
            'a later release of repoze.bfg')
+
+getSiteManager = get_current_registry # b/c
+
+deprecated('getSiteManager',
+           'As of repoze.bfg 1.0, any import of getSiteManager from'
+           '``repoze.bfg.registry`` is '
+           'deprecated.  Use ``from zope.compponent import getSiteManager '
+           'instead.')
+
+get_registry = get_current_registry # b/c
+
+deprecated('get_registry',
+           'As of repoze.bfg 1.0, any import of get_registry from'
+           '``repoze.bfg.registry`` is '
+           'deprecated.  Use ``from repoze.bfg.threadlocal import '
+           'get_current_registry instead.')
diff --git a/repoze/bfg/request.py b/repoze/bfg/request.py
index e5482848b..e26ed1b5f 100644
--- a/repoze/bfg/request.py
+++ b/repoze/bfg/request.py
@@ -1,6 +1,7 @@
 from zope.interface import implements
 from webob import Request as WebobRequest
 
+from zope.deprecation import deprecated
 from zope.interface.interface import InterfaceClass
 
 from repoze.bfg.interfaces import IRequest
@@ -10,76 +11,6 @@ from repoze.bfg.interfaces import IPUTRequest
 from repoze.bfg.interfaces import IDELETERequest
 from repoze.bfg.interfaces import IHEADRequest
 
-from repoze.bfg.threadlocal import manager
-
-def current_request():
-    """Return the currently active request or ``None`` if no request
-    is currently active.  This is *not* an official API, but it's
-    going to live here 'forever' and so can be relied on to exist.
-
-    **This function should be used extremely sparingly** (read: almost
-    never), because its usage makes it possible to write code that can
-    be neither easily tested nor scripted.  The author of this
-    function reserves the right to point and laugh at code which uses
-    it inappropriately.  Inappropriate usage is defined as follows:
-
-    - This function should never be called within :term:`view` code.
-      View code already has access to the request (it's passed in).
-
-    - This function should never be called in :term:`model` code.
-      Model code should never require any access to the request; if
-      your model code requires access to a request object, you've
-      almost certainly factored something wrong, and you should change
-      your code rather than using this function.
-
-    - This function should never be called within application-specific
-      forks of third-party library code.  The library you've forked
-      almost certainly has nothing to do with repoze.bfg, and making
-      it dependent on repoze.bfg (rather than making your repoze.bfg
-      application depend upon it) means you're forming a dependency in
-      the wrong direction.
-
-    - This function should never be called because it's 'easier' or
-      'more elegant' to think about calling it than to pass a request
-      through a series of function calls when creating some API
-      design.  Your application should instead almost certainly pass
-      data derived from the request around rather than relying on
-      being able to call this function to obtain the request in places
-      that actually have no business knowing about it.  Parameters are
-      meant to be passed around as function arguments, not obtained
-      from some pseudo-global.  Don't try to 'save typing' or create
-      'nicer APIs' by using this function in the place where a request
-      is required; this will only lead to sadness later.
-
-    However, this function *is* still useful in very limited
-    circumstances.  As a rule of thumb, usage of ``current_request``
-    is useful **within code which is meant to eventually be removed**.
-    For instance, you may find yourself wanting to deprecate some API
-    that expects to be passed a request object in favor of one that
-    does not expect to be passed a request object.  But you need to
-    keep implementations of the old API working for some period of
-    time while you deprecate the older API.  So you write a 'facade'
-    implementation of the new API which calls into the code which
-    implements the older API.  Since the new API does not require the
-    request, your facade implementation doesn't have local access to
-    the request when it needs to pass it into the older API
-    implementaton.  After some period of time, the older
-    implementation code is disused and the hack that uses
-    ``current_request`` is removed.  This would be an appropriate
-    place to use the ``current_request`` function.
-
-    ``current_request`` retrieves a request object from a thread-local
-    stack that is managed by a :term:`Router` object.  Therefore the
-    very definition of 'current request' is defined entirely by the
-    behavior of a repoze.bfg Router.  Scripts which use
-    :mod:`repoze.bfg` machinery but never actually start a WSGI server
-    or receive requests via HTTP (such as scripts which use the
-    :mod:`repoze.bfg.scripting`` API) will never cause any Router code
-    to be executed.  Such scripts should expect this function to
-    always return ``None``.
-    """
-    return manager.get()['request']
-
 def request_factory(environ):
     try:
         method = environ['REQUEST_METHOD']
@@ -174,3 +105,10 @@ def named_request_factories(name=None):
 
 DEFAULT_REQUEST_FACTORIES = named_request_factories()
 
+from repoze.bfg.threadlocal import get_current_request as get_request # b/c
+
+deprecated('get_request',
+           'As of repoze.bfg 1.0, any import of get_request from'
+           '``repoze.bfg.request`` is '
+           'deprecated.  Use ``from repoze.bfg.threadlocal import '
+           'get_current_request instead.')
diff --git a/repoze/bfg/secpols.py b/repoze/bfg/secpols.py
index f697a41aa..407988a02 100644
--- a/repoze/bfg/secpols.py
+++ b/repoze/bfg/secpols.py
@@ -6,7 +6,7 @@ from repoze.bfg.interfaces import IAuthorizationPolicy
 from repoze.bfg.interfaces import IAuthenticationPolicy
 
 from repoze.bfg.location import lineage
-from repoze.bfg.request import current_request
+from repoze.bfg.threadlocal import get_current_request
 
 from repoze.bfg.security import Allow
 from repoze.bfg.security import Deny
@@ -434,7 +434,7 @@ class SecurityPolicyToAuthorizationPolicyAdapter(object):
         self.secpol = secpol
 
     def permits(self, context, principals, permission):
-        request = current_request()
+        request = get_current_request()
         return self.secpol.permits(context, request, permission)
 
     def principals_allowed_by_permission(self, context, permission):
diff --git a/repoze/bfg/tests/test_registry.py b/repoze/bfg/tests/test_registry.py
index 3ac0dab88..1d29aef73 100644
--- a/repoze/bfg/tests/test_registry.py
+++ b/repoze/bfg/tests/test_registry.py
@@ -54,54 +54,17 @@ class TestPopulateRegistry(unittest.TestCase):
         from repoze.bfg.tests import fixtureapp
         dummylock = DummyLock()
         dummyregmgr = DummyThreadLocalManager({'registry':None})
-        import repoze.bfg.threadlocal
-        try:
-            old = repoze.bfg.threadlocal.manager
-            repoze.bfg.threadlocal.manager = dummyregmgr
-            from zope.component.registry import Components
-            registry = Components('hello')
-            self._callFUT(registry,
-                          'configure.zcml',
-                          fixtureapp,
-                          lock=dummylock)
-            self.assertEqual(dummylock.acquired, True)
-            self.assertEqual(dummylock.released, True)
-            self.assertEqual(dummyregmgr.data['registry'], None)
-        finally:
-            repoze.bfg.threadlocal.manager = old
-
-class GetSiteManagerTests(unittest.TestCase):
-    def _callFUT(self, context=None):
-        from repoze.bfg.registry import getSiteManager
-        return getSiteManager(context)
-
-    def test_no_context(self):
-        from zope.component import getGlobalSiteManager
-        self.assertEqual(self._callFUT(), getGlobalSiteManager())
-    
-    def test_with_context(self):
-        from zope.component.interfaces import ComponentLookupError
-        self.assertRaises(ComponentLookupError, self._callFUT, object)
-
-class GetRegistryTests(unittest.TestCase):
-    def setUp(self):
-        cleanUp()
-
-    def tearDown(self):
-        cleanUp()
-        
-    def _callFUT(self):
-        from repoze.bfg.registry import get_registry
-        return get_registry()
+        from zope.component.registry import Components
+        registry = Components('hello')
+        self._callFUT(registry,
+                      'configure.zcml',
+                      fixtureapp,
+                      lock=dummylock,
+                      manager=dummyregmgr)
+        self.assertEqual(dummylock.acquired, True)
+        self.assertEqual(dummylock.released, True)
+        self.assertEqual(dummyregmgr.data['registry'], None)
 
-    def test_it(self):
-        from repoze.bfg.threadlocal import manager
-        try:
-            manager.push({'registry':123})
-            self.assertEqual(self._callFUT(), 123)
-        finally:
-            manager.pop()
-    
 class TestFakeRegistry(unittest.TestCase):
     def _getTargetClass(self):
         from repoze.bfg.registry import FakeRegistryManager
diff --git a/repoze/bfg/tests/test_request.py b/repoze/bfg/tests/test_request.py
index 9cae6643a..557e1db71 100644
--- a/repoze/bfg/tests/test_request.py
+++ b/repoze/bfg/tests/test_request.py
@@ -37,25 +37,6 @@ class TestSubclassedRequest(unittest.TestCase):
         request.charset = None
         self.assertEqual(request.GET['la'], 'La Pe\xc3\xb1a')
 
-class TestCurrentRequest(unittest.TestCase):
-    def _callFUT(self):
-        from repoze.bfg.request import current_request
-        return current_request()
-
-    def test_it_None(self):
-        request = self._callFUT()
-        self.assertEqual(request, None)
-
-    def test_it(self):
-        from repoze.bfg.threadlocal import manager
-        request = DummyRequest()
-        try:
-            manager.push({'request':request})
-            self.assertEqual(self._callFUT(), request)
-        finally:
-            manager.pop()
-        self.assertEqual(self._callFUT(), None)
-
 class TestRequestFactory(unittest.TestCase):
     def _callFUT(self, environ):
         from repoze.bfg.request import request_factory
diff --git a/repoze/bfg/tests/test_threadlocal.py b/repoze/bfg/tests/test_threadlocal.py
index 230bb3726..2cf405508 100644
--- a/repoze/bfg/tests/test_threadlocal.py
+++ b/repoze/bfg/tests/test_threadlocal.py
@@ -44,3 +44,46 @@ class TestThreadLocalManager(unittest.TestCase):
         local.clear()
         self.assertEqual(local.get(), 1)
 
+
+class TestGetCurrentRequest(unittest.TestCase):
+    def _callFUT(self):
+        from repoze.bfg.threadlocal import get_current_request
+        return get_current_request()
+
+    def test_it_None(self):
+        request = self._callFUT()
+        self.assertEqual(request, None)
+
+    def test_it(self):
+        from repoze.bfg.threadlocal import manager
+        request = object()
+        try:
+            manager.push({'request':request})
+            self.assertEqual(self._callFUT(), request)
+        finally:
+            manager.pop()
+        self.assertEqual(self._callFUT(), None)
+
+class GetCurrentRegistryTests(unittest.TestCase):
+    def setUp(self):
+        cleanUp()
+
+    def tearDown(self):
+        cleanUp()
+        
+    def _callFUT(self):
+        from repoze.bfg.threadlocal import get_current_registry
+        return get_current_registry()
+
+    def test_local(self):
+        from repoze.bfg.threadlocal import manager
+        try:
+            manager.push({'registry':123})
+            self.assertEqual(self._callFUT(), 123)
+        finally:
+            manager.pop()
+    
+    def test_global(self):
+        from zope.component import getGlobalSiteManager
+        self.assertEqual(self._callFUT(), getGlobalSiteManager())
+    
diff --git a/repoze/bfg/threadlocal.py b/repoze/bfg/threadlocal.py
index 613ea4b65..a300f13b7 100644
--- a/repoze/bfg/threadlocal.py
+++ b/repoze/bfg/threadlocal.py
@@ -31,3 +31,91 @@ def defaults():
     return defaults
 
 manager = ThreadLocalManager(defaults)
+
+## **The below function ``get_current*`` functions are special.  They
+## are not part of the official BFG API, however, they're guaranteed
+## to live here "forever", so they may be relied on in emergencies.
+## However, they should be used extremely sparingly** (read: almost
+## never).
+
+## In particular, it's almost always usually a mistake to use
+## ``get_current_request`` because its usage makes it possible to
+## write code that can be neither easily tested nor scripted.  The
+## author of this function reserves the right to point and laugh at
+## code which uses this function inappropriately.  Inappropriate usage
+## is defined as follows:
+
+## - ``get_current_request`` should never be called within
+##   :term:`view` code, or code called by view code.  View code
+##   already has access to the request (it's passed in).
+
+## - ``get_current_request`` should never be called in :term:`model`
+##   code.  Model code should never require any access to the
+##   request; if your model code requires access to a request object,
+##   you've almost certainly factored something wrong, and you should
+##   change your code rather than using this function.
+
+## - The ``get_current_request`` function should never be called
+##   because it's 'easier' or 'more elegant' to think about calling
+##   it than to pass a request through a series of function calls
+##   when creating some API design.  Your application should instead
+##   almost certainly pass data derived from the request around
+##   rather than relying on being able to call this function to
+##   obtain the request in places that actually have no business
+##   knowing about it.  Parameters are meant to be passed around as
+##   function arguments, not obtained from some pseudo-global.  Don't
+##   try to 'save typing' or create 'nicer APIs' by using this
+##   function in the place where a request is required; this will
+##   only lead to sadness later.
+
+## - Neither ``get_current_request`` nor ``get_current_registry``
+##   should never be called within application-specific forks of
+##   third-party library code.  The library you've forked almost
+##   certainly has nothing to do with repoze.bfg, and making it
+##   dependent on repoze.bfg (rather than making your repoze.bfg
+##   application depend upon it) means you're forming a dependency in
+##   the wrong direction.
+
+## The ``get_current_request`` function *is* still useful in very
+## limited circumstances.  As a rule of thumb, usage of
+## ``get_current_request`` is useful **within code which is meant to
+## eventually be removed**.  For instance, you may find yourself
+## wanting to deprecate some API that expects to be passed a request
+## object in favor of one that does not expect to be passed a request
+## object.  But you need to keep implementations of the old API
+## working for some period of time while you deprecate the older API.
+## So you write a 'facade' implementation of the new API which calls
+## into the code which implements the older API.  Since the new API
+## does not require the request, your facade implementation doesn't
+## have local access to the request when it needs to pass it into the
+## older API implementaton.  After some period of time, the older
+## implementation code is disused and the hack that uses
+## ``get_current_request`` is removed.  This would be an appropriate
+## place to use the ``get_current_request`` function.
+
+## ``get_current_request`` retrieves a request object from a
+## thread-local stack that is managed by a :term:`Router` object.
+## Therefore the very definition of 'current request' is defined
+## entirely by the behavior of a repoze.bfg Router.  Scripts which
+## use :mod:`repoze.bfg` machinery but never actually start a WSGI
+## server or receive requests via HTTP (such as scripts which use the
+## :mod:`repoze.bfg.scripting`` API) will never cause any Router code
+## to be executed.  Such scripts should expect this function to
+## always return ``None``.
+
+## ``get_current_registry`` is mostly non-useful if you use the ZCA
+## API zope.component.getSiteManager will call it for you.
+
+def get_current_request():
+    """Return the currently active request or ``None`` if no request
+    is currently active.  This is *not* an official API.
+    """
+    return manager.get()['request']
+
+def get_current_registry(context=None): # context required by getSiteManager API
+    """Return the currently active component registry or the global
+    component registry if no request is currently active.  This is
+    *not* an official API.
+    """
+    return manager.get()['registry']
+