Skip to content

Commit be70a65

Browse files
authored
Allow an empty EachPromise to be resolved by running the queue (#178)
* Allow an empty EachPromise to be resolved by running the queue. Fix #176 * Remove unused import * fix typing doc comments * Add whitespace to please php-cs-fixer For #176 * more php-cs-fixer changes
1 parent 90c5be4 commit be70a65

File tree

2 files changed

+51
-9
lines changed

2 files changed

+51
-9
lines changed

src/EachPromise.php

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ class EachPromise implements PromisorInterface
2828
/** @var callable|null */
2929
private $onRejected;
3030

31-
/** @var Promise|null */
31+
/** @var PromiseInterface|null */
3232
private $aggregate;
3333

3434
/** @var bool|null */
@@ -80,9 +80,9 @@ public function promise(): PromiseInterface
8080
}
8181

8282
try {
83+
$this->iterable->rewind();
8384
$this->createPromise();
8485
/** @psalm-assert Promise $this->aggregate */
85-
$this->iterable->rewind();
8686
$this->refillPending();
8787
} catch (\Throwable $e) {
8888
$this->aggregate->reject($e);
@@ -96,6 +96,23 @@ public function promise(): PromiseInterface
9696

9797
private function createPromise(): void
9898
{
99+
// Clear the references when the promise is resolved.
100+
$clearFn = function (): void {
101+
$this->iterable = $this->concurrency = $this->pending = null;
102+
$this->onFulfilled = $this->onRejected = null;
103+
$this->nextPendingIndex = 0;
104+
};
105+
106+
// In the case of empty, create a promise that will immediately become resolved via wait() or
107+
// via Utils::queue()->run() (https://github.com/guzzle/promises/issues/176). We could simply
108+
// create a fulfilled promise here, but that would be an observable behavioral change (see
109+
// EachPromiseTest::testResolvesInCaseOfAnEmptyListAndInvokesFulfilled)
110+
if (!$this->iterable->valid()) {
111+
$this->aggregate = Create::promiseFor(null)->then($clearFn);
112+
113+
return;
114+
}
115+
99116
$this->mutex = false;
100117
$this->aggregate = new Promise(function (): void {
101118
if ($this->checkIfFinished()) {
@@ -113,13 +130,6 @@ private function createPromise(): void
113130
}
114131
});
115132

116-
// Clear the references when the promise is resolved.
117-
$clearFn = function (): void {
118-
$this->iterable = $this->concurrency = $this->pending = null;
119-
$this->onFulfilled = $this->onRejected = null;
120-
$this->nextPendingIndex = 0;
121-
};
122-
123133
$this->aggregate->then($clearFn, $clearFn);
124134
}
125135

tests/EachPromiseTest.php

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,17 @@ public function testResolvesInCaseOfAnEmptyList(): void
3131
$this->assertTrue(P\Is::fulfilled($p));
3232
}
3333

34+
// See https://github.com/guzzle/promises/issues/176
35+
public function testResolvesWithQueueInCaseOfEmptyList(): void
36+
{
37+
$promises = [];
38+
$each = new EachPromise($promises);
39+
$p = $each->promise();
40+
P\Utils::queue()->run();
41+
$this->assertTrue(P\Is::fulfilled($p));
42+
$this->assertNull($p->wait());
43+
}
44+
3445
public function testResolvesInCaseOfAnEmptyListAndInvokesFulfilled(): void
3546
{
3647
$promises = [];
@@ -430,4 +441,25 @@ public function testIsWaitableWhenLimited(): void
430441
$this->assertSame(['a', 'c', 'b', 'd'], $called);
431442
$this->assertTrue(P\Is::fulfilled($p));
432443
}
444+
445+
public function testRewindsExhaustedIterator(): void
446+
{
447+
$promises = P\Create::iterFor([
448+
$this->createSelfResolvingPromise('a'),
449+
$this->createSelfResolvingPromise('b'),
450+
]);
451+
while ($promises->valid()) {
452+
$promises->next();
453+
}
454+
$called = [];
455+
$each = new EachPromise($promises, [
456+
'fulfilled' => function ($value) use (&$called): void {
457+
$called[] = $value;
458+
},
459+
]);
460+
$p = $each->promise();
461+
$this->assertNull($p->wait());
462+
$this->assertSame(['a', 'b'], $called);
463+
$this->assertTrue(P\Is::fulfilled($p));
464+
}
433465
}

0 commit comments

Comments
 (0)