diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e55a44b..0f120a92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,14 @@ Latest ------ +## 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. +* [#369](https://github.com/cleverage/process-bundle/issues/369) `StopTask` (`stop`): a process that has already failed (e.g. after a task initialization failure, going on being deprecated) stays failed instead of being flagged as stopped: the `Process failed` log is written at the `failed_level` and `ProcessEvent::isProcessStopped()` is `false`, as before #337. Update documentation, add tests. +* [#370](https://github.com/cleverage/process-bundle/issues/370) `RulesTransformer` (`rules`): the result of a `condition` is cast to `bool`, so a condition returning an `int` (`preg_match()`), a string, `null` or any other non-boolean value (a single variable) no longer throws a `TypeError`: the rule matches if the result is truthy. Update documentation, add tests. +* [#372](https://github.com/cleverage/process-bundle/issues/372) `ProcessConfigurationRegistry`: an unknown task in the `outputs` or `error_outputs` of a task, or in the `entry_point` or `end_point` of a process, throws an `InvalidProcessConfigurationException` naming the process, the task and the key holding the reference, instead of a `MissingTaskConfigurationException` (`No task configuration with code : `). A process whose configuration is not valid is left out of `getProcessConfigurations()` (`cleverage:process:list`, lists of the UI) with an error log on the `cleverage_process` channel, instead of making every listing fail; getting or executing this process still throws. Update documentation, add tests. +* [#373](https://github.com/cleverage/process-bundle/issues/373) `ProcessManager`: the tasks are finalized (`FinalizableTaskInterface::finalize()`) when the process fails too, so that a long-running worker (Messenger, UI bundle) no longer keeps the resources of a failed process open (files of the CSV, JSON stream and XML tasks, cursor of `doctrine_database_reader` blocking the next queries with MySQL unbuffered queries). Only the initialized tasks are finalized, each one once; a finalization failure is logged as critical and the process fails with its original exception. `StatCounterTask` (`stat_counter`) logs its count when the process fails too. Update documentation, add tests. + v5.5 ----- diff --git a/UPGRADE.md b/UPGRADE.md index 1cd439a9..8b24d68e 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,6 +1,19 @@ Upgrade Guide ============= +## v5.5.1 + +Some behaviours change: check the points below. + +### Finalization of the tasks when the process fails + +`FinalizableTaskInterface::finalize()` is now also called when the process fails, on every task that has been +initialized (not on a task whose `initialize()` failed, nor on the tasks not reached). A custom finalizable task must not +assume that the process succeeded nor that its data are complete: only release its resources (close files, free +cursors), do not write a footer, move / publish a partial file or run long operations. Check +`$state->getProcessHistory()->isFailed()` in `finalize()` to skip such work when the process has failed. +`StatCounterTask` (`stat_counter`) now logs its count when the process fails too. + ## v5.5 No deprecation in this version. Some behaviours change: check the points below. diff --git a/config/services/registry.yaml b/config/services/registry.yaml index e476e992..b8c70fbb 100644 --- a/config/services/registry.yaml +++ b/config/services/registry.yaml @@ -6,8 +6,10 @@ services: - ~ - ~ - ~ + - '@logger' # Processes left out of the listings because their configuration is not valid tags: - { name: kernel.reset, method: reset } + - { name: monolog.logger, channel: cleverage_process } CleverAge\ProcessBundle\Registry\ProcessConfigurationRegistry: alias: cleverage_process.registry.process_configuration CleverAge\ProcessBundle\Registry\ProcessConfigurationRegistryInterface: diff --git a/docs/02-task_types.md b/docs/02-task_types.md index fc40b2c4..2578e90d 100644 --- a/docs/02-task_types.md +++ b/docs/02-task_types.md @@ -24,7 +24,7 @@ A task is *resolved* when all its executions are over, as well as those of all i | `BlockingTaskInterface` | `proceed(ProcessState $state): void` | Once, when all parent tasks are resolved, only if the task received at least one input | | `FlushableTaskInterface` | `flush(ProcessState $state): void` | When an ancestor (or the task itself) is resolved or finishes iterating: possibly several times | | `InitializableTaskInterface` | `initialize(ProcessState $state): void` | Once, before the process starts | -| `FinalizableTaskInterface` | `finalize(ProcessState $state): void` | Once, at the end of the process | +| `FinalizableTaskInterface` | `finalize(ProcessState $state): void` | Once, at the end of the process, whether it succeeds or fails | ## Iterable tasks @@ -162,9 +162,20 @@ temporary folder, log some statistics). Finalizable tasks implement `CleverAge\ProcessBundle\Model\FinalizableTaskInterface`. Their `finalize` method is called once all tasks are resolved, for every task of the process, in the order they are configured. -Note that finalization only happens when the process reaches its end: if a task stops the process because of an -exception (error strategy `stop`), the process fails immediately and `finalize` is not called. A process stopped -without exception (e.g. by the [StopTask](reference/tasks/stop_task.md)) is still finalized. +A process stopped without exception (e.g. by the [StopTask](reference/tasks/stop_task.md)) is finalized the same way. + +`finalize` is also called when the process fails (e.g. a task stops the process because of an exception with the error +strategy `stop`), so that the resources are released (files, database cursors...), e.g. for the next executions of a +long-running worker (Messenger, UI). In this case: +* only the tasks that have been initialized are finalized: not the tasks whose `initialize` has failed, nor the tasks + that were not reached because the process failed during the initialization; +* the tasks are finalized after the failure, in the order they are configured: `finalize` must not assume that the + data are complete (e.g. a writer must only close its file, not write a footer or move a partial file to its + destination), nor take long: `$state->getProcessHistory()->isFailed()` tells whether the process has failed; +* an exception thrown by `finalize` is logged as critical and the next tasks are still finalized: the process fails + with its original exception. + +Each task is finalized only once: a task already finalized when the process fails is not finalized again. Examples of finalizable tasks: [CsvReaderTask](reference/tasks/csv_reader_task.md) and the other CSV tasks (close the file), [StatCounterTask](reference/tasks/stat_counter_task.md) (logs the number of processed items). diff --git a/docs/04-advanced_workflow.md b/docs/04-advanced_workflow.md index 24d6e848..e1133458 100644 --- a/docs/04-advanced_workflow.md +++ b/docs/04-advanced_workflow.md @@ -23,7 +23,9 @@ When a process is executed (with the `cleverage:process:execute` command or with `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. +If an exception is thrown at any point, the tasks that have been initialized (and not finalized yet) are finalized, +the `cleverage_process.fail` event is dispatched and the exception is rethrown (see +[finalizable tasks](02-task_types.md#finalizable-tasks)). Note that a task error handled by the `stop` strategy does not surface as the original exception: the process manager throws a `CleverAge\ProcessBundle\Exception\ProcessFailedException` (a `\RuntimeException`), whose message contains the process code, the task code and the original message. The original exception is available with `getPrevious()`. @@ -262,7 +264,7 @@ When a checker returns `true`, the task is not executed and the process is stopp `stop` error strategy, whatever its `error_strategy` (its error outputs are not executed): the process fails with a `ProcessFailedException` whose previous exception is a `CleverAge\ProcessBundle\Exception\ProcessStoppedException`, and the `cleverage_process.fail` event is dispatched. As for any failure, the remaining items are not flushed and the -tasks are not finalized. +initialized tasks are finalized. ```php namespace App\Process; @@ -316,7 +318,8 @@ PHP executes a process in a single thread. To use several CPU cores, the of its `end_point` is written to a JSON stream file whose path is outputted by the task once the sub-process is over (use a [JsonStreamReaderTask](reference/tasks/json_stream_reader_task.md) to read it back). The file is only written if the sub-process has an `end_point` whose last output is an array; otherwise, as when `json_buffering` is disabled, - the task outputs nothing + the task outputs nothing (and removes the empty file). The task does not remove the files it outputs: remove them + once read - if a sub-process fails (non-zero exit code), all the running sub-processes are stopped and the task throws an exception diff --git a/docs/reference/01-process_definition.md b/docs/reference/01-process_definition.md index b7e91e89..a1911a3c 100644 --- a/docs/reference/01-process_definition.md +++ b/docs/reference/01-process_definition.md @@ -74,6 +74,18 @@ reads its `ui` key to configure the launch form. **tasks**: list of task definitions contained in the process, indexed by task code. See [task definition](02-task_definition.md). +### Invalid task references + +The task codes referenced by the `entry_point` and the `end_point` of the process, and by the `outputs` and the +`error_outputs` of its tasks, must be tasks of the process. They are not checked when the container is compiled, but +when the process configuration is resolved: getting or executing the process throws an +`InvalidProcessConfigurationException` naming the process, the unknown task and where it is referenced (e.g. +`Unknown task 'missing' in the 'outputs' of the task 'read' (from process: app.import)`). + +Such a process, as any process whose configuration is not valid, is left out of the lists of processes +(`cleverage:process:list`, `ProcessConfigurationRegistryInterface::getProcessConfigurations()` used e.g. by the UI), +with an error log on the `cleverage_process` channel, instead of making them fail. + ### Main branch Only the *main branch* of the process is executed: the group of connected tasks containing the `entry_point`, or else diff --git a/docs/reference/tasks/process_launcher_task.md b/docs/reference/tasks/process_launcher_task.md index 347c2cf4..f0785e1b 100644 --- a/docs/reference/tasks/process_launcher_task.md +++ b/docs/reference/tasks/process_launcher_task.md @@ -26,8 +26,9 @@ Possible outputs Nothing is output when an input is received (the task is skipped). When `json_buffering` is enabled, the sub-process end point output (only if it is an array) is written to a JSON stream file of the system temporary directory (`/cdm_buffer_*.json-stream`, in `var/` of the project before 5.4) and, once the sub-process is -finished, the task outputs the path of this file (`string`), during the next iterations or when flushed. Without -`json_buffering`, the task outputs nothing. +finished, the task outputs the path of this file (`string`), during the next iterations or when flushed. If nothing +was written (no `end_point`, or an output that is not an array), the empty file is removed and the task outputs +nothing for this sub-process. Without `json_buffering`, the task outputs nothing. If a sub-process exits with a non-zero code, its error output is logged as critical, all running sub-processes are stopped and a `RuntimeException` is thrown. @@ -69,5 +70,7 @@ Notes * Sub-processes are run with the console of the project (`%kernel.project_dir%/bin/console`) and the same Symfony environment as the current one (`--env`), the arguments of the service. -* The JSON stream files are not removed by the task: remove them once read (e.g. with a [file remover](file_remover_task.md) task). +* The JSON stream files output by the task are not removed by it: remove them once read (e.g. with a + [file remover](file_remover_task.md) task). The task only removes the files it does not output: empty ones, and + those of a failed sub-process and of the still running sub-processes stopped because of it. * The incremental error output (stderr) of the running sub-processes is echoed directly. diff --git a/docs/reference/tasks/stat_counter_task.md b/docs/reference/tasks/stat_counter_task.md index 70b002f3..f6f115df 100644 --- a/docs/reference/tasks/stat_counter_task.md +++ b/docs/reference/tasks/stat_counter_task.md @@ -20,7 +20,7 @@ Possible outputs `any`: the input, unchanged. -At finalization, the message `Processed item count: ` is logged. +At finalization, the message `Processed item count: ` is logged, also when the process fails. Options ------- diff --git a/docs/reference/tasks/stop_task.md b/docs/reference/tasks/stop_task.md index 1876c178..05c90a1c 100644 --- a/docs/reference/tasks/stop_task.md +++ b/docs/reference/tasks/stop_task.md @@ -13,6 +13,10 @@ file does not exist. Unlike [DieTask](die_task.md), the stop is handled by the p * `cleverage:process:execute` exits with code 0, and the task running a sub-process ([ProcessExecutorTask](process_executor_task.md)) goes on. +A process that has already failed (e.g. a task whose initialization failed, see the +[initializable tasks](../../02-task_types.md)) stays **failed**: the `StopTask` still stops the flow of the +tasks, but it does not turn the failure into a stop. + To stop a process because of an error, let the error fail the process instead (error strategy `stop`). Task reference diff --git a/docs/reference/tasks/yaml_writer_task.md b/docs/reference/tasks/yaml_writer_task.md index c911428c..7a604a98 100644 --- a/docs/reference/tasks/yaml_writer_task.md +++ b/docs/reference/tasks/yaml_writer_task.md @@ -24,7 +24,7 @@ Options | Code | Type | Required | Default | Description | |-------------|----------|:--------:|---------|------------------------------------------------------------------------| -| `file_path` | `string` | **X** | | Path of the file to write to. Its parent directory is created if needed | +| `file_path` | `string` | **X** | | Path of the file to write to. Its parent directory is created if needed (local path only) | | `inline` | `int` | | `4` | Level at which the dumper switches to inline YAML (see `Yaml::dump()`) | Examples @@ -44,4 +44,6 @@ Notes * This task is not blocking: when receiving several inputs, each one overwrites the file. Aggregate the data first (e.g. with [AggregateIterableTask](aggregate_iterable_task.md)) if needed. +* The parent directory is created only for a local path (no scheme, or `file://`): a stream wrapper path + (`compress.zlib://`, `s3://`, `php://stdout`...) is written as is, its directory is never created. * When the file cannot be written (e.g. not writable directory), the task fails with a `RuntimeException`. diff --git a/docs/reference/transformers/rules_transformer.md b/docs/reference/transformers/rules_transformer.md index 04589890..b4c8f46f 100644 --- a/docs/reference/transformers/rules_transformer.md +++ b/docs/reference/transformers/rules_transformer.md @@ -39,7 +39,7 @@ Each rule of `rules_set` has the following options: | Code | Type | Required | Default | Description | |----------------|----------------|:--------:|---------|---------------------------------------------------------------------------------------------------------------------------------------| -| `condition` | `string\|ParsedExpression\|null` | | `null` | Expression (or an already parsed `Symfony\Component\ExpressionLanguage\ParsedExpression`); the rule matches if it is truthy | +| `condition` | `string\|ParsedExpression\|null` | | `null` | Expression (or an already parsed `Symfony\Component\ExpressionLanguage\ParsedExpression`); the rule matches if its result is truthy (cast to `bool`: `1` of `preg_match()` matches, `0`, `''`, `null` do not) | | `default` | `bool` | | `false` | Mark the rule as the default one (always matches). It cannot have a `condition`, no conditional rule may follow it, only one allowed | | `set_null` | `bool` | | `false` | If `true`, return `null` (takes precedence on `constant` and `transformers`) | | `constant` | `any` | | `null` | If not `null`, return this value (takes precedence on `transformers`) | @@ -88,4 +88,6 @@ Notes ----- Conditions are parsed with the bundle's `cleverage_process.expression_language` service, which also exposes the PHP -`preg_match` function. +`preg_match` function. The result of a condition does not need to be a boolean: it is cast to `bool`, so a +`preg_match('/^foo/', value)` (`1` or `0`), a `matches` expression or a single variable (`value`) can be used as is. +Beware that PHP truthiness applies: the strings `"0"` and `""`, `0`, `[]` and `null` do not match. diff --git a/src/Exception/InvalidProcessConfigurationException.php b/src/Exception/InvalidProcessConfigurationException.php index 3366fe24..8ad2f891 100644 --- a/src/Exception/InvalidProcessConfigurationException.php +++ b/src/Exception/InvalidProcessConfigurationException.php @@ -44,4 +44,19 @@ public static function createEntryPointHasAncestors( "The entry-point '{$taskConfig->getCode()}' cannot have an ancestor (from process: {$processConfiguration->getCode()})" ); } + + /** + * A task code referenced by the "outputs" or "error_outputs" of a task, or by the "entry_point" or "end_point" of + * the process, is not a task of the process. + */ + public static function createUnknownTaskReference( + string $processCode, + string $key, + string $unknownTaskCode, + ?string $taskCode = null, + ): self { + $holder = null === $taskCode ? "the '{$key}'" : "the '{$key}' of the task '{$taskCode}'"; + + return new self("Unknown task '{$unknownTaskCode}' in {$holder} (from process: {$processCode})"); + } } diff --git a/src/Manager/ProcessManager.php b/src/Manager/ProcessManager.php index 02ad208f..7fa9c332 100644 --- a/src/Manager/ProcessManager.php +++ b/src/Manager/ProcessManager.php @@ -183,12 +183,18 @@ protected function doExecute(string $processCode, mixed $input = null, array $co $processHistory = $this->initializeStates($processConfiguration, $context); $this->processHistory = $processHistory; + // Finalized if the process fails: the initialized tasks that are not finalized yet (indexed by spl_object_id()) + $tasksToFinalize = []; try { $this->checkProcess($processConfiguration); // First initialize the whole stack in a linear way, tasks are initialized in the order they are configured foreach ($processConfiguration->getTaskConfigurations() as $taskConfiguration) { $this->initialize($taskConfiguration); + // A task whose initialization has failed is stopped: it is not finalized if the process fails + if (!$taskConfiguration->getState()->isStopped()) { + $tasksToFinalize[spl_object_id($taskConfiguration)] = $taskConfiguration; + } } // If defined, set the input of a task @@ -211,6 +217,8 @@ protected function doExecute(string $processCode, mixed $input = null, array $co // Finalize the process in a linear way foreach ($processConfiguration->getTaskConfigurations() as $taskConfiguration) { + // Never finalized twice, even if the process fails after this point + unset($tasksToFinalize[spl_object_id($taskConfiguration)]); $this->finalize($taskConfiguration); } @@ -224,9 +232,16 @@ protected function doExecute(string $processCode, mixed $input = null, array $co ->getOutput(); } } catch (\Throwable $error) { + // Before the finalization: the tasks can tell the process has failed (ProcessHistory::isFailed()) if ($processHistory->isStarted() || $processHistory->isStopped()) { $processHistory->setFailed(); } + + // Release the resources of the tasks (files, database cursors...), e.g. for the next executions of a + // long-running worker. A finalization failure is logged by finalize(), and does not hide the original error + foreach ($tasksToFinalize as $taskConfiguration) { + $this->finalize($taskConfiguration); + } $this->endProcess($processHistory); throw $error; diff --git a/src/Model/SubprocessInstance.php b/src/Model/SubprocessInstance.php index b45f32ea..c7df6296 100644 --- a/src/Model/SubprocessInstance.php +++ b/src/Model/SubprocessInstance.php @@ -175,14 +175,36 @@ public function getContext(): array return $this->context; } + /** + * Path of the JSON buffer once the sub-process is terminated, null if nothing was written into it (no end point, + * output that is not an array): the empty buffer is then removed. + */ public function getResult(): ?string { $fs = new Filesystem(); - if ($this->process->isTerminated() && $fs->exists($this->bufferPath)) { - return $this->bufferPath; + if (!$this->process->isTerminated() || !$fs->exists($this->bufferPath)) { + return null; + } + + // Created before the launch, the buffer exists even when the sub-process wrote nothing into it + clearstatcache(true, $this->bufferPath); + if (0 === filesize($this->bufferPath)) { + $this->removeBuffer(); + + return null; } - return null; + return $this->bufferPath; + } + + /** + * Remove the JSON buffer, e.g. when the sub-process has failed or was stopped: it is never output. + */ + public function removeBuffer(): void + { + if ('' !== $this->bufferPath) { + (new Filesystem())->remove($this->bufferPath); + } } /** diff --git a/src/Registry/ProcessConfigurationRegistry.php b/src/Registry/ProcessConfigurationRegistry.php index 73b067b3..f6a82d08 100644 --- a/src/Registry/ProcessConfigurationRegistry.php +++ b/src/Registry/ProcessConfigurationRegistry.php @@ -18,6 +18,7 @@ use CleverAge\ProcessBundle\Exception\InvalidProcessConfigurationException; use CleverAge\ProcessBundle\Exception\LogicException; use CleverAge\ProcessBundle\Exception\MissingProcessException; +use Psr\Log\LoggerInterface; use Psr\Log\LogLevel; /** @@ -40,6 +41,7 @@ public function __construct( protected array $rawConfiguration, protected string $defaultErrorStrategy, protected array $defaultLogLevels = [], + protected ?LoggerInterface $logger = null, ) { } @@ -54,15 +56,29 @@ public function getProcessConfiguration(string $processCode): ProcessConfigurati } /** + * The processes whose configuration is not valid are left out (with an error log): they do not prevent listing the + * other ones, getProcessConfiguration() throws when such a process is requested (e.g. to be executed). + * * @return array */ public function getProcessConfigurations(): array { + $processConfigurations = []; foreach (array_keys($this->rawConfiguration) as $processCode) { - $this->resolveConfiguration($processCode); + try { + $this->resolveConfiguration($processCode); + } catch (InvalidProcessConfigurationException|LogicException $exception) { + $this->logger?->error( + "The process {$processCode} is left out of the process list, its configuration is not valid: {$exception->getMessage()}", + ['process_code' => $processCode, 'exception' => $exception] + ); + + continue; + } + $processConfigurations[$processCode] = $this->processConfigurations[$processCode]; } - return $this->processConfigurations; + return $processConfigurations; } /** @@ -87,6 +103,8 @@ protected function resolveConfiguration(string $processCode): void $rawProcessConfiguration = $this->rawConfiguration[$processCode]; /** @var TaskConfiguration[] $taskConfigurations */ $taskConfigurations = []; + /** @var array $errorOutputsKeys */ + $errorOutputsKeys = []; foreach ($rawProcessConfiguration['tasks'] as $taskCode => $rawTaskConfiguration) { if ((is_countable($rawTaskConfiguration['errors']) ? \count($rawTaskConfiguration['errors']) : 0) > 0) { if ((is_countable($rawTaskConfiguration['error_outputs']) ? \count( @@ -97,6 +115,7 @@ protected function resolveConfiguration(string $processCode): void throw new LogicException($m); } $rawTaskConfiguration['error_outputs'] = $rawTaskConfiguration['errors']; + $errorOutputsKeys[$taskCode] = 'errors'; } if (!\is_string($rawTaskConfiguration['service'] ?? null)) { // The "task" codes are resolved into services when the container is compiled (TaskLocatorCompilerPass) @@ -128,6 +147,9 @@ protected function resolveConfiguration(string $processCode): void $rawProcessConfiguration['logs']['failed_level'] ?? $this->defaultLogLevels['failed_level'] ?? LogLevel::DEBUG ); + // Check the task references, the links below would fail with a message naming neither the process nor the task + $this->checkTaskReferences($processCode, $rawProcessConfiguration, $taskConfigurations, $errorOutputsKeys); + // Set links between tasks foreach ($taskConfigurations as $taskConfig) { foreach ($taskConfig->getOutputs() as $nextTaskCode) { @@ -166,6 +188,37 @@ protected function resolveConfiguration(string $processCode): void $this->processConfigurations[$processCode] = $processConfig; } + /** + * @param array $rawProcessConfiguration + * @param TaskConfiguration[] $taskConfigurations indexed by task code + * @param array $errorOutputsKeys "errors" for the tasks using this deprecated key + */ + protected function checkTaskReferences( + string $processCode, + array $rawProcessConfiguration, + array $taskConfigurations, + array $errorOutputsKeys, + ): void { + foreach ($taskConfigurations as $taskCode => $taskConfig) { + foreach ($taskConfig->getOutputs() as $nextTaskCode) { + if (!\array_key_exists($nextTaskCode, $taskConfigurations)) { + throw InvalidProcessConfigurationException::createUnknownTaskReference($processCode, 'outputs', $nextTaskCode, $taskCode); + } + } + foreach ($taskConfig->getErrorOutputs() as $errorTaskCode) { + if (!\array_key_exists($errorTaskCode, $taskConfigurations)) { + throw InvalidProcessConfigurationException::createUnknownTaskReference($processCode, $errorOutputsKeys[$taskCode] ?? 'error_outputs', $errorTaskCode, $taskCode); + } + } + } + foreach (['entry_point', 'end_point'] as $key) { + $referencedTaskCode = $rawProcessConfiguration[$key] ?? null; + if (\is_string($referencedTaskCode) && !\array_key_exists($referencedTaskCode, $taskConfigurations)) { + throw InvalidProcessConfigurationException::createUnknownTaskReference($processCode, $key, $referencedTaskCode); + } + } + } + protected function markErrorBranch(TaskConfiguration $taskConfig, bool $isErrorBranch = true): void { if ($taskConfig->isInErrorBranch() !== $isErrorBranch) { diff --git a/src/Registry/ProcessConfigurationRegistryInterface.php b/src/Registry/ProcessConfigurationRegistryInterface.php index d727d8cd..42d4a638 100644 --- a/src/Registry/ProcessConfigurationRegistryInterface.php +++ b/src/Registry/ProcessConfigurationRegistryInterface.php @@ -29,9 +29,10 @@ interface ProcessConfigurationRegistryInterface public function getProcessConfiguration(string $processCode): ProcessConfiguration; /** - * @return array indexed by process code + * The processes whose configuration is not valid are left out (ProcessConfigurationRegistry logs an error), they + * do not prevent listing the other ones. * - * @throws InvalidProcessConfigurationException when a process configuration is not valid + * @return array indexed by process code */ public function getProcessConfigurations(): array; diff --git a/src/Task/File/Yaml/YamlWriterTask.php b/src/Task/File/Yaml/YamlWriterTask.php index 92d1751d..ba87cdb4 100644 --- a/src/Task/File/Yaml/YamlWriterTask.php +++ b/src/Task/File/Yaml/YamlWriterTask.php @@ -30,10 +30,13 @@ public function execute(ProcessState $state): void { $options = $this->getOptions($state); $filePath = $options['file_path']; - // Same as the other writers: the parent directory is created if needed - $dirname = \dirname($filePath); - if (!@mkdir($dirname, 0o755, true) && !is_dir($dirname)) { - throw new RuntimeException(\sprintf('Directory "%s" was not created', $dirname)); + // Same as the other writers: the parent directory is created if needed, for a local path only (not for a stream + // wrapper such as compress.zlib://, s3:// or php://stdout) + if (!preg_match('#^[a-z][a-z0-9+.-]*://#i', $filePath) || str_starts_with(strtolower($filePath), 'file://')) { + $dirname = \dirname($filePath); + if (!@mkdir($dirname, 0o755, true) && !is_dir($dirname)) { + throw new RuntimeException(\sprintf('Directory "%s" was not created', $dirname)); + } } // The PHP warning is replaced by the exception below, also outside of the debug mode if (false === @file_put_contents($filePath, Yaml::dump($state->getInput(), $options['inline']))) { diff --git a/src/Task/Process/ProcessLauncherTask.php b/src/Task/Process/ProcessLauncherTask.php index a5a3267a..6840ce2e 100644 --- a/src/Task/Process/ProcessLauncherTask.php +++ b/src/Task/Process/ProcessLauncherTask.php @@ -202,6 +202,7 @@ protected function handleProcesses(ProcessState $state): void unset($this->launchedProcesses[$key]); if (0 !== $process->getProcess()->getExitCode()) { + $process->removeBuffer(); $this->logger->critical($process->getProcess()->getErrorOutput(), $logContext); $this->killProcesses(); @@ -272,7 +273,12 @@ static function (Options $options, array $value): array { protected function killProcesses(): void { foreach ($this->launchedProcesses as $process) { + // An already terminated sub-process keeps its buffer: it is handled normally at the next call + $running = !$process->getProcess()->isTerminated(); $process->stop(5); + if ($running) { + $process->removeBuffer(); + } } } } diff --git a/src/Task/StopTask.php b/src/Task/StopTask.php index 06ed1645..cad6fef0 100644 --- a/src/Task/StopTask.php +++ b/src/Task/StopTask.php @@ -26,7 +26,10 @@ class StopTask implements TaskInterface public function execute(ProcessState $state): void { $state->setStopped(true); - $state->getProcessHistory() - ->setStopped(); + $processHistory = $state->getProcessHistory(); + // A process already failed (e.g. by a task initialization failure) stays failed + if (!$processHistory->isFailed()) { + $processHistory->setStopped(); + } } } diff --git a/src/Transformer/RulesTransformer.php b/src/Transformer/RulesTransformer.php index 6c59aa23..583338f9 100644 --- a/src/Transformer/RulesTransformer.php +++ b/src/Transformer/RulesTransformer.php @@ -145,7 +145,7 @@ protected function matchRule(mixed $value, array $rule, bool $useValueAsVariable 'value' => $value, ]; - return $this->language->evaluate($rule['condition'], $expressionValues); + return (bool) $this->language->evaluate($rule['condition'], $expressionValues); } /* @noinspection PhpStrictTypeCheckingInspection */ diff --git a/tests/Command/ListProcessCommandTest.php b/tests/Command/ListProcessCommandTest.php index 29982424..f8222953 100644 --- a/tests/Command/ListProcessCommandTest.php +++ b/tests/Command/ListProcessCommandTest.php @@ -16,6 +16,7 @@ use CleverAge\ProcessBundle\Command\ListProcessCommand; use CleverAge\ProcessBundle\Configuration\ProcessConfiguration; use CleverAge\ProcessBundle\Configuration\TaskConfiguration; +use CleverAge\ProcessBundle\Exception\InvalidProcessConfigurationException; use CleverAge\ProcessBundle\Registry\ProcessConfigurationRegistry; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\UsesClass; @@ -26,6 +27,7 @@ #[UsesClass(ProcessConfiguration::class)] #[UsesClass(TaskConfiguration::class)] #[UsesClass(ProcessConfigurationRegistry::class)] +#[UsesClass(InvalidProcessConfigurationException::class)] class ListProcessCommandTest extends TestCase { public function testThePublicProcessesAreListedSortedByCode(): void @@ -82,6 +84,8 @@ private function createTester(): CommandTester [ 'test.private' => $process('Private process', false), 'test.b' => $process('', true), + // Left out of the list (with an error log) instead of making the command fail + 'test.invalid' => ['entry_point' => 'missing'] + $process('Invalid process', true), 'test.a' => $process('First process', true, [ 'task' => [ 'service' => '@app.task', diff --git a/tests/Manager/ProcessManagerFinalizationTest.php b/tests/Manager/ProcessManagerFinalizationTest.php new file mode 100644 index 00000000..93f3b505 --- /dev/null +++ b/tests/Manager/ProcessManagerFinalizationTest.php @@ -0,0 +1,402 @@ + + */ + private array $finalized = []; + + /** + * Whether the process history was failed during each finalization. + * + * @var list + */ + private array $failedAtFinalization = []; + + /** + * The logs of the tasks, with the task configuration seen by the logs. + * + * @var list + */ + private array $taskLogs = []; + + /** + * A message of the process logs that throws an exception when logged. + */ + private ?string $failingProcessLog = null; + + protected function setUp(): void + { + $this->container = new Container(); + $this->container->set('test.noop', new class implements TaskInterface { + public function execute(ProcessState $state): void + { + } + }); + $this->container->set('test.failing', new class implements TaskInterface { + public function execute(ProcessState $state): void + { + throw new \LogicException('Something went wrong'); + } + }); + // Runs the given sub-process, as the process_executor task + $this->container->set('test.sub_process', new class($this) implements TaskInterface { + public function __construct( + private readonly ProcessManagerFinalizationTest $test, + ) { + } + + public function execute(ProcessState $state): void + { + $this->test->executeSubProcess((string) $state->getTaskConfiguration()->getOption('process')); + } + }); + foreach (['first', 'second', 'third', 'child'] as $code) { + $this->container->set("test.finalizable_{$code}", $this->createFinalizableTask($code)); + } + $this->container->set('test.finalizable_throwing', $this->createFinalizableTask('throwing', failFinalization: true)); + $this->container->set('test.finalizable_bad_init', $this->createFinalizableTask('bad_init', failInitialization: true)); + + $this->eventDispatcher = new EventDispatcher(); + } + + public function executeSubProcess(string $processCode): void + { + $this->processManager->execute($processCode); + } + + public function testTheTasksAreFinalizedWhenTheProcessSucceeds(): void + { + $this->execute([ + 'first' => $this->createTaskConfiguration('@test.finalizable_first', ['second']), + 'second' => $this->createTaskConfiguration('@test.finalizable_second'), + ]); + + self::assertSame(['first (first)', 'second (second)'], $this->finalized); + self::assertSame([false, false], $this->failedAtFinalization); + self::assertNull($this->processManager->getTaskConfiguration()); + } + + public function testTheTasksAreFinalizedWhenTheProcessFails(): void + { + $failingTask = null; + $this->eventDispatcher->addListener(ProcessEvent::EVENT_PROCESS_FAILED, function () use (&$failingTask): void { + $failingTask = $this->processManager->getTaskConfiguration()?->getCode(); + }); + + $error = $this->executeFailing([ + 'first' => $this->createTaskConfiguration('@test.finalizable_first', ['fail']), + 'fail' => $this->createTaskConfiguration('@test.failing', ['second']), + // Initialized, never executed + 'second' => $this->createTaskConfiguration('@test.finalizable_second'), + ]); + + self::assertInstanceOf(ProcessFailedException::class, $error); + self::assertSame('Something went wrong', $error->getPrevious()?->getMessage()); + // The logs of each finalization are attributed to the finalized task + self::assertSame(['first (first)', 'second (second)'], $this->finalized); + // The tasks can tell the process has failed + self::assertSame([true, true], $this->failedAtFinalization); + // The failing task is kept for the logs of the failure + self::assertSame('fail', $failingTask); + self::assertNull($this->processManager->getTaskConfiguration()); + } + + public function testATaskWhoseInitializationHasFailedIsNotFinalizedWhenTheProcessFails(): void + { + set_error_handler(static fn (): bool => true, \E_USER_DEPRECATED); + try { + $error = $this->executeFailing([ + 'first' => $this->createTaskConfiguration('@test.finalizable_first', ['bad_init']), + 'bad_init' => $this->createTaskConfiguration('@test.finalizable_bad_init'), + ]); + } finally { + restore_error_handler(); + } + + self::assertInstanceOf(ProcessFailedException::class, $error); + self::assertSame('Invalid configuration', $error->getPrevious()?->getMessage()); + self::assertSame(['first (first)'], $this->finalized); + } + + public function testTheTasksThatAreNotInitializedAreNotFinalizedWhenTheProcessFails(): void + { + $error = $this->executeFailing([ + 'first' => $this->createTaskConfiguration('@test.finalizable_first', ['missing']), + 'missing' => $this->createTaskConfiguration('test.missing', ['second']), + 'second' => $this->createTaskConfiguration('@test.finalizable_second'), + ]); + + self::assertInstanceOf(UnexpectedValueException::class, $error); + self::assertSame(['first (first)'], $this->finalized); + // Also failed for an exception thrown outside a task + self::assertSame([true], $this->failedAtFinalization); + } + + public function testAFinalizationFailureIsLoggedAndDoesNotHideTheErrorOfTheProcess(): void + { + $error = $this->executeFailing([ + 'throwing' => $this->createTaskConfiguration('@test.finalizable_throwing', ['fail']), + 'fail' => $this->createTaskConfiguration('@test.failing'), + 'second' => $this->createTaskConfiguration('@test.finalizable_second'), + ]); + + self::assertInstanceOf(ProcessFailedException::class, $error); + self::assertSame('Something went wrong', $error->getPrevious()?->getMessage()); + // The next tasks are still finalized + self::assertSame(['throwing (throwing)', 'second (second)'], $this->finalized); + self::assertContains(['critical', 'Finalization failed', 'throwing'], $this->taskLogs); + } + + public function testTheTasksAreNotFinalizedTwiceWhenTheProcessFailsAfterTheirFinalization(): void + { + $this->failingProcessLog = 'Process test.process succeed'; + + $error = $this->executeFailing([ + 'first' => $this->createTaskConfiguration('@test.finalizable_first', ['second']), + 'second' => $this->createTaskConfiguration('@test.finalizable_second'), + ]); + + self::assertInstanceOf(\RuntimeException::class, $error); + self::assertSame('Logger failure', $error->getMessage()); + self::assertSame(['first (first)', 'second (second)'], $this->finalized); + } + + public function testAFailedSubProcessIsFinalizedAndItsParentGoesOn(): void + { + $this->createProcessManager([ + 'test.process' => [ + 'first' => $this->createTaskConfiguration('@test.finalizable_first', ['sub_process']), + 'sub_process' => $this->createTaskConfiguration('@test.sub_process', ['second'], ['process' => 'test.child'], 'skip'), + 'second' => $this->createTaskConfiguration('@test.finalizable_second'), + ], + 'test.child' => [ + 'child' => $this->createTaskConfiguration('@test.finalizable_child', ['fail']), + 'fail' => $this->createTaskConfiguration('@test.failing'), + ], + ]); + + $this->processManager->execute('test.process'); + + // The task of the sub-process is finalized when it fails, the tasks of the parent when it ends + self::assertSame(['child (child)', 'first (first)', 'second (second)'], $this->finalized); + self::assertNull($this->processManager->getTaskConfiguration()); + } + + public function record(string $code, bool $failed): void + { + $this->failedAtFinalization[] = $failed; + $this->finalized[] = "{$code} ({$this->processManager->getTaskConfiguration()?->getCode()})"; + } + + private function createFinalizableTask( + string $code, + bool $failFinalization = false, + bool $failInitialization = false, + ): TaskInterface { + return new class($this, $code, $failFinalization, $failInitialization) implements InitializableTaskInterface, FinalizableTaskInterface { + public function __construct( + private readonly ProcessManagerFinalizationTest $test, + private readonly string $code, + private readonly bool $failFinalization, + private readonly bool $failInitialization, + ) { + } + + public function initialize(ProcessState $state): void + { + if ($this->failInitialization) { + throw new \InvalidArgumentException('Invalid configuration'); + } + } + + /** + * Like configurable tasks (options are resolved again), fail again when executed. + */ + public function execute(ProcessState $state): void + { + $this->initialize($state); + $state->setOutput($state->getInput()); + } + + public function finalize(ProcessState $state): void + { + $this->test->record($this->code, $state->getProcessHistory()->isFailed()); + if ($this->failFinalization) { + throw new \RuntimeException('Finalization failed'); + } + } + }; + } + + /** + * @param array> $tasks + */ + private function execute(array $tasks): void + { + $this->createProcessManager(['test.process' => $tasks]); + $this->processManager->execute('test.process'); + } + + /** + * @param array> $tasks + */ + private function executeFailing(array $tasks): \Throwable + { + try { + $this->execute($tasks); + } catch (\Throwable $error) { + return $error; + } + + self::fail('The process should have failed'); + } + + /** + * @param array>> $processes + */ + private function createProcessManager(array $processes): void + { + $configurations = []; + foreach ($processes as $code => $tasks) { + $configurations[$code] = [ + 'options' => [], + 'entry_point' => null, + 'end_point' => null, + 'description' => '', + 'help' => '', + 'public' => true, + 'tasks' => $tasks, + ]; + } + + $test = $this; + $processLogger = new class($test) extends PsrAbstractLogger { + public function __construct( + private readonly ProcessManagerFinalizationTest $test, + ) { + } + + public function log($level, string|\Stringable $message, array $context = []): void + { + $this->test->logProcess((string) $message); + } + }; + $taskLogger = new class($test) extends PsrAbstractLogger { + public function __construct( + private readonly ProcessManagerFinalizationTest $test, + ) { + } + + public function log($level, string|\Stringable $message, array $context = []): void + { + $this->test->logTask((string) $level, (string) $message); + } + }; + + $this->processManager = new ProcessManager( + $this->container, + new ProcessLogger($processLogger), + new TaskLogger($taskLogger), + new ProcessConfigurationRegistry($configurations, 'stop'), + new ContextualOptionResolver(), + $this->eventDispatcher, + taskRegistry: new TaskRegistry($this->container), + ); + } + + public function logProcess(string $message): void + { + if ($message === $this->failingProcessLog) { + throw new \RuntimeException('Logger failure'); + } + } + + public function logTask(string $level, string $message): void + { + $this->taskLogs[] = [$level, $message, $this->processManager->getTaskConfiguration()?->getCode()]; + } + + /** + * @param list $outputs + * @param array $options + * + * @return array + */ + private function createTaskConfiguration( + string $service, + array $outputs = [], + array $options = [], + ?string $errorStrategy = null, + ): array { + return [ + 'service' => $service, + 'options' => $options, + 'description' => '', + 'help' => '', + 'outputs' => $outputs, + 'errors' => [], + 'error_outputs' => [], + 'error_strategy' => $errorStrategy, + 'log_level' => null, + ]; + } +} diff --git a/tests/Manager/ProcessManagerInitializationTest.php b/tests/Manager/ProcessManagerInitializationTest.php index a4dc6f12..ac920cd4 100644 --- a/tests/Manager/ProcessManagerInitializationTest.php +++ b/tests/Manager/ProcessManagerInitializationTest.php @@ -28,6 +28,7 @@ use CleverAge\ProcessBundle\Model\TaskInterface; use CleverAge\ProcessBundle\Registry\ProcessConfigurationRegistry; use CleverAge\ProcessBundle\Registry\TaskRegistry; +use CleverAge\ProcessBundle\Task\StopTask; use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; use Symfony\Component\DependencyInjection\Container; @@ -48,11 +49,15 @@ #[\PHPUnit\Framework\Attributes\UsesClass(ProcessHistory::class)] #[\PHPUnit\Framework\Attributes\UsesClass(ProcessState::class)] #[\PHPUnit\Framework\Attributes\UsesClass(ProcessConfigurationRegistry::class)] +#[\PHPUnit\Framework\Attributes\UsesClass(StopTask::class)] class ProcessManagerInitializationTest extends TestCase { /** @var list */ private array $executedTasks = []; + /** @var list */ + private array $endedEvents = []; + /** @var list */ private array $deprecations = []; @@ -92,6 +97,18 @@ public function testNoDeprecationWithoutInitializationFailure(): void self::assertSame([], $this->deprecations); } + public function testStopTaskDoesNotTurnAFailedProcessIntoAStoppedOne(): void + { + $processManager = $this->createProcessManager(skipBeforeBadTask: false, withStopTask: true); + + $this->execute($processManager); + + self::assertSame(['entry'], $this->executedTasks, 'The stop signal still prevents the next tasks from running'); + self::assertSame([$this->getExpectedDeprecation()], $this->deprecations); + self::assertCount(1, $this->endedEvents); + self::assertFalse($this->endedEvents[0]->isProcessStopped(), 'The failed process must not be flagged as stopped'); + } + private function getExpectedDeprecation(): string { return 'Since cleverage/process-bundle 5.1: The initialization of the task "bad" of the process "test.process" has failed with message "Invalid configuration". Going on with the process after an initialization failure is deprecated: in v6.0, the process will fail before executing any task.'; @@ -111,7 +128,7 @@ private function execute(ProcessManager $processManager): void } } - private function createProcessManager(bool $skipBeforeBadTask, bool $failInitialization = true): ProcessManager + private function createProcessManager(bool $skipBeforeBadTask, bool $failInitialization = true, bool $withStopTask = false): ProcessManager { $recorder = function (string $taskCode): void { $this->executedTasks[] = $taskCode; @@ -159,6 +176,7 @@ public function execute(ProcessState $state): void $container = new Container(); $container->set('test.entry', $entryTask); $container->set('test.bad', $badTask); + $container->set('test.stop', new StopTask()); $registry = new ProcessConfigurationRegistry( [ @@ -170,21 +188,30 @@ public function execute(ProcessState $state): void 'help' => '', 'public' => true, 'tasks' => [ - 'entry' => $this->createTaskConfiguration('@test.entry', ['bad']), + 'entry' => $this->createTaskConfiguration('@test.entry', $withStopTask ? ['stop', 'bad'] : ['bad']), 'bad' => $this->createTaskConfiguration('@test.bad'), + ...($withStopTask ? ['stop' => $this->createTaskConfiguration('@test.stop')] : []), ], ], ], 'stop' ); + $eventDispatcher = new EventDispatcher(); + $eventDispatcher->addListener(ProcessEvent::EVENT_PROCESS_ENDED, function (ProcessEvent $event): void { + $this->endedEvents[] = $event; + }); + $eventDispatcher->addListener(ProcessEvent::EVENT_PROCESS_FAILED, function (ProcessEvent $event): void { + $this->endedEvents[] = $event; + }); + return new ProcessManager( $container, new ProcessLogger(new NullLogger()), new TaskLogger(new NullLogger()), $registry, new ContextualOptionResolver(), - new EventDispatcher(), + $eventDispatcher, taskRegistry: new TaskRegistry($container), ); } diff --git a/tests/Model/SubprocessInstanceTest.php b/tests/Model/SubprocessInstanceTest.php index c2b76b73..52bf7b13 100644 --- a/tests/Model/SubprocessInstanceTest.php +++ b/tests/Model/SubprocessInstanceTest.php @@ -75,6 +75,49 @@ public function testJsonBufferIsWrittenInTheTemporaryDirectory(): void unlink($bufferPath); } + public function testEmptyJsonBufferIsRemovedAndNotReturned(): void + { + // The console outputs nothing: no end point, or an output that is not an array + $subprocess = $this->runWithJsonBuffering(); + $bufferPath = $this->getBufferPath($subprocess); + + $this->assertNull($subprocess->getResult()); + $this->assertFileDoesNotExist($bufferPath); + } + + public function testWrittenJsonBufferIsReturned(): void + { + file_put_contents($this->projectDir.'/bin/console', 'runWithJsonBuffering(); + $bufferPath = $this->getBufferPath($subprocess); + + $this->assertSame($bufferPath, $subprocess->getResult()); + $this->assertStringEqualsFile($bufferPath, "[\"a\"]\n"); + unlink($bufferPath); + } + + public function testNoResultWithoutJsonBuffering(): void + { + $subprocess = (new SubprocessInstance($this->projectDir.'/bin/console', 'child', null))->buildProcess()->start(); + $subprocess->getProcess()->wait(); + + $this->assertNull($subprocess->getResult()); + // Nothing to remove + $subprocess->removeBuffer(); + } + + public function testRemoveBuffer(): void + { + $subprocess = (new SubprocessInstance($this->projectDir.'/bin/console', 'child', null, [], [SubprocessInstance::OPTION_JSON_BUFFERING => true])) + ->buildProcess(); + $bufferPath = $this->getBufferPath($subprocess); + $this->assertFileExists($bufferPath); + + $subprocess->removeBuffer(); + + $this->assertFileDoesNotExist($bufferPath); + } + public function testMissingConsoleThrows(): void { $this->expectException(\RuntimeException::class); @@ -110,4 +153,22 @@ public function testPassingTheKernelIsDeprecated(): void // The log directory of the kernel is no longer created $this->assertDirectoryDoesNotExist($this->projectDir.'/var'); } + + private function runWithJsonBuffering(): SubprocessInstance + { + $subprocess = (new SubprocessInstance($this->projectDir.'/bin/console', 'child', null, [], [SubprocessInstance::OPTION_JSON_BUFFERING => true])) + ->buildProcess() + ->start(); + $subprocess->getProcess()->wait(); + $this->assertSame(0, $subprocess->getProcess()->getExitCode(), $subprocess->getProcess()->getErrorOutput()); + + return $subprocess; + } + + private function getBufferPath(SubprocessInstance $subprocess): string + { + preg_match('#--output=([^\']+)#', $subprocess->getProcess()->getCommandLine(), $matches); + + return $matches[1] ?? ''; + } } diff --git a/tests/Registry/ProcessConfigurationRegistryTest.php b/tests/Registry/ProcessConfigurationRegistryTest.php index 50e09f96..1ab28cdd 100644 --- a/tests/Registry/ProcessConfigurationRegistryTest.php +++ b/tests/Registry/ProcessConfigurationRegistryTest.php @@ -15,12 +15,16 @@ use CleverAge\ProcessBundle\Configuration\ProcessConfiguration; use CleverAge\ProcessBundle\Configuration\TaskConfiguration; +use CleverAge\ProcessBundle\Exception\InvalidProcessConfigurationException; use CleverAge\ProcessBundle\Registry\ProcessConfigurationRegistry; use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; +use Psr\Log\AbstractLogger; #[CoversClass(ProcessConfigurationRegistry::class)] +#[CoversClass(InvalidProcessConfigurationException::class)] #[UsesClass(ProcessConfiguration::class)] #[UsesClass(TaskConfiguration::class)] class ProcessConfigurationRegistryTest extends TestCase @@ -83,4 +87,140 @@ public function testResetForgetsTheResolvedConfigurations(): void self::assertNotSame($configuration, $resolvedAgain); self::assertSame('test.process', $resolvedAgain->getCode()); } + + /** + * @return iterable, array>, string}> + */ + public static function provideUnknownTaskReferences(): iterable + { + yield 'outputs' => [ + [], + ['read' => ['outputs' => ['missing']], 'write' => []], + "Unknown task 'missing' in the 'outputs' of the task 'read' (from process: test.process)", + ]; + yield 'error_outputs' => [ + [], + ['read' => ['outputs' => ['write'], 'error_outputs' => ['missing']], 'write' => []], + "Unknown task 'missing' in the 'error_outputs' of the task 'read' (from process: test.process)", + ]; + yield 'deprecated errors' => [ + [], + ['read' => ['errors' => ['missing']], 'write' => []], + "Unknown task 'missing' in the 'errors' of the task 'read' (from process: test.process)", + ]; + yield 'entry_point' => [ + ['entry_point' => 'missing'], + ['read' => ['outputs' => ['write']], 'write' => []], + "Unknown task 'missing' in the 'entry_point' (from process: test.process)", + ]; + yield 'end_point' => [ + ['entry_point' => 'read', 'end_point' => 'missing'], + ['read' => ['outputs' => ['write']], 'write' => []], + "Unknown task 'missing' in the 'end_point' (from process: test.process)", + ]; + } + + /** + * @param array $processConfiguration + * @param array> $tasks + */ + #[DataProvider('provideUnknownTaskReferences')] + public function testUnknownTaskReferenceNamesTheProcessAndTheTask(array $processConfiguration, array $tasks, string $message): void + { + $registry = new ProcessConfigurationRegistry(['test.process' => $this->rawProcess($tasks, $processConfiguration)], 'stop'); + + $this->expectException(InvalidProcessConfigurationException::class); + $this->expectExceptionMessage($message); + + $registry->getProcessConfiguration('test.process'); + } + + public function testListingLeavesOutAndLogsTheInvalidProcesses(): void + { + $logger = new class extends AbstractLogger { + /** @var list}> */ + public array $records = []; + + public function log($level, \Stringable|string $message, array $context = []): void + { + $this->records[] = [$level, (string) $message, $context]; + } + }; + $registry = new ProcessConfigurationRegistry([ + 'test.valid' => $this->rawProcess(['read' => ['outputs' => ['write']], 'write' => []]), + 'test.unknown_output' => $this->rawProcess(['read' => ['outputs' => ['missing']]]), + 'test.no_service' => $this->rawProcess(['read' => ['service' => null]]), + 'test.other' => $this->rawProcess(['read' => []]), + ], 'stop', [], $logger); + + $configurations = $registry->getProcessConfigurations(); + + self::assertSame(['test.valid', 'test.other'], array_keys($configurations)); + self::assertSame(['test.valid', 'test.other'], array_keys($registry->getProcessConfigurations())); + self::assertCount(4, $logger->records); + [$level, $message, $context] = $logger->records[0]; + self::assertSame('error', $level); + self::assertSame("The process test.unknown_output is left out of the process list, its configuration is not valid: Unknown task 'missing' in the 'outputs' of the task 'read' (from process: test.unknown_output)", $message); + self::assertSame('test.unknown_output', $context['process_code']); + self::assertInstanceOf(InvalidProcessConfigurationException::class, $context['exception']); + self::assertStringStartsWith('The process test.no_service is left out of the process list', $logger->records[1][1]); + + // Requested by its code (e.g. to be executed), the invalid process still throws + $this->expectException(InvalidProcessConfigurationException::class); + $this->expectExceptionMessage("Unknown task 'missing' in the 'outputs' of the task 'read' (from process: test.unknown_output)"); + $registry->getProcessConfiguration('test.unknown_output'); + } + + public function testListingWithoutLogger(): void + { + $registry = new ProcessConfigurationRegistry([ + 'test.invalid' => $this->rawProcess(['read' => []], ['entry_point' => 'missing']), + 'test.valid' => $this->rawProcess(['read' => []]), + ], 'stop'); + + self::assertSame(['test.valid'], array_keys($registry->getProcessConfigurations())); + } + + public function testListingKeepsTheConfigurationsResolvedBefore(): void + { + $registry = new ProcessConfigurationRegistry([ + 'test.a' => $this->rawProcess(['read' => []]), + 'test.b' => $this->rawProcess(['read' => []]), + ], 'stop'); + $configuration = $registry->getProcessConfiguration('test.b'); + + $configurations = $registry->getProcessConfigurations(); + + self::assertSame(['test.a', 'test.b'], array_keys($configurations)); + self::assertSame($configuration, $configurations['test.b']); + } + + /** + * @param array> $tasks + * @param array $processConfiguration + * + * @return array + */ + private function rawProcess(array $tasks, array $processConfiguration = []): array + { + return $processConfiguration + [ + 'options' => [], + 'entry_point' => null, + 'end_point' => null, + 'description' => '', + 'help' => '', + 'public' => true, + 'tasks' => array_map(static fn (array $task): array => $task + [ + 'service' => '@app.task', + 'options' => [], + 'description' => '', + 'help' => '', + 'outputs' => [], + 'errors' => [], + 'error_outputs' => [], + 'error_strategy' => null, + 'log_level' => null, + ], $tasks), + ]; + } } diff --git a/tests/Task/File/Yaml/YamlWriterTaskTest.php b/tests/Task/File/Yaml/YamlWriterTaskTest.php index c0599882..5d46d433 100644 --- a/tests/Task/File/Yaml/YamlWriterTaskTest.php +++ b/tests/Task/File/Yaml/YamlWriterTaskTest.php @@ -109,6 +109,46 @@ public function testTheParentDirectoryIsCreated(): void self::assertSame(self::DATA, Yaml::parseFile($filePath)); } + public function testTheParentDirectoryOfAFileUrlIsCreated(): void + { + $filePath = $this->tmpDir.'/url/file.yaml'; + $this->execute(['file_path' => 'file://'.$filePath], self::DATA); + + self::assertSame(self::DATA, Yaml::parseFile($filePath)); + } + + public function testAStreamWrapperPathIsWritten(): void + { + $filePath = 'compress.zlib://'.$this->tmpDir.'/file.yaml.gz'; + $state = $this->execute(['file_path' => $filePath], self::DATA); + + self::assertSame($filePath, $state->getOutput()); + self::assertSame(self::DATA, Yaml::parse((string) file_get_contents($filePath))); + self::assertSame("\x1f\x8b", substr((string) file_get_contents($this->tmpDir.'/file.yaml.gz'), 0, 2)); + } + + public function testAStreamWrapperPathDoesNotCreateADirectory(): void + { + $cwd = getcwd(); + self::assertNotFalse($cwd); + chdir($this->tmpDir); + try { + $this->execute(['file_path' => 'php://memory'], self::DATA); + ob_start(); + try { + $this->execute(['file_path' => 'php://output'], self::DATA); + } finally { + $output = ob_get_clean(); + } + } finally { + chdir($cwd); + } + + self::assertSame(self::DATA, Yaml::parse((string) $output)); + + self::assertSame([], glob($this->tmpDir.'/*')); + } + public function testAWriteFailureThrows(): void { // A directory cannot be written as a file diff --git a/tests/Task/Process/ProcessLauncherTaskTest.php b/tests/Task/Process/ProcessLauncherTaskTest.php index 49de82a3..b31d81af 100644 --- a/tests/Task/Process/ProcessLauncherTaskTest.php +++ b/tests/Task/Process/ProcessLauncherTaskTest.php @@ -16,9 +16,11 @@ use CleverAge\ProcessBundle\Configuration\ProcessConfiguration; use CleverAge\ProcessBundle\Configuration\TaskConfiguration; use CleverAge\ProcessBundle\Context\ContextualOptionResolver; +use CleverAge\ProcessBundle\Filesystem\TemporaryFile; use CleverAge\ProcessBundle\Model\AbstractConfigurableTask; use CleverAge\ProcessBundle\Model\ProcessHistory; use CleverAge\ProcessBundle\Model\ProcessState; +use CleverAge\ProcessBundle\Model\SubprocessInstance; use CleverAge\ProcessBundle\Registry\ProcessConfigurationRegistry; use CleverAge\ProcessBundle\Task\Process\ProcessLauncherTask; use PHPUnit\Framework\TestCase; @@ -34,8 +36,12 @@ #[\PHPUnit\Framework\Attributes\UsesClass(ContextualOptionResolver::class)] #[\PHPUnit\Framework\Attributes\UsesClass(ProcessHistory::class)] #[\PHPUnit\Framework\Attributes\UsesClass(ProcessState::class)] +#[\PHPUnit\Framework\Attributes\UsesClass(SubprocessInstance::class)] +#[\PHPUnit\Framework\Attributes\UsesClass(TemporaryFile::class)] class ProcessLauncherTaskTest extends TestCase { + private string $consoleDir; + #[\PHPUnit\Framework\Attributes\DoesNotPerformAssertions] public function testDefaultOptionsAreResolved(): void { @@ -87,6 +93,153 @@ public function testPassingTheKernelIsDeprecated(): void ]); } + public function testJsonBufferingOutputsThePathOfTheWrittenBuffer(): void + { + [$task, $state] = $this->launch('write'); + + $this->assertTrue($task->next($state)); + $state->reset(false); + $task->execute($state); + + $bufferPath = $state->getOutput(); + $this->assertIsString($bufferPath); + $this->assertStringEqualsFile($bufferPath, "[\"a\"]\n"); + // Removed by the reader of the path, not by the task + unlink($bufferPath); + } + + public function testJsonBufferingOutputsNothingWhenTheSubprocessWroteNothing(): void + { + [$task, $state, $bufferPath] = $this->launch('nothing'); + + $this->assertFalse($task->next($state)); + $state->reset(false); + $task->flush($state); + + $this->assertTrue($state->isSkipped()); + $this->assertNull($state->getOutput()); + $this->assertFileDoesNotExist($bufferPath); + } + + public function testJsonBufferIsRemovedWhenTheSubprocessFails(): void + { + [$task, $state, $bufferPath] = $this->launch('fail'); + + try { + $task->next($state); + $this->fail('The failure of the sub-process must throw'); + } catch (\RuntimeException $e) { + $this->assertSame('Sub-process has failed: General error', $e->getMessage()); + } + $this->assertFileDoesNotExist($bufferPath); + } + + public function testJsonBuffersOfTheStoppedSubprocessesAreRemovedWhenASubprocessFails(): void + { + [$task, $state, $runningBufferPath] = $this->launch('sleep', wait: false); + [, , $failedBufferPath] = $this->launch('fail', $task, $state); + + try { + $task->next($state); + $this->fail('The failure of the sub-process must throw'); + } catch (\RuntimeException) { + } + $this->assertFileDoesNotExist($runningBufferPath); + $this->assertFileDoesNotExist($failedBufferPath); + } + + public function testJsonBufferOfATerminatedSubprocessIsKeptWhenAnotherSubprocessFails(): void + { + // The failed sub-process is handled first, the other one is already terminated but not handled yet + [$task, $state, $failedBufferPath] = $this->launch('slowfail', wait: false); + [, , $bufferPath] = $this->launch('write', $task, $state); + $launchedProcesses = (new \ReflectionProperty($task, 'launchedProcesses'))->getValue($task); + $this->assertIsArray($launchedProcesses); + $failedSubprocess = reset($launchedProcesses); + $this->assertInstanceOf(SubprocessInstance::class, $failedSubprocess); + $failedSubprocess->getProcess()->wait(); + + try { + $state->reset(true); + $task->execute($state); + $this->fail('The failure of the sub-process must throw'); + } catch (\RuntimeException) { + } + $this->assertFileDoesNotExist($failedBufferPath); + $this->assertFileExists($bufferPath); + + // With the "skip" error strategy, the process goes on: the result of the terminated sub-process is output + $this->assertTrue($task->next($state)); + $state->reset(false); + $task->execute($state); + $this->assertSame($bufferPath, $state->getOutput()); + unlink($bufferPath); + } + + protected function tearDown(): void + { + if (isset($this->consoleDir)) { + unlink($this->consoleDir.'/console'); + rmdir($this->consoleDir); + } + } + + /** + * Launches a sub-process with JSON buffering and waits for it. The fake console depends on the input: "write" + * writes an array in the buffer, "fail" exits with an error ("slowfail" after 0.5 s), "sleep" keeps running, + * otherwise it outputs nothing. + * + * @return array{ProcessLauncherTask, ProcessState, string} the task, its state and the path of the buffer + */ + private function launch(string $input, ?ProcessLauncherTask $task = null, ?ProcessState $state = null, bool $wait = true): array + { + if (!isset($this->consoleDir)) { + $this->consoleDir = sys_get_temp_dir().'/process_launcher_test_'.uniqid('', true); + mkdir($this->consoleDir); + file_put_contents($this->consoleDir.'/console', <<<'PHP' + createRegistry(), $this->consoleDir.'/console'); + $state = $this->createState([ + 'process' => 'child', + 'json_buffering' => true, + 'sleep_interval_after_launch' => 0, + 'sleep_on_finalize_interval' => 0, + ]); + $task->initialize($state); + } + $state->reset(true); + $state->setInput($input); + $task->execute($state); + $this->assertTrue($state->isSkipped()); + + $launchedProcesses = (new \ReflectionProperty($task, 'launchedProcesses'))->getValue($task); + $this->assertIsArray($launchedProcesses); + $subprocess = end($launchedProcesses); + $this->assertInstanceOf(SubprocessInstance::class, $subprocess); + if ($wait) { + $subprocess->getProcess()->wait(); + } + preg_match('#--output=([^\']+)#', $subprocess->getProcess()->getCommandLine(), $matches); + $bufferPath = $matches[1] ?? ''; + $this->assertFileExists($bufferPath); + + return [$task, $state, $bufferPath]; + } + private function createTask(): ProcessLauncherTask { return new ProcessLauncherTask(new NullLogger(), $this->createRegistry(), '/app/bin/console', 'test'); diff --git a/tests/Task/StopTaskTest.php b/tests/Task/StopTaskTest.php index c0ac1680..48cac14a 100644 --- a/tests/Task/StopTaskTest.php +++ b/tests/Task/StopTaskTest.php @@ -44,4 +44,17 @@ public function testProcessIsStoppedAndMarkedAsStopped(): void self::assertNull($state->getOutput()); self::assertNull($state->getException()); } + + public function testFailedProcessStaysFailedButIsStopped(): void + { + $processConfiguration = new ProcessConfiguration('test', []); + $state = new ProcessState($processConfiguration, new ProcessHistory($processConfiguration)); + $state->getProcessHistory()->setFailed(); + + (new StopTask())->execute($state); + + self::assertTrue($state->isStopped()); + self::assertTrue($state->getProcessHistory()->isFailed()); + self::assertFalse($state->getProcessHistory()->isStopped()); + } } diff --git a/tests/Transformer/RulesTransformerTest.php b/tests/Transformer/RulesTransformerTest.php index 6a72ae86..3aedea35 100644 --- a/tests/Transformer/RulesTransformerTest.php +++ b/tests/Transformer/RulesTransformerTest.php @@ -17,9 +17,12 @@ use CleverAge\ProcessBundle\Transformer\Array\ArrayLastTransformer; use CleverAge\ProcessBundle\Transformer\RulesTransformer; use CleverAge\ProcessBundle\Transformer\TransformerTrait; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\ServiceLocator; +use Symfony\Component\ExpressionLanguage\ExpressionFunction; use Symfony\Component\ExpressionLanguage\ExpressionLanguage; +use Symfony\Component\ExpressionLanguage\ParsedExpression; use Symfony\Component\OptionsResolver\OptionsResolver; #[\PHPUnit\Framework\Attributes\CoversClass(RulesTransformer::class)] @@ -42,6 +45,68 @@ public function testTransform(): void $this->assertSame('is not foo', $transformer->transform('bar', $options)); } + /** + * @return iterable + */ + public static function nonBooleanConditionProvider(): iterable + { + yield 'preg_match int match' => ['preg_match("/^foo/", value)', 'foobar', 'matched']; + yield 'preg_match int no match' => ['preg_match("/^foo/", value)', 'barfoo', 'default']; + yield 'matches operator' => ['value matches "/^foo/"', 'foobar', 'matched']; + yield 'matches operator no match' => ['value matches "/^foo/"', 'barfoo', 'default']; + yield 'truthy string variable' => ['value', 'foo', 'matched']; + yield 'falsy string variable' => ['value', '0', 'default']; + yield 'null variable' => ['value', null, 'default']; + yield 'truthy array variable' => ['value', ['a'], 'matched']; + yield 'empty array variable' => ['value', [], 'default']; + } + + #[DataProvider('nonBooleanConditionProvider')] + public function testNonBooleanCondition(string $condition, mixed $input, string $expected): void + { + $transformer = $this->createTransformer(); + $options = $this->resolveOptions($transformer, [ + 'rules_set' => [ + ['condition' => $condition, 'constant' => 'matched'], + ['default' => true, 'constant' => 'default'], + ], + ]); + + $this->assertSame($expected, $transformer->transform($input, $options)); + } + + public function testNonBooleanConditionWithValueAsVariables(): void + { + $transformer = $this->createTransformer(); + $options = $this->resolveOptions($transformer, [ + 'use_value_as_variables' => true, + 'expression_variables' => ['name'], + 'rules_set' => [ + ['condition' => 'name', 'constant' => 'matched'], + ['default' => true, 'constant' => 'default'], + ], + ]); + + $this->assertSame('matched', $transformer->transform(['name' => 'foo'], $options)); + $this->assertSame('default', $transformer->transform(['name' => null], $options)); + } + + public function testParsedExpressionCondition(): void + { + $transformer = $this->createTransformer(); + $parsed = (new ExpressionLanguage())->parse('value', ['value']); + $this->assertInstanceOf(ParsedExpression::class, $parsed); + $options = $this->resolveOptions($transformer, [ + 'rules_set' => [ + ['condition' => $parsed, 'constant' => 'matched'], + ['default' => true, 'constant' => 'default'], + ], + ]); + + $this->assertSame('matched', $transformer->transform('foo', $options)); + $this->assertSame('default', $transformer->transform(null, $options)); + } + public function testRulesSetCannotHaveMoreThanOneDefaultRule(): void { $transformer = $this->createTransformer(); @@ -77,7 +142,10 @@ private function createTransformer(): RulesTransformer 'array_last' => static fn (): ArrayLastTransformer => new ArrayLastTransformer(), ])); - return new RulesTransformer($registry, new ExpressionLanguage()); + $language = new ExpressionLanguage(); + $language->addFunction(ExpressionFunction::fromPhp('preg_match')); + + return new RulesTransformer($registry, $language); } /**