From 30a10bbc036942c944101d88606914d13bbf0504 Mon Sep 17 00:00:00 2001 From: bennr01 Date: Tue, 13 Apr 2021 14:34:11 +0200 Subject: [PATCH] treq.testing.StubTreq: fix session persistence between requests Before this commit, twisted.web.server.Session objects were not persisted properly between requests. Thus, two requests with the same cookies would result in two different sessions. See #327 for details. --- changelog.d/327.bugfix.rst | 1 + docs/api.rst | 8 +++++++ docs/testing.rst | 3 +++ src/treq/test/test_testing.py | 45 +++++++++++++++++++++++++++++++++++ src/treq/testing.py | 20 ++++++++++++---- 5 files changed, 72 insertions(+), 5 deletions(-) create mode 100644 changelog.d/327.bugfix.rst diff --git a/changelog.d/327.bugfix.rst b/changelog.d/327.bugfix.rst new file mode 100644 index 00000000..e6391df7 --- /dev/null +++ b/changelog.d/327.bugfix.rst @@ -0,0 +1 @@ +``treq.testing.StubTreq`` now persists ``twisted.web.server.Session`` instances between requests. diff --git a/docs/api.rst b/docs/api.rst index b01ee010..30e76f77 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -114,6 +114,14 @@ StubTreq Objects asynchronous. In that case, after each write from the server, :meth:`flush()` must be called so the client can see it. + .. method:: cleanSessions() + + Clean up sessions to prevent leaving behind a dirty reactor. + + If you are using :obj:`StubTreq` with :obj:`twisted.web.server.Session` + objects, you most likely have to call this method once you are done, + for example during the tearDown of a unittest TestCase. + As the methods on :class:`treq.client.HTTPClient`: .. method:: request diff --git a/docs/testing.rst b/docs/testing.rst index 2f292b51..1cf504b9 100644 --- a/docs/testing.rst +++ b/docs/testing.rst @@ -57,3 +57,6 @@ This is superior to calling your resource's methods directly or passing mock obj Thus, the ``request`` object your code interacts with is a *real* :class:`twisted.web.server.Request` and behaves the same as it would in production. Note that if your resource returns :data:`~twisted.web.server.NOT_DONE_YET` you must keep a reference to the :class:`~treq.testing.RequestTraversalAgent` and call its :meth:`~treq.testing.RequestTraversalAgent.flush()` method to spin the memory reactor once the server writes additional data before the client will receive it. + +If you are using :class:`~twisted.web.server.Session` instances in the wrapped resource, you may need to call :meth:`~treq.testing.StubTreq.cleanSessions()` to expire all sessions and prevent an unclean/dirty reactor. + diff --git a/src/treq/test/test_testing.py b/src/treq/test/test_testing.py index eb804158..61308163 100644 --- a/src/treq/test/test_testing.py +++ b/src/treq/test/test_testing.py @@ -52,6 +52,18 @@ def render(self, request): return NOT_DONE_YET +class _SessionIdTestResource(Resource): + """ + Resource that returns the current session ID. + """ + isLeaf = True + + def render(self, request): + session = request.getSession() + uid = session.uid + return uid + + class StubbingTests(TestCase): """ Tests for :class:`StubTreq`. @@ -242,6 +254,39 @@ def test_handles_successful_asynchronous_requests_with_streaming(self): stub.flush() self.successResultOf(d) + def test_session_persistence_between_requests(self): + """ + Calling request.getSession() in the wrapped resource will return + a session with the same ID, until the sessions are cleaned. + """ + rsrc = _SessionIdTestResource() + stub = StubTreq(rsrc) + # request 1, getting original session ID + d = stub.request("method", "http://example.com/") + resp = self.successResultOf(d) + cookies = resp.cookies() + sid_1 = self.successResultOf(resp.content()) + # request 2, ensuring session ID stays the same + d = stub.request("method", "http://example.com/", cookies=cookies) + resp = self.successResultOf(d) + sid_2 = self.successResultOf(resp.content()) + self.assertEqual(sid_1, sid_2) + # request 3, ensuring the session IDs are different after cleaning + # or expiring the sessions + stub.cleanSessions() + d = stub.request("method", "http://example.com/") + resp = self.successResultOf(d) + cookies = resp.cookies() + sid_3 = self.successResultOf(resp.content()) + self.assertNotEqual(sid_1, sid_3) + # request 4, ensuring that once again the session IDs are the same + d = stub.request("method", "http://example.com/", cookies=cookies) + resp = self.successResultOf(d) + sid_4 = self.successResultOf(resp.content()) + self.assertEqual(sid_3, sid_4) + # when done, clean sessions to avoid leaving a dirty reactor behind + stub.cleanSessions() + class HasHeadersTests(TestCase): """ diff --git a/src/treq/testing.py b/src/treq/testing.py index df633beb..88a9b9ee 100644 --- a/src/treq/testing.py +++ b/src/treq/testing.py @@ -88,6 +88,7 @@ def __init__(self, rootResource): reactor=self._memoryReactor, endpointFactory=_EndpointFactory(self._memoryReactor)) self._rootResource = rootResource + self._serverFactory = Site(self._rootResource, reactor=self._memoryReactor) self._pumps = set() def request(self, method, uri, headers=None, bodyProducer=None): @@ -126,8 +127,7 @@ def check_already_called(r): # Create the protocol and fake transport for the client and server, # using the factory that was passed to the MemoryReactor for the # client, and a Site around our rootResource for the server. - serverFactory = Site(self._rootResource, reactor=self._memoryReactor) - serverProtocol = serverFactory.buildProtocol(clientAddress) + serverProtocol = self._serverFactory.buildProtocol(clientAddress) serverTransport = iosim.FakeTransport( serverProtocol, isServer=True, hostAddress=serverAddress, peerAddress=clientAddress) @@ -228,8 +228,8 @@ def __init__(self, resource): :param resource: A :obj:`Resource` object that provides the fake responses """ - _agent = RequestTraversalAgent(resource) - _client = HTTPClient(agent=_agent, + self._agent = RequestTraversalAgent(resource) + _client = HTTPClient(agent=self._agent, data_to_body_producer=_SynchronousProducer) for function_name in treq.__all__: function = getattr(_client, function_name, None) @@ -239,7 +239,17 @@ def __init__(self, resource): function = _reject_files(function) setattr(self, function_name, function) - self.flush = _agent.flush + self.flush = self._agent.flush + + def cleanSessions(self): + """ + Clean up sessions to prevent leaving behind a dirty reactor. + If you are using :obj:`StubTreq` with :obj:`twisted.web.server.Session` + objects, you most likely have to call this method once you are done, + for example during the tearDown of a unittest TestCase. + """ + for sid in list(self._agent._serverFactory.sessions.keys()): + self._agent._serverFactory.sessions[sid].expire() class StringStubbingResource(Resource):