From 178f8adc4947eef15478823ea929747cce5dc1ca Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Thu, 15 Oct 2026 17:49:41 +0200 Subject: [PATCH] #58 - Load the REST clients lazily from a service locator indexed by code - RegisterClientsPass: the services tagged cleverage.rest.client with a code attribute are registered in a service locator passed to the ClientRegistry (new optional constructor argument): a client is only instantiated when a task uses it. Duplicate or empty codes fail the container compilation. Without code, the client is still registered with addClient(), now deprecated (the code will be required in 4.0). - ClientRegistry: getClient() checks that the service implements ClientInterface and that its getCode() is the code of the tag; getClients() instantiates every client; FQCN alias to autowire it. - New #[AsRestClient(code: '...')] attribute (autoconfiguration), adding the tag with its code. - Require symfony/service-contracts (ServiceProviderInterface). - Docs: code attribute in every example, registration section. Same model as process-bundle#276 (TransformerRegistry). Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 1 + composer.json | 3 +- config/services/registry.yaml | 6 + docs/cookbooks/api_to_dto.md | 2 +- docs/cookbooks/csv_to_api.md | 2 +- docs/index.md | 4 +- docs/reference/client.md | 45 +++++-- docs/reference/tasks/request_task.md | 2 +- src/Attribute/AsRestClient.php | 29 +++++ .../CleverAgeRestProcessExtension.php | 9 ++ .../Compiler/RegisterClientsPass.php | 36 +++++- src/Registry/ClientRegistry.php | 66 ++++++++-- .../CleverAgeRestProcessExtensionTest.php | 23 ++++ .../Compiler/RegisterClientsPassTest.php | 59 +++++++-- tests/Fixtures/AttributedClient.php | 22 ++++ tests/Registry/ClientRegistryTest.php | 120 ++++++++++++++++-- tests/Task/RequestTaskTest.php | 5 +- 17 files changed, 377 insertions(+), 57 deletions(-) create mode 100644 src/Attribute/AsRestClient.php create mode 100644 tests/Fixtures/AttributedClient.php diff --git a/CHANGELOG.md b/CHANGELOG.md index fca379a..8d9eb80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ Latest ------ ### Changes +* [#58](https://github.com/cleverage/rest-process-bundle/issues/58) ClientRegistry: the clients are loaded lazily from a service locator indexed by code: a client is only instantiated when a task uses it (an error in a client that a process does not use no longer makes it fail). The code is given by the new `code` attribute of the `cleverage.rest.client` tag, or by the new `#[AsRestClient(code: '...')]` attribute (autoconfiguration); it must be the one returned by `getCode()`. Duplicate or empty codes fail the container compilation. A client tagged without `code` is still registered eagerly, with `ClientRegistry::addClient()`, which is deprecated (the `code` will be required in 4.0). New optional `ClientRegistry` constructor argument (the locator); `ClientRegistry` is autowirable by its class name. Require `symfony/service-contracts`. Update documentation, add tests. * [#56](https://github.com/cleverage/rest-process-bundle/issues/56) Require `symfony/polyfill-php83`, `symfony/polyfill-php84` and `symfony/polyfill-php85` ([cleverage/process-bundle#299](https://github.com/cleverage/process-bundle/issues/299)): the functions of PHP 8.3 to 8.5 can be used while supporting PHP 8.2. Rector targets PHP 8.2 (`withPhpVersion()`, `withPhpSets()` read from `composer.json`) and applies the rules using a newer function when its polyfill is required (no change needed in the current code). * [#52](https://github.com/cleverage/rest-process-bundle/issues/52) Give a code to the task ([cleverage/process-bundle#91](https://github.com/cleverage/process-bundle/issues/91)): `rest_request` (`task: rest_request`, with `cleverage/process-bundle` 5.4 or later, referencing it by `service` being deprecated there). The task has its class name as service id; fetching it directly from the service container is deprecated (`container.private`), it will be private in 4.0. The former id `cleverage_rest_process.task.request` is a deprecated alias, removed in 4.0. Update documentation, add tests. * [#50](https://github.com/cleverage/rest-process-bundle/issues/50) CI: test the lowest versions allowed by `composer.json` (`--prefer-lowest`, PHP 8.2). Raise `psr/log` to `^2 || ^3`: `cleverage/process-bundle` loggers and the test loggers use the typed `log()` signature, a fatal error with `psr/log` 1. diff --git a/composer.json b/composer.json index 48ee50f..072ddfb 100644 --- a/composer.json +++ b/composer.json @@ -62,7 +62,8 @@ "symfony/options-resolver": "^6.4 || ^7.4 || ^8", "symfony/polyfill-php83": "^1.33", "symfony/polyfill-php84": "^1.33", - "symfony/polyfill-php85": "^1.33" + "symfony/polyfill-php85": "^1.33", + "symfony/service-contracts": "^2.5 || ^3" }, "require-dev": { "friendsofphp/php-cs-fixer": "*", diff --git a/config/services/registry.yaml b/config/services/registry.yaml index f2f59d9..826ad48 100644 --- a/config/services/registry.yaml +++ b/config/services/registry.yaml @@ -2,3 +2,9 @@ services: cleverage_rest_process.registry.client: class: CleverAge\RestProcessBundle\Registry\ClientRegistry public: false + arguments: + - ~ # Service locator of the clients by code, see RegisterClientsPass + + CleverAge\RestProcessBundle\Registry\ClientRegistry: + alias: cleverage_rest_process.registry.client + public: false diff --git a/docs/cookbooks/api_to_dto.md b/docs/cookbooks/api_to_dto.md index 00d2cc0..4db5966 100644 --- a/docs/cookbooks/api_to_dto.md +++ b/docs/cookbooks/api_to_dto.md @@ -43,7 +43,7 @@ services: $code: 'apicarto_ign' $uri: 'https://apicarto.ign.fr/api' tags: - - { name: cleverage.rest.client } + - { name: cleverage.rest.client, code: apicarto_ign } ``` The process, executed with `bin/console cleverage:process:execute app.communes_export -c codePostal:"'46800'"`: diff --git a/docs/cookbooks/csv_to_api.md b/docs/cookbooks/csv_to_api.md index 4ad3c34..c06ed08 100644 --- a/docs/cookbooks/csv_to_api.md +++ b/docs/cookbooks/csv_to_api.md @@ -23,7 +23,7 @@ services: $code: 'catalog' $uri: '%env(CATALOG_API_URI)%' # e.g. https://catalog.example.com/api tags: - - { name: cleverage.rest.client } + - { name: cleverage.rest.client, code: catalog } ``` The process: diff --git a/docs/index.md b/docs/index.md index 7b4465e..5503118 100644 --- a/docs/index.md +++ b/docs/index.md @@ -31,7 +31,7 @@ Its service id is its class name. Fetching it directly from the service containe private in 4.0); its former id, `cleverage_rest_process.task.request`, is a deprecated alias since 3.3, removed in 4.0. Each API is accessed through a [REST client](reference/client.md): a -service tagged `cleverage.rest.client`, identified by a unique code and holding the base URI of the API. +service tagged `cleverage.rest.client` with a unique code, holding the base URI of the API. ```yaml # config/services.yaml @@ -44,7 +44,7 @@ services: $code: 'apicarto_ign' $uri: 'https://apicarto.ign.fr/api' tags: - - { name: cleverage.rest.client } + - { name: cleverage.rest.client, code: apicarto_ign } ``` The [RequestTask](reference/tasks/request_task.md) then references this client by its code: diff --git a/docs/reference/client.md b/docs/reference/client.md index 183630e..b8c38f2 100644 --- a/docs/reference/client.md +++ b/docs/reference/client.md @@ -2,8 +2,8 @@ REST client =========== A REST client holds the connection to one API (its base URI, the HTTP client to use) and sends the requests built by -the [RequestTask](tasks/request_task.md). Each client is a service tagged `cleverage.rest.client`, identified by a -unique code that the task references with its `client` option. +the [RequestTask](tasks/request_task.md). Each client is a service tagged `cleverage.rest.client` with its code (or +using the `#[AsRestClient]` attribute), a unique code that the task references with its `client` option. Client reference ---------------- @@ -11,8 +11,11 @@ Client reference * **Interface**: `CleverAge\RestProcessBundle\Client\ClientInterface` * **Default implementation**: `CleverAge\RestProcessBundle\Client\Client`, based on the Symfony [HttpClient](https://symfony.com/doc/current/http_client.html) -* **Service tag**: `cleverage.rest.client` -* **Registry**: `CleverAge\RestProcessBundle\Registry\ClientRegistry` (service `cleverage_rest_process.registry.client`) +* **Service tag**: `cleverage.rest.client`, with a `code` attribute (`- { name: cleverage.rest.client, code: my_api }`) +* **Attribute**: `CleverAge\RestProcessBundle\Attribute\AsRestClient` (`#[AsRestClient(code: 'my_api')]`, with + autoconfiguration), equivalent to the tag +* **Registry**: `CleverAge\RestProcessBundle\Registry\ClientRegistry` (service `cleverage_rest_process.registry.client`, + autowirable by its class name) Constructor arguments --------------------- @@ -69,13 +72,31 @@ only occur when the task reads the response. Registration ------------ -Every service tagged `cleverage.rest.client` is added to the registry (a compiler pass calls -`ClientRegistry::addClient()` for each of them, with the service id). Two clients with the same code throw an -`UnexpectedValueException` giving the ids of both services -(`Client is already defined by service "", cannot register service ""`) when the registry is -instantiated; a task referencing an unknown code throws a +The services tagged `cleverage.rest.client` are registered in the registry by their `code` attribute, in a service +locator built when the container is compiled: a client is only instantiated when a task uses it (an error in a client +that a process does not use does not make it fail). The `code` must be the one returned by the `getCode()` method of +the client (an `UnexpectedValueException` is thrown otherwise when it is fetched). Two clients with the same code, or an +empty code, fail the container compilation; a task referencing an unknown code throws a `CleverAge\RestProcessBundle\Exception\MissingClientException` (`No rest client with code : `). +The `#[AsRestClient]` attribute adds the same tag to an autoconfigured service: + +```php +use CleverAge\RestProcessBundle\Attribute\AsRestClient; +use CleverAge\RestProcessBundle\Client\Client; + +#[AsRestClient(code: 'my_api')] +class MyApiClient extends Client +{ +} +``` + +**Deprecated**: a service tagged without `code` is still registered, with `ClientRegistry::addClient()`, instantiated +with the registry (deprecated since 3.3, the `code` will be required in 4.0). Two clients with the same code then throw +an `UnexpectedValueException` giving the ids of both services +(`Client is already defined by service "", cannot register service ""`) when the registry is +instantiated. + Implementing a client --------------------- @@ -110,7 +131,7 @@ services: $code: 'apicarto_ign' $uri: 'https://apicarto.ign.fr/api' tags: - - { name: cleverage.rest.client } + - { name: cleverage.rest.client, code: apicarto_ign } ``` * Default client without autowiring, using a @@ -137,7 +158,7 @@ services: $code: 'domain_sample' $uri: 'https://domain/api' tags: - - { name: cleverage.rest.client } + - { name: cleverage.rest.client, code: domain_sample } ``` * Custom client adding an API key header to every request @@ -186,5 +207,5 @@ services: $uri: 'https://domain/api' $apiKey: '%env(DOMAIN_API_KEY)%' tags: - - { name: cleverage.rest.client } + - { name: cleverage.rest.client, code: domain_sample } ``` diff --git a/docs/reference/tasks/request_task.md b/docs/reference/tasks/request_task.md index 8960e84..b76628c 100644 --- a/docs/reference/tasks/request_task.md +++ b/docs/reference/tasks/request_task.md @@ -69,7 +69,7 @@ services: $code: 'apicarto_ign' $uri: 'https://apicarto.ign.fr/api' tags: - - { name: cleverage.rest.client } + - { name: cleverage.rest.client, code: apicarto_ign } ``` ```yaml diff --git a/src/Attribute/AsRestClient.php b/src/Attribute/AsRestClient.php new file mode 100644 index 0000000..458985f --- /dev/null +++ b/src/Attribute/AsRestClient.php @@ -0,0 +1,29 @@ +findServices($container, __DIR__.'/../../config/services'); + + $container->registerAttributeForAutoconfiguration( + AsRestClient::class, + static function (ChildDefinition $definition, AsRestClient $attribute): void { + $definition->addTag('cleverage.rest.client', ['code' => $attribute->code]); + } + ); } /** diff --git a/src/DependencyInjection/Compiler/RegisterClientsPass.php b/src/DependencyInjection/Compiler/RegisterClientsPass.php index 81a3aad..481f10e 100644 --- a/src/DependencyInjection/Compiler/RegisterClientsPass.php +++ b/src/DependencyInjection/Compiler/RegisterClientsPass.php @@ -14,11 +14,14 @@ namespace CleverAge\RestProcessBundle\DependencyInjection\Compiler; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; +use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; use Symfony\Component\DependencyInjection\Reference; /** - * Adds the tagged REST clients to the client registry, with their service id. + * Registers the `cleverage.rest.client` tagged services in the client registry: in a service locator indexed by the + * `code` attribute of the tag (lazy), or, without `code` (deprecated), with an addClient() call (eager). */ class RegisterClientsPass implements CompilerPassInterface { @@ -27,10 +30,35 @@ public function process(ContainerBuilder $container): void if (!$container->has('cleverage_rest_process.registry.client')) { return; } - $definition = $container->findDefinition('cleverage_rest_process.registry.client'); - foreach (array_keys($container->findTaggedServiceIds('cleverage.rest.client')) as $id) { - $definition->addMethodCall('addClient', [new Reference($id), $id]); + + $services = []; + $ids = []; + foreach ($container->findTaggedServiceIds('cleverage.rest.client') as $id => $tags) { + $codes = []; + foreach ($tags as $attributes) { + if (!\is_array($attributes) || !isset($attributes['code'])) { + continue; + } + if (!\is_string($attributes['code']) || '' === $attributes['code']) { + throw new InvalidArgumentException(\sprintf('The "code" attribute of the "cleverage.rest.client" tag of the "%s" service must be a non-empty string.', $id)); + } + $codes[] = $attributes['code']; + } + if ([] === $codes) { + // Deprecated: instantiated with the registry, addClient() triggers the deprecation + $definition->addMethodCall('addClient', [new Reference($id), $id]); + continue; + } + foreach (array_unique($codes) as $code) { + if (isset($ids[$code])) { + throw new InvalidArgumentException(\sprintf('The REST client code "%s" is used by the "%s" and "%s" services.', $code, $ids[$code], $id)); + } + $ids[$code] = $id; + $services[$code] = new Reference($id); + } } + + $definition->setArgument(0, ServiceLocatorTagPass::register($container, $services)); } } diff --git a/src/Registry/ClientRegistry.php b/src/Registry/ClientRegistry.php index 54c0c2a..bf99bda 100644 --- a/src/Registry/ClientRegistry.php +++ b/src/Registry/ClientRegistry.php @@ -15,28 +15,57 @@ use CleverAge\RestProcessBundle\Client\ClientInterface; use CleverAge\RestProcessBundle\Exception\MissingClientException; +use Symfony\Contracts\Service\ServiceProviderInterface; /** - * Holds all tagged rest client services. + * Holds the REST clients, indexed by code. + * + * The client services are fetched lazily from a service locator indexed by code (`code` attribute of the + * `cleverage.rest.client` tag, or `#[AsRestClient]`, see RegisterClientsPass): a client is only instantiated when it + * is used. */ class ClientRegistry { - /** @var ClientInterface[] */ + /** + * Clients already fetched from the locator, and the ones registered with addClient() (deprecated). + * + * @var array + */ private array $clients = []; - /** @var array Service ids of the clients, indexed by code */ + /** @var array Service ids of the clients registered with addClient(), indexed by code */ private array $serviceIds = []; + /** + * @param ServiceProviderInterface|null $locator Client services, checked when fetched + */ + public function __construct( + private readonly ?ServiceProviderInterface $locator = null, + ) { + } + /** * @param string|null $serviceId Id of the client service, used to identify the clients with the same code + * + * @deprecated since 3.3, tag the client service with a "code" attribute or use #[AsRestClient] instead */ public function addClient(ClientInterface $client, ?string $serviceId = null): void { $code = $client->getCode(); - if (\array_key_exists($code, $this->getClients())) { + trigger_deprecation( + 'cleverage/rest-process-bundle', + '3.3', + 'Registering the REST client "%s" (%s) with "%s()" is deprecated, tag its service with a "code" attribute ({ name: cleverage.rest.client, code: %s }) or use the #[AsRestClient] attribute instead, so that it is only instantiated when used. It will be required in 4.0.', + $code, + $client::class, + __METHOD__, + $code, + ); + if ($this->hasClient($code)) { $message = "Client {$code} is already defined"; - if (null !== $this->serviceIds[$code] && null !== $serviceId) { - $message .= " by service \"{$this->serviceIds[$code]}\", cannot register service \"{$serviceId}\""; + $definedBy = $this->serviceIds[$code] ?? null; + if (null !== $definedBy && null !== $serviceId) { + $message .= " by service \"{$definedBy}\", cannot register service \"{$serviceId}\""; } throw new \UnexpectedValueException($message); @@ -46,10 +75,16 @@ public function addClient(ClientInterface $client, ?string $serviceId = null): v } /** - * @return ClientInterface[] + * Instantiates every client of the locator. + * + * @return array */ public function getClients(): array { + foreach (array_keys($this->locator?->getProvidedServices() ?? []) as $code) { + $this->getClient((string) $code); + } + return $this->clients; } @@ -58,15 +93,26 @@ public function getClients(): array */ public function getClient(string $code): ClientInterface { - if (!$this->hasClient($code)) { + if (isset($this->clients[$code])) { + return $this->clients[$code]; + } + if (!$this->locator?->has($code)) { throw MissingClientException::create($code); } - return $this->getClients()[$code]; + $client = $this->locator->get($code); + if (!$client instanceof ClientInterface) { + throw new \UnexpectedValueException(\sprintf('The service of the REST client "%s" must implement "%s".', $code, ClientInterface::class)); + } + if ($code !== $client->getCode()) { + throw new \UnexpectedValueException(\sprintf('The REST client "%s" is registered with the code "%s" but its getCode() method returns "%s": both must be the same.', $client::class, $code, $client->getCode())); + } + + return $this->clients[$code] = $client; } public function hasClient(string $code): bool { - return \array_key_exists($code, $this->getClients()); + return isset($this->clients[$code]) || ($this->locator?->has($code) ?? false); } } diff --git a/tests/DependencyInjection/CleverAgeRestProcessExtensionTest.php b/tests/DependencyInjection/CleverAgeRestProcessExtensionTest.php index f076424..38b11f1 100644 --- a/tests/DependencyInjection/CleverAgeRestProcessExtensionTest.php +++ b/tests/DependencyInjection/CleverAgeRestProcessExtensionTest.php @@ -13,15 +13,23 @@ namespace CleverAge\RestProcessBundle\Tests\DependencyInjection; +use CleverAge\RestProcessBundle\Attribute\AsRestClient; use CleverAge\RestProcessBundle\DependencyInjection\CleverAgeRestProcessExtension; use CleverAge\RestProcessBundle\Registry\ClientRegistry; use CleverAge\RestProcessBundle\Task\RequestTask; +use CleverAge\RestProcessBundle\Tests\Fixtures\AttributedClient; use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; +use Symfony\Component\DependencyInjection\Compiler\AttributeAutoconfigurationPass; +use Symfony\Component\DependencyInjection\Compiler\ResolveChildDefinitionsPass; +use Symfony\Component\DependencyInjection\Compiler\ResolveInstanceofConditionalsPass; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Reference; #[CoversClass(CleverAgeRestProcessExtension::class)] +#[UsesClass(AsRestClient::class)] class CleverAgeRestProcessExtensionTest extends TestCase { public function testServices(): void @@ -30,6 +38,8 @@ public function testServices(): void (new CleverAgeRestProcessExtension())->load([], $container); self::assertSame(ClientRegistry::class, $container->getDefinition('cleverage_rest_process.registry.client')->getClass()); + // Autowirable by its class name + self::assertSame('cleverage_rest_process.registry.client', (string) $container->getAlias(ClientRegistry::class)); // The class name is the service id $definition = $container->getDefinition(RequestTask::class); @@ -54,4 +64,17 @@ public function testServices(): void $alias->getDeprecation($deprecatedId) ); } + + public function testAsRestClientAttributeRegistersTheCode(): void + { + $container = new ContainerBuilder(); + (new CleverAgeRestProcessExtension())->load([], $container); + $container->setDefinition('app.attributed', (new Definition(AttributedClient::class))->setAutoconfigured(true)); + + (new AttributeAutoconfigurationPass())->process($container); + (new ResolveInstanceofConditionalsPass())->process($container); + (new ResolveChildDefinitionsPass())->process($container); + + self::assertSame([['code' => 'app_attributed']], $container->getDefinition('app.attributed')->getTag('cleverage.rest.client')); + } } diff --git a/tests/DependencyInjection/Compiler/RegisterClientsPassTest.php b/tests/DependencyInjection/Compiler/RegisterClientsPassTest.php index 76fe226..351fef3 100644 --- a/tests/DependencyInjection/Compiler/RegisterClientsPassTest.php +++ b/tests/DependencyInjection/Compiler/RegisterClientsPassTest.php @@ -22,6 +22,7 @@ use Psr\Log\NullLogger; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; +use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; use Symfony\Component\HttpClient\MockHttpClient; #[CoversClass(RegisterClientsPass::class)] @@ -29,25 +30,62 @@ #[UsesClass(Client::class)] class RegisterClientsPassTest extends TestCase { - public function testClientsAreRegistered(): void + public function testClientsWithACodeAreRegisteredInALocator(): void { - $container = $this->createContainer(['app.client' => 'client', 'app.other' => 'other']); + $container = $this->createContainer(['app.client' => 'client', 'app.other' => 'other'], withCode: true); $container->compile(true); /** @var ClientRegistry $registry */ $registry = $container->get('cleverage_rest_process.registry.client'); + // Not instantiated with the registry + self::assertFalse($container->initialized('app.client')); self::assertSame('client', $registry->getClient('client')->getCode()); + self::assertTrue($container->initialized('app.client')); + self::assertFalse($container->initialized('app.other')); self::assertSame('other', $registry->getClient('other')->getCode()); } - public function testDuplicateCodeGivesTheServiceIds(): void + public function testClientsWithoutCodeAreAddedToTheRegistry(): void { - $container = $this->createContainer(['app.client' => 'client', 'app.client_duplicate' => 'client']); + $container = $this->createContainer(['app.client' => 'client', 'app.other' => 'other']); $container->compile(true); - $this->expectException(\UnexpectedValueException::class); - $this->expectExceptionMessage('Client client is already defined by service "app.client", cannot register service "app.client_duplicate"'); - $container->get('cleverage_rest_process.registry.client'); + $deprecations = []; + set_error_handler(static function (int $errno, string $errstr) use (&$deprecations): bool { + $deprecations[] = $errstr; + + return true; + }, \E_USER_DEPRECATED); + try { + /** @var ClientRegistry $registry */ + $registry = $container->get('cleverage_rest_process.registry.client'); + } finally { + restore_error_handler(); + } + + self::assertCount(2, $deprecations); + self::assertSame('client', $registry->getClient('client')->getCode()); + self::assertSame('other', $registry->getClient('other')->getCode()); + } + + public function testDuplicateCodeFailsTheCompilation(): void + { + $container = $this->createContainer(['app.client' => 'client', 'app.client_duplicate' => 'client'], withCode: true); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('The REST client code "client" is used by the "app.client" and "app.client_duplicate" services.'); + $container->compile(true); + } + + public function testEmptyCodeFailsTheCompilation(): void + { + $container = $this->createContainer([]); + $container->setDefinition('app.client', new Definition(Client::class)) + ->addTag('cleverage.rest.client', ['code' => '']); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('The "code" attribute of the "cleverage.rest.client" tag of the "app.client" service must be a non-empty string.'); + $container->compile(true); } public function testWithoutRegistry(): void @@ -61,15 +99,16 @@ public function testWithoutRegistry(): void /** * @param array $clients Codes of the clients, indexed by service id */ - private function createContainer(array $clients): ContainerBuilder + private function createContainer(array $clients, bool $withCode = false): ContainerBuilder { $container = new ContainerBuilder(); $container->addCompilerPass(new RegisterClientsPass()); - $container->setDefinition('cleverage_rest_process.registry.client', new Definition(ClientRegistry::class)) + $container->setDefinition('cleverage_rest_process.registry.client', new Definition(ClientRegistry::class, [null])) ->setPublic(true); foreach ($clients as $id => $code) { $container->setDefinition($id, new Definition(Client::class, [new Definition(MockHttpClient::class), new Definition(NullLogger::class), $code, 'https://example.com'])) - ->addTag('cleverage.rest.client'); + ->setPublic(true) + ->addTag('cleverage.rest.client', $withCode ? ['code' => $code] : []); } return $container; diff --git a/tests/Fixtures/AttributedClient.php b/tests/Fixtures/AttributedClient.php new file mode 100644 index 0000000..5a824c8 --- /dev/null +++ b/tests/Fixtures/AttributedClient.php @@ -0,0 +1,22 @@ + Codes of the clients instantiated by the locator */ + private array $instantiated = []; + + public function testClientsAreFetchedLazilyFromTheLocator(): void { - $registry = new ClientRegistry(); - $client = $this->createClient('client'); - $other = $this->createClient('other'); - $registry->addClient($client, 'app.client'); - $registry->addClient($other); + $registry = new ClientRegistry($this->createLocator(['client' => 'client', 'other' => 'other'])); - self::assertSame($client, $registry->getClient('client')); - self::assertSame($other, $registry->getClient('other')); self::assertTrue($registry->hasClient('client')); - self::assertSame(['client' => $client, 'other' => $other], $registry->getClients()); + self::assertFalse($registry->hasClient('missing')); + self::assertSame([], $this->instantiated); + + $client = $registry->getClient('client'); + self::assertSame('client', $client->getCode()); + self::assertSame($client, $registry->getClient('client')); + self::assertSame(['client'], $this->instantiated); + + self::assertSame(['client', 'other'], array_keys($registry->getClients())); + self::assertSame(['client', 'other'], $this->instantiated); } public function testMissingClient(): void { $this->expectException(MissingClientException::class); $this->expectExceptionMessage('No rest client with code : missing'); + (new ClientRegistry($this->createLocator([])))->getClient('missing'); + } + + public function testMissingClientWithoutLocator(): void + { + $this->expectException(MissingClientException::class); (new ClientRegistry())->getClient('missing'); } + public function testCodeOfTheTagMustBeTheCodeOfTheClient(): void + { + $registry = new ClientRegistry($this->createLocator(['client' => 'other'])); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessageMatches('/is registered with the code "client" but its getCode\(\) method returns "other"/'); + $registry->getClient('client'); + } + + public function testServiceMustBeAClient(): void + { + $registry = new ClientRegistry(new ServiceLocator(['client' => static fn (): \stdClass => new \stdClass()])); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('The service of the REST client "client" must implement "'.ClientInterface::class.'".'); + $registry->getClient('client'); + } + + public function testAddClientIsDeprecated(): void + { + $registry = new ClientRegistry($this->createLocator(['client' => 'client'])); + $other = $this->createClient('other'); + + $deprecations = $this->collectDeprecations(static fn () => $registry->addClient($other, 'app.other')); + + self::assertSame( + ['Since cleverage/rest-process-bundle 3.3: Registering the REST client "other" ('.$other::class.') with "'.ClientRegistry::class.'::addClient()" is deprecated, tag its service with a "code" attribute ({ name: cleverage.rest.client, code: other }) or use the #[AsRestClient] attribute instead, so that it is only instantiated when used. It will be required in 4.0.'], + $deprecations + ); + self::assertSame($other, $registry->getClient('other')); + self::assertEqualsCanonicalizing(['client', 'other'], array_keys($registry->getClients())); + } + + public function testAddClientWithTheCodeOfALocatorClient(): void + { + $registry = new ClientRegistry($this->createLocator(['client' => 'client'])); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessageMatches('/^Client client is already defined$/'); + $this->collectDeprecations(fn () => $registry->addClient($this->createClient('client'), 'app.client_duplicate')); + } + public function testDuplicateCodeGivesTheServiceIds(): void { $registry = new ClientRegistry(); - $registry->addClient($this->createClient('client'), 'app.client'); + $this->collectDeprecations(fn () => $registry->addClient($this->createClient('client'), 'app.client')); $this->expectException(\UnexpectedValueException::class); $this->expectExceptionMessage('Client client is already defined by service "app.client", cannot register service "app.client_duplicate"'); - $registry->addClient($this->createClient('client'), 'app.client_duplicate'); + $this->collectDeprecations(fn () => $registry->addClient($this->createClient('client'), 'app.client_duplicate')); } public function testDuplicateCodeWithoutServiceIds(): void { $registry = new ClientRegistry(); - $registry->addClient($this->createClient('client')); + $this->collectDeprecations(fn () => $registry->addClient($this->createClient('client'))); $this->expectException(\UnexpectedValueException::class); $this->expectExceptionMessageMatches('/^Client client is already defined$/'); - $registry->addClient($this->createClient('client'), 'app.client_duplicate'); + $this->collectDeprecations(fn () => $registry->addClient($this->createClient('client'), 'app.client_duplicate')); + } + + /** + * @param array $codes Code returned by each client, indexed by the code of the locator + * + * @return ServiceLocator + */ + private function createLocator(array $codes): ServiceLocator + { + $factories = []; + foreach ($codes as $code => $clientCode) { + $factories[$code] = function () use ($code, $clientCode): ClientInterface { + $this->instantiated[] = $code; + + return $this->createClient($clientCode); + }; + } + + return new ServiceLocator($factories); } private function createClient(string $code): ClientInterface @@ -72,4 +146,24 @@ private function createClient(string $code): ClientInterface return $client; } + + /** + * @return list + */ + private function collectDeprecations(callable $callable): array + { + $deprecations = []; + set_error_handler(static function (int $errno, string $errstr) use (&$deprecations): bool { + $deprecations[] = $errstr; + + return true; + }, \E_USER_DEPRECATED); + try { + $callable(); + } finally { + restore_error_handler(); + } + + return $deprecations; + } } diff --git a/tests/Task/RequestTaskTest.php b/tests/Task/RequestTaskTest.php index 74f6ad5..61651e3 100644 --- a/tests/Task/RequestTaskTest.php +++ b/tests/Task/RequestTaskTest.php @@ -27,6 +27,7 @@ use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; use Psr\Log\AbstractLogger; +use Symfony\Component\DependencyInjection\ServiceLocator; use Symfony\Component\HttpClient\Exception\TransportException; use Symfony\Component\HttpClient\MockHttpClient; use Symfony\Component\HttpClient\Response\MockResponse; @@ -211,8 +212,8 @@ public function log($level, string|\Stringable $message, array $context = []): v } }; - $registry = new ClientRegistry(); - $registry->addClient(new Client($httpClient, $logger, 'api', 'https://example.com/api')); + $client = new Client($httpClient, $logger, 'api', 'https://example.com/api'); + $registry = new ClientRegistry(new ServiceLocator(['api' => static fn (): Client => $client])); $processConfiguration = new ProcessConfiguration('test', []); $state = new ProcessState($processConfiguration, new ProcessHistory($processConfiguration));