From 1dead5dc74bc165ed5e1cdcdf6c3bde53c88398b Mon Sep 17 00:00:00 2001 From: Jan Sorgalla Date: Tue, 19 Jul 2016 20:34:16 +0200 Subject: [PATCH 1/4] Revert automatic cancellation of pending collection promises This reverts 42d86b729204fe498688b1f4b47b4ec03482f152 --- CHANGELOG.md | 6 ++++++ src/functions.php | 18 +++--------------- tests/FunctionAnyTest.php | 4 ++-- tests/FunctionRaceTest.php | 8 ++++---- tests/FunctionSomeTest.php | 8 ++++---- 5 files changed, 19 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e031c21..b44de734 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,12 @@ CHANGELOG for 2.x ================= +* 2.4.2 (xxxx-xx-xx) + + * Revert automatic cancellation of pending collection promises once the + output promise resolves. This was introduced in 42d86b7 and was both + unintended and backward incompatible. + * 2.4.1 (2016-05-03) * Fix `some()` not cancelling pending promises when too much input promises diff --git a/src/functions.php b/src/functions.php index 5f55f2ea..4f49b011 100644 --- a/src/functions.php +++ b/src/functions.php @@ -54,21 +54,11 @@ function race($promisesOrValues) return; } - $fulfiller = function ($value) use ($cancellationQueue, $resolve) { - $cancellationQueue(); - $resolve($value); - }; - - $rejecter = function ($reason) use ($cancellationQueue, $reject) { - $cancellationQueue(); - $reject($reason); - }; - foreach ($array as $promiseOrValue) { $cancellationQueue->enqueue($promiseOrValue); resolve($promiseOrValue) - ->done($fulfiller, $rejecter, $notify); + ->done($resolve, $reject, $notify); } }, $reject, $notify); }, $cancellationQueue); @@ -115,7 +105,7 @@ function some($promisesOrValues, $howMany) $reasons = []; foreach ($array as $i => $promiseOrValue) { - $fulfiller = function ($val) use ($i, &$values, &$toResolve, $toReject, $resolve, $cancellationQueue) { + $fulfiller = function ($val) use ($i, &$values, &$toResolve, $toReject, $resolve) { if ($toResolve < 1 || $toReject < 1) { return; } @@ -123,12 +113,11 @@ function some($promisesOrValues, $howMany) $values[$i] = $val; if (0 === --$toResolve) { - $cancellationQueue(); $resolve($values); } }; - $rejecter = function ($reason) use ($i, &$reasons, &$toReject, $toResolve, $reject, $cancellationQueue) { + $rejecter = function ($reason) use ($i, &$reasons, &$toReject, $toResolve, $reject) { if ($toResolve < 1 || $toReject < 1) { return; } @@ -136,7 +125,6 @@ function some($promisesOrValues, $howMany) $reasons[$i] = $reason; if (0 === --$toReject) { - $cancellationQueue(); $reject($reasons); } }; diff --git a/tests/FunctionAnyTest.php b/tests/FunctionAnyTest.php index 6c40674b..811473e0 100644 --- a/tests/FunctionAnyTest.php +++ b/tests/FunctionAnyTest.php @@ -175,7 +175,7 @@ public function shouldCancelInputArrayPromises() } /** @test */ - public function shouldCancelOtherPendingInputArrayPromisesIfOnePromiseFulfills() + public function shouldNotCancelOtherPendingInputArrayPromisesIfOnePromiseFulfills() { $mock = $this->createCallableMock(); $mock @@ -188,7 +188,7 @@ public function shouldCancelOtherPendingInputArrayPromisesIfOnePromiseFulfills() $mock2 = $this->getMock('React\Promise\CancellablePromiseInterface'); $mock2 - ->expects($this->once()) + ->expects($this->never()) ->method('cancel'); some([$deferred->promise(), $mock2], 1)->cancel(); diff --git a/tests/FunctionRaceTest.php b/tests/FunctionRaceTest.php index 6cd1e987..7fd8acbe 100644 --- a/tests/FunctionRaceTest.php +++ b/tests/FunctionRaceTest.php @@ -162,7 +162,7 @@ public function shouldCancelInputArrayPromises() } /** @test */ - public function shouldCancelOtherPendingInputArrayPromisesIfOnePromiseFulfills() + public function shouldNotCancelOtherPendingInputArrayPromisesIfOnePromiseFulfills() { $mock = $this->createCallableMock(); $mock @@ -174,14 +174,14 @@ public function shouldCancelOtherPendingInputArrayPromisesIfOnePromiseFulfills() $mock2 = $this->getMock('React\Promise\CancellablePromiseInterface'); $mock2 - ->expects($this->once()) + ->expects($this->never()) ->method('cancel'); race([$deferred->promise(), $mock2])->cancel(); } /** @test */ - public function shouldCancelOtherPendingInputArrayPromisesIfOnePromiseRejects() + public function shouldNotCancelOtherPendingInputArrayPromisesIfOnePromiseRejects() { $mock = $this->createCallableMock(); $mock @@ -193,7 +193,7 @@ public function shouldCancelOtherPendingInputArrayPromisesIfOnePromiseRejects() $mock2 = $this->getMock('React\Promise\CancellablePromiseInterface'); $mock2 - ->expects($this->once()) + ->expects($this->never()) ->method('cancel'); race([$deferred->promise(), $mock2])->cancel(); diff --git a/tests/FunctionSomeTest.php b/tests/FunctionSomeTest.php index 374c84f0..4808e20e 100644 --- a/tests/FunctionSomeTest.php +++ b/tests/FunctionSomeTest.php @@ -209,7 +209,7 @@ public function shouldCancelInputArrayPromises() } /** @test */ - public function shouldCancelOtherPendingInputArrayPromisesIfEnoughPromisesFulfill() + public function shouldNotCancelOtherPendingInputArrayPromisesIfEnoughPromisesFulfill() { $mock = $this->createCallableMock(); $mock @@ -221,14 +221,14 @@ public function shouldCancelOtherPendingInputArrayPromisesIfEnoughPromisesFulfil $mock2 = $this->getMock('React\Promise\CancellablePromiseInterface'); $mock2 - ->expects($this->once()) + ->expects($this->never()) ->method('cancel'); some([$deferred->promise(), $mock2], 1); } /** @test */ - public function shouldCancelOtherPendingInputArrayPromisesIfEnoughPromisesReject() + public function shouldNotCancelOtherPendingInputArrayPromisesIfEnoughPromisesReject() { $mock = $this->createCallableMock(); $mock @@ -240,7 +240,7 @@ public function shouldCancelOtherPendingInputArrayPromisesIfEnoughPromisesReject $mock2 = $this->getMock('React\Promise\CancellablePromiseInterface'); $mock2 - ->expects($this->once()) + ->expects($this->never()) ->method('cancel'); some([$deferred->promise(), $mock2], 2); From b5a66642e47b692b6a34edf3664672b9b6df5aa8 Mon Sep 17 00:00:00 2001 From: Jan Sorgalla Date: Wed, 20 Jul 2016 08:49:32 +0200 Subject: [PATCH 2/4] Add issue and version the commit is releated to --- CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b44de734..05230e6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,9 @@ CHANGELOG for 2.x * 2.4.2 (xxxx-xx-xx) * Revert automatic cancellation of pending collection promises once the - output promise resolves. This was introduced in 42d86b7 and was both - unintended and backward incompatible. + output promise resolves. This was introduced in 42d86b7 (PR #36, released + in [v2.3.0](https://github.com/reactphp/promise/releases/tag/v2.3.0)) and + was both unintended and backward incompatible. * 2.4.1 (2016-05-03) From 487190ae27bfa8a736b98dd2dd2330fcddfbf792 Mon Sep 17 00:00:00 2001 From: Jan Sorgalla Date: Thu, 21 Jul 2016 08:05:13 +0200 Subject: [PATCH 3/4] Fix next version According to semver, reverting a BC break should go into a new minor version. --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05230e6c..963ba0b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ CHANGELOG for 2.x ================= -* 2.4.2 (xxxx-xx-xx) +* 2.5.0 (xxxx-xx-xx) * Revert automatic cancellation of pending collection promises once the output promise resolves. This was introduced in 42d86b7 (PR #36, released From 71f1f435b6f985a024dc8f34bd2bbf6c9ea6d327 Mon Sep 17 00:00:00 2001 From: Jan Sorgalla Date: Tue, 25 Oct 2016 10:02:30 +0200 Subject: [PATCH 4/4] Add sample code on how to achieve reverted automatic cancellation behavior --- CHANGELOG.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 963ba0b9..b5133eaf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,22 @@ CHANGELOG for 2.x in [v2.3.0](https://github.com/reactphp/promise/releases/tag/v2.3.0)) and was both unintended and backward incompatible. + If you need automatic cancellation, you can use something like: + + ```php + function allAndCancel(array $promises) + { + return \React\Promise\all($promises) + ->always(function() use ($promises) { + foreach ($promises as $promise) { + if ($promise instanceof \React\Promise\CancellablePromiseInterface) { + $promise->cancel(); + } + } + }); + } + ``` + * 2.4.1 (2016-05-03) * Fix `some()` not cancelling pending promises when too much input promises