From f4d0b59ecfd00e93f298dab088af8ea5be75401e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Wed, 20 Feb 2019 12:17:15 +0100 Subject: [PATCH] Cancellation support, timeout option and improve long paginations --- README.md | 70 ++++++++++++++++++++++++++++++++++++++++++++ composer.json | 2 +- examples/search.php | 12 ++++---- src/Client.php | 25 ++++++++++++---- tests/ClientTest.php | 36 +++++++++++++++++++++++ 5 files changed, 132 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index f9b69da..4e74b0c 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,8 @@ enriched with the comfort of [ReactPHP's Promises](https://github.com/reactphp/p * [Usage](#usage) * [Client](#client) * [Promises](#promises) + * [Cancellation](#cancellation) + * [Timeouts](#timeouts) * [search()](#search) * [get()](#get) * [all()](#all) @@ -92,6 +94,68 @@ Sending requests is async (non-blocking), so you can actually send multiple requ Packagist will respond to each request with a response message, the order is not guaranteed. Sending requests uses a [Promise](https://github.com/reactphp/promise)-based interface that makes it easy to react to when a request is fulfilled (i.e. either successfully resolved or rejected with an error). +```php +$client->get('clue/graph-composer')->then( + function ($result) { + // result received for get() function + }, + function (Exception $e) { + // an error occured while executing the request + } +}); +``` + +#### Cancellation + +The returned Promise is implemented in such a way that it can be cancelled +when it is still pending. +Cancelling a pending promise will reject its value with an Exception and +clean up any underlying resources. + +```php +$promise = $client->get('clue/graph-composer'); + +$loop->addTimer(2.0, function () use ($promise) { + $promise->cancel(); +}); +``` + +#### Timeouts + +This library uses a very efficient HTTP implementation, so most API requests +should usually be completed in mere milliseconds. However, when sending API +requests over an unreliable network (the internet), there are a number of things +that can go wrong and may cause the request to fail after a time. As such, +timeouts are handled by the underlying HTTP library and this library respects +PHP's `default_socket_timeout` setting (default 60s) as a timeout for sending the +outgoing API request and waiting for a successful response and will otherwise +cancel the pending request and reject its value with an Exception. + +Note that this timeout value covers creating the underlying transport connection, +sending the API request, waiting for the Packagist service to process the request +and receiving the full API response. To pass a custom timeout value, you can +assign the underlying [`timeout` option](https://github.com/clue/reactphp-buzz#timeouts) +like this: + +```php +$browser = new Browser($loop); +$browser = $browser->withOptions(array( + 'timeout' => 10.0 +)); + +$client = new Client($browser); + +$client->get('clue/graph-composer')->then(function ($result) { + // result received within 10 seconds maximum + var_dump($result); +}); +``` + +Similarly, you can use a negative timeout value to not apply a timeout at all +or use a `null` value to restore the default handling. Note that the underlying +connection may still impose a different timeout value. See also the underlying +[`timeout` option](https://github.com/clue/reactphp-buzz#timeouts) for more details. + #### search() The `search(string $query, array $filters = array()): PromiseInterface` method can be used to @@ -108,6 +172,12 @@ $client->search('packagist')->then(function (array $packages) { }); ``` +Note that this method follows Packagist's paginated search results which +may contain a large number of matches depending on your search. +Accordingly, this method sends one API request for each page which may +take a while for the whole search to be completed. It is not uncommon to +take around 5-10 seconds to fetch search results for 1000 matches. + #### get() The `get(string $name): PromiseInterface` method can be used to diff --git a/composer.json b/composer.json index bad0e7e..98aa038 100644 --- a/composer.json +++ b/composer.json @@ -12,8 +12,8 @@ ], "require": { "php": ">=5.3", + "clue/buzz-react": "^2.5", "knplabs/packagist-api": "~1.0", - "clue/buzz-react": "^2.0 || ^1.0 || ^0.5", "rize/uri-template": "^0.3" }, "require-dev": { diff --git a/examples/search.php b/examples/search.php index a5417dd..bf826e3 100644 --- a/examples/search.php +++ b/examples/search.php @@ -10,19 +10,17 @@ $browser = new Browser($loop); $client = new Client($browser); -$client->search('packagist')->then(function ($result) { - var_dump('found ' . count($result) . ' packages matching "packagist"'); +$client->search('reactphp')->then(function ($result) { + var_dump('found ' . count($result) . ' packages matching "reactphp"'); //var_dump($result); -}, function ($error) { - echo $e; -}); +}, 'printf'); $client->get('clue/phar-composer')->then(function (Package $package) { var_dump($package->getName(), $package->getDescription()); -}); +}, 'printf'); $client->get('clue/graph')->then(function (Package $package) { var_dump($package->getName(), $package->getDescription()); -}); +}, 'printf'); $loop->run(); diff --git a/src/Client.php b/src/Client.php index 403b70d..7a15422 100644 --- a/src/Client.php +++ b/src/Client.php @@ -6,6 +6,7 @@ use Packagist\Api\Result\Factory; use Packagist\Api\Result\Package; use Psr\Http\Message\ResponseInterface; +use React\Promise\Deferred; use React\Promise\PromiseInterface; use Rize\UriTemplate; @@ -45,6 +46,12 @@ public function __construct(Browser $http, Factory $resultFactory = null, UriTem * }); * ``` * + * Note that this method follows Packagist's paginated search results which + * may contain a large number of matches depending on your search. + * Accordingly, this method sends one API request for each page which may + * take a while for the whole search to be completed. It is not uncommon to + * take around 5-10 seconds to fetch search results for 1000 matches. + * * @param string $query * @param array $filters * @return PromiseInterface @@ -63,20 +70,28 @@ public function search($query, array $filters = array()) $results = array(); $that = $this; - $fetch = function ($url) use (&$results, $that, &$fetch) { - return $that->request($url)->then(function (ResponseInterface $response) use (&$results, $that, $fetch) { + $pending = null; + $deferred = new Deferred(function () use (&$pending) { + $pending->cancel(); + }); + + $fetch = function ($url) use (&$results, $that, &$fetch, $deferred, &$pending) { + $pending = $that->request($url)->then(function (ResponseInterface $response) use (&$results, $that, $fetch, $deferred) { $parsed = $that->parse((string)$response->getBody()); $results = array_merge($results, $that->create($parsed)); if (isset($parsed['next'])) { - return $fetch($parsed['next']); + $fetch($parsed['next']); } else { - return $results; + $deferred->resolve($results); } + }, function ($e) use ($deferred) { + $deferred->reject($e); }); }; + $fetch($url); - return $fetch($url); + return $deferred->promise(); } /** diff --git a/tests/ClientTest.php b/tests/ClientTest.php index 521489e..d08b9d7 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -65,6 +65,42 @@ public function testSearchPagination() $this->expectPromiseResolve($this->client->search('zenity')); } + public function testSearchRejectsWhenRequestRejects() + { + $this->browser->expects($this->once())->method('get')->willReturn( + $this->createRejectedPromise(new RuntimeException()) + ); + + $promise = $this->client->search('foo'); + + $promise->then($this->expectCallableNever(), $this->expectCallableOnce()); + } + + public function testSearchCancelPendingPromiseWillCancelInitialRequest() + { + $deferred = new Deferred($this->expectCallableOnce()); + $this->browser->expects($this->once())->method('get')->willReturn($deferred->promise()); + + $promise = $this->client->search('foo'); + $promise->cancel(); + } + + public function testSearchCancelPendingPromiseWillCancelNextRequestWhenInitialIsCompleted() + { + $first = new Deferred($this->expectCallableNever()); + $second = new Deferred($this->expectCallableOnce()); + $this->browser->expects($this->exactly(2))->method('get')->willReturnOnConsecutiveCalls( + $first->promise(), + $second->promise() + ); + + $promise = $this->client->search('foo'); + + $first->resolve($this->createResponsePromise('{"results":[{"name":"clue\/zenity-react","description":"Build graphical desktop (GUI) applications in PHP","url":"https:\/\/packagist.org\/packages\/clue\/zenity-react","downloads":57,"favers":0,"repository":"https:\/\/github.com\/clue\/reactphp-zenity"}],"total":2, "next": ""}')); + + $promise->cancel(); + } + public function testHttpError() { $this->setupBrowser('/packages/clue%2Finvalid.json', $this->createRejectedPromise(new RuntimeException('error')));