diff --git a/CHANGELOG.md b/CHANGELOG.md index 815f2c3d..ed5765d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ Latest ------ ## Changes +* [#318](https://github.com/cleverage/process-bundle/issues/318) `CommandRunnerTask`, `ProcessLauncherTask` and `SubprocessInstance` receive the project directory, the path of the console and the environment (`%kernel.project_dir%`, `%kernel.environment%`) instead of the kernel: passing a `KernelInterface` is deprecated, the arguments will only accept strings in v6.0. `SubprocessInstance` writes the JSON buffer of `ProcessLauncherTask` (`json_buffering`) in the system temporary directory instead of `var/` of the project (which may be read-only), no longer creates the unused `/process` directory, and omits `--env` when no environment is given (default environment of the console). Update documentation, add tests. * [#317](https://github.com/cleverage/process-bundle/issues/317) The bundle throws exceptions implementing `ProcessExceptionInterface` instead of raw SPL exceptions (124 `throw`): new `Exception\UnexpectedValueException`, `RuntimeException`, `InvalidArgumentException` and `LogicException`, extending the SPL exceptions thrown before (same messages, the existing `catch` keep working). All the errors of the bundle can be caught with `ProcessExceptionInterface`, except the exceptions of the Symfony components. Update documentation, add tests. * [#311](https://github.com/cleverage/process-bundle/issues/311) Deprecate the `cleverage_process.event_listener.data_queue` service and `DataQueueEventListener` (it keeps a copy of the `ProcessState` of every `cleverage_process.data_queue` event in memory, without limit): injecting or fetching the service triggers a deprecation, they will be removed in v6.0. Write your own listener of the event dispatched by the `event_dispatcher` task. Update documentation, add tests. * [#312](https://github.com/cleverage/process-bundle/issues/312) Add read-only interfaces for the process manager and the registries, implemented by the existing classes and autowired by their interface: `ProcessManagerInterface` (`execute()`), `ProcessConfigurationRegistryInterface` (`getProcessConfiguration()`, `getProcessConfigurations()`, `hasProcessConfiguration()`), `TransformerRegistryInterface` (`getTransformer()`, `getTransformers()`, `hasTransformer()`) and `TaskRegistryInterface` (`getTask()`). Type your services on them: the classes will be `final` in v6.0 (the services of the bundle will then be typed on the interfaces). Update documentation, add tests. diff --git a/config/services/task.yaml b/config/services/task.yaml index d6df2447..7091a49c 100644 --- a/config/services/task.yaml +++ b/config/services/task.yaml @@ -235,7 +235,7 @@ services: - { name: cleverage.task, code: command_runner } shared: false arguments: - - '@kernel' + - '%kernel.project_dir%' CleverAge\ProcessBundle\Task\Process\ProcessExecutorTask: tags: @@ -253,7 +253,8 @@ services: arguments: - '@logger' - '@cleverage_process.registry.process_configuration' - - '@kernel' + - '%kernel.project_dir%/bin/console' + - '%kernel.environment%' CleverAge\ProcessBundle\Task\PropertyGetterTask: tags: diff --git a/docs/reference/tasks/process_launcher_task.md b/docs/reference/tasks/process_launcher_task.md index 6957c04b..9e3bec94 100644 --- a/docs/reference/tasks/process_launcher_task.md +++ b/docs/reference/tasks/process_launcher_task.md @@ -25,7 +25,7 @@ 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 (`var/cdm_buffer_*.json-stream`) and, once the sub-process is +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. @@ -67,5 +67,7 @@ parallel_import: Notes ----- -* Sub-processes are run with the same Symfony environment as the current one (`--env`). +* 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 incremental error output (stderr) of the running sub-processes is echoed directly. diff --git a/src/Model/SubprocessInstance.php b/src/Model/SubprocessInstance.php index 37d4db98..274b9f4e 100644 --- a/src/Model/SubprocessInstance.php +++ b/src/Model/SubprocessInstance.php @@ -38,29 +38,45 @@ class SubprocessInstance protected string $consolePath; - protected string $environment; - - protected string $logDir; + /** + * @deprecated since 5.4, not used; only set when the kernel is passed to the constructor + */ + protected ?string $logDir = null; /** - * @param array $context - * @param array $options + * @param string|KernelInterface $consolePath path of the Symfony console; passing the kernel is deprecated since 5.4 + * @param array $context + * @param array $options + * @param string|null $environment environment of the sub-process (--env), the default one of the console + * if null; taken from the kernel when it is passed */ public function __construct( - KernelInterface $kernel, + KernelInterface|string $consolePath, protected string $processCode, protected ?string $input, protected array $context = [], array $options = [], + protected ?string $environment = null, ) { $resolver = new OptionsResolver(); $this->configureOptions($resolver); $this->options = $resolver->resolve($options); - $this->consolePath = $kernel->getProjectDir().'/bin/console'; - $this->environment = $kernel->getEnvironment(); - $this->bufferPath = $kernel->getProjectDir().'/var/cdm_buffer_'.uniqid('', true).'.json-stream'; - $this->logDir = $kernel->getLogDir().'/process'; + if ($consolePath instanceof KernelInterface) { + trigger_deprecation( + 'cleverage/process-bundle', + '5.4', + 'Passing a "%s" to "%s::__construct()" is deprecated, pass the path of the console and the environment instead. The argument will only accept a string in v6.0.', + KernelInterface::class, + self::class, + ); + $this->environment ??= $consolePath->getEnvironment(); + $this->logDir = $consolePath->getLogDir().'/process'; + $consolePath = $consolePath->getProjectDir().'/bin/console'; + } + $this->consolePath = $consolePath; + // The project directory may be read-only (e.g. in a container) + $this->bufferPath = sys_get_temp_dir().'/cdm_buffer_'.uniqid('', true).'.json-stream'; } /** @@ -77,13 +93,12 @@ public function buildProcess(): static 'nohup', $phpBinary, $this->consolePath, - '--env='.$this->environment, + ...(null !== $this->environment ? ['--env='.$this->environment] : []), 'cleverage:process:execute', '--input-from-stdin', ]; $fs = new Filesystem(); - $fs->mkdir($this->logDir); if (!$fs->exists($this->consolePath)) { throw new RuntimeException("Unable to resolve path to symfony console '{$this->consolePath}'"); } diff --git a/src/Task/Process/CommandRunnerTask.php b/src/Task/Process/CommandRunnerTask.php index 9482d8eb..da324256 100644 --- a/src/Task/Process/CommandRunnerTask.php +++ b/src/Task/Process/CommandRunnerTask.php @@ -26,9 +26,31 @@ */ class CommandRunnerTask extends AbstractConfigurableTask { - public function __construct( - protected KernelInterface $kernel, - ) { + /** + * @deprecated since 5.4, only set when the kernel is passed to the constructor + */ + protected ?KernelInterface $kernel = null; + + protected string $projectDir; + + /** + * @param string|KernelInterface $projectDir default working directory of the commands; passing the kernel is + * deprecated since 5.4 + */ + public function __construct(KernelInterface|string $projectDir) + { + if ($projectDir instanceof KernelInterface) { + trigger_deprecation( + 'cleverage/process-bundle', + '5.4', + 'Passing a "%s" to "%s::__construct()" is deprecated, pass the project directory instead. The argument will only accept a string in v6.0.', + KernelInterface::class, + self::class, + ); + $this->kernel = $projectDir; + $projectDir = $projectDir->getProjectDir(); + } + $this->projectDir = $projectDir; } public function execute(ProcessState $state): void @@ -56,7 +78,7 @@ protected function configureOptions(OptionsResolver $resolver): void $resolver->setAllowedTypes('commandline', ['string', 'array']); $resolver->setDefaults( [ - 'cwd' => $this->kernel->getProjectDir(), // This method is not actually in the interface, this is bad + 'cwd' => $this->projectDir, 'env' => null, 'timeout' => 60, 'options' => null, diff --git a/src/Task/Process/ProcessLauncherTask.php b/src/Task/Process/ProcessLauncherTask.php index 227c4b0b..2d8a079f 100644 --- a/src/Task/Process/ProcessLauncherTask.php +++ b/src/Task/Process/ProcessLauncherTask.php @@ -46,12 +46,39 @@ class ProcessLauncherTask extends AbstractConfigurableTask implements FlushableT protected bool $flushMode = false; + /** + * @deprecated since 5.4, only set when the kernel is passed to the constructor + */ + protected ?KernelInterface $kernel = null; + + protected string $consolePath; + + /** + * @param string|KernelInterface $consolePath path of the Symfony console running the sub-processes; passing the + * kernel is deprecated since 5.4 + * @param string|null $environment environment of the sub-processes (--env), the default one of the + * console if null; taken from the kernel when it is passed + */ public function __construct( protected LoggerInterface $logger, protected ProcessConfigurationRegistry $processRegistry, - protected KernelInterface $kernel, + KernelInterface|string $consolePath, + protected ?string $environment = null, ) { $this->finishedBuffers = new \SplQueue(); + if ($consolePath instanceof KernelInterface) { + trigger_deprecation( + 'cleverage/process-bundle', + '5.4', + 'Passing a "%s" to "%s::__construct()" is deprecated, pass the path of the console and the environment instead. The argument will only accept a string in v6.0.', + KernelInterface::class, + self::class, + ); + $this->kernel = $consolePath; + $this->environment ??= $consolePath->getEnvironment(); + $consolePath = $consolePath->getProjectDir().'/bin/console'; + } + $this->consolePath = $consolePath; } public function execute(ProcessState $state): void @@ -136,13 +163,14 @@ protected function launchProcess(ProcessState $state): SubprocessInstance $input = null !== $state->getInput() ? (string) $state->getInput() : null; $subprocess = new SubprocessInstance( - $this->kernel, + $this->consolePath, $this->getOption($state, 'process'), $input, $this->getOption($state, 'context'), [ SubprocessInstance::OPTION_JSON_BUFFERING => $this->getOption($state, 'json_buffering'), - ] + ], + $this->environment, ); return $subprocess->buildProcess() diff --git a/tests/Model/SubprocessInstanceTest.php b/tests/Model/SubprocessInstanceTest.php new file mode 100644 index 00000000..6e0a482c --- /dev/null +++ b/tests/Model/SubprocessInstanceTest.php @@ -0,0 +1,104 @@ +projectDir = sys_get_temp_dir().'/subprocess_instance_test_'.uniqid('', true); + mkdir($this->projectDir.'/bin', 0o777, true); + touch($this->projectDir.'/bin/console'); + } + + protected function tearDown(): void + { + unlink($this->projectDir.'/bin/console'); + rmdir($this->projectDir.'/bin'); + rmdir($this->projectDir); + } + + public function testCommandLine(): void + { + $subprocess = (new SubprocessInstance($this->projectDir.'/bin/console', 'child', 'input', ['key' => 'value'], [], 'prod')) + ->buildProcess(); + + $commandLine = $subprocess->getProcess()->getCommandLine(); + $this->assertStringContainsString("'{$this->projectDir}/bin/console' '--env=prod' 'cleverage:process:execute' '--input-from-stdin'", $commandLine); + $this->assertStringEndsWith("'--context=key:value' 'child'", $commandLine); + $this->assertSame('input', $subprocess->getProcess()->getInput()); + } + + public function testDefaultEnvironmentOfTheConsoleWithoutEnvironment(): void + { + $subprocess = (new SubprocessInstance($this->projectDir.'/bin/console', 'child', null))->buildProcess(); + + $this->assertStringNotContainsString('--env', $subprocess->getProcess()->getCommandLine()); + } + + public function testJsonBufferIsWrittenInTheTemporaryDirectory(): void + { + $subprocess = (new SubprocessInstance($this->projectDir.'/bin/console', 'child', null, [], [SubprocessInstance::OPTION_JSON_BUFFERING => true])) + ->buildProcess(); + + $this->assertMatchesRegularExpression( + '#\'--output='.preg_quote(sys_get_temp_dir(), '#').'/cdm_buffer_[^/\']+\.json-stream\' \'--output-format=json-stream\'#', + $subprocess->getProcess()->getCommandLine() + ); + } + + public function testMissingConsoleThrows(): void + { + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage("Unable to resolve path to symfony console '{$this->projectDir}/missing'"); + + (new SubprocessInstance($this->projectDir.'/missing', 'child', null))->buildProcess(); + } + + public function testPassingTheKernelIsDeprecated(): void + { + $kernel = $this->createStub(KernelInterface::class); + $kernel->method('getProjectDir')->willReturn($this->projectDir); + $kernel->method('getEnvironment')->willReturn('prod'); + $kernel->method('getLogDir')->willReturn($this->projectDir.'/var/log'); + $deprecations = []; + set_error_handler(static function (int $errno, string $errstr) use (&$deprecations): bool { + $deprecations[] = $errstr; + + return true; + }, \E_USER_DEPRECATED); + try { + $subprocess = new SubprocessInstance($kernel, 'child', null); + } finally { + restore_error_handler(); + } + + $this->assertSame( + ['Since cleverage/process-bundle 5.4: Passing a "Symfony\Component\HttpKernel\KernelInterface" to "CleverAge\ProcessBundle\Model\SubprocessInstance::__construct()" is deprecated, pass the path of the console and the environment instead. The argument will only accept a string in v6.0.'], + $deprecations + ); + $commandLine = $subprocess->buildProcess()->getProcess()->getCommandLine(); + $this->assertStringContainsString("'{$this->projectDir}/bin/console' '--env=prod'", $commandLine); + // The log directory of the kernel is no longer created + $this->assertDirectoryDoesNotExist($this->projectDir.'/var'); + } +} diff --git a/tests/Task/Process/CommandRunnerTaskTest.php b/tests/Task/Process/CommandRunnerTaskTest.php index b222c8b7..f339ac4f 100644 --- a/tests/Task/Process/CommandRunnerTaskTest.php +++ b/tests/Task/Process/CommandRunnerTaskTest.php @@ -75,6 +75,35 @@ public function testCwdAndEnvOptions(): void $this->assertSame('bar'.\PHP_EOL.realpath(sys_get_temp_dir()).\PHP_EOL, $state->getOutput()); } + public function testProjectDirIsTheDefaultCwd(): void + { + $state = $this->createState(['commandline' => 'pwd']); + + $this->createTask()->execute($state); + + $this->assertSame(realpath(sys_get_temp_dir()).\PHP_EOL, $state->getOutput()); + } + + public function testPassingTheKernelIsDeprecated(): void + { + $kernel = $this->createStub(KernelInterface::class); + $kernel->method('getProjectDir')->willReturn(sys_get_temp_dir()); + $task = null; + + $deprecations = $this->collectDeprecations(static function () use ($kernel, &$task): void { + $task = new CommandRunnerTask($kernel); + }); + + $this->assertSame( + ['Since cleverage/process-bundle 5.4: Passing a "Symfony\Component\HttpKernel\KernelInterface" to "CleverAge\ProcessBundle\Task\Process\CommandRunnerTask::__construct()" is deprecated, pass the project directory instead. The argument will only accept a string in v6.0.'], + $deprecations + ); + $this->assertInstanceOf(CommandRunnerTask::class, $task); + $state = $this->createState(['commandline' => 'pwd']); + $task->execute($state); + $this->assertSame(realpath(sys_get_temp_dir()).\PHP_EOL, $state->getOutput()); + } + public function testProcessOptionsArePassedToProcess(): void { $state = $this->createState([ @@ -108,10 +137,29 @@ public function testFailingCommandThrows(): void private function createTask(): CommandRunnerTask { - $kernel = $this->createStub(KernelInterface::class); - $kernel->method('getProjectDir')->willReturn(sys_get_temp_dir()); + return new CommandRunnerTask(sys_get_temp_dir()); + } - return new CommandRunnerTask($kernel); + /** + * @param callable(): mixed $callback + * + * @return list + */ + private function collectDeprecations(callable $callback): array + { + $deprecations = []; + set_error_handler(static function (int $errno, string $errstr) use (&$deprecations): bool { + $deprecations[] = $errstr; + + return true; + }, \E_USER_DEPRECATED); + try { + $callback(); + } finally { + restore_error_handler(); + } + + return $deprecations; } /** diff --git a/tests/Task/Process/ProcessLauncherTaskTest.php b/tests/Task/Process/ProcessLauncherTaskTest.php index 1d9ec9c0..49de82a3 100644 --- a/tests/Task/Process/ProcessLauncherTaskTest.php +++ b/tests/Task/Process/ProcessLauncherTaskTest.php @@ -65,14 +65,63 @@ public function testUnknownProcessThrows(): void $this->createTask()->initialize($state); } + public function testPassingTheKernelIsDeprecated(): void + { + $kernel = $this->createStub(KernelInterface::class); + $kernel->method('getProjectDir')->willReturn('/app'); + $kernel->method('getEnvironment')->willReturn('prod'); + $task = null; + + $deprecations = $this->collectDeprecations(function () use ($kernel, &$task): void { + $task = new ProcessLauncherTask(new NullLogger(), $this->createRegistry(), $kernel); + }); + + $this->assertSame( + ['Since cleverage/process-bundle 5.4: Passing a "Symfony\Component\HttpKernel\KernelInterface" to "CleverAge\ProcessBundle\Task\Process\ProcessLauncherTask::__construct()" is deprecated, pass the path of the console and the environment instead. The argument will only accept a string in v6.0.'], + $deprecations + ); + $this->assertInstanceOf(ProcessLauncherTask::class, $task); + $this->assertSame(['/app/bin/console', 'prod'], [ + (new \ReflectionProperty($task, 'consolePath'))->getValue($task), + (new \ReflectionProperty($task, 'environment'))->getValue($task), + ]); + } + private function createTask(): ProcessLauncherTask + { + return new ProcessLauncherTask(new NullLogger(), $this->createRegistry(), '/app/bin/console', 'test'); + } + + private function createRegistry(): ProcessConfigurationRegistry { $registry = $this->createStub(ProcessConfigurationRegistry::class); $registry->method('hasProcessConfiguration')->willReturnCallback( static fn (string $processCode): bool => 'child' === $processCode ); - return new ProcessLauncherTask(new NullLogger(), $registry, $this->createStub(KernelInterface::class)); + return $registry; + } + + /** + * @param callable(): mixed $callback + * + * @return list + */ + private function collectDeprecations(callable $callback): array + { + $deprecations = []; + set_error_handler(static function (int $errno, string $errstr) use (&$deprecations): bool { + $deprecations[] = $errstr; + + return true; + }, \E_USER_DEPRECATED); + try { + $callback(); + } finally { + restore_error_handler(); + } + + return $deprecations; } /**