Skip to content

Commit d9b8537

Browse files
cosmastechtaylorotwell
cosmastech
and
taylorotwell
authored
[12.x] Fix PendingRequest@pool() && batch() concurrency (#57973)
* indicate what concurrency does * Update PendingRequest.php * Change default concurrency to 0 in pool method Updated the default value of the concurrency parameter in the pool method to 0 for concurrent execution. * Update PendingRequest.php * Update FluentPromise.php * types and pending * laziness and the benefits therein * wip * FluentPromise is dead, long live LazyPromise * clean up * fix test * actually, FluentPromise still has its place * clean up LazyPromise * backwards compatibility * remove unused trait * return concurrency to null * fix Batch * static analysis * convert to generator * clean up PendingRequest * clean up Batch * static all the things * Update LazyPromise.php * formatting --------- Co-authored-by: Taylor Otwell <taylor@laravel.com>
1 parent ef5aa3d commit d9b8537

File tree

5 files changed

+170
-23
lines changed

5 files changed

+170
-23
lines changed

‎src/Illuminate/Http/Client/Batch.php‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
use GuzzleHttp\Exception\RequestException;
88
use GuzzleHttp\Promise\EachPromise;
99
use GuzzleHttp\Utils;
10+
use Illuminate\Http\Client\Promises\LazyPromise;
11+
use Illuminate\Support\Collection;
1012
use Illuminate\Support\Defer\DeferredCallback;
1113

1214
use function Illuminate\Support\defer;
@@ -270,18 +272,8 @@ public function send(): array
270272
}
271273

272274
$results = [];
273-
$promises = [];
274275

275-
foreach ($this->requests as $key => $item) {
276-
$promise = match (true) {
277-
$item instanceof PendingRequest => $item->getPromise(),
278-
default => $item,
279-
};
280-
281-
$promises[$key] = $promise;
282-
}
283-
284-
if (! empty($promises)) {
276+
if (! empty($this->requests)) {
285277
$eachPromiseOptions = [
286278
'fulfilled' => function ($result, $key) use (&$results) {
287279
$results[$key] = $result;
@@ -329,7 +321,16 @@ public function send(): array
329321
$eachPromiseOptions['concurrency'] = $this->concurrencyLimit;
330322
}
331323

332-
(new EachPromise($promises, $eachPromiseOptions))->promise()->wait();
324+
$promiseGenerator = function () {
325+
foreach ($this->requests as $key => $item) {
326+
$promise = $item instanceof PendingRequest ? $item->getPromise() : $item;
327+
yield $key => $promise instanceof LazyPromise ? $promise->buildPromise() : $promise;
328+
}
329+
};
330+
331+
(new EachPromise($promiseGenerator(), $eachPromiseOptions))
332+
->promise()
333+
->wait();
333334
}
334335

335336
// Before returning the results, we must ensure that the results are sorted

‎src/Illuminate/Http/Client/PendingRequest.php‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
use Illuminate\Http\Client\Events\ConnectionFailed;
1919
use Illuminate\Http\Client\Events\RequestSending;
2020
use Illuminate\Http\Client\Events\ResponseReceived;
21+
use Illuminate\Http\Client\Promises\FluentPromise;
22+
use Illuminate\Http\Client\Promises\LazyPromise;
2123
use Illuminate\Support\Arr;
2224
use Illuminate\Support\Collection;
2325
use Illuminate\Support\Str;
@@ -886,7 +888,7 @@ public function delete(string $url, $data = [])
886888
* Send a pool of asynchronous requests concurrently.
887889
*
888890
* @param (callable(\Illuminate\Http\Client\Pool): mixed) $callback
889-
* @param int|null $concurrency
891+
* @param non-negative-int|null $concurrency
890892
* @return array<array-key, \Illuminate\Http\Client\Response|\Illuminate\Http\Client\ConnectionException|\Illuminate\Http\Client\RequestException>
891893
*/
892894
public function pool(callable $callback, ?int $concurrency = null)
@@ -896,20 +898,33 @@ public function pool(callable $callback, ?int $concurrency = null)
896898
$requests = tap(new Pool($this->factory), $callback)->getRequests();
897899

898900
if ($concurrency === null) {
901+
(new Collection($requests))->each(static function ($item) {
902+
if ($item instanceof static) {
903+
$item = $item->getPromise();
904+
}
905+
906+
if ($item instanceof LazyPromise) {
907+
$item->buildPromise();
908+
}
909+
});
910+
899911
foreach ($requests as $key => $item) {
900912
$results[$key] = $item instanceof static ? $item->getPromise()->wait() : $item->wait();
901913
}
902914

903915
return $results;
904916
}
905917

906-
$promises = [];
918+
$concurrency = $concurrency === 0 ? count($requests) : $concurrency;
907919

908-
foreach ($requests as $key => $item) {
909-
$promises[$key] = $item instanceof static ? $item->getPromise() : $item;
910-
}
920+
$promiseGenerator = static function () use ($requests) {
921+
foreach ($requests as $key => $item) {
922+
$promise = $item instanceof static ? $item->getPromise() : $item;
923+
yield $key => $promise instanceof LazyPromise ? $promise->buildPromise() : $promise;
924+
}
925+
};
911926

912-
(new EachPromise($promises, [
927+
(new EachPromise($promiseGenerator(), [
913928
'fulfilled' => function ($result, $key) use (&$results) {
914929
$results[$key] = $result;
915930
},
@@ -939,7 +954,7 @@ public function batch(callable $callback): Batch
939954
* @param string $method
940955
* @param string $url
941956
* @param array $options
942-
* @return \Illuminate\Http\Client\Response
957+
* @return \Illuminate\Http\Client\Response|\Illuminate\Http\Client\Promises\LazyPromise
943958
*
944959
* @throws \Exception
945960
* @throws \Illuminate\Http\Client\ConnectionException
@@ -957,7 +972,9 @@ public function send(string $method, string $url, array $options = [])
957972
[$this->pendingBody, $this->pendingFiles] = [null, []];
958973

959974
if ($this->async) {
960-
return $this->makePromise($method, $url, $options);
975+
return $this->promise = new LazyPromise(
976+
fn () => $this->makePromise($method, $url, $options)
977+
);
961978
}
962979

963980
$shouldRetry = null;
@@ -1198,7 +1215,7 @@ protected function handlePromiseResponse(Response|ConnectionException|TransferEx
11981215
* @param string $method
11991216
* @param string $url
12001217
* @param array $options
1201-
* @return \Psr\Http\Message\MessageInterface|\Illuminate\Http\Client\FluentPromise
1218+
* @return \Psr\Http\Message\MessageInterface|\GuzzleHttp\Promise\PromiseInterface
12021219
*
12031220
* @throws \Exception
12041221
*/

src/Illuminate/Http/Client/FluentPromise.php renamed to src/Illuminate/Http/Client/Promises/FluentPromise.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?php
22

3-
namespace Illuminate\Http\Client;
3+
namespace Illuminate\Http\Client\Promises;
44

55
use GuzzleHttp\Promise\PromiseInterface;
66
use Illuminate\Support\Traits\ForwardsCalls;

‎src/Illuminate/Http/Client/Promises/LazyPromise.php‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
<?php
2+
3+
namespace Illuminate\Http\Client\Promises;
4+
5+
use Closure;
6+
use GuzzleHttp\Promise\PromiseInterface;
7+
use RuntimeException;
8+
9+
class LazyPromise implements PromiseInterface
10+
{
11+
/**
12+
* The callbacks to execute after the Guzzle Promise has been built.
13+
*
14+
* @var list<callable>
15+
*/
16+
protected array $pending = [];
17+
18+
/**
19+
* The promise built by the creator.
20+
*
21+
* @var \GuzzleHttp\Promise\PromiseInterface
22+
*/
23+
protected PromiseInterface $guzzlePromise;
24+
25+
/**
26+
* Create a new lazy promise instance.
27+
*
28+
* @param (\Closure(): \GuzzleHttp\Promise\PromiseInterface) $promiseBuilder The callback to build a new PromiseInterface.
29+
*/
30+
public function __construct(protected Closure $promiseBuilder)
31+
{
32+
}
33+
34+
/**
35+
* Build the promise from the promise builder.
36+
*
37+
* @return \GuzzleHttp\Promise\PromiseInterface
38+
*
39+
* @throws \RuntimeException If the promise has already been built
40+
*/
41+
public function buildPromise(): PromiseInterface
42+
{
43+
if (! $this->promiseNeedsBuilt()) {
44+
throw new RuntimeException('Promise already built');
45+
}
46+
47+
$this->guzzlePromise = call_user_func($this->promiseBuilder);
48+
49+
foreach ($this->pending as $pendingCallback) {
50+
$pendingCallback($this->guzzlePromise);
51+
}
52+
53+
$this->pending = [];
54+
55+
return $this->guzzlePromise;
56+
}
57+
58+
#[\Override]
59+
public function then(?callable $onFulfilled = null, ?callable $onRejected = null): PromiseInterface
60+
{
61+
if ($this->promiseNeedsBuilt()) {
62+
$this->pending[] = static fn (PromiseInterface $promise) => $promise->then($onFulfilled, $onRejected);
63+
64+
return $this;
65+
}
66+
67+
return $this->guzzlePromise->then($onFulfilled, $onRejected);
68+
}
69+
70+
#[\Override]
71+
public function otherwise(callable $onRejected): PromiseInterface
72+
{
73+
if ($this->promiseNeedsBuilt()) {
74+
$this->pending[] = static fn (PromiseInterface $promise) => $promise->otherwise($onRejected);
75+
76+
return $this;
77+
}
78+
79+
return $this->guzzlePromise->otherwise($onRejected);
80+
}
81+
82+
#[\Override]
83+
public function getState(): string
84+
{
85+
if ($this->promiseNeedsBuilt()) {
86+
return PromiseInterface::PENDING;
87+
}
88+
89+
return $this->guzzlePromise->getState();
90+
}
91+
92+
#[\Override]
93+
public function resolve($value): void
94+
{
95+
throw new \LogicException('Cannot resolve a lazy promise.');
96+
}
97+
98+
#[\Override]
99+
public function reject($reason): void
100+
{
101+
throw new \LogicException('Cannot reject a lazy promise.');
102+
}
103+
104+
#[\Override]
105+
public function cancel(): void
106+
{
107+
throw new \LogicException('Cannot cancel a lazy promise.');
108+
}
109+
110+
#[\Override]
111+
public function wait(bool $unwrap = true)
112+
{
113+
if ($this->promiseNeedsBuilt()) {
114+
$this->buildPromise();
115+
}
116+
117+
return $this->guzzlePromise->wait($unwrap);
118+
}
119+
120+
/**
121+
* Determine if the promise has been created from the promise builder.
122+
*
123+
* @return bool
124+
*/
125+
public function promiseNeedsBuilt(): bool
126+
{
127+
return ! isset($this->guzzlePromise);
128+
}
129+
}

‎tests/Http/HttpClientTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3536,7 +3536,7 @@ public function testItCanEnforceFakingInThePool()
35363536
return [
35373537
$pool->get('https://laravel.com'),
35383538
];
3539-
});
3539+
}, null);
35403540
}
35413541

35423542
public function testPreventingStrayRequests()

0 commit comments

Comments
 (0)