Fix Canceling of stale workflow cronjob

Refactor workflow cancellation logic to encapsulate transition checks in a dedicated method, and update CronJob handling to use entity workflows instead of IDs. Enhance test coverage to ensure proper handling and instantiate mocks for EntityManagerInterface.

src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php

@@ -205,13 +205,13 @@ class EntityWorkflowRepository implements ObjectRepository
      *
      * @param \DateTimeImmutable $olderThanDate the date to compare against
      *
-     * @return list<int> the list of workflow IDs that meet the criteria
+     * @return iterable<EntityWorkflow>
      */
-    public function findWorkflowsWithoutFinalStepAndOlderThan(\DateTimeImmutable $olderThanDate): array
+    public function findWorkflowsWithoutFinalStepAndOlderThan(\DateTimeImmutable $olderThanDate): iterable
     {
         $qb = $this->repository->createQueryBuilder('sw');
 
-        $qb->select('sw.id')
+        $qb->select('sw')
             // only the workflow which are not finalized
             ->where(sprintf('NOT EXISTS (SELECT 1 FROM %s ews WHERE ews.isFinal = TRUE AND ews.entityWorkflow = sw.id)', EntityWorkflowStep::class))
             ->andWhere(
@@ -227,7 +227,7 @@ class EntityWorkflowRepository implements ObjectRepository
             ->setParameter('initial', 'initial')
         ;
 
-        return $qb->getQuery()->getResult();
+        return $qb->getQuery()->toIterable();
     }
 
     public function getClassName(): string

src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php

@@ -14,6 +14,7 @@ namespace Chill\MainBundle\Service\Workflow;
 use Chill\MainBundle\Cron\CronJobInterface;
 use Chill\MainBundle\Entity\CronJobExecution;
 use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository;
+use Doctrine\ORM\EntityManagerInterface;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\Clock\ClockInterface;
 use Symfony\Component\Messenger\MessageBusInterface;
@@ -31,6 +32,7 @@ class CancelStaleWorkflowCronJob implements CronJobInterface
         private readonly ClockInterface $clock,
         private readonly MessageBusInterface $messageBus,
         private readonly LoggerInterface $logger,
+        private readonly EntityManagerInterface $entityManager,
     ) {}
 
     public function canRun(?CronJobExecution $cronJobExecution): bool
@@ -48,19 +50,23 @@ class CancelStaleWorkflowCronJob implements CronJobInterface
         $this->logger->info('Cronjob started: Canceling stale workflows.');
 
         $olderThanDate = $this->clock->now()->sub(new \DateInterval(self::KEEP_INTERVAL));
-        $staleWorkflowIds = $this->workflowRepository->findWorkflowsWithoutFinalStepAndOlderThan($olderThanDate);
+        $staleEntityWorkflows = $this->workflowRepository->findWorkflowsWithoutFinalStepAndOlderThan($olderThanDate);
         $lastCanceled = $lastExecutionData[self::LAST_CANCELED_WORKFLOW] ?? 0;
         $processedCount = 0;
 
-        foreach ($staleWorkflowIds as $wId) {
+        foreach ($staleEntityWorkflows as $staleEntityWorkflow) {
             try {
-                $this->messageBus->dispatch(new CancelStaleWorkflowMessage($wId));
-                $lastCanceled = max($wId, $lastCanceled);
+                $this->messageBus->dispatch(new CancelStaleWorkflowMessage($staleEntityWorkflow->getId()));
+                $lastCanceled = max($staleEntityWorkflow->getId(), $lastCanceled);
                 ++$processedCount;
             } catch (\Exception $e) {
-                $this->logger->error("Failed to dispatch CancelStaleWorkflow for ID {$wId}", ['exception' => $e]);
+                $this->logger->error('Failed to dispatch CancelStaleWorkflow', ['exception' => $e, 'entityWorkflowId' => $staleEntityWorkflow->getId()]);
                 continue;
             }
+
+            if (0 === $processedCount % 10) {
+                $this->entityManager->clear();
+            }
         }
 
         $this->logger->info("Cronjob completed: {$processedCount} workflows processed.");

src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php

@@ -18,7 +18,9 @@ use Psr\Log\LoggerInterface;
 use Symfony\Component\Clock\ClockInterface;
 use Symfony\Component\Messenger\Attribute\AsMessageHandler;
 use Symfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException;
+use Symfony\Component\Workflow\Metadata\MetadataStoreInterface;
 use Symfony\Component\Workflow\Registry;
+use Symfony\Component\Workflow\Transition;
 
 #[AsMessageHandler]
 final readonly class CancelStaleWorkflowHandler
@@ -57,10 +59,7 @@ final readonly class CancelStaleWorkflowHandler
         $wasInInitialPosition = 'initial' === $workflow->getStep();
 
         foreach ($transitions as $transition) {
-            $isFinal = $metadataStore->getMetadata('isFinal', $transition);
-            $isFinalPositive = $metadataStore->getMetadata('isFinalPositive', $transition);
-
-            if ($isFinal && !$isFinalPositive) {
+            if ($this->willTransitionLeadToFinalNegative($transition, $metadataStore)) {
                 $dto = new WorkflowTransitionContextDTO($workflow);
                 $workflowComponent->apply($workflow, $transition->getName(), [
                     'context' => $dto,
@@ -85,4 +84,16 @@ final readonly class CancelStaleWorkflowHandler
 
         $this->em->flush();
     }
+
+    private function willTransitionLeadToFinalNegative(Transition $transition, MetadataStoreInterface $metadataStore): bool
+    {
+        foreach ($transition->getTos() as $place) {
+            $metadata = $metadataStore->getPlaceMetadata($place);
+            if (($metadata['isFinal'] ?? true) && false === ($metadata['isFinalPositive'] ?? true)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
 }

src/Bundle/ChillMainBundle/Tests/Repository/EntityWorkflowRepositoryTest.php

@@ -12,6 +12,7 @@ declare(strict_types=1);
 namespace ChillMainBundle\Tests\Repository;
 
 use Chill\MainBundle\Entity\User;
+use Chill\MainBundle\Entity\Workflow\EntityWorkflow;
 use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository;
 use Doctrine\ORM\EntityManagerInterface;
 use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
@@ -40,9 +41,13 @@ class EntityWorkflowRepositoryTest extends KernelTestCase
     {
         $repository = new EntityWorkflowRepository($this->em);
 
-        $actual = $repository->findWorkflowsWithoutFinalStepAndOlderThan(new \DateTimeImmutable('10 years ago'));
+        $actual = $repository->findWorkflowsWithoutFinalStepAndOlderThan((new \DateTimeImmutable('now'))->add(new \DateInterval('P10Y')));
 
-        self::assertIsArray($actual, 'check that the query is successful');
+        self::assertIsIterable($actual, 'check that the query is successful');
+
+        foreach ($actual as $entityWorkflow) {
+            self::assertInstanceOf(EntityWorkflow::class, $entityWorkflow);
+        }
     }
 
     public function testCountQueryByDest(): void

src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php

@@ -9,12 +9,14 @@ declare(strict_types=1);
  * the LICENSE file that was distributed with this source code.
  */
 
-namespace Services\Workflow;
+namespace ChillMainBundle\Tests\Services\Workflow;
 
 use Chill\MainBundle\Entity\CronJobExecution;
+use Chill\MainBundle\Entity\Workflow\EntityWorkflow;
 use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository;
 use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowCronJob;
 use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowMessage;
+use Doctrine\ORM\EntityManagerInterface;
 use PHPUnit\Framework\MockObject\Exception;
 use PHPUnit\Framework\TestCase;
 use Psr\Log\LoggerInterface;
@@ -39,8 +41,9 @@ class CancelStaleWorkflowCronJobTest extends TestCase
     {
         $clock = new MockClock(new \DateTimeImmutable('2024-01-01 00:00:00', new \DateTimeZone('+00:00')));
         $logger = $this->createMock(LoggerInterface::class);
+        $entityManager = $this->createMock(EntityManagerInterface::class);
 
-        $cronJob = new CancelStaleWorkflowCronJob($this->createMock(EntityWorkflowRepository::class), $clock, $this->buildMessageBus(), $logger);
+        $cronJob = new CancelStaleWorkflowCronJob($this->createMock(EntityWorkflowRepository::class), $clock, $this->buildMessageBus(), $logger, $entityManager);
 
         self::assertEquals($expected, $cronJob->canRun($cronJobExecution));
     }
@@ -54,11 +57,16 @@ class CancelStaleWorkflowCronJobTest extends TestCase
     {
         $clock = new MockClock((new \DateTimeImmutable('now', new \DateTimeZone('+00:00')))->add(new \DateInterval('P120D')));
         $workflowRepository = $this->createMock(EntityWorkflowRepository::class);
+        $entityManager = $this->createMock(EntityManagerInterface::class);
 
-        $workflowRepository->method('findWorkflowsWithoutFinalStepAndOlderThan')->willReturn([1, 3, 2]);
+        $workflowRepository->method('findWorkflowsWithoutFinalStepAndOlderThan')->willReturn([
+            $this->buildEntityWorkflow(1),
+            $this->buildEntityWorkflow(3),
+            $this->buildEntityWorkflow(2),
+        ]);
         $messageBus = $this->buildMessageBus(true);
 
-        $cronJob = new CancelStaleWorkflowCronJob($workflowRepository, $clock, $messageBus, new NullLogger());
+        $cronJob = new CancelStaleWorkflowCronJob($workflowRepository, $clock, $messageBus, new NullLogger(), $entityManager);
 
         $results = $cronJob->run([]);
 
@@ -67,6 +75,16 @@ class CancelStaleWorkflowCronJobTest extends TestCase
         self::assertEquals(3, $results['last-canceled-workflow-id']);
     }
 
+    private function buildEntityWorkflow(int $id): EntityWorkflow
+    {
+        $entityWorkflow = new EntityWorkflow();
+        $reflectionClass = new \ReflectionClass($entityWorkflow);
+        $idProperty = $reflectionClass->getProperty('id');
+        $idProperty->setValue($entityWorkflow, $id);
+
+        return $entityWorkflow;
+    }
+
     /**
      * @throws \Exception
      */

src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php

@@ -131,18 +131,22 @@ class CancelStaleWorkflowHandlerTest extends TestCase
             ->setInitialPlaces('initial')
             ->addPlaces(['initial', 'step1', 'canceled', 'final'])
             ->addTransition(new Transition('to_step1', 'initial', 'step1'))
-            ->addTransition($cancelInit = new Transition('cancel', 'initial', 'canceled'))
-            ->addTransition($finalizeInit = new Transition('finalize', 'initial', 'final'))
-            ->addTransition($cancelStep1 = new Transition('cancel', 'step1', 'canceled'))
-            ->addTransition($finalizeStep1 = new Transition('finalize', 'step1', 'final'));
+            ->addTransition(new Transition('cancel', 'initial', 'canceled'))
+            ->addTransition(new Transition('finalize', 'initial', 'final'))
+            ->addTransition(new Transition('cancel', 'step1', 'canceled'))
+            ->addTransition(new Transition('finalize', 'step1', 'final'));
 
-        $transitionStorage = new \SplObjectStorage();
-        $transitionStorage->attach($finalizeInit, ['isFinal' => true, 'isFinalPositive' => true]);
-        $transitionStorage->attach($cancelInit, ['isFinal' => true, 'isFinalPositive' => false]);
-        $transitionStorage->attach($finalizeStep1, ['isFinal' => true, 'isFinalPositive' => true]);
-        $transitionStorage->attach($cancelStep1, ['isFinal' => true, 'isFinalPositive' => false]);
+        $definitionBuilder->setMetadataStore(new InMemoryMetadataStore(placesMetadata: [
+            'canceled' => [
+                'isFinal' => true,
+                'isFinalPositive' => false,
+            ],
+            'final' => [
+                'isFinal' => true,
+                'isFinalPositive', true,
+            ],
+        ]));
 
-        $definitionBuilder->setMetadataStore(new InMemoryMetadataStore([], [], $transitionStorage));
         $workflow = new Workflow($definitionBuilder->build(), new EntityWorkflowMarkingStore(), null, 'dummy_workflow');
         $supports =
             new class () implements WorkflowSupportStrategyInterface {