diff --git a/CHANGELOG.md b/CHANGELOG.md index 49306f3..67b1346 100644 --- a/CHANGELOG.md +++ b/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 ------ diff --git a/src/Entity/LogRecord.php b/src/Entity/LogRecord.php index 045233d..ab10f2a 100644 --- a/src/Entity/LogRecord.php +++ b/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, ) { diff --git a/src/Monolog/Handler/DoctrineProcessHandler.php b/src/Monolog/Handler/DoctrineProcessHandler.php index 3c47678..d7e4409 100644 --- a/src/Monolog/Handler/DoctrineProcessHandler.php +++ b/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(); } diff --git a/tests/Monolog/Handler/DoctrineProcessHandlerEntityManagerTest.php b/tests/Monolog/Handler/DoctrineProcessHandlerEntityManagerTest.php new file mode 100644 index 0000000..12b25e2 --- /dev/null +++ b/tests/Monolog/Handler/DoctrineProcessHandlerEntityManagerTest.php @@ -0,0 +1,104 @@ + 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') + ); + } +} diff --git a/tests/Monolog/Handler/DoctrineProcessHandlerTest.php b/tests/Monolog/Handler/DoctrineProcessHandlerTest.php index 7e8bf08..6a0fcb9 100644 --- a/tests/Monolog/Handler/DoctrineProcessHandlerTest.php +++ b/tests/Monolog/Handler/DoctrineProcessHandlerTest.php @@ -43,6 +43,13 @@ public function testRecordsArePersistedOnFlush(): void $persisted[] = $entity; }); $entityManager->expects(self::exactly(2))->method('flush'); + /** @var \ArrayObject $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'));