From fc531a245627fd1104384a6734eea5ae9ddd6d80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Fri, 21 Aug 2020 10:27:55 +0200 Subject: [PATCH] Remove internal Response class, use PSR-7 response instead --- src/Client/Request.php | 66 +------- src/Client/Response.php | 173 --------------------- src/Io/Sender.php | 21 +-- tests/Client/FunctionalIntegrationTest.php | 19 +-- tests/Client/RequestTest.php | 168 ++------------------ tests/Client/ResponseTest.php | 103 ------------ 6 files changed, 36 insertions(+), 514 deletions(-) delete mode 100644 src/Client/Response.php delete mode 100644 tests/Client/ResponseTest.php diff --git a/src/Client/Request.php b/src/Client/Request.php index 7ebb627f..51e03313 100644 --- a/src/Client/Request.php +++ b/src/Client/Request.php @@ -138,7 +138,8 @@ public function handleData($data) // buffer until double CRLF (or double LF for compatibility with legacy servers) if (false !== strpos($this->buffer, "\r\n\r\n") || false !== strpos($this->buffer, "\n\n")) { try { - list($response, $bodyChunk) = $this->parseResponse($this->buffer); + $response = gPsr\parse_response($this->buffer); + $bodyChunk = (string) $response->getBody(); } catch (\InvalidArgumentException $exception) { $this->emit('error', array($exception)); } @@ -155,17 +156,9 @@ public function handleData($data) return; } - $response->on('close', array($this, 'close')); - $that = $this; - $response->on('error', function (\Exception $error) use ($that) { - $that->closeError(new \RuntimeException( - "An error occured in the response", - 0, - $error - )); - }); + $this->stream->on('close', array($this, 'handleClose')); - $this->emit('response', array($response, $this)); + $this->emit('response', array($response, $this->stream)); $this->stream->emit('data', array($bodyChunk)); } @@ -222,30 +215,6 @@ public function close() $this->removeAllListeners(); } - protected function parseResponse($data) - { - $psrResponse = gPsr\parse_response($data); - $headers = array_map(function($val) { - if (1 === count($val)) { - $val = $val[0]; - } - - return $val; - }, $psrResponse->getHeaders()); - - $factory = $this->getResponseFactory(); - - $response = $factory( - 'HTTP', - $psrResponse->getProtocolVersion(), - $psrResponse->getStatusCode(), - $psrResponse->getReasonPhrase(), - $headers - ); - - return array($response, (string)($psrResponse->getBody())); - } - protected function connect() { $scheme = $this->requestData->getScheme(); @@ -265,31 +234,4 @@ protected function connect() return $this->connector ->connect($host . ':' . $port); } - - public function setResponseFactory($factory) - { - $this->responseFactory = $factory; - } - - public function getResponseFactory() - { - if (null === $factory = $this->responseFactory) { - $stream = $this->stream; - - $factory = function ($protocol, $version, $code, $reasonPhrase, $headers) use ($stream) { - return new Response( - $stream, - $protocol, - $version, - $code, - $reasonPhrase, - $headers - ); - }; - - $this->responseFactory = $factory; - } - - return $factory; - } } diff --git a/src/Client/Response.php b/src/Client/Response.php deleted file mode 100644 index 132e0a54..00000000 --- a/src/Client/Response.php +++ /dev/null @@ -1,173 +0,0 @@ -stream = $stream; - $this->protocol = $protocol; - $this->version = $version; - $this->code = $code; - $this->reasonPhrase = $reasonPhrase; - $this->headers = $headers; - - $this->stream->on('data', array($this, 'handleData')); - $this->stream->on('error', array($this, 'handleError')); - $this->stream->on('end', array($this, 'handleEnd')); - $this->stream->on('close', array($this, 'handleClose')); - } - - public function getProtocol() - { - return $this->protocol; - } - - public function getVersion() - { - return $this->version; - } - - public function getCode() - { - return $this->code; - } - - public function getReasonPhrase() - { - return $this->reasonPhrase; - } - - public function getHeaders() - { - return $this->headers; - } - - private function getHeader($name) - { - $name = strtolower($name); - $normalized = array_change_key_case($this->headers, CASE_LOWER); - - return isset($normalized[$name]) ? (array)$normalized[$name] : array(); - } - - /** - * @param string $name - * @return string - */ - public function getHeaderLine($name) - { - return implode(', ' , $this->getHeader($name)); - } - - /** - * @param string $name - * @return bool - */ - public function hasHeader($name) - { - return $this->getHeader($name) !== array(); - } - - /** @internal */ - public function handleData($data) - { - if ($this->readable) { - $this->emit('data', array($data)); - } - } - - /** @internal */ - public function handleEnd() - { - if (!$this->readable) { - return; - } - $this->emit('end'); - $this->close(); - } - - /** @internal */ - public function handleError(\Exception $error) - { - if (!$this->readable) { - return; - } - $this->emit('error', array(new \RuntimeException( - "An error occurred in the underlying stream", - 0, - $error - ))); - - $this->close(); - } - - /** @internal */ - public function handleClose() - { - $this->close(); - } - - public function close() - { - if (!$this->readable) { - return; - } - - $this->readable = false; - $this->stream->close(); - - $this->emit('close'); - $this->removeAllListeners(); - } - - public function isReadable() - { - return $this->readable; - } - - public function pause() - { - if (!$this->readable) { - return; - } - - $this->stream->pause(); - } - - public function resume() - { - if (!$this->readable) { - return; - } - - $this->stream->resume(); - } - - public function pipe(WritableStreamInterface $dest, array $options = array()) - { - Util::pipe($this, $dest, $options); - - return $dest; - } -} diff --git a/src/Io/Sender.php b/src/Io/Sender.php index 353fa536..328b68ad 100644 --- a/src/Io/Sender.php +++ b/src/Io/Sender.php @@ -3,6 +3,7 @@ namespace React\Http\Io; use Psr\Http\Message\RequestInterface; +use Psr\Http\Message\ResponseInterface; use React\EventLoop\LoopInterface; use React\Http\Client\Client as HttpClient; use React\Http\Client\Response as ResponseStream; @@ -108,26 +109,18 @@ public function send(RequestInterface $request) $deferred->reject($error); }); - $requestStream->on('response', function (ResponseStream $responseStream) use ($deferred, $request) { + $requestStream->on('response', function (ResponseInterface $response, ReadableStreamInterface $body) use ($deferred, $request) { $length = null; - $body = $responseStream; - $code = $responseStream->getCode(); + $code = $response->getStatusCode(); if ($request->getMethod() === 'HEAD' || ($code >= 100 && $code < 200) || $code == 204 || $code == 304) { $length = 0; - } elseif (\strtolower($responseStream->getHeaderLine('Transfer-Encoding')) === 'chunked') { + } elseif (\strtolower($response->getHeaderLine('Transfer-Encoding')) === 'chunked') { $body = new ChunkedDecoder($body); - } elseif ($responseStream->hasHeader('Content-Length')) { - $length = (int) $responseStream->getHeaderLine('Content-Length'); + } elseif ($response->hasHeader('Content-Length')) { + $length = (int) $response->getHeaderLine('Content-Length'); } - // apply response header values from response stream - $deferred->resolve(new Response( - $responseStream->getCode(), - $responseStream->getHeaders(), - new ReadableBodyStream($body, $length), - $responseStream->getVersion(), - $responseStream->getReasonPhrase() - )); + $deferred->resolve($response->withBody(new ReadableBodyStream($body, $length))); }); if ($body instanceof ReadableStreamInterface) { diff --git a/tests/Client/FunctionalIntegrationTest.php b/tests/Client/FunctionalIntegrationTest.php index db82b1f1..57861f2c 100644 --- a/tests/Client/FunctionalIntegrationTest.php +++ b/tests/Client/FunctionalIntegrationTest.php @@ -3,13 +3,14 @@ namespace React\Tests\Http\Client; use Clue\React\Block; +use Psr\Http\Message\ResponseInterface; use React\EventLoop\Factory; use React\Http\Client\Client; -use React\Http\Client\Response; use React\Promise\Deferred; use React\Promise\Stream; use React\Socket\Server; use React\Socket\ConnectionInterface; +use React\Stream\ReadableStreamInterface; use React\Tests\Http\TestCase; class FunctionalIntegrationTest extends TestCase @@ -69,8 +70,8 @@ public function testRequestLegacyHttpServerWithOnlyLineFeedReturnsSuccessfulResp $request = $client->request('GET', str_replace('tcp:', 'http:', $server->getAddress())); $once = $this->expectCallableOnceWith('body'); - $request->on('response', function (Response $response) use ($once) { - $response->on('data', $once); + $request->on('response', function (ResponseInterface $response, ReadableStreamInterface $body) use ($once) { + $body->on('data', $once); }); $promise = Stream\first($request, 'close'); @@ -88,8 +89,8 @@ public function testSuccessfulResponseEmitsEnd() $request = $client->request('GET', 'http://www.google.com/'); $once = $this->expectCallableOnce(); - $request->on('response', function (Response $response) use ($once) { - $response->on('end', $once); + $request->on('response', function (ResponseInterface $response, ReadableStreamInterface $body) use ($once) { + $body->on('end', $once); }); $promise = Stream\first($request, 'close'); @@ -112,8 +113,8 @@ public function testPostDataReturnsData() $request = $client->request('POST', 'https://' . (mt_rand(0, 1) === 0 ? 'eu.' : '') . 'httpbin.org/post', array('Content-Length' => strlen($data))); $deferred = new Deferred(); - $request->on('response', function (Response $response) use ($deferred) { - $deferred->resolve(Stream\buffer($response)); + $request->on('response', function (ResponseInterface $response, ReadableStreamInterface $body) use ($deferred) { + $deferred->resolve(Stream\buffer($body)); }); $request->on('error', 'printf'); @@ -145,8 +146,8 @@ public function testPostJsonReturnsData() $request = $client->request('POST', 'https://httpbin.org/post', array('Content-Length' => strlen($data), 'Content-Type' => 'application/json')); $deferred = new Deferred(); - $request->on('response', function (Response $response) use ($deferred) { - $deferred->resolve(Stream\buffer($response)); + $request->on('response', function (ResponseInterface $response, ReadableStreamInterface $body) use ($deferred) { + $deferred->resolve(Stream\buffer($body)); }); $request->on('error', 'printf'); diff --git a/tests/Client/RequestTest.php b/tests/Client/RequestTest.php index e702d315..207aa15f 100644 --- a/tests/Client/RequestTest.php +++ b/tests/Client/RequestTest.php @@ -26,10 +26,6 @@ public function setUpStream() $this->connector = $this->getMockBuilder('React\Socket\ConnectorInterface') ->getMock(); - - $this->response = $this->getMockBuilder('React\Http\Client\Response') - ->disableOriginalConstructor() - ->getMock(); } /** @test */ @@ -81,48 +77,13 @@ public function requestShouldBindToStreamEventsAndUseconnector() ->method('removeListener') ->with('close', $this->identicalTo(array($request, 'handleClose'))); - $response = $this->response; - - $this->stream->expects($this->once()) - ->method('emit') - ->with('data', $this->identicalTo(array('body'))); - - $response->expects($this->at(0)) - ->method('on') - ->with('close', $this->anything()) - ->will($this->returnCallback(function ($event, $cb) use (&$endCallback) { - $endCallback = $cb; - })); - - $factory = $this->createCallableMock(); - $factory->expects($this->once()) - ->method('__invoke') - ->with('HTTP', '1.0', '200', 'OK', array('Content-Type' => 'text/plain')) - ->will($this->returnValue($response)); - - $request->setResponseFactory($factory); - - $handler = $this->createCallableMock(); - $handler->expects($this->once()) - ->method('__invoke') - ->with($response); - - $request->on('response', $handler); $request->on('end', $this->expectCallableNever()); - $handler = $this->createCallableMock(); - $handler->expects($this->once()) - ->method('__invoke'); - - $request->on('close', $handler); $request->end(); $request->handleData("HTTP/1.0 200 OK\r\n"); $request->handleData("Content-Type: text/plain\r\n"); $request->handleData("\r\nbody"); - - $this->assertNotNull($endCallback); - call_user_func($endCallback); } /** @test */ @@ -209,7 +170,7 @@ public function requestShouldEmitErrorIfConnectionEmitsError() } /** @test */ - public function requestShouldEmitErrorIfGuzzleParseThrowsException() + public function requestShouldEmitErrorIfRequestParserThrowsException() { $requestData = new RequestData('GET', 'http://www.example.com'); $request = new Request($this->connector, $requestData); @@ -288,12 +249,6 @@ public function postRequestShouldSendAPostRequest() ->method('write') ->with($this->matchesRegularExpression("#^POST / HTTP/1\.0\r\nHost: www.example.com\r\nUser-Agent:.*\r\n\r\nsome post data$#")); - $factory = $this->createCallableMock(); - $factory->expects($this->once()) - ->method('__invoke') - ->will($this->returnValue($this->response)); - - $request->setResponseFactory($factory); $request->end('some post data'); $request->handleData("HTTP/1.0 200 OK\r\n"); @@ -322,13 +277,6 @@ public function writeWithAPostRequestShouldSendToTheStream() ->method('write') ->with($this->identicalTo("data")); - $factory = $this->createCallableMock(); - $factory->expects($this->once()) - ->method('__invoke') - ->will($this->returnValue($this->response)); - - $request->setResponseFactory($factory); - $request->write("some"); $request->write("post"); $request->end("data"); @@ -356,13 +304,6 @@ public function writeWithAPostRequestShouldSendBodyAfterHeadersAndEmitDrainEvent ->method('write') ->with($this->identicalTo("data")); - $factory = $this->createCallableMock(); - $factory->expects($this->once()) - ->method('__invoke') - ->will($this->returnValue($this->response)); - - $request->setResponseFactory($factory); - $this->assertFalse($request->write("some")); $this->assertFalse($request->write("post")); @@ -402,13 +343,6 @@ public function writeWithAPostRequestShouldForwardDrainEventIfFirstChunkExceedsB ->method('write') ->with($this->identicalTo("data")); - $factory = $this->createCallableMock(); - $factory->expects($this->once()) - ->method('__invoke') - ->will($this->returnValue($this->response)); - - $request->setResponseFactory($factory); - $this->assertFalse($request->write("some")); $this->assertFalse($request->write("post")); @@ -447,17 +381,10 @@ public function pipeShouldPipeDataIntoTheRequestBody() ->method('write') ->with($this->identicalTo("data")); - $factory = $this->createCallableMock(); - $factory->expects($this->once()) - ->method('__invoke') - ->will($this->returnValue($this->response)); - $loop = $this ->getMockBuilder('React\EventLoop\LoopInterface') ->getMock(); - $request->setResponseFactory($factory); - $stream = fopen('php://memory', 'r+'); $stream = new DuplexResourceStream($stream, $loop); @@ -572,43 +499,6 @@ public function closeShouldCancelPendingConnectionAttempt() $request->close(); } - /** @test */ - public function requestShouldRelayErrorEventsFromResponse() - { - $requestData = new RequestData('GET', 'http://www.example.com'); - $request = new Request($this->connector, $requestData); - - $this->successfulConnectionMock(); - - $response = $this->response; - - $response->expects($this->at(0)) - ->method('on') - ->with('close', $this->anything()); - $response->expects($this->at(1)) - ->method('on') - ->with('error', $this->anything()) - ->will($this->returnCallback(function ($event, $cb) use (&$errorCallback) { - $errorCallback = $cb; - })); - - $factory = $this->createCallableMock(); - $factory->expects($this->once()) - ->method('__invoke') - ->with('HTTP', '1.0', '200', 'OK', array('Content-Type' => 'text/plain')) - ->will($this->returnValue($response)); - - $request->setResponseFactory($factory); - $request->end(); - - $request->handleData("HTTP/1.0 200 OK\r\n"); - $request->handleData("Content-Type: text/plain\r\n"); - $request->handleData("\r\nbody"); - - $this->assertNotNull($errorCallback); - call_user_func($errorCallback, new \Exception('test')); - } - /** @test */ public function requestShouldRemoveAllListenerAfterClosed() { @@ -660,25 +550,12 @@ public function multivalueHeader() $this->successfulConnectionMock(); - $response = $this->response; - - $response->expects($this->at(0)) - ->method('on') - ->with('close', $this->anything()); - $response->expects($this->at(1)) - ->method('on') - ->with('error', $this->anything()) - ->will($this->returnCallback(function ($event, $cb) use (&$errorCallback) { - $errorCallback = $cb; - })); - - $factory = $this->createCallableMock(); - $factory->expects($this->once()) - ->method('__invoke') - ->with('HTTP', '1.0', '200', 'OK', array('Content-Type' => 'text/plain', 'X-Xss-Protection' => '1; mode=block', 'Cache-Control' => 'public, must-revalidate, max-age=0')) - ->will($this->returnValue($response)); - - $request->setResponseFactory($factory); + $response = null; + $request->on('response', $this->expectCallableOnce()); + $request->on('response', function ($value) use (&$response) { + $response = $value; + }); + $request->end(); $request->handleData("HTTP/1.0 200 OK\r\n"); @@ -687,28 +564,13 @@ public function multivalueHeader() $request->handleData("Cache-Control:public, must-revalidate, max-age=0\r\n"); $request->handleData("\r\nbody"); - $this->assertNotNull($errorCallback); - call_user_func($errorCallback, new \Exception('test')); - } - - /** @test */ - public function chunkedStreamDecoder() - { - $requestData = new RequestData('GET', 'http://www.example.com'); - $request = new Request($this->connector, $requestData); - - $this->successfulConnectionMock(); - - $request->end(); - - $this->stream->expects($this->once()) - ->method('emit') - ->with('data', array("1\r\nb\r")); - - $request->handleData("HTTP/1.0 200 OK\r\n"); - $request->handleData("Transfer-Encoding: chunked\r\n"); - $request->handleData("\r\n1\r\nb\r"); - $request->handleData("\n3\t\nody\r\n0\t\n\r\n"); - + /** @var \Psr\Http\Message\ResponseInterface $response */ + $this->assertInstanceOf('Psr\Http\Message\ResponseInterface', $response); + $this->assertEquals('1.0', $response->getProtocolVersion()); + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals('OK', $response->getReasonPhrase()); + $this->assertEquals('text/plain', $response->getHeaderLine('Content-Type')); + $this->assertEquals('1; mode=block', $response->getHeaderLine('X-Xss-Protection')); + $this->assertEquals('public, must-revalidate, max-age=0', $response->getHeaderLine('Cache-Control')); } } diff --git a/tests/Client/ResponseTest.php b/tests/Client/ResponseTest.php deleted file mode 100644 index 0e757619..00000000 --- a/tests/Client/ResponseTest.php +++ /dev/null @@ -1,103 +0,0 @@ -stream = $this->getMockBuilder('React\Stream\DuplexStreamInterface') - ->getMock(); - } - - /** @test */ - public function responseShouldEmitEndEventOnEnd() - { - $this->stream - ->expects($this->at(0)) - ->method('on') - ->with('data', $this->anything()); - $this->stream - ->expects($this->at(1)) - ->method('on') - ->with('error', $this->anything()); - $this->stream - ->expects($this->at(2)) - ->method('on') - ->with('end', $this->anything()); - $this->stream - ->expects($this->at(3)) - ->method('on') - ->with('close', $this->anything()); - - $response = new Response($this->stream, 'HTTP', '1.0', '200', 'OK', array('Content-Type' => 'text/plain')); - - $handler = $this->createCallableMock(); - $handler->expects($this->once()) - ->method('__invoke') - ->with('some data'); - - $response->on('data', $handler); - - $handler = $this->createCallableMock(); - $handler->expects($this->once()) - ->method('__invoke'); - - $response->on('end', $handler); - - $handler = $this->createCallableMock(); - $handler->expects($this->once()) - ->method('__invoke'); - - $response->on('close', $handler); - - $this->stream - ->expects($this->at(0)) - ->method('close'); - - $response->handleData('some data'); - $response->handleEnd(); - - $this->assertSame( - array( - 'Content-Type' => 'text/plain' - ), - $response->getHeaders() - ); - } - - /** @test */ - public function closedResponseShouldNotBeResumedOrPaused() - { - $response = new Response($this->stream, 'http', '1.0', '200', 'ok', array('content-type' => 'text/plain')); - - $this->stream - ->expects($this->never()) - ->method('pause'); - $this->stream - ->expects($this->never()) - ->method('resume'); - - $response->handleEnd(); - - $response->resume(); - $response->pause(); - - $this->assertSame( - array( - 'content-type' => 'text/plain', - ), - $response->getHeaders() - ); - } -} -