diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b8712c..9ec4db1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ Latest * [#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. * [#97](https://github.com/cleverage/ui-process-bundle/issues/97) Align the mapping and the schema created by the migrations: `ProcessSchedule::$input` mapped as `VARCHAR(255)` as created by the migrations (was `TEXT`); `Version20261005120000` migration (MySQL / MariaDB, PostgreSQL) making `log_record.process_execution_id` `NOT NULL` (log records without process execution are deleted) and `process_execution.context` nullable, as in the mapping. * [#99](https://github.com/cleverage/ui-process-bundle/issues/99) Fix the migrations on PostgreSQL (they could not create a working schema): id columns created as identity columns (the sequences were not used: inserts failed with the IDENTITY generation), `process_schedule` created with the PostgreSQL syntax (`AUTO_INCREMENT` failed). PostgreSQL support documented. -* [#103](https://github.com/cleverage/ui-process-bundle/issues/103) When a process task clears the entity manager (shared with the UI bundle, e.g. `ClearEntityManagerTask`), the current process execution is attached again before being saved and before writing the logs: it was inserted again (an execution left `started` without logs, a duplicate one with the final status and all the logs). Add `ProcessExecutionManager::getManagedProcessExecution()` and `ProcessExecutionRepository::getManaged()`. Add tests. +* [#103](https://github.com/cleverage/ui-process-bundle/issues/103) When a process task clears the entity manager (shared with the UI bundle, e.g. `ClearEntityManagerTask`), the current process execution is attached again before being saved and before writing the logs: it was inserted again (an execution left `started` without logs, a duplicate one with the final status and all the logs). Add `ProcessExecutionManager::getManagedProcessExecution()` and `ProcessExecutionRepository::getManaged()`. When a process task closes the entity manager (Doctrine closes it after an error during a flush), it is reset (`ManagerRegistry::resetManager()`, new optional `ProcessExecutionRepository` argument) before saving the execution and writing the logs: the execution was left `started`, without end date nor logs. Add tests. * [#105](https://github.com/cleverage/ui-process-bundle/issues/105) `LogProcessFilter` ("Process" filter of the logs): the process codes condition is added with `andWhere()` (`where()` replaced the search clause, whose parameters stayed bound: 500 when searching with this filter), and the "is not" comparison is applied (it was ignored: the logs of the process were displayed). Add tests. * [#107](https://github.com/cleverage/ui-process-bundle/issues/107) `CronScheduler`: an error on a process schedule (e.g. `every 0 seconds`, accepted by the validator but not by the Symfony Scheduler) is logged and the schedule skipped; it skipped all the next schedules. Add tests. * [#109](https://github.com/cleverage/ui-process-bundle/issues/109) `ProcessExecutionCrudController::downloadLogFile()`: 404 when the log file no longer exists (`file_get_contents()` warning: 500). Add tests. diff --git a/config/services/repository.yaml b/config/services/repository.yaml index 7269f02..f3d9c37 100644 --- a/config/services/repository.yaml +++ b/config/services/repository.yaml @@ -4,6 +4,7 @@ services: public: false arguments: - '@doctrine.orm.entity_manager' + - '@doctrine' cleverage_ui_process.repository.process_schedule: class: CleverAge\UiProcessBundle\Repository\ProcessScheduleRepository diff --git a/src/Monolog/Handler/DoctrineProcessHandler.php b/src/Monolog/Handler/DoctrineProcessHandler.php index 5b50acc..ca564df 100644 --- a/src/Monolog/Handler/DoctrineProcessHandler.php +++ b/src/Monolog/Handler/DoctrineProcessHandler.php @@ -62,6 +62,11 @@ public function flush(): void if (!$this->enabled) { return; } + if ($this->records->isEmpty() && false === $this->em?->isOpen()) { + // Nothing to write, and a closed entity manager cannot be flushed (it is reset when writing the next logs + // or saving the process execution, see ProcessExecutionRepository::getManaged()) + return; + } $entities = []; $currentProcessExecution = $this->records->isEmpty() ? null : $this->processExecutionManager?->getManagedProcessExecution(); foreach ($this->records as $record) { diff --git a/src/Repository/ProcessExecutionRepository.php b/src/Repository/ProcessExecutionRepository.php index 57dfeb1..d7e0e7d 100644 --- a/src/Repository/ProcessExecutionRepository.php +++ b/src/Repository/ProcessExecutionRepository.php @@ -17,6 +17,7 @@ use CleverAge\UiProcessBundle\Entity\ProcessExecution; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityRepository; +use Doctrine\Persistence\ManagerRegistry; /** * @extends EntityRepository @@ -28,7 +29,7 @@ */ class ProcessExecutionRepository extends EntityRepository { - public function __construct(EntityManagerInterface $em) + public function __construct(EntityManagerInterface $em, private readonly ?ManagerRegistry $registry = null) { parent::__construct($em, $em->getClassMetadata(ProcessExecution::class)); } @@ -43,9 +44,16 @@ public function save(ProcessExecution $processExecution): void * The process execution managed by the entity manager: the given one, or the persisted one, updated with the state * of the given one, when it has been detached (e.g. the entity manager, shared with the process tasks, has been * cleared). Persisting a detached process execution would insert it again. + * + * A closed entity manager (e.g. closed by Doctrine after an error during a flush of a process task) is reset + * first: the process execution could not be saved anymore. */ public function getManaged(ProcessExecution $processExecution): ProcessExecution { + if (!$this->getEntityManager()->isOpen()) { + // Reset in place (lazy service): the services holding the entity manager get an open one + $this->registry?->resetManager(); + } $id = $processExecution->getId(); if (null === $id || $this->getEntityManager()->contains($processExecution)) { return $processExecution; diff --git a/tests/Manager/ProcessExecutionManagerEntityManagerTest.php b/tests/Manager/ProcessExecutionManagerEntityManagerTest.php index 0aa99c2..50f4955 100644 --- a/tests/Manager/ProcessExecutionManagerEntityManagerTest.php +++ b/tests/Manager/ProcessExecutionManagerEntityManagerTest.php @@ -26,6 +26,7 @@ use CleverAge\UiProcessBundle\Tests\App\TestKernel; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Tools\SchemaTool; +use Doctrine\Persistence\ManagerRegistry; use Monolog\Level; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\UsesClass; @@ -33,7 +34,8 @@ /** * The entity manager is shared with the process tasks, which may clear it (e.g. ClearEntityManagerTask): the current - * process execution is then detached. Entity manager of the test application (SQLite). + * process execution is then detached; or close it (Doctrine closes it after an error during a flush). Entity manager of + * the test application (SQLite). */ #[CoversClass(ProcessExecutionManager::class)] #[CoversClass(ProcessExecutionRepository::class)] @@ -107,6 +109,53 @@ public function testEntityManagerClearedDuringTheProcess(): void self::assertSame(0, (int) $connection->fetchOne('SELECT COUNT(*) FROM log_record WHERE process_execution_id <> ?', [$executions[0]['id']])); } + public function testEntityManagerClosedDuringTheProcess(): void + { + /** @var ManagerRegistry $registry */ + $registry = static::getContainer()->get('doctrine'); + $manager = new ProcessExecutionManager(new ProcessExecutionRepository($this->entityManager, $registry)); + $handler = new DoctrineProcessHandler(); + $handler->setEntityManager($this->entityManager); + $handler->setProcessExecutionManager($manager); + + // Process start (ProcessEventSubscriber::onProcessStart()) + $manager->setCurrentProcessExecution(new ProcessExecution('test.process', 'test.log', ['key' => 'value']))->save(); + $handler->handle($this->createRecord('before the close')); + $handler->flush(); + $manager->increment('Warning'); + + // A process task closes the entity manager (Doctrine closes it after an error during a flush) + $this->entityManager->close(); + + $manager->increment('Warning'); + $handler->handle($this->createRecord('after the close')); + $handler->flush(); + + // Process end (ProcessEventSubscriber::success()) + $manager->getCurrentProcessExecution()?->setStatus(ProcessExecutionStatus::Finish); + $manager->getCurrentProcessExecution()?->end(); + $manager->save(); + $handler->flush(); // no record, nothing to write + $handler->disable(); + + // The entity manager has been reset in place: open again for the services holding it + self::assertTrue($this->entityManager->isOpen()); + self::assertSame($this->entityManager, static::getContainer()->get('doctrine.orm.entity_manager')); + + // A single execution, with its final state and all its logs + $connection = $this->entityManager->getConnection(); + $executions = $connection->fetchAllAssociative('SELECT id, status, end_date, report, context FROM process_execution'); + self::assertCount(1, $executions); + self::assertSame('finish', $executions[0]['status']); + self::assertNotNull($executions[0]['end_date']); + self::assertSame(['Warning' => 2], json_decode((string) $executions[0]['report'], true)); + self::assertSame(['key' => 'value'], json_decode((string) $executions[0]['context'], true)); + self::assertSame( + ['before the close', 'after the close'], + $connection->fetchFirstColumn('SELECT message FROM log_record WHERE process_execution_id = ? ORDER BY id', [$executions[0]['id']]) + ); + } + public function testProcessExecutionNotDetached(): void { $repository = new ProcessExecutionRepository($this->entityManager); diff --git a/tests/Monolog/Handler/DoctrineProcessHandlerTest.php b/tests/Monolog/Handler/DoctrineProcessHandlerTest.php index ed5a037..aa21f8d 100644 --- a/tests/Monolog/Handler/DoctrineProcessHandlerTest.php +++ b/tests/Monolog/Handler/DoctrineProcessHandlerTest.php @@ -36,6 +36,7 @@ public function testRecordsArePersistedOnFlush(): void /** @var \ArrayObject $persisted */ $persisted = new \ArrayObject(); $entityManager = $this->createMock(EntityManagerInterface::class); + $entityManager->method('isOpen')->willReturn(true); $entityManager->expects(self::exactly(2)) ->method('persist') ->willReturnCallback(static function (object $entity) use ($persisted): void { @@ -110,6 +111,27 @@ public function testRecordsAreDroppedWithoutProcessExecution(): void $handler->disable(); } + public function testClosedEntityManagerWithoutRecords(): void + { + $entityManager = $this->createMock(EntityManagerInterface::class); + $entityManager->method('isOpen')->willReturn(false); + $entityManager->expects(self::never())->method('flush'); + + $handler = $this->createHandler($entityManager, new ProcessExecution('test.process', 'test.log')); + $handler->flush(); + } + + public function testOpenEntityManagerWithoutRecords(): void + { + $entityManager = $this->createMock(EntityManagerInterface::class); + $entityManager->method('isOpen')->willReturn(true); + $entityManager->expects(self::once())->method('flush'); + + $handler = $this->createHandler($entityManager, new ProcessExecution('test.process', 'test.log')); + $handler->flush(); + $handler->disable(); + } + public function testDisabledHandler(): void { $entityManager = $this->createMock(EntityManagerInterface::class);