Merge pull request #1373 from dobesv/patch-5

Allow the last callback called to add a callback

5 files changed, 50 insertions, 23 deletions

diff --git a/pyramid/request.py b/pyramid/request.py
index 6318049ee..bc2889310 100644
--- a/pyramid/request.py
+++ b/pyramid/request.py
@@ -1,3 +1,4 @@
+from collections import deque
 import json
 
 from zope.interface import implementer
@@ -32,8 +33,8 @@ class TemplateContext(object):
     pass
 
 class CallbackMethodsMixin(object):
-    response_callbacks = ()
-    finished_callbacks = ()
+    response_callbacks = None
+    finished_callbacks = None
     def add_response_callback(self, callback):
         """
         Add a callback to the set of callbacks to be called by the
@@ -72,15 +73,15 @@ class CallbackMethodsMixin(object):
         """
 
         callbacks = self.response_callbacks
-        if not callbacks:
-            callbacks = []
+        if callbacks is None:
+            callbacks = deque()
         callbacks.append(callback)
         self.response_callbacks = callbacks
 
     def _process_response_callbacks(self, response):
         callbacks = self.response_callbacks
         while callbacks:
-            callback = callbacks.pop(0)
+            callback = callbacks.popleft()
             callback(self, response)
 
     def add_finished_callback(self, callback):
@@ -132,15 +133,15 @@ class CallbackMethodsMixin(object):
         """
 
         callbacks = self.finished_callbacks
-        if not callbacks:
-            callbacks = []
+        if callbacks is None:
+            callbacks = deque()
         callbacks.append(callback)
         self.finished_callbacks = callbacks
 
     def _process_finished_callbacks(self):
         callbacks = self.finished_callbacks
         while callbacks:
-            callback = callbacks.pop(0)
+            callback = callbacks.popleft()
             callback(self)
 
 @implementer(IRequest)
diff --git a/pyramid/tests/test_request.py b/pyramid/tests/test_request.py
index ed41b62ff..48af98f59 100644
--- a/pyramid/tests/test_request.py
+++ b/pyramid/tests/test_request.py
@@ -1,3 +1,4 @@
+from collections import deque
 import unittest
 from pyramid import testing
 
@@ -119,13 +120,13 @@ class TestRequest(unittest.TestCase):
 
     def test_add_response_callback(self):
         inst = self._makeOne()
-        self.assertEqual(inst.response_callbacks, ())
+        self.assertEqual(inst.response_callbacks, None)
         def callback(request, response):
             """ """
         inst.add_response_callback(callback)
-        self.assertEqual(inst.response_callbacks, [callback])
+        self.assertEqual(list(inst.response_callbacks), [callback])
         inst.add_response_callback(callback)
-        self.assertEqual(inst.response_callbacks, [callback, callback])
+        self.assertEqual(list(inst.response_callbacks), [callback, callback])
 
     def test__process_response_callbacks(self):
         inst = self._makeOne()
@@ -135,24 +136,48 @@ class TestRequest(unittest.TestCase):
         def callback2(request, response):
             request.called2  = True
             response.called2 = True
-        inst.response_callbacks = [callback1, callback2]
+        inst.add_response_callback(callback1)
+        inst.add_response_callback(callback2)
         response = DummyResponse()
         inst._process_response_callbacks(response)
         self.assertEqual(inst.called1, True)
         self.assertEqual(inst.called2, True)
         self.assertEqual(response.called1, True)
         self.assertEqual(response.called2, True)
-        self.assertEqual(inst.response_callbacks, [])
+        self.assertEqual(len(inst.response_callbacks), 0)
+
+    def test__process_response_callback_adding_response_callback(self):
+        """
+        When a response callback adds another callback, that new callback should still be called.
+
+        See https://github.com/Pylons/pyramid/pull/1373
+        """
+        inst = self._makeOne()
+        def callback1(request, response):
+            request.called1 = True
+            response.called1 = True
+            request.add_response_callback(callback2)
+        def callback2(request, response):
+            request.called2  = True
+            response.called2 = True
+        inst.add_response_callback(callback1)
+        response = DummyResponse()
+        inst._process_response_callbacks(response)
+        self.assertEqual(inst.called1, True)
+        self.assertEqual(inst.called2, True)
+        self.assertEqual(response.called1, True)
+        self.assertEqual(response.called2, True)
+        self.assertEqual(len(inst.response_callbacks), 0)
 
     def test_add_finished_callback(self):
         inst = self._makeOne()
-        self.assertEqual(inst.finished_callbacks, ())
+        self.assertEqual(inst.finished_callbacks, None)
         def callback(request):
             """ """
         inst.add_finished_callback(callback)
-        self.assertEqual(inst.finished_callbacks, [callback])
+        self.assertEqual(list(inst.finished_callbacks), [callback])
         inst.add_finished_callback(callback)
-        self.assertEqual(inst.finished_callbacks, [callback, callback])
+        self.assertEqual(list(inst.finished_callbacks), [callback, callback])
 
     def test__process_finished_callbacks(self):
         inst = self._makeOne()
@@ -160,11 +185,12 @@ class TestRequest(unittest.TestCase):
             request.called1 = True
         def callback2(request):
             request.called2  = True
-        inst.finished_callbacks = [callback1, callback2]
+        inst.add_finished_callback(callback1)
+        inst.add_finished_callback(callback2)
         inst._process_finished_callbacks()
         self.assertEqual(inst.called1, True)
         self.assertEqual(inst.called2, True)
-        self.assertEqual(inst.finished_callbacks, [])
+        self.assertEqual(len(inst.finished_callbacks), 0)
 
     def test_resource_url(self):
         self._registerResourceURL()
diff --git a/pyramid/tests/test_router.py b/pyramid/tests/test_router.py
index 838e52db0..c6c6eea1c 100644
--- a/pyramid/tests/test_router.py
+++ b/pyramid/tests/test_router.py
@@ -522,7 +522,7 @@ class TestRouter(unittest.TestCase):
         def view(context, request):
             def callback(request, response):
                 response.called_back = True
-            request.response_callbacks = [callback]
+            request.add_response_callback(callback)
             return response
         environ = self._makeEnviron()
         self._registerView(view, '', IViewClassifier, IRequest, IContext)
@@ -545,7 +545,7 @@ class TestRouter(unittest.TestCase):
         def view(context, request):
             def callback(request):
                 request.environ['called_back'] = True
-            request.finished_callbacks = [callback]
+            request.add_finished_callback(callback)
             return response
         environ = self._makeEnviron()
         self._registerView(view, '', IViewClassifier, IRequest, IContext)
@@ -567,7 +567,7 @@ class TestRouter(unittest.TestCase):
         def view(context, request):
             def callback(request):
                 request.environ['called_back'] = True
-            request.finished_callbacks = [callback]
+            request.add_finished_callback(callback)
             raise NotImplementedError
         environ = self._makeEnviron()
         self._registerView(view, '', IViewClassifier, IRequest, IContext)
diff --git a/pyramid/tests/test_session.py b/pyramid/tests/test_session.py
index 35c234e99..b013ffa66 100644
--- a/pyramid/tests/test_session.py
+++ b/pyramid/tests/test_session.py
@@ -521,7 +521,7 @@ class Test_manage_accessed(unittest.TestCase):
         result = wrapper(session, 'a')
         self.assertEqual(result, 1)
         callbacks = request.response_callbacks
-        self.assertEqual(len(callbacks), 0)
+        if callbacks is not None: self.assertEqual(len(callbacks), 0)
 
 class Test_manage_changed(unittest.TestCase):
     def _makeOne(self, wrapped):
diff --git a/pyramid/tests/test_testing.py b/pyramid/tests/test_testing.py
index 2d0548b33..dfcad2a0c 100644
--- a/pyramid/tests/test_testing.py
+++ b/pyramid/tests/test_testing.py
@@ -217,7 +217,7 @@ class TestDummyRequest(unittest.TestCase):
     def test_add_response_callback(self):
         request = self._makeOne()
         request.add_response_callback(1)
-        self.assertEqual(request.response_callbacks, [1])
+        self.assertEqual(list(request.response_callbacks), [1])
 
     def test_registry_is_config_registry_when_setup_is_called_after_ctor(self):
         # see https://github.com/Pylons/pyramid/issues/165