From 1646770c4eee27f37e53dcb50f8a271d5c278abf Mon Sep 17 00:00:00 2001 From: Chris McDonough Date: Thu, 29 Oct 2009 23:07:48 +0000 Subject: - The ``repoze.bfg.request.Request`` class, which is a subclass of ``webob.Request`` now defines its own ``__setattr__``, ``__getattr__`` and ``__delattr__`` methods, which override the default WebOb behavior. The default WebOb behavior stores attributes of the request in ``self.environ['webob.adhoc_attrs']``, and retrieves them from that dictionary during a ``__getattr__``. This behavior was undesirable for speed and "expectation" reasons. Now attributes of the ``request`` are stored in ``request.__dict__`` (as you otherwise might expect from an object that did not override these methods). - The router no longer calls ``repoze.bfg.traversal._traverse`` and does its work "inline" (speed). --- CHANGES.txt | 17 +++++++++++ repoze/bfg/request.py | 16 ++++++++-- repoze/bfg/router.py | 14 ++++----- repoze/bfg/tests/test_authentication.py | 3 +- repoze/bfg/tests/test_request.py | 22 ++++++-------- repoze/bfg/tests/test_router.py | 52 +++++++++++++-------------------- repoze/bfg/tests/test_traversal.py | 33 ++++----------------- repoze/bfg/tests/test_view.py | 10 +++---- repoze/bfg/traversal.py | 7 ++--- repoze/bfg/urldispatch.py | 2 -- repoze/bfg/view.py | 2 +- 11 files changed, 82 insertions(+), 96 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 622880d82..c47693284 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -10,6 +10,23 @@ Bug Fixes combination of a predicate value with an ``=`` sign and one without (e.g. ``a`` vs. ``a=123``). +Internal +-------- + +- The ``repoze.bfg.request.Request`` class, which is a subclass of + ``webob.Request`` now defines its own ``__setattr__``, + ``__getattr__`` and ``__delattr__`` methods, which override the + default WebOb behavior. The default WebOb behavior stores + attributes of the request in ``self.environ['webob.adhoc_attrs']``, + and retrieves them from that dictionary during a ``__getattr__``. + This behavior was undesirable for speed and "expectation" reasons. + Now attributes of the ``request`` are stored in ``request.__dict__`` + (as you otherwise might expect from an object that did not override + these methods). + +- The router no longer calls ``repoze.bfg.traversal._traverse`` and + does its work "inline" (speed). + 1.1a8 (2009-10-27) ================== diff --git a/repoze/bfg/request.py b/repoze/bfg/request.py index 416413c33..e81710ae1 100644 --- a/repoze/bfg/request.py +++ b/repoze/bfg/request.py @@ -19,12 +19,19 @@ class Request(WebobRequest): implements(IRequest) charset = 'utf-8' + # override default WebOb "environ['adhoc_attr']" mutation behavior + __getattr__ = object.__getattribute__ + __setattr__ = object.__setattr__ + __delattr__ = object.__delattr__ + def request_factory(environ): if 'bfg.routes.route' in environ: route = environ['bfg.routes.route'] factory = queryUtility(IRouteRequest, name=route.name) if factory is not None: - return factory(environ) + request = factory(environ) + request.matchdict = environ['bfg.routes.matchdict'] + return request return Request(environ) def create_route_request_factory(name): @@ -34,10 +41,15 @@ def create_route_request_factory(name): implements(iface) charset = 'utf-8' + # override default WebOb "environ['adhoc_attr']" mutation behavior + __getattr__ = object.__getattribute__ + __setattr__ = object.__setattr__ + __delattr__ = object.__delattr__ + return RouteRequest def add_global_response_headers(request, headerlist): - attrs = request.environ.setdefault('webob.adhoc_attrs', {}) + attrs = request.__dict__ response_headers = attrs.setdefault('global_response_headers', []) response_headers.extend(headerlist) diff --git a/repoze/bfg/router.py b/repoze/bfg/router.py index abb2c6f0a..5f5214c10 100644 --- a/repoze/bfg/router.py +++ b/repoze/bfg/router.py @@ -9,6 +9,7 @@ from repoze.bfg.interfaces import INotFoundView from repoze.bfg.interfaces import IRootFactory from repoze.bfg.interfaces import IRouter from repoze.bfg.interfaces import ISettings +from repoze.bfg.interfaces import ITraverser from repoze.bfg.interfaces import IView from repoze.bfg.configuration import make_registry @@ -20,7 +21,7 @@ from repoze.bfg.exceptions import Forbidden from repoze.bfg.exceptions import NotFound from repoze.bfg.request import request_factory from repoze.bfg.threadlocal import manager -from repoze.bfg.traversal import _traverse +from repoze.bfg.traversal import ModelGraphTraverser from repoze.bfg.view import default_forbidden_view from repoze.bfg.view import default_notfound_view @@ -62,17 +63,16 @@ class Router(object): root = self.root_factory(environ) request = request_factory(environ) - # webob.Request's __setattr__ (as of 0.9.5 and lower) is a - # bottleneck; since we're sure we're using a - # webob.Request, we can go around its back and set stuff - # into the environ directly - attrs = environ.setdefault('webob.adhoc_attrs', {}) + attrs = request.__dict__ attrs['registry'] = registry attrs['root'] = root threadlocals['request'] = request registry.has_listeners and registry.notify(NewRequest(request)) - tdict = _traverse(root, environ) + traverser = registry.queryAdapter(root, ITraverser) + if traverser is None: + traverser = ModelGraphTraverser(root) + tdict = traverser(environ) context, view_name, subpath, traversed, vroot, vroot_path = ( tdict['context'], tdict['view_name'], tdict['subpath'], tdict['traversed'], tdict['virtual_root'], diff --git a/repoze/bfg/tests/test_authentication.py b/repoze/bfg/tests/test_authentication.py index 6546fd672..7b20e647c 100644 --- a/repoze/bfg/tests/test_authentication.py +++ b/repoze/bfg/tests/test_authentication.py @@ -420,8 +420,7 @@ class TestAuthTktCookieHelper(unittest.TestCase): request = self._makeRequest({'HTTP_COOKIE':'auth_tkt=bogus'}) result = plugin.identify(request) self.failUnless(result) - attrs = request.environ['webob.adhoc_attrs'] - response_headers = attrs['global_response_headers'] + response_headers = request.global_response_headers self.assertEqual(len(response_headers), 3) self.assertEqual(response_headers[0][0], 'Set-Cookie') diff --git a/repoze/bfg/tests/test_request.py b/repoze/bfg/tests/test_request.py index 78564e57e..8c868fcf5 100644 --- a/repoze/bfg/tests/test_request.py +++ b/repoze/bfg/tests/test_request.py @@ -78,14 +78,18 @@ class TestRequestFactory(unittest.TestCase): sm = getSiteManager() sm.registerUtility(DummyRequest, IRouteRequest, 'routename') route = DummyRoute('routename') - result = self._callFUT({'bfg.routes.route':route}) + result = self._callFUT({'bfg.routes.route':route, + 'bfg.routes.matchdict':{'match':'1'}}) self.assertEqual(result.__class__, DummyRequest) + self.assertEqual(result.matchdict, {'match':'1'}) def test_it_with_route_notfound(self): from repoze.bfg.request import Request route = DummyRoute('routename') - result = self._callFUT({'bfg.routes.route':route}) + result = self._callFUT({'bfg.routes.route':route, + 'bfg.routes.matchdict':{'match':'1'}}) self.assertEqual(result.__class__, Request) + self.failIf(getattr(result, 'matchdict', None) is not None) class Test_create_route_request_factory(unittest.TestCase): def _callFUT(self, name): @@ -104,20 +108,12 @@ class Test_add_global_response_headers(unittest.TestCase): from repoze.bfg.request import add_global_response_headers return add_global_response_headers(request, headerlist) - def test_no_adhoc_attrs(self): - request = DummyRequest() - headers = [('a', 1), ('b', 2)] - self._callFUT(request, headers) - attrs = request.environ['webob.adhoc_attrs'] - self.assertEqual(attrs['global_response_headers'], headers) - - def test_with_adhoc_attrs(self): + def test_it(self): request = DummyRequest() headers = [('a', 1), ('b', 2)] - attrs = request.environ['webob.adhoc_attrs'] = {} - attrs['global_response_headers'] = headers[:] + request.global_response_headers = headers[:] self._callFUT(request, [('c', 1)]) - self.assertEqual(attrs['global_response_headers'], headers + [('c', 1)]) + self.assertEqual(request.global_response_headers, headers + [('c', 1)]) class DummyRoute: def __init__(self, name): diff --git a/repoze/bfg/tests/test_router.py b/repoze/bfg/tests/test_router.py index 13a5bd11a..d2fbf4b2d 100644 --- a/repoze/bfg/tests/test_router.py +++ b/repoze/bfg/tests/test_router.py @@ -147,22 +147,6 @@ class TestRouter(unittest.TestCase): self.failIf('debug_notfound' in result[0]) self.assertEqual(len(logger.messages), 0) - def test_has_webob_adhoc_attrs(self): - environ = self._makeEnviron() - environ['webob.adhoc_attrs'] = {} - context = DummyContext() - logger = self._registerLogger() - router = self._makeOne() - start_response = DummyStartResponse() - result = router(environ, start_response) - headers = start_response.headers - self.assertEqual(len(headers), 2) - status = start_response.status - self.assertEqual(status, '404 Not Found') - self.failUnless('/' in result[0], result) - self.failIf('debug_notfound' in result[0]) - self.assertEqual(len(logger.messages), 0) - def test_call_no_view_registered_no_isettings(self): environ = self._makeEnviron() context = DummyContext() @@ -260,10 +244,11 @@ class TestRouter(unittest.TestCase): self.assertEqual(result, ['Hello world']) self.assertEqual(start_response.headers, ()) self.assertEqual(start_response.status, '200 OK') - self.assertEqual(environ['webob.adhoc_attrs']['view_name'], '') - self.assertEqual(environ['webob.adhoc_attrs']['subpath'], []) - self.assertEqual(environ['webob.adhoc_attrs']['context'], context) - self.assertEqual(environ['webob.adhoc_attrs']['root'], context) + request = view.request + self.assertEqual(request.view_name, '') + self.assertEqual(request.subpath, []) + self.assertEqual(request.context, context) + self.assertEqual(request.root, context) def test_call_view_registered_nonspecific_nondefault_path_and_subpath(self): context = DummyContext() @@ -282,10 +267,11 @@ class TestRouter(unittest.TestCase): self.assertEqual(result, ['Hello world']) self.assertEqual(start_response.headers, ()) self.assertEqual(start_response.status, '200 OK') - self.assertEqual(environ['webob.adhoc_attrs']['view_name'], 'foo') - self.assertEqual(environ['webob.adhoc_attrs']['subpath'], ['bar']) - self.assertEqual(environ['webob.adhoc_attrs']['context'], context) - self.assertEqual(environ['webob.adhoc_attrs']['root'], context) + request = view.request + self.assertEqual(request.view_name, 'foo') + self.assertEqual(request.subpath, ['bar']) + self.assertEqual(request.context, context) + self.assertEqual(request.root, context) def test_call_view_registered_specific_success(self): from zope.interface import Interface @@ -308,10 +294,11 @@ class TestRouter(unittest.TestCase): self.assertEqual(result, ['Hello world']) self.assertEqual(start_response.headers, ()) self.assertEqual(start_response.status, '200 OK') - self.assertEqual(environ['webob.adhoc_attrs']['view_name'], '') - self.assertEqual(environ['webob.adhoc_attrs']['subpath'], []) - self.assertEqual(environ['webob.adhoc_attrs']['context'], context) - self.assertEqual(environ['webob.adhoc_attrs']['root'], context) + request = view.request + self.assertEqual(request.view_name, '') + self.assertEqual(request.subpath, []) + self.assertEqual(request.context, context) + self.assertEqual(request.root, context) def test_call_view_registered_specific_fail(self): from zope.interface import Interface @@ -383,13 +370,14 @@ class TestRouter(unittest.TestCase): self._registerTraverserFactory(context, subpath=['']) response = DummyResponse('200 OK') response.headerlist = [('a', 1)] - view = DummyView(response) + def view(context, request): + request.global_response_headers = [('b', 2)] + return response environ = self._makeEnviron() - environ['webob.adhoc_attrs'] = {'global_response_headers':[('b', 2)]} self._registerView(view, '', IContext, IRequest) router = self._makeOne() start_response = DummyStartResponse() - response = router(environ, start_response) + router(environ, start_response) self.assertEqual(start_response.status, '200 OK') self.assertEqual(start_response.headers, [('a', 1), ('b', 2)]) @@ -493,6 +481,8 @@ class DummyView: self.raise_notfound = raise_notfound def __call__(self, context, request): + self.context = context + self.request = request if self.raise_unauthorized: from repoze.bfg.exceptions import Forbidden raise Forbidden('unauthorized') diff --git a/repoze/bfg/tests/test_traversal.py b/repoze/bfg/tests/test_traversal.py index 8337385c4..451642e8c 100644 --- a/repoze/bfg/tests/test_traversal.py +++ b/repoze/bfg/tests/test_traversal.py @@ -964,35 +964,12 @@ class TraverseTests(unittest.TestCase): self.assertEqual(model.wascontext, True) self.assertEqual(model.environ['PATH_INFO'], '') -class UnderTraverseTests(unittest.TestCase): - def setUp(self): - cleanUp() - - def tearDown(self): - cleanUp() - - def _callFUT(self, context, environ): - from repoze.bfg.traversal import _traverse - return _traverse(context, environ) - - def _registerTraverser(self, traverser): - import zope.component - sm = zope.component.getSiteManager() - from repoze.bfg.interfaces import ITraverser - from zope.interface import Interface - sm.registerAdapter(traverser, (Interface,), ITraverser) - - def test_default_traverser_factory(self): - context = DummyContext() - result = self._callFUT(context, {}) + def test_default_traverser(self): + model = DummyContext() + result = self._callFUT(model, '') self.assertEqual(result['view_name'], '') - - def test_isdict(self): - traverser = make_traverser({}) - self._registerTraverser(traverser) - context = DummyContext() - result = self._callFUT(context, None) - self.assertEqual(result, {}) + self.assertEqual(result['context'], model) + def make_traverser(result): class DummyTraverser(object): diff --git a/repoze/bfg/tests/test_view.py b/repoze/bfg/tests/test_view.py index bbef24359..14923f5b8 100644 --- a/repoze/bfg/tests/test_view.py +++ b/repoze/bfg/tests/test_view.py @@ -1255,7 +1255,7 @@ class Test_rendered_response(unittest.TestCase): response = {'a':'1'} request = DummyRequest() attrs = {'response_content_type':'text/nonsense'} - request.environ['webob.adhoc_attrs'] = attrs + request.__dict__.update(attrs) result = self._callFUT(renderer, response, request=request) self.assertEqual(result.content_type, 'text/nonsense') @@ -1264,7 +1264,7 @@ class Test_rendered_response(unittest.TestCase): response = {'a':'1'} request = DummyRequest() attrs = {'response_headerlist':[('a', '1'), ('b', '2')]} - request.environ['webob.adhoc_attrs'] = attrs + request.__dict__.update(attrs) result = self._callFUT(renderer, response, request=request) self.assertEqual(result.headerlist, [('Content-Type', 'text/html; charset=UTF-8'), @@ -1277,7 +1277,7 @@ class Test_rendered_response(unittest.TestCase): response = {'a':'1'} request = DummyRequest() attrs = {'response_status':'406 You Lose'} - request.environ['webob.adhoc_attrs'] = attrs + request.__dict__.update(attrs) result = self._callFUT(renderer, response, request=request) self.assertEqual(result.status, '406 You Lose') @@ -1286,7 +1286,7 @@ class Test_rendered_response(unittest.TestCase): response = {'a':'1'} request = DummyRequest() attrs = {'response_charset':'UTF-16'} - request.environ['webob.adhoc_attrs'] = attrs + request.__dict__.update(attrs) result = self._callFUT(renderer, response, request=request) self.assertEqual(result.charset, 'UTF-16') @@ -1295,7 +1295,7 @@ class Test_rendered_response(unittest.TestCase): response = {'a':'1'} request = DummyRequest() attrs = {'response_cache_for':100} - request.environ['webob.adhoc_attrs'] = attrs + request.__dict__.update(attrs) result = self._callFUT(renderer, response, request=request) self.assertEqual(result.cache_control.max_age, 100) diff --git a/repoze/bfg/traversal.py b/repoze/bfg/traversal.py index 48d7ef1c1..43adcdebb 100644 --- a/repoze/bfg/traversal.py +++ b/repoze/bfg/traversal.py @@ -268,15 +268,12 @@ def traverse(model, path): if path and path[0] == '/': model = find_root(model) - return _traverse(model, {'PATH_INFO':path}) - -def _traverse(model, environ): + environ = {'PATH_INFO':path} traverser = queryAdapter(model, ITraverser) if traverser is None: traverser = ModelGraphTraverser(model) - result = traverser(environ) - return result + return traverser(environ) def model_path_tuple(model, *elements): """ diff --git a/repoze/bfg/urldispatch.py b/repoze/bfg/urldispatch.py index 58ea192c6..14a81062c 100644 --- a/repoze/bfg/urldispatch.py +++ b/repoze/bfg/urldispatch.py @@ -46,8 +46,6 @@ class RoutesRootFactory(object): environ['wsgiorg.routing_args'] = ((), match) environ['bfg.routes.route'] = route environ['bfg.routes.matchdict'] = match - adhoc_attrs = environ.setdefault('webob.adhoc_attrs', {}) - adhoc_attrs['matchdict'] = match factory = route.factory or self.default_root_factory return factory(environ) diff --git a/repoze/bfg/view.py b/repoze/bfg/view.py index 603992551..09a434f7e 100644 --- a/repoze/bfg/view.py +++ b/repoze/bfg/view.py @@ -531,7 +531,7 @@ def rendered_response(renderer, response, view, context,request, renderer_name): 'context':context, 'request':request}) response_factory = queryUtility(IResponseFactory, default=Response) response = response_factory(result) - attrs = request.environ.get('webob.adhoc_attrs', {}) + attrs = request.__dict__ content_type = attrs.get('response_content_type', None) if content_type is not None: response.content_type = content_type -- cgit v1.2.3