fix #95 Detach the written log entities without detaching the process execution

CHANGELOG.md

@@ -10,6 +10,7 @@ Latest
 ## Fixes
 * [#89](https://github.com/cleverage/ui-process-bundle/issues/89) `ProcessConfigurationsManager`: resolve the `ui.default` option with a normalizer instead of nested options defined with `setDefault()` (deprecated since symfony/options-resolver 7.3, removed in 8.0). With Symfony 8, a process launched with the UI form (`ui_launch_mode: form`) without `ui.default` no longer fails (`Cannot use object of type Closure as array`), and `ui.default` is validated again. Add tests.
 * [#91](https://github.com/cleverage/ui-process-bundle/issues/91) `LoginController`: pass `error` and `last_username` (`AuthenticationUtils`) to the login template, so that a failed login displays the error message and keeps the email. Test updated.
+* [#95](https://github.com/cleverage/ui-process-bundle/issues/95) `DoctrineProcessHandler`: detach the written `LogRecord` entities after each flush (the Monolog records were detached instead), so that the identity map no longer grows during long processes; `LogRecord::$processExecution` cascade reduced from `all` to `persist`, so that detaching a log record does not detach the current process execution (which would then be inserted again). Add tests.
 
 v3.0.2
 ------

src/Entity/LogRecord.php

@@ -50,7 +50,7 @@ public function getId(): ?int
 
     public function __construct(
         \Monolog\LogRecord $record,
-        #[ORM\ManyToOne(targetEntity: ProcessExecution::class, cascade: ['all'])]
+        #[ORM\ManyToOne(targetEntity: ProcessExecution::class, cascade: ['persist'])]
         #[ORM\JoinColumn(name: 'process_execution_id', referencedColumnName: 'id', nullable: false, onDelete: 'CASCADE')]
         private readonly ProcessExecution $processExecution,
     ) {

src/Monolog/Handler/DoctrineProcessHandler.php

@@ -62,15 +62,19 @@ public function flush(): void
         if (!$this->enabled) {
             return;
         }
+        $entities = [];
         foreach ($this->records as $record) {
             if (($currentProcessExecution = $this->processExecutionManager?->getCurrentProcessExecution()) instanceof ProcessExecution) {
                 $entity = new \CleverAge\UiProcessBundle\Entity\LogRecord($record, $currentProcessExecution);
                 $this->em?->persist($entity);
+                $entities[] = $entity;
             }
         }
         $this->em?->flush();
-        foreach ($this->records as $record) {
-            $this->em?->detach($record);
+        // Written log records are no longer needed: detached so that the identity map does not grow during long
+        // processes (detach is not cascaded to the process execution, see LogRecord::$processExecution)
+        foreach ($entities as $entity) {
+            $this->em?->detach($entity);
         }
         $this->records = new ArrayCollection();
     }

tests/Monolog/Handler/DoctrineProcessHandlerEntityManagerTest.php

@@ -0,0 +1,104 @@
+<?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\CleverAgeUiProcessBundle;
+use CleverAge\UiProcessBundle\DependencyInjection\CleverAgeUiProcessExtension;
+use CleverAge\UiProcessBundle\DependencyInjection\Configuration;
+use CleverAge\UiProcessBundle\Entity\Enum\ProcessExecutionStatus;
+use CleverAge\UiProcessBundle\Entity\LogRecord;
+use CleverAge\UiProcessBundle\Entity\ProcessExecution;
+use CleverAge\UiProcessBundle\EventSubscriber\ProcessEventSubscriber;
+use CleverAge\UiProcessBundle\Manager\ProcessExecutionManager;
+use CleverAge\UiProcessBundle\Monolog\Handler\DoctrineProcessHandler;
+use CleverAge\UiProcessBundle\Repository\ProcessExecutionRepository;
+use CleverAge\UiProcessBundle\Tests\App\TestKernel;
+use Doctrine\ORM\EntityManagerInterface;
+use Doctrine\ORM\Tools\SchemaTool;
+use Monolog\Level;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\UsesClass;
+use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
+
+/**
+ * DoctrineProcessHandler with the entity manager of the test application (SQLite).
+ */
+#[CoversClass(DoctrineProcessHandler::class)]
+#[UsesClass(CleverAgeUiProcessBundle::class)]
+#[UsesClass(CleverAgeUiProcessExtension::class)]
+#[UsesClass(Configuration::class)]
+#[UsesClass(ProcessEventSubscriber::class)]
+#[UsesClass(LogRecord::class)]
+#[UsesClass(ProcessExecution::class)]
+#[UsesClass(ProcessExecutionManager::class)]
+#[UsesClass(ProcessExecutionRepository::class)]
+class DoctrineProcessHandlerEntityManagerTest extends KernelTestCase
+{
+    private EntityManagerInterface $entityManager;
+
+    protected function setUp(): void
+    {
+        self::bootKernel(['debug' => false]);
+
+        /** @var EntityManagerInterface $entityManager */
+        $entityManager = static::getContainer()->get('doctrine.orm.entity_manager');
+        $this->entityManager = $entityManager;
+        $schemaTool = new SchemaTool($entityManager);
+        $metadata = $entityManager->getMetadataFactory()->getAllMetadata();
+        $schemaTool->dropSchema($metadata);
+        $schemaTool->createSchema($metadata);
+    }
+
+    protected static function getKernelClass(): string
+    {
+        return TestKernel::class;
+    }
+
+    public function testWrittenLogRecordsAreDetached(): void
+    {
+        $processExecutionManager = new ProcessExecutionManager(new ProcessExecutionRepository($this->entityManager));
+        $processExecution = new ProcessExecution('test.process', 'test.log');
+        $processExecutionManager->setCurrentProcessExecution($processExecution)->save();
+        $handler = new DoctrineProcessHandler();
+        $handler->setEntityManager($this->entityManager);
+        $handler->setProcessExecutionManager($processExecutionManager);
+
+        for ($i = 0; $i < 3; ++$i) {
+            for ($j = 0; $j < 100; ++$j) {
+                $handler->handle(new \Monolog\LogRecord(new \DateTimeImmutable(), 'cleverage_process', Level::Warning, 'message '.$j));
+            }
+            $handler->flush();
+
+            // The identity map does not grow with the written log records, the process execution is still managed
+            self::assertSame([], $this->entityManager->getUnitOfWork()->getIdentityMap()[LogRecord::class] ?? []);
+            self::assertTrue($this->entityManager->contains($processExecution));
+        }
+        $handler->disable();
+
+        // The process execution is updated, not inserted again
+        $processExecution->setStatus(ProcessExecutionStatus::Finish);
+        $processExecution->end();
+        $processExecutionManager->save();
+
+        $connection = $this->entityManager->getConnection();
+        self::assertSame(
+            [['id' => $processExecution->getId(), 'status' => 'finish']],
+            $connection->fetchAllAssociative('SELECT id, status FROM process_execution')
+        );
+        self::assertSame(
+            [['process_execution_id' => $processExecution->getId(), 'count' => 300]],
+            $connection->fetchAllAssociative('SELECT process_execution_id, COUNT(*) AS count FROM log_record GROUP BY process_execution_id')
+        );
+    }
+}

tests/Monolog/Handler/DoctrineProcessHandlerTest.php

@@ -43,6 +43,13 @@ public function testRecordsArePersistedOnFlush(): void
                 $persisted[] = $entity;
             });
         $entityManager->expects(self::exactly(2))->method('flush');
+        /** @var \ArrayObject<int, object> $detached */
+        $detached = new \ArrayObject();
+        $entityManager->expects(self::exactly(2))
+            ->method('detach')
+            ->willReturnCallback(static function (object $entity) use ($detached): void {
+                $detached[] = $entity;
+            });
 
         $handler = $this->createHandler($entityManager, $processExecution);
         $handler->handle($this->createRecord(Level::Info, 'first'));
@@ -57,6 +64,8 @@ public function testRecordsArePersistedOnFlush(): void
         self::assertSame($processExecution, $first->getProcessExecution());
         self::assertSame('second', $second->message);
         self::assertSame(Level::Error->value, $second->level);
+        // Only the persisted log entities are detached after the flush
+        self::assertSame($persisted->getArrayCopy(), $detached->getArrayCopy());
 
         // Records are flushed only once
         $handler->flush();
@@ -93,6 +102,7 @@ public function testRecordsAreDroppedWithoutProcessExecution(): void
         $entityManager = $this->createMock(EntityManagerInterface::class);
         $entityManager->expects(self::never())->method('persist');
         $entityManager->expects(self::once())->method('flush');
+        $entityManager->expects(self::never())->method('detach');
 
         $handler = $this->createHandler($entityManager, null);
         $handler->handle($this->createRecord(Level::Info, 'message'));