From 41e8aed688ecbdb24f13146b3c01fd42b4e92706 Mon Sep 17 00:00:00 2001 From: Bilge Date: Sun, 22 Apr 2018 01:28:06 +0100 Subject: [PATCH] Simplified AsyncHttpConnector by applying Porter 5 changes. Increased ExponentialBackoffExceptionHandler delay in functional test. --- src/AsyncHttpConnector.php | 43 +++++++++++++-------------- src/HttpConnector.php | 34 ++++++++++----------- test/FixtureFactory.php | 9 ++---- test/Functional/HttpConnectorTest.php | 35 +++++++++------------- 4 files changed, 52 insertions(+), 69 deletions(-) diff --git a/src/AsyncHttpConnector.php b/src/AsyncHttpConnector.php index dac5493..6e813d5 100644 --- a/src/AsyncHttpConnector.php +++ b/src/AsyncHttpConnector.php @@ -8,10 +8,10 @@ use Amp\Artax\Response; use Amp\Artax\SocketException; use Amp\Artax\TimeoutException; -use Amp\Promise; use ScriptFUSION\Porter\Connector\AsyncConnector; use ScriptFUSION\Porter\Connector\ConnectionContext; use ScriptFUSION\Porter\Connector\ConnectorOptions; +use ScriptFUSION\Porter\Options\EncapsulatedOptions; class AsyncHttpConnector implements AsyncConnector, ConnectorOptions { @@ -27,31 +27,28 @@ public function __clone() $this->options = clone $this->options; } - public function fetchAsync(ConnectionContext $context, string $source): Promise + public function fetchAsync(string $source, ConnectionContext $context) { - return \Amp\call(function () use ($context, $source) { - $client = new DefaultClient($this->getOptions()->getCookieJar()); - $client->setOptions($this->getOptions()->extractArtaxOptions()); - - return $context->retryAsync( - static function () use ($client, $source) { - try { - /** @var Response $response */ - $response = yield $client->request($source); - $body = yield $response->getBody(); - // Retry HTTP timeouts, socket timeouts and DNS resolution errors. - } catch (TimeoutException | SocketException | DnsException $exception) { - // Convert exception to recoverable exception. - throw new HttpConnectionException($exception->getMessage(), $exception->getCode(), $exception); - } - - return HttpResponse::fromArtaxResponse($response, $body); - } - ); - }); + $client = new DefaultClient($this->options->getCookieJar()); + $client->setOptions($this->options->extractArtaxOptions()); + + try { + /** @var Response $response */ + $response = yield $client->request($source); + $body = yield $response->getBody(); + // Retry HTTP timeouts, socket timeouts and DNS resolution errors. + } catch (TimeoutException | SocketException | DnsException $exception) { + // Convert exception to recoverable exception. + throw new HttpConnectionException($exception->getMessage(), $exception->getCode(), $exception); + } + + return HttpResponse::fromArtaxResponse($response, $body); } - public function getOptions() + /** + * @return ArtaxHttpOptions + */ + public function getOptions(): EncapsulatedOptions { return $this->options; } diff --git a/src/HttpConnector.php b/src/HttpConnector.php index 97d86ed..633367f 100644 --- a/src/HttpConnector.php +++ b/src/HttpConnector.php @@ -6,6 +6,7 @@ use ScriptFUSION\Porter\Connector\ConnectionContext; use ScriptFUSION\Porter\Connector\Connector; use ScriptFUSION\Porter\Connector\ConnectorOptions; +use ScriptFUSION\Porter\Options\EncapsulatedOptions; /** * Fetches data from an HTTP server via the PHP wrapper. @@ -15,7 +16,6 @@ */ class HttpConnector implements Connector, ConnectorOptions { - /** @var HttpOptions */ private $options; public function __construct(HttpOptions $options = null) @@ -31,8 +31,8 @@ public function __clone() /** * {@inheritdoc} * - * @param ConnectionContext $context Runtime connection settings and methods. * @param string $source Source. + * @param ConnectionContext $context Runtime connection settings and methods. * * @return HttpResponse Response. * @@ -40,7 +40,7 @@ public function __clone() * @throws HttpConnectionException Failed to connect to source. * @throws HttpServerException Server sent an error code. */ - public function fetch(ConnectionContext $context, $source) + public function fetch(string $source, ConnectionContext $context) { $streamContext = stream_context_create([ 'http' => @@ -51,29 +51,27 @@ public function fetch(ConnectionContext $context, $source) 'ssl' => $this->options->getSslOptions()->extractSslContextOptions(), ]); - return $context->retry(static function () use ($source, $streamContext) { - if (false === $body = @file_get_contents($source, false, $streamContext)) { - $error = error_get_last(); - throw new HttpConnectionException($error['message'], $error['type']); - } + if (false === $body = @file_get_contents($source, false, $streamContext)) { + $error = error_get_last(); + throw new HttpConnectionException($error['message'], $error['type']); + } - $response = HttpResponse::fromPhpWrapper($http_response_header, $body); + $response = HttpResponse::fromPhpWrapper($http_response_header, $body); - if ($response->getStatusCode() < 200 || $response->getStatusCode() >= 400) { - throw new HttpServerException( - "HTTP server responded with error: \"{$response->getReasonPhrase()}\".\n\n$response", - $response - ); - } + if ($response->getStatusCode() < 200 || $response->getStatusCode() >= 400) { + throw new HttpServerException( + "HTTP server responded with error: \"{$response->getReasonPhrase()}\".\n\n$response", + $response + ); + } - return $response; - }); + return $response; } /** * @return HttpOptions */ - public function getOptions() + public function getOptions(): EncapsulatedOptions { return $this->options; } diff --git a/test/FixtureFactory.php b/test/FixtureFactory.php index 7241b65..1dc453d 100644 --- a/test/FixtureFactory.php +++ b/test/FixtureFactory.php @@ -2,7 +2,6 @@ namespace ScriptFUSIONTest; use ScriptFUSION\Porter\Connector\ConnectionContext; -use ScriptFUSION\Porter\Connector\FetchExceptionHandler\FetchExceptionHandler; use ScriptFUSION\StaticClass; final class FixtureFactory @@ -14,12 +13,8 @@ final class FixtureFactory * * @return ConnectionContext */ - public static function createConnectionContext() + public static function createConnectionContext(): ConnectionContext { - return new ConnectionContext( - false, - \Mockery::spy(FetchExceptionHandler::class), - 1 - ); + return new ConnectionContext(false); } } diff --git a/test/Functional/HttpConnectorTest.php b/test/Functional/HttpConnectorTest.php index c3c7f73..142604f 100644 --- a/test/Functional/HttpConnectorTest.php +++ b/test/Functional/HttpConnectorTest.php @@ -3,6 +3,7 @@ namespace ScriptFUSIONTest\Functional\Porter\Net\Http; +use Amp\Coroutine; use ScriptFUSION\Porter\Connector\Connector; use ScriptFUSION\Porter\Net\Http\AsyncHttpConnector; use ScriptFUSION\Porter\Net\Http\HttpConnectionException; @@ -95,30 +96,24 @@ public function testAsyncConnectionToLocalWebserver(): void public function testConnectionTimeout(): void { - try { - $this->fetch(); + $this->setExpectedException(HttpConnectionException::class); - self::fail('Expected FailingTooHardException exception.'); - } catch (FailingTooHardException $exception) { - self::assertInstanceOf(HttpConnectionException::class, $exception->getPrevious()); - } + $this->fetch(); } public function testErrorResponse(): void { $server = $this->startServer(); + $this->setExpectedException(HttpServerException::class); + try { $this->fetch('404.php'); + } catch (HttpServerException $exception) { + $this->assertStringEndsWith('foo', $exception->getMessage()); + $this->assertSame('foo', $exception->getResponse()->getBody()); - self::fail('Expected FailingTooHardException exception.'); - } catch (FailingTooHardException $exception) { - /** @var HttpServerException $innerException */ - self::assertInstanceOf(HttpServerException::class, $innerException = $exception->getPrevious()); - - self::assertSame(404, $innerException->getResponse()->getStatusCode()); - self::assertSame('foo', $innerException->getResponse()->getBody()); - self::assertStringEndsWith("\n\nfoo", $innerException->getMessage()); + throw $exception; } finally { $this->stopServer($server); } @@ -204,17 +199,17 @@ private function fetch(string $url = self::URI) $fullUrl = 'http://' . self::HOST . "/$url"; if ($this->connector instanceof AsyncHttpConnector) { - return \Amp\Promise\wait($this->connector->fetchAsync($context, $fullUrl)); + return \Amp\Promise\wait(new Coroutine($this->connector->fetchAsync($fullUrl, $context))); } - return $this->connector->fetch($context, $fullUrl); + return $this->connector->fetch($fullUrl, $context); } private function fetchViaSsl(Connector $connector) { return $connector->fetch( - FixtureFactory::createConnectionContext(), - 'https://' . self::SSL_HOST . '/' . self::URI + 'https://' . self::SSL_HOST . '/' . self::URI, + FixtureFactory::createConnectionContext() ); } @@ -229,9 +224,7 @@ private static function waitForHttpServer(\Closure $serverInvoker): void ImportSpecification::DEFAULT_FETCH_ATTEMPTS, $serverInvoker, function (\Exception $exception) { - if (!$exception instanceof FailingTooHardException - || !$exception->getPrevious() instanceof HttpConnectionException - ) { + if (!$exception instanceof HttpConnectionException) { return false; }