Merge branch 'master' into security-docs

9 files changed, 365 insertions, 36 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 9f16b06ea..0e32a40c6 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -71,6 +71,29 @@ 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
+
+- Finished callbacks are now executed as part of the ``closer`` that is
+  invoked as part of ``pyramid.scripting.prepare`` and
+  ``pyramid.paster.bootstrap``.
+  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
+  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.
+  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:
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/request.py b/src/pyramid/request.py
index d65be2a2f..62bd22589 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,134 @@ 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.
+
+    Wrapping Functions
+
+    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
+
+        @RequestLocalCache()
+        def get_user(request):
+            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 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
+
+        class SecurityPolicy:
+            def __init__(self):
+                self.identity_cache = RequestLocalCache(self.load_identity)
+
+            def load_identity(self, request):
+                result = ...  # do some expensive computations
+                return result
+
+            def authenticated_identity(self, request):
+                return self.identity_cache.get_or_create(request)
+
+    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
+
+    """
+
+    NO_VALUE = Sentinel('NO_VALUE')
+
+    def __init__(self, creator=None):
+        self._store = weakref.WeakKeyDictionary()
+        self._creator = creator
+
+    def __call__(self, fn):
+        @functools.wraps(fn)
+        def wrapper(request):
+            return wrapper.cache.get_or_create(request, fn)
+
+        wrapper.cache = self
+        self._creator = fn
+        return wrapper
+
+    def get_or_create(self, request, creator=None):
+        """
+        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
+        by decorating a function or supplied as a constructor argument.
+
+        """
+        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):
+        """
+        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.
+
+        """
+        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/src/pyramid/scripting.py b/src/pyramid/scripting.py
index abcdd1030..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)
@@ -94,6 +100,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/src/pyramid/util.py b/src/pyramid/util.py
index e552b37de..504516631 100644
--- a/src/pyramid/util.py
+++ b/src/pyramid/util.py
@@ -73,6 +73,26 @@ def as_sorted_tuple(val):
     return val
 
 
+class SettableProperty(object):
+    def __init__(self, wrapped):
+        self.wrapped = wrapped
+        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.wrapped.__name__, _marker)
+        if value is _marker:
+            value = self.wrapped(obj)
+        return value
+
+    def __set__(self, obj, value):
+        obj.__dict__[self.wrapped.__name__] = value
+
+    def __delete__(self, obj):
+        del obj.__dict__[self.wrapped.__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
@@ -94,26 +114,29 @@ 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__
+        name = get_callable_name(name)
+        is_data_descriptor = hasattr(callable, '__set__')
+        if reify and is_data_descriptor:
+            raise ValueError('cannot reify a data descriptor')
+        if is_data_descriptor:
             fn = callable
-        if reify:
-            import pyramid.decorator  # avoid circular import
+        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(fn)
-        elif not is_property:
-            fn = property(fn)
+                fn = pyramid.decorator.reify(wrapped)
+            else:
+                fn = SettableProperty(wrapped)
 
         return name, fn
 
diff --git a/tests/test_request.py b/tests/test_request.py
index bbf6aa47c..3c5535b0e 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,124 @@ class Test_subclassing_Request(unittest.TestCase):
         self.assertTrue(IRequest.implementedBy(RequestSub))
 
 
+class TestRequestLocalCache(unittest.TestCase):
+    def _makeOne(self, *args, **kwargs):
+        from pyramid.request import RequestLocalCache
+
+        return RequestLocalCache(*args, **kwargs)
+
+    def test_it_works_with_functions(self):
+        a = [0]
+
+        @self._makeOne()
+        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_clear_works(self):
+        a = [0]
+
+        @self._makeOne()
+        def foo(request):
+            a[0] += 1
+            return a[0]
+
+        req = DummyRequest()
+        self.assertEqual(foo(req), 1)
+        self.assertEqual(len(req.finished_callbacks), 1)
+        foo.cache.clear(req)
+        self.assertEqual(foo(req), 2)
+        self.assertEqual(len(req.finished_callbacks), 1)
+
+    def test_set_overrides_current_value(self):
+        a = [0]
+
+        @self._makeOne()
+        def foo(request):
+            a[0] += 1
+            return a[0]
+
+        req = DummyRequest()
+        self.assertEqual(foo(req), 1)
+        self.assertEqual(len(req.finished_callbacks), 1)
+        foo.cache.set(req, 8)
+        self.assertEqual(foo(req), 8)
+        self.assertEqual(len(req.finished_callbacks), 1)
+        self.assertEqual(foo.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)
+
+    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
+
+
 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
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:
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