From 61ee3d32e2eb5dec2fe413da874356f02eb1fd31 Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Wed, 14 Oct 2026 18:26:25 +0200 Subject: [PATCH 1/3] #276 - TransformerRegistry: lazy-load transformers through a service locator indexed by code - New "code" attribute of the cleverage.transformer tag, and new #[AsTransformer(code: '...')] attribute (autoconfiguration) adding it. - New internal TransformerLocatorCompilerPass (replaces RegistryCompilerPass, deprecated): the transformers with a code go in a service locator given to TransformerRegistry (new optional constructor argument), and are only instantiated when used; duplicate / invalid codes fail the compilation. Without code, the transformer is still added eagerly with addTransformer(), deprecated since 5.4 (code required in v6.0, #280). - TransformerRegistry checks that the code of the locator is the one returned by getCode(), memoizes the fetched transformers; getTransformers() still returns every transformer. - The 37 bundle transformers and the generic transformers are tagged with their code. - Tests no longer use the deprecated addTransformer(); documentation updated. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 1 + config/services/registry.yaml | 2 + config/services/transformer.yaml | 75 +++++++++- docs/01-quick_start.md | 10 +- docs/03-custom_tasks.md | 26 +++- docs/cookbooks/01-common_setup.md | 14 +- .../transformers/generic_transformer.md | 7 +- src/Attribute/AsTransformer.php | 29 ++++ src/CleverAgeProcessBundle.php | 7 +- .../CleverAgeProcessExtension.php | 11 +- .../Compiler/RegistryCompilerPass.php | 4 + .../TransformerLocatorCompilerPass.php | 66 +++++++++ src/Registry/TransformerRegistry.php | 58 +++++++- .../CleverAgeProcessExtensionTest.php | 44 +++++- .../Compiler/RegistryCompilerPassTest.php | 17 +++ .../TransformerLocatorCompilerPassTest.php | 92 ++++++++++++ tests/Fixtures/AttributedTransformer.php | 31 ++++ tests/Registry/TransformerRegistryTest.php | 138 ++++++++++++++++++ tests/Task/TransformerTaskTest.php | 15 +- .../Array/ArrayMapTransformerTest.php | 6 +- tests/Transformer/CachedTransformerTest.php | 6 +- tests/Transformer/GenericTransformerTest.php | 11 +- tests/Transformer/MappingTransformerTest.php | 6 +- tests/Transformer/RulesTransformerTest.php | 6 +- tests/Transformer/TransformerTraitTest.php | 13 +- 25 files changed, 641 insertions(+), 54 deletions(-) create mode 100644 src/Attribute/AsTransformer.php create mode 100644 src/DependencyInjection/Compiler/TransformerLocatorCompilerPass.php create mode 100644 tests/DependencyInjection/Compiler/TransformerLocatorCompilerPassTest.php create mode 100644 tests/Fixtures/AttributedTransformer.php create mode 100644 tests/Registry/TransformerRegistryTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index e79dc14e..5185636a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ Latest ------ ## Changes +* [#276](https://github.com/cleverage/process-bundle/issues/276) TransformerRegistry: the transformers are loaded lazily from a service locator indexed by code (new internal `TransformerLocatorCompilerPass`, new optional `TransformerRegistry` constructor argument): a transformer is only instantiated when a process uses it. The code is given by the new `code` attribute of the `cleverage.transformer` tag, or by the new `#[AsTransformer(code: '...')]` attribute (autoconfiguration); it must be the one returned by `getCode()`. All the bundle transformers and the generic transformers have their code. Duplicate codes fail the container compilation. A transformer tagged without `code` is still registered eagerly, with `TransformerRegistry::addTransformer()`, which is deprecated (the `code` will be required in v6.0, #280). `RegistryCompilerPass` is no longer used and deprecated. Update documentation, add tests. * [#149](https://github.com/cleverage/process-bundle/issues/149) Apply Symfony best practices to the tasks and transformers services: they are defined explicitly (no more autowiring, autoconfiguration nor bindings in the bundle), with `cleverage_process.task.*` / `cleverage_process.transformer.*` ids (private) and a public alias named after their class, so the `service: '@CleverAge\ProcessBundle\Task\...'` references, the autowiring of the transformers by class and `$container->get()` keep working. Same arguments, tags and `shared` settings as before (generated from the previously autowired definitions). Update documentation. * [#275](https://github.com/cleverage/process-bundle/issues/275) The tasks are fetched from a service locator built at compile time from the services referenced by the process configurations (new `TaskRegistry`, `cleverage_process.registry.task`, and internal `TaskLocatorCompilerPass`) instead of the whole service container: a task service can be private. `ProcessManager` and `ProcessHelpCommand` get the registry as a new optional last constructor argument; not passing it is deprecated (the tasks are then fetched from the container), and fetching a task service not referenced by the process configurations known at compile time falls back on the container with a deprecation. Both fallbacks will be removed in v6.0 (#114). Update documentation, add tests. * [#278](https://github.com/cleverage/process-bundle/issues/278) The container no longer fails to compile when the Symfony serializer or validator is not enabled: the tasks and transformers depending on them (`NormalizerTask`, `DenormalizerTask`, `SerializerTask`, `DeserializerTask`, `normalize` and `denormalize` transformers, `ValidatorTask`) are replaced by services failing with an explicit message when they are used (new internal `UnavailableServicesCompilerPass`). `CheckSerializerCompilerPass` is no longer used and deprecated. Update documentation, add tests. diff --git a/config/services/registry.yaml b/config/services/registry.yaml index de7e140d..0db42234 100644 --- a/config/services/registry.yaml +++ b/config/services/registry.yaml @@ -12,6 +12,8 @@ services: cleverage_process.registry.transformer: class: CleverAge\ProcessBundle\Registry\TransformerRegistry public: false + arguments: + - ~ # Service locator of the transformers by code, see TransformerLocatorCompilerPass CleverAge\ProcessBundle\Registry\TransformerRegistry: alias: cleverage_process.registry.transformer diff --git a/config/services/transformer.yaml b/config/services/transformer.yaml index aafa4cf0..f94e7148 100644 --- a/config/services/transformer.yaml +++ b/config/services/transformer.yaml @@ -2,17 +2,20 @@ services: _defaults: public: false tags: - - { name: cleverage.transformer } - { name: monolog.logger, channel: cleverage_process_transformer } cleverage_process.transformer.array_element: class: CleverAge\ProcessBundle\Transformer\Array\ArrayElementTransformer + tags: + - { name: cleverage.transformer, code: array_element } CleverAge\ProcessBundle\Transformer\Array\ArrayElementTransformer: alias: cleverage_process.transformer.array_element public: true cleverage_process.transformer.array_filter: class: CleverAge\ProcessBundle\Transformer\Array\ArrayFilterTransformer + tags: + - { name: cleverage.transformer, code: array_filter } arguments: - '@property_accessor' CleverAge\ProcessBundle\Transformer\Array\ArrayFilterTransformer: @@ -21,18 +24,24 @@ services: cleverage_process.transformer.array_first: class: CleverAge\ProcessBundle\Transformer\Array\ArrayFirstTransformer + tags: + - { name: cleverage.transformer, code: array_first } CleverAge\ProcessBundle\Transformer\Array\ArrayFirstTransformer: alias: cleverage_process.transformer.array_first public: true cleverage_process.transformer.array_last: class: CleverAge\ProcessBundle\Transformer\Array\ArrayLastTransformer + tags: + - { name: cleverage.transformer, code: array_last } CleverAge\ProcessBundle\Transformer\Array\ArrayLastTransformer: alias: cleverage_process.transformer.array_last public: true cleverage_process.transformer.array_map: class: CleverAge\ProcessBundle\Transformer\Array\ArrayMapTransformer + tags: + - { name: cleverage.transformer, code: array_map } arguments: - '@cleverage_process.registry.transformer' CleverAge\ProcessBundle\Transformer\Array\ArrayMapTransformer: @@ -41,12 +50,16 @@ services: cleverage_process.transformer.array_unset: class: CleverAge\ProcessBundle\Transformer\Array\ArrayUnsetTransformer + tags: + - { name: cleverage.transformer, code: array_unset } CleverAge\ProcessBundle\Transformer\Array\ArrayUnsetTransformer: alias: cleverage_process.transformer.array_unset public: true cleverage_process.transformer.cached: class: CleverAge\ProcessBundle\Transformer\CachedTransformer + tags: + - { name: cleverage.transformer, code: cached } arguments: - '@cleverage_process.registry.transformer' - '@cache.app' @@ -57,60 +70,80 @@ services: cleverage_process.transformer.callback: class: CleverAge\ProcessBundle\Transformer\CallbackTransformer + tags: + - { name: cleverage.transformer, code: callback } CleverAge\ProcessBundle\Transformer\CallbackTransformer: alias: cleverage_process.transformer.callback public: true cleverage_process.transformer.cast: class: CleverAge\ProcessBundle\Transformer\CastTransformer + tags: + - { name: cleverage.transformer, code: cast } CleverAge\ProcessBundle\Transformer\CastTransformer: alias: cleverage_process.transformer.cast public: true cleverage_process.transformer.constant: class: CleverAge\ProcessBundle\Transformer\ConstantTransformer + tags: + - { name: cleverage.transformer, code: constant } CleverAge\ProcessBundle\Transformer\ConstantTransformer: alias: cleverage_process.transformer.constant public: true cleverage_process.transformer.convert_value: class: CleverAge\ProcessBundle\Transformer\ConvertValueTransformer + tags: + - { name: cleverage.transformer, code: convert_value } CleverAge\ProcessBundle\Transformer\ConvertValueTransformer: alias: cleverage_process.transformer.convert_value public: true cleverage_process.transformer.date_format: class: CleverAge\ProcessBundle\Transformer\Date\DateFormatTransformer + tags: + - { name: cleverage.transformer, code: date_format } CleverAge\ProcessBundle\Transformer\Date\DateFormatTransformer: alias: cleverage_process.transformer.date_format public: true cleverage_process.transformer.date_parser: class: CleverAge\ProcessBundle\Transformer\Date\DateParserTransformer + tags: + - { name: cleverage.transformer, code: date_parser } CleverAge\ProcessBundle\Transformer\Date\DateParserTransformer: alias: cleverage_process.transformer.date_parser public: true cleverage_process.transformer.debug: class: CleverAge\ProcessBundle\Transformer\DebugTransformer + tags: + - { name: cleverage.transformer, code: dump } CleverAge\ProcessBundle\Transformer\DebugTransformer: alias: cleverage_process.transformer.debug public: true cleverage_process.transformer.default: class: CleverAge\ProcessBundle\Transformer\DefaultTransformer + tags: + - { name: cleverage.transformer, code: default } CleverAge\ProcessBundle\Transformer\DefaultTransformer: alias: cleverage_process.transformer.default public: true cleverage_process.transformer.evaluator: class: CleverAge\ProcessBundle\Transformer\EvaluatorTransformer + tags: + - { name: cleverage.transformer, code: evaluator } CleverAge\ProcessBundle\Transformer\EvaluatorTransformer: alias: cleverage_process.transformer.evaluator public: true cleverage_process.transformer.expression_language_map: class: CleverAge\ProcessBundle\Transformer\ExpressionLanguageMapTransformer + tags: + - { name: cleverage.transformer, code: expression_language_map } arguments: - '@cleverage_process.expression_language' CleverAge\ProcessBundle\Transformer\ExpressionLanguageMapTransformer: @@ -119,6 +152,8 @@ services: cleverage_process.transformer.mapping: class: CleverAge\ProcessBundle\Transformer\MappingTransformer + tags: + - { name: cleverage.transformer, code: mapping } arguments: - '@cleverage_process.registry.transformer' - '@logger' @@ -129,18 +164,24 @@ services: cleverage_process.transformer.multi_replace: class: CleverAge\ProcessBundle\Transformer\MultiReplaceTransformer + tags: + - { name: cleverage.transformer, code: multi_replace } CleverAge\ProcessBundle\Transformer\MultiReplaceTransformer: alias: cleverage_process.transformer.multi_replace public: true cleverage_process.transformer.instantiate: class: CleverAge\ProcessBundle\Transformer\Object\InstantiateTransformer + tags: + - { name: cleverage.transformer, code: instantiate } CleverAge\ProcessBundle\Transformer\Object\InstantiateTransformer: alias: cleverage_process.transformer.instantiate public: true cleverage_process.transformer.property_accessor: class: CleverAge\ProcessBundle\Transformer\Object\PropertyAccessorTransformer + tags: + - { name: cleverage.transformer, code: property_accessor } arguments: - '@property_accessor' CleverAge\ProcessBundle\Transformer\Object\PropertyAccessorTransformer: @@ -149,6 +190,8 @@ services: cleverage_process.transformer.recursive_property_setter: class: CleverAge\ProcessBundle\Transformer\Object\RecursivePropertySetterTransformer + tags: + - { name: cleverage.transformer, code: recursive_property_setter } arguments: - '@property_accessor' CleverAge\ProcessBundle\Transformer\Object\RecursivePropertySetterTransformer: @@ -157,12 +200,16 @@ services: cleverage_process.transformer.preg_filter: class: CleverAge\ProcessBundle\Transformer\PregFilterTransformer + tags: + - { name: cleverage.transformer, code: preg_filter } CleverAge\ProcessBundle\Transformer\PregFilterTransformer: alias: cleverage_process.transformer.preg_filter public: true cleverage_process.transformer.rules: class: CleverAge\ProcessBundle\Transformer\RulesTransformer + tags: + - { name: cleverage.transformer, code: rules } arguments: - '@cleverage_process.registry.transformer' - '@cleverage_process.expression_language' @@ -172,6 +219,8 @@ services: cleverage_process.transformer.denormalize: class: CleverAge\ProcessBundle\Transformer\Serialization\DenormalizeTransformer + tags: + - { name: cleverage.transformer, code: denormalize } arguments: - '@serializer' CleverAge\ProcessBundle\Transformer\Serialization\DenormalizeTransformer: @@ -180,6 +229,8 @@ services: cleverage_process.transformer.normalize: class: CleverAge\ProcessBundle\Transformer\Serialization\NormalizeTransformer + tags: + - { name: cleverage.transformer, code: normalize } arguments: - '@serializer' CleverAge\ProcessBundle\Transformer\Serialization\NormalizeTransformer: @@ -188,54 +239,72 @@ services: cleverage_process.transformer.explode: class: CleverAge\ProcessBundle\Transformer\String\ExplodeTransformer + tags: + - { name: cleverage.transformer, code: explode } CleverAge\ProcessBundle\Transformer\String\ExplodeTransformer: alias: cleverage_process.transformer.explode public: true cleverage_process.transformer.hash: class: CleverAge\ProcessBundle\Transformer\String\HashTransformer + tags: + - { name: cleverage.transformer, code: hash } CleverAge\ProcessBundle\Transformer\String\HashTransformer: alias: cleverage_process.transformer.hash public: true cleverage_process.transformer.implode: class: CleverAge\ProcessBundle\Transformer\String\ImplodeTransformer + tags: + - { name: cleverage.transformer, code: implode } CleverAge\ProcessBundle\Transformer\String\ImplodeTransformer: alias: cleverage_process.transformer.implode public: true cleverage_process.transformer.preg_match: class: CleverAge\ProcessBundle\Transformer\String\PregMatchTransformer + tags: + - { name: cleverage.transformer, code: preg_match } CleverAge\ProcessBundle\Transformer\String\PregMatchTransformer: alias: cleverage_process.transformer.preg_match public: true cleverage_process.transformer.slugify: class: CleverAge\ProcessBundle\Transformer\String\SlugifyTransformer + tags: + - { name: cleverage.transformer, code: slugify } CleverAge\ProcessBundle\Transformer\String\SlugifyTransformer: alias: cleverage_process.transformer.slugify public: true cleverage_process.transformer.sprintf: class: CleverAge\ProcessBundle\Transformer\String\SprintfTransformer + tags: + - { name: cleverage.transformer, code: sprintf } CleverAge\ProcessBundle\Transformer\String\SprintfTransformer: alias: cleverage_process.transformer.sprintf public: true cleverage_process.transformer.trim: class: CleverAge\ProcessBundle\Transformer\String\TrimTransformer + tags: + - { name: cleverage.transformer, code: trim } CleverAge\ProcessBundle\Transformer\String\TrimTransformer: alias: cleverage_process.transformer.trim public: true cleverage_process.transformer.type_setter: class: CleverAge\ProcessBundle\Transformer\TypeSetterTransformer + tags: + - { name: cleverage.transformer, code: type_setter } CleverAge\ProcessBundle\Transformer\TypeSetterTransformer: alias: cleverage_process.transformer.type_setter public: true cleverage_process.transformer.unset: class: CleverAge\ProcessBundle\Transformer\UnsetTransformer + tags: + - { name: cleverage.transformer, code: unset } arguments: - '@property_accessor' CleverAge\ProcessBundle\Transformer\UnsetTransformer: @@ -244,12 +313,16 @@ services: cleverage_process.transformer.wrapper: class: CleverAge\ProcessBundle\Transformer\WrapperTransformer + tags: + - { name: cleverage.transformer, code: wrapper } CleverAge\ProcessBundle\Transformer\WrapperTransformer: alias: cleverage_process.transformer.wrapper public: true cleverage_process.transformer.xpath_evaluator: class: CleverAge\ProcessBundle\Transformer\Xml\XpathEvaluatorTransformer + tags: + - { name: cleverage.transformer, code: xpath_evaluator } CleverAge\ProcessBundle\Transformer\Xml\XpathEvaluatorTransformer: alias: cleverage_process.transformer.xpath_evaluator public: true diff --git a/docs/01-quick_start.md b/docs/01-quick_start.md index a227baba..ed0ad362 100644 --- a/docs/01-quick_start.md +++ b/docs/01-quick_start.md @@ -80,18 +80,20 @@ bundle configures them: - services implementing `CleverAge\ProcessBundle\Model\TaskInterface` are not shared (no data shared between task usages), with a `monolog.logger` tag on the `cleverage_process_task` channel; - services implementing `CleverAge\ProcessBundle\Transformer\TransformerInterface` get the `cleverage.transformer` tag - (needed by the transformer registry) and a `monolog.logger` tag on the `cleverage_process_transformer` channel. + (needed by the transformer registry) and a `monolog.logger` tag on the `cleverage_process_transformer` channel. Add + the `#[CleverAge\ProcessBundle\Attribute\AsTransformer(code: '...')]` attribute to their class, with the code returned + by `getCode()`: the transformer is then only instantiated when a process uses it (see + [transformers](03-custom_tasks.md#transformers)). Without autoconfiguration, declare these settings yourself: ```yaml # config/services.yaml services: - App\Transformer\: - resource: '../src/Transformer/*' + App\Transformer\VatTransformer: # One definition per transformer, for its code autowire: true tags: - - { name: cleverage.transformer } # Needed by the transformer registry to find transformers + - { name: cleverage.transformer, code: app_vat } # Needed by the transformer registry, code returned by getCode() - { name: monolog.logger, channel: cleverage_process_transformer } # Optional, see logging App\Task\: diff --git a/docs/03-custom_tasks.md b/docs/03-custom_tasks.md index eeaa385f..f10820e6 100644 --- a/docs/03-custom_tasks.md +++ b/docs/03-custom_tasks.md @@ -200,8 +200,10 @@ must skip the state (`ProcessState::setSkipped(true)`) instead of outputting the Transformers are another kind of service. They implement `CleverAge\ProcessBundle\Transformer\TransformerInterface` (`transform(mixed $value, array $options = []): mixed` and `getCode(): string`) or `CleverAge\ProcessBundle\Transformer\ConfigurableTransformerInterface` (which adds -`configureOptions(OptionsResolver $resolver): void`) and are declared with a `cleverage.transformer` tag (added by -autoconfiguration, with a `monolog.logger` tag on the `cleverage_process_transformer` channel). +`configureOptions(OptionsResolver $resolver): void`) and are declared with a `cleverage.transformer` tag whose `code` +attribute is the code returned by `getCode()`: with autoconfiguration, add the +`CleverAge\ProcessBundle\Attribute\AsTransformer` attribute to the class (the `cleverage.transformer` and +`monolog.logger` tags are added for you). They're meant to be lightweight, composable, and stateless pieces of your process. Feel free to implement custom ones as soon as provided ones don't fit your goal. @@ -209,9 +211,11 @@ as soon as provided ones don't fit your goal. ```php namespace App\Transformer; +use CleverAge\ProcessBundle\Attribute\AsTransformer; use CleverAge\ProcessBundle\Transformer\ConfigurableTransformerInterface; use Symfony\Component\OptionsResolver\OptionsResolver; +#[AsTransformer(code: 'app_vat')] class VatTransformer implements ConfigurableTransformerInterface { public function transform(mixed $value, array $options = []): mixed @@ -238,12 +242,24 @@ services: App\Transformer\: resource: '../src/Transformer/*' autowire: true - autoconfigure: true # or, without autoconfiguration: tags: [{ name: cleverage.transformer }] + autoconfigure: true + + # or, without autoconfiguration, one definition per transformer: + App\Transformer\VatTransformer: + autowire: true + tags: + - { name: cleverage.transformer, code: app_vat } ``` Once properly set up, they are registered in the `CleverAge\ProcessBundle\Registry\TransformerRegistry` (used by the -`TransformerTask` and every transformer using a sub-list of transformers), using the code from -`TransformerInterface::getCode`. Two transformers cannot share the same code. When creating a new transformer for your +`TransformerTask` and every transformer using a sub-list of transformers) under their code, in a service locator: a +transformer is only instantiated when a process uses it. The code of the tag must be the one returned by +`TransformerInterface::getCode()` (an exception is thrown otherwise when the transformer is used). Two transformers +cannot share the same code (the container compilation fails). + +A transformer tagged without `code` (e.g. autoconfigured without the attribute) still works, but it is instantiated with +the registry and registered with `TransformerRegistry::addTransformer()`, which is deprecated since 5.4: the `code` will +be required in v6.0. When creating a new transformer for your project you should use an internal prefix in your codes to avoid conflicts with potential standard additions. ```yaml diff --git a/docs/cookbooks/01-common_setup.md b/docs/cookbooks/01-common_setup.md index 6328e7f8..746cc3fe 100644 --- a/docs/cookbooks/01-common_setup.md +++ b/docs/cookbooks/01-common_setup.md @@ -7,7 +7,8 @@ Task & transformer declaration With autoconfiguration (`autoconfigure: true`, set in the `_defaults` of the Symfony skeleton), nothing more is needed: the bundle makes the services implementing `CleverAge\ProcessBundle\Model\TaskInterface` not shared, tags the ones implementing `CleverAge\ProcessBundle\Transformer\TransformerInterface` with `cleverage.transformer`, and binds -both to their logging channel (`monolog.logger` tags). +both to their logging channel (`monolog.logger` tags). Add the `#[AsTransformer(code: '...')]` attribute to your +transformer classes, so that they are only instantiated when used (see [transformers](../03-custom_tasks.md#transformers)). Without autoconfiguration, example of a generic declaration for all the tasks and transformers of your application: @@ -21,11 +22,11 @@ services: tags: - { name: monolog.logger, channel: cleverage_process_task } - App\Transformer\: - resource: '../src/Transformer/*' + # One definition per transformer, the code of the tag being the one returned by getCode() + App\Transformer\VatTransformer: autowire: true tags: - - { name: cleverage.transformer } + - { name: cleverage.transformer, code: app_vat } - { name: monolog.logger, channel: cleverage_process_transformer } ``` @@ -47,10 +48,13 @@ services: - { name: monolog.logger, channel: cleverage_process_task } CleverAge\ProcessBundle\Transformer\TransformerInterface: tags: - - { name: cleverage.transformer } - { name: monolog.logger, channel: cleverage_process_transformer } ``` +An `_instanceof` rule cannot give a code per transformer: declare the `cleverage.transformer` tag with its `code` on each +transformer, or use the `#[AsTransformer]` attribute with autoconfiguration. A `cleverage.transformer` tag without +`code` is deprecated since 5.4. + Configuration ------------- diff --git a/docs/reference/transformers/generic_transformer.md b/docs/reference/transformers/generic_transformer.md index a8b9ec0a..ecf2eb27 100644 --- a/docs/reference/transformers/generic_transformer.md +++ b/docs/reference/transformers/generic_transformer.md @@ -7,9 +7,10 @@ instance is created per declared generic transformer, and it simply applies a pr optionally parametrized by `contextual_options`. For each entry of `generic_transformers`, the bundle extension (`CleverAgeProcessExtension`) registers a private -service `CleverAge\ProcessBundle\Transformer\GenericTransformer\`, tagged `cleverage.transformer`, and calls -`initialize(, )` on it. The instance is then added to the transformer registry like any other -transformer, under the configured code (which must not collide with an existing transformer code). +service `CleverAge\ProcessBundle\Transformer\GenericTransformer\`, tagged `cleverage.transformer` with its code +(`code` attribute), and calls `initialize(, )` on it. It is then registered in the transformer +registry like any other transformer, under the configured code (which must not collide with an existing transformer +code: the container compilation fails), and only instantiated when used. When the generic transformer is used, its options are the declared contextual options. The `{{ option_code }}` placeholders in the preconfigured `transformers` (keys and values, recursively) are replaced by the option values, diff --git a/src/Attribute/AsTransformer.php b/src/Attribute/AsTransformer.php new file mode 100644 index 00000000..22a9d837 --- /dev/null +++ b/src/Attribute/AsTransformer.php @@ -0,0 +1,29 @@ +addCompilerPass( - new RegistryCompilerPass('cleverage_process.registry.transformer', 'cleverage.transformer', 'addTransformer') - ); - $container->addCompilerPass(new UnavailableServicesCompilerPass()); + $container->addCompilerPass(new TransformerLocatorCompilerPass()); $container->addCompilerPass(new TaskLocatorCompilerPass()); } diff --git a/src/DependencyInjection/CleverAgeProcessExtension.php b/src/DependencyInjection/CleverAgeProcessExtension.php index 9777e2c0..c51b6c31 100644 --- a/src/DependencyInjection/CleverAgeProcessExtension.php +++ b/src/DependencyInjection/CleverAgeProcessExtension.php @@ -13,11 +13,13 @@ namespace CleverAge\ProcessBundle\DependencyInjection; +use CleverAge\ProcessBundle\Attribute\AsTransformer; use CleverAge\ProcessBundle\Model\ProcessStopCheckerInterface; use CleverAge\ProcessBundle\Model\TaskInterface; use CleverAge\ProcessBundle\Transformer\GenericTransformer; use CleverAge\ProcessBundle\Transformer\TransformerInterface; use Symfony\Component\Config\FileLocator; +use Symfony\Component\DependencyInjection\ChildDefinition; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Extension\Extension; @@ -46,6 +48,13 @@ public function load(array $configs, ContainerBuilder $container): void $container->registerForAutoconfiguration(TransformerInterface::class) ->addTag('cleverage.transformer') ->addTag('monolog.logger', ['channel' => 'cleverage_process_transformer']); + // Lazy loading of the transformers by code + $container->registerAttributeForAutoconfiguration( + AsTransformer::class, + static function (ChildDefinition $definition, AsTransformer $attribute): void { + $definition->addTag('cleverage.transformer', ['code' => $attribute->code]); + } + ); $configuration = new Configuration(); $config = $this->processConfiguration($configuration, $configs); @@ -65,7 +74,7 @@ public function load(array $configs, ContainerBuilder $container): void new Reference('cleverage_process.registry.transformer'), ]); $transformerDefinition->addMethodCall('initialize', [$transformerCode, $transformerConfig]); - $transformerDefinition->addTag('cleverage.transformer'); + $transformerDefinition->addTag('cleverage.transformer', ['code' => $transformerCode]); $container->setDefinition(GenericTransformer::class.'\\'.$transformerCode, $transformerDefinition); } diff --git a/src/DependencyInjection/Compiler/RegistryCompilerPass.php b/src/DependencyInjection/Compiler/RegistryCompilerPass.php index 7896c4b5..bf684e40 100644 --- a/src/DependencyInjection/Compiler/RegistryCompilerPass.php +++ b/src/DependencyInjection/Compiler/RegistryCompilerPass.php @@ -19,6 +19,9 @@ /** * Generic compiler pass to add tagged services to a registry. + * + * @deprecated since 5.4, no longer used by the bundle (the transformers are registered by + * TransformerLocatorCompilerPass), will be removed in v6.0 */ class RegistryCompilerPass implements CompilerPassInterface { @@ -27,6 +30,7 @@ public function __construct( protected ?string $tag = null, protected ?string $method = null, ) { + trigger_deprecation('cleverage/process-bundle', '5.4', 'The "%s" class is deprecated, it will be removed in v6.0.', self::class); } /** diff --git a/src/DependencyInjection/Compiler/TransformerLocatorCompilerPass.php b/src/DependencyInjection/Compiler/TransformerLocatorCompilerPass.php new file mode 100644 index 00000000..63793dcc --- /dev/null +++ b/src/DependencyInjection/Compiler/TransformerLocatorCompilerPass.php @@ -0,0 +1,66 @@ +hasDefinition('cleverage_process.registry.transformer')) { + return; + } + $registry = $container->getDefinition('cleverage_process.registry.transformer'); + + $services = []; + $ids = []; + foreach ($container->findTaggedServiceIds('cleverage.transformer') as $id => $tags) { + $codes = []; + foreach ($tags as $attributes) { + if (!isset($attributes['code'])) { + continue; + } + if (!\is_string($attributes['code']) || '' === $attributes['code']) { + throw new InvalidArgumentException(\sprintf('The "code" attribute of the "cleverage.transformer" tag of the "%s" service must be a non-empty string.', $id)); + } + $codes[] = $attributes['code']; + } + if ([] === $codes) { + // Deprecated: instantiated with the registry, addTransformer() triggers the deprecation + $registry->addMethodCall('addTransformer', [new Reference($id)]); + continue; + } + foreach (array_unique($codes) as $code) { + if (isset($ids[$code])) { + throw new InvalidArgumentException(\sprintf('The transformer code "%s" is used by the "%s" and "%s" services.', $code, $ids[$code], $id)); + } + $ids[$code] = $id; + $services[$code] = new Reference($id); + } + } + + $registry->setArgument(0, ServiceLocatorTagPass::register($container, $services)); + } +} diff --git a/src/Registry/TransformerRegistry.php b/src/Registry/TransformerRegistry.php index 9fd4fbff..8d11f5d0 100644 --- a/src/Registry/TransformerRegistry.php +++ b/src/Registry/TransformerRegistry.php @@ -15,44 +15,88 @@ use CleverAge\ProcessBundle\Exception\MissingTransformerException; use CleverAge\ProcessBundle\Transformer\TransformerInterface; +use Symfony\Contracts\Service\ServiceProviderInterface; /** - * Holds all tagged transformer services. + * Holds the transformers, indexed by code. + * + * The transformer services are fetched lazily from a service locator indexed by code (`code` attribute of the + * `cleverage.transformer` tag, or `#[AsTransformer]`, see TransformerLocatorCompilerPass): a transformer is only + * instantiated when it is used. */ class TransformerRegistry { /** - * @var TransformerInterface[] + * Transformers already fetched from the locator, and the ones registered with addTransformer() (deprecated). + * + * @var array */ protected array $transformers = []; + /** + * @param ServiceProviderInterface|null $locator + */ + public function __construct( + protected ?ServiceProviderInterface $locator = null, + ) { + } + + /** + * @deprecated since 5.4, tag the transformer service with a "code" attribute or use #[AsTransformer] instead + */ public function addTransformer(TransformerInterface $transformer): void { - if (\array_key_exists($transformer->getCode(), $this->transformers)) { + trigger_deprecation( + 'cleverage/process-bundle', + '5.4', + 'Registering the transformer "%s" (%s) with "%s()" is deprecated, tag its service with a "code" attribute ({ name: cleverage.transformer, code: %s }) or use the #[AsTransformer] attribute instead, so that it is only instantiated when used. It will be required in v6.0.', + $transformer->getCode(), + $transformer::class, + __METHOD__, + $transformer->getCode(), + ); + if ($this->hasTransformer($transformer->getCode())) { throw new \UnexpectedValueException("Transformer {$transformer->getCode()} is already defined"); } $this->transformers[$transformer->getCode()] = $transformer; } /** - * @return TransformerInterface[] + * Instantiates every transformer of the locator. + * + * @return array */ public function getTransformers(): array { + foreach (array_keys($this->locator?->getProvidedServices() ?? []) as $code) { + $this->getTransformer((string) $code); + } + return $this->transformers; } public function getTransformer(string $code): TransformerInterface { - if (!$this->hasTransformer($code)) { + if (isset($this->transformers[$code])) { + return $this->transformers[$code]; + } + if (!$this->locator?->has($code)) { throw MissingTransformerException::create($code); } - return $this->transformers[$code]; + $transformer = $this->locator->get($code); + if (!$transformer instanceof TransformerInterface) { + throw new \UnexpectedValueException(\sprintf('The service of the transformer "%s" must implement "%s".', $code, TransformerInterface::class)); + } + if ($code !== $transformer->getCode()) { + throw new \UnexpectedValueException(\sprintf('The transformer "%s" is registered with the code "%s" but its getCode() method returns "%s": both must be the same.', $transformer::class, $code, $transformer->getCode())); + } + + return $this->transformers[$code] = $transformer; } public function hasTransformer(string $code): bool { - return \array_key_exists($code, $this->transformers); + return isset($this->transformers[$code]) || ($this->locator?->has($code) ?? false); } } diff --git a/tests/DependencyInjection/CleverAgeProcessExtensionTest.php b/tests/DependencyInjection/CleverAgeProcessExtensionTest.php index 3194aae2..c2b7b94e 100644 --- a/tests/DependencyInjection/CleverAgeProcessExtensionTest.php +++ b/tests/DependencyInjection/CleverAgeProcessExtensionTest.php @@ -21,6 +21,8 @@ use CleverAge\ProcessBundle\Registry\ProcessConfigurationRegistry; use CleverAge\ProcessBundle\Registry\TransformerRegistry; use CleverAge\ProcessBundle\Task\DummyTask; +use CleverAge\ProcessBundle\Tests\Fixtures\AttributedTransformer; +use CleverAge\ProcessBundle\Transformer\GenericTransformer; use CleverAge\ProcessBundle\Transformer\String\TrimTransformer; use CleverAge\ProcessBundle\Transformer\TransformerInterface; use PHPUnit\Framework\Attributes\CoversClass; @@ -28,6 +30,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument; use Symfony\Component\DependencyInjection\ChildDefinition; +use Symfony\Component\DependencyInjection\Compiler\AttributeAutoconfigurationPass; use Symfony\Component\DependencyInjection\Compiler\ResolveChildDefinitionsPass; use Symfony\Component\DependencyInjection\Compiler\ResolveInstanceofConditionalsPass; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -116,7 +119,46 @@ public function testTasksAndTransformersAreAutoconfigured(): void self::assertSame([['channel' => 'cleverage_process_transformer']], $transformer->getTag('monolog.logger'), $id); } // The bundle transformers (explicitly defined and tagged, not autoconfigured) are tagged once - self::assertSame([[]], $container->findDefinition(TrimTransformer::class)->getTag('cleverage.transformer')); + self::assertSame([['code' => 'trim']], $container->findDefinition(TrimTransformer::class)->getTag('cleverage.transformer')); + } + + public function testEveryTransformerIsRegisteredWithItsCode(): void + { + $container = new ContainerBuilder(); + (new CleverAgeProcessExtension())->load([[ + 'generic_transformers' => ['app_generic' => ['transformers' => ['trim' => []]]], + ]], $container); + + $tagged = $container->findTaggedServiceIds('cleverage.transformer'); + self::assertCount(38, $tagged); + foreach ($tagged as $id => $tags) { + $class = $container->getDefinition($id)->getClass(); + self::assertIsString($class); + if (GenericTransformer::class === $class) { + $code = 'app_generic'; + } else { + self::assertTrue(is_a($class, TransformerInterface::class, true), $class); + $transformer = (new \ReflectionClass($class))->newInstanceWithoutConstructor(); + self::assertInstanceOf(TransformerInterface::class, $transformer); + $code = $transformer->getCode(); + } + // Lazy loading: the code of the tag must be the one returned by getCode() + self::assertSame([['code' => $code]], $tags, $id); + } + } + + public function testAsTransformerAttributeRegistersTheCode(): void + { + $container = new ContainerBuilder(); + (new CleverAgeProcessExtension())->load([], $container); + $container->setDefinition('app.attributed', (new Definition(AttributedTransformer::class))->setAutoconfigured(true)); + + (new AttributeAutoconfigurationPass())->process($container); + (new ResolveInstanceofConditionalsPass())->process($container); + (new ResolveChildDefinitionsPass())->process($container); + + $tags = $container->getDefinition('app.attributed')->getTag('cleverage.transformer'); + self::assertContains(['code' => 'app_attributed'], $tags); } public function testStopCheckersAreInjectedInTheProcessManager(): void diff --git a/tests/DependencyInjection/Compiler/RegistryCompilerPassTest.php b/tests/DependencyInjection/Compiler/RegistryCompilerPassTest.php index 9538c49d..de3314dd 100644 --- a/tests/DependencyInjection/Compiler/RegistryCompilerPassTest.php +++ b/tests/DependencyInjection/Compiler/RegistryCompilerPassTest.php @@ -21,6 +21,23 @@ #[\PHPUnit\Framework\Attributes\CoversClass(RegistryCompilerPass::class)] class RegistryCompilerPassTest extends TestCase { + public function testIsDeprecated(): void + { + $deprecations = []; + set_error_handler(static function (int $errno, string $errstr) use (&$deprecations): bool { + $deprecations[] = $errstr; + + return true; + }, \E_USER_DEPRECATED); + try { + new RegistryCompilerPass('registry', 'tag', 'append'); + } finally { + restore_error_handler(); + } + + self::assertSame(['Since cleverage/process-bundle 5.4: The "'.RegistryCompilerPass::class.'" class is deprecated, it will be removed in v6.0.'], $deprecations); + } + public function testTaggedServicesAreAddedToTheRegistry(): void { $container = new ContainerBuilder(); diff --git a/tests/DependencyInjection/Compiler/TransformerLocatorCompilerPassTest.php b/tests/DependencyInjection/Compiler/TransformerLocatorCompilerPassTest.php new file mode 100644 index 00000000..bc4563bd --- /dev/null +++ b/tests/DependencyInjection/Compiler/TransformerLocatorCompilerPassTest.php @@ -0,0 +1,92 @@ +createContainer(); + $container->register('app.coded', \stdClass::class)->addTag('cleverage.transformer', ['code' => 'coded']); + // Autoconfiguration on TransformerInterface adds a tag without code next to the one of #[AsTransformer] + $container->register('app.attributed', \stdClass::class) + ->addTag('cleverage.transformer') + ->addTag('cleverage.transformer', ['code' => 'attributed']); + $container->register('app.legacy', \stdClass::class)->addTag('cleverage.transformer'); + + (new TransformerLocatorCompilerPass())->process($container); + + $registry = $container->getDefinition('cleverage_process.registry.transformer'); + $locatorReference = $registry->getArgument(0); + self::assertInstanceOf(Reference::class, $locatorReference); + $services = $container->getDefinition((string) $locatorReference)->getArgument(0); + self::assertIsArray($services); + self::assertSame(['coded', 'attributed'], array_keys($services)); + self::assertInstanceOf(ServiceClosureArgument::class, $services['coded']); + self::assertSame('app.coded', (string) $services['coded']->getValues()[0]); + // Deprecated: eager registration + self::assertEquals([['addTransformer', [new Reference('app.legacy')]]], $registry->getMethodCalls()); + } + + public function testDuplicateCodeThrows(): void + { + $container = $this->createContainer(); + $container->register('app.first', \stdClass::class)->addTag('cleverage.transformer', ['code' => 'same']); + $container->register('app.second', \stdClass::class)->addTag('cleverage.transformer', ['code' => 'same']); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('The transformer code "same" is used by the "app.first" and "app.second" services.'); + + (new TransformerLocatorCompilerPass())->process($container); + } + + public function testInvalidCodeThrows(): void + { + $container = $this->createContainer(); + $container->register('app.invalid', \stdClass::class)->addTag('cleverage.transformer', ['code' => '']); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('The "code" attribute of the "cleverage.transformer" tag of the "app.invalid" service must be a non-empty string.'); + + (new TransformerLocatorCompilerPass())->process($container); + } + + public function testNothingIsDoneWithoutTheRegistry(): void + { + $container = new ContainerBuilder(); + $container->register('app.coded', \stdClass::class)->addTag('cleverage.transformer', ['code' => 'coded']); + + (new TransformerLocatorCompilerPass())->process($container); + + self::assertFalse($container->hasDefinition('cleverage_process.registry.transformer')); + } + + private function createContainer(): ContainerBuilder + { + $container = new ContainerBuilder(); + $container->register('cleverage_process.registry.transformer', TransformerRegistry::class)->setArguments([null]); + + return $container; + } +} diff --git a/tests/Fixtures/AttributedTransformer.php b/tests/Fixtures/AttributedTransformer.php new file mode 100644 index 00000000..bacb583e --- /dev/null +++ b/tests/Fixtures/AttributedTransformer.php @@ -0,0 +1,31 @@ +createRegistry(); + + self::assertTrue($registry->hasTransformer('trim')); + self::assertFalse($registry->hasTransformer('unknown')); + self::assertSame(0, $this->instantiations); + + $transformer = $registry->getTransformer('trim'); + + self::assertInstanceOf(TrimTransformer::class, $transformer); + self::assertSame($transformer, $registry->getTransformer('trim')); + self::assertSame(1, $this->instantiations); + } + + public function testMissingTransformerThrows(): void + { + $this->expectException(MissingTransformerException::class); + + $this->createRegistry()->getTransformer('unknown'); + } + + public function testCodeMustMatchGetCode(): void + { + $registry = new TransformerRegistry(new ServiceLocator(['other' => static fn (): TrimTransformer => new TrimTransformer()])); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('The transformer "'.TrimTransformer::class.'" is registered with the code "other" but its getCode() method returns "trim": both must be the same.'); + + $registry->getTransformer('other'); + } + + public function testServiceMustBeATransformer(): void + { + $registry = new TransformerRegistry(new ServiceLocator(['trim' => static fn (): \stdClass => new \stdClass()])); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('The service of the transformer "trim" must implement'); + + $registry->getTransformer('trim'); + } + + public function testGetTransformersReturnsEveryTransformer(): void + { + $registry = $this->createRegistry(); + $this->collectDeprecations(static fn () => $registry->addTransformer(new CastTransformer())); + + $transformers = $registry->getTransformers(); + ksort($transformers); + self::assertSame(['cast', 'trim'], array_keys($transformers)); + } + + public function testAddTransformerIsDeprecated(): void + { + $registry = new TransformerRegistry(); + + $deprecations = $this->collectDeprecations(static fn () => $registry->addTransformer(new CastTransformer())); + + self::assertInstanceOf(CastTransformer::class, $registry->getTransformer('cast')); + self::assertSame( + ['Since cleverage/process-bundle 5.4: Registering the transformer "cast" ('.CastTransformer::class.') with "'.TransformerRegistry::class.'::addTransformer()" is deprecated, tag its service with a "code" attribute ({ name: cleverage.transformer, code: cast }) or use the #[AsTransformer] attribute instead, so that it is only instantiated when used. It will be required in v6.0.'], + $deprecations + ); + } + + public function testAddTransformerWithACodeOfTheLocatorThrows(): void + { + $registry = $this->createRegistry(); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('Transformer trim is already defined'); + + $this->collectDeprecations(static fn () => $registry->addTransformer(new TrimTransformer())); + } + + private function createRegistry(): TransformerRegistry + { + return new TransformerRegistry(new ServiceLocator([ + 'trim' => function (): TrimTransformer { + ++$this->instantiations; + + return new TrimTransformer(); + }, + ])); + } + + /** + * @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/TransformerTaskTest.php b/tests/Task/TransformerTaskTest.php index d7268691..eed4ebbb 100644 --- a/tests/Task/TransformerTaskTest.php +++ b/tests/Task/TransformerTaskTest.php @@ -30,6 +30,7 @@ use CleverAge\ProcessBundle\Transformer\TransformerTrait; use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; +use Symfony\Component\DependencyInjection\ServiceLocator; use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException; use Symfony\Component\OptionsResolver\Exception\MissingOptionsException; use Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException; @@ -220,11 +221,15 @@ private function execute(array $options, mixed $input): ProcessState $state->setTaskConfiguration(new TaskConfiguration('transform', TransformerTask::class, $options)); $state->setInput($input); - $registry = new TransformerRegistry(); - $registry->addTransformer(new ArrayLastTransformer()); - $registry->addTransformer(new CallbackTransformer()); - $registry->addTransformer(new TrimTransformer()); - $registry->addTransformer(new MappingTransformer($registry, new NullLogger(), PropertyAccess::createPropertyAccessor())); + $registry = new TransformerRegistry(new ServiceLocator([ + 'array_last' => static fn (): ArrayLastTransformer => new ArrayLastTransformer(), + 'callback' => static fn (): CallbackTransformer => new CallbackTransformer(), + 'trim' => static fn (): TrimTransformer => new TrimTransformer(), + // The mapping transformer uses the registry itself + 'mapping' => static function () use (&$registry): MappingTransformer { + return new MappingTransformer($registry, new NullLogger(), PropertyAccess::createPropertyAccessor()); + }, + ])); $task = new TransformerTask(new NullLogger(), $registry); $task->initialize($state); diff --git a/tests/Transformer/Array/ArrayMapTransformerTest.php b/tests/Transformer/Array/ArrayMapTransformerTest.php index b60a5b81..6426b5f5 100644 --- a/tests/Transformer/Array/ArrayMapTransformerTest.php +++ b/tests/Transformer/Array/ArrayMapTransformerTest.php @@ -19,6 +19,7 @@ use CleverAge\ProcessBundle\Transformer\CallbackTransformer; use CleverAge\ProcessBundle\Transformer\TransformerTrait; use PHPUnit\Framework\TestCase; +use Symfony\Component\DependencyInjection\ServiceLocator; use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException; use Symfony\Component\OptionsResolver\OptionsResolver; @@ -136,8 +137,9 @@ public static function failOnBar(string $value): string private function createTransformer(): ArrayMapTransformer { - $registry = new TransformerRegistry(); - $registry->addTransformer(new CallbackTransformer()); + $registry = new TransformerRegistry(new ServiceLocator([ + 'callback' => static fn (): CallbackTransformer => new CallbackTransformer(), + ])); return new ArrayMapTransformer($registry); } diff --git a/tests/Transformer/CachedTransformerTest.php b/tests/Transformer/CachedTransformerTest.php index da79df88..5d1a6663 100644 --- a/tests/Transformer/CachedTransformerTest.php +++ b/tests/Transformer/CachedTransformerTest.php @@ -19,6 +19,7 @@ use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; use Symfony\Component\Cache\Adapter\ArrayAdapter; +use Symfony\Component\DependencyInjection\ServiceLocator; use Symfony\Component\OptionsResolver\OptionsResolver; #[\PHPUnit\Framework\Attributes\CoversClass(CachedTransformer::class)] @@ -36,8 +37,9 @@ class CachedTransformerTest extends TestCase protected function setUp(): void { - $registry = new TransformerRegistry(); - $registry->addTransformer(new CastTransformer()); + $registry = new TransformerRegistry(new ServiceLocator([ + 'cast' => static fn (): CastTransformer => new CastTransformer(), + ])); $this->cache = new ArrayAdapter(); $this->transformer = new CachedTransformer($registry, $this->cache, new NullLogger()); } diff --git a/tests/Transformer/GenericTransformerTest.php b/tests/Transformer/GenericTransformerTest.php index 829a6aa3..7a2041df 100644 --- a/tests/Transformer/GenericTransformerTest.php +++ b/tests/Transformer/GenericTransformerTest.php @@ -18,6 +18,7 @@ use CleverAge\ProcessBundle\Transformer\CallbackTransformer; use CleverAge\ProcessBundle\Transformer\GenericTransformer; use PHPUnit\Framework\TestCase; +use Symfony\Component\DependencyInjection\ServiceLocator; use Symfony\Component\OptionsResolver\Exception\MissingOptionsException; use Symfony\Component\OptionsResolver\OptionsResolver; @@ -73,8 +74,9 @@ public function testOptionalOptionWithoutDefaultIsNullWhenOmitted(): void public function testTransformersCanBeGivenAsAList(): void { - $registry = new TransformerRegistry(); - $registry->addTransformer(new CallbackTransformer()); + $registry = new TransformerRegistry(new ServiceLocator([ + 'callback' => static fn (): CallbackTransformer => new CallbackTransformer(), + ])); $transformer = new GenericTransformer(new ContextualOptionResolver(), $registry); $transformer->initialize('substr_upper', [ @@ -105,8 +107,9 @@ public function testGetCodeRequiresInitialization(): void */ private function createSubstrTransformer(array $contextualOptions): GenericTransformer { - $registry = new TransformerRegistry(); - $registry->addTransformer(new CallbackTransformer()); + $registry = new TransformerRegistry(new ServiceLocator([ + 'callback' => static fn (): CallbackTransformer => new CallbackTransformer(), + ])); $transformer = new GenericTransformer(new ContextualOptionResolver(), $registry); $transformer->initialize('substr', [ diff --git a/tests/Transformer/MappingTransformerTest.php b/tests/Transformer/MappingTransformerTest.php index 4b788ab3..2e188cd4 100644 --- a/tests/Transformer/MappingTransformerTest.php +++ b/tests/Transformer/MappingTransformerTest.php @@ -23,6 +23,7 @@ use Psr\Log\AbstractLogger; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; +use Symfony\Component\DependencyInjection\ServiceLocator; use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException; use Symfony\Component\OptionsResolver\Exception\MissingOptionsException; use Symfony\Component\OptionsResolver\OptionsResolver; @@ -622,8 +623,9 @@ public static function failingCallback(): never private function createTransformer(?LoggerInterface $logger = null): MappingTransformer { - $registry = new TransformerRegistry(); - $registry->addTransformer(new CallbackTransformer()); + $registry = new TransformerRegistry(new ServiceLocator([ + 'callback' => static fn (): CallbackTransformer => new CallbackTransformer(), + ])); return new MappingTransformer( $registry, diff --git a/tests/Transformer/RulesTransformerTest.php b/tests/Transformer/RulesTransformerTest.php index d2499509..6a72ae86 100644 --- a/tests/Transformer/RulesTransformerTest.php +++ b/tests/Transformer/RulesTransformerTest.php @@ -18,6 +18,7 @@ use CleverAge\ProcessBundle\Transformer\RulesTransformer; use CleverAge\ProcessBundle\Transformer\TransformerTrait; use PHPUnit\Framework\TestCase; +use Symfony\Component\DependencyInjection\ServiceLocator; use Symfony\Component\ExpressionLanguage\ExpressionLanguage; use Symfony\Component\OptionsResolver\OptionsResolver; @@ -72,8 +73,9 @@ public function testNonConfigurableTransformerCannotHaveOptions(): void private function createTransformer(): RulesTransformer { - $registry = new TransformerRegistry(); - $registry->addTransformer(new ArrayLastTransformer()); + $registry = new TransformerRegistry(new ServiceLocator([ + 'array_last' => static fn (): ArrayLastTransformer => new ArrayLastTransformer(), + ])); return new RulesTransformer($registry, new ExpressionLanguage()); } diff --git a/tests/Transformer/TransformerTraitTest.php b/tests/Transformer/TransformerTraitTest.php index 2f517bc0..85a20e5c 100644 --- a/tests/Transformer/TransformerTraitTest.php +++ b/tests/Transformer/TransformerTraitTest.php @@ -21,6 +21,7 @@ use CleverAge\ProcessBundle\Transformer\TransformerTrait; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; +use Symfony\Component\DependencyInjection\ServiceLocator; #[\PHPUnit\Framework\Attributes\CoversTrait(TransformerTrait::class)] #[\PHPUnit\Framework\Attributes\UsesClass(TransformerRegistry::class)] @@ -86,9 +87,10 @@ public function testTransformersCanBeGivenAsAList(): void public function testListedTransformerCodeWithoutOptions(): void { - $registry = new TransformerRegistry(); - $registry->addTransformer(new CallbackTransformer()); - $registry->addTransformer(new TrimTransformer()); + $registry = new TransformerRegistry(new ServiceLocator([ + 'callback' => static fn (): CallbackTransformer => new CallbackTransformer(), + 'trim' => static fn (): TrimTransformer => new TrimTransformer(), + ])); $holder = new TransformerTraitHolder($registry); $transformers = $holder->resolve(['trim', ['trim' => null], ['callback' => ['callback' => 'strrev']]]); @@ -142,8 +144,9 @@ public function testMissingRegistryThrows(): void private function createHolder(): TransformerTraitHolder { - $registry = new TransformerRegistry(); - $registry->addTransformer(new CallbackTransformer()); + $registry = new TransformerRegistry(new ServiceLocator([ + 'callback' => static fn (): CallbackTransformer => new CallbackTransformer(), + ])); return new TransformerTraitHolder($registry); } From f219c33e50064c4102ab15cc7ad7e848582d4c20 Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Wed, 14 Oct 2026 18:41:23 +0200 Subject: [PATCH 2/3] #276 - Deprecate TransformerInterface::getCode() - TransformerInterface::getCode(): @deprecated since 5.4 (the code is given by the "code" attribute of the cleverage.transformer tag or #[AsTransformer]), removed from the interface in v6.0 (#280). PHPDoc only: no runtime notice is possible for an interface method (nor reported by DebugClassLoader). - UnavailableServicesCompilerPass reads the code of the replaced transformer from its tag, getCode() (by reflection) only for a definition tagged without code. - Documentation: custom transformers (getCode() deprecation, paragraph split). Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 2 +- docs/03-custom_tasks.md | 9 +++++++-- .../UnavailableServicesCompilerPass.php | 17 +++++++++++++++-- src/Transformer/TransformerInterface.php | 4 ++++ .../UnavailableServicesCompilerPassTest.php | 13 +++++++++++++ 5 files changed, 40 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5185636a..c46d4e2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ Latest ------ ## Changes -* [#276](https://github.com/cleverage/process-bundle/issues/276) TransformerRegistry: the transformers are loaded lazily from a service locator indexed by code (new internal `TransformerLocatorCompilerPass`, new optional `TransformerRegistry` constructor argument): a transformer is only instantiated when a process uses it. The code is given by the new `code` attribute of the `cleverage.transformer` tag, or by the new `#[AsTransformer(code: '...')]` attribute (autoconfiguration); it must be the one returned by `getCode()`. All the bundle transformers and the generic transformers have their code. Duplicate codes fail the container compilation. A transformer tagged without `code` is still registered eagerly, with `TransformerRegistry::addTransformer()`, which is deprecated (the `code` will be required in v6.0, #280). `RegistryCompilerPass` is no longer used and deprecated. Update documentation, add tests. +* [#276](https://github.com/cleverage/process-bundle/issues/276) TransformerRegistry: the transformers are loaded lazily from a service locator indexed by code (new internal `TransformerLocatorCompilerPass`, new optional `TransformerRegistry` constructor argument): a transformer is only instantiated when a process uses it. The code is given by the new `code` attribute of the `cleverage.transformer` tag, or by the new `#[AsTransformer(code: '...')]` attribute (autoconfiguration); it must be the one returned by `getCode()`. All the bundle transformers and the generic transformers have their code. Duplicate codes fail the container compilation. A transformer tagged without `code` is still registered eagerly, with `TransformerRegistry::addTransformer()`, which is deprecated (the `code` will be required in v6.0, #280). `RegistryCompilerPass` is no longer used and deprecated. `TransformerInterface::getCode()` is deprecated (`@deprecated` PHPDoc: no runtime notice is possible for an interface method), it will be removed from the interface in v6.0 (#280): keep implementing it, returning the code of the tag until then. Update documentation, add tests. * [#149](https://github.com/cleverage/process-bundle/issues/149) Apply Symfony best practices to the tasks and transformers services: they are defined explicitly (no more autowiring, autoconfiguration nor bindings in the bundle), with `cleverage_process.task.*` / `cleverage_process.transformer.*` ids (private) and a public alias named after their class, so the `service: '@CleverAge\ProcessBundle\Task\...'` references, the autowiring of the transformers by class and `$container->get()` keep working. Same arguments, tags and `shared` settings as before (generated from the previously autowired definitions). Update documentation. * [#275](https://github.com/cleverage/process-bundle/issues/275) The tasks are fetched from a service locator built at compile time from the services referenced by the process configurations (new `TaskRegistry`, `cleverage_process.registry.task`, and internal `TaskLocatorCompilerPass`) instead of the whole service container: a task service can be private. `ProcessManager` and `ProcessHelpCommand` get the registry as a new optional last constructor argument; not passing it is deprecated (the tasks are then fetched from the container), and fetching a task service not referenced by the process configurations known at compile time falls back on the container with a deprecation. Both fallbacks will be removed in v6.0 (#114). Update documentation, add tests. * [#278](https://github.com/cleverage/process-bundle/issues/278) The container no longer fails to compile when the Symfony serializer or validator is not enabled: the tasks and transformers depending on them (`NormalizerTask`, `DenormalizerTask`, `SerializerTask`, `DeserializerTask`, `normalize` and `denormalize` transformers, `ValidatorTask`) are replaced by services failing with an explicit message when they are used (new internal `UnavailableServicesCompilerPass`). `CheckSerializerCompilerPass` is no longer used and deprecated. Update documentation, add tests. diff --git a/docs/03-custom_tasks.md b/docs/03-custom_tasks.md index f10820e6..6fbb962d 100644 --- a/docs/03-custom_tasks.md +++ b/docs/03-custom_tasks.md @@ -257,10 +257,15 @@ transformer is only instantiated when a process uses it. The code of the tag mus `TransformerInterface::getCode()` (an exception is thrown otherwise when the transformer is used). Two transformers cannot share the same code (the container compilation fails). +`TransformerInterface::getCode()` is deprecated since 5.4: the code is given by the tag or the attribute, and the method +will be removed from the interface in v6.0. Until then, keep implementing it, returning the same code. + A transformer tagged without `code` (e.g. autoconfigured without the attribute) still works, but it is instantiated with the registry and registered with `TransformerRegistry::addTransformer()`, which is deprecated since 5.4: the `code` will -be required in v6.0. When creating a new transformer for your -project you should use an internal prefix in your codes to avoid conflicts with potential standard additions. +be required in v6.0. + +When creating a new transformer for your project, use an internal prefix in your codes to avoid conflicts with potential +standard additions. ```yaml transform: diff --git a/src/DependencyInjection/Compiler/UnavailableServicesCompilerPass.php b/src/DependencyInjection/Compiler/UnavailableServicesCompilerPass.php index d7b381f1..b3162e22 100644 --- a/src/DependencyInjection/Compiler/UnavailableServicesCompilerPass.php +++ b/src/DependencyInjection/Compiler/UnavailableServicesCompilerPass.php @@ -72,8 +72,10 @@ private function replace(ContainerBuilder $container, string $message, array $cl $original = $container->getDefinition($id); $text = \sprintf($message, $class); if (is_a($class, TransformerInterface::class, true)) { - // The code is needed by the transformer registry: the bundle transformers return a constant - $code = (new \ReflectionClass($class))->newInstanceWithoutConstructor()->getCode(); + // The code is needed by the transformer registry: the one of the tag (see TransformerLocatorCompilerPass), + // or, for a definition tagged without code (deprecated), the one returned by the bundle transformer + $code = $this->getTagCode($original) + ?? (new \ReflectionClass($class))->newInstanceWithoutConstructor()->getCode(); $definition = new Definition(UnavailableTransformer::class, [$code, $text]); } else { $definition = new Definition(UnavailableTask::class, [$text]); @@ -84,4 +86,15 @@ private function replace(ContainerBuilder $container, string $message, array $cl $container->setDefinition($id, $definition); } } + + private function getTagCode(Definition $definition): ?string + { + foreach ($definition->getTag('cleverage.transformer') as $attributes) { + if (isset($attributes['code']) && \is_string($attributes['code'])) { + return $attributes['code']; + } + } + + return null; + } } diff --git a/src/Transformer/TransformerInterface.php b/src/Transformer/TransformerInterface.php index 8a61e7fe..a6980db0 100644 --- a/src/Transformer/TransformerInterface.php +++ b/src/Transformer/TransformerInterface.php @@ -27,6 +27,10 @@ public function transform(mixed $value, array $options = []): mixed; /** * Returns the unique code to identify the transformer. + * + * @deprecated since 5.4, the code of a transformer is given by the "code" attribute of its "cleverage.transformer" + * tag or by the #[AsTransformer] attribute; this method will be removed from the interface in v6.0. + * Until then, it must return the same code. */ public function getCode(): string; } diff --git a/tests/DependencyInjection/Compiler/UnavailableServicesCompilerPassTest.php b/tests/DependencyInjection/Compiler/UnavailableServicesCompilerPassTest.php index a420899b..137bf0d7 100644 --- a/tests/DependencyInjection/Compiler/UnavailableServicesCompilerPassTest.php +++ b/tests/DependencyInjection/Compiler/UnavailableServicesCompilerPassTest.php @@ -72,6 +72,19 @@ public function testSerializerServicesAreReplacedWithoutSerializer(): void self::assertSame(ValidatorTask::class, $container->findDefinition(ValidatorTask::class)->getClass()); } + public function testTransformerTaggedWithoutCodeKeepsTheCodeOfTheClass(): void + { + $container = new ContainerBuilder(); + // e.g. a definition overridden by the application, tagged without code (deprecated) + $container->register(NormalizeTransformer::class, NormalizeTransformer::class)->addTag('cleverage.transformer'); + + (new UnavailableServicesCompilerPass())->process($container); + + $definition = $container->getDefinition(NormalizeTransformer::class); + self::assertSame(UnavailableTransformer::class, $definition->getClass()); + self::assertSame('normalize', $definition->getArgument(0)); + } + public function testValidatorTaskIsReplacedWithoutValidator(): void { $container = $this->createContainer(serializer: true, validator: false); From 6d1499d8f69fd0e364fd17e1cc0e03c69ef45ee8 Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Wed, 14 Oct 2026 18:49:27 +0200 Subject: [PATCH 3/3] #276 - Fix the risky tests with code coverage The CI runs PHPUnit with code coverage (beStrictAboutCoverageMetadata): - testAsTransformerAttributeRegistersTheCode: declare AsTransformer as used; - the check of the transformer codes runs getCode() of every transformer of the bundle: moved to a #[CoversNothing] TransformerCodesTest. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../CleverAgeProcessExtensionTest.php | 28 +--------- .../TransformerCodesTest.php | 54 +++++++++++++++++++ 2 files changed, 56 insertions(+), 26 deletions(-) create mode 100644 tests/DependencyInjection/TransformerCodesTest.php diff --git a/tests/DependencyInjection/CleverAgeProcessExtensionTest.php b/tests/DependencyInjection/CleverAgeProcessExtensionTest.php index c2b7b94e..135958a4 100644 --- a/tests/DependencyInjection/CleverAgeProcessExtensionTest.php +++ b/tests/DependencyInjection/CleverAgeProcessExtensionTest.php @@ -13,6 +13,7 @@ namespace CleverAge\ProcessBundle\Tests\DependencyInjection; +use CleverAge\ProcessBundle\Attribute\AsTransformer; use CleverAge\ProcessBundle\Context\ContextualOptionResolver; use CleverAge\ProcessBundle\DependencyInjection\CleverAgeProcessExtension; use CleverAge\ProcessBundle\DependencyInjection\Configuration; @@ -22,7 +23,6 @@ use CleverAge\ProcessBundle\Registry\TransformerRegistry; use CleverAge\ProcessBundle\Task\DummyTask; use CleverAge\ProcessBundle\Tests\Fixtures\AttributedTransformer; -use CleverAge\ProcessBundle\Transformer\GenericTransformer; use CleverAge\ProcessBundle\Transformer\String\TrimTransformer; use CleverAge\ProcessBundle\Transformer\TransformerInterface; use PHPUnit\Framework\Attributes\CoversClass; @@ -39,6 +39,7 @@ #[CoversClass(CleverAgeProcessExtension::class)] #[UsesClass(Configuration::class)] +#[UsesClass(AsTransformer::class)] class CleverAgeProcessExtensionTest extends TestCase { public function testEveryServiceDefinitionFileIsLoaded(): void @@ -122,31 +123,6 @@ public function testTasksAndTransformersAreAutoconfigured(): void self::assertSame([['code' => 'trim']], $container->findDefinition(TrimTransformer::class)->getTag('cleverage.transformer')); } - public function testEveryTransformerIsRegisteredWithItsCode(): void - { - $container = new ContainerBuilder(); - (new CleverAgeProcessExtension())->load([[ - 'generic_transformers' => ['app_generic' => ['transformers' => ['trim' => []]]], - ]], $container); - - $tagged = $container->findTaggedServiceIds('cleverage.transformer'); - self::assertCount(38, $tagged); - foreach ($tagged as $id => $tags) { - $class = $container->getDefinition($id)->getClass(); - self::assertIsString($class); - if (GenericTransformer::class === $class) { - $code = 'app_generic'; - } else { - self::assertTrue(is_a($class, TransformerInterface::class, true), $class); - $transformer = (new \ReflectionClass($class))->newInstanceWithoutConstructor(); - self::assertInstanceOf(TransformerInterface::class, $transformer); - $code = $transformer->getCode(); - } - // Lazy loading: the code of the tag must be the one returned by getCode() - self::assertSame([['code' => $code]], $tags, $id); - } - } - public function testAsTransformerAttributeRegistersTheCode(): void { $container = new ContainerBuilder(); diff --git a/tests/DependencyInjection/TransformerCodesTest.php b/tests/DependencyInjection/TransformerCodesTest.php new file mode 100644 index 00000000..4496deae --- /dev/null +++ b/tests/DependencyInjection/TransformerCodesTest.php @@ -0,0 +1,54 @@ +load([[ + 'generic_transformers' => ['app_generic' => ['transformers' => ['trim' => []]]], + ]], $container); + + $tagged = $container->findTaggedServiceIds('cleverage.transformer'); + self::assertCount(38, $tagged); + foreach ($tagged as $id => $tags) { + $class = $container->getDefinition($id)->getClass(); + self::assertIsString($class); + if (GenericTransformer::class === $class) { + $code = 'app_generic'; + } else { + self::assertTrue(is_a($class, TransformerInterface::class, true), $class); + $transformer = (new \ReflectionClass($class))->newInstanceWithoutConstructor(); + self::assertInstanceOf(TransformerInterface::class, $transformer); + $code = $transformer->getCode(); + } + // Lazy loading: the code of the tag must be the one returned by getCode() + self::assertSame([['code' => $code]], $tags, $id); + } + } +}