Add test for detecting stale workflows and enhance handler

Added a new test to check if workflows are stale in EntityWorkflowTest. Enhanced CancelStaleWorkflowHandler to handle stale workflows more accurately, including checking if workflows have transitioned recently. Updated EntityWorkflow entity to cascade remove workflow steps.

Refactor tests for handler, to avoid using $kernel during tests

src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php

@@ -56,7 +56,7 @@ class EntityWorkflow implements TrackCreationInterface, TrackUpdateInterface
      * @var Collection<int, EntityWorkflowStep>&Selectable<int, EntityWorkflowStep>
      */
     #[Assert\Valid(traverse: true)]
-    #[ORM\OneToMany(mappedBy: 'entityWorkflow', targetEntity: EntityWorkflowStep::class, cascade: ['persist'], orphanRemoval: true)]
+    #[ORM\OneToMany(mappedBy: 'entityWorkflow', targetEntity: EntityWorkflowStep::class, cascade: ['persist', 'remove'], orphanRemoval: true)]
     #[ORM\OrderBy(['transitionAt' => \Doctrine\Common\Collections\Criteria::ASC, 'id' => 'ASC'])]
     private Collection&Selectable $steps;
 
@@ -488,4 +488,36 @@ class EntityWorkflow implements TrackCreationInterface, TrackUpdateInterface
     {
         return $this->getCurrentStep()->getHoldsOnStep()->count() > 0;
     }
+
+    /**
+     * Determines if the workflow has become stale after a given date.
+     *
+     * This function checks the creation date and the transition states of the workflow steps.
+     * A workflow is considered stale if:
+     * - The creation date is before the given date and no transitions have occurred since the creation.
+     * - Or if there are no transitions after the given date.
+     *
+     * @param \DateTimeImmutable $at the date to compare against the workflow's status
+     *
+     * @return bool true if the workflow is stale after the given date, false otherwise
+     */
+    public function isStaledAt(\DateTimeImmutable $at): bool
+    {
+        // if there is no transition since the creation, then the workflow is staled
+        if ('initial' === $this->getCurrentStep()->getCurrentStep()
+            && null === $this->getCurrentStep()->getTransitionAt()
+        ) {
+            if (null === $this->getCreatedAt()) {
+                return false;
+            }
+
+            if ($this->getCreatedAt() < $at) {
+                return true;
+            }
+
+            return false;
+        }
+
+        return $this->getCurrentStepChained()->getPrevious()->getTransitionAt() < $at;
+    }
 }

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

@@ -12,6 +12,7 @@ declare(strict_types=1);
 namespace Chill\MainBundle\Service\Workflow;
 
 use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository;
+use Chill\MainBundle\Workflow\WorkflowTransitionContextDTO;
 use Doctrine\ORM\EntityManagerInterface;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\Clock\ClockInterface;
@@ -20,58 +21,68 @@ use Symfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException;
 use Symfony\Component\Workflow\Registry;
 
 #[AsMessageHandler]
-class CancelStaleWorkflowHandler
+final readonly class CancelStaleWorkflowHandler
 {
-    public const string KEEP_INTERVAL = 'P90D';
+    public function __construct(
-
+        private EntityWorkflowRepository $workflowRepository,
-    public function __construct(private readonly EntityWorkflowRepository $workflowRepository, private readonly Registry $registry, private readonly EntityManagerInterface $em, private readonly LoggerInterface $logger, private readonly ClockInterface $clock) {}
+        private Registry $registry,
+        private EntityManagerInterface $em,
+        private LoggerInterface $logger,
+        private ClockInterface $clock,
+    ) {}
 
     public function __invoke(CancelStaleWorkflowMessage $message): void
     {
         $workflowId = $message->getWorkflowId();
+        $olderThanDate = $this->clock->now()->sub(new \DateInterval(CancelStaleWorkflowCronJob::KEEP_INTERVAL));
 
-        $olderThanDate = $this->clock->now()->sub(new \DateInterval(self::KEEP_INTERVAL));
+        $workflow = $this->workflowRepository->find($message->getWorkflowId());
-        $staleWorkflows = $this->workflowRepository->findWorkflowsWithoutFinalStepAndOlderThan($olderThanDate);
+        if (null === $workflow) {
+            $this->logger->alert('Workflow was not found!', [$workflowId]);
 
-        $workflow = $this->workflowRepository->find($workflowId);
-
-        if (!in_array($workflow, $staleWorkflows, true)) {
-            $this->logger->alert('Workflow has transitioned in the meantime.', [$workflowId]);
             return;
         }
 
-        if (null === $workflow) {
+        if (false === $workflow->isStaledAt($olderThanDate)) {
-            $this->logger->alert('Workflow was not found!', [$workflowId]);
+            $this->logger->alert('Workflow has transitioned in the meantime.', [$workflowId]);
-            return;
+
+            throw new UnrecoverableMessageHandlingException('the workflow is not staled any more');
         }
 
         $workflowComponent = $this->registry->get($workflow, $workflow->getWorkflowName());
         $metadataStore = $workflowComponent->getMetadataStore();
         $transitions = $workflowComponent->getEnabledTransitions($workflow);
-        $steps = $workflow->getSteps();
 
         $transitionApplied = false;
+        $wasInInitialPosition = 'initial' === $workflow->getStep();
 
-        if (1 === count($steps)) {
+        foreach ($transitions as $transition) {
-            $this->em->remove($workflow->getCurrentStep());
+            $isFinal = $metadataStore->getMetadata('isFinal', $transition);
-            $this->em->remove($workflow);
+            $isFinalPositive = $metadataStore->getMetadata('isFinalPositive', $transition);
-        } else {
-            foreach ($transitions as $transition) {
-                $isFinal = $metadataStore->getMetadata('isFinal', $transition);
-                $isFinalPositive = $metadataStore->getMetadata('isFinalPositive', $transition);
 
-                if ($isFinal && !$isFinalPositive) {
+            if ($isFinal && !$isFinalPositive) {
-                    $workflowComponent->apply($workflow, $transition->getName());
+                $dto = new WorkflowTransitionContextDTO($workflow);
-                    $this->logger->info('EntityWorkflow has been cancelled automatically.', [$workflowId]);
+                $workflowComponent->apply($workflow, $transition->getName(), [
-                    $transitionApplied = true;
+                    'context' => $dto,
-                    break;
+                    'byUser' => null,
-                }
+                    'transitionAt' => $this->clock->now(),
-            }
+                    'transition' => $transition->getName(),
-
+                ]);
-            if (!$transitionApplied) {
+                $this->logger->info('EntityWorkflow has been cancelled automatically.', [$workflowId]);
-                $this->logger->error('No valid transition found for EntityWorkflow.', [$workflowId]);
+                $transitionApplied = true;
-                throw new UnrecoverableMessageHandlingException(sprintf('No valid transition found for EntityWorkflow %d.', $workflowId));
+                break;
             }
         }
+
+        if (!$transitionApplied) {
+            $this->logger->error('No valid transition found for EntityWorkflow.', [$workflowId]);
+            throw new UnrecoverableMessageHandlingException(sprintf('No valid transition found for EntityWorkflow %d.', $workflowId));
+        }
+
+        if ($wasInInitialPosition) {
+            $this->em->remove($workflow);
+        }
+
+        $this->em->flush();
     }
 }

src/Bundle/ChillMainBundle/Tests/Entity/Workflow/EntityWorkflowTest.php

@@ -138,4 +138,31 @@ final class EntityWorkflowTest extends TestCase
         self::assertContains($person1, $persons);
         self::assertContains($person2, $persons);
     }
+
+    public function testIsStaledAt(): void
+    {
+        $creationDate = new \DateTimeImmutable('2024-01-01');
+        $firstStepDate = new \DateTimeImmutable('2024-01-02');
+        $afterFistStep = new \DateTimeImmutable('2024-01-03');
+
+        $entityWorkflow = new EntityWorkflow();
+
+        self::assertFalse($entityWorkflow->isStaledAt($creationDate), 'an entityWorkflow with null createdAt date should never be staled at initial step');
+        self::assertFalse($entityWorkflow->isStaledAt($firstStepDate), 'an entityWorkflow with null createdAt date should never be staled at initial step');
+        self::assertFalse($entityWorkflow->isStaledAt($afterFistStep), 'an entityWorkflow with null createdAt date should never be staled at initial step');
+
+        $entityWorkflow->setCreatedAt($creationDate);
+
+        self::assertFalse($entityWorkflow->isStaledAt($creationDate), 'an entityWorkflow with no step after initial should be staled');
+        self::assertTrue($entityWorkflow->isStaledAt($firstStepDate), 'an entityWorkflow with no step after initial should be staled');
+        self::assertTrue($entityWorkflow->isStaledAt($afterFistStep), 'an entityWorkflow with no step after initial should be staled');
+
+        // apply a first step
+        $dto = new WorkflowTransitionContextDTO($entityWorkflow);
+        $entityWorkflow->setStep('new_step', $dto, 'to_new_step', $firstStepDate);
+
+        self::assertFalse($entityWorkflow->isStaledAt($creationDate));
+        self::assertFalse($entityWorkflow->isStaledAt($firstStepDate));
+        self::assertTrue($entityWorkflow->isStaledAt($afterFistStep));
+    }
 }

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

@@ -105,9 +105,7 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase
             false => $messageBus->method('dispatch'),
         };
 
-        $methodDispatch->willReturnCallback(function (CancelStaleWorkflowMessage $message) {
+        $methodDispatch->willReturnCallback(fn (CancelStaleWorkflowMessage $message) => new Envelope($message));
-            return new Envelope($message);
-        });
 
         return $messageBus;
     }

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

@@ -2,109 +2,160 @@
 
 declare(strict_types=1);
 
+/*
+ * Chill is a software for social workers
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
 namespace Services\Workflow;
 
+use Chill\MainBundle\Entity\User;
 use Chill\MainBundle\Entity\Workflow\EntityWorkflow;
-use Chill\MainBundle\Entity\Workflow\EntityWorkflowStep;
 use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository;
 use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowHandler;
 use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowMessage;
+use Chill\MainBundle\Workflow\EntityWorkflowMarkingStore;
+use Chill\MainBundle\Workflow\WorkflowTransitionContextDTO;
 use Doctrine\ORM\EntityManagerInterface;
 use PHPUnit\Framework\TestCase;
-use Psr\Log\LoggerInterface;
+use Prophecy\PhpUnit\ProphecyTrait;
-use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
+use Psr\Log\NullLogger;
 use Symfony\Component\Clock\ClockInterface;
+use Symfony\Component\Clock\MockClock;
+use Symfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException;
+use Symfony\Component\Workflow\DefinitionBuilder;
+use Symfony\Component\Workflow\Metadata\InMemoryMetadataStore;
 use Symfony\Component\Workflow\Registry;
+use Symfony\Component\Workflow\SupportStrategy\WorkflowSupportStrategyInterface;
+use Symfony\Component\Workflow\Transition;
+use Symfony\Component\Workflow\Workflow;
 use Symfony\Component\Workflow\WorkflowInterface;
-use Symfony\Component\Yaml\Yaml;
-use DateTimeImmutable;
 
-class CancelStaleWorkflowHandlerTest extends KernelTestCase
+/**
+ * @internal
+ *
+ * @coversNothing
+ */
+class CancelStaleWorkflowHandlerTest extends TestCase
 {
-    private EntityManagerInterface $em;
+    use ProphecyTrait;
-    private Registry $registry;
-    private LoggerInterface $logger;
-    private EntityWorkflowRepository $workflowRepository;
-    private ClockInterface $clock;
 
-    protected function setUp(): void
+    public function testWorkflowWithOneStepOlderThan90DaysIsCanceled(): void
     {
-        // Boot the Symfony kernel
+        $clock = new MockClock('2024-01-01');
-        self::bootKernel();
+        $daysAgos = new \DateTimeImmutable('2023-09-01');
 
-        // Get the actual services from the container
-        $this->em = self::getContainer()->get(EntityManagerInterface::class);
-        $this->registry = self::getContainer()->get(Registry::class);
-        $this->logger = self::getContainer()->get(LoggerInterface::class);
-        $this->clock = self::getContainer()->get(ClockInterface::class);
-        $this->workflowRepository = $this->createMock(EntityWorkflowRepository::class);
-    }
-
-    public function testWorkflowWithOneStepOlderThan90DaysIsDeleted(): void
-    {
         $workflow = new EntityWorkflow();
-        $initialStep = new EntityWorkflowStep();
+        $workflow->setWorkflowName('dummy_workflow');
-        $initialStep->setTransitionAt(new DateTimeImmutable('-93 days'));
+        $workflow->setCreatedAt(new \DateTimeImmutable('2023-09-01'));
-        $workflow->addStep($initialStep);
+        $workflow->setStep('step1', new WorkflowTransitionContextDTO($workflow), 'to_step1', $daysAgos, new User());
 
-        $this->em->persist($workflow);
+        $em = $this->prophesize(EntityManagerInterface::class);
-        $this->em->flush();
+        $em->flush()->shouldBeCalled();
+        $em->remove($workflow)->shouldNotBeCalled();
 
-        $this->handleStaleWorkflow($workflow);
+        $handler = $this->buildHandler($workflow, $em->reveal(), $clock);
 
-        $deletedWorkflow = $this->workflowRepository->find($workflow->getId());
+        $handler(new CancelStaleWorkflowMessage(1));
-        $this->assertNull($deletedWorkflow, 'The workflow should be deleted.');
 
-        $this->assertNull($this->em->getRepository(EntityWorkflowStep::class)->find($initialStep->getId()), 'The workflow step should be deleted.');
+        self::assertEquals('canceled', $workflow->getStep());
+        self::assertCount(3, $workflow->getSteps());
     }
 
-    public function testWorkflowWithMultipleStepsAndNoRecentTransitionsIsCanceled(): void
+    public function testWorkflowNotInStaledHandlerIsUnrecoverable(): void
     {
+        $this->expectException(UnrecoverableMessageHandlingException::class);
+
+        $clock = new MockClock('2024-01-01');
+        $daysAgos = new \DateTimeImmutable('2023-12-31');
+
         $workflow = new EntityWorkflow();
-        $step1 = new EntityWorkflowStep();
+        $workflow->setWorkflowName('dummy_workflow');
-        $step2 = new EntityWorkflowStep();
+        $workflow->setCreatedAt(new \DateTimeImmutable('2023-12-31'));
+        $workflow->setStep('step1', new WorkflowTransitionContextDTO($workflow), 'to_step1', $daysAgos, new User());
 
-        $step1->setTransitionAt(new DateTimeImmutable('-92 days'));
+        $em = $this->prophesize(EntityManagerInterface::class);
-        $step2->setTransitionAt(new DateTimeImmutable('-91 days'));
+        $em->flush()->shouldNotBeCalled();
+        $em->remove($workflow)->shouldNotBeCalled();
 
-        $workflow->addStep($step1);
+        $handler = $this->buildHandler($workflow, $em->reveal(), $clock);
-        $workflow->addStep($step2);
 
-        $this->em->persist($workflow);
+        $handler(new CancelStaleWorkflowMessage(1));
-        $this->em->flush();
-
-        /** @var WorkflowInterface $workflowComponent */
-        $workflowComponent = $this->registry->get($workflowComponent);
-
-        $transitions = $workflowComponent->getEnabledTransitions($workflow);
-        $metadataStore = $workflowComponent->getMetadataStore();
-
-        $expectedTransition = null;
-
-        // Find the transition that was expected to be applied by the handler
-        foreach ($transitions as $transition) {
-            $isFinal = $metadataStore->getMetadata('isFinal', $transition);
-            $isFinalPositive = $metadataStore->getMetadata('isFinalPositive', $transition);
-
-            if ($isFinal === true && $isFinalPositive === false) {
-                $expectedTransition = $transition;
-                break;
-            }
-        }
-
-        $this->assertNotNull($expectedTransition, 'Expected to find a valid transition with isFinal = true and isFinalPositive = false.');
-
-        $this->handleStaleWorkflow($workflow);
-        $updatedWorkflow = $this->workflowRepository->find($workflow->getId());
-
-        $this->assertEquals($expectedTransition->getName(), $updatedWorkflow->getCurrentStep());
 
+        self::assertEquals('canceled', $workflow->getStep());
+        self::assertCount(3, $workflow->getSteps());
     }
 
-    public function handleStaleWorkflow($workflow): void
+    public function testWorkflowStaledInInitialStateIsCompletelyRemoved(): void
     {
-        $handler = new CancelStaleWorkflowHandler($this->workflowRepository, $this->registry, $this->em, $this->logger, $this->clock);
+        $clock = new MockClock('2024-01-01');
-        $handler(new CancelStaleWorkflowMessage($workflow->getId()));
+
+        $workflow = new EntityWorkflow();
+        $workflow->setWorkflowName('dummy_workflow');
+        $workflow->setCreatedAt(new \DateTimeImmutable('2023-09-01'));
+
+        $em = $this->prophesize(EntityManagerInterface::class);
+        $em->flush()->shouldBeCalled();
+        $em->remove($workflow)->shouldBeCalled();
+
+        $handler = $this->buildHandler($workflow, $em->reveal(), $clock);
+
+        $handler(new CancelStaleWorkflowMessage(1));
+
+        self::assertEquals('canceled', $workflow->getStep());
+        self::assertCount(2, $workflow->getSteps());
     }
 
+    private function buildHandler(
+        EntityWorkflow $entityWorkflow,
+        EntityManagerInterface $entityManager,
+        ClockInterface $clock,
+    ): CancelStaleWorkflowHandler {
+        // set an id for the workflow
+        $reflection = new \ReflectionClass($entityWorkflow);
+        $reflection->getProperty('id')->setValue($entityWorkflow, 1);
+
+        $repository = $this->prophesize(EntityWorkflowRepository::class);
+        $repository->find(1)->willReturn($entityWorkflow);
+
+        return new CancelStaleWorkflowHandler($repository->reveal(), $this->buildRegistry(), $entityManager, new NullLogger(), $clock);
+    }
+
+    private function buildRegistry(): Registry
+    {
+        $definitionBuilder = new DefinitionBuilder();
+
+        $definitionBuilder
+            ->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'));
+
+        $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([], [], $transitionStorage));
+        $workflow = new Workflow($definitionBuilder->build(), new EntityWorkflowMarkingStore(), null, 'dummy_workflow');
+        $supports =
+            new class () implements WorkflowSupportStrategyInterface {
+                public function supports(WorkflowInterface $workflow, object $subject): bool
+                {
+                    return true;
+                }
+            };
+
+
+        $registry = new Registry();
+        $registry->addWorkflow($workflow, $supports);
+
+        return $registry;
+    }
 }