#83 #84 #85 Remove the broken UploadAndExecuteAction, LogRecord::hasContext(), ProcessHandler default level, declare symfony/ux-twig-component

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

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": {

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'

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),
         ];

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');

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;

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(

tests/Entity/LogRecordTest.php

@@ -0,0 +1,50 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the CleverAge/UiProcessBundle package.
+ *
+ * Copyright (c) Clever-Age
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace CleverAge\UiProcessBundle\Tests\Entity;
+
+use CleverAge\UiProcessBundle\Entity\LogRecord;
+use CleverAge\UiProcessBundle\Entity\ProcessExecution;
+use Monolog\Level;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\UsesClass;
+use PHPUnit\Framework\TestCase;
+
+#[CoversClass(LogRecord::class)]
+#[UsesClass(ProcessExecution::class)]
+class LogRecordTest extends TestCase
+{
+    public function testHasContext(): void
+    {
+        self::assertTrue($this->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<mixed> $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')
+        );
+    }
+}

tests/Monolog/Handler/ProcessHandlerTest.php

@@ -0,0 +1,84 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the CleverAge/UiProcessBundle package.
+ *
+ * Copyright (c) Clever-Age
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace CleverAge\UiProcessBundle\Tests\Monolog\Handler;
+
+use CleverAge\UiProcessBundle\Manager\ProcessExecutionManager;
+use CleverAge\UiProcessBundle\Monolog\Handler\ProcessHandler;
+use Monolog\Level;
+use Monolog\LogRecord;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\TestCase;
+
+#[CoversClass(ProcessHandler::class)]
+class ProcessHandlerTest extends TestCase
+{
+    private string $directory;
+
+    protected function setUp(): void
+    {
+        $this->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');
+    }
+}