From ddd8ebed283505a066a51ac53be34789b18a7445 Mon Sep 17 00:00:00 2001 From: Joel Wurtz Date: Fri, 6 Apr 2018 00:06:52 +0200 Subject: [PATCH 1/3] Use deferred to be full async in retry --- spec/Plugin/RetryPluginSpec.php | 17 +++-- src/Deferred.php | 131 ++++++++++++++++++++++++++++++++ src/Plugin/RetryPlugin.php | 27 +++++-- 3 files changed, 165 insertions(+), 10 deletions(-) create mode 100644 src/Deferred.php diff --git a/spec/Plugin/RetryPluginSpec.php b/spec/Plugin/RetryPluginSpec.php index 37800ae..852fb73 100644 --- a/spec/Plugin/RetryPluginSpec.php +++ b/spec/Plugin/RetryPluginSpec.php @@ -30,7 +30,9 @@ function it_returns_response(RequestInterface $request, ResponseInterface $respo } }; - $this->handleRequest($request, $next, function () {})->shouldReturnAnInstanceOf('Http\Client\Promise\HttpFulfilledPromise'); + $promise = $this->handleRequest($request, $next, function () {}); + $promise->shouldReturnAnInstanceOf('Http\Client\Common\Deferred'); + $promise->wait()->shouldReturn($response); } function it_throws_exception_on_multiple_exceptions(RequestInterface $request) @@ -53,7 +55,7 @@ function it_throws_exception_on_multiple_exceptions(RequestInterface $request) }; $promise = $this->handleRequest($request, $next, function () {}); - $promise->shouldReturnAnInstanceOf('Http\Client\Promise\HttpRejectedPromise'); + $promise->shouldReturnAnInstanceOf('Http\Client\Common\Deferred'); $promise->shouldThrow($exception2)->duringWait(); } @@ -76,7 +78,7 @@ function it_returns_response_on_second_try(RequestInterface $request, ResponseIn }; $promise = $this->handleRequest($request, $next, function () {}); - $promise->shouldReturnAnInstanceOf('Http\Client\Promise\HttpFulfilledPromise'); + $promise->shouldReturnAnInstanceOf('Http\Client\Common\Deferred'); $promise->wait()->shouldReturn($response); } @@ -98,8 +100,13 @@ function it_does_not_keep_history_of_old_failure(RequestInterface $request, Resp } }; - $this->handleRequest($request, $next, function () {})->shouldReturnAnInstanceOf('Http\Client\Promise\HttpFulfilledPromise'); - $this->handleRequest($request, $next, function () {})->shouldReturnAnInstanceOf('Http\Client\Promise\HttpFulfilledPromise'); + $promise = $this->handleRequest($request, $next, function () {}); + $promise->shouldReturnAnInstanceOf('Http\Client\Common\Deferred'); + $promise->wait()->shouldReturn($response); + + $promise = $this->handleRequest($request, $next, function () {}); + $promise->shouldReturnAnInstanceOf('Http\Client\Common\Deferred'); + $promise->wait()->shouldReturn($response); } function it_has_an_exponential_default_delay(RequestInterface $request, Exception\HttpException $exception) diff --git a/src/Deferred.php b/src/Deferred.php new file mode 100644 index 0000000..f327a66 --- /dev/null +++ b/src/Deferred.php @@ -0,0 +1,131 @@ +waitCallback = $waitCallback; + $this->state = Promise::PENDING; + $this->onFulfilledCallbacks = []; + $this->onRejectedCallbacks = []; + } + + /** + * {@inheritdoc} + */ + public function then(callable $onFulfilled = null, callable $onRejected = null) + { + $deferred = new Deferred($this->waitCallback); + + $this->onFulfilledCallbacks[] = function (ResponseInterface $response) use($onFulfilled, $deferred) { + try { + if (null !== $onFulfilled) { + $response = $onFulfilled($response); + } + $deferred->resolve($response); + } catch (Exception $exception) { + $deferred->reject($exception); + } + }; + + $this->onRejectedCallbacks[] = function (Exception $exception) use($onRejected, $deferred) { + try { + if (null !== $onRejected) { + $response = $onRejected($exception); + $deferred->resolve($response); + + return; + } + $deferred->reject($exception); + } catch (Exception $newException) { + $deferred->reject($newException); + } + }; + + return $deferred; + } + + /** + * {@inheritdoc} + */ + public function getState() + { + return $this->state; + } + + /** + * Resolve this deferred with a Response + */ + public function resolve(ResponseInterface $response) + { + if ($this->state !== self::PENDING) { + return; + } + + $this->value = $response; + $this->state = self::FULFILLED; + + foreach ($this->onFulfilledCallbacks as $onFulfilledCallback) { + $onFulfilledCallback($response); + } + } + + /** + * Reject this deferred with an Exception + */ + public function reject(Exception $exception) + { + if ($this->state !== self::PENDING) { + return; + } + + $this->failure = $exception; + $this->state = self::REJECTED; + + foreach ($this->onRejectedCallbacks as $onRejectedCallback) { + $onRejectedCallback($exception); + } + } + + /** + * {@inheritdoc} + */ + public function wait($unwrap = true) + { + if ($this->state === self::PENDING) { + $callback = $this->waitCallback; + $callback(); + } + + if (!$unwrap) { + return; + } + + if ($this->state === self::FULFILLED) { + return $this->value; + } + + throw $this->failure; + } +} diff --git a/src/Plugin/RetryPlugin.php b/src/Plugin/RetryPlugin.php index 8446246..3769db9 100644 --- a/src/Plugin/RetryPlugin.php +++ b/src/Plugin/RetryPlugin.php @@ -2,6 +2,7 @@ namespace Http\Client\Common\Plugin; +use Http\Client\Common\Deferred; use Http\Client\Common\Plugin; use Http\Client\Exception; use Psr\Http\Message\RequestInterface; @@ -76,13 +77,22 @@ public function handleRequest(RequestInterface $request, callable $next, callabl { $chainIdentifier = spl_object_hash((object) $first); - return $next($request)->then(function (ResponseInterface $response) use ($request, $chainIdentifier) { + $promise = $next($request); + $deferred = new Deferred(function () use($promise) { + $promise->wait(false); + }); + + $onFulfilled = function (ResponseInterface $response) use ($chainIdentifier, $deferred) { if (array_key_exists($chainIdentifier, $this->retryStorage)) { unset($this->retryStorage[$chainIdentifier]); } + $deferred->resolve($response); + return $response; - }, function (Exception $exception) use ($request, $next, $first, $chainIdentifier) { + }; + + $onRejected = function (Exception $exception) use ($request, $next, $onFulfilled, &$onRejected, $chainIdentifier, $deferred) { if (!array_key_exists($chainIdentifier, $this->retryStorage)) { $this->retryStorage[$chainIdentifier] = 0; } @@ -90,6 +100,8 @@ public function handleRequest(RequestInterface $request, callable $next, callabl if ($this->retryStorage[$chainIdentifier] >= $this->retry) { unset($this->retryStorage[$chainIdentifier]); + $deferred->reject($exception); + throw $exception; } @@ -102,10 +114,15 @@ public function handleRequest(RequestInterface $request, callable $next, callabl // Retry in synchrone ++$this->retryStorage[$chainIdentifier]; - $promise = $this->handleRequest($request, $next, $first); - return $promise->wait(); - }); + $next($request)->then($onFulfilled, $onRejected); + + throw $exception; + }; + + $promise->then($onFulfilled, $onRejected); + + return $deferred; } /** From 794ad22198c01b74b5036021ef41143fd7717301 Mon Sep 17 00:00:00 2001 From: Joel Wurtz Date: Fri, 6 Apr 2018 00:14:43 +0200 Subject: [PATCH 2/3] Fix cs --- src/Deferred.php | 20 ++++++++++---------- src/Plugin/RetryPlugin.php | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Deferred.php b/src/Deferred.php index f327a66..075a30e 100644 --- a/src/Deferred.php +++ b/src/Deferred.php @@ -7,7 +7,7 @@ use Psr\Http\Message\ResponseInterface; /** - * A deferred allow to return a promise which has not been resolved yet + * A deferred allow to return a promise which has not been resolved yet. */ class Deferred implements Promise { @@ -36,9 +36,9 @@ public function __construct(callable $waitCallback) */ public function then(callable $onFulfilled = null, callable $onRejected = null) { - $deferred = new Deferred($this->waitCallback); + $deferred = new self($this->waitCallback); - $this->onFulfilledCallbacks[] = function (ResponseInterface $response) use($onFulfilled, $deferred) { + $this->onFulfilledCallbacks[] = function (ResponseInterface $response) use ($onFulfilled, $deferred) { try { if (null !== $onFulfilled) { $response = $onFulfilled($response); @@ -49,7 +49,7 @@ public function then(callable $onFulfilled = null, callable $onRejected = null) } }; - $this->onRejectedCallbacks[] = function (Exception $exception) use($onRejected, $deferred) { + $this->onRejectedCallbacks[] = function (Exception $exception) use ($onRejected, $deferred) { try { if (null !== $onRejected) { $response = $onRejected($exception); @@ -75,11 +75,11 @@ public function getState() } /** - * Resolve this deferred with a Response + * Resolve this deferred with a Response. */ public function resolve(ResponseInterface $response) { - if ($this->state !== self::PENDING) { + if (self::PENDING !== $this->state) { return; } @@ -92,11 +92,11 @@ public function resolve(ResponseInterface $response) } /** - * Reject this deferred with an Exception + * Reject this deferred with an Exception. */ public function reject(Exception $exception) { - if ($this->state !== self::PENDING) { + if (self::PENDING !== $this->state) { return; } @@ -113,7 +113,7 @@ public function reject(Exception $exception) */ public function wait($unwrap = true) { - if ($this->state === self::PENDING) { + if (self::PENDING === $this->state) { $callback = $this->waitCallback; $callback(); } @@ -122,7 +122,7 @@ public function wait($unwrap = true) return; } - if ($this->state === self::FULFILLED) { + if (self::FULFILLED === $this->state) { return $this->value; } diff --git a/src/Plugin/RetryPlugin.php b/src/Plugin/RetryPlugin.php index 3769db9..02ec39b 100644 --- a/src/Plugin/RetryPlugin.php +++ b/src/Plugin/RetryPlugin.php @@ -78,7 +78,7 @@ public function handleRequest(RequestInterface $request, callable $next, callabl $chainIdentifier = spl_object_hash((object) $first); $promise = $next($request); - $deferred = new Deferred(function () use($promise) { + $deferred = new Deferred(function () use ($promise) { $promise->wait(false); }); From 353e66c8edcce5bb1b95811519743469eb983b06 Mon Sep 17 00:00:00 2001 From: Joel Wurtz Date: Mon, 28 May 2018 16:11:46 +0200 Subject: [PATCH 3/3] Add a line in changelog about retry plugin async --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c31194..e757d80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ### Changed - AddPathPlugin no longer add prefix multiple times if a request is restarted - it now only adds the prefix if that request chain has not yet passed through the AddPathPlugin +- RetryPlugin no longer wait for retried requests and use a deferred promise instead ## 1.7.0 - 2017-11-30