Skip to content

Commit ec92315

Browse files
nicolas-grekasnicolas-grekas
committed
bug #62169 [HttpClient] Fix caching client decorating scoping client (pierredup)
This PR was squashed before being merged into the 7.4 branch. Discussion ---------- [HttpClient] Fix caching client decorating scoping client | Q | A | ------------- | --- | Branch? | 7.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix #62166 | License | MIT When registering a scoped client and enabling caching, the scoping client should decorate the caching one, while currently it's registered the wrong way around. This causes `Invalid URL` exceptions, since the URL is not resolved first by the scoping client Commits ------- bfde96233bf [HttpClient] Fix caching client decorating scoping client
2 parents 1eac81d + d7b321e commit ec92315

File tree

3 files changed

+28
-19
lines changed

3 files changed

+28
-19
lines changed

‎DependencyInjection/Configuration.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2076,7 +2076,7 @@ private function addHttpClientSection(ArrayNodeDefinition $rootNode, callable $e
20762076
->acceptAndWrap(['string'], 'base_uri')
20772077
->validate()
20782078
->ifTrue(static fn () => !class_exists(HttpClient::class))
2079-
->then(static fn () => 'HttpClient support cannot be enabled as the component is not installed. Try running "composer require symfony/http-client".')
2079+
->then(static fn () => throw new LogicException('HttpClient support cannot be enabled as the component is not installed. Try running "composer require symfony/http-client".'))
20802080
->end()
20812081
->validate()
20822082
->ifTrue(static fn ($v) => !isset($v['scope']) && !isset($v['base_uri']))

‎DependencyInjection/FrameworkExtension.php‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2880,39 +2880,45 @@ private function registerHttpClientConfiguration(array $config, ContainerBuilder
28802880
$retryOptions = $scopeConfig['retry_failed'] ?? ['enabled' => false];
28812881
unset($scopeConfig['retry_failed']);
28822882

2883+
$transport = $name.'.transport';
2884+
$container->register($transport, HttpClientInterface::class)
2885+
->setFactory('current')
2886+
->setArguments([[new Reference('http_client.transport')]])
2887+
;
2888+
28832889
if (null === $scope) {
28842890
$baseUri = $scopeConfig['base_uri'];
28852891
unset($scopeConfig['base_uri']);
28862892

28872893
$container->register($name, ScopingHttpClient::class)
28882894
->setFactory([ScopingHttpClient::class, 'forBaseUri'])
2889-
->setArguments([new Reference('http_client.transport'), $baseUri, $scopeConfig])
2895+
->setArguments([new Reference($transport), $baseUri, $scopeConfig])
28902896
->addTag('http_client.client')
28912897
->addTag('kernel.reset', ['method' => 'reset', 'on_invalid' => 'ignore'])
28922898
;
28932899
} else {
28942900
$container->register($name, ScopingHttpClient::class)
2895-
->setArguments([new Reference('http_client.transport'), [$scope => $scopeConfig], $scope])
2901+
->setArguments([new Reference($transport), [$scope => $scopeConfig], $scope])
28962902
->addTag('http_client.client')
28972903
->addTag('kernel.reset', ['method' => 'reset', 'on_invalid' => 'ignore'])
28982904
;
28992905
}
29002906

29012907
if ($this->readConfigEnabled('http_client.scoped_clients.'.$name.'.caching', $container, $cachingOptions)) {
2902-
$this->registerCachingHttpClient($cachingOptions, $scopeConfig, $name, $container);
2908+
$this->registerCachingHttpClient($cachingOptions, $scopeConfig, $transport, $container);
29032909
}
29042910

29052911
if (null !== $rateLimiter) {
2906-
$this->registerThrottlingHttpClient($rateLimiter, $name, $container);
2912+
$this->registerThrottlingHttpClient($rateLimiter, $transport, $container);
29072913
}
29082914

29092915
if ($this->readConfigEnabled('http_client.scoped_clients.'.$name.'.retry_failed', $container, $retryOptions)) {
2910-
$this->registerRetryableHttpClient($retryOptions, $name, $container);
2916+
$this->registerRetryableHttpClient($retryOptions, $transport, $container);
29112917
}
29122918

29132919
$container
29142920
->register($name.'.uri_template', UriTemplateHttpClient::class)
2915-
->setDecoratedService($name, null, 7) // Between TraceableHttpClient (5) and RetryableHttpClient (10)
2921+
->setDecoratedService($name, null, 7) // After TraceableHttpClient (5) and on top of ScopingHttpClient
29162922
->setArguments([
29172923
new Reference($name.'.uri_template.inner'),
29182924
new Reference('http_client.uri_template_expander', ContainerInterface::NULL_ON_INVALID_REFERENCE),
@@ -2946,7 +2952,7 @@ private function registerHttpClientConfiguration(array $config, ContainerBuilder
29462952
private function registerCachingHttpClient(array $options, array $defaultOptions, string $name, ContainerBuilder $container): void
29472953
{
29482954
if (!class_exists(ChunkCacheItemNotFoundException::class)) {
2949-
throw new LogicException('Caching cannot be enabled as version 7.3+ of the HttpClient component is required.');
2955+
throw new LogicException('Caching cannot be enabled as version 7.4+ of the HttpClient component is required.');
29502956
}
29512957

29522958
$container

‎Tests/DependencyInjection/FrameworkExtensionTestCase.php‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2295,15 +2295,18 @@ public function testCachingHttpClient()
22952295
$this->assertFalse($arguments[3]);
22962296
$this->assertSame(2, $arguments[4]);
22972297

2298-
$this->assertTrue($container->hasDefinition('bar.caching'));
2299-
$definition = $container->getDefinition('bar.caching');
2298+
$this->assertTrue($container->hasDefinition('bar.transport.caching'));
2299+
$definition = $container->getDefinition('bar.transport.caching');
23002300
$this->assertSame(CachingHttpClient::class, $definition->getClass());
2301-
$this->assertSame('bar', $definition->getDecoratedService()[0]);
23022301
$arguments = $definition->getArguments();
23032302
$this->assertInstanceOf(Reference::class, $arguments[0]);
2304-
$this->assertSame('bar.caching.inner', (string) $arguments[0]);
2303+
$this->assertSame('bar.transport.caching.inner', (string) $arguments[0]);
23052304
$this->assertInstanceOf(Reference::class, $arguments[1]);
23062305
$this->assertSame('baz', (string) $arguments[1]);
2306+
$scopedClient = $container->getDefinition('bar');
2307+
2308+
$this->assertSame('bar.transport', (string) $scopedClient->getArgument(0));
2309+
$this->assertNull($scopedClient->getDecoratedService());
23072310
}
23082311

23092312
public function testHttpClientRetry()
@@ -2317,8 +2320,8 @@ public function testHttpClientRetry()
23172320
$this->assertSame(0.3, $container->getDefinition('http_client.retry_strategy')->getArgument(4));
23182321
$this->assertSame(2, $container->getDefinition('http_client.retryable')->getArgument(2));
23192322

2320-
$this->assertSame(RetryableHttpClient::class, $container->getDefinition('foo.retryable')->getClass());
2321-
$this->assertSame(4, $container->getDefinition('foo.retry_strategy')->getArgument(2));
2323+
$this->assertSame(RetryableHttpClient::class, $container->getDefinition('foo.transport.retryable')->getClass());
2324+
$this->assertSame(4, $container->getDefinition('foo.transport.retry_strategy')->getArgument(2));
23222325
}
23232326

23242327
public function testHttpClientWithQueryParameterKey()
@@ -2383,15 +2386,15 @@ public function testHttpClientRateLimiter()
23832386
$this->assertInstanceOf(Reference::class, $arguments[1]);
23842387
$this->assertSame('http_client.throttling.limiter', (string) $arguments[1]);
23852388

2386-
$this->assertTrue($container->hasDefinition('foo.throttling'));
2387-
$definition = $container->getDefinition('foo.throttling');
2389+
$this->assertTrue($container->hasDefinition('foo.transport.throttling'));
2390+
$definition = $container->getDefinition('foo.transport.throttling');
23882391
$this->assertSame(ThrottlingHttpClient::class, $definition->getClass());
2389-
$this->assertSame('foo', $definition->getDecoratedService()[0]);
2392+
$this->assertSame('foo.transport', $definition->getDecoratedService()[0]);
23902393
$this->assertCount(2, $arguments = $definition->getArguments());
23912394
$this->assertInstanceOf(Reference::class, $arguments[0]);
2392-
$this->assertSame('foo.throttling.inner', (string) $arguments[0]);
2395+
$this->assertSame('foo.transport.throttling.inner', (string) $arguments[0]);
23932396
$this->assertInstanceOf(Reference::class, $arguments[1]);
2394-
$this->assertSame('foo.throttling.limiter', (string) $arguments[1]);
2397+
$this->assertSame('foo.transport.throttling.limiter', (string) $arguments[1]);
23952398
}
23962399

23972400
public static function provideMailer(): iterable

0 commit comments

Comments
 (0)