#58 - Load the REST clients lazily from a service locator indexed by code

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.

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": "*",

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

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'"`:

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:

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:

docs/reference/client.md

@@ -2,17 +2,20 @@ 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
 ----------------
 
 * **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 <code> is already defined by service "<id>", cannot register service "<id>"`) 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 : <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 <code> is already defined by service "<id>", cannot register service "<id>"`) 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 }
 ```

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

src/Attribute/AsRestClient.php

@@ -0,0 +1,29 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the CleverAge/RestProcessBundle package.
+ *
+ * Copyright (c) Clever-Age
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace CleverAge\RestProcessBundle\Attribute;
+
+/**
+ * Registers a REST client service under its code, so that it is only instantiated when a process uses it.
+ *
+ * Requires autoconfiguration; equivalent to a `cleverage.rest.client` tag with a `code` attribute. The code must be
+ * the one returned by `ClientInterface::getCode()`.
+ */
+#[\Attribute(\Attribute::TARGET_CLASS)]
+final readonly class AsRestClient
+{
+    public function __construct(
+        public string $code,
+    ) {
+    }
+}

src/DependencyInjection/CleverAgeRestProcessExtension.php

@@ -13,7 +13,9 @@
 
 namespace CleverAge\RestProcessBundle\DependencyInjection;
 
+use CleverAge\RestProcessBundle\Attribute\AsRestClient;
 use Symfony\Component\Config\FileLocator;
+use Symfony\Component\DependencyInjection\ChildDefinition;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Extension\Extension;
 use Symfony\Component\DependencyInjection\Loader\YamlFileLoader;
@@ -24,6 +26,13 @@ class CleverAgeRestProcessExtension extends Extension
     public function load(array $configs, ContainerBuilder $container): void
     {
         $this->findServices($container, __DIR__.'/../../config/services');
+
+        $container->registerAttributeForAutoconfiguration(
+            AsRestClient::class,
+            static function (ChildDefinition $definition, AsRestClient $attribute): void {
+                $definition->addTag('cleverage.rest.client', ['code' => $attribute->code]);
+            }
+        );
     }
 
     /**

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));
     }
 }

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<string, ClientInterface>
+     */
     private array $clients = [];
 
-    /** @var array<string, string|null> Service ids of the clients, indexed by code */
+    /** @var array<string, string|null> Service ids of the clients registered with addClient(), indexed by code */
     private array $serviceIds = [];
 
+    /**
+     * @param ServiceProviderInterface<mixed>|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<string, ClientInterface>
      */
     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);
     }
 }