From c12a34da346fabd874ce7a1dba21d227a278f8f3 Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Tue, 20 Oct 2026 11:22:55 +0200 Subject: [PATCH] #305 - Bundle best practices: residues of the review - The generic transformers are no longer autowired (explicit arguments). - ProcessHelpCommand receives the TaskRegistry as second argument instead of the service container; passing a container (constructor of v5.3) is deprecated. The unreleased third argument is removed. - New internal PublicTaskDeprecationCompilerPass: the tasks of the application made public only by the TaskInterface autoconfiguration get the container.private tag (fetching them from the container is deprecated, as for the tasks of the bundle). Tasks made public by the application itself (service, _defaults, _instanceof, #[Autoconfigure]) are not concerned. - Missing deprecation notices: InputAggregatorTask, CheckSerializerCompilerPass, "errors" task configuration key (setDeprecated(), use "error_outputs"). - TaskConfiguration::getErrors(): @deprecated PHPDoc instead of the native #[\Deprecated] attribute (a single notice on PHP 8.4+); $logErrors marked @deprecated. - CleverAgeProcessBundle marked @final since 5.4. - Documentation and CHANGELOG updated, tests added. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 1 + config/services/command.yaml | 1 - docs/03-custom_tasks.md | 5 +- docs/05-good_practices.md | 3 +- docs/reference/02-task_definition.md | 3 +- docs/reference/tasks/input_aggregator_task.md | 3 +- src/CleverAgeProcessBundle.php | 7 + src/Command/ProcessHelpCommand.php | 18 ++- src/Configuration/TaskConfiguration.php | 8 +- .../CleverAgeProcessExtension.php | 4 +- .../Compiler/CheckSerializerCompilerPass.php | 9 +- .../PublicTaskDeprecationCompilerPass.php | 64 +++++++++ src/DependencyInjection/Configuration.php | 12 +- src/Task/InputAggregatorTask.php | 7 +- tests/Command/ProcessHelpCommandTest.php | 6 +- tests/Configuration/TaskConfigurationTest.php | 5 +- .../CheckSerializerCompilerPassTest.php | 59 +++++++++ .../PublicTaskDeprecationCompilerPassTest.php | 122 ++++++++++++++++++ .../DependencyInjection/ConfigurationTest.php | 25 ++++ tests/Fixtures/PublicTask.php | 26 ++++ tests/Functional/KernelTest.php | 10 +- tests/Task/InputAggregatorTaskTest.php | 48 +++++-- 22 files changed, 408 insertions(+), 38 deletions(-) create mode 100644 src/DependencyInjection/Compiler/PublicTaskDeprecationCompilerPass.php create mode 100644 tests/DependencyInjection/Compiler/CheckSerializerCompilerPassTest.php create mode 100644 tests/DependencyInjection/Compiler/PublicTaskDeprecationCompilerPassTest.php create mode 100644 tests/Fixtures/PublicTask.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 20585412..808d72eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ Latest ------ ## Changes +* [#305](https://github.com/cleverage/process-bundle/issues/305) Bundle best practices, residues: the generic transformers are no longer autowired (explicit arguments). `ProcessHelpCommand` receives the `TaskRegistry` instead of the service container (passing a container is deprecated). Fetching from the service container a task of the application made public by the `TaskInterface` autoconfiguration is deprecated (`container.private` tag, new internal `PublicTaskDeprecationCompilerPass`); the tasks made public by the application itself are not concerned. Trigger the missing deprecation notices: `InputAggregatorTask`, `CheckSerializerCompilerPass`, `errors` task configuration key (use `error_outputs`). `TaskConfiguration::getErrors()` triggers a single notice on PHP 8.4+. `CleverAgeProcessBundle` is marked `@final since 5.4`. Update documentation, add tests. * [#299](https://github.com/cleverage/process-bundle/issues/299) Require `symfony/polyfill-php83`, `symfony/polyfill-php84` and `symfony/polyfill-php85`: 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: `ProcessConfiguration` uses `array_any()` again; `array_first()` replaces `reset()` / `current()` in `ProcessConfiguration::getMainTask()`, `ConstraintLoader` and `ProcessHelpCommand` (no internal array pointer, same results). * [#279](https://github.com/cleverage/process-bundle/issues/279) Define the public API (Symfony BC promise): the classes not meant to be extended are marked `@final since 5.4` (the engine: `ProcessManager`, `ProcessConfigurationRegistry`, `TransformerRegistry`, `ContextualOptionResolver`; the models: `ProcessState`, `ProcessHistory`, `SubprocessInstance`, the configurations, the events, the `Filesystem` files; the concrete tasks and transformers), they will be `final` in v6.0: extending them triggers a deprecation with the Symfony `DebugClassLoader`. The abstract classes, interfaces, traits, exceptions and the file tasks extended by other tasks (`FileReaderTask`, `LineReaderTask`, `FolderBrowserTask`, `CsvReaderTask`, `InputCsvReaderTask`, `Filesystem\CsvResource`) stay open. `TaskRegistry`, new in this version, is `final`. The infrastructure classes are marked `@internal`: `CleverAgeProcessExtension`, `Configuration`, the commands, `DataQueueEventListener`, `PhpFunctionProvider`, the Monolog processors, `ProcessLogger` and `TaskLogger`. Update documentation. * [#269](https://github.com/cleverage/process-bundle/issues/269) Composer: `symfony/process`, `symfony/stopwatch`, `symfony/var-dumper` and `symfony/monolog-bundle` are no longer required, they are suggested (applications using them must require them). Without `symfony/process` (`CommandRunnerTask`, `ProcessLauncherTask`) or `symfony/stopwatch` (`StopwatchTask`), the container still compiles and these tasks fail with an explicit message (`UnavailableServicesCompilerPass`); without `symfony/var-dumper`, nothing is dumped (as before); without `symfony/monolog-bundle`, the `cleverage_process*` channels and processors are not registered and the default logger is used. `monolog/monolog` stays required (`Logger\*Processor`). Remove the unused `symfony/monolog-bridge` dependency. `ProcessConfiguration` no longer uses `array_any()` (PHP 8.4), whose polyfill was only installed by the removed packages. Add a `conflict` with `cleverage/doctrine-process-bundle`, `cleverage/soap-process-bundle` and `cleverage/ui-process-bundle` `<3.3`, which use the MonologBundle without requiring it. Update documentation, add tests. diff --git a/config/services/command.yaml b/config/services/command.yaml index 605446db..c0e3f852 100644 --- a/config/services/command.yaml +++ b/config/services/command.yaml @@ -24,5 +24,4 @@ services: - { name: console.command } arguments: - '@cleverage_process.registry.process_configuration' - - '@service_container' - '@cleverage_process.registry.task' diff --git a/docs/03-custom_tasks.md b/docs/03-custom_tasks.md index 3f190222..386cd043 100644 --- a/docs/03-custom_tasks.md +++ b/docs/03-custom_tasks.md @@ -25,7 +25,10 @@ each time a task configuration is initialized. With autoconfiguration (`autoconfigure: true`, set in the `_defaults` of the Symfony skeleton), the bundle applies these settings to every service implementing `TaskInterface`: `shared: false` and a `monolog.logger` tag on the `cleverage_process_task` channel (see [logging](#logging)), as well as `public: true`, only kept for the deprecated -fallback on the service container. A setting explicitly defined on the service wins (e.g. `shared: true`). +fallback on the service container: fetching such a task directly from the service container is deprecated since 5.4 +(the task will be private in v6.0), unless the application makes it public itself (`public: true` on the service, in +`_defaults` or `_instanceof`, or `#[Autoconfigure(public: true)]`). A setting explicitly defined on the service wins +(e.g. `shared: true`). ```yaml # config/services.yaml diff --git a/docs/05-good_practices.md b/docs/05-good_practices.md index 2f23df41..fadcd204 100644 --- a/docs/05-good_practices.md +++ b/docs/05-good_practices.md @@ -71,8 +71,7 @@ See [errors and skips](04-advanced_workflow.md#errors-and-skips). When a task has several `outputs`, each output is fully processed one after the other, in the configured order, but this is an implementation detail. If a task needs the result of another branch, make the dependency explicit in the -graph (e.g. with a blocking task or an [InputAggregatorTask](reference/tasks/input_aggregator_task.md)) rather than -relying on the order in which branches are executed. +graph (e.g. with a blocking task) rather than relying on the order in which branches are executed. ## Use the context for runtime parameters diff --git a/docs/reference/02-task_definition.md b/docs/reference/02-task_definition.md index adeb9bd7..29d1d946 100644 --- a/docs/reference/02-task_definition.md +++ b/docs/reference/02-task_definition.md @@ -54,7 +54,8 @@ are replaced by the process [contextual values](../01-quick_start.md#contextual- **error_outputs**: optional list of following tasks, receiving the error output of this task (by default its input when an error occurs). It can be a simple string. See [errors and skips](../04-advanced_workflow.md#errors-and-skips). -**errors**: deprecated alias of `error_outputs`. Defining both on the same task throws an exception. +**errors**: **deprecated since 5.4, removed in v6.0**, use `error_outputs` (same behaviour). Defining both on the same +task throws an exception. **error_strategy**: optional, either *skip* or *stop*. Defines if the process continues with the next input or stops when the task fails. When not defined, the global `default_error_strategy` is used (*stop* by default). diff --git a/docs/reference/tasks/input_aggregator_task.md b/docs/reference/tasks/input_aggregator_task.md index 2ade9256..f6b136b7 100644 --- a/docs/reference/tasks/input_aggregator_task.md +++ b/docs/reference/tasks/input_aggregator_task.md @@ -1,7 +1,8 @@ InputAggregatorTask =================== -**Deprecated**: this class is marked `@deprecated` (too error-prone, should be refactored as a blocking task). +**Deprecated since v5.4, removed in v6.0**: this task is too error-prone (it should be refactored as a blocking task). +An `E_USER_DEPRECATED` error is triggered when it is instantiated. Accumulates inputs coming from several parent tasks. The task is skipped until an input has been received from every parent declared in `input_codes`; it then outputs an array of all received inputs, indexed by their destination key, diff --git a/src/CleverAgeProcessBundle.php b/src/CleverAgeProcessBundle.php index 2508d296..81af58b1 100644 --- a/src/CleverAgeProcessBundle.php +++ b/src/CleverAgeProcessBundle.php @@ -13,12 +13,17 @@ namespace CleverAge\ProcessBundle; +use CleverAge\ProcessBundle\DependencyInjection\Compiler\PublicTaskDeprecationCompilerPass; use CleverAge\ProcessBundle\DependencyInjection\Compiler\TaskLocatorCompilerPass; use CleverAge\ProcessBundle\DependencyInjection\Compiler\TransformerLocatorCompilerPass; use CleverAge\ProcessBundle\DependencyInjection\Compiler\UnavailableServicesCompilerPass; +use Symfony\Component\DependencyInjection\Compiler\PassConfig; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpKernel\Bundle\Bundle; +/** + * @final since 5.4 + */ class CleverAgeProcessBundle extends Bundle { /** @@ -31,6 +36,8 @@ public function build(ContainerBuilder $container): void $container->addCompilerPass(new UnavailableServicesCompilerPass()); $container->addCompilerPass(new TransformerLocatorCompilerPass()); $container->addCompilerPass(new TaskLocatorCompilerPass()); + // Before ResolveInstanceofConditionalsPass (priority 100) + $container->addCompilerPass(new PublicTaskDeprecationCompilerPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, 110); } #[\Override] diff --git a/src/Command/ProcessHelpCommand.php b/src/Command/ProcessHelpCommand.php index f02109a4..c9ba520b 100644 --- a/src/Command/ProcessHelpCommand.php +++ b/src/Command/ProcessHelpCommand.php @@ -64,22 +64,28 @@ class ProcessHelpCommand extends Command protected TaskRegistry $taskRegistry; + /** + * @deprecated since 5.4, only set when a container is passed to the constructor, will be removed in v6.0 + */ + protected ?ContainerInterface $container = null; + public function __construct( protected ProcessConfigurationRegistry $processConfigRegistry, - protected ContainerInterface $container, - ?TaskRegistry $taskRegistry = null, + TaskRegistry|ContainerInterface $taskRegistry, ) { parent::__construct(); - if (!$taskRegistry instanceof TaskRegistry) { + if ($taskRegistry instanceof ContainerInterface) { trigger_deprecation( 'cleverage/process-bundle', '5.4', - 'Not passing a "%s" to "%s::__construct()" is deprecated: the tasks are fetched from the service container. The argument will be required in v6.0.', - TaskRegistry::class, + 'Passing a service container to "%s::__construct()" is deprecated, pass a "%s" instead: the tasks are fetched from the service container. Only a "%s" will be accepted in v6.0.', self::class, + TaskRegistry::class, + TaskRegistry::class, ); + $this->container = $taskRegistry; // Same behaviour as before the task registry: any service of the container - $taskRegistry = new TaskRegistry($this->container, $this->container); + $taskRegistry = new TaskRegistry($taskRegistry, $taskRegistry); } $this->taskRegistry = $taskRegistry; } diff --git a/src/Configuration/TaskConfiguration.php b/src/Configuration/TaskConfiguration.php index eafa061e..9dd6890a 100644 --- a/src/Configuration/TaskConfiguration.php +++ b/src/Configuration/TaskConfiguration.php @@ -49,6 +49,9 @@ class TaskConfiguration protected bool $inErrorBranch = false; + /** + * @deprecated not used, will be removed in v6.0 + */ protected bool $logErrors; /** @@ -67,7 +70,7 @@ public function __construct( protected string $errorStrategy = self::STRATEGY_SKIP, protected string $logLevel = LogLevel::CRITICAL, ) { - $this->logErrors = LogLevel::DEBUG !== $logLevel; // @deprecated, remove me in next version + $this->logErrors = LogLevel::DEBUG !== $logLevel; } public function getCode(): string @@ -126,9 +129,10 @@ public function getOutputs(): array } /** + * @deprecated since 2.0.4, use getErrorOutputs() instead, will be removed in v6.0 + * * @return list */ - #[\Deprecated(message: 'Use getErrorOutputs method instead')] public function getErrors(): array { trigger_deprecation('cleverage/process-bundle', '2.0.4', 'The "%s()" method is deprecated, use "getErrorOutputs()" instead.', __METHOD__); diff --git a/src/DependencyInjection/CleverAgeProcessExtension.php b/src/DependencyInjection/CleverAgeProcessExtension.php index b266012c..8b62613f 100644 --- a/src/DependencyInjection/CleverAgeProcessExtension.php +++ b/src/DependencyInjection/CleverAgeProcessExtension.php @@ -43,7 +43,8 @@ public function load(array $configs, ContainerBuilder $container): void $container->registerForAutoconfiguration(ProcessStopCheckerInterface::class) ->addTag('cleverage_process.stop_checker'); - // Tasks are fetched from the container by the process manager, and keep state between executions + // Tasks keep state between executions. Public for the deprecated fallback of the TaskRegistry on the service + // container: their direct access is deprecated (PublicTaskDeprecationCompilerPass), private in v6.0 $container->registerForAutoconfiguration(TaskInterface::class) ->setPublic(true) ->setShared(false) @@ -77,7 +78,6 @@ static function (ChildDefinition $definition, AsTransformer $attribute): void { // Automatic transformer creation from config foreach ($config['generic_transformers'] as $transformerCode => $transformerConfig) { $transformerDefinition = new Definition(GenericTransformer::class); - $transformerDefinition->setAutowired(true); $transformerDefinition->setPublic(false); $transformerDefinition->setArguments([ new Reference('cleverage_process.context.contextual_option_resolver'), diff --git a/src/DependencyInjection/Compiler/CheckSerializerCompilerPass.php b/src/DependencyInjection/Compiler/CheckSerializerCompilerPass.php index adb661c8..873a18df 100644 --- a/src/DependencyInjection/Compiler/CheckSerializerCompilerPass.php +++ b/src/DependencyInjection/Compiler/CheckSerializerCompilerPass.php @@ -21,13 +21,18 @@ /** * Check the presence of the serializer (required for this bundle), and help the user to set it. * - * @deprecated no longer used by the bundle, which compiles without the serializer: the services depending on it - * fail when they are used (see UnavailableServicesCompilerPass). Will be removed in v6.0. + * @deprecated since 5.4, no longer used by the bundle, which compiles without the serializer: the services depending + * on it fail when they are used (see UnavailableServicesCompilerPass). Will be removed in v6.0. */ class CheckSerializerCompilerPass implements CompilerPassInterface { final public const MSG = 'The Symfony serializer component do not seem enabled, consider toggling framework.serializer.enable (see https://symfony.com/doc/current/reference/configuration/framework.html#reference-serializer-enabled)'; + public function __construct() + { + trigger_deprecation('cleverage/process-bundle', '5.4', 'The "%s" class is deprecated, it will be removed in v6.0.', self::class); + } + public function process(ContainerBuilder $container): void { if (!$container->has('serializer') && !$container->has(DenormalizerInterface::class)) { diff --git a/src/DependencyInjection/Compiler/PublicTaskDeprecationCompilerPass.php b/src/DependencyInjection/Compiler/PublicTaskDeprecationCompilerPass.php new file mode 100644 index 00000000..cc80009f --- /dev/null +++ b/src/DependencyInjection/Compiler/PublicTaskDeprecationCompilerPass.php @@ -0,0 +1,64 @@ +getDefinitions() as $id => $definition) { + if (!$definition->isAutoconfigured() + || $definition->isAbstract() + || $definition->isSynthetic() + || $definition->hasTag('container.private') + || isset($definition->getChanges()['public'])) { + continue; + } + // Same resolution as ResolveClassPass for the services whose id is the class + $class = $container->getParameterBag()->resolveValue($definition->getClass() ?? $id); + if (!\is_string($class) + || !($reflectionClass = $container->getReflectionClass($class, false)) instanceof \ReflectionClass + || !$reflectionClass->implementsInterface(TaskInterface::class)) { + continue; + } + foreach ($definition->getInstanceofConditionals() as $type => $instanceof) { + if (isset($instanceof->getChanges()['public']) && is_a($class, $type, true)) { + continue 2; + } + } + foreach ($reflectionClass->getAttributes(Autoconfigure::class, \ReflectionAttribute::IS_INSTANCEOF) as $attribute) { + if (null !== $attribute->newInstance()->public) { + continue 2; + } + } + + $definition->addTag('container.private', ['package' => 'cleverage/process-bundle', 'version' => '5.4']); + } + } +} diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 3d8ef275..44144ae2 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -210,8 +210,16 @@ protected function appendTaskConfigDefinition(NodeBuilder $definition): void ->defaultValue(LogLevel::CRITICAL); foreach (['outputs', 'errors', 'error_outputs'] as $nodeName) { - $definition->arrayNode($nodeName) - ->beforeNormalization() + $node = $definition->arrayNode($nodeName); + if ('errors' === $nodeName) { + $node->info('Deprecated, use "error_outputs"') + ->setDeprecated( + 'cleverage/process-bundle', + '5.4', + 'The "%node%" option of the task "%path%" is deprecated, use "error_outputs" instead. It will be removed in v6.0.' + ); + } + $node->beforeNormalization() ->ifString() ->then(static fn ($item): array => [$item])->end() ->scalarPrototype(); diff --git a/src/Task/InputAggregatorTask.php b/src/Task/InputAggregatorTask.php index 12af9859..7fffe702 100644 --- a/src/Task/InputAggregatorTask.php +++ b/src/Task/InputAggregatorTask.php @@ -22,10 +22,15 @@ * Should have been a BlockingTask, but due to limitations in the current model, it's a hack using skips and finalize. * * @see README.md:Known issues - * @deprecated It's way too much error prone - should be refactored as a blocking task + * @deprecated since 5.4, it's way too much error prone - should be refactored as a blocking task, will be removed in v6.0 */ class InputAggregatorTask extends AbstractConfigurableTask { + public function __construct() + { + trigger_deprecation('cleverage/process-bundle', '5.4', 'The "%s" task is deprecated: it is too error-prone. It will be removed in v6.0.', self::class); + } + /** * @var array */ diff --git a/tests/Command/ProcessHelpCommandTest.php b/tests/Command/ProcessHelpCommandTest.php index 39e0fd74..b23f0e72 100644 --- a/tests/Command/ProcessHelpCommandTest.php +++ b/tests/Command/ProcessHelpCommandTest.php @@ -27,10 +27,10 @@ #[UsesClass(TaskRegistry::class)] class ProcessHelpCommandTest extends TestCase { - public function testWithoutTaskRegistryIsDeprecated(): void + public function testWithContainerIsDeprecated(): void { self::assertSame( - ['Since cleverage/process-bundle 5.4: Not passing a "'.TaskRegistry::class.'" to "'.ProcessHelpCommand::class.'::__construct()" is deprecated: the tasks are fetched from the service container. The argument will be required in v6.0.'], + ['Since cleverage/process-bundle 5.4: Passing a service container to "'.ProcessHelpCommand::class.'::__construct()" is deprecated, pass a "'.TaskRegistry::class.'" instead: the tasks are fetched from the service container. Only a "'.TaskRegistry::class.'" will be accepted in v6.0.'], $this->collectDeprecations(static fn (): ProcessHelpCommand => new ProcessHelpCommand(new ProcessConfigurationRegistry([], 'stop'), new Container())) ); } @@ -39,7 +39,7 @@ public function testWithTaskRegistryIsNotDeprecated(): void { self::assertSame( [], - $this->collectDeprecations(static fn (): ProcessHelpCommand => new ProcessHelpCommand(new ProcessConfigurationRegistry([], 'stop'), new Container(), new TaskRegistry(new ServiceLocator([])))) + $this->collectDeprecations(static fn (): ProcessHelpCommand => new ProcessHelpCommand(new ProcessConfigurationRegistry([], 'stop'), new TaskRegistry(new ServiceLocator([])))) ); } diff --git a/tests/Configuration/TaskConfigurationTest.php b/tests/Configuration/TaskConfigurationTest.php index bbe36a47..366cdb53 100644 --- a/tests/Configuration/TaskConfigurationTest.php +++ b/tests/Configuration/TaskConfigurationTest.php @@ -37,9 +37,8 @@ public function testGetErrorsIsDeprecated(): void } self::assertSame(['error'], $errors); - // PHP 8.4+ also triggers its own deprecation for the #[\Deprecated] attribute - self::assertContains( - 'Since cleverage/process-bundle 2.0.4: The "CleverAge\ProcessBundle\Configuration\TaskConfiguration::getErrors()" method is deprecated, use "getErrorOutputs()" instead.', + self::assertSame( + ['Since cleverage/process-bundle 2.0.4: The "CleverAge\ProcessBundle\Configuration\TaskConfiguration::getErrors()" method is deprecated, use "getErrorOutputs()" instead.'], $deprecations ); } diff --git a/tests/DependencyInjection/Compiler/CheckSerializerCompilerPassTest.php b/tests/DependencyInjection/Compiler/CheckSerializerCompilerPassTest.php new file mode 100644 index 00000000..58797805 --- /dev/null +++ b/tests/DependencyInjection/Compiler/CheckSerializerCompilerPassTest.php @@ -0,0 +1,59 @@ +expectException(AutowiringFailedException::class); + + self::ignoreDeprecations(static fn (): CheckSerializerCompilerPass => new CheckSerializerCompilerPass())->process(new ContainerBuilder()); + } + + #[\PHPUnit\Framework\Attributes\DoesNotPerformAssertions] + public function testSerializerIsFound(): void + { + $container = new ContainerBuilder(); + $container->register('serializer', \stdClass::class); + + self::ignoreDeprecations(static fn (): CheckSerializerCompilerPass => new CheckSerializerCompilerPass())->process($container); + } +} diff --git a/tests/DependencyInjection/Compiler/PublicTaskDeprecationCompilerPassTest.php b/tests/DependencyInjection/Compiler/PublicTaskDeprecationCompilerPassTest.php new file mode 100644 index 00000000..e2c4f2b6 --- /dev/null +++ b/tests/DependencyInjection/Compiler/PublicTaskDeprecationCompilerPassTest.php @@ -0,0 +1,122 @@ +compile([ + 'app.task' => (new Definition(DummyTask::class))->setAutoconfigured(true), + // Class given by the id + AttributedTask::class => (new Definition())->setAutoconfigured(true), + ]); + + foreach (['app.task', AttributedTask::class] as $id) { + self::assertTrue($container->hasAlias($id), $id); + $alias = $container->getAlias($id); + self::assertTrue($alias->isPublic(), $id); + self::assertTrue($alias->isDeprecated(), $id); + self::assertSame( + ['package' => 'cleverage/process-bundle', 'version' => '5.4', 'message' => 'Accessing the "'.$id.'" service directly from the container is deprecated, use dependency injection instead.'], + $alias->getDeprecation($id), + ); + self::assertFalse($container->getDefinition('.container.private.'.$id)->isShared(), $id); + } + } + + public function testTasksMadePublicByTheApplicationAreNotDeprecated(): void + { + $container = $this->compile([ + // Service or "_defaults" configuration + 'app.public_task' => (new Definition(DummyTask::class))->setAutoconfigured(true)->setPublic(true), + // "_instanceof" configuration + 'app.instanceof_task' => (new Definition(DummyTask::class)) + ->setAutoconfigured(true) + ->setInstanceofConditionals([TaskInterface::class => (new ChildDefinition(''))->setPublic(true)]), + // #[Autoconfigure(public: true)] + PublicTask::class => (new Definition(PublicTask::class))->setAutoconfigured(true), + ]); + + foreach (['app.public_task', 'app.instanceof_task', PublicTask::class] as $id) { + self::assertFalse($container->hasAlias($id), $id); + self::assertTrue($container->getDefinition($id)->isPublic(), $id); + } + } + + public function testOtherServicesAreIgnored(): void + { + $container = $this->compile([ + 'app.not_autoconfigured_task' => new Definition(DummyTask::class), + 'app.private_task' => (new Definition(DummyTask::class))->setAutoconfigured(true)->setPublic(false), + 'app.abstract_task' => (new Definition(DummyTask::class))->setAutoconfigured(true)->setAbstract(true), + 'app.transformer' => (new Definition(TrimTransformer::class))->setAutoconfigured(true), + 'app.unknown_class' => (new Definition('App\Unknown'))->setAutoconfigured(true), + ]); + + foreach (['app.not_autoconfigured_task', 'app.private_task', 'app.abstract_task', 'app.transformer', 'app.unknown_class'] as $id) { + self::assertFalse($container->hasAlias($id), $id); + } + self::assertFalse($container->getDefinition('app.private_task')->isPublic()); + // The tasks of the bundle keep their own tag + self::assertSame( + [['package' => 'cleverage/process-bundle', 'version' => '5.4']], + $container->getDefinition('.container.private.'.DummyTask::class)->getTag('container.private'), + ); + } + + /** + * Same order as the compilation of the container (PassConfig). + * + * @param array $definitions + */ + private function compile(array $definitions): ContainerBuilder + { + $container = new ContainerBuilder(); + (new CleverAgeProcessExtension())->load([], $container); + $container->addDefinitions($definitions); + + (new PublicTaskDeprecationCompilerPass())->process($container); + (new ResolveClassPass())->process($container); + (new RegisterAutoconfigureAttributesPass())->process($container); + (new ResolveInstanceofConditionalsPass())->process($container); + (new ResolveChildDefinitionsPass())->process($container); + (new AliasDeprecatedPublicServicesPass())->process($container); + + return $container; + } +} diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php index a5baf3cd..413dd138 100644 --- a/tests/DependencyInjection/ConfigurationTest.php +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -93,6 +93,31 @@ public function testServiceIsDeprecated(): void ); } + public function testErrorsIsDeprecated(): void + { + $deprecations = []; + set_error_handler(static function (int $errno, string $errstr) use (&$deprecations): bool { + $deprecations[] = $errstr; + + return true; + }, \E_USER_DEPRECATED); + try { + $config = $this->process(['configurations' => ['test.process' => ['tasks' => [ + 'with_errors' => ['task' => 'transformer', 'errors' => 'error'], + 'with_error_outputs' => ['task' => 'transformer', 'error_outputs' => 'error'], + 'error' => ['task' => 'transformer'], + ]]]]); + } finally { + restore_error_handler(); + } + + self::assertSame( + ['Since cleverage/process-bundle 5.4: The "errors" option of the task "clever_age_process.configurations.test.process.tasks.with_errors" is deprecated, use "error_outputs" instead. It will be removed in v6.0.'], + $deprecations + ); + self::assertSame(['error'], $config['configurations']['test.process']['tasks']['with_errors']['errors']); + } + /** * @return iterable}> */ diff --git a/tests/Fixtures/PublicTask.php b/tests/Fixtures/PublicTask.php new file mode 100644 index 00000000..076be0dd --- /dev/null +++ b/tests/Fixtures/PublicTask.php @@ -0,0 +1,26 @@ +getTask($processConfiguration->getTaskConfiguration($code)), $code); + $taskConfiguration = $processConfiguration->getTaskConfiguration($code); + // Deprecated InputAggregatorTask, see InputAggregatorTaskTest::testTaskIsDeprecated() + $task = 'input_aggregator' === $code + ? self::ignoreDeprecations(static fn (): TaskInterface => $taskRegistry->getTask($taskConfiguration)) + : $taskRegistry->getTask($taskConfiguration); + self::assertInstanceOf(TaskInterface::class, $task, $code); } } diff --git a/tests/Task/InputAggregatorTaskTest.php b/tests/Task/InputAggregatorTaskTest.php index b5a39b40..f4cb2014 100644 --- a/tests/Task/InputAggregatorTaskTest.php +++ b/tests/Task/InputAggregatorTaskTest.php @@ -21,6 +21,7 @@ use CleverAge\ProcessBundle\Model\ProcessState; use CleverAge\ProcessBundle\Task\DummyTask; use CleverAge\ProcessBundle\Task\InputAggregatorTask; +use CleverAge\ProcessBundle\Tests\IgnoreDeprecationsTrait; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException; @@ -35,11 +36,33 @@ #[\PHPUnit\Framework\Attributes\UsesClass(ProcessState::class)] class InputAggregatorTaskTest extends TestCase { + use IgnoreDeprecationsTrait; + private const INPUT_CODES = ['branch_a' => 'a', 'branch_b' => 'b']; + public function testTaskIsDeprecated(): void + { + $deprecations = []; + set_error_handler(static function (int $errno, string $errstr) use (&$deprecations): bool { + $deprecations[] = $errstr; + + return true; + }, \E_USER_DEPRECATED); + try { + new InputAggregatorTask(); + } finally { + restore_error_handler(); + } + + self::assertSame( + ['Since cleverage/process-bundle 5.4: The "'.InputAggregatorTask::class.'" task is deprecated: it is too error-prone. It will be removed in v6.0.'], + $deprecations + ); + } + public function testTaskIsSkippedUntilEveryInputIsReceived(): void { - $task = new InputAggregatorTask(); + $task = $this->createTask(); $options = ['input_codes' => self::INPUT_CODES]; $state = $this->receive($task, $options, 'branch_a', 'A'); @@ -53,7 +76,7 @@ public function testTaskIsSkippedUntilEveryInputIsReceived(): void public function testBufferIsClearedAfterOutput(): void { - $task = new InputAggregatorTask(); + $task = $this->createTask(); $options = ['input_codes' => self::INPUT_CODES]; $this->receive($task, $options, 'branch_a', 'A1'); @@ -68,7 +91,7 @@ public function testBufferIsClearedAfterOutput(): void public function testOverriddenInputClearsBufferByDefault(): void { - $task = new InputAggregatorTask(); + $task = $this->createTask(); $options = ['input_codes' => ['branch_a' => 'a', 'branch_b' => 'b', 'branch_c' => 'c']]; $this->receive($task, $options, 'branch_a', 'A1'); @@ -85,7 +108,7 @@ public function testOverriddenInputClearsBufferByDefault(): void public function testOverriddenInputThrowsWhenCleaningIsDisabled(): void { - $task = new InputAggregatorTask(); + $task = $this->createTask(); $options = ['input_codes' => self::INPUT_CODES, 'clean_input_on_override' => false]; $this->receive($task, $options, 'branch_a', 'A1'); @@ -98,7 +121,7 @@ public function testOverriddenInputThrowsWhenCleaningIsDisabled(): void public function testKeptInputsAreReusedForNextOutputs(): void { - $task = new InputAggregatorTask(); + $task = $this->createTask(); $options = ['input_codes' => self::INPUT_CODES, 'keep_inputs' => ['a']]; $this->receive($task, $options, 'branch_a', 'A'); @@ -112,7 +135,7 @@ public function testKeptInputsAreReusedForNextOutputs(): void public function testSeveralParentsCanTargetTheSameKey(): void { - $task = new InputAggregatorTask(); + $task = $this->createTask(); $options = ['input_codes' => ['branch_a' => 'value', 'branch_b' => 'value']]; $state = $this->receive($task, $options, 'branch_b', 'B'); @@ -122,7 +145,7 @@ public function testSeveralParentsCanTargetTheSameKey(): void public function testTaskWithoutPreviousStateThrows(): void { - $task = new InputAggregatorTask(); + $task = $this->createTask(); $state = $this->createState(['input_codes' => self::INPUT_CODES]); $task->initialize($state); @@ -134,7 +157,7 @@ public function testTaskWithoutPreviousStateThrows(): void public function testUnmappedPreviousTaskThrows(): void { - $task = new InputAggregatorTask(); + $task = $this->createTask(); $this->expectException(\UnexpectedValueException::class); $this->expectExceptionMessage("Task 'unknown' is not mapped in the input_codes option"); @@ -144,7 +167,7 @@ public function testUnmappedPreviousTaskThrows(): void public function testInputCodesOptionIsRequired(): void { - $task = new InputAggregatorTask(); + $task = $this->createTask(); $this->expectException(MissingOptionsException::class); @@ -167,7 +190,7 @@ public static function invalidOptionsProvider(): iterable #[DataProvider('invalidOptionsProvider')] public function testInvalidOptionsAreRejected(array $options): void { - $task = new InputAggregatorTask(); + $task = $this->createTask(); $this->expectException(InvalidOptionsException::class); @@ -190,6 +213,11 @@ private function receive(InputAggregatorTask $task, array $options, string $prev return $state; } + private function createTask(): InputAggregatorTask + { + return self::ignoreDeprecations(static fn (): InputAggregatorTask => new InputAggregatorTask()); + } + /** * @param array $options */