From 0db2652f08008f1f5d5dd4eedf76f56710eb61e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Wed, 28 Aug 2024 11:41:43 +0200 Subject: [PATCH] Add cron job for removing expired stored objects Introduced `RemoveExpiredStoredObjectCronJob` to automate the deletion of expired stored objects every 7 days. Enhanced associated tests and updated relevant interfaces and classes to support the new cron job functionality. --- .../Entity/StoredObject.php | 27 +++- .../Entity/StoredObjectVersion.php | 23 +++- .../Repository/StoredObjectRepository.php | 11 ++ .../StoredObjectRepositoryInterface.php | 8 +- .../RemoveExpiredStoredObjectCronJob.php | 65 ++++++++++ .../RemoveOldVersionMessageHandler.php | 20 ++- .../Service/StoredObjectManager.php | 4 +- .../Service/StoredObjectManagerInterface.php | 9 +- .../RemoveExpiredStoredObjectCronJobTest.php | 115 ++++++++++++++++++ .../RemoveOldVersionMessageHandlerTest.php | 74 ++++++++++- 10 files changed, 344 insertions(+), 12 deletions(-) create mode 100644 src/Bundle/ChillDocStoreBundle/Service/StoredObjectCleaner/RemoveExpiredStoredObjectCronJob.php create mode 100644 src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectCleaner/RemoveExpiredStoredObjectCronJobTest.php diff --git a/src/Bundle/ChillDocStoreBundle/Entity/StoredObject.php b/src/Bundle/ChillDocStoreBundle/Entity/StoredObject.php index 4fec9f6ca..fe68ec31c 100644 --- a/src/Bundle/ChillDocStoreBundle/Entity/StoredObject.php +++ b/src/Bundle/ChillDocStoreBundle/Entity/StoredObject.php @@ -90,7 +90,7 @@ class StoredObject implements Document, TrackCreationInterface /** * @var Collection */ - #[ORM\OneToMany(targetEntity: StoredObjectVersion::class, cascade: ['persist'], mappedBy: 'storedObject')] + #[ORM\OneToMany(targetEntity: StoredObjectVersion::class, cascade: ['persist'], mappedBy: 'storedObject', orphanRemoval: true)] private Collection $versions; /** @@ -333,6 +333,15 @@ class StoredObject implements Document, TrackCreationInterface return $version; } + public function removeVersion(StoredObjectVersion $storedObjectVersion): void + { + if (!$this->versions->contains($storedObjectVersion)) { + throw new \UnexpectedValueException('This stored object does not contains this version'); + } + $this->versions->removeElement($storedObjectVersion); + $storedObjectVersion->resetStoredObject(); + } + /** * @deprecated */ @@ -359,4 +368,20 @@ class StoredObject implements Document, TrackCreationInterface return uniqid(more_entropy: true); } } + + /** + * Checks if a stored object can be deleted. + * + * Currently, return true if the deletedAt date is below the current date, and the object + * does not contains any version (which must be removed first). + * + * @param \DateTimeImmutable $now the current date and time + * @param StoredObject $storedObject the stored object to check + * + * @return bool returns true if the stored object can be deleted, false otherwise + */ + public static function canBeDeleted(\DateTimeImmutable $now, StoredObject $storedObject): bool + { + return $storedObject->getDeleteAt() < $now && $storedObject->getVersions()->isEmpty(); + } } diff --git a/src/Bundle/ChillDocStoreBundle/Entity/StoredObjectVersion.php b/src/Bundle/ChillDocStoreBundle/Entity/StoredObjectVersion.php index 2b8852513..84b10688f 100644 --- a/src/Bundle/ChillDocStoreBundle/Entity/StoredObjectVersion.php +++ b/src/Bundle/ChillDocStoreBundle/Entity/StoredObjectVersion.php @@ -33,6 +33,13 @@ class StoredObjectVersion implements TrackCreationInterface #[ORM\Column(type: \Doctrine\DBAL\Types\Types::INTEGER)] private ?int $id = null; + /** + * The stored object associated with this version. + */ + #[ORM\ManyToOne(targetEntity: StoredObject::class, inversedBy: 'versions')] + #[ORM\JoinColumn(name: 'stored_object_id', nullable: true)] + private ?StoredObject $storedObject; + /** * filename of the version in the stored object. */ @@ -40,12 +47,7 @@ class StoredObjectVersion implements TrackCreationInterface private string $filename = ''; public function __construct( - /** - * The stored object associated with this version. - */ - #[ORM\ManyToOne(targetEntity: StoredObject::class, inversedBy: 'versions')] - #[ORM\JoinColumn(name: 'stored_object_id', nullable: true)] - private StoredObject $storedObject, + StoredObject $storedObject, /** * The incremental version. @@ -76,6 +78,7 @@ class StoredObjectVersion implements TrackCreationInterface private string $type = '', ?string $filename = null, ) { + $this->storedObject = $storedObject; $this->filename = $filename ?? self::generateFilename($this); } @@ -124,4 +127,12 @@ class StoredObjectVersion implements TrackCreationInterface { return $this->version; } + + /** + * @internal to be used by StoredObject::removeVersion + */ + public function resetStoredObject(): void + { + $this->storedObject = null; + } } diff --git a/src/Bundle/ChillDocStoreBundle/Repository/StoredObjectRepository.php b/src/Bundle/ChillDocStoreBundle/Repository/StoredObjectRepository.php index 84bc7d4cb..2f39f2021 100644 --- a/src/Bundle/ChillDocStoreBundle/Repository/StoredObjectRepository.php +++ b/src/Bundle/ChillDocStoreBundle/Repository/StoredObjectRepository.php @@ -14,6 +14,7 @@ namespace Chill\DocStoreBundle\Repository; use Chill\DocStoreBundle\Entity\StoredObject; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityRepository; +use Doctrine\ORM\Query; final readonly class StoredObjectRepository implements StoredObjectRepositoryInterface { @@ -53,6 +54,16 @@ final readonly class StoredObjectRepository implements StoredObjectRepositoryInt return $this->repository->findOneBy($criteria); } + public function findByExpired(\DateTimeImmutable $expiredAtDate): iterable + { + $qb = $this->repository->createQueryBuilder('stored_object'); + $qb + ->where('stored_object.deleteAt <= :expiredAt') + ->setParameter('expiredAt', $expiredAtDate); + + return $qb->getQuery()->toIterable(hydrationMode: Query::HYDRATE_OBJECT); + } + public function getClassName(): string { return StoredObject::class; diff --git a/src/Bundle/ChillDocStoreBundle/Repository/StoredObjectRepositoryInterface.php b/src/Bundle/ChillDocStoreBundle/Repository/StoredObjectRepositoryInterface.php index c694f1e09..45dcfcf94 100644 --- a/src/Bundle/ChillDocStoreBundle/Repository/StoredObjectRepositoryInterface.php +++ b/src/Bundle/ChillDocStoreBundle/Repository/StoredObjectRepositoryInterface.php @@ -17,4 +17,10 @@ use Doctrine\Persistence\ObjectRepository; /** * @extends ObjectRepository */ -interface StoredObjectRepositoryInterface extends ObjectRepository {} +interface StoredObjectRepositoryInterface extends ObjectRepository +{ + /** + * @return iterable + */ + public function findByExpired(\DateTimeImmutable $expiredAtDate): iterable; +} diff --git a/src/Bundle/ChillDocStoreBundle/Service/StoredObjectCleaner/RemoveExpiredStoredObjectCronJob.php b/src/Bundle/ChillDocStoreBundle/Service/StoredObjectCleaner/RemoveExpiredStoredObjectCronJob.php new file mode 100644 index 000000000..1dbf94080 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Service/StoredObjectCleaner/RemoveExpiredStoredObjectCronJob.php @@ -0,0 +1,65 @@ +clock->now() >= $cronJobExecution->getLastEnd()->add(new \DateInterval('P7D')); + } + + public function getKey(): string + { + return self::KEY; + } + + public function run(array $lastExecutionData): ?array + { + $lastDeleted = $lastExecutionData[self::LAST_DELETED_KEY] ?? 0; + + foreach ($this->storedObjectRepository->findByExpired($this->clock->now()) as $storedObject) { + foreach ($storedObject->getVersions() as $version) { + $this->messageBus->dispatch(new RemoveOldVersionMessage($version->getId())); + } + $lastDeleted = max($lastDeleted, $storedObject->getId()); + } + + return [self::LAST_DELETED_KEY => $lastDeleted]; + } +} diff --git a/src/Bundle/ChillDocStoreBundle/Service/StoredObjectCleaner/RemoveOldVersionMessageHandler.php b/src/Bundle/ChillDocStoreBundle/Service/StoredObjectCleaner/RemoveOldVersionMessageHandler.php index 3d48f0267..69c9f283d 100644 --- a/src/Bundle/ChillDocStoreBundle/Service/StoredObjectCleaner/RemoveOldVersionMessageHandler.php +++ b/src/Bundle/ChillDocStoreBundle/Service/StoredObjectCleaner/RemoveOldVersionMessageHandler.php @@ -11,13 +11,24 @@ declare(strict_types=1); namespace Chill\DocStoreBundle\Service\StoredObjectCleaner; +use Chill\DocStoreBundle\Entity\StoredObject; use Chill\DocStoreBundle\Exception\StoredObjectManagerException; use Chill\DocStoreBundle\Repository\StoredObjectVersionRepository; use Chill\DocStoreBundle\Service\StoredObjectManagerInterface; use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; +use Symfony\Component\Clock\ClockInterface; use Symfony\Component\Messenger\Handler\MessageHandlerInterface; +/** + * Class RemoveOldVersionMessageHandler. + * + * This class is responsible for handling the RemoveOldVersionMessage. It implements the MessageHandlerInterface. + * It removes old versions of stored objects based on certain conditions. + * + * If a StoredObject is a candidate for deletion (is expired and no more version stored), it is also removed from the + * database. + */ final readonly class RemoveOldVersionMessageHandler implements MessageHandlerInterface { private const LOG_PREFIX = '[RemoveOldVersionMessageHandler] '; @@ -27,6 +38,7 @@ final readonly class RemoveOldVersionMessageHandler implements MessageHandlerInt private LoggerInterface $logger, private EntityManagerInterface $entityManager, private StoredObjectManagerInterface $storedObjectManager, + private ClockInterface $clock, ) {} /** @@ -37,6 +49,7 @@ final readonly class RemoveOldVersionMessageHandler implements MessageHandlerInt $this->logger->info(self::LOG_PREFIX.'Received one message', ['storedObjectVersionId' => $message->storedObjectVersionId]); $storedObjectVersion = $this->storedObjectVersionRepository->find($message->storedObjectVersionId); + $storedObject = $storedObjectVersion->getStoredObject(); if (null === $storedObjectVersion) { $this->logger->error(self::LOG_PREFIX.'StoredObjectVersion not found in database', ['storedObjectVersionId' => $message->storedObjectVersionId]); @@ -44,8 +57,13 @@ final readonly class RemoveOldVersionMessageHandler implements MessageHandlerInt } $this->storedObjectManager->delete($storedObjectVersion); - + // to ensure an immediate deletion $this->entityManager->remove($storedObjectVersion); + + if (StoredObject::canBeDeleted($this->clock->now(), $storedObject)) { + $this->entityManager->remove($storedObject); + } + $this->entityManager->flush(); // clear the entity manager for future usage diff --git a/src/Bundle/ChillDocStoreBundle/Service/StoredObjectManager.php b/src/Bundle/ChillDocStoreBundle/Service/StoredObjectManager.php index 51f32b98a..f87c60f82 100644 --- a/src/Bundle/ChillDocStoreBundle/Service/StoredObjectManager.php +++ b/src/Bundle/ChillDocStoreBundle/Service/StoredObjectManager.php @@ -226,9 +226,11 @@ final class StoredObjectManager implements StoredObjectManagerInterface try { $response = $this->client->request('DELETE', $signedUrl->url); - if (Response::HTTP_NO_CONTENT !== $response->getStatusCode()) { + if (! (Response::HTTP_NO_CONTENT === $response->getStatusCode() || Response::HTTP_NOT_FOUND === $response->getStatusCode())) { throw StoredObjectManagerException::invalidStatusCode($response->getStatusCode()); } + + $storedObjectVersion->getStoredObject()->removeVersion($storedObjectVersion); } catch (TransportExceptionInterface $exception) { throw StoredObjectManagerException::errorDuringHttpRequest($exception); } diff --git a/src/Bundle/ChillDocStoreBundle/Service/StoredObjectManagerInterface.php b/src/Bundle/ChillDocStoreBundle/Service/StoredObjectManagerInterface.php index 7d4711b06..8dc6f27da 100644 --- a/src/Bundle/ChillDocStoreBundle/Service/StoredObjectManagerInterface.php +++ b/src/Bundle/ChillDocStoreBundle/Service/StoredObjectManagerInterface.php @@ -39,7 +39,9 @@ interface StoredObjectManagerInterface public function read(StoredObject|StoredObjectVersion $document): string; /** - * Set the content of a StoredObject. + * Register the content of a new version for the StoredObject. + * + * The manager is also responsible for registering a version in the StoredObject, and return this version. * * @param StoredObject $document the document * @param string $clearContent The content to store in clear @@ -52,6 +54,11 @@ interface StoredObjectManagerInterface public function write(StoredObject $document, string $clearContent, ?string $contentType = null): StoredObjectVersion; /** + * Remove a version from the storage. + * + * This method is also responsible for removing the version from the StoredObject (using @see{StoredObject::removeVersion}) + * in case of success. + * * @throws StoredObjectManagerException */ public function delete(StoredObjectVersion $storedObjectVersion): void; diff --git a/src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectCleaner/RemoveExpiredStoredObjectCronJobTest.php b/src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectCleaner/RemoveExpiredStoredObjectCronJobTest.php new file mode 100644 index 000000000..ed7341a38 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectCleaner/RemoveExpiredStoredObjectCronJobTest.php @@ -0,0 +1,115 @@ +createMock(StoredObjectRepositoryInterface::class); + $clock = new MockClock(new \DateTimeImmutable('2024-01-01 00:00:00', new \DateTimeZone('+00:00'))); + + $cronJob = new RemoveExpiredStoredObjectCronJob($clock, $this->buildMessageBus(), $repository); + + self::assertEquals($expected, $cronJob->canRun($cronJobExecution)); + } + + public static function buildTestCanRunData(): iterable + { + yield [ + (new CronJobExecution('remove-expired-stored-object'))->setLastEnd(new \DateTimeImmutable('2023-12-25 00:00:00', new \DateTimeZone('+00:00'))), + true, + ]; + + yield [ + (new CronJobExecution('remove-expired-stored-object'))->setLastEnd(new \DateTimeImmutable('2023-12-24 23:59:59', new \DateTimeZone('+00:00'))), + true, + ]; + + yield [ + (new CronJobExecution('remove-expired-stored-object'))->setLastEnd(new \DateTimeImmutable('2023-12-25 00:00:01', new \DateTimeZone('+00:00'))), + false, + ]; + + yield [ + null, + true, + ]; + } + + public function testRun(): void + { + $repository = $this->createMock(StoredObjectRepositoryInterface::class); + $repository->expects($this->atLeastOnce())->method('findByExpired')->withAnyParameters()->willReturnCallback( + function (\DateTimeImmutable $date): iterable { + yield $this->buildStoredObject(3); + yield $this->buildStoredObject(1); + } + ); + $clock = new MockClock(); + + $cronJob = new RemoveExpiredStoredObjectCronJob($clock, $this->buildMessageBus(true), $repository); + + $actual = $cronJob->run([]); + + self::assertEquals(3, $actual['last-deleted-stored-object-id']); + } + + private function buildStoredObject(int $id): StoredObject + { + $object = new StoredObject(); + $object->registerVersion(); + $class = new \ReflectionClass($object); + $idProperty = $class->getProperty('id'); + $idProperty->setValue($object, $id); + + $classVersion = new \ReflectionClass($object->getCurrentVersion()); + $idPropertyVersion = $classVersion->getProperty('id'); + $idPropertyVersion->setValue($object->getCurrentVersion(), $id); + + return $object; + } + + private function buildMessageBus(bool $expectDistpatchAtLeastOnce = false): MessageBusInterface + { + $messageBus = $this->createMock(MessageBusInterface::class); + + $methodDispatch = match ($expectDistpatchAtLeastOnce) { + true => $messageBus->expects($this->atLeastOnce())->method('dispatch')->with($this->isInstanceOf(RemoveOldVersionMessage::class)), + false => $messageBus->method('dispatch'), + }; + + $methodDispatch->willReturnCallback(function (RemoveOldVersionMessage $message) { + return new Envelope($message); + }); + + return $messageBus; + } +} diff --git a/src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectCleaner/RemoveOldVersionMessageHandlerTest.php b/src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectCleaner/RemoveOldVersionMessageHandlerTest.php index 109453bb7..be268225d 100644 --- a/src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectCleaner/RemoveOldVersionMessageHandlerTest.php +++ b/src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectCleaner/RemoveOldVersionMessageHandlerTest.php @@ -12,6 +12,7 @@ declare(strict_types=1); namespace Tests\Service\StoredObjectCleaner; use Chill\DocStoreBundle\Entity\StoredObject; +use Chill\DocStoreBundle\Entity\StoredObjectVersion; use Chill\DocStoreBundle\Repository\StoredObjectVersionRepository; use Chill\DocStoreBundle\Service\StoredObjectCleaner\RemoveOldVersionMessage; use Chill\DocStoreBundle\Service\StoredObjectCleaner\RemoveOldVersionMessageHandler; @@ -19,6 +20,7 @@ use Chill\DocStoreBundle\Service\StoredObjectManagerInterface; use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; +use Symfony\Component\Clock\MockClock; /** * @internal @@ -44,8 +46,78 @@ class RemoveOldVersionMessageHandlerTest extends TestCase $storedObjectManager = $this->createMock(StoredObjectManagerInterface::class); $storedObjectManager->expects($this->once())->method('delete')->with($this->identicalTo($version)); - $handler = new RemoveOldVersionMessageHandler($storedObjectVersionRepository, new NullLogger(), $entityManager, $storedObjectManager); + $handler = new RemoveOldVersionMessageHandler($storedObjectVersionRepository, new NullLogger(), $entityManager, $storedObjectManager, new MockClock()); $handler(new RemoveOldVersionMessage(1)); } + + public function testInvokeWithStoredObjectToDelete(): void + { + $object = new StoredObject(); + $object->setDeleteAt(new \DateTimeImmutable('2023-12-01')); + $version = $object->registerVersion(); + + $storedObjectVersionRepository = $this->createMock(StoredObjectVersionRepository::class); + $storedObjectVersionRepository->expects($this->once())->method('find') + ->with($this->identicalTo(1)) + ->willReturn($version); + + $entityManager = $this->createMock(EntityManagerInterface::class); + $entityManager->expects($this->exactly(2))->method('remove')->with( + $this->logicalOr($this->identicalTo($version), $this->identicalTo($object)) + ); + $entityManager->expects($this->once())->method('flush'); + $entityManager->expects($this->once())->method('clear'); + + $handler = new RemoveOldVersionMessageHandler( + $storedObjectVersionRepository, + new NullLogger(), + $entityManager, + new DummyStoredObjectManager(), + new MockClock(new \DateTimeImmutable('2024-01-01')) + ); + + $handler(new RemoveOldVersionMessage(1)); + + self::assertCount(0, $object->getVersions()); + } +} + +class DummyStoredObjectManager implements StoredObjectManagerInterface +{ + public function getLastModified(StoredObject|StoredObjectVersion $document): \DateTimeInterface + { + throw new \RuntimeException(); + } + + public function getContentLength(StoredObject|StoredObjectVersion $document): int + { + throw new \RuntimeException(); + } + + public function read(StoredObject|StoredObjectVersion $document): string + { + throw new \RuntimeException(); + } + + public function write(StoredObject $document, string $clearContent, ?string $contentType = null): StoredObjectVersion + { + throw new \RuntimeException(); + } + + public function delete(StoredObjectVersion $storedObjectVersion): void + { + $object = $storedObjectVersion->getStoredObject(); + $object->removeVersion($storedObjectVersion); + } + + public function etag(StoredObject|StoredObjectVersion $document): string + { + throw new \RuntimeException(); + } + + public function clearCache(): void + { + throw new \RuntimeException(); + } }