diff --git a/CHANGELOG.md b/CHANGELOG.md index 12ef068..678e064 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,13 +1,16 @@ Latest ------ -v3.0.2 ------- - -## Fixes -* [GHSA-r3m7-69c2-2vmp](…/security/advisories/GHSA-r3m7-69c2-2vmp) ProcessScheduleCrudController: require `ROLE_USER`, so that the scheduler -pages require an authenticated user again (since EasyAdmin 5, the `#[IsGranted]` of the dashboard does not apply to the CRUD routes); -ProcessExecuteController: same attribute, as defense in depth. Add a test checking that every controller is protected. +## Changes +* [#83](https://github.com/cleverage/ui-process-bundle/issues/83) Remove `UploadAndExecuteAction` (route `process_upload_and_execute`), `ProcessUploadFileType` and `ProcessConfigurationValueResolver`: the action was broken and unreachable, file uploads are handled by `LaunchAction` (`entrypoint_type: file`). +* [#84](https://github.com/cleverage/ui-process-bundle/issues/84) Add `LogRecord::hasContext()`, deprecate the misnamed `LogRecord::contextIsEmpty()` (it returns `true` when the context is not empty). Add tests. +* [#85](https://github.com/cleverage/ui-process-bundle/issues/85) `ProcessHandler`: default report increment level aligned on the bundle configuration (`Warning`); declare the `symfony/ux-twig-component` dependency. Add tests. + +v3.0.2 +------ + +## Fixes +* [GHSA-r3m7-69c2-2vmp](https://github.com/cleverage/ui-process-bundle/security/advisories/GHSA-r3m7-69c2-2vmp) ProcessScheduleCrudController: require `ROLE_USER`, so that the scheduler pages require an authenticated user again (since EasyAdmin 5, the `#[IsGranted]` of the dashboard does not apply to the CRUD routes); ProcessExecuteController: same attribute, as defense in depth. Add a test checking that every controller is protected. v3.0.1 ------ diff --git a/composer.json b/composer.json index 4edd666..c538091 100644 --- a/composer.json +++ b/composer.json @@ -74,6 +74,7 @@ "symfony/translation": "^6.4 || ^7.4 || ^8", "symfony/translation-contracts": "^3", "symfony/uid": "^6.4 || ^7.4 || ^8", + "symfony/ux-twig-component": "^2.32 || ^3.0", "symfony/validator": "^6.4 || ^7.4 || ^8" }, "require-dev": { diff --git a/config/services/http_value_resolver.yaml b/config/services/http_value_resolver.yaml index 3f59048..a11c517 100644 --- a/config/services/http_value_resolver.yaml +++ b/config/services/http_value_resolver.yaml @@ -6,10 +6,3 @@ services: arguments: - '%upload_directory%' - '@serializer' - - cleverage_ui_process.http_value_resolver.process_configuration: - class: CleverAge\UiProcessBundle\Http\ValueResolver\ProcessConfigurationValueResolver - autoconfigure: true - public: false - arguments: - - '@cleverage_process.registry.process_configuration' diff --git a/src/Controller/Admin/LogRecordCrudController.php b/src/Controller/Admin/LogRecordCrudController.php index 95fab08..a7a9aea 100644 --- a/src/Controller/Admin/LogRecordCrudController.php +++ b/src/Controller/Admin/LogRecordCrudController.php @@ -58,7 +58,7 @@ public function configureFields(string $pageName): iterable DateTimeField::new('createdAt')->setFormat('short', 'medium'), ContextField::new('context') ->onlyOnDetail(), - BooleanField::new('contextIsEmpty', 'Has context info ?') + BooleanField::new('hasContext', 'Has context info ?') ->onlyOnIndex() ->renderAsSwitch(false), ]; diff --git a/src/Controller/Admin/Process/UploadAndExecuteAction.php b/src/Controller/Admin/Process/UploadAndExecuteAction.php deleted file mode 100644 index 17ec45c..0000000 --- a/src/Controller/Admin/Process/UploadAndExecuteAction.php +++ /dev/null @@ -1,85 +0,0 @@ - '\w+'], - methods: ['POST', 'GET'] -)] -#[IsGranted('ROLE_USER')] -class UploadAndExecuteAction extends AbstractController -{ - public function __construct( - private readonly RequestStack $requestStack, - private readonly MessageBusInterface $messageBus, - ) { - } - - public function __invoke( - string $uploadDirectory, - #[ValueResolver('process')] ProcessConfiguration $processConfiguration, - ): Response { - if (!$processConfiguration->getEntryPoint() instanceof TaskConfiguration) { - throw new \RuntimeException('You must set an entry_point.'); - } - $form = $this->createForm( - ProcessUploadFileType::class, - null, - ['process_code' => $this->requestStack->getMainRequest()?->request->get('process')] - ); - $form->handleRequest($this->requestStack->getMainRequest()); - if ($form->isSubmitted() && $form->isValid()) { - /** @var UploadedFile $file */ - $file = $form->getData(); - $savedFilepath = \sprintf('%s/%s.%s', $uploadDirectory, Uuid::v4(), $file->getClientOriginalExtension()); - (new Filesystem())->dumpFile($savedFilepath, $file->getContent()); - $this->messageBus->dispatch( - new ProcessExecuteMessage( - $form->getConfig()->getOption('process_code'), - $savedFilepath - ) - ); - $this->addFlash( - 'success', - 'Process has been added to queue. It will start as soon as possible' - ); - - return $this->redirectToRoute('process', ['routeName' => 'process_list']); - } - - return $this->render( - '@CleverAgeUiProcess/admin/process/upload_and_execute.html.twig', - [ - 'form' => $form->createView(), - ] - ); - } -} diff --git a/src/Controller/Admin/ProcessScheduleCrudController.php b/src/Controller/Admin/ProcessScheduleCrudController.php index 05c5825..b0497eb 100644 --- a/src/Controller/Admin/ProcessScheduleCrudController.php +++ b/src/Controller/Admin/ProcessScheduleCrudController.php @@ -120,6 +120,11 @@ public function index(AdminContext $context): KeyValueStore|RedirectResponse|Res return parent::index($context); } + /** + * Only detects a worker running on the same host as the web server (it looks for it in the local processes): with + * workers on another host or container, the warning is displayed although the scheduler runs (see + * docs/reference/05-scheduler.md). + */ private function schedulerIsRunning(): bool { $process = Process::fromShellCommandline('ps -faux'); diff --git a/src/Entity/LogRecord.php b/src/Entity/LogRecord.php index 83eea47..c1afe75 100644 --- a/src/Entity/LogRecord.php +++ b/src/Entity/LogRecord.php @@ -61,11 +61,19 @@ public function __construct( $this->createdAt = $record->datetime; } - public function contextIsEmpty(): bool + public function hasContext(): bool { return [] !== $this->context; } + /** + * @deprecated misnamed, returns true when the context is NOT empty: use hasContext() instead + */ + public function contextIsEmpty(): bool + { + return $this->hasContext(); + } + public function getProcessExecution(): ProcessExecution { return $this->processExecution; diff --git a/src/Form/Type/ProcessUploadFileType.php b/src/Form/Type/ProcessUploadFileType.php deleted file mode 100644 index 94beef2..0000000 --- a/src/Form/Type/ProcessUploadFileType.php +++ /dev/null @@ -1,33 +0,0 @@ - */ -class ProcessUploadFileType extends AbstractType -{ - public function configureOptions(OptionsResolver $resolver): void - { - $resolver->setRequired('process_code'); - } - - #[\Override] - public function getParent(): string - { - return FileType::class; - } -} diff --git a/src/Http/ValueResolver/ProcessConfigurationValueResolver.php b/src/Http/ValueResolver/ProcessConfigurationValueResolver.php deleted file mode 100644 index 9412d3f..0000000 --- a/src/Http/ValueResolver/ProcessConfigurationValueResolver.php +++ /dev/null @@ -1,37 +0,0 @@ - - */ - public function resolve(Request $request, ArgumentMetadata $argument): iterable - { - return [$this->registry->getProcessConfiguration((string) $request->request->get('process'))]; - } -} diff --git a/src/Monolog/Handler/ProcessHandler.php b/src/Monolog/Handler/ProcessHandler.php index a086e88..b1f2d39 100644 --- a/src/Monolog/Handler/ProcessHandler.php +++ b/src/Monolog/Handler/ProcessHandler.php @@ -20,7 +20,7 @@ class ProcessHandler extends StreamHandler { - private Level $reportIncrementLevel = Level::Error; + private Level $reportIncrementLevel = Level::Warning; private bool $filenameSet = false; public function __construct( diff --git a/tests/Entity/LogRecordTest.php b/tests/Entity/LogRecordTest.php new file mode 100644 index 0000000..7ba2fd6 --- /dev/null +++ b/tests/Entity/LogRecordTest.php @@ -0,0 +1,50 @@ +createLogRecord(['file' => 'a.txt'])->hasContext()); + self::assertFalse($this->createLogRecord([])->hasContext()); + } + + public function testDeprecatedContextIsEmptyIsUnchanged(): void + { + // Misnamed: returns true when the context is NOT empty + self::assertTrue($this->createLogRecord(['file' => 'a.txt'])->contextIsEmpty()); + self::assertFalse($this->createLogRecord([])->contextIsEmpty()); + } + + /** + * @param array $context + */ + private function createLogRecord(array $context): LogRecord + { + return new LogRecord( + new \Monolog\LogRecord(new \DateTimeImmutable(), 'cleverage_process', Level::Info, 'message', $context), + new ProcessExecution('demo.process', 'demo.process.log') + ); + } +} diff --git a/tests/Monolog/Handler/ProcessHandlerTest.php b/tests/Monolog/Handler/ProcessHandlerTest.php new file mode 100644 index 0000000..f39c6e5 --- /dev/null +++ b/tests/Monolog/Handler/ProcessHandlerTest.php @@ -0,0 +1,84 @@ +directory = sys_get_temp_dir().'/'.uniqid('process_handler_test_', true); + mkdir($this->directory); + } + + protected function tearDown(): void + { + array_map(unlink(...), glob($this->directory.'/*') ?: []); + rmdir($this->directory); + } + + public function testWarningsAreCountedByDefault(): void + { + // Same default as the bundle configuration (logs.report_increment_level) + $manager = $this->createMock(ProcessExecutionManager::class); + $manager->expects(self::once())->method('increment')->with('Warning'); + + $handler = new ProcessHandler($this->directory, $manager); + $handler->setFilename('process.log'); + $handler->handle($this->createRecord(Level::Info)); + $handler->handle($this->createRecord(Level::Warning)); + + self::assertTrue($handler->hasFilename()); + self::assertStringContainsString('warning message', (string) file_get_contents($this->directory.'/process.log')); + } + + public function testReportIncrementLevel(): void + { + $manager = $this->createMock(ProcessExecutionManager::class); + $manager->expects(self::once())->method('increment')->with('Error'); + + $handler = new ProcessHandler($this->directory, $manager); + $handler->setReportIncrementLevel('Error'); + $handler->setFilename('process.log'); + $handler->handle($this->createRecord(Level::Warning)); + $handler->handle($this->createRecord(Level::Error)); + } + + public function testNothingIsWrittenWithoutFilename(): void + { + $manager = $this->createMock(ProcessExecutionManager::class); + $manager->expects(self::never())->method('increment'); + + $handler = new ProcessHandler($this->directory, $manager); + $handler->handle($this->createRecord(Level::Error)); + $handler->close(); + + self::assertFalse($handler->hasFilename()); + self::assertSame([], glob($this->directory.'/*')); + } + + private function createRecord(Level $level): LogRecord + { + return new LogRecord(new \DateTimeImmutable(), 'cleverage_process', $level, strtolower($level->name).' message'); + } +}