Merge readbody-cancellation-6686-3: Implement cancellation for readBody.

Author: kaizhang, mithrandi Reviewers: itamar, glyph, herrwolfe Fixes: twisted#6686 git-svn-id: svn://svn.twistedmatrix.com/svn/Twisted/trunk@43388 bbbe8e31-12d6-0310-92fd-ac37d47ddeeb
yuankui · Oct 16, 2014 · f31a6b2 · f31a6b2
1 parent 35306e2
commit f31a6b2
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 8 deletions.
diff --git a/docs/web/howto/client.rst b/docs/web/howto/client.rst
@@ -318,7 +318,7 @@ application is not interested in the body, it should issue a
 
 
 If the body of the response isn't going to be consumed incrementally, then :api:`twisted.web.client.readBody <readBody>` can be used to get the body as a byte-string.
-This function returns a ``Deferred`` that fires with the body after the request has been completed.
+This function returns a ``Deferred`` that fires with the body after the request has been completed; cancelling this ``Deferred`` will close the connection to the HTTP server immediately.
 
 
 

diff --git a/twisted/web/client.py b/twisted/web/client.py
@@ -2071,9 +2071,25 @@ def readBody(response):
     @type response: L{IResponse} provider
 
     @return: A L{Deferred} which will fire with the body of the response.
+        Cancelling it will close the connection to the server immediately.
     """
-    d = defer.Deferred()
-    response.deliverBody(_ReadBodyProtocol(response.code, response.phrase, d))
+    def cancel(deferred):
+        """
+        Cancel a L{readBody} call, close the connection to the HTTP server
+        immediately.
+
+        @param deferred: The cancelled L{defer.Deferred}.
+        """
+        getattr(protocol.transport, 'abortConnection', lambda: None)()
+    d = defer.Deferred(cancel)
+    protocol = _ReadBodyProtocol(response.code, response.phrase, d)
+    response.deliverBody(protocol)
+    if getattr(protocol.transport, 'abortConnection', None) is None:
+        warnings.warn(
+            'Using readBody with a transport that does not implement '
+            'ITCPTransport',
+            category=DeprecationWarning,
+            stacklevel=2)
     return d
 
 

diff --git a/twisted/web/test/test_agent.py b/twisted/web/test/test_agent.py
@@ -2698,35 +2698,62 @@ def test_redirectToGet302(self):
 
 
 
+class AbortableStringTransport(StringTransport):
+    """
+    A version of L{StringTransport} that supports C{abortConnection}.
+    """
+    # This should be replaced by a common version in #6530.
+    aborting = False
+
+
+    def abortConnection(self):
+        """
+        A testable version of the C{ITCPTransport.abortConnection} method.
+
+        Since this is a special case of closing the connection,
+        C{loseConnection} is also called.
+        """
+        self.aborting = True
+        self.loseConnection()
+
+
+
 class DummyResponse(object):
     """
-    Fake L{IResponse} for testing readBody that just captures the protocol
-    passed to deliverBody.
+    Fake L{IResponse} for testing readBody that captures the protocol passed to
+    deliverBody and uses it to make a connection with a transport.
 
     @ivar protocol: After C{deliverBody} is called, the protocol it was called
         with.
+
+    @ivar transport: An instance created by calling C{transportFactory} which
+        is used by L{DummyResponse.protocol} to make a connection.
     """
 
     code = 200
     phrase = "OK"
 
-    def __init__(self, headers=None):
+    def __init__(self, headers=None, transportFactory=AbortableStringTransport):
         """
         @param headers: The headers for this response.  If C{None}, an empty
             L{Headers} instance will be used.
         @type headers: L{Headers}
+
+        @param transportFactory: A callable used to construct the transport.
         """
         if headers is None:
             headers = Headers()
         self.headers = headers
+        self.transport = transportFactory()
 
 
     def deliverBody(self, protocol):
         """
-        Just record the given protocol without actually delivering anything to
-        it.
+        Record the given protocol and use it to make a connection with
+        L{DummyResponse.transport}.
         """
         self.protocol = protocol
+        self.protocol.makeConnection(self.transport)
 
 
 
@@ -2747,6 +2774,18 @@ def test_success(self):
         self.assertEqual(self.successResultOf(d), "firstsecond")
 
 
+    def test_cancel(self):
+        """
+        When cancelling the L{Deferred} returned by L{client.readBody}, the
+        connection to the server will be aborted.
+        """
+        response = DummyResponse()
+        deferred = client.readBody(response)
+        deferred.cancel()
+        self.failureResultOf(deferred, defer.CancelledError)
+        self.assertTrue(response.transport.aborting)
+
+
     def test_withPotentialDataLoss(self):
         """
         If the full body of the L{IResponse} passed to L{client.readBody} is
@@ -2786,3 +2825,20 @@ def test_otherErrors(self):
         reason = self.failureResultOf(d)
         reason.trap(ConnectionLost)
         self.assertEqual(reason.value.args, ("mystery problem",))
+
+
+    def test_deprecatedTransport(self):
+        """
+        Calling L{client.readBody} with a transport that does not implement
+        L{twisted.internet.interfaces.ITCPTransport} produces a deprecation
+        warning, but no exception when cancelling.
+        """
+        response = DummyResponse(transportFactory=StringTransport)
+        d = self.assertWarns(
+            DeprecationWarning,
+            'Using readBody with a transport that does not implement '
+            'ITCPTransport',
+            __file__,
+            lambda: client.readBody(response))
+        d.cancel()
+        self.failureResultOf(d, defer.CancelledError)
diff --git a/twisted/web/topfiles/6686.feature b/twisted/web/topfiles/6686.feature
@@ -0,0 +1 @@
+The Deferred returned by twisted.web.client.readBody can now be cancelled.
Original file line number	Diff line number	Diff line change
Expand Up		@@ -318,7 +318,7 @@ application is not interested in the body, it should issue a


		If the body of the response isn't going to be consumed incrementally, then :api:`twisted.web.client.readBody <readBody>` can be used to get the body as a byte-string.
		This function returns a ``Deferred`` that fires with the body after the request has been completed.
		This function returns a ``Deferred`` that fires with the body after the request has been completed; cancelling this ``Deferred`` will close the connection to the HTTP server immediately.



Expand Down
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		The Deferred returned by twisted.web.client.readBody can now be cancelled.