From 13afd2b40660e06fada7433fdde1765371d1299d Mon Sep 17 00:00:00 2001 From: Benjamin Zaslavsky Date: Tue, 19 Dec 2023 09:34:40 +0100 Subject: [PATCH 1/6] [FrameworkBundle] Fix config for array of base_uri in http_client Allow array of `base_uri` in config for HttpClient keys, as per [comment in PR](https://github.com/symfony/symfony/pull/49809#issuecomment-1860729153). --- .../FrameworkBundle/DependencyInjection/Configuration.php | 8 ++++++-- .../Resources/config/schema/symfony-1.0.xsd | 6 ++++++ .../Fixtures/php/http_client_retry.php | 4 ++++ .../Fixtures/xml/http_client_retry.xml | 5 +++++ .../Fixtures/yml/http_client_retry.yml | 6 ++++++ 5 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index ad352160822ae..048d0f3fe3f7c 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -1979,13 +1979,17 @@ private function addHttpClientSection(ArrayNodeDefinition $rootNode, callable $e ->ifTrue(fn ($v) => !empty($v['query']) && !isset($v['base_uri'])) ->thenInvalid('"query" applies to "base_uri" but no base URI is defined.') ->end() + ->validate() + ->ifTrue(fn ($v) => (isset($v['base_uri']) && \is_array($v['base_uri'])) && !isset($v['retry_failed'])) + ->thenInvalid('"base_uri" can only be an array if "retry_failed" is defined.') + ->end() ->children() ->scalarNode('scope') ->info('The regular expression that the request URL must match before adding the other options. When none is provided, the base URI is used instead.') ->cannotBeEmpty() ->end() - ->scalarNode('base_uri') - ->info('The URI to resolve relative URLs, following rules in RFC 3985, section 2.') + ->variableNode('base_uri') + ->info('The URI(s) to resolve relative URLs, following rules in RFC 3985, section 2.') ->cannotBeEmpty() ->end() ->scalarNode('auth_basic') diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd index aedd4a86fd113..7750db1f84637 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd @@ -668,6 +668,7 @@ + @@ -694,6 +695,7 @@ + @@ -747,6 +749,10 @@ + + + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/http_client_retry.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/http_client_retry.php index 28205f8e4ed8f..5ee718e762ce8 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/http_client_retry.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/http_client_retry.php @@ -22,6 +22,10 @@ 'base_uri' => 'http://example.com', 'retry_failed' => ['multiplier' => 4], ], + 'bar' => [ + 'base_uri' => ['http://a.example.com', 'http://b.example.com'], + 'retry_failed' => ['max_retries' => 4], + ], ], ], ]); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/http_client_retry.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/http_client_retry.xml index 296d1b29cd7a6..889cdc040f722 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/http_client_retry.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/http_client_retry.xml @@ -26,6 +26,11 @@ + + http://a.example.com + http://a.example.com + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/http_client_retry.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/http_client_retry.yml index ea59b82d98e7a..dd98cbebb7007 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/http_client_retry.yml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/http_client_retry.yml @@ -21,3 +21,9 @@ framework: base_uri: http://example.com retry_failed: multiplier: 4 + bar: + base_uri: + - http://a.example.com + - http://b.example.com + retry_failed: + max_retries: 3 From ff930e56973268b8dbc775f8a67c447246d781df Mon Sep 17 00:00:00 2001 From: Benjamin Zaslavsky Date: Tue, 19 Dec 2023 09:40:13 +0100 Subject: [PATCH 2/6] Fix indent --- .../FrameworkBundle/DependencyInjection/Configuration.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index 048d0f3fe3f7c..59db3b7ef5b04 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -1988,7 +1988,7 @@ private function addHttpClientSection(ArrayNodeDefinition $rootNode, callable $e ->info('The regular expression that the request URL must match before adding the other options. When none is provided, the base URI is used instead.') ->cannotBeEmpty() ->end() - ->variableNode('base_uri') + ->variableNode('base_uri') ->info('The URI(s) to resolve relative URLs, following rules in RFC 3985, section 2.') ->cannotBeEmpty() ->end() From 62fee937ee888b4efce5782b53109e45c1dd6510 Mon Sep 17 00:00:00 2001 From: Benjamin Zaslavsky Date: Tue, 19 Dec 2023 13:10:15 +0100 Subject: [PATCH 3/6] Base uri argument for `RetryableHttpClient::__construct` Add optional `$baseUri` argument for `RetryableHttpClient::__construct` to allow proper integration in franework --- .../DependencyInjection/Configuration.php | 8 +++++++- .../DependencyInjection/FrameworkExtension.php | 14 +++++++++----- .../Component/HttpClient/RetryableHttpClient.php | 3 ++- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index 59db3b7ef5b04..ae87094405eb0 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -1980,7 +1980,13 @@ private function addHttpClientSection(ArrayNodeDefinition $rootNode, callable $e ->thenInvalid('"query" applies to "base_uri" but no base URI is defined.') ->end() ->validate() - ->ifTrue(fn ($v) => (isset($v['base_uri']) && \is_array($v['base_uri'])) && !isset($v['retry_failed'])) + ->ifTrue(fn ($v) => ( + (isset($v['base_uri']) && \is_array($v['base_uri'])) + && ( + (!isset($v['retry_failed']) || $v['retry_failed']['enabled'] === false) + || \count($v['base_uri']) !== \count(\array_filter($v['base_uri'], 'is_string')) + ) + )) ->thenInvalid('"base_uri" can only be an array if "retry_failed" is defined.') ->end() ->children() diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 039707722dee3..46ed42a7d0ed9 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -2546,8 +2546,7 @@ private function registerHttpClientConfiguration(array $config, ContainerBuilder unset($scopeConfig['retry_failed']); if (null === $scope) { - $baseUri = $scopeConfig['base_uri']; - unset($scopeConfig['base_uri']); + $baseUri = \is_array($scopeConfig['base_uri']) ? $scopeConfig['base_uri'][0] : $scopeConfig['base_uri']; $container->register($name, ScopingHttpClient::class) ->setFactory([ScopingHttpClient::class, 'forBaseUri']) @@ -2562,7 +2561,12 @@ private function registerHttpClientConfiguration(array $config, ContainerBuilder } if ($this->readConfigEnabled('http_client.scoped_clients.'.$name.'.retry_failed', $container, $retryOptions)) { - $this->registerRetryableHttpClient($retryOptions, $name, $container); + $baseUris = []; + if (isset($scopeConfig['base_uri']) && \is_array($scopeConfig['base_uri'])) { + $baseUris = $scopeConfig['base_uri']; + unset($scopeConfig['base_uri']); + } + $this->registerRetryableHttpClient($retryOptions, $name, $container, $baseUris); } $container @@ -2598,7 +2602,7 @@ private function registerHttpClientConfiguration(array $config, ContainerBuilder } } - private function registerRetryableHttpClient(array $options, string $name, ContainerBuilder $container): void + private function registerRetryableHttpClient(array $options, string $name, ContainerBuilder $container, array $baseUris = []): void { if (null !== $options['retry_strategy']) { $retryStrategy = new Reference($options['retry_strategy']); @@ -2627,7 +2631,7 @@ private function registerRetryableHttpClient(array $options, string $name, Conta $container ->register($name.'.retryable', RetryableHttpClient::class) ->setDecoratedService($name, null, 10) // higher priority than TraceableHttpClient (5) - ->setArguments([new Reference($name.'.retryable.inner'), $retryStrategy, $options['max_retries'], new Reference('logger')]) + ->setArguments([new Reference($name.'.retryable.inner'), $retryStrategy, $options['max_retries'], new Reference('logger'), $baseUris]) ->addTag('monolog.logger', ['channel' => 'http_client']); } diff --git a/src/Symfony/Component/HttpClient/RetryableHttpClient.php b/src/Symfony/Component/HttpClient/RetryableHttpClient.php index d3b779420ffa9..c577a2d3ea3a4 100644 --- a/src/Symfony/Component/HttpClient/RetryableHttpClient.php +++ b/src/Symfony/Component/HttpClient/RetryableHttpClient.php @@ -39,12 +39,13 @@ class RetryableHttpClient implements HttpClientInterface, ResetInterface /** * @param int $maxRetries The maximum number of times to retry */ - public function __construct(HttpClientInterface $client, ?RetryStrategyInterface $strategy = null, int $maxRetries = 3, ?LoggerInterface $logger = null) + public function __construct(HttpClientInterface $client, ?RetryStrategyInterface $strategy = null, int $maxRetries = 3, ?LoggerInterface $logger = null, array $baseUris = []) { $this->client = $client; $this->strategy = $strategy ?? new GenericRetryStrategy(); $this->maxRetries = $maxRetries; $this->logger = $logger; + $this->baseUris = $baseUris; } public function withOptions(array $options): static From 1aaeb6ab891123c01aa326b545a89d6556b7d03a Mon Sep 17 00:00:00 2001 From: Benjamin Zaslavsky Date: Tue, 19 Dec 2023 13:18:44 +0100 Subject: [PATCH 4/6] Fix scopeConfig array for base_uri --- .../FrameworkBundle/DependencyInjection/FrameworkExtension.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 46ed42a7d0ed9..16a22b3d35352 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -2547,10 +2547,11 @@ private function registerHttpClientConfiguration(array $config, ContainerBuilder if (null === $scope) { $baseUri = \is_array($scopeConfig['base_uri']) ? $scopeConfig['base_uri'][0] : $scopeConfig['base_uri']; + $config = \array_filter($scopeConfig, fn($k) => $k !== 'base_uri', ARRAY_FILTER_USE_KEY); $container->register($name, ScopingHttpClient::class) ->setFactory([ScopingHttpClient::class, 'forBaseUri']) - ->setArguments([new Reference('http_client.transport'), $baseUri, $scopeConfig]) + ->setArguments([new Reference('http_client.transport'), $baseUri, $config]) ->addTag('http_client.client') ; } else { From f93711e2e643e4a14498c67b21938bea6c61f8b9 Mon Sep 17 00:00:00 2001 From: Benjamin Zaslavsky Date: Tue, 19 Dec 2023 14:10:49 +0100 Subject: [PATCH 5/6] Fix CS from patch --- .../FrameworkBundle/DependencyInjection/Configuration.php | 4 ++-- .../DependencyInjection/FrameworkExtension.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index ae87094405eb0..0430945be5fd1 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -1983,8 +1983,8 @@ private function addHttpClientSection(ArrayNodeDefinition $rootNode, callable $e ->ifTrue(fn ($v) => ( (isset($v['base_uri']) && \is_array($v['base_uri'])) && ( - (!isset($v['retry_failed']) || $v['retry_failed']['enabled'] === false) - || \count($v['base_uri']) !== \count(\array_filter($v['base_uri'], 'is_string')) + (!isset($v['retry_failed']) || false === $v['retry_failed']['enabled']) + || \count($v['base_uri']) !== \count(array_filter($v['base_uri'], 'is_string')) ) )) ->thenInvalid('"base_uri" can only be an array if "retry_failed" is defined.') diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 16a22b3d35352..7baa56e2bae1f 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -2547,7 +2547,7 @@ private function registerHttpClientConfiguration(array $config, ContainerBuilder if (null === $scope) { $baseUri = \is_array($scopeConfig['base_uri']) ? $scopeConfig['base_uri'][0] : $scopeConfig['base_uri']; - $config = \array_filter($scopeConfig, fn($k) => $k !== 'base_uri', ARRAY_FILTER_USE_KEY); + $config = array_filter($scopeConfig, fn ($k) => 'base_uri' !== $k, \ARRAY_FILTER_USE_KEY); $container->register($name, ScopingHttpClient::class) ->setFactory([ScopingHttpClient::class, 'forBaseUri']) From 19a43c9c959e672d91cc9067a83c31877148568a Mon Sep 17 00:00:00 2001 From: Benjamin Zaslavsky Date: Wed, 20 Dec 2023 09:48:43 +0100 Subject: [PATCH 6/6] Rework to avoid refactoring of RetryableHttpClient --- .../DependencyInjection/FrameworkExtension.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 7baa56e2bae1f..d529efc72d447 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -2632,8 +2632,9 @@ private function registerRetryableHttpClient(array $options, string $name, Conta $container ->register($name.'.retryable', RetryableHttpClient::class) ->setDecoratedService($name, null, 10) // higher priority than TraceableHttpClient (5) - ->setArguments([new Reference($name.'.retryable.inner'), $retryStrategy, $options['max_retries'], new Reference('logger'), $baseUris]) - ->addTag('monolog.logger', ['channel' => 'http_client']); + ->setArguments([new Reference($name.'.retryable.inner'), $retryStrategy, $options['max_retries'], new Reference('logger')]) + ->addTag('monolog.logger', ['channel' => 'http_client']) + ->addMethodCall('withOptions', [['base_uri' => $baseUris]], true); } private function registerMailerConfiguration(array $config, ContainerBuilder $container, PhpFileLoader $loader, bool $webhookEnabled): void