diff --git a/CHANGELOG.md b/CHANGELOG.md index 94fe8770..6da66b1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Latest * [#333](https://github.com/cleverage/process-bundle/issues/333) `ProcessManager`: restore the previous task configuration (`getTaskConfiguration()`, read by the log processors for `task_code` and `task_service`) when a task, an initialization, a finalization or a sub-process ends. The logs of an iterable task no longer lose their task after its first item, and the logs of a parent process are no longer attributed to a task of its sub-process; the failing task is kept for the logs of the process failure. Add tests. * [#335](https://github.com/cleverage/process-bundle/issues/335) `LineReaderTask` (`line_reader`) and `InputLineReaderTask` (`input_line_reader`): the last line of a file that does not end with a line break is no longer lost. Add tests. * [#345](https://github.com/cleverage/process-bundle/issues/345) `YamlWriterTask` (`yaml_writer`): a write failure throws a `RuntimeException` instead of a PHP warning (an exception in debug mode only: in production, the path was output as if the file had been written), and the parent directory is created if needed, as for the other writers. Update documentation, add tests. +* [#353](https://github.com/cleverage/process-bundle/issues/353) `cleverage:process:execute`: the process codes and the output format are checked before any process is executed (explicit message for `dump` without `symfony/var-dumper`); the output is printed to STDOUT whatever the verbosity when `--output-format` is given (it required `-vv`); a non-array output that cannot be stored as JSON stream is reported on the error output instead of being silently lost. Update documentation, add tests. v5.4.1 ----- diff --git a/docs/01-quick_start.md b/docs/01-quick_start.md index 1a862aa8..f3775a7b 100644 --- a/docs/01-quick_start.md +++ b/docs/01-quick_start.md @@ -267,7 +267,7 @@ Process 'project_prefix.process_name' executed successfully | `--input-from-stdin` | | Read the input value from STDIN instead (e.g. `cat file.json \| bin/console cleverage:process:execute ...`) | | `--context=:` | `-c` | Contextual value, can be repeated. The key must only contain word characters (`\w+`: letters, digits, `_`). The value is parsed as YAML, so `-c limit:10` gives an integer and `-c name:"'foo'"` forces a string (also needed for dates: `-c date:2024-01-01` gives an integer timestamp) | | `--output=` | `-o` | Where to dump the value returned by the task defined by the process `end_point`: `-` (default) for STDOUT, or a file path | -| `--output-format=` | `-t` | Format of the dumped output: `dump` (Symfony VarDumper, STDOUT only, requires `-vv`) or `json-stream` (written to STDOUT with `-vv`, or into the `--output` file, only if the value is an array). Nothing is dumped when this option is omitted. Without `end_point`, the output is `null` | +| `--output-format=` | `-t` | Format of the dumped output: `dump` (Symfony VarDumper, STDOUT only) or `json-stream` (written to STDOUT, or into the `--output` file if the value is an array: otherwise a warning is written on the error output). Nothing is dumped when this option is omitted. Without `end_point`, the output is `null`. The format and the process codes are checked before any process is executed | Example: diff --git a/src/Command/ExecuteProcessCommand.php b/src/Command/ExecuteProcessCommand.php index 95418fe8..8b56aba1 100644 --- a/src/Command/ExecuteProcessCommand.php +++ b/src/Command/ExecuteProcessCommand.php @@ -24,6 +24,7 @@ use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Output\ConsoleOutputInterface; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\VarDumper\VarDumper; @@ -89,13 +90,17 @@ protected function execute(InputInterface $input, OutputInterface $output): int $context = $this->parseContextValues($input); - $this->eventDispatcher->dispatch(new ConsoleProcessEvent($input, $output, $inputData, $context)); - + // Checked before executing anything: a wrong option must not be detected once the work is done foreach ($input->getArgument('processCodes') as $code) { if (!$this->processRegistry->hasProcessConfiguration($code)) { throw new InvalidConfigurationException("Unknown process {$code}"); } + } + $this->checkOutputFormat($input); + $this->eventDispatcher->dispatch(new ConsoleProcessEvent($input, $output, $inputData, $context)); + + foreach ($input->getArgument('processCodes') as $code) { if (!$output->isQuiet()) { $output->writeln("Starting process '{$code}'..."); } @@ -134,6 +139,26 @@ protected function parseContextValues(InputInterface $input): array return $context; } + /** + * The output format must be "dump" (to STDOUT, with the VarDumper component) or "json-stream". + */ + protected function checkOutputFormat(InputInterface $input): void + { + $format = $input->getOption('output-format'); + if (!$format) { + return; + } + $toStdout = self::OUTPUT_STDOUT === $input->getOption('output'); + if (self::OUTPUT_FORMAT_JSON === $format || ($toStdout && self::OUTPUT_FORMAT_DUMP === $format && class_exists(VarDumper::class))) { + return; + } + if ($toStdout && self::OUTPUT_FORMAT_DUMP === $format) { + throw new InvalidArgumentException("The 'dump' output format requires the VarDumper component: run \"composer require symfony/var-dumper\""); + } + + throw new InvalidArgumentException(\sprintf("Cannot handle data output with format '%s'", $format)); + } + protected function handleOutputData(mixed $data, InputInterface $input, OutputInterface $output): void { // Skip all if undefined @@ -141,32 +166,28 @@ protected function handleOutputData(mixed $data, InputInterface $input, OutputIn return; } - // Handle printing the output + // Handle printing the output: asked explicitly, whatever the verbosity if (self::OUTPUT_STDOUT === $input->getOption('output')) { - if ($output->isVeryVerbose()) { - if (class_exists(VarDumper::class) && (self::OUTPUT_FORMAT_DUMP === $input->getOption( - 'output-format' - ))) { - VarDumper::dump($data); - } elseif (self::OUTPUT_FORMAT_JSON === $input->getOption('output-format')) { - $output->writeln(json_encode($data, \JSON_THROW_ON_ERROR)); - } else { - throw new InvalidArgumentException(\sprintf("Cannot handle data output with format '%s'", $input->getOption('output-format'))); - } - } - } elseif (self::OUTPUT_FORMAT_JSON === $input->getOption('output-format')) { - // JsonStreamFile::writeLine only takes an array... - // TODO how to handle other cases ? - if (\is_array($data)) { - $outputFile = new JsonStreamFile($input->getOption('output'), 'wb'); - $outputFile->writeLine($data); + if (self::OUTPUT_FORMAT_DUMP === $input->getOption('output-format')) { + VarDumper::dump($data); + } else { + $output->writeln(json_encode($data, \JSON_THROW_ON_ERROR)); } - if (isset($outputFile) && $output->isVerbose()) { - $output->writeln(\sprintf("Output stored in '%s'", $input->getOption('output'))); - } - } else { - throw new InvalidArgumentException(\sprintf("Cannot handle data output with format '%s'", $input->getOption('output-format'))); + return; + } + + // JsonStreamFile::writeLine() only takes an array + if (!\is_array($data)) { + $errorOutput = $output instanceof ConsoleOutputInterface ? $output->getErrorOutput() : $output; + $errorOutput->writeln(\sprintf("Output not stored in '%s': the JSON stream format requires an array, %s given", $input->getOption('output'), get_debug_type($data))); + + return; + } + $outputFile = new JsonStreamFile($input->getOption('output'), 'wb'); + $outputFile->writeLine($data); + if ($output->isVerbose()) { + $output->writeln(\sprintf("Output stored in '%s'", $input->getOption('output'))); } } } diff --git a/tests/Command/ExecuteProcessCommandTest.php b/tests/Command/ExecuteProcessCommandTest.php new file mode 100644 index 00000000..13d1ace5 --- /dev/null +++ b/tests/Command/ExecuteProcessCommandTest.php @@ -0,0 +1,134 @@ +outputFile = sys_get_temp_dir().\DIRECTORY_SEPARATOR.uniqid('execute_process_command_test_', true).'.json-stream'; + } + + protected function tearDown(): void + { + if (file_exists($this->outputFile)) { + unlink($this->outputFile); + } + } + + public function testUnknownProcessCodesAreDetectedBeforeAnyExecution(): void + { + $tester = $this->createTester($this->never()); + + $this->expectException(InvalidConfigurationException::class); + $this->expectExceptionMessage('Unknown process test.unknown'); + + $tester->execute(['processCodes' => ['test.process', 'test.unknown']]); + } + + public function testAnInvalidOutputFormatIsDetectedBeforeAnyExecution(): void + { + $tester = $this->createTester($this->never()); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage("Cannot handle data output with format 'xml'"); + + $tester->execute(['processCodes' => ['test.process'], '--output-format' => 'xml']); + } + + public function testTheDumpFormatIsOnlyForTheStandardOutput(): void + { + $tester = $this->createTester($this->never()); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage("Cannot handle data output with format 'dump'"); + + $tester->execute(['processCodes' => ['test.process'], '--output' => $this->outputFile, '--output-format' => 'dump']); + } + + public function testTheJsonOutputIsPrintedWithoutVerbosity(): void + { + $tester = $this->createTester($this->once(), ['foo' => 'bar']); + + self::assertSame(0, $tester->execute(['processCodes' => ['test.process'], '--output-format' => 'json-stream'])); + self::assertStringContainsString('{"foo":"bar"}', $tester->getDisplay()); + } + + public function testAnArrayOutputIsStoredInTheFile(): void + { + $tester = $this->createTester($this->once(), ['foo' => 'bar']); + + self::assertSame(0, $tester->execute(['processCodes' => ['test.process'], '--output' => $this->outputFile, '--output-format' => 'json-stream'])); + self::assertSame('{"foo":"bar"}', trim((string) file_get_contents($this->outputFile))); + } + + public function testANonArrayOutputIsReportedInsteadOfBeingSilentlyLost(): void + { + $tester = $this->createTester($this->once(), 'scalar'); + + self::assertSame(0, $tester->execute( + ['processCodes' => ['test.process'], '--output' => $this->outputFile, '--output-format' => 'json-stream'], + ['capture_stderr_separately' => true], + )); + self::assertFileDoesNotExist($this->outputFile); + self::assertStringContainsString( + "Output not stored in '{$this->outputFile}': the JSON stream format requires an array, string given", + $tester->getErrorOutput() + ); + } + + private function createTester(\PHPUnit\Framework\MockObject\Rule\InvocationOrder $executions, mixed $result = null): CommandTester + { + $processManager = $this->createMock(ProcessManager::class); + $processManager->expects($executions)->method('execute')->willReturn($result); + $registry = new ProcessConfigurationRegistry( + [ + 'test.process' => [ + 'options' => [], + 'entry_point' => null, + 'end_point' => null, + 'description' => '', + 'help' => '', + 'public' => true, + 'tasks' => [], + ], + ], + 'stop' + ); + + return new CommandTester(new ExecuteProcessCommand($processManager, new EventDispatcher(), $registry)); + } +}