diff --git a/CHANGELOG.md b/CHANGELOG.md index 27ccef87..050b40d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ Latest ------ ## Changes +* [#337](https://github.com/cleverage/process-bundle/issues/337) `StopTask` (`stop`): a process stopped on purpose is identified as stopped, not as failed. Its history is flagged as stopped (new `ProcessHistory::STATE_STOPPED`, `setStopped()`, `isStopped()`) instead of failed, the `Process stopped` log is written at the `success_level` (instead of `Process failed` at the `failed_level`), and the `cleverage_process.end` event tells it with the new `ProcessEvent::isProcessStopped()`. The process still ends normally (tasks finalized, exit code 0, parent process going on). Update documentation, add tests. * [#339](https://github.com/cleverage/process-bundle/issues/339) A shared service referenced by several tasks of the same process is rejected when the container is compiled (`InvalidArgumentException` naming the service, the tasks and the process): all these tasks ran with the same instance, initialized with the options of each task in turn, without error. Declare the task services with `shared: false` (applied by the autoconfiguration of `TaskInterface` since 5.4). Update documentation, add tests. * [#351](https://github.com/cleverage/process-bundle/issues/351) `ProcessManager` and `ProcessHelpCommand` are typed on `TaskRegistryInterface` instead of the final `TaskRegistry` class: the task registry can be decorated or replaced. Add tests. diff --git a/docs/04-advanced_workflow.md b/docs/04-advanced_workflow.md index 0c107e8e..df70a961 100644 --- a/docs/04-advanced_workflow.md +++ b/docs/04-advanced_workflow.md @@ -20,7 +20,8 @@ When a process is executed (with the `cleverage:process:execute` command or with 6. **finalizes** every task, in the order they are configured (see [finalizable tasks](02-task_types.md#finalizable-tasks)) 7. returns the last output of the `end_point` task (or `null` if there is none) and dispatches the - `cleverage_process.end` event + `cleverage_process.end` event (`ProcessEvent::isProcessStopped()` tells whether the process was stopped on purpose by + a [StopTask](reference/tasks/stop_task.md)) If an exception is thrown at any point, the `cleverage_process.fail` event is dispatched and the exception is rethrown. Note that a task error handled by the `stop` strategy does not surface as the original exception: the process manager @@ -153,7 +154,7 @@ A task may also control the flow without any error: tasks are executed, blocking tasks that already received inputs are proceeded, flushable tasks are flushed and every task is finalized. It is not a failure for the process manager: the `cleverage_process.end` event is dispatched (not `cleverage_process.fail`) and the command exits with code 0. The [StopTask](reference/tasks/stop_task.md) also flags - the process history as failed, but this does not change the event nor the exit code + the process history as stopped: the process ends as stopped on purpose, not as a failure The legacy `errors` key is a deprecated alias of `error_outputs` (defining both throws an exception). @@ -209,7 +210,7 @@ to the process code, input, context and, depending on the event, output or error | Event name | Constant | Dispatched | | ---------- | -------- | ---------- | | `cleverage_process.start` | `ProcessEvent::EVENT_PROCESS_STARTED` | on process start | -| `cleverage_process.end` | `ProcessEvent::EVENT_PROCESS_ENDED` | on successful process end (with the process output) | +| `cleverage_process.end` | `ProcessEvent::EVENT_PROCESS_ENDED` | on successful process end (with the process output), including a process stopped on purpose (`isProcessStopped()`) | | `cleverage_process.fail` | `ProcessEvent::EVENT_PROCESS_FAILED` | on failed process end (with the associated error) | Those events are also dispatched for sub-processes executed with the `ProcessExecutorTask`. diff --git a/docs/reference/01-process_definition.md b/docs/reference/01-process_definition.md index 6a1fdea7..b7e91e89 100644 --- a/docs/reference/01-process_definition.md +++ b/docs/reference/01-process_definition.md @@ -33,10 +33,10 @@ not define its own `error_strategy` (see [task definition](02-task_definition.md **logs**: optional [RFC 5424](https://datatracker.ietf.org/doc/html/rfc5424) severities of the logs written at the end of each process (channel `cleverage_process`, with the `duration` of the process in seconds in the record context): -* `success_level` (default *info*): level of the `Process succeed` log -* `failed_level` (default *debug*): level of the `Process failed` log, written when the process fails, - either with an exception (in addition to the `Critical process failure` log) or when it is stopped without exception - (e.g. by a [StopTask](tasks/stop_task.md)) +* `success_level` (default *info*): level of the `Process succeed` log, and of the + `Process stopped` log of a process stopped on purpose by a [StopTask](tasks/stop_task.md) +* `failed_level` (default *debug*): level of the `Process failed` log, written when the process fails + (in addition to the `Critical process failure` log) Without UI, the success log is the only trace of a succeeded process: lower its level (e.g. *debug*) only if processes are followed by other means. diff --git a/docs/reference/tasks/stop_task.md b/docs/reference/tasks/stop_task.md index 65ff7474..1876c178 100644 --- a/docs/reference/tasks/stop_task.md +++ b/docs/reference/tasks/stop_task.md @@ -1,9 +1,19 @@ StopTask ======== -Immediately stops the process and marks its history as failed. Unlike [DieTask](die_task.md), the stop is handled by -the process manager (no following task is executed, including remaining iterations). Useful to halt execution in an -error branch. +Stops the process on purpose: this is not a failure, but a decision of the process, e.g. not going on when the source +file does not exist. Unlike [DieTask](die_task.md), the stop is handled by the process manager: +* the stop signal goes back up to the root task: no following task is executed, including remaining iterations (other + root tasks are still executed, see [advanced workflow](../../04-advanced_workflow.md)); +* every task is finalized (e.g. a writer closes its file); +* the process ends as **stopped**: its history is flagged as stopped (`ProcessHistory::isStopped()`), the + `Process stopped` log is written (at the `success_level`, see + [process definition](../01-process_definition.md)), and the `cleverage_process.end` event is dispatched with + `ProcessEvent::isProcessStopped()` returning `true` (the UI shows the execution as *Stopped*); +* `cleverage:process:execute` exits with code 0, and the task running a sub-process + ([ProcessExecutorTask](process_executor_task.md)) goes on. + +To stop a process because of an error, let the error fail the process instead (error strategy `stop`). Task reference -------------- @@ -29,7 +39,38 @@ This task has no option. Examples -------- -* Stop the process on the first validation error +* Do not go on when the source file does not exist (`-c source:"'path/to/file.csv'"`) + +```yaml +# Task configuration level +source_path: + task: constant_output + options: + output: '{{ source }}' + outputs: [source_exists] +source_exists: + task: transformer + options: + transformers: + callback: + callback: file_exists + outputs: [check_source] +check_source: + task: filter + options: + match: + '': true + outputs: [read] + error_outputs: [stop] +stop: + task: stop +read: + task: csv_reader + options: + file_path: '{{ source }}' +``` + +* Stop the process on the first invalid item ```yaml # Task configuration level diff --git a/src/Event/ProcessEvent.php b/src/Event/ProcessEvent.php index 15ac9555..0c3ae34d 100644 --- a/src/Event/ProcessEvent.php +++ b/src/Event/ProcessEvent.php @@ -37,6 +37,7 @@ public function __construct( protected array $processContext = [], protected mixed $processOutput = null, protected ?\Throwable $processError = null, + protected bool $processStopped = false, ) { } @@ -67,4 +68,12 @@ public function getProcessError(): ?\Throwable { return $this->processError; } + + /** + * The process has ended because it was stopped on purpose (e.g. by the StopTask), see ProcessHistory::isStopped(). + */ + public function isProcessStopped(): bool + { + return $this->processStopped; + } } diff --git a/src/Manager/ProcessManager.php b/src/Manager/ProcessManager.php index b601b3b1..02ad208f 100644 --- a/src/Manager/ProcessManager.php +++ b/src/Manager/ProcessManager.php @@ -142,10 +142,13 @@ public function execute(string $processCode, mixed $input = null, array $context $this->processLogger->debug('Process start'); $result = $this->doExecute($processCode, $input, $context); + // doExecute() leaves the history of the executed process, read before restoring the one of the parent + $processStopped = $this->processHistory?->isStopped() ?? false; + $this->processHistory = $parentProcessHistory; $this->processLogger->debug('Process end'); $this->eventDispatcher->dispatch( - new ProcessEvent($processCode, $input, $context, $result), + new ProcessEvent($processCode, $input, $context, $result, null, $processStopped), ProcessEvent::EVENT_PROCESS_ENDED ); } catch (\Throwable $error) { @@ -159,8 +162,8 @@ public function execute(string $processCode, mixed $input = null, array $context throw $error; } finally { - // When the process has failed, doExecute() did not restore the history of the parent process (if any): the - // logs above are still attributed to the failed process, the next ones to the parent process + // When the process has failed, the history of the parent process (if any) is not restored yet: the logs + // above are still attributed to the failed process, the next ones to the parent process $this->processHistory = $parentProcessHistory; // The logs of the parent process are attributed to the task that has executed this process (if any) $this->taskConfiguration = $parentTaskConfiguration; @@ -176,7 +179,6 @@ public function execute(string $processCode, mixed $input = null, array $context */ protected function doExecute(string $processCode, mixed $input = null, array $context = []): mixed { - $parentProcessHistory = $this->processHistory; $processConfiguration = $this->processConfigurationRegistry->getProcessConfiguration($processCode); $processHistory = $this->initializeStates($processConfiguration, $context); $this->processHistory = $processHistory; @@ -222,7 +224,7 @@ protected function doExecute(string $processCode, mixed $input = null, array $co ->getOutput(); } } catch (\Throwable $error) { - if ($processHistory->isStarted()) { + if ($processHistory->isStarted() || $processHistory->isStopped()) { $processHistory->setFailed(); } $this->endProcess($processHistory); @@ -236,8 +238,6 @@ protected function doExecute(string $processCode, mixed $input = null, array $co } } - $this->processHistory = $parentProcessHistory; - return $returnValue; } @@ -617,6 +617,14 @@ protected function endProcess(ProcessHistory $history): void 'duration' => $history->getDuration(), ] ); + } elseif ($history->isStopped()) { + $this->processLogger->log( + $processConfiguration->getSuccessLogLevel(), + "Process {$history->getProcessCode()} stopped", + [ + 'duration' => $history->getDuration(), + ] + ); } elseif ($history->isFailed()) { $this->processLogger->log( $processConfiguration->getFailedLogLevel(), diff --git a/src/Model/ProcessHistory.php b/src/Model/ProcessHistory.php index 5cf7d050..5c57666b 100644 --- a/src/Model/ProcessHistory.php +++ b/src/Model/ProcessHistory.php @@ -28,6 +28,8 @@ class ProcessHistory implements \Stringable final public const STATE_FAILED = 'failed'; + final public const STATE_STOPPED = 'stopped'; + protected float $id; protected string $processCode; @@ -101,6 +103,15 @@ public function setFailed(): void $this->state = self::STATE_FAILED; } + /** + * Set the process as stopped on purpose (e.g. by the StopTask): it is not a failure. + */ + public function setStopped(): void + { + $this->endDate = new \DateTime(); + $this->state = self::STATE_STOPPED; + } + /** * Set the process as succeded. */ @@ -123,6 +134,11 @@ public function isFailed(): bool return self::STATE_FAILED === $this->state; } + public function isStopped(): bool + { + return self::STATE_STOPPED === $this->state; + } + /** * Get process duration in seconds. */ diff --git a/src/Task/StopTask.php b/src/Task/StopTask.php index 6d1aba2f..06ed1645 100644 --- a/src/Task/StopTask.php +++ b/src/Task/StopTask.php @@ -17,7 +17,7 @@ use CleverAge\ProcessBundle\Model\TaskInterface; /** - * Allows to directly stop a process, marking it as failed. + * Allows to directly stop a process on purpose, marking it as stopped: it is not a failure. * * @final since 5.4 */ @@ -27,6 +27,6 @@ public function execute(ProcessState $state): void { $state->setStopped(true); $state->getProcessHistory() - ->setFailed(); + ->setStopped(); } } diff --git a/tests/Manager/ProcessManagerEndLogTest.php b/tests/Manager/ProcessManagerEndLogTest.php index 3262f194..a02b8ed5 100644 --- a/tests/Manager/ProcessManagerEndLogTest.php +++ b/tests/Manager/ProcessManagerEndLogTest.php @@ -58,11 +58,18 @@ class ProcessManagerEndLogTest extends TestCase */ private array $records = []; + /** + * @var list + */ + private array $endedEvents = []; + public function testSucceededProcessIsLoggedAsInfoByDefault(): void { $this->createProcessManager('@test.constant')->execute('test.process'); self::assertSame([[LogLevel::INFO, 'Process test.process succeed']], $this->getEndRecords()); + self::assertCount(1, $this->endedEvents); + self::assertFalse($this->endedEvents[0]->isProcessStopped()); } public function testSucceededProcessUsesTheDefaultSuccessLevel(): void @@ -91,19 +98,25 @@ public function testSkippedErrorDoesNotFailTheProcess(): void self::assertSame([[LogLevel::INFO, 'Process test.process succeed']], $this->getEndRecords()); } - public function testStoppedProcessIsLoggedAsFailedAsDebugByDefault(): void + public function testStoppedProcessEndsAsStoppedAndIsLoggedAsInfoByDefault(): void { $this->createProcessManager('@test.stop')->execute('test.process'); - self::assertSame([[LogLevel::DEBUG, 'Process test.process failed']], $this->getEndRecords()); + self::assertSame([[LogLevel::INFO, 'Process test.process stopped']], $this->getEndRecords()); + self::assertCount(1, $this->endedEvents); + self::assertSame('test.process', $this->endedEvents[0]->getProcessCode()); + self::assertTrue($this->endedEvents[0]->isProcessStopped()); + self::assertNull($this->endedEvents[0]->getProcessError()); } - public function testStoppedProcessUsesTheDefaultFailedLevel(): void + public function testStoppedProcessUsesTheDefaultSuccessLevel(): void { - $this->createProcessManager('@test.stop', defaultLogLevels: ['failed_level' => LogLevel::ERROR]) - ->execute('test.process'); + $this->createProcessManager( + '@test.stop', + defaultLogLevels: ['success_level' => LogLevel::NOTICE, 'failed_level' => LogLevel::ERROR] + )->execute('test.process'); - self::assertSame([[LogLevel::ERROR, 'Process test.process failed']], $this->getEndRecords()); + self::assertSame([[LogLevel::NOTICE, 'Process test.process stopped']], $this->getEndRecords()); } public function testFailingProcessIsLoggedAsFailedWithTheLevelOfTheProcess(): void @@ -178,13 +191,18 @@ public function log($level, string|\Stringable $message, array $context = []): v } }; + $eventDispatcher = new EventDispatcher(); + $eventDispatcher->addListener(ProcessEvent::EVENT_PROCESS_ENDED, function (ProcessEvent $event): void { + $this->endedEvents[] = $event; + }); + return new ProcessManager( $container, new ProcessLogger($logger), new TaskLogger(new NullLogger()), new ProcessConfigurationRegistry(['test.process' => $rawConfiguration], 'stop', $defaultLogLevels), new ContextualOptionResolver(), - new EventDispatcher(), + $eventDispatcher, taskRegistry: new TaskRegistry($container), ); } @@ -198,7 +216,7 @@ private function getEndRecords(): array { $endRecords = []; foreach ($this->records as [$level, $message, $context]) { - if (str_ends_with($message, ' succeed') || str_ends_with($message, ' failed')) { + if (str_ends_with($message, ' succeed') || str_ends_with($message, ' failed') || str_ends_with($message, ' stopped')) { self::assertIsInt($context['duration'] ?? null); $endRecords[] = [$level, $message]; } diff --git a/tests/Manager/ProcessManagerProcessHistoryTest.php b/tests/Manager/ProcessManagerProcessHistoryTest.php index 6891f5d7..b675a56b 100644 --- a/tests/Manager/ProcessManagerProcessHistoryTest.php +++ b/tests/Manager/ProcessManagerProcessHistoryTest.php @@ -29,6 +29,7 @@ use CleverAge\ProcessBundle\Registry\ProcessConfigurationRegistry; use CleverAge\ProcessBundle\Registry\TaskRegistry; use CleverAge\ProcessBundle\Task\ConstantOutputTask; +use CleverAge\ProcessBundle\Task\StopTask; use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; use Symfony\Component\DependencyInjection\Container; @@ -50,6 +51,7 @@ #[\PHPUnit\Framework\Attributes\UsesClass(ProcessState::class)] #[\PHPUnit\Framework\Attributes\UsesClass(ProcessConfigurationRegistry::class)] #[\PHPUnit\Framework\Attributes\UsesClass(ConstantOutputTask::class)] +#[\PHPUnit\Framework\Attributes\UsesClass(StopTask::class)] class ProcessManagerProcessHistoryTest extends TestCase { private Container $container; @@ -61,10 +63,20 @@ class ProcessManagerProcessHistoryTest extends TestCase */ private mixed $observedHistory = null; + private string $subProcessCode = 'test.failing'; + + /** + * Code of each ended process => whether it was stopped. + * + * @var list + */ + private array $endedProcesses = []; + protected function setUp(): void { $this->container = new Container(); $this->container->set('test.constant', new ConstantOutputTask()); + $this->container->set('test.stop', new StopTask()); $this->container->set('test.failing', new class implements TaskInterface { public function execute(ProcessState $state): void { @@ -97,17 +109,26 @@ public function execute(ProcessState $state): void 'entry' => $this->createTaskConfiguration('@test.constant', ['output' => 'value'], ['fail']), 'fail' => $this->createTaskConfiguration('@test.failing'), ]), + 'test.stopped' => $this->createProcessConfiguration([ + 'entry' => $this->createTaskConfiguration('@test.constant', ['output' => 'value'], ['stop']), + 'stop' => $this->createTaskConfiguration('@test.stop'), + ]), ], 'stop' ); + $eventDispatcher = new EventDispatcher(); + $eventDispatcher->addListener(ProcessEvent::EVENT_PROCESS_ENDED, function (ProcessEvent $event): void { + $this->endedProcesses[] = [$event->getProcessCode(), $event->isProcessStopped()]; + }); + $this->processManager = new ProcessManager( $this->container, new ProcessLogger(new NullLogger()), new TaskLogger(new NullLogger()), $registry, new ContextualOptionResolver(), - new EventDispatcher(), + $eventDispatcher, taskRegistry: new TaskRegistry($this->container), ); } @@ -115,7 +136,7 @@ public function execute(ProcessState $state): void public function executeSubProcess(): void { try { - $this->processManager->execute('test.failing'); + $this->processManager->execute($this->subProcessCode); } catch (ProcessFailedException) { } $this->observedHistory = $this->processManager->getProcessHistory()?->getProcessCode(); @@ -129,6 +150,17 @@ public function testTheParentHistoryIsRestoredAfterAFailedSubProcess(): void self::assertNull($this->processManager->getProcessHistory()); } + public function testOnlyTheStoppedSubProcessEndsAsStopped(): void + { + $this->subProcessCode = 'test.stopped'; + + $this->processManager->execute('test.parent'); + + self::assertSame('test.parent', $this->observedHistory); + self::assertSame([['test.stopped', true], ['test.parent', false]], $this->endedProcesses); + self::assertNull($this->processManager->getProcessHistory()); + } + public function testNoHistoryIsLeftAfterAFailedProcess(): void { try { diff --git a/tests/Task/StopTaskTest.php b/tests/Task/StopTaskTest.php index a1f436c2..c0ac1680 100644 --- a/tests/Task/StopTaskTest.php +++ b/tests/Task/StopTaskTest.php @@ -25,20 +25,21 @@ #[\PHPUnit\Framework\Attributes\UsesClass(ProcessState::class)] class StopTaskTest extends TestCase { - public function testProcessIsStoppedAndMarkedAsFailed(): void + public function testProcessIsStoppedAndMarkedAsStopped(): void { $processConfiguration = new ProcessConfiguration('test', []); $state = new ProcessState($processConfiguration, new ProcessHistory($processConfiguration)); $state->setInput(['ignored']); $state->reset(false); self::assertFalse($state->isStopped()); - self::assertFalse($state->getProcessHistory()->isFailed()); + self::assertFalse($state->getProcessHistory()->isStopped()); (new StopTask())->execute($state); self::assertTrue($state->isStopped()); - self::assertTrue($state->getProcessHistory()->isFailed()); - self::assertSame(ProcessHistory::STATE_FAILED, $state->getProcessHistory()->getState()); + self::assertTrue($state->getProcessHistory()->isStopped()); + self::assertFalse($state->getProcessHistory()->isFailed()); + self::assertSame(ProcessHistory::STATE_STOPPED, $state->getProcessHistory()->getState()); self::assertNotNull($state->getProcessHistory()->getEndDate()); self::assertNull($state->getOutput()); self::assertNull($state->getException());