#372 - Name the process and the task of an unknown task reference, leave the invalid processes out of the listings

CHANGELOG.md

@@ -6,6 +6,7 @@ Latest
 * [#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 <code> 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 : <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.
 
 v5.5
 -----

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:

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

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})");
+    }
 }

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<string, ProcessConfiguration>
      */
     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<string, string> $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<string, mixed>  $rawProcessConfiguration
+     * @param TaskConfiguration[]   $taskConfigurations      indexed by task code
+     * @param array<string, string> $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) {

src/Registry/ProcessConfigurationRegistryInterface.php

@@ -29,9 +29,10 @@ interface ProcessConfigurationRegistryInterface
     public function getProcessConfiguration(string $processCode): ProcessConfiguration;
 
     /**
-     * @return array<string, ProcessConfiguration> 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<string, ProcessConfiguration> indexed by process code
      */
     public function getProcessConfigurations(): array;
 

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',

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<string, array{array<string, mixed>, array<string, array<string, mixed>>, 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<string, mixed>                $processConfiguration
+     * @param array<string, array<string, mixed>> $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<array{mixed, string, array<mixed>}> */
+            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<string, array<string, mixed>> $tasks
+     * @param array<string, mixed>                $processConfiguration
+     *
+     * @return array<string, mixed>
+     */
+    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),
+        ];
+    }
 }