From 3d16dee6c98960889c85beedfc03e3d895b4a8a4 Mon Sep 17 00:00:00 2001 From: Chris McDonough Date: Tue, 2 Jun 2009 02:44:00 +0000 Subject: - Add ``repoze.bfg.request.current_request`` function. This function should be used (**very sparingly**) to retrieve the "current" request. See the ``repoze.bfg.request`` API documentation for more information. --- CHANGES.txt | 11 ++++++- docs/api/request.rst | 7 ++++ repoze/bfg/request.py | 69 ++++++++++++++++++++++++++++++++++++++++ repoze/bfg/secpols.py | 5 ++- repoze/bfg/tests/test_request.py | 19 +++++++++++ 5 files changed, 107 insertions(+), 4 deletions(-) create mode 100644 docs/api/request.rst diff --git a/CHANGES.txt b/CHANGES.txt index e7ce02bc2..8e5d33465 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,10 +1,19 @@ Next release ============ +Features +-------- + +- Add ``repoze.bfg.request.current_request`` function. This function + should be used (**very sparingly**) to retrieve the "current" + request. See the ``repoze.bfg.request`` API documentation for more + information. + Bug Fixes --------- -- Restored missing entry point declaration for bfg_alchemy paster template, which was accidentally removed in 0.9. +- Restored missing entry point declaration for bfg_alchemy paster + template, which was accidentally removed in 0.9. Documentation ------------- diff --git a/docs/api/request.rst b/docs/api/request.rst new file mode 100644 index 000000000..59463fb22 --- /dev/null +++ b/docs/api/request.rst @@ -0,0 +1,7 @@ +.. _request_module: + +:mod:`repoze.bfg.request` +------------------------- + +.. autofunction:: repoze.bfg.request.current_request + diff --git a/repoze/bfg/request.py b/repoze/bfg/request.py index 44a232d0d..27e60c1d1 100644 --- a/repoze/bfg/request.py +++ b/repoze/bfg/request.py @@ -3,6 +3,75 @@ from webob import Request as WebobRequest import repoze.bfg.interfaces +from repoze.bfg.threadlocal import manager + +def current_request(): + """Return the currently active request or ``None`` if no request + is currently active. + + **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 make_request_ascii(event): """ An event handler that causes the request charset to be ASCII; used as an INewRequest subscriber so code written before 0.7.0 can diff --git a/repoze/bfg/secpols.py b/repoze/bfg/secpols.py index a34bcae77..f697a41aa 100644 --- a/repoze/bfg/secpols.py +++ b/repoze/bfg/secpols.py @@ -6,8 +6,7 @@ from repoze.bfg.interfaces import IAuthorizationPolicy from repoze.bfg.interfaces import IAuthenticationPolicy from repoze.bfg.location import lineage - -from repoze.bfg.threadlocal import manager +from repoze.bfg.request import current_request from repoze.bfg.security import Allow from repoze.bfg.security import Deny @@ -435,7 +434,7 @@ class SecurityPolicyToAuthorizationPolicyAdapter(object): self.secpol = secpol def permits(self, context, principals, permission): - request = manager.get()['request'] + request = current_request() return self.secpol.permits(context, request, permission) def principals_allowed_by_permission(self, context, permission): diff --git a/repoze/bfg/tests/test_request.py b/repoze/bfg/tests/test_request.py index f2124d4c9..4771c3e1c 100644 --- a/repoze/bfg/tests/test_request.py +++ b/repoze/bfg/tests/test_request.py @@ -37,6 +37,25 @@ 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 DummyRequest: pass -- cgit v1.2.3