Pyramid tween: Delete _v_ attributes between retries.

Fixes #54

CHANGES.rst

@@ -8,6 +8,11 @@
 
 - Add support for Python 3.9.
 - Move CI from Travis CI to Github Actions.
+- When the Pyramid tween retries, any volatile attributes (those
+  beginning with ``_v_``) in the request dictionary are deleted. This
+  is inspired by ``persistent`` and meant to facilitate safe caching.
+  When compared to events sent by the transaction loop, there is no
+  specified order. See `issue 54 <https://github.com/NextThought/nti.transactions/issues/54>`_.
 
 
 4.1.0 (2020-07-22)

src/nti/transactions/pyramid_tween.py

@@ -170,8 +170,17 @@ def prep_for_retry(self, attempts_remaining, tx, request): # pylint:disable=argu
 
         .. caution::
             This doesn't do anything with the WSGI
-            environment or request object; changes to those persist
+            environment; changes to that persist
             between retries.
+
+        .. versionchanged:: 4.2.0
+           Now, transient attributes of the request object (those whose
+           name begins with ``_v_``) are deleted by this function before
+           it returns for the second and susquent attempts.
+           Previously, there were left with their existing values.
+           This is intended to make request-specific caching easier,
+           following a similar model as :mod:`persistent` and allowing for
+           common patterns such as :class:`zope.cachedescriptors.property.Lazy`.
         """
         # make_body_seekable will copy wsgi.input if necessary,
         # otherwise it will rewind the copy to position zero
@@ -230,6 +239,13 @@ def prep_for_retry(self, attempts_remaining, tx, request): # pylint:disable=argu
                 # encoded data will never start with these values, they would be
                 # escaped. so this must be meant to be JSON
                 request.content_type = 'application/json'
+        if attempt_number > 0:
+            # We are making a retry. We need to reset transient state.
+            for attr_name in list(attr_name
+                                  for attr_name
+                                  in vars(request)
+                                  if attr_name.startswith('_v_')):
+                delattr(request, attr_name)
 
     def should_abort_due_to_no_side_effects(self, request): # pylint:disable=arguments-differ
         """
@@ -246,7 +262,7 @@ def should_veto_commit(self, response, request): # pylint:disable=arguments-diff
         """
         Tests with :func:`commit_veto`.
         """
-        return  commit_veto(request, response)
+        return commit_veto(request, response)
 
     def describe_transaction(self, request): # pylint:disable=arguments-differ
         return request.path_info
@@ -273,13 +289,13 @@ def run_handler(self, request): # pylint:disable=arguments-differ
             # Of course, this is only needed if the exception was actually
             # raised, not deliberately returned (commonly HTTPFound and the like
             # are returned)...raising those could have unintended consequences
-            request._nti_raised_exception = True
+            request._v_nti_raised_exception = True
             return e
 
     def __call__(self, request): # pylint:disable=arguments-differ
         result = TransactionLoop.__call__(self, request)  # not super() for speed
         if  isinstance(result, HTTPException) and \
-            getattr(request, '_nti_raised_exception', False):
+            getattr(request, '_v_nti_raised_exception', False):
             raise result
         return result
 

src/nti/transactions/tests/test_pyramid_tween.py

@@ -18,6 +18,7 @@
 from hamcrest import is_
 from hamcrest import none
 from hamcrest import has_entries
+from hamcrest import is_not as does_not
 
 from nti.testing.matchers import is_true
 from nti.testing.matchers import is_false
@@ -144,6 +145,42 @@ def handler(request):
         assert_that(environs[2], has_entries('retry.attempt', 2,
                                              'retry.attempts', 3))
 
+    def test_retry_attempts_reset_v_props(self):
+        environs = []
+        def handler(request):
+            if not environs:
+                # First time in.
+                request._non_transient_private = 42
+                request.non_transient_public = 24
+                # It wasn't reset before beginning
+                assert_that(request, has_attr('_v_from_before', 36))
+                del request._v_from_before
+            environs.append(request.environ.copy())
+            assert_that(request, does_not(has_attr('_v_foo1')))
+            assert_that(request, does_not(has_attr('_v_foo2')))
+            assert_that(request, does_not(has_attr('_v_from_before')))
+            assert_that(request, has_attr('_non_transient_private', 42))
+            assert_that(request, has_attr('non_transient_public', 24))
+            request._v_foo1 = 1
+            request._v_foo2 = 2
+
+            raise TransientError
+
+        loop = self._makeOne(handler)
+        req = MockRequest()
+        req._v_from_before = 36 # pylint:disable=attribute-defined-outside-init
+        try:
+            loop(req)
+        except TransientError:
+            pass
+        assert_that(environs, has_length(3))
+        assert_that(environs[0], has_entries('retry.attempt', 0,
+                                             'retry.attempts', 3))
+        assert_that(environs[1], has_entries('retry.attempt', 1,
+                                             'retry.attempts', 3))
+        assert_that(environs[2], has_entries('retry.attempt', 2,
+                                             'retry.attempts', 3))
+
     def test_is_last(self):
         is_lasts = []
         def handler(request):
@@ -241,7 +278,7 @@ def handler(_req):
         req = MockRequest()
         res = loop.run_handler(req)
 
-        assert_that(req, has_attr('_nti_raised_exception'))
+        assert_that(req, has_attr('_v_nti_raised_exception', True))
         assert_that(res, is_(HTTPBadRequest))
 
     def test_run_handler_throws(self):