Merge lp:~james-w/python-timeline-django/start-end-request into lp:python-timeline-django

[James Westby]

Save the environ before sending the wsgi_request_started signal.

Code that listens to the wsgi_request_started signal may want to use the
environ, but instead of using the one passed to the signal, it may
call get_environ() to get the saved version. That has to be the
current request's environ, not any previous request, so save the environ
before triggering the signal.

Hi,

This fixes the oopses we are currently seeing on sso. They are caused
because a stale timeline from the previous request is sometimes being used.

The commit message has a fuller explanation of the problem and the fix.

Thanks,

James

[Michael Nelson (michael.nelson)]

LGTM - it's not clear to me how this results in the OverlappingActionError - but makes sense (and I trust you :-) ).

review: Approve

=== modified file 'timeline_django/tests/test_wsgi.py'
--- timeline_django/tests/test_wsgi.py	2013-08-26 02:13:01 +0000
+++ timeline_django/tests/test_wsgi.py	2013-09-03 00:27:53 +0000
@@ -74,3 +74,22 @@
             pass
         wrapped_app(environ, start_response)
         self.assertEqual([environ], called_environs)
+
+    def test_sends_wsgi_request_started_after_saving_environ(self):
+        # Code that listens to the wsgi_request_started signal may
+        # want to use the environ, but instead of using the one
+        # passed to the signal, they may call get_environ() to get
+        # the saved version. That has to be the current request's
+        # environ, not any previous request.
+        signal = self.patch_signal('wsgi_request_started')
+        called_environs = []
+        def capture_signal(sender, environ, signal=None):
+            # Save the environ that we get from get_environ() at
+            # the time of the signal
+            called_environs.append(wsgi.get_environ())
+        signal.connect(capture_signal)
+        wrapped_app = wsgi.make_app(lambda environ, start_response: None)
+        environ = object()
+        wrapped_app(environ, None)
+        # Check that the saved environ matches the one passed to the app
+        self.assertEqual([environ], called_environs)
 
=== modified file 'timeline_django/wsgi.py'
--- timeline_django/wsgi.py	2013-08-26 02:13:01 +0000
+++ timeline_django/wsgi.py	2013-09-03 00:27:53 +0000
@@ -45,9 +45,9 @@
     :return: A WSGI app wrapping application.
     """
     def save_environ(environ, start_response):
+        _thread_local.environ = environ
         signals.wsgi_request_started.send(sender=save_environ,
                                           environ=environ)
-        _thread_local.environ = environ
         def signalled_start_response(*args, **kwargs):
             signals.wsgi_response_started.send(sender=save_environ,
                                                environ=environ)