From 860ae5cedfda6aa67511555d06362b0f502e2afb Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Thu, 8 Aug 2024 17:00:27 +0200 Subject: [PATCH 01/14] Create CancelStaleWorkflow message and handler --- .../Service/Workflow/CancelStaleWorkflow.php | 15 ++++++++++ .../Workflow/CancelStaleWorkflowHandler.php | 30 +++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php create mode 100644 src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php new file mode 100644 index 000000000..d13ca039e --- /dev/null +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php @@ -0,0 +1,15 @@ +workflowIds; + } +} diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php new file mode 100644 index 000000000..aff22d1e3 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php @@ -0,0 +1,30 @@ +getWorkflowIds(); + + foreach ($workflowIds as $workflowId) { + $workflow = $this->workflowRepository->find($workflowId); + + $steps = $workflow->getSteps(); + + + } + } + +} From 34edb02cd06ea8bfcf662b14123682017ad9c625 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Mon, 12 Aug 2024 11:44:31 +0200 Subject: [PATCH 02/14] Create message and handler for canceling stale workflows --- .../Service/Workflow/CancelStaleWorkflow.php | 6 +-- .../Workflow/CancelStaleWorkflowHandler.php | 43 +++++++++++++++---- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php index d13ca039e..078402855 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php @@ -4,12 +4,12 @@ namespace Chill\MainBundle\Service\Workflow; class CancelStaleWorkflow { - public function __construct(public array $workflowIds) + public function __construct(public int $workflowId) { } - public function getWorkflowIds(): array + public function getWorkflowId(): int { - return $this->workflowIds; + return $this->workflowId; } } diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php index aff22d1e3..e35464f1d 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php @@ -5,26 +5,53 @@ namespace Chill\MainBundle\Service\Workflow; use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository; use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; -use Symfony\Component\Messenger\Handler\MessageHandlerInterface; +use Symfony\Component\Messenger\Attribute\AsMessageHandler; +use Symfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException; +use Symfony\Component\Workflow\Registry; -class CancelStaleWorkflowHandler implements MessageHandlerInterface +#[AsMessageHandler] +class CancelStaleWorkflowHandler { - public function __construct(private readonly EntityWorkflowRepository $workflowRepository, private EntityManagerInterface $em, private LoggerInterface $logger) + public function __construct(private readonly EntityWorkflowRepository $workflowRepository, private readonly Registry $registry, private EntityManagerInterface $em, private LoggerInterface $logger) { - } public function __invoke(CancelStaleWorkflow $message): void { - $workflowIds = $message->getWorkflowIds(); + $workflowId = $message->getWorkflowId(); - foreach ($workflowIds as $workflowId) { - $workflow = $this->workflowRepository->find($workflowId); + $workflow = $this->workflowRepository->find($workflowId); + $workflowComponent = $this->registry->get($workflow, $workflow->getWorkflowName()); + $metadataStore = $workflowComponent->getMetadataStore(); + $transitions = $workflowComponent->getEnabledTransitions($workflow); + $steps = $workflow->getSteps(); - $steps = $workflow->getSteps(); + $transitionApplied = false; + if (count($steps) === 1) { + $this->em->remove($workflow->getCurrentStep()); + $this->em->remove($workflow); + } else { + foreach ($transitions as $transition) { + $isFinal = $metadataStore->getMetadata('isFinal', $transition); + $isFinalPositive = $metadataStore->getMetadata('isFinalPositive', $transition); + if ($isFinal && !$isFinalPositive) { + $workflowComponent->apply($workflow, 'annule'); + $this->logger->info(sprintf('EntityWorkflow %d has been transitioned.', $workflowId)); + $transitionApplied = true; + break; + } + } + + if (!$transitionApplied) { + $this->logger->error(sprintf('No valid transition found for EntityWorkflow %d.', $workflowId)); + throw new UnrecoverableMessageHandlingException( + sprintf('No valid transition found for EntityWorkflow %d.', $workflowId) + ); + } } + } } From 29fec50515e3daa1864913b8b4249198ef92f246 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Mon, 12 Aug 2024 11:45:27 +0200 Subject: [PATCH 03/14] Add cronjob and repository method to find and cancel stale workflows every other day --- .../Workflow/EntityWorkflowRepository.php | 13 +++++ .../Workflow/CancelStaleWorkflowCronJob.php | 55 +++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php diff --git a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php index f5696d2b9..b6e2e0814 100644 --- a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php +++ b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php @@ -198,6 +198,19 @@ class EntityWorkflowRepository implements ObjectRepository return $this->repository->findOneBy($criteria); } + public function findWorkflowsWithoutFinalStepAndOlderThan(\DateTimeImmutable $olderThanDate) + { + $qb = $this->repository->createQueryBuilder('sw'); + + $qb->select('sw.id') + ->where('NOT EXISTS (SELECT 1 FROM chill_main_entity_workflow_step ews WHERE ews.isFinal = TRUE AND ews.entityWorkflow = sw.id)') + ->andWhere(':olderThanDate > ALL (SELECT ews.transitionAt FROM chill_main_entity_workflow_step ews WHERE ews.transitionAt IS NOT NULL AND ews.entityWorkflow = sw.id)') + ->andWhere('sw.createdAt < :olderThanDate') + ->setParameter('olderThanDate', $olderThanDate); + + return $qb->getQuery()->getResult(); + } + public function getClassName(): string { return EntityWorkflow::class; diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php new file mode 100644 index 000000000..2114396f0 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php @@ -0,0 +1,55 @@ +clock->now() >= $cronJobExecution->getLastEnd()->add(new DateInterval('P1D')); + } + + public function getKey(): string + { + return self::KEY; + } + + public function run(array $lastExecutionData): ?array + { + $olderThanDate = $this->clock->now()->sub(new DateInterval(self::KEEP_INTERVAL)); + + $staleWorkflowIds = $this->workflowRepository->findWorkflowsWithoutFinalStepAndOlderThan($olderThanDate); + + $lastCanceled = self::LAST_CANCELED_WORKFLOW; + + foreach ($staleWorkflowIds as $wId) { + $this->messageBus->dispatch(new CancelStaleWorkflow($wId)); + $lastCanceled = $wId; + } + + return [self::LAST_CANCELED_WORKFLOW => $lastCanceled]; + } + +} From 6001bb6447e025f4d18fea1a2952295987932be5 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Mon, 12 Aug 2024 12:05:03 +0200 Subject: [PATCH 04/14] Add logger messages for possible debugging purposes --- .../Workflow/CancelStaleWorkflowCronJob.php | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php index 2114396f0..d8888efe8 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php @@ -6,22 +6,22 @@ use Chill\MainBundle\Cron\CronJobInterface; use Chill\MainBundle\Entity\CronJobExecution; use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository; use DateInterval; -use Doctrine\DBAL\Connection; use Psr\Log\LoggerInterface; use Symfony\Component\Clock\ClockInterface; use Symfony\Component\Messenger\MessageBusInterface; class CancelStaleWorkflowCronJob implements CronJobInterface { - public const KEY = 'remove-stale-workflow'; + public const string KEY = 'remove-stale-workflow'; - public const KEEP_INTERVAL = 'P90D'; + public const string KEEP_INTERVAL = 'P90D'; - private const LAST_CANCELED_WORKFLOW = 'last-canceled-workflow-id'; + private const string LAST_CANCELED_WORKFLOW = 'last-canceled-workflow-id'; public function __construct(private readonly EntityWorkflowRepository $workflowRepository, private readonly ClockInterface $clock, private readonly MessageBusInterface $messageBus, + private readonly LoggerInterface $logger, ) { } @@ -38,18 +38,26 @@ class CancelStaleWorkflowCronJob implements CronJobInterface public function run(array $lastExecutionData): ?array { + $this->logger->info('Cronjob started: Canceling stale workflows.'); + $olderThanDate = $this->clock->now()->sub(new DateInterval(self::KEEP_INTERVAL)); - $staleWorkflowIds = $this->workflowRepository->findWorkflowsWithoutFinalStepAndOlderThan($olderThanDate); - $lastCanceled = self::LAST_CANCELED_WORKFLOW; + $processedCount = 0; foreach ($staleWorkflowIds as $wId) { - $this->messageBus->dispatch(new CancelStaleWorkflow($wId)); - $lastCanceled = $wId; + try { + $this->messageBus->dispatch(new CancelStaleWorkflow($wId)); + $lastCanceled = $wId; + $processedCount++; + } catch (\Exception $e) { + $this->logger->error("Failed to dispatch CancelStaleWorkflow for ID {$wId}", ['exception' => $e]); + continue; + } } + $this->logger->info("Cronjob completed: {$processedCount} workflows processed."); + return [self::LAST_CANCELED_WORKFLOW => $lastCanceled]; } - } From dab68fb4090c801bfe0ff4ad7ee0b8ac68f3228f Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Mon, 12 Aug 2024 12:05:31 +0200 Subject: [PATCH 05/14] Add CancelStaleWorkflowCronJobTest --- .../CancelStaleWorkflowCronJobTest.php | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php new file mode 100644 index 000000000..6b654f4df --- /dev/null +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php @@ -0,0 +1,107 @@ +connection = self::getContainer()->get(Connection::class); + } + + /** + * @dataProvider buildTestCanRunData + * @throws \Exception + */ + public function testCanRun(?CronJobExecution $cronJobExecution, bool $expected): void + { + $clock = new MockClock(new \DateTimeImmutable('2024-01-01 00:00:00', new \DateTimeZone('+00:00'))); + $logger = $this->createMock(LoggerInterface::class); + + $cronJob = new CancelStaleWorkflowCronJob($this->createMock(EntityWorkflowRepository::class), $clock, $this->buildMessageBus(), $logger); + + self::assertEquals($expected, $cronJob->canRun($cronJobExecution)); + } + + /** + * @throws \DateMalformedStringException + * @throws \DateInvalidTimeZoneException + * @throws Exception + */ + public function testRun(): void + { + $clock = new MockClock((new \DateTimeImmutable('now', new \DateTimeZone('+00:00')))->add(new \DateInterval('P120D'))); + $workflowRepository = $this->createMock(EntityWorkflowRepository::class); + $logger = $this->createMock(LoggerInterface::class); + + $workflowRepository->method('findWorkflowsWithoutFinalStepAndOlderThan')->willReturn([1, 2, 3]); + $messageBus = $this->buildMessageBus(true); + + $cronJob = new CancelStaleWorkflowCronJob($workflowRepository, $clock, $messageBus, $logger); + + $results = $cronJob->run([]); + + // Assert the result has the last canceled workflow ID + self::assertArrayHasKey('last-canceled-workflow-id', $results); + self::assertEquals(3, $results['last-canceled-workflow-id']); + } + + /** + * @throws \Exception + */ + public static function buildTestCanRunData(): iterable + { + yield [ + (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new \DateTimeImmutable('2023-12-31 00:00:00', new \DateTimeZone('+00:00'))), + true, + ]; + + yield [ + (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new \DateTimeImmutable('2023-12-30 23:59:59', new \DateTimeZone('+00:00'))), + true, + ]; + + yield [ + (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new \DateTimeImmutable('2023-12-31 00:00:01', new \DateTimeZone('+00:00'))), + false, + ]; + } + + private function buildMessageBus(bool $expectDispatchAtLeastOnce = false): MessageBusInterface + { + $messageBus = $this->createMock(MessageBusInterface::class); + + $methodDispatch = match ($expectDispatchAtLeastOnce) { + true => $messageBus->expects($this->atLeastOnce())->method('dispatch')->with($this->isInstanceOf(CancelStaleWorkflow::class)), + false => $messageBus->method('dispatch'), + }; + + $methodDispatch->willReturnCallback(function (CancelStaleWorkflow $message) { + return new Envelope($message); + }); + + return $messageBus; + } +} From 35199b69934729e9956bcde41df501f014e74113 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Mon, 12 Aug 2024 15:49:22 +0200 Subject: [PATCH 06/14] Create test for CancelStaleWorkflowHandler: wip state --- .../CancelStaleWorkflowHandlerTest.php | 126 ++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php new file mode 100644 index 000000000..8e2a5ec95 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php @@ -0,0 +1,126 @@ +createMock(EntityWorkflow::class); + $workflow->method('getSteps')->willReturn(new ArrayCollection([$this->createMock(EntityWorkflowStep::class)])); + $workflow->expects($this->once())->method('getCurrentStep')->willReturn(new EntityWorkflowStep()); + + $workflowRepository = $this->createMock(EntityWorkflowRepository::class); + $workflowRepository->expects($this->once())->method('find')->with($this->identicalTo(1))->willReturn($workflow); + + $em = $this->createMock(EntityManagerInterface::class); + $em->expects($this->exactly(2))->method('remove')->with($this->isInstanceOf(EntityWorkflowStep::class)); + $em->expects($this->once())->method('flush'); + + $registry = $this->createMock(Registry::class); + $logger = $this->createMock(LoggerInterface::class); + + $handler = new CancelStaleWorkflowHandler($workflowRepository, $registry, $em, $logger); + + $handler(new CancelStaleWorkflow(1)); + } + + public function testInvokeWorkflowWithMultipleStepsAndValidTransition(): void + { + $workflow = $this->createMock(EntityWorkflow::class); + $workflow->method('getSteps')->willReturn(new ArrayCollection([$this->createMock(EntityWorkflowStep::class), $this->createMock(EntityWorkflowStep::class)])); + + $transition = $this->createMock(Transition::class); + + $workflowComponent = $this->createMock(WorkflowInterface::class); + $registryMock = $this->createMock(Registry::class); + $registryMock->method('get') + ->willReturn($workflowComponent); + + $workflowComponent->method('getEnabledTransitions')->willReturn([$transition]); + $workflowComponent->expects($this->once())->method('apply')->with($workflow, 'annule'); + + $metadataStore = $this->createMock(MetadataStore::class); + $metadataStore->method('getMetadata')->willReturnMap([ + ['isFinal', $transition, true], + ['isFinalPositive', $transition, false], + ]); + + $workflowComponent->method('getMetadataStore')->willReturn($metadataStore); + + $workflowRepository = $this->createMock(EntityWorkflowRepository::class); + $workflowRepository->expects($this->once())->method('find')->with($this->identicalTo(1))->willReturn($workflow); + + $em = $this->createMock(EntityManagerInterface::class); + $em->expects($this->never())->method('remove'); + $em->expects($this->once())->method('flush'); + + $registry = $this->createMock(Registry::class); + $registry->method('get')->willReturn($workflowComponent); + + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($this->once())->method('info')->with('EntityWorkflow 1 has been transitioned.'); + + $handler = new CancelStaleWorkflowHandler($workflowRepository, $registry, $em, $logger); + + $handler(new CancelStaleWorkflow(1)); + } + + public function testInvokeWorkflowWithMultipleStepsAndNoValidTransition(): void + { + $this->expectException(UnrecoverableMessageHandlingException::class); + $this->expectExceptionMessage('No valid transition found for EntityWorkflow 1.'); + + $workflow = $this->createMock(EntityWorkflow::class); + $workflow->method('getSteps')->willReturn(new ArrayCollection([$this->createMock(EntityWorkflowStep::class), $this->createMock(EntityWorkflowStep::class)])); + + $transition = $this->createMock(Transition::class); + + $workflowComponent = $this->createMock(WorkflowInterface::class); + $registryMock = $this->createMock(Registry::class); + $registryMock->method('get') + ->willReturn($workflowComponent); + $workflowComponent->method('getEnabledTransitions')->willReturn([$transition]); + + $metadataStore = $this->createMock(MetadataStoreInterface::class); + $metadataStore->method('getMetadata')->willReturnMap([ + ['isFinal', $transition, false], + ['isFinalPositive', $transition, true], + ]); + + $workflowComponent->method('getMetadataStore')->willReturn($metadataStore); + + $workflowRepository = $this->createMock(EntityWorkflowRepository::class); + $workflowRepository->expects($this->once())->method('find')->with($this->identicalTo(1))->willReturn($workflow); + + $em = $this->createMock(EntityManagerInterface::class); + $em->expects($this->never())->method('remove'); + $em->expects($this->never())->method('flush'); + + $registry = $this->createMock(Registry::class); + $registry->method('get')->willReturn($workflowComponent); + + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($this->once())->method('error')->with('No valid transition found for EntityWorkflow 1.'); + + $handler = new CancelStaleWorkflowHandler($workflowRepository, $registry, $em, $logger); + + $handler(new CancelStaleWorkflow(1)); + } +} From 5d84e997c1e5337f73cbd1061a424291a52deef5 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Mon, 12 Aug 2024 15:52:21 +0200 Subject: [PATCH 07/14] Php cs fixes --- .../Service/Workflow/CancelStaleWorkflow.php | 13 ++++++-- .../Workflow/CancelStaleWorkflowCronJob.php | 33 +++++++++++-------- .../Workflow/CancelStaleWorkflowHandler.php | 21 +++++++----- .../CancelStaleWorkflowCronJobTest.php | 6 ++++ .../CancelStaleWorkflowHandlerTest.php | 15 ++++++++- 5 files changed, 62 insertions(+), 26 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php index 078402855..118b9f57e 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php @@ -1,12 +1,19 @@ clock->now() >= $cronJobExecution->getLastEnd()->add(new DateInterval('P1D')); + return $this->clock->now() >= $cronJobExecution->getLastEnd()->add(new \DateInterval('P1D')); } public function getKey(): string @@ -40,7 +47,7 @@ class CancelStaleWorkflowCronJob implements CronJobInterface { $this->logger->info('Cronjob started: Canceling stale workflows.'); - $olderThanDate = $this->clock->now()->sub(new DateInterval(self::KEEP_INTERVAL)); + $olderThanDate = $this->clock->now()->sub(new \DateInterval(self::KEEP_INTERVAL)); $staleWorkflowIds = $this->workflowRepository->findWorkflowsWithoutFinalStepAndOlderThan($olderThanDate); $lastCanceled = self::LAST_CANCELED_WORKFLOW; $processedCount = 0; @@ -49,10 +56,10 @@ class CancelStaleWorkflowCronJob implements CronJobInterface try { $this->messageBus->dispatch(new CancelStaleWorkflow($wId)); $lastCanceled = $wId; - $processedCount++; + ++$processedCount; } catch (\Exception $e) { - $this->logger->error("Failed to dispatch CancelStaleWorkflow for ID {$wId}", ['exception' => $e]); - continue; + $this->logger->error("Failed to dispatch CancelStaleWorkflow for ID {$wId}", ['exception' => $e]); + continue; } } diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php index e35464f1d..533283a59 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php @@ -1,5 +1,14 @@ em->remove($workflow->getCurrentStep()); $this->em->remove($workflow); } else { @@ -46,12 +53,8 @@ class CancelStaleWorkflowHandler if (!$transitionApplied) { $this->logger->error(sprintf('No valid transition found for EntityWorkflow %d.', $workflowId)); - throw new UnrecoverableMessageHandlingException( - sprintf('No valid transition found for EntityWorkflow %d.', $workflowId) - ); + throw new UnrecoverableMessageHandlingException(sprintf('No valid transition found for EntityWorkflow %d.', $workflowId)); } } - } - } diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php index 6b654f4df..b56faf79b 100644 --- a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php @@ -23,6 +23,11 @@ use Symfony\Component\Clock\MockClock; use Symfony\Component\Messenger\Envelope; use Symfony\Component\Messenger\MessageBusInterface; +/** + * @internal + * + * @coversNothing + */ class CancelStaleWorkflowCronJobTest extends KernelTestCase { protected function setUp(): void @@ -33,6 +38,7 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase /** * @dataProvider buildTestCanRunData + * * @throws \Exception */ public function testCanRun(?CronJobExecution $cronJobExecution, bool $expected): void diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php index 8e2a5ec95..f7887a6cd 100644 --- a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php @@ -1,5 +1,14 @@ Date: Wed, 28 Aug 2024 11:52:01 +0200 Subject: [PATCH 08/14] Suffix message class with 'Message' and add check on workflow to assert no transitions were applied since message placed in queue --- .../Workflow/CancelStaleWorkflowCronJob.php | 4 +-- .../Workflow/CancelStaleWorkflowHandler.php | 27 +++++++++++++++---- ...low.php => CancelStaleWorkflowMessage.php} | 2 +- .../CancelStaleWorkflowCronJobTest.php | 24 ++++++++++------- .../CancelStaleWorkflowHandlerTest.php | 7 ++--- 5 files changed, 43 insertions(+), 21 deletions(-) rename src/Bundle/ChillMainBundle/Service/Workflow/{CancelStaleWorkflow.php => CancelStaleWorkflowMessage.php} (92%) diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php index cb6f1c971..786bae3c9 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php @@ -54,8 +54,8 @@ class CancelStaleWorkflowCronJob implements CronJobInterface foreach ($staleWorkflowIds as $wId) { try { - $this->messageBus->dispatch(new CancelStaleWorkflow($wId)); - $lastCanceled = $wId; + $this->messageBus->dispatch(new CancelStaleWorkflowMessage($wId)); + $lastCanceled = max($wId, $lastCanceled); ++$processedCount; } catch (\Exception $e) { $this->logger->error("Failed to dispatch CancelStaleWorkflow for ID {$wId}", ['exception' => $e]); diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php index 533283a59..a3eefb193 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php @@ -14,6 +14,7 @@ namespace Chill\MainBundle\Service\Workflow; use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository; use Doctrine\ORM\EntityManagerInterface; 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\Registry; @@ -21,13 +22,29 @@ use Symfony\Component\Workflow\Registry; #[AsMessageHandler] class CancelStaleWorkflowHandler { - public function __construct(private readonly EntityWorkflowRepository $workflowRepository, private readonly Registry $registry, private EntityManagerInterface $em, private LoggerInterface $logger) {} + public const string KEEP_INTERVAL = 'P90D'; - public function __invoke(CancelStaleWorkflow $message): void + public function __construct(private readonly EntityWorkflowRepository $workflowRepository, private readonly Registry $registry, private readonly EntityManagerInterface $em, private readonly LoggerInterface $logger, private readonly ClockInterface $clock) {} + + public function __invoke(CancelStaleWorkflowMessage $message): void { $workflowId = $message->getWorkflowId(); + $olderThanDate = $this->clock->now()->sub(new \DateInterval(self::KEEP_INTERVAL)); + $staleWorkflows = $this->workflowRepository->findWorkflowsWithoutFinalStepAndOlderThan($olderThanDate); + $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) { + $this->logger->alert('Workflow was not found!', [$workflowId]); + return; + } + $workflowComponent = $this->registry->get($workflow, $workflow->getWorkflowName()); $metadataStore = $workflowComponent->getMetadataStore(); $transitions = $workflowComponent->getEnabledTransitions($workflow); @@ -44,15 +61,15 @@ class CancelStaleWorkflowHandler $isFinalPositive = $metadataStore->getMetadata('isFinalPositive', $transition); if ($isFinal && !$isFinalPositive) { - $workflowComponent->apply($workflow, 'annule'); - $this->logger->info(sprintf('EntityWorkflow %d has been transitioned.', $workflowId)); + $workflowComponent->apply($workflow, $transition->getName()); + $this->logger->info('EntityWorkflow has been cancelled automatically.', [$workflowId]); $transitionApplied = true; break; } } if (!$transitionApplied) { - $this->logger->error(sprintf('No valid transition found for EntityWorkflow %d.', $workflowId)); + $this->logger->error('No valid transition found for EntityWorkflow %d.', [$workflowId]); throw new UnrecoverableMessageHandlingException(sprintf('No valid transition found for EntityWorkflow %d.', $workflowId)); } } diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowMessage.php similarity index 92% rename from src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php rename to src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowMessage.php index 118b9f57e..30d2b6ab8 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflow.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowMessage.php @@ -11,7 +11,7 @@ declare(strict_types=1); namespace Chill\MainBundle\Service\Workflow; -class CancelStaleWorkflow +class CancelStaleWorkflowMessage { public function __construct(public int $workflowId) {} diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php index b56faf79b..f433e8b48 100644 --- a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php @@ -15,6 +15,10 @@ use Chill\MainBundle\Entity\CronJobExecution; use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflow; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowCronJob; +use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowMessage; +use DateInvalidTimeZoneException; +use DateMalformedStringException; +use DateTimeImmutable; use Doctrine\DBAL\Connection; use PHPUnit\Framework\MockObject\Exception; use Psr\Log\LoggerInterface; @@ -43,7 +47,7 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase */ public function testCanRun(?CronJobExecution $cronJobExecution, bool $expected): void { - $clock = new MockClock(new \DateTimeImmutable('2024-01-01 00:00:00', new \DateTimeZone('+00:00'))); + $clock = new MockClock(new DateTimeImmutable('2024-01-01 00:00:00', new \DateTimeZone('+00:00'))); $logger = $this->createMock(LoggerInterface::class); $cronJob = new CancelStaleWorkflowCronJob($this->createMock(EntityWorkflowRepository::class), $clock, $this->buildMessageBus(), $logger); @@ -52,17 +56,17 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase } /** - * @throws \DateMalformedStringException - * @throws \DateInvalidTimeZoneException - * @throws Exception + * @throws DateMalformedStringException + * @throws DateInvalidTimeZoneException + * @throws \Exception|Exception */ public function testRun(): void { - $clock = new MockClock((new \DateTimeImmutable('now', new \DateTimeZone('+00:00')))->add(new \DateInterval('P120D'))); + $clock = new MockClock((new DateTimeImmutable('now', new \DateTimeZone('+00:00')))->add(new \DateInterval('P120D'))); $workflowRepository = $this->createMock(EntityWorkflowRepository::class); $logger = $this->createMock(LoggerInterface::class); - $workflowRepository->method('findWorkflowsWithoutFinalStepAndOlderThan')->willReturn([1, 2, 3]); + $workflowRepository->method('findWorkflowsWithoutFinalStepAndOlderThan')->willReturn([1, 3, 2]); $messageBus = $this->buildMessageBus(true); $cronJob = new CancelStaleWorkflowCronJob($workflowRepository, $clock, $messageBus, $logger); @@ -80,17 +84,17 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase public static function buildTestCanRunData(): iterable { yield [ - (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new \DateTimeImmutable('2023-12-31 00:00:00', new \DateTimeZone('+00:00'))), + (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new DateTimeImmutable('2023-12-31 00:00:00', new \DateTimeZone('+00:00'))), true, ]; yield [ - (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new \DateTimeImmutable('2023-12-30 23:59:59', new \DateTimeZone('+00:00'))), + (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new DateTimeImmutable('2023-12-30 23:59:59', new \DateTimeZone('+00:00'))), true, ]; yield [ - (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new \DateTimeImmutable('2023-12-31 00:00:01', new \DateTimeZone('+00:00'))), + (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new DateTimeImmutable('2023-12-31 00:00:01', new \DateTimeZone('+00:00'))), false, ]; } @@ -104,7 +108,7 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase false => $messageBus->method('dispatch'), }; - $methodDispatch->willReturnCallback(function (CancelStaleWorkflow $message) { + $methodDispatch->willReturnCallback(function (CancelStaleWorkflowMessage $message) { return new Envelope($message); }); diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php index f7887a6cd..dbf88098f 100644 --- a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php @@ -16,6 +16,7 @@ use Chill\MainBundle\Entity\Workflow\EntityWorkflowStep; use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflow; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowHandler; +use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowMessage; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\TestCase; @@ -51,7 +52,7 @@ class CancelStaleWorkflowHandlerTest extends TestCase $handler = new CancelStaleWorkflowHandler($workflowRepository, $registry, $em, $logger); - $handler(new CancelStaleWorkflow(1)); + $handler(new CancelStaleWorkflowMessage(1)); } public function testInvokeWorkflowWithMultipleStepsAndValidTransition(): void @@ -92,7 +93,7 @@ class CancelStaleWorkflowHandlerTest extends TestCase $handler = new CancelStaleWorkflowHandler($workflowRepository, $registry, $em, $logger); - $handler(new CancelStaleWorkflow(1)); + $handler(new CancelStaleWorkflowMessage(1)); } public function testInvokeWorkflowWithMultipleStepsAndNoValidTransition(): void @@ -134,6 +135,6 @@ class CancelStaleWorkflowHandlerTest extends TestCase $handler = new CancelStaleWorkflowHandler($workflowRepository, $registry, $em, $logger); - $handler(new CancelStaleWorkflow(1)); + $handler(new CancelStaleWorkflowMessage(1)); } } From 2e69d2df90ddac5a55942874fef87619938b18fb Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Wed, 28 Aug 2024 14:27:27 +0200 Subject: [PATCH 09/14] Adjust test to work with actual workflow + minor fix of handler logic --- .../Entity/Workflow/EntityWorkflow.php | 2 +- .../Workflow/CancelStaleWorkflowHandler.php | 4 +- .../CancelStaleWorkflowHandlerTest.php | 168 ++++++++---------- 3 files changed, 76 insertions(+), 98 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php index 1f32a1985..6e6cee077 100644 --- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php +++ b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php @@ -38,7 +38,7 @@ class EntityWorkflow implements TrackCreationInterface, TrackUpdateInterface /** * @var Collection */ - #[ORM\OneToMany(targetEntity: EntityWorkflowComment::class, mappedBy: 'entityWorkflow', orphanRemoval: true)] + #[ORM\OneToMany(mappedBy: 'entityWorkflow', targetEntity: EntityWorkflowComment::class, orphanRemoval: true)] private Collection $comments; #[ORM\Id] diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php index a3eefb193..2aae89f3c 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php @@ -35,7 +35,7 @@ class CancelStaleWorkflowHandler $workflow = $this->workflowRepository->find($workflowId); - if (in_array($workflow, $staleWorkflows, true)) { + if (!in_array($workflow, $staleWorkflows, true)) { $this->logger->alert('Workflow has transitioned in the meantime.', [$workflowId]); return; } @@ -69,7 +69,7 @@ class CancelStaleWorkflowHandler } if (!$transitionApplied) { - $this->logger->error('No valid transition found for EntityWorkflow %d.', [$workflowId]); + $this->logger->error('No valid transition found for EntityWorkflow.', [$workflowId]); throw new UnrecoverableMessageHandlingException(sprintf('No valid transition found for EntityWorkflow %d.', $workflowId)); } } diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php index dbf88098f..23c99d16a 100644 --- a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php @@ -2,139 +2,117 @@ 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\Workflow\EntityWorkflow; use Chill\MainBundle\Entity\Workflow\EntityWorkflowStep; use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository; -use Chill\MainBundle\Service\Workflow\CancelStaleWorkflow; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowHandler; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowMessage; -use Doctrine\Common\Collections\ArrayCollection; use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; -use Symfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException; -use Symfony\Component\Workflow\Metadata\MetadataStoreInterface; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use Symfony\Component\Clock\ClockInterface; use Symfony\Component\Workflow\Registry; -use Symfony\Component\Workflow\Transition; use Symfony\Component\Workflow\WorkflowInterface; +use Symfony\Component\Yaml\Yaml; +use DateTimeImmutable; -/** - * @internal - * - * @coversNothing - */ -class CancelStaleWorkflowHandlerTest extends TestCase +class CancelStaleWorkflowHandlerTest extends KernelTestCase { - public function testInvokeWorkflowWithOneStep(): void + private EntityManagerInterface $em; + private Registry $registry; + private LoggerInterface $logger; + private EntityWorkflowRepository $workflowRepository; + private ClockInterface $clock; + private string $workflowName; + + protected function setUp(): void { - $workflow = $this->createMock(EntityWorkflow::class); - $workflow->method('getSteps')->willReturn(new ArrayCollection([$this->createMock(EntityWorkflowStep::class)])); - $workflow->expects($this->once())->method('getCurrentStep')->willReturn(new EntityWorkflowStep()); + // Boot the Symfony kernel + self::bootKernel(); - $workflowRepository = $this->createMock(EntityWorkflowRepository::class); - $workflowRepository->expects($this->once())->method('find')->with($this->identicalTo(1))->willReturn($workflow); + // 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); - $em = $this->createMock(EntityManagerInterface::class); - $em->expects($this->exactly(2))->method('remove')->with($this->isInstanceOf(EntityWorkflowStep::class)); - $em->expects($this->once())->method('flush'); + // Retrieve the workflow configuration dynamically + $configPath = self::$kernel->getProjectDir() . '/config/packages/workflow.yaml'; // Adjust the path if needed + $config = Yaml::parseFile($configPath); - $registry = $this->createMock(Registry::class); - $logger = $this->createMock(LoggerInterface::class); - - $handler = new CancelStaleWorkflowHandler($workflowRepository, $registry, $em, $logger); - - $handler(new CancelStaleWorkflowMessage(1)); + // Extract the workflow name from the configuration + $this->workflowName = array_key_first($config['framework']['workflows']); } - public function testInvokeWorkflowWithMultipleStepsAndValidTransition(): void + public function testWorkflowWithOneStepOlderThan90DaysIsDeleted(): void { - $workflow = $this->createMock(EntityWorkflow::class); - $workflow->method('getSteps')->willReturn(new ArrayCollection([$this->createMock(EntityWorkflowStep::class), $this->createMock(EntityWorkflowStep::class)])); + $workflow = new EntityWorkflow(); + $initialStep = new EntityWorkflowStep(); + $initialStep->setTransitionAt(new DateTimeImmutable('-93 days')); + $workflow->addStep($initialStep); - $transition = $this->createMock(Transition::class); + $this->em->persist($workflow); + $this->em->flush(); - $workflowComponent = $this->createMock(WorkflowInterface::class); - $registryMock = $this->createMock(Registry::class); - $registryMock->method('get') - ->willReturn($workflowComponent); + $this->handleStaleWorkflow($workflow); - $workflowComponent->method('getEnabledTransitions')->willReturn([$transition]); - $workflowComponent->expects($this->once())->method('apply')->with($workflow, 'annule'); + $deletedWorkflow = $this->workflowRepository->find($workflow->getId()); + $this->assertNull($deletedWorkflow, 'The workflow should be deleted.'); - $metadataStore = $this->createMock(MetadataStore::class); - $metadataStore->method('getMetadata')->willReturnMap([ - ['isFinal', $transition, true], - ['isFinalPositive', $transition, false], - ]); - - $workflowComponent->method('getMetadataStore')->willReturn($metadataStore); - - $workflowRepository = $this->createMock(EntityWorkflowRepository::class); - $workflowRepository->expects($this->once())->method('find')->with($this->identicalTo(1))->willReturn($workflow); - - $em = $this->createMock(EntityManagerInterface::class); - $em->expects($this->never())->method('remove'); - $em->expects($this->once())->method('flush'); - - $registry = $this->createMock(Registry::class); - $registry->method('get')->willReturn($workflowComponent); - - $logger = $this->createMock(LoggerInterface::class); - $logger->expects($this->once())->method('info')->with('EntityWorkflow 1 has been transitioned.'); - - $handler = new CancelStaleWorkflowHandler($workflowRepository, $registry, $em, $logger); - - $handler(new CancelStaleWorkflowMessage(1)); + $this->assertNull($this->em->getRepository(EntityWorkflowStep::class)->find($initialStep->getId()), 'The workflow step should be deleted.'); } - public function testInvokeWorkflowWithMultipleStepsAndNoValidTransition(): void + public function testWorkflowWithMultipleStepsAndNoRecentTransitionsIsCanceled(): void { - $this->expectException(UnrecoverableMessageHandlingException::class); - $this->expectExceptionMessage('No valid transition found for EntityWorkflow 1.'); + $workflow = new EntityWorkflow(); + $step1 = new EntityWorkflowStep(); + $step2 = new EntityWorkflowStep(); - $workflow = $this->createMock(EntityWorkflow::class); - $workflow->method('getSteps')->willReturn(new ArrayCollection([$this->createMock(EntityWorkflowStep::class), $this->createMock(EntityWorkflowStep::class)])); + $step1->setTransitionAt(new DateTimeImmutable('-92 days')); + $step2->setTransitionAt(new DateTimeImmutable('-91 days')); - $transition = $this->createMock(Transition::class); + $workflow->addStep($step1); + $workflow->addStep($step2); - $workflowComponent = $this->createMock(WorkflowInterface::class); - $registryMock = $this->createMock(Registry::class); - $registryMock->method('get') - ->willReturn($workflowComponent); - $workflowComponent->method('getEnabledTransitions')->willReturn([$transition]); + $this->em->persist($workflow); + $this->em->flush(); - $metadataStore = $this->createMock(MetadataStoreInterface::class); - $metadataStore->method('getMetadata')->willReturnMap([ - ['isFinal', $transition, false], - ['isFinalPositive', $transition, true], - ]); + /** @var WorkflowInterface $workflowComponent */ + $workflowComponent = $this->registry->get($workflow, $this->workflowName); - $workflowComponent->method('getMetadataStore')->willReturn($metadataStore); + $transitions = $workflowComponent->getEnabledTransitions($workflow); + $metadataStore = $workflowComponent->getMetadataStore(); - $workflowRepository = $this->createMock(EntityWorkflowRepository::class); - $workflowRepository->expects($this->once())->method('find')->with($this->identicalTo(1))->willReturn($workflow); + $expectedTransition = null; - $em = $this->createMock(EntityManagerInterface::class); - $em->expects($this->never())->method('remove'); - $em->expects($this->never())->method('flush'); + // 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); - $registry = $this->createMock(Registry::class); - $registry->method('get')->willReturn($workflowComponent); + if ($isFinal === true && $isFinalPositive === false) { + $expectedTransition = $transition; + break; + } + } - $logger = $this->createMock(LoggerInterface::class); - $logger->expects($this->once())->method('error')->with('No valid transition found for EntityWorkflow 1.'); + $this->assertNotNull($expectedTransition, 'Expected to find a valid transition with isFinal = true and isFinalPositive = false.'); - $handler = new CancelStaleWorkflowHandler($workflowRepository, $registry, $em, $logger); + $this->handleStaleWorkflow($workflow); + $updatedWorkflow = $this->workflowRepository->find($workflow->getId()); + + $this->assertEquals($expectedTransition->getName(), $updatedWorkflow->getCurrentStep()); - $handler(new CancelStaleWorkflowMessage(1)); } + + public function handleStaleWorkflow($workflow): void + { + $handler = new CancelStaleWorkflowHandler($this->workflowRepository, $this->registry, $this->em, $this->logger, $this->clock); + $handler(new CancelStaleWorkflowMessage($workflow->getId())); + } + } From b97eabf0d28e48f61bac510783fa238caef7fbf6 Mon Sep 17 00:00:00 2001 From: Julie Lenaerts Date: Thu, 29 Aug 2024 16:51:25 +0200 Subject: [PATCH 10/14] Get workflowComponent directly from registry --- .../Workflow/CancelStaleWorkflowHandlerTest.php | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php index 23c99d16a..2c3016f82 100644 --- a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php @@ -26,7 +26,6 @@ class CancelStaleWorkflowHandlerTest extends KernelTestCase private LoggerInterface $logger; private EntityWorkflowRepository $workflowRepository; private ClockInterface $clock; - private string $workflowName; protected function setUp(): void { @@ -39,13 +38,6 @@ class CancelStaleWorkflowHandlerTest extends KernelTestCase $this->logger = self::getContainer()->get(LoggerInterface::class); $this->clock = self::getContainer()->get(ClockInterface::class); $this->workflowRepository = $this->createMock(EntityWorkflowRepository::class); - - // Retrieve the workflow configuration dynamically - $configPath = self::$kernel->getProjectDir() . '/config/packages/workflow.yaml'; // Adjust the path if needed - $config = Yaml::parseFile($configPath); - - // Extract the workflow name from the configuration - $this->workflowName = array_key_first($config['framework']['workflows']); } public function testWorkflowWithOneStepOlderThan90DaysIsDeleted(): void @@ -82,7 +74,7 @@ class CancelStaleWorkflowHandlerTest extends KernelTestCase $this->em->flush(); /** @var WorkflowInterface $workflowComponent */ - $workflowComponent = $this->registry->get($workflow, $this->workflowName); + $workflowComponent = $this->registry->get($workflowComponent); $transitions = $workflowComponent->getEnabledTransitions($workflow); $metadataStore = $workflowComponent->getMetadataStore(); From ee9530d03f88c15c35d107036b73dc34eb66a536 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Mon, 9 Sep 2024 10:40:05 +0200 Subject: [PATCH 11/14] More conditions to find staled workflows --- .../Workflow/EntityWorkflowRepository.php | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php index b6e2e0814..62282fbe9 100644 --- a/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php +++ b/src/Bundle/ChillMainBundle/Repository/Workflow/EntityWorkflowRepository.php @@ -198,13 +198,28 @@ class EntityWorkflowRepository implements ObjectRepository return $this->repository->findOneBy($criteria); } - public function findWorkflowsWithoutFinalStepAndOlderThan(\DateTimeImmutable $olderThanDate) + /** + * Finds workflows that are not finalized and are older than the specified date. + * + * @param \DateTimeImmutable $olderThanDate the date to compare against + * + * @return list the list of workflow IDs that meet the criteria + */ + public function findWorkflowsWithoutFinalStepAndOlderThan(\DateTimeImmutable $olderThanDate): array { $qb = $this->repository->createQueryBuilder('sw'); $qb->select('sw.id') + // only the workflow which are not finalized ->where('NOT EXISTS (SELECT 1 FROM chill_main_entity_workflow_step ews WHERE ews.isFinal = TRUE AND ews.entityWorkflow = sw.id)') - ->andWhere(':olderThanDate > ALL (SELECT ews.transitionAt FROM chill_main_entity_workflow_step ews WHERE ews.transitionAt IS NOT NULL AND ews.entityWorkflow = sw.id)') + ->andWhere( + $qb->expr()->orX( + // only the workflow where all the last transition is older than transitionAt + ':olderThanDate > ALL (SELECT ews.transitionAt FROM chill_main_entity_workflow_step ews WHERE ews.transitionAt IS NOT NULL AND ews.entityWorkflow = sw.id)', + // or the workflow which have only the initial step, with no transition + '1 = (SELECT COUNT(ews.id) FROM chill_main_entity_workflow_step ews WHERE ews.step = :initial AND ews.transitionAt IS NULL AND ews.createdAt < :olderThanDate AND ews.entityWorkflow = sw.id)', + ) + ) ->andWhere('sw.createdAt < :olderThanDate') ->setParameter('olderThanDate', $olderThanDate); From d152efe084aa4d3455d2ae9c928128ec4d560deb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Mon, 9 Sep 2024 14:58:19 +0200 Subject: [PATCH 12/14] Refactor imports and remove redundant type strings This commit refactors the usage of \DateTimeImmutable to ensure consistent namespacing and removes unnecessary string type declarations from constants in CancelStaleWorkflowCronJob. These changes improve code readability and maintainability. --- .../Workflow/CancelStaleWorkflowCronJob.php | 6 +++--- .../Workflow/CancelStaleWorkflowCronJobTest.php | 17 +++++++---------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php index 786bae3c9..767af2c5c 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php @@ -20,11 +20,11 @@ use Symfony\Component\Messenger\MessageBusInterface; class CancelStaleWorkflowCronJob implements CronJobInterface { - public const string KEY = 'remove-stale-workflow'; + public const KEY = 'remove-stale-workflow'; - public const string KEEP_INTERVAL = 'P90D'; + public const KEEP_INTERVAL = 'P90D'; - private const string LAST_CANCELED_WORKFLOW = 'last-canceled-workflow-id'; + private const LAST_CANCELED_WORKFLOW = 'last-canceled-workflow-id'; public function __construct( private readonly EntityWorkflowRepository $workflowRepository, diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php index f433e8b48..8e06735f6 100644 --- a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php @@ -16,9 +16,6 @@ use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflow; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowCronJob; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowMessage; -use DateInvalidTimeZoneException; -use DateMalformedStringException; -use DateTimeImmutable; use Doctrine\DBAL\Connection; use PHPUnit\Framework\MockObject\Exception; use Psr\Log\LoggerInterface; @@ -47,7 +44,7 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase */ public function testCanRun(?CronJobExecution $cronJobExecution, bool $expected): void { - $clock = new MockClock(new DateTimeImmutable('2024-01-01 00:00:00', new \DateTimeZone('+00:00'))); + $clock = new MockClock(new \DateTimeImmutable('2024-01-01 00:00:00', new \DateTimeZone('+00:00'))); $logger = $this->createMock(LoggerInterface::class); $cronJob = new CancelStaleWorkflowCronJob($this->createMock(EntityWorkflowRepository::class), $clock, $this->buildMessageBus(), $logger); @@ -56,13 +53,13 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase } /** - * @throws DateMalformedStringException - * @throws DateInvalidTimeZoneException + * @throws \DateMalformedStringException + * @throws \DateInvalidTimeZoneException * @throws \Exception|Exception */ public function testRun(): void { - $clock = new MockClock((new DateTimeImmutable('now', new \DateTimeZone('+00:00')))->add(new \DateInterval('P120D'))); + $clock = new MockClock((new \DateTimeImmutable('now', new \DateTimeZone('+00:00')))->add(new \DateInterval('P120D'))); $workflowRepository = $this->createMock(EntityWorkflowRepository::class); $logger = $this->createMock(LoggerInterface::class); @@ -84,17 +81,17 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase public static function buildTestCanRunData(): iterable { yield [ - (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new DateTimeImmutable('2023-12-31 00:00:00', new \DateTimeZone('+00:00'))), + (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new \DateTimeImmutable('2023-12-31 00:00:00', new \DateTimeZone('+00:00'))), true, ]; yield [ - (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new DateTimeImmutable('2023-12-30 23:59:59', new \DateTimeZone('+00:00'))), + (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new \DateTimeImmutable('2023-12-30 23:59:59', new \DateTimeZone('+00:00'))), true, ]; yield [ - (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new DateTimeImmutable('2023-12-31 00:00:01', new \DateTimeZone('+00:00'))), + (new CronJobExecution('last-canceled-workflow-id'))->setLastEnd(new \DateTimeImmutable('2023-12-31 00:00:01', new \DateTimeZone('+00:00'))), false, ]; } From f4356ac249ef91bb93c8beb04700d11f6a2250e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Mon, 9 Sep 2024 14:59:26 +0200 Subject: [PATCH 13/14] 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 --- .../Entity/Workflow/EntityWorkflow.php | 34 ++- .../Workflow/CancelStaleWorkflowHandler.php | 75 ++++--- .../Entity/Workflow/EntityWorkflowTest.php | 27 +++ .../CancelStaleWorkflowCronJobTest.php | 4 +- .../CancelStaleWorkflowHandlerTest.php | 193 +++++++++++------- 5 files changed, 226 insertions(+), 107 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php index 6e6cee077..08b5ee41f 100644 --- a/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php +++ b/src/Bundle/ChillMainBundle/Entity/Workflow/EntityWorkflow.php @@ -56,7 +56,7 @@ class EntityWorkflow implements TrackCreationInterface, TrackUpdateInterface * @var Collection&Selectable */ #[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; + } } diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php index 2aae89f3c..54485dad5 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowHandler.php +++ b/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 readonly EntityWorkflowRepository $workflowRepository, private readonly Registry $registry, private readonly EntityManagerInterface $em, private readonly LoggerInterface $logger, private readonly ClockInterface $clock) {} + public function __construct( + private EntityWorkflowRepository $workflowRepository, + 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)); - $staleWorkflows = $this->workflowRepository->findWorkflowsWithoutFinalStepAndOlderThan($olderThanDate); + $workflow = $this->workflowRepository->find($message->getWorkflowId()); + 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) { - $this->logger->alert('Workflow was not found!', [$workflowId]); - return; + if (false === $workflow->isStaledAt($olderThanDate)) { + $this->logger->alert('Workflow has transitioned in the meantime.', [$workflowId]); + + 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)) { - $this->em->remove($workflow->getCurrentStep()); - $this->em->remove($workflow); - } else { - foreach ($transitions as $transition) { - $isFinal = $metadataStore->getMetadata('isFinal', $transition); - $isFinalPositive = $metadataStore->getMetadata('isFinalPositive', $transition); + foreach ($transitions as $transition) { + $isFinal = $metadataStore->getMetadata('isFinal', $transition); + $isFinalPositive = $metadataStore->getMetadata('isFinalPositive', $transition); - if ($isFinal && !$isFinalPositive) { - $workflowComponent->apply($workflow, $transition->getName()); - $this->logger->info('EntityWorkflow has been cancelled automatically.', [$workflowId]); - $transitionApplied = true; - 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 ($isFinal && !$isFinalPositive) { + $dto = new WorkflowTransitionContextDTO($workflow); + $workflowComponent->apply($workflow, $transition->getName(), [ + 'context' => $dto, + 'byUser' => null, + 'transitionAt' => $this->clock->now(), + 'transition' => $transition->getName(), + ]); + $this->logger->info('EntityWorkflow has been cancelled automatically.', [$workflowId]); + $transitionApplied = true; + 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(); } } diff --git a/src/Bundle/ChillMainBundle/Tests/Entity/Workflow/EntityWorkflowTest.php b/src/Bundle/ChillMainBundle/Tests/Entity/Workflow/EntityWorkflowTest.php index 4eb56f995..3241a41fc 100644 --- a/src/Bundle/ChillMainBundle/Tests/Entity/Workflow/EntityWorkflowTest.php +++ b/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)); + } } diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php index 8e06735f6..07bca6bc5 100644 --- a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php +++ b/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) { - return new Envelope($message); - }); + $methodDispatch->willReturnCallback(fn (CancelStaleWorkflowMessage $message) => new Envelope($message)); return $messageBus; } diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php index 2c3016f82..b6cac68de 100644 --- a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowHandlerTest.php +++ b/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 Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use Prophecy\PhpUnit\ProphecyTrait; +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; - private Registry $registry; - private LoggerInterface $logger; - private EntityWorkflowRepository $workflowRepository; - private ClockInterface $clock; + use ProphecyTrait; - protected function setUp(): void + public function testWorkflowWithOneStepOlderThan90DaysIsCanceled(): void { - // Boot the Symfony kernel - self::bootKernel(); + $clock = new MockClock('2024-01-01'); + $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(); - $initialStep->setTransitionAt(new DateTimeImmutable('-93 days')); - $workflow->addStep($initialStep); + $workflow->setWorkflowName('dummy_workflow'); + $workflow->setCreatedAt(new \DateTimeImmutable('2023-09-01')); + $workflow->setStep('step1', new WorkflowTransitionContextDTO($workflow), 'to_step1', $daysAgos, new User()); - $this->em->persist($workflow); - $this->em->flush(); + $em = $this->prophesize(EntityManagerInterface::class); + $em->flush()->shouldBeCalled(); + $em->remove($workflow)->shouldNotBeCalled(); - $this->handleStaleWorkflow($workflow); + $handler = $this->buildHandler($workflow, $em->reveal(), $clock); - $deletedWorkflow = $this->workflowRepository->find($workflow->getId()); - $this->assertNull($deletedWorkflow, 'The workflow should be deleted.'); + $handler(new CancelStaleWorkflowMessage(1)); - $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(); - $step2 = new EntityWorkflowStep(); + $workflow->setWorkflowName('dummy_workflow'); + $workflow->setCreatedAt(new \DateTimeImmutable('2023-12-31')); + $workflow->setStep('step1', new WorkflowTransitionContextDTO($workflow), 'to_step1', $daysAgos, new User()); - $step1->setTransitionAt(new DateTimeImmutable('-92 days')); - $step2->setTransitionAt(new DateTimeImmutable('-91 days')); + $em = $this->prophesize(EntityManagerInterface::class); + $em->flush()->shouldNotBeCalled(); + $em->remove($workflow)->shouldNotBeCalled(); - $workflow->addStep($step1); - $workflow->addStep($step2); + $handler = $this->buildHandler($workflow, $em->reveal(), $clock); - $this->em->persist($workflow); - $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()); + $handler(new CancelStaleWorkflowMessage(1)); + 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); - $handler(new CancelStaleWorkflowMessage($workflow->getId())); + $clock = new MockClock('2024-01-01'); + + $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; + } } From 2fb46c65c27893e867749ad27462863b9290349e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Mon, 9 Sep 2024 15:16:10 +0200 Subject: [PATCH 14/14] Refactor CancelStaleWorkflowCronJobTest to simplify setup Replaced KernelTestCase with TestCase to simplify test setup and removed dependency on the database connection. Added NullLogger to replace mocked LoggerInterface during testing. Updated method call in tests to correctly reference CancelStaleWorkflowMessage class. --- .../Workflow/CancelStaleWorkflowCronJob.php | 2 +- .../CancelStaleWorkflowCronJobTest.php | 18 +++++------------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php index 767af2c5c..be3dd2a5e 100644 --- a/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php +++ b/src/Bundle/ChillMainBundle/Service/Workflow/CancelStaleWorkflowCronJob.php @@ -49,7 +49,7 @@ class CancelStaleWorkflowCronJob implements CronJobInterface $olderThanDate = $this->clock->now()->sub(new \DateInterval(self::KEEP_INTERVAL)); $staleWorkflowIds = $this->workflowRepository->findWorkflowsWithoutFinalStepAndOlderThan($olderThanDate); - $lastCanceled = self::LAST_CANCELED_WORKFLOW; + $lastCanceled = $lastExecutionData[self::LAST_CANCELED_WORKFLOW] ?? 0; $processedCount = 0; foreach ($staleWorkflowIds as $wId) { diff --git a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php index 07bca6bc5..200b92099 100644 --- a/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Services/Workflow/CancelStaleWorkflowCronJobTest.php @@ -13,13 +13,12 @@ namespace Services\Workflow; use Chill\MainBundle\Entity\CronJobExecution; use Chill\MainBundle\Repository\Workflow\EntityWorkflowRepository; -use Chill\MainBundle\Service\Workflow\CancelStaleWorkflow; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowCronJob; use Chill\MainBundle\Service\Workflow\CancelStaleWorkflowMessage; -use Doctrine\DBAL\Connection; use PHPUnit\Framework\MockObject\Exception; +use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; -use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use Psr\Log\NullLogger; use Symfony\Component\Clock\MockClock; use Symfony\Component\Messenger\Envelope; use Symfony\Component\Messenger\MessageBusInterface; @@ -29,14 +28,8 @@ use Symfony\Component\Messenger\MessageBusInterface; * * @coversNothing */ -class CancelStaleWorkflowCronJobTest extends KernelTestCase +class CancelStaleWorkflowCronJobTest extends TestCase { - protected function setUp(): void - { - self::bootKernel(); - $this->connection = self::getContainer()->get(Connection::class); - } - /** * @dataProvider buildTestCanRunData * @@ -61,12 +54,11 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase { $clock = new MockClock((new \DateTimeImmutable('now', new \DateTimeZone('+00:00')))->add(new \DateInterval('P120D'))); $workflowRepository = $this->createMock(EntityWorkflowRepository::class); - $logger = $this->createMock(LoggerInterface::class); $workflowRepository->method('findWorkflowsWithoutFinalStepAndOlderThan')->willReturn([1, 3, 2]); $messageBus = $this->buildMessageBus(true); - $cronJob = new CancelStaleWorkflowCronJob($workflowRepository, $clock, $messageBus, $logger); + $cronJob = new CancelStaleWorkflowCronJob($workflowRepository, $clock, $messageBus, new NullLogger()); $results = $cronJob->run([]); @@ -101,7 +93,7 @@ class CancelStaleWorkflowCronJobTest extends KernelTestCase $messageBus = $this->createMock(MessageBusInterface::class); $methodDispatch = match ($expectDispatchAtLeastOnce) { - true => $messageBus->expects($this->atLeastOnce())->method('dispatch')->with($this->isInstanceOf(CancelStaleWorkflow::class)), + true => $messageBus->expects($this->atLeastOnce())->method('dispatch')->with($this->isInstanceOf(CancelStaleWorkflowMessage::class)), false => $messageBus->method('dispatch'), };