From 563602ac2d6585b30bc712fad2fda8f4d0f027ae Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Fri, 23 Oct 2026 16:13:26 +0200 Subject: [PATCH] #371 - ProcessManager: deprecate executing a process while it is already running (by itself or through its sub-processes), which resets the states of the running process; it will throw in 6.0 Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 3 + UPGRADE.md | 14 +- docs/04-advanced_workflow.md | 3 + docs/reference/tasks/process_executor_task.md | 7 + src/Manager/ProcessManager.php | 26 +- tests/Manager/ProcessManagerRecursionTest.php | 310 ++++++++++++++++++ 6 files changed, 361 insertions(+), 2 deletions(-) create mode 100644 tests/Manager/ProcessManagerRecursionTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f120a92..0da39796 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ Latest ------ +## Changes +* [#371](https://github.com/cleverage/process-bundle/issues/371) `ProcessManager`: executing a process while it is already running, by itself or through its sub-processes (`A → B → A` with `ProcessExecutorTask` (`process_executor`)), is deprecated and will throw an exception in 6.0: the second execution resets the states and task instances of the running process (one configuration per process code), which then returns the output of the deepest execution. The deprecation names the chain of processes; the execution goes on as before. The same sub-process executed several times in a row, or a process executed again by a listener of the process events, is not concerned. Update documentation, add tests. + ## Fixes * [#367](https://github.com/cleverage/process-bundle/issues/367) `ProcessLauncherTask` (`process_launcher`) with `json_buffering`: when the sub-process writes nothing into its JSON buffer (no `end_point`, or an output that is not an array), the task outputs nothing again, as before 5.5, instead of the path of an empty file, and the empty buffer is removed. The buffer of a failed sub-process, and those of the still running sub-processes stopped because of it, are removed too (one file was left in the temporary directory for each launch). Update documentation, add tests. * [#368](https://github.com/cleverage/process-bundle/issues/368) `YamlWriterTask` (`yaml_writer`): the parent directory is created for a local path only (no scheme, or `file://`), not for a stream wrapper (`compress.zlib://`, `s3://`...), which failed, or `php://stdout`, which created a `php:` directory. Update documentation, add tests. diff --git a/UPGRADE.md b/UPGRADE.md index aa0a4139..f7a4208a 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -3,7 +3,19 @@ Upgrade Guide ## v5.6 -Some behaviours change: check the points below. +One deprecation, and some behaviours change: check the points below. + +### Recursive process execution + +Executing a process while it is already running, by itself or through its sub-processes (`A → B → A` with the +`process_executor` task), is deprecated since 5.6 (`Since cleverage/process-bundle 5.6: Executing the process "A" while +it is already running (A -> B -> A) is deprecated...`) and will throw an exception in 6.0. A process has only one set of +task instances and states: the second execution resets them while the first one is running, which then returns the +output of the deepest execution. + +Iterate with an iterable task (e.g. a reader or `constant_iterable_output` followed by `process_executor`) instead of +recursing, or use the `process_launcher` task, which runs the process in a separate PHP process. Executing the same +sub-process several times in a row is not concerned. ### Finalization of the tasks when the process fails diff --git a/docs/04-advanced_workflow.md b/docs/04-advanced_workflow.md index e1133458..ed5b9dde 100644 --- a/docs/04-advanced_workflow.md +++ b/docs/04-advanced_workflow.md @@ -206,6 +206,9 @@ values when executing the parent process. This is a good way to split a big workflow into small, testable processes. Private processes (`public: false`) are hidden from `cleverage:process:list` (unless `--all` is used) but can still be executed. +Executing a process while it is already running, by itself or through its sub-processes, is deprecated and will throw +an exception in 6.0 (see the [ProcessExecutorTask](reference/tasks/process_executor_task.md#notes)). + ## Events Events are dispatched around process execution (see `CleverAge\ProcessBundle\Event\ProcessEvent`, which gives access diff --git a/docs/reference/tasks/process_executor_task.md b/docs/reference/tasks/process_executor_task.md index 49ca2699..9642b1cd 100644 --- a/docs/reference/tasks/process_executor_task.md +++ b/docs/reference/tasks/process_executor_task.md @@ -54,3 +54,10 @@ Notes * The `process` option is checked when the task is initialized: with an unknown process, the initialization fails, the error is logged and, with process-bundle v5, the parent process fails when the task is first executed (see [initializable tasks](../../02-task_types.md#initializable-tasks)). +* Executing a process while it is already running, by itself or through its sub-processes (`A → B → A`), is + deprecated since 5.6 and will throw an exception in 6.0: a process has only one set of task instances and states, + which the second execution resets while the first one is running (the running process then returns the output of the + deepest execution). A deprecation naming the chain of processes (`A -> B -> A`) is triggered, and the execution goes + on. Iterate with an iterable task instead of recursing, or use the [ProcessLauncherTask](process_launcher_task.md), + which runs the process in a separate PHP process. Executing the same sub-process several times in a row (e.g. for + each input) is not concerned. diff --git a/src/Manager/ProcessManager.php b/src/Manager/ProcessManager.php index 7fa9c332..e230af58 100644 --- a/src/Manager/ProcessManager.php +++ b/src/Manager/ProcessManager.php @@ -67,6 +67,14 @@ class ProcessManager implements ProcessManagerInterface, ResetInterface */ protected array $processedBlockings = []; + /** + * Codes of the processes being executed, a sub-process after its parent: a process has only one configuration (task + * instances and states), executing it again while it is running resets them. + * + * @var list + */ + protected array $runningProcessCodes = []; + protected ?ProcessHistory $processHistory = null; protected ?TaskConfiguration $taskConfiguration = null; @@ -107,6 +115,7 @@ public function reset(): void { $this->processedIterables = []; $this->processedBlockings = []; + $this->runningProcessCodes = []; $this->processHistory = null; $this->taskConfiguration = null; } @@ -141,7 +150,22 @@ public function execute(string $processCode, mixed $input = null, array $context ); $this->processLogger->debug('Process start'); - $result = $this->doExecute($processCode, $input, $context); + // @deprecated since 5.6, in v6.0 an exception will be thrown + if (\in_array($processCode, $this->runningProcessCodes, true)) { + trigger_deprecation( + 'cleverage/process-bundle', + '5.6', + 'Executing the process "%s" while it is already running (%s) is deprecated: it resets the states and task instances of the running process. In v6.0, an exception will be thrown.', + $processCode, + implode(' -> ', [...$this->runningProcessCodes, $processCode]), + ); + } + $this->runningProcessCodes[] = $processCode; + try { + $result = $this->doExecute($processCode, $input, $context); + } finally { + array_pop($this->runningProcessCodes); + } // doExecute() leaves the history of the executed process, read before restoring the one of the parent $processStopped = $this->processHistory?->isStopped() ?? false; $this->processHistory = $parentProcessHistory; diff --git a/tests/Manager/ProcessManagerRecursionTest.php b/tests/Manager/ProcessManagerRecursionTest.php new file mode 100644 index 00000000..63e0fc0b --- /dev/null +++ b/tests/Manager/ProcessManagerRecursionTest.php @@ -0,0 +1,310 @@ + + */ + private array $deprecations = []; + + protected function setUp(): void + { + $registry = new ProcessConfigurationRegistry( + [ + 'test.recursive' => $this->createProcessConfiguration([ + 'recurse' => $this->createTaskConfiguration('@test.recurse', ['process' => 'test.recursive']), + ], 'recurse'), + 'test.a' => $this->createProcessConfiguration([ + 'recurse' => $this->createTaskConfiguration('@test.recurse', ['process' => 'test.b']), + ], 'recurse'), + 'test.b' => $this->createProcessConfiguration([ + 'recurse' => $this->createTaskConfiguration('@test.recurse', ['process' => 'test.a']), + ], 'recurse'), + 'test.parent' => $this->createProcessConfiguration([ + 'iterate' => $this->createTaskConfiguration('@test.parent.iterate', ['output' => [1, 2, 3]], ['execute']), + 'execute' => $this->createTaskConfiguration('@test.parent.execute', ['process' => 'test.child'], ['aggregate']), + 'aggregate' => $this->createTaskConfiguration('@test.parent.aggregate'), + ], 'aggregate'), + 'test.child' => $this->createProcessConfiguration([ + 'echo' => $this->createTaskConfiguration('@test.child.echo'), + ], 'echo', 'echo'), + 'test.failing' => $this->createProcessConfiguration([ + 'fail' => $this->createTaskConfiguration('@test.failing.fail'), + ]), + 'test.stopped' => $this->createProcessConfiguration([ + 'stop' => $this->createTaskConfiguration('@test.stopped.stop'), + ]), + ], + 'stop' + ); + + $container = new Container(); + $this->eventDispatcher = new EventDispatcher(); + $this->processManager = new ProcessManager( + $container, + new ProcessLogger(new NullLogger()), + new TaskLogger(new NullLogger()), + $registry, + new ContextualOptionResolver(), + $this->eventDispatcher, + taskRegistry: new TaskRegistry($container), + ); + + // Executes the given process while recursions are allowed, outputs the number of nested executions + $container->set('test.recurse', new class($this) implements TaskInterface { + public function __construct( + private readonly ProcessManagerRecursionTest $test, + ) { + } + + public function execute(ProcessState $state): void + { + $state->setOutput($this->test->recurse((string) $state->getTaskConfiguration()->getOption('process'))); + } + }); + $container->set('test.parent.iterate', new ConstantIterableOutputTask()); + $container->set('test.parent.execute', new ProcessExecutorTask($this->processManager, $registry, new NullLogger())); + $container->set('test.parent.aggregate', new AggregateIterableTask()); + $container->set('test.child.echo', new class implements TaskInterface { + public function execute(ProcessState $state): void + { + $state->setOutput("child {$state->getInput()}"); + } + }); + $container->set('test.failing.fail', new class implements TaskInterface { + public function execute(ProcessState $state): void + { + throw new \RuntimeException('Failure'); + } + }); + $container->set('test.stopped.stop', new StopTask()); + + set_error_handler(function (int $errno, string $errstr): bool { + $this->deprecations[] = $errstr; + + return true; + }, \E_USER_DEPRECATED); + } + + protected function tearDown(): void + { + restore_error_handler(); + } + + public function recurse(string $processCode): int + { + if ($this->recursions <= 0) { + return 0; + } + --$this->recursions; + + return 1 + (int) $this->processManager->execute($processCode); + } + + public function testAProcessExecutingItselfIsDeprecated(): void + { + $this->recursions = 2; + + // The execution goes on as before: the states of the running process are reset, its output is the deepest one + self::assertSame(0, $this->processManager->execute('test.recursive')); + self::assertSame(0, $this->recursions); + self::assertSame([ + $this->getExpectedDeprecation('test.recursive', 'test.recursive -> test.recursive'), + $this->getExpectedDeprecation('test.recursive', 'test.recursive -> test.recursive -> test.recursive'), + ], $this->deprecations); + } + + public function testAProcessExecutedAgainThroughASubProcessIsDeprecated(): void + { + $this->recursions = 2; + + self::assertSame(0, $this->processManager->execute('test.a')); + self::assertSame(0, $this->recursions); + self::assertSame([ + $this->getExpectedDeprecation('test.a', 'test.a -> test.b -> test.a'), + ], $this->deprecations); + } + + public function testTheSameSubProcessExecutedSeveralTimesInARowIsNotDeprecated(): void + { + self::assertSame(['child 1', 'child 2', 'child 3'], $this->processManager->execute('test.parent')); + self::assertSame([], $this->deprecations); + } + + /** + * @return iterable + */ + public static function provideListenerEvents(): iterable + { + yield 'start' => [ProcessEvent::EVENT_PROCESS_STARTED, 'test.child']; + yield 'end' => [ProcessEvent::EVENT_PROCESS_ENDED, 'test.child']; + yield 'fail' => [ProcessEvent::EVENT_PROCESS_FAILED, 'test.failing']; + } + + #[DataProvider('provideListenerEvents')] + public function testAListenerExecutingTheProcessAgainIsNotDeprecated(string $eventName, string $processCode): void + { + $executions = 0; + $this->eventDispatcher->addListener($eventName, function () use ($processCode, &$executions): void { + if ($executions++ > 0) { + return; + } + try { + $this->processManager->execute($processCode, 0); + } catch (ProcessFailedException) { + } + }); + + try { + $this->processManager->execute($processCode, 0); + } catch (ProcessFailedException) { + } + + self::assertSame(2, $executions); + self::assertSame([], $this->deprecations); + } + + public function testAProcessExecutedAgainAfterAFailureIsNotDeprecated(): void + { + for ($i = 0; $i < 2; ++$i) { + try { + $this->processManager->execute('test.failing'); + self::fail('No exception thrown'); + } catch (ProcessFailedException $exception) { + self::assertSame('Failure', $exception->getPrevious()?->getMessage()); + } + } + self::assertSame([], $this->deprecations); + } + + public function testAProcessExecutedAgainAfterAStopIsNotDeprecated(): void + { + self::assertNull($this->processManager->execute('test.stopped')); + self::assertNull($this->processManager->execute('test.stopped')); + self::assertSame([], $this->deprecations); + } + + public function testResetForgetsTheRunningProcesses(): void + { + // As if the execution of a process had been interrupted + (new \ReflectionProperty(ProcessManager::class, 'runningProcessCodes'))->setValue($this->processManager, ['test.child']); + $this->processManager->reset(); + + self::assertSame('child 0', $this->processManager->execute('test.child', 0)); + self::assertSame([], $this->deprecations); + } + + private function getExpectedDeprecation(string $processCode, string $chain): string + { + return "Since cleverage/process-bundle 5.6: Executing the process \"{$processCode}\" while it is already running ({$chain}) is deprecated: it resets the states and task instances of the running process. In v6.0, an exception will be thrown."; + } + + /** + * @param array> $tasks + * + * @return array + */ + private function createProcessConfiguration(array $tasks, ?string $endPoint = null, ?string $entryPoint = null): array + { + return [ + 'options' => [], + 'entry_point' => $entryPoint, + 'end_point' => $endPoint, + 'description' => '', + 'help' => '', + 'public' => true, + 'tasks' => $tasks, + ]; + } + + /** + * @param array $options + * @param list $outputs + * + * @return array + */ + private function createTaskConfiguration(string $service, array $options = [], array $outputs = []): array + { + return [ + 'service' => $service, + 'options' => $options, + 'description' => '', + 'help' => '', + 'outputs' => $outputs, + 'errors' => [], + 'error_outputs' => [], + 'error_strategy' => null, + 'log_level' => null, + ]; + } +}