From 947b8bb21235cdaaa7d1b203ef74c814a59c31ed Mon Sep 17 00:00:00 2001 From: Chris McDonough Date: Thu, 18 Jun 2009 07:30:21 +0000 Subject: 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. --- repoze/bfg/registry.py | 38 +++++++++------- repoze/bfg/request.py | 78 ++++---------------------------- repoze/bfg/secpols.py | 4 +- repoze/bfg/tests/test_registry.py | 57 ++++------------------- repoze/bfg/tests/test_request.py | 19 -------- repoze/bfg/tests/test_threadlocal.py | 43 ++++++++++++++++++ repoze/bfg/threadlocal.py | 88 ++++++++++++++++++++++++++++++++++++ 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'] + -- cgit v1.2.3