From c5dd748a3bc06b6e7e17797863bb363a2f377a5e Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sun, 5 Jan 2020 18:47:52 -0600 Subject: allow overriding synthesized properties --- src/pyramid/util.py | 23 ++++++++++++++++++++++- tests/test_util.py | 30 ++++++++++++++++-------------- 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/src/pyramid/util.py b/src/pyramid/util.py index e552b37de..ca644dcce 100644 --- a/src/pyramid/util.py +++ b/src/pyramid/util.py @@ -73,6 +73,27 @@ def as_sorted_tuple(val): return val +class SettableProperty(object): + def __init__(self, wrapped, name): + self.wrapped = wrapped + self.name = name + functools.update_wrapper(self, wrapped) + + def __get__(self, obj, type=None): + if obj is None: # pragma: no cover + return self + value = obj.__dict__.get(self.name, _marker) + if value is _marker: + value = self.wrapped(obj) + return value + + def __set__(self, obj, value): + obj.__dict__[self.name] = value + + def __delete__(self, obj): + del obj.__dict__[self.name] + + class InstancePropertyHelper(object): """A helper object for assigning properties and descriptors to instances. It is not normally possible to do this because descriptors must be @@ -113,7 +134,7 @@ class InstancePropertyHelper(object): fn = pyramid.decorator.reify(fn) elif not is_property: - fn = property(fn) + fn = SettableProperty(fn, name) return name, fn diff --git a/tests/test_util.py b/tests/test_util.py index 293036c10..1553d8e60 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -103,25 +103,26 @@ class Test_InstancePropertyHelper(unittest.TestCase): ) def test_override_property(self): - def worker(obj): # pragma: no cover + def worker(obj): pass foo = Dummy() helper = self._getTargetClass() helper.set_property(foo, worker, name='x') - - def doit(): - foo.x = 1 - - self.assertRaises(AttributeError, doit) + self.assertIsNone(foo.x) + foo.x = 1 + self.assertEqual(foo.x, 1) + del foo.x + self.assertIsNone(foo.x) def test_override_reify(self): - def worker(obj): # pragma: no cover + def worker(obj): pass foo = Dummy() helper = self._getTargetClass() helper.set_property(foo, worker, name='x', reify=True) + self.assertIsNone(foo.x) foo.x = 1 self.assertEqual(1, foo.x) foo.x = 2 @@ -301,23 +302,24 @@ class Test_InstancePropertyMixin(unittest.TestCase): ) def test_override_property(self): - def worker(obj): # pragma: no cover + def worker(obj): pass foo = self._makeOne() foo.set_property(worker, name='x') - - def doit(): - foo.x = 1 - - self.assertRaises(AttributeError, doit) + self.assertIsNone(foo.x) + foo.x = 1 + self.assertEqual(foo.x, 1) + del foo.x + self.assertIsNone(foo.x) def test_override_reify(self): - def worker(obj): # pragma: no cover + def worker(obj): pass foo = self._makeOne() foo.set_property(worker, name='x', reify=True) + self.assertIsNone(foo.x) foo.x = 1 self.assertEqual(1, foo.x) foo.x = 2 -- cgit v1.2.3 From 1b555e2b7911bc3d8cee811986ec67c88fa36cad Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sun, 5 Jan 2020 18:57:39 -0600 Subject: add changelog for #3559 --- CHANGES.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 383906e00..70ee43b96 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -71,6 +71,13 @@ Features - Fix ``DeprecationWarning`` emitted by using the ``imp`` module. See https://github.com/Pylons/pyramid/pull/3553 +- Properties created via ``config.add_request_method(..., property=True)`` or + ``request.set_property`` used to be readonly. They can now be overridden + via ``request.foo = ...`` and until the value is deleted it will return + the overridden value. This is most useful when mocking request properties + in testing. + See https://github.com/Pylons/pyramid/pull/3559 + Deprecations ------------ -- cgit v1.2.3 From db8ab619a09b90c633d78ce5487703bf72ea4111 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sun, 5 Jan 2020 19:24:19 -0600 Subject: support using descriptors other than property --- src/pyramid/util.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/pyramid/util.py b/src/pyramid/util.py index ca644dcce..9024fe816 100644 --- a/src/pyramid/util.py +++ b/src/pyramid/util.py @@ -115,26 +115,26 @@ class InstancePropertyHelper(object): (name, property) pair. """ - is_property = isinstance(callable, property) - if is_property: - fn = callable - if name is None: - raise ValueError('must specify "name" for a property') - if reify: - raise ValueError('cannot reify a property') - elif name is not None: - fn = lambda this: callable(this) - fn.__name__ = get_callable_name(name) - fn.__doc__ = callable.__doc__ - else: + if name is None: + if not hasattr(callable, '__name__'): + raise ValueError( + 'missing __name__, must specify "name" for property' + ) name = callable.__name__ - fn = callable + name = get_callable_name(name) + is_data_descriptor = hasattr(callable, '__set__') + if reify and is_data_descriptor: + raise ValueError('cannot reify a data descriptor') + fn = callable if reify: import pyramid.decorator # avoid circular import + fn = lambda this: callable(this) + fn.__name__ = name + fn.__doc__ = callable.__doc__ fn = pyramid.decorator.reify(fn) - elif not is_property: - fn = SettableProperty(fn, name) + elif not is_data_descriptor: + fn = SettableProperty(callable, name) return name, fn -- cgit v1.2.3 From 5cad7ad7ce47f1fe151b40ae9398fb5cbbfd3806 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sun, 5 Jan 2020 21:24:35 -0600 Subject: handle settable property same as reify --- src/pyramid/util.py | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/pyramid/util.py b/src/pyramid/util.py index 9024fe816..504516631 100644 --- a/src/pyramid/util.py +++ b/src/pyramid/util.py @@ -74,24 +74,23 @@ def as_sorted_tuple(val): class SettableProperty(object): - def __init__(self, wrapped, name): + def __init__(self, wrapped): self.wrapped = wrapped - self.name = name functools.update_wrapper(self, wrapped) def __get__(self, obj, type=None): if obj is None: # pragma: no cover return self - value = obj.__dict__.get(self.name, _marker) + value = obj.__dict__.get(self.wrapped.__name__, _marker) if value is _marker: value = self.wrapped(obj) return value def __set__(self, obj, value): - obj.__dict__[self.name] = value + obj.__dict__[self.wrapped.__name__] = value def __delete__(self, obj): - del obj.__dict__[self.name] + del obj.__dict__[self.wrapped.__name__] class InstancePropertyHelper(object): @@ -125,16 +124,19 @@ class InstancePropertyHelper(object): is_data_descriptor = hasattr(callable, '__set__') if reify and is_data_descriptor: raise ValueError('cannot reify a data descriptor') - fn = callable - if reify: - import pyramid.decorator # avoid circular import - - fn = lambda this: callable(this) - fn.__name__ = name - fn.__doc__ = callable.__doc__ - fn = pyramid.decorator.reify(fn) - elif not is_data_descriptor: - fn = SettableProperty(callable, name) + if is_data_descriptor: + fn = callable + else: + wrapped = lambda this: callable(this) + wrapped.__name__ = name + wrapped.__doc__ = callable.__doc__ + + if reify: + import pyramid.decorator # avoid circular import + + fn = pyramid.decorator.reify(wrapped) + else: + fn = SettableProperty(wrapped) return name, fn -- cgit v1.2.3 From 5c05cd9837adcd9b0ee0ee775added2a6d3b6688 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Wed, 8 Jan 2020 01:15:02 -0600 Subject: invoke finished callbacks in prepare/bootstrap closers --- CHANGES.rst | 4 ++++ src/pyramid/scripting.py | 2 ++ tests/test_scripting.py | 24 ++++++++++++++++++++++++ 3 files changed, 30 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 70ee43b96..70f5bbb64 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -78,6 +78,10 @@ Features in testing. See https://github.com/Pylons/pyramid/pull/3559 +- Finished callbacks are now executed as part of the ``closer`` that is + invoked as part of ``pyramid.scripting.prepare`` and + ``pyramid.paster.boostrap``. + Deprecations ------------ diff --git a/src/pyramid/scripting.py b/src/pyramid/scripting.py index abcdd1030..4178b6526 100644 --- a/src/pyramid/scripting.py +++ b/src/pyramid/scripting.py @@ -94,6 +94,8 @@ def prepare(request=None, registry=None): apply_request_extensions(request) def closer(): + if request.finished_callbacks: + request._process_finished_callbacks() ctx.end() root_factory = registry.queryUtility( diff --git a/tests/test_scripting.py b/tests/test_scripting.py index 8f74f35f8..b8a18f57e 100644 --- a/tests/test_scripting.py +++ b/tests/test_scripting.py @@ -1,3 +1,4 @@ +from collections import deque import unittest @@ -162,6 +163,20 @@ class Test_prepare(unittest.TestCase): self.assertEqual(request.context, root) self.assertEqual(request.registry, registry) + def test_closer_invokes_finished_callbacks(self): + finish_called = [False] + + def finished_callback(request): + finish_called[0] = True + + request = DummyRequest({}) + request.registry = self._makeRegistry() + info = self._callFUT(request=request) + request.add_finished_callback(finished_callback) + closer = info['closer'] + closer() + self.assertTrue(finish_called[0]) + class Test__make_request(unittest.TestCase): def _callFUT(self, path='/', registry=None): @@ -234,6 +249,15 @@ class DummyRequest(object): def __init__(self, environ): self.environ = environ + self.finished_callbacks = deque() + + def add_finished_callback(self, cb): + self.finished_callbacks.append(cb) + + def _process_finished_callbacks(self): + while self.finished_callbacks: + cb = self.finished_callbacks.popleft() + cb(self) class DummyExtensions: -- cgit v1.2.3 From 8415bc1778ea080aeed69ef2d752f3d0063e26f9 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Wed, 8 Jan 2020 01:15:31 -0600 Subject: add a RequestLocalCache class --- CHANGES.rst | 10 +++++ src/pyramid/request.py | 111 +++++++++++++++++++++++++++++++++++++++++++++++++ tests/test_request.py | 100 +++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 216 insertions(+), 5 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 70f5bbb64..e93f8a813 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -82,6 +82,16 @@ Features invoked as part of ``pyramid.scripting.prepare`` and ``pyramid.paster.boostrap``. +- Added ``pyramid.request.RequestLocalCache`` which can be used to create + simple objects that are shared across requests and can be used to store + per-request data. This is useful when the source of data is external to + the request itself. Often a reified property is used on a request via + ``pyramid.config.Configurator.add_request_method``, or + ``pyramid.decorator.reify``, and these work great when the data is + generated on-demand when accessing the request property. However, often + the case is that the data is generated when accessing some other system + and then we want to cache the data for the duration of the request. + Deprecations ------------ diff --git a/src/pyramid/request.py b/src/pyramid/request.py index d65be2a2f..83134666d 100644 --- a/src/pyramid/request.py +++ b/src/pyramid/request.py @@ -1,4 +1,6 @@ from collections import deque +import functools +import weakref from webob import BaseRequest from zope.interface import implementer from zope.interface.interface import InterfaceClass @@ -17,6 +19,7 @@ from pyramid.url import URLMethodsMixin from pyramid.util import ( InstancePropertyHelper, InstancePropertyMixin, + Sentinel, bytes_, text_, ) @@ -330,3 +333,111 @@ def apply_request_extensions(request, extensions=None): InstancePropertyHelper.apply_properties( request, extensions.descriptors ) + + +class RequestLocalCache: + """ + A store that caches values during for the lifecycle of a request. + + Instantiate a cache object and use it to decorate methods or functions + that accept a request parameter. Any other arguments are not used as + cache keys so make sure they are constant across calls per-request. + + Wrapping methods: + + .. code-block:: python + + class SecurityPolicy: + identity_cache = RequestLocalCache() + + @identity_cache + def authenticated_identity(self, request): + result = ... # do some expensive computations + return result + + Wrapping functions: + + .. code-block:: python + + user_cache = RequestLocalCache() + + @user_cache + def get_user(request): + result = ... # do some expensive computations + return result + + It's also possible to inspect and influence the cache during runtime using + :meth:`.get`, :meth:`.set` and :meth:`.clear`. Using these methods, the + cache can be used directly as well, without using it as a decorator. + + The cache will clean release resources aggressively by utilizing + :meth:`pyramid.request.Request.add_finished_callback`, but it will also + maintain a weakref to the request and cleanup when it is garbage collected + if the callbacks are not invoked for some reason. + + .. versionadded:: 2.0 + + """ + NO_VALUE = Sentinel('NO_VALUE') + + def __init__(self): + self.store = weakref.WeakKeyDictionary() + + def auto_adapt(decorator): + class FunctionOrMethodAdapter: + def __init__(self, inst, func): + self.inst = inst + self.__wrapped__ = func + + def __call__(self, *args, **kwargs): + return decorator(self.inst, self.__wrapped__)(*args, **kwargs) + + def __get__(self, instance, owner): + return decorator(self.inst, self.__wrapped__.__get__(instance, owner)) + + def adapt(inst, fn): + return FunctionOrMethodAdapter(inst, fn) + return adapt + + @auto_adapt + def __call__(self, fn): + """ + Decorate a method or function. + + """ + @functools.wraps(fn) + def wrapper(request, *args, **kwargs): + result = self.get(request) + if result is self.NO_VALUE: + result = fn(request, *args, **kwargs) + self.set(request, result) + request.add_finished_callback(self.clear) + return result + return wrapper + + del auto_adapt + + def clear(self, request): + """ + Delete the value from the cache. + + The cached value is returned or :attr:`.NO_VALUE`. + + """ + return self.store.pop(request, self.NO_VALUE) + + def get(self, request, default=NO_VALUE): + """ + Return the value from the cache. + + The cached value is returned or ``default``. + + """ + return self.store.get(request, default) + + def set(self, request, value): + """ + Update the cache with a new value. + + """ + self.store[request] = value diff --git a/tests/test_request.py b/tests/test_request.py index bbf6aa47c..3cff4bb53 100644 --- a/tests/test_request.py +++ b/tests/test_request.py @@ -535,10 +535,6 @@ class Test_apply_request_extensions(unittest.TestCase): self.assertEqual(request.foo('abc'), 'abc') -class Dummy(object): - pass - - class Test_subclassing_Request(unittest.TestCase): def test_subclass(self): from pyramid.interfaces import IRequest @@ -598,14 +594,108 @@ class Test_subclassing_Request(unittest.TestCase): self.assertTrue(IRequest.implementedBy(RequestSub)) +class TestRequestLocalCache(unittest.TestCase): + def _makeOne(self): + from pyramid.request import RequestLocalCache + + return RequestLocalCache() + + def test_it_works_with_functions(self): + cache = self._makeOne() + a = [0] + + @cache + def foo(request): + a[0] += 1 + return a[0] + + req1 = DummyRequest() + req2 = DummyRequest() + self.assertEqual(foo(req1), 1) + self.assertEqual(foo(req2), 2) + self.assertEqual(foo(req1), 1) + self.assertEqual(foo(req2), 2) + self.assertEqual(len(req1.finished_callbacks), 1) + self.assertEqual(len(req2.finished_callbacks), 1) + + def test_it_works_with_methods(self): + cache = self._makeOne() + a = [0] + + class DummyPolicy: + @cache + def foo(self, request): + a[0] += 1 + return a[0] + + policy = DummyPolicy() + req1 = DummyRequest() + req2 = DummyRequest() + self.assertEqual(policy.foo(req1), 1) + self.assertEqual(policy.foo(req2), 2) + self.assertEqual(policy.foo(req1), 1) + self.assertEqual(policy.foo(req2), 2) + self.assertEqual(len(req1.finished_callbacks), 1) + self.assertEqual(len(req2.finished_callbacks), 1) + + def test_clear_works(self): + cache = self._makeOne() + a = [0] + + @cache + def foo(request): + a[0] += 1 + return a[0] + + req = DummyRequest() + self.assertEqual(foo(req), 1) + self.assertEqual(len(req.finished_callbacks), 1) + cache.clear(req) + self.assertEqual(foo(req), 2) + self.assertEqual(len(req.finished_callbacks), 2) + + def test_set_overrides_current_value(self): + cache = self._makeOne() + a = [0] + + @cache + def foo(request): + a[0] += 1 + return a[0] + + req = DummyRequest() + self.assertEqual(foo(req), 1) + self.assertEqual(len(req.finished_callbacks), 1) + cache.set(req, 8) + self.assertEqual(foo(req), 8) + self.assertEqual(len(req.finished_callbacks), 1) + self.assertEqual(cache.get(req), 8) + + def test_get_works(self): + cache = self._makeOne() + req = DummyRequest() + self.assertIs(cache.get(req), cache.NO_VALUE) + cache.set(req, 2) + self.assertIs(cache.get(req), 2) + + +class Dummy(object): + pass + + class DummyRequest(object): def __init__(self, environ=None): if environ is None: environ = {} self.environ = environ + self.response_callbacks = [] + self.finished_callbacks = [] def add_response_callback(self, callback): - self.response_callbacks = [callback] + self.response_callbacks.append(callback) + + def add_finished_callback(self, callback): + self.finished_callbacks.append(callback) def get_response(self, app): return app -- cgit v1.2.3 From df1c19ac429d680f094b6323ad9fc02623d5e953 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Wed, 8 Jan 2020 01:19:17 -0600 Subject: link to pr --- CHANGES.rst | 2 ++ docs/api/request.rst | 3 +++ 2 files changed, 5 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index e93f8a813..d1807103f 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -81,6 +81,7 @@ Features - Finished callbacks are now executed as part of the ``closer`` that is invoked as part of ``pyramid.scripting.prepare`` and ``pyramid.paster.boostrap``. + See https://github.com/Pylons/pyramid/pull/3561 - Added ``pyramid.request.RequestLocalCache`` which can be used to create simple objects that are shared across requests and can be used to store @@ -91,6 +92,7 @@ Features generated on-demand when accessing the request property. However, often the case is that the data is generated when accessing some other system and then we want to cache the data for the duration of the request. + See https://github.com/Pylons/pyramid/pull/3561 Deprecations ------------ diff --git a/docs/api/request.rst b/docs/api/request.rst index 9e9c70d3a..59d85ac2a 100644 --- a/docs/api/request.rst +++ b/docs/api/request.rst @@ -362,3 +362,6 @@ see :class:`pyramid.interfaces.IMultiDict`. .. autofunction:: apply_request_extensions(request) + +.. autoclass:: RequestLocalCache + :members: -- cgit v1.2.3 From 5f114efcc63a9158dff7afb2a3974f56a5d09784 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Wed, 8 Jan 2020 01:21:25 -0600 Subject: add changed directives --- src/pyramid/paster.py | 6 ++++++ src/pyramid/scripting.py | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/src/pyramid/paster.py b/src/pyramid/paster.py index 22c09e41a..00c1a8915 100644 --- a/src/pyramid/paster.py +++ b/src/pyramid/paster.py @@ -107,6 +107,12 @@ def bootstrap(config_uri, request=None, options=None): Added the ability to use the return value as a context manager. + .. versionchanged:: 2.0 + + Request finished callbacks added via + :meth:`pyramid.request.Request.add_finished_callback` will be invoked + by the ``closer``. + """ app = get_app(config_uri, options=options) env = prepare(request) diff --git a/src/pyramid/scripting.py b/src/pyramid/scripting.py index 4178b6526..bf23f1008 100644 --- a/src/pyramid/scripting.py +++ b/src/pyramid/scripting.py @@ -74,6 +74,12 @@ def prepare(request=None, registry=None): Added the ability to use the return value as a context manager. + .. versionchanged:: 2.0 + + Request finished callbacks added via + :meth:`pyramid.request.Request.add_finished_callback` will be invoked + by the ``closer``. + """ if registry is None: registry = getattr(request, 'registry', global_registries.last) -- cgit v1.2.3 From 2bf834c98a6d125c820996ad86d5bc1ac2dbf7e7 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Wed, 8 Jan 2020 01:22:03 -0600 Subject: fix lint --- src/pyramid/request.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/pyramid/request.py b/src/pyramid/request.py index 83134666d..8bb1e2632 100644 --- a/src/pyramid/request.py +++ b/src/pyramid/request.py @@ -378,6 +378,7 @@ class RequestLocalCache: .. versionadded:: 2.0 """ + NO_VALUE = Sentinel('NO_VALUE') def __init__(self): @@ -393,10 +394,13 @@ class RequestLocalCache: return decorator(self.inst, self.__wrapped__)(*args, **kwargs) def __get__(self, instance, owner): - return decorator(self.inst, self.__wrapped__.__get__(instance, owner)) + return decorator( + self.inst, self.__wrapped__.__get__(instance, owner) + ) def adapt(inst, fn): return FunctionOrMethodAdapter(inst, fn) + return adapt @auto_adapt @@ -405,6 +409,7 @@ class RequestLocalCache: Decorate a method or function. """ + @functools.wraps(fn) def wrapper(request, *args, **kwargs): result = self.get(request) @@ -413,6 +418,7 @@ class RequestLocalCache: self.set(request, result) request.add_finished_callback(self.clear) return result + return wrapper del auto_adapt -- cgit v1.2.3 From 025824417f0d779f41717a28047dc420991015aa Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Wed, 8 Jan 2020 10:11:35 -0600 Subject: grammar fixes --- CHANGES.rst | 2 +- src/pyramid/request.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index d1807103f..8159cea36 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -80,7 +80,7 @@ Features - Finished callbacks are now executed as part of the ``closer`` that is invoked as part of ``pyramid.scripting.prepare`` and - ``pyramid.paster.boostrap``. + ``pyramid.paster.bootstrap``. See https://github.com/Pylons/pyramid/pull/3561 - Added ``pyramid.request.RequestLocalCache`` which can be used to create diff --git a/src/pyramid/request.py b/src/pyramid/request.py index 8bb1e2632..d79aad9bf 100644 --- a/src/pyramid/request.py +++ b/src/pyramid/request.py @@ -370,7 +370,7 @@ class RequestLocalCache: :meth:`.get`, :meth:`.set` and :meth:`.clear`. Using these methods, the cache can be used directly as well, without using it as a decorator. - The cache will clean release resources aggressively by utilizing + The cache will release resources aggressively by utilizing :meth:`pyramid.request.Request.add_finished_callback`, but it will also maintain a weakref to the request and cleanup when it is garbage collected if the callbacks are not invoked for some reason. -- cgit v1.2.3 From 79d6a38a66a68231e651a6c81e784ab1a78c07de Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Thu, 9 Jan 2020 00:57:15 -0600 Subject: fix paradigm to avoid incorrect usages It's almost impossible to create a decorator that works on both methods and functions, but more importantly the original approach was sharing a cache across instances of the policy. It needed to be local to the policy instance, but shared across requests. The new example demonstrates that. The cache is also much more flexible in its usage patterns now. --- docs/api/request.rst | 1 + src/pyramid/request.py | 143 +++++++++++++++++++++++++++++-------------------- tests/test_request.py | 82 +++++++++++++++++----------- 3 files changed, 136 insertions(+), 90 deletions(-) diff --git a/docs/api/request.rst b/docs/api/request.rst index 59d85ac2a..357e49b76 100644 --- a/docs/api/request.rst +++ b/docs/api/request.rst @@ -365,3 +365,4 @@ .. autoclass:: RequestLocalCache :members: + :special-members: diff --git a/src/pyramid/request.py b/src/pyramid/request.py index d79aad9bf..0d3cf481b 100644 --- a/src/pyramid/request.py +++ b/src/pyramid/request.py @@ -339,41 +339,41 @@ class RequestLocalCache: """ A store that caches values during for the lifecycle of a request. - Instantiate a cache object and use it to decorate methods or functions - that accept a request parameter. Any other arguments are not used as - cache keys so make sure they are constant across calls per-request. + Wrapping Functions - Wrapping methods: + Instantiate and use it to decorate functions that accept a request + parameter. The result is cached and returned in subsequent invocations + of the function. .. code-block:: python - class SecurityPolicy: - identity_cache = RequestLocalCache() + @RequestLocalCache() + def get_user(request): + result = ... # do some expensive computations + return result - @identity_cache - def authenticated_identity(self, request): - result = ... # do some expensive computations - return result + Wrapping Methods - Wrapping functions: + A method can be wrapped but it needs to be bound to an instance such that + it only accepts one argument - the request. .. code-block:: python - user_cache = RequestLocalCache() + class SecurityPolicy: + def __init__(self): + self.identity_cache = RequestLocalCache(self.load_identity) - @user_cache - def get_user(request): - result = ... # do some expensive computations - return result + def load_identity(self, request): + result = ... # do some expensive computations + return result - It's also possible to inspect and influence the cache during runtime using - :meth:`.get`, :meth:`.set` and :meth:`.clear`. Using these methods, the - cache can be used directly as well, without using it as a decorator. + def authenticated_identity(self, request): + return self.identity_cache.get_or_create(request) - The cache will release resources aggressively by utilizing - :meth:`pyramid.request.Request.add_finished_callback`, but it will also - maintain a weakref to the request and cleanup when it is garbage collected - if the callbacks are not invoked for some reason. + The cache maintains a weakref to each request and will release the cached + values when the request is garbage-collected. However, in most scenarios, + it will release resources earlier via + :meth:`pyramid.request.Request.add_finished_callback`. .. versionadded:: 2.0 @@ -381,56 +381,62 @@ class RequestLocalCache: NO_VALUE = Sentinel('NO_VALUE') - def __init__(self): - self.store = weakref.WeakKeyDictionary() + def __init__(self, creator=None): + self._store = weakref.WeakKeyDictionary() + self._creator = creator - def auto_adapt(decorator): - class FunctionOrMethodAdapter: - def __init__(self, inst, func): - self.inst = inst - self.__wrapped__ = func + def __call__(self, fn): + """ + Decorate and return a new function that utilizes the cache. - def __call__(self, *args, **kwargs): - return decorator(self.inst, self.__wrapped__)(*args, **kwargs) + The cache is attached as an attribute to the decorated function + such that it may be manipulated directly. For example: - def __get__(self, instance, owner): - return decorator( - self.inst, self.__wrapped__.__get__(instance, owner) - ) + .. code-block:: python - def adapt(inst, fn): - return FunctionOrMethodAdapter(inst, fn) + @RequestLocalCache() + def do_something_expensive(request): + return ... - return adapt + value = do_something_expensive(request) + do_something_expensive.cache.clear(request) - @auto_adapt - def __call__(self, fn): - """ - Decorate a method or function. + The ``fn`` is also bound as the creator on the cache such that + invocations of :meth:`.get_or_create` will use it. """ @functools.wraps(fn) - def wrapper(request, *args, **kwargs): - result = self.get(request) - if result is self.NO_VALUE: - result = fn(request, *args, **kwargs) - self.set(request, result) - request.add_finished_callback(self.clear) - return result + def wrapper(request): + return wrapper.cache.get_or_create(request, fn) + wrapper.cache = self + self._creator = fn return wrapper - del auto_adapt - - def clear(self, request): + def get_or_create(self, request, creator=None): """ - Delete the value from the cache. + Return the cached value. - The cached value is returned or :attr:`.NO_VALUE`. + If no value is cached then execute the creator, cache the result, + and return it. + + The creator may be passed in as an argument or bound to the cache + using the ``__call__`` or constructor arguments. """ - return self.store.pop(request, self.NO_VALUE) + result = self._store.get(request, self.NO_VALUE) + if result is self.NO_VALUE: + if creator is None: + creator = self._creator + if creator is None: + raise ValueError( + 'no creator function has been registered with the ' + 'cache or supplied to "get_or_create"' + ) + result = creator(request) + self.set(request, result) + return result def get(self, request, default=NO_VALUE): """ @@ -439,11 +445,32 @@ class RequestLocalCache: The cached value is returned or ``default``. """ - return self.store.get(request, default) + return self._store.get(request, default) def set(self, request, value): """ Update the cache with a new value. """ - self.store[request] = value + already_set = request in self._store + self._store[request] = value + + # avoid registering the callback more than once + if not already_set: + request.add_finished_callback(self._store.pop) + + def clear(self, request): + """ + Delete the value from the cache. + + The cached value is returned or :attr:`.NO_VALUE`. + + """ + old_value = self.NO_VALUE + if request in self._store: + old_value = self._store[request] + + # keep a value in the store so that we don't register another + # finished callback when set is invoked + self._store[request] = self.NO_VALUE + return old_value diff --git a/tests/test_request.py b/tests/test_request.py index 3cff4bb53..a36bd238c 100644 --- a/tests/test_request.py +++ b/tests/test_request.py @@ -595,16 +595,15 @@ class Test_subclassing_Request(unittest.TestCase): class TestRequestLocalCache(unittest.TestCase): - def _makeOne(self): + def _makeOne(self, *args, **kwargs): from pyramid.request import RequestLocalCache - return RequestLocalCache() + return RequestLocalCache(*args, **kwargs) def test_it_works_with_functions(self): - cache = self._makeOne() a = [0] - @cache + @self._makeOne() def foo(request): a[0] += 1 return a[0] @@ -618,31 +617,10 @@ class TestRequestLocalCache(unittest.TestCase): self.assertEqual(len(req1.finished_callbacks), 1) self.assertEqual(len(req2.finished_callbacks), 1) - def test_it_works_with_methods(self): - cache = self._makeOne() - a = [0] - - class DummyPolicy: - @cache - def foo(self, request): - a[0] += 1 - return a[0] - - policy = DummyPolicy() - req1 = DummyRequest() - req2 = DummyRequest() - self.assertEqual(policy.foo(req1), 1) - self.assertEqual(policy.foo(req2), 2) - self.assertEqual(policy.foo(req1), 1) - self.assertEqual(policy.foo(req2), 2) - self.assertEqual(len(req1.finished_callbacks), 1) - self.assertEqual(len(req2.finished_callbacks), 1) - def test_clear_works(self): - cache = self._makeOne() a = [0] - @cache + @self._makeOne() def foo(request): a[0] += 1 return a[0] @@ -650,15 +628,14 @@ class TestRequestLocalCache(unittest.TestCase): req = DummyRequest() self.assertEqual(foo(req), 1) self.assertEqual(len(req.finished_callbacks), 1) - cache.clear(req) + foo.cache.clear(req) self.assertEqual(foo(req), 2) - self.assertEqual(len(req.finished_callbacks), 2) + self.assertEqual(len(req.finished_callbacks), 1) def test_set_overrides_current_value(self): - cache = self._makeOne() a = [0] - @cache + @self._makeOne() def foo(request): a[0] += 1 return a[0] @@ -666,10 +643,10 @@ class TestRequestLocalCache(unittest.TestCase): req = DummyRequest() self.assertEqual(foo(req), 1) self.assertEqual(len(req.finished_callbacks), 1) - cache.set(req, 8) + foo.cache.set(req, 8) self.assertEqual(foo(req), 8) self.assertEqual(len(req.finished_callbacks), 1) - self.assertEqual(cache.get(req), 8) + self.assertEqual(foo.cache.get(req), 8) def test_get_works(self): cache = self._makeOne() @@ -678,6 +655,47 @@ class TestRequestLocalCache(unittest.TestCase): cache.set(req, 2) self.assertIs(cache.get(req), 2) + def test_creator_in_constructor(self): + + def foo(request): + return 8 + + cache = self._makeOne(foo) + req = DummyRequest() + result = cache.get_or_create(req) + self.assertEqual(result, 8) + + def test_decorator_overrides_creator(self): + + def foo(request): # pragma: no cover + raise AssertionError + + cache = self._makeOne(foo) + + @cache + def bar(request): + return 8 + + req = DummyRequest() + result = cache.get_or_create(req) + self.assertEqual(result, 8) + + def test_get_or_create_overrides_creator(self): + cache = self._makeOne() + + @cache + def foo(request): # pragma: no cover + raise AssertionError + + req = DummyRequest() + result = cache.get_or_create(req, lambda r: 8) + self.assertEqual(result, 8) + + def test_get_or_create_with_no_creator(self): + cache = self._makeOne() + req = DummyRequest() + self.assertRaises(ValueError, cache.get_or_create, req) + class Dummy(object): pass -- cgit v1.2.3 From 9eb2c71366bff05745c49e84aac64ac8c819d6b8 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Thu, 9 Jan 2020 00:58:50 -0600 Subject: fix lint --- tests/test_request.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_request.py b/tests/test_request.py index a36bd238c..3c5535b0e 100644 --- a/tests/test_request.py +++ b/tests/test_request.py @@ -656,7 +656,6 @@ class TestRequestLocalCache(unittest.TestCase): self.assertIs(cache.get(req), 2) def test_creator_in_constructor(self): - def foo(request): return 8 @@ -666,7 +665,6 @@ class TestRequestLocalCache(unittest.TestCase): self.assertEqual(result, 8) def test_decorator_overrides_creator(self): - def foo(request): # pragma: no cover raise AssertionError -- cgit v1.2.3 From f04c06c1de47373e51f6fb1b5dc1330b3df58299 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Thu, 9 Jan 2020 01:13:56 -0600 Subject: clarify the docs --- docs/api/request.rst | 1 - src/pyramid/request.py | 38 ++++++++++++++------------------------ 2 files changed, 14 insertions(+), 25 deletions(-) diff --git a/docs/api/request.rst b/docs/api/request.rst index 357e49b76..59d85ac2a 100644 --- a/docs/api/request.rst +++ b/docs/api/request.rst @@ -365,4 +365,3 @@ .. autoclass:: RequestLocalCache :members: - :special-members: diff --git a/src/pyramid/request.py b/src/pyramid/request.py index 0d3cf481b..62bd22589 100644 --- a/src/pyramid/request.py +++ b/src/pyramid/request.py @@ -352,10 +352,20 @@ class RequestLocalCache: result = ... # do some expensive computations return result + value = get_user(request) + + # manipulate the cache directly + get_user.cache.clear(request) + + The cache instance is attached to the resulting function as the ``cache`` + attribute such that the function may be used to manipulate the cache. + Wrapping Methods - A method can be wrapped but it needs to be bound to an instance such that - it only accepts one argument - the request. + A method can be used as the creator function but it needs to be bound to + an instance such that it only accepts one argument - the request. An easy + way to do this is to bind the creator in the constructor and then use + :meth:`.get_or_create`: .. code-block:: python @@ -386,26 +396,6 @@ class RequestLocalCache: self._creator = creator def __call__(self, fn): - """ - Decorate and return a new function that utilizes the cache. - - The cache is attached as an attribute to the decorated function - such that it may be manipulated directly. For example: - - .. code-block:: python - - @RequestLocalCache() - def do_something_expensive(request): - return ... - - value = do_something_expensive(request) - do_something_expensive.cache.clear(request) - - The ``fn`` is also bound as the creator on the cache such that - invocations of :meth:`.get_or_create` will use it. - - """ - @functools.wraps(fn) def wrapper(request): return wrapper.cache.get_or_create(request, fn) @@ -416,13 +406,13 @@ class RequestLocalCache: def get_or_create(self, request, creator=None): """ - Return the cached value. + Return the value from the cache. Compute if necessary. If no value is cached then execute the creator, cache the result, and return it. The creator may be passed in as an argument or bound to the cache - using the ``__call__`` or constructor arguments. + by decorating a function or supplied as a constructor argument. """ result = self._store.get(request, self.NO_VALUE) -- cgit v1.2.3