minor #49513 [FrameworkBundle][HttpClient] Refactor http_client decoration strategy (fancyweb)

This PR was merged into the 6.3 branch.

Discussion
----------

[FrameworkBundle][HttpClient] Refactor http_client decoration strategy

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | symfony/symfony#49302 (comment)
| License       | MIT
| Doc PR        | -

Commits
-------

11e2164ee5 [FrameworkBundle][HttpClient] Refactor http_client decoration strategy

Author: nicolas-grekas

DependencyInjection/FrameworkExtension.php

@@ -2342,7 +2342,7 @@ private function registerHttpClientConfiguration(array $config, ContainerBuilder
         unset($options['retry_failed']);
         $defaultUriTemplateVars = $options['vars'] ?? [];
         unset($options['vars']);
-        $container->getDefinition('http_client')->setArguments([$options, $config['max_host_connections'] ?? 6]);
+        $container->getDefinition('http_client.transport')->setArguments([$options, $config['max_host_connections'] ?? 6]);
 
         if (!$hasPsr18 = ContainerBuilder::willBeAvailable('psr/http-client', ClientInterface::class, ['symfony/framework-bundle', 'symfony/http-client'])) {
             $container->removeDefinition('psr18.http_client');
@@ -2353,7 +2353,7 @@ private function registerHttpClientConfiguration(array $config, ContainerBuilder
             $container->removeDefinition(HttpClient::class);
         }
 
-        if ($hasRetryFailed = $this->readConfigEnabled('http_client.retry_failed', $container, $retryOptions)) {
+        if ($this->readConfigEnabled('http_client.retry_failed', $container, $retryOptions)) {
             $this->registerRetryableHttpClient($retryOptions, 'http_client', $container);
         }
 
@@ -2371,15 +2371,8 @@ private function registerHttpClientConfiguration(array $config, ContainerBuilder
             throw new LogicException('Support for URI template requires symfony/http-client 6.3 or higher, try upgrading.');
         }
 
-        $httpClientId = match (true) {
-            $hasUriTemplate => 'http_client.uri_template.inner',
-            $hasRetryFailed => 'http_client.retryable.inner',
-            $this->isInitializedConfigEnabled('profiler') => '.debug.http_client.inner',
-            default => 'http_client',
-        };
-
         foreach ($config['scoped_clients'] as $name => $scopeConfig) {
-            if ('http_client' === $name) {
+            if ($container->has($name)) {
                 throw new InvalidArgumentException(sprintf('Invalid scope name: "%s" is reserved.', $name));
             }
 
@@ -2394,17 +2387,17 @@ private function registerHttpClientConfiguration(array $config, ContainerBuilder
 
                 $container->register($name, ScopingHttpClient::class)
                     ->setFactory([ScopingHttpClient::class, 'forBaseUri'])
-                    ->setArguments([new Reference($httpClientId), $baseUri, $scopeConfig])
+                    ->setArguments([new Reference('http_client.transport'), $baseUri, $scopeConfig])
                     ->addTag('http_client.client')
                 ;
             } else {
                 $container->register($name, ScopingHttpClient::class)
-                    ->setArguments([new Reference($httpClientId), [$scope => $scopeConfig], $scope])
+                    ->setArguments([new Reference('http_client.transport'), [$scope => $scopeConfig], $scope])
                     ->addTag('http_client.client')
                 ;
             }
 
-            if ($this->readConfigEnabled('http_client.scoped_clients.'.$name.'retry_failed', $container, $retryOptions)) {
+            if ($this->readConfigEnabled('http_client.scoped_clients.'.$name.'.retry_failed', $container, $retryOptions)) {
                 $this->registerRetryableHttpClient($retryOptions, $name, $container);
             }
 
@@ -2413,7 +2406,7 @@ private function registerHttpClientConfiguration(array $config, ContainerBuilder
                     ->register($name.'.uri_template', UriTemplateHttpClient::class)
                     ->setDecoratedService($name, null, 7) // Between TraceableHttpClient (5) and RetryableHttpClient (10)
                     ->setArguments([
-                        new Reference('.inner'),
+                        new Reference($name.'.uri_template.inner'),
                         new Reference('http_client.uri_template_expander', ContainerInterface::NULL_ON_INVALID_REFERENCE),
                         $defaultUriTemplateVars,
                     ]);
@@ -2430,8 +2423,8 @@ private function registerHttpClientConfiguration(array $config, ContainerBuilder
         }
 
         if ($responseFactoryId = $config['mock_response_factory'] ?? null) {
-            $container->register($httpClientId.'.mock_client', MockHttpClient::class)
-                ->setDecoratedService($httpClientId, null, -10) // lower priority than TraceableHttpClient
+            $container->register('http_client.mock_client', MockHttpClient::class)
+                ->setDecoratedService('http_client.transport', null, -10)  // lower priority than TraceableHttpClient (5)
                 ->setArguments([new Reference($responseFactoryId)]);
         }
     }
@@ -2464,7 +2457,7 @@ private function registerRetryableHttpClient(array $options, string $name, Conta
 
         $container
             ->register($name.'.retryable', RetryableHttpClient::class)
-            ->setDecoratedService($name, null, 10) // higher priority than TraceableHttpClient
+            ->setDecoratedService($name, null, 10) // higher priority than TraceableHttpClient (5)
             ->setArguments([new Reference($name.'.retryable.inner'), $retryStrategy, $options['max_retries'], new Reference('logger')])
             ->addTag('monolog.logger', ['channel' => 'http_client']);
     }

Resources/config/http_client.php

@@ -23,7 +23,7 @@
 
 return static function (ContainerConfigurator $container) {
     $container->services()
-        ->set('http_client', HttpClientInterface::class)
+        ->set('http_client.transport', HttpClientInterface::class)
             ->factory([HttpClient::class, 'create'])
             ->args([
                 [], // default options
@@ -32,6 +32,10 @@
             ->call('setLogger', [service('logger')->ignoreOnInvalid()])
             ->tag('monolog.logger', ['channel' => 'http_client'])
             ->tag('kernel.reset', ['method' => 'reset', 'on_invalid' => 'ignore'])
+
+        ->set('http_client', HttpClientInterface::class)
+            ->factory('current')
+            ->args([[service('http_client.transport')]])
             ->tag('http_client.client')
 
         ->alias(HttpClientInterface::class, 'http_client')

Tests/DependencyInjection/FrameworkExtensionTestCase.php

@@ -55,7 +55,6 @@
 use Symfony\Component\HttpClient\MockHttpClient;
 use Symfony\Component\HttpClient\RetryableHttpClient;
 use Symfony\Component\HttpClient\ScopingHttpClient;
-use Symfony\Component\HttpClient\UriTemplateHttpClient;
 use Symfony\Component\HttpKernel\DependencyInjection\LoggerPass;
 use Symfony\Component\HttpKernel\Fragment\FragmentUriGeneratorInterface;
 use Symfony\Component\Messenger\Transport\TransportFactory;
@@ -1846,14 +1845,14 @@ public function testRobotsTagListenerIsRegisteredInDebugMode()
     public function testHttpClientDefaultOptions()
     {
         $container = $this->createContainerFromFile('http_client_default_options');
-        $this->assertTrue($container->hasDefinition('http_client'), '->registerHttpClientConfiguration() loads http_client.xml');
+        $this->assertTrue($container->hasDefinition('http_client.transport'), '->registerHttpClientConfiguration() loads http_client.xml');
 
         $defaultOptions = [
             'headers' => [],
             'resolve' => [],
             'extra' => [],
         ];
-        $this->assertSame([$defaultOptions, 4], $container->getDefinition('http_client')->getArguments());
+        $this->assertSame([$defaultOptions, 4], $container->getDefinition('http_client.transport')->getArguments());
 
         $this->assertTrue($container->hasDefinition('foo'), 'should have the "foo" service.');
         $this->assertSame(ScopingHttpClient::class, $container->getDefinition('foo')->getClass());
@@ -1871,9 +1870,9 @@ public function testHttpClientOverrideDefaultOptions()
     {
         $container = $this->createContainerFromFile('http_client_override_default_options');
 
-        $this->assertSame(['foo' => 'bar'], $container->getDefinition('http_client')->getArgument(0)['headers']);
-        $this->assertSame(['foo' => 'bar'], $container->getDefinition('http_client')->getArgument(0)['extra']);
-        $this->assertSame(4, $container->getDefinition('http_client')->getArgument(1));
+        $this->assertSame(['foo' => 'bar'], $container->getDefinition('http_client.transport')->getArgument(0)['headers']);
+        $this->assertSame(['foo' => 'bar'], $container->getDefinition('http_client.transport')->getArgument(0)['extra']);
+        $this->assertSame(4, $container->getDefinition('http_client.transport')->getArgument(1));
         $this->assertSame('http://example.com', $container->getDefinition('foo')->getArgument(1));
 
         $expected = [
@@ -1926,7 +1925,7 @@ public function testHttpClientFullDefaultOptions()
     {
         $container = $this->createContainerFromFile('http_client_full_default_options');
 
-        $defaultOptions = $container->getDefinition('http_client')->getArgument(0);
+        $defaultOptions = $container->getDefinition('http_client.transport')->getArgument(0);
 
         $this->assertSame(['X-powered' => 'PHP'], $defaultOptions['headers']);
         $this->assertSame(2, $defaultOptions['max_redirects']);
@@ -2004,15 +2003,15 @@ public function testHttpClientMockResponseFactory()
     {
         $container = $this->createContainerFromFile('http_client_mock_response_factory');
 
-        $definition = $container->getDefinition(($uriTemplateHttpClientExists = class_exists(UriTemplateHttpClient::class)) ? 'http_client.uri_template.inner.mock_client' : 'http_client.mock_client');
+        $definition = $container->getDefinition('http_client.mock_client');
 
         $this->assertSame(MockHttpClient::class, $definition->getClass());
         $this->assertCount(1, $definition->getArguments());
 
         $argument = $definition->getArgument(0);
 
         $this->assertInstanceOf(Reference::class, $argument);
-        $this->assertSame($uriTemplateHttpClientExists ? 'http_client.uri_template.inner' : 'http_client', current($definition->getDecoratedService()));
+        $this->assertSame('http_client.transport', current($definition->getDecoratedService()));
         $this->assertSame('my_response_factory', (string) $argument);
     }