From f92a720d63b5e8aa7a425bd60d3a0747244d2879 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 3 Apr 2024 09:06:43 +0200 Subject: [PATCH 1/6] Use short_url_visits_counts table when excluding short URLs which reached max visits --- .../Core/src/ShortUrl/Repository/ShortUrlListRepository.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php index e66bbdc2..b639ace4 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php @@ -147,7 +147,10 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh $qb->expr()->isNull('s.maxVisits'), $qb->expr()->gt( 's.maxVisits', - sprintf('(SELECT COUNT(innerV.id) FROM %s as innerV WHERE innerV.shortUrl=s)', Visit::class), + sprintf( + '(SELECT COALESCE(SUM(vc.count), 0) FROM %s as vc WHERE vc.shortUrl=s)', + ShortUrlVisitsCount::class, + ), ), )); } From fd882834d30f53b06ec103e40fe1894a4198245f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 3 Apr 2024 09:41:05 +0200 Subject: [PATCH 2/6] Create repository to handle expired short URLs deletion --- module/Core/config/dependencies.config.php | 4 + .../Model/ExpiredShortUrlsConditions.php | 25 +++++ .../Repository/ExpiredShortUrlsRepository.php | 77 ++++++++++++++ .../ExpiredShortUrlsRepositoryInterface.php | 20 ++++ .../DeleteExpiredShortUrlsRepositoryTest.php | 100 ++++++++++++++++++ 5 files changed, 226 insertions(+) create mode 100644 module/Core/src/ShortUrl/Model/ExpiredShortUrlsConditions.php create mode 100644 module/Core/src/ShortUrl/Repository/ExpiredShortUrlsRepository.php create mode 100644 module/Core/src/ShortUrl/Repository/ExpiredShortUrlsRepositoryInterface.php create mode 100644 module/Core/test-db/ShortUrl/Repository/DeleteExpiredShortUrlsRepositoryTest.php diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 5437555b..c78ce31a 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -57,6 +57,10 @@ return [ EntityRepositoryFactory::class, ShortUrl\Entity\ShortUrl::class, ], + ShortUrl\Repository\ExpiredShortUrlsRepository::class => [ + EntityRepositoryFactory::class, + ShortUrl\Entity\ShortUrl::class, + ], Tag\TagService::class => ConfigAbstractFactory::class, diff --git a/module/Core/src/ShortUrl/Model/ExpiredShortUrlsConditions.php b/module/Core/src/ShortUrl/Model/ExpiredShortUrlsConditions.php new file mode 100644 index 00000000..565b9e98 --- /dev/null +++ b/module/Core/src/ShortUrl/Model/ExpiredShortUrlsConditions.php @@ -0,0 +1,25 @@ +pastValidUntil || $this->maxVisitsReached; + } +} diff --git a/module/Core/src/ShortUrl/Repository/ExpiredShortUrlsRepository.php b/module/Core/src/ShortUrl/Repository/ExpiredShortUrlsRepository.php new file mode 100644 index 00000000..6d8aa0df --- /dev/null +++ b/module/Core/src/ShortUrl/Repository/ExpiredShortUrlsRepository.php @@ -0,0 +1,77 @@ +getEntityManager()->createQueryBuilder(); + $qb->delete(ShortUrl::class, 's'); + + return $this->applyConditions($qb, $conditions, fn () => (int) $qb->getQuery()->execute()); + } + + /** + * @inheritDoc + */ + public function dryCount(ExpiredShortUrlsConditions $conditions = new ExpiredShortUrlsConditions()): int + { + $qb = $this->getEntityManager()->createQueryBuilder(); + $qb->select('COUNT(s.id)') + ->from(ShortUrl::class, 's'); + + return $this->applyConditions($qb, $conditions, fn () => (int) $qb->getQuery()->getSingleScalarResult()); + } + + /** + * @param callable(): int $getResultFromQueryBuilder + */ + private function applyConditions( + QueryBuilder $qb, + ExpiredShortUrlsConditions $conditions, + callable $getResultFromQueryBuilder, + ): int { + if (! $conditions->hasConditions()) { + return 0; + } + + if ($conditions->pastValidUntil) { + $qb + ->where($qb->expr()->andX( + $qb->expr()->isNotNull('s.validUntil'), + $qb->expr()->lt('s.validUntil', ':now'), + )) + ->setParameter('now', Chronos::now()->toDateTimeString()); + } + + if ($conditions->maxVisitsReached) { + $qb->orWhere($qb->expr()->andX( + $qb->expr()->isNotNull('s.maxVisits'), + $qb->expr()->lte( + 's.maxVisits', + sprintf( + '(SELECT COALESCE(SUM(vc.count), 0) FROM %s as vc WHERE vc.shortUrl=s)', + ShortUrlVisitsCount::class, + ), + ), + )); + } + + return $getResultFromQueryBuilder(); + } +} diff --git a/module/Core/src/ShortUrl/Repository/ExpiredShortUrlsRepositoryInterface.php b/module/Core/src/ShortUrl/Repository/ExpiredShortUrlsRepositoryInterface.php new file mode 100644 index 00000000..e82c3e43 --- /dev/null +++ b/module/Core/src/ShortUrl/Repository/ExpiredShortUrlsRepositoryInterface.php @@ -0,0 +1,20 @@ +getEntityManager(); + $this->repository = new ExpiredShortUrlsRepository($em, $em->getClassMetadata(ShortUrl::class)); + } + + #[Test] + #[TestWith([new ExpiredShortUrlsConditions(pastValidUntil: false, maxVisitsReached: false), 0])] + #[TestWith([new ExpiredShortUrlsConditions(pastValidUntil: true, maxVisitsReached: false), 7])] + #[TestWith([new ExpiredShortUrlsConditions(pastValidUntil: false, maxVisitsReached: true), 6])] + #[TestWith([new ExpiredShortUrlsConditions(pastValidUntil: true, maxVisitsReached: true), 9])] + public function deletesExpectedAmountOfShortUrls( + ExpiredShortUrlsConditions $conditions, + int $expectedDeletedShortUrls, + ): void { + $createdShortUrls = $this->createDataSet(); + + self::assertEquals($expectedDeletedShortUrls, $this->repository->delete($conditions)); + self::assertEquals( + $createdShortUrls - $expectedDeletedShortUrls, + $this->getEntityManager()->getRepository(ShortUrl::class)->count(), + ); + } + + #[Test] + #[TestWith([new ExpiredShortUrlsConditions(pastValidUntil: false, maxVisitsReached: false), 0])] + #[TestWith([new ExpiredShortUrlsConditions(pastValidUntil: true, maxVisitsReached: false), 7])] + #[TestWith([new ExpiredShortUrlsConditions(pastValidUntil: false, maxVisitsReached: true), 6])] + #[TestWith([new ExpiredShortUrlsConditions(pastValidUntil: true, maxVisitsReached: true), 9])] + public function countsExpectedAmountOfShortUrls( + ExpiredShortUrlsConditions $conditions, + int $expectedShortUrlsCount, + ): void { + $createdShortUrls = $this->createDataSet(); + + self::assertEquals($expectedShortUrlsCount, $this->repository->dryCount($conditions)); + self::assertEquals($createdShortUrls, $this->getEntityManager()->getRepository(ShortUrl::class)->count()); + } + + private function createDataSet(): int + { + // Create some non-expired short URLs + $this->createShortUrls(5); + $this->createShortUrls(2, [ShortUrlInputFilter::VALID_UNTIL => Chronos::now()->addDays(1)->toAtomString()]); + $this->createShortUrls(3, [ShortUrlInputFilter::MAX_VISITS => 4], visitsPerShortUrl: 2); + + // Create some short URLs with a valid date in the past + $this->createShortUrls(3, [ShortUrlInputFilter::VALID_UNTIL => Chronos::now()->subDays(1)->toAtomString()]); + + // Create some short URLs which reached the max amount of visits + $this->createShortUrls(2, [ShortUrlInputFilter::MAX_VISITS => 3], visitsPerShortUrl: 3); + + // Create some short URLs with a valid date in the past which also reached the max amount of visits + $this->createShortUrls(4, [ + ShortUrlInputFilter::VALID_UNTIL => Chronos::now()->subDays(1)->toAtomString(), + ShortUrlInputFilter::MAX_VISITS => 3, + ], visitsPerShortUrl: 4); + + $this->getEntityManager()->flush(); + + return 5 + 2 + 3 + 3 + 2 + 4; + } + + private function createShortUrls(int $amountOfShortUrls, array $metadata = [], int $visitsPerShortUrl = 0): void + { + for ($i = 0; $i < $amountOfShortUrls; $i++) { + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ + ShortUrlInputFilter::LONG_URL => 'https://shlink.io', + ...$metadata, + ])); + $this->getEntityManager()->persist($shortUrl); + + for ($j = 0; $j < $visitsPerShortUrl; $j++) { + $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance())); + } + } + } +} From f2371e8a80270dc1982a66427270ab7974c2e3ed Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 3 Apr 2024 18:57:09 +0200 Subject: [PATCH 3/6] Add command to delete expired short URLs --- module/CLI/config/cli.config.php | 2 + module/CLI/config/dependencies.config.php | 2 + .../DeleteExpiredShortUrlsCommand.php | 75 +++++++++++++++++++ module/Core/config/dependencies.config.php | 1 + .../src/ShortUrl/DeleteShortUrlService.php | 21 +++++- .../DeleteShortUrlServiceInterface.php | 11 +++ .../Model/ExpiredShortUrlsConditions.php | 8 -- .../Repository/ExpiredShortUrlsRepository.php | 4 +- .../ExpiredShortUrlsRepositoryInterface.php | 4 +- .../Repository/ShortUrlListRepository.php | 1 - .../ShortUrl/DeleteShortUrlServiceTest.php | 29 ++++++- 11 files changed, 140 insertions(+), 18 deletions(-) create mode 100644 module/CLI/src/Command/ShortUrl/DeleteExpiredShortUrlsCommand.php diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index 94237c15..63b2de6f 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -14,6 +14,8 @@ return [ Command\ShortUrl\GetShortUrlVisitsCommand::NAME => Command\ShortUrl\GetShortUrlVisitsCommand::class, Command\ShortUrl\DeleteShortUrlCommand::NAME => Command\ShortUrl\DeleteShortUrlCommand::class, Command\ShortUrl\DeleteShortUrlVisitsCommand::NAME => Command\ShortUrl\DeleteShortUrlVisitsCommand::class, + Command\ShortUrl\DeleteExpiredShortUrlsCommand::NAME => + Command\ShortUrl\DeleteExpiredShortUrlsCommand::class, Command\Visit\LocateVisitsCommand::NAME => Command\Visit\LocateVisitsCommand::class, Command\Visit\DownloadGeoLiteDbCommand::NAME => Command\Visit\DownloadGeoLiteDbCommand::class, diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 282a1db5..875c8226 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -45,6 +45,7 @@ return [ Command\ShortUrl\GetShortUrlVisitsCommand::class => ConfigAbstractFactory::class, Command\ShortUrl\DeleteShortUrlCommand::class => ConfigAbstractFactory::class, Command\ShortUrl\DeleteShortUrlVisitsCommand::class => ConfigAbstractFactory::class, + Command\ShortUrl\DeleteExpiredShortUrlsCommand::class => ConfigAbstractFactory::class, Command\Visit\DownloadGeoLiteDbCommand::class => ConfigAbstractFactory::class, Command\Visit\LocateVisitsCommand::class => ConfigAbstractFactory::class, @@ -96,6 +97,7 @@ return [ Command\ShortUrl\GetShortUrlVisitsCommand::class => [Visit\VisitsStatsHelper::class], Command\ShortUrl\DeleteShortUrlCommand::class => [ShortUrl\DeleteShortUrlService::class], Command\ShortUrl\DeleteShortUrlVisitsCommand::class => [ShortUrl\ShortUrlVisitsDeleter::class], + Command\ShortUrl\DeleteExpiredShortUrlsCommand::class => [ShortUrl\DeleteShortUrlService::class], Command\Visit\DownloadGeoLiteDbCommand::class => [GeoLite\GeolocationDbUpdater::class], Command\Visit\LocateVisitsCommand::class => [ diff --git a/module/CLI/src/Command/ShortUrl/DeleteExpiredShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/DeleteExpiredShortUrlsCommand.php new file mode 100644 index 00000000..109beff7 --- /dev/null +++ b/module/CLI/src/Command/ShortUrl/DeleteExpiredShortUrlsCommand.php @@ -0,0 +1,75 @@ +setName(self::NAME) + ->setDescription( + 'Deletes all short URLs that are considered expired, because they have a validUntil date in the past', + ) + ->addOption( + 'evaluate-max-visits', + mode: InputOption::VALUE_NONE, + description: 'Also take into consideration short URLs which have reached their max amount of visits.', + ) + ->addOption('force', 'f', InputOption::VALUE_NONE, 'Delete short URLs with no confirmation') + ->addOption( + 'dry-run', + mode: InputOption::VALUE_NONE, + description: 'Delete short URLs with no confirmation', + ); + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $io = new SymfonyStyle($input, $output); + $force = $input->getOption('force') || ! $input->isInteractive(); + $dryRun = $input->getOption('dry-run'); + $conditions = new ExpiredShortUrlsConditions(maxVisitsReached: $input->getOption('evaluate-max-visits')); + + if (! $force && ! $dryRun) { + $io->warning([ + 'Careful!', + 'You are about to perform a destructive operation that can result in deleted short URLs and visits.', + 'This action cannot be undone. Proceed at your own risk', + ]); + if (! $io->confirm('Continue?', default: false)) { + return ExitCode::EXIT_WARNING; + } + } + + if ($dryRun) { + $result = $this->deleteShortUrlService->countExpiredShortUrls($conditions); + $io->success(sprintf('There are %s expired short URLs matching provided conditions', $result)); + return ExitCode::EXIT_SUCCESS; + } + + $result = $this->deleteShortUrlService->deleteExpiredShortUrls($conditions); + $io->success(sprintf('%s expired short URLs have been deleted', $result)); + return ExitCode::EXIT_SUCCESS; + } +} diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index c78ce31a..5f3d8fae 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -151,6 +151,7 @@ return [ 'em', Options\DeleteShortUrlsOptions::class, ShortUrl\ShortUrlResolver::class, + ShortUrl\Repository\ExpiredShortUrlsRepository::class, ], ShortUrl\ShortUrlResolver::class => ['em', Options\UrlShortenerOptions::class], ShortUrl\ShortUrlVisitsDeleter::class => [ diff --git a/module/Core/src/ShortUrl/DeleteShortUrlService.php b/module/Core/src/ShortUrl/DeleteShortUrlService.php index 2a39e695..b65381aa 100644 --- a/module/Core/src/ShortUrl/DeleteShortUrlService.php +++ b/module/Core/src/ShortUrl/DeleteShortUrlService.php @@ -8,15 +8,18 @@ use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Core\Exception; use Shlinkio\Shlink\Core\Options\DeleteShortUrlsOptions; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; +use Shlinkio\Shlink\Core\ShortUrl\Model\ExpiredShortUrlsConditions; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; +use Shlinkio\Shlink\Core\ShortUrl\Repository\ExpiredShortUrlsRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; -class DeleteShortUrlService implements DeleteShortUrlServiceInterface +readonly class DeleteShortUrlService implements DeleteShortUrlServiceInterface { public function __construct( - private readonly EntityManagerInterface $em, - private readonly DeleteShortUrlsOptions $deleteShortUrlsOptions, - private readonly ShortUrlResolverInterface $urlResolver, + private EntityManagerInterface $em, + private DeleteShortUrlsOptions $deleteShortUrlsOptions, + private ShortUrlResolverInterface $urlResolver, + private ExpiredShortUrlsRepositoryInterface $expiredShortUrlsRepository, ) { } @@ -47,4 +50,14 @@ class DeleteShortUrlService implements DeleteShortUrlServiceInterface $this->deleteShortUrlsOptions->visitsThreshold, ); } + + public function deleteExpiredShortUrls(ExpiredShortUrlsConditions $conditions): int + { + return $this->expiredShortUrlsRepository->delete($conditions); + } + + public function countExpiredShortUrls(ExpiredShortUrlsConditions $conditions): int + { + return $this->expiredShortUrlsRepository->dryCount($conditions); + } } diff --git a/module/Core/src/ShortUrl/DeleteShortUrlServiceInterface.php b/module/Core/src/ShortUrl/DeleteShortUrlServiceInterface.php index 0a7420f1..32eaffa1 100644 --- a/module/Core/src/ShortUrl/DeleteShortUrlServiceInterface.php +++ b/module/Core/src/ShortUrl/DeleteShortUrlServiceInterface.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl; use Shlinkio\Shlink\Core\Exception; +use Shlinkio\Shlink\Core\ShortUrl\Model\ExpiredShortUrlsConditions; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -19,4 +20,14 @@ interface DeleteShortUrlServiceInterface bool $ignoreThreshold = false, ?ApiKey $apiKey = null, ): void; + + /** + * Deletes short URLs that are considered expired based on provided conditions + */ + public function deleteExpiredShortUrls(ExpiredShortUrlsConditions $conditions): int; + + /** + * Counts short URLs that are considered expired based on provided conditions, without really deleting them + */ + public function countExpiredShortUrls(ExpiredShortUrlsConditions $conditions): int; } diff --git a/module/Core/src/ShortUrl/Model/ExpiredShortUrlsConditions.php b/module/Core/src/ShortUrl/Model/ExpiredShortUrlsConditions.php index 565b9e98..d4f0c063 100644 --- a/module/Core/src/ShortUrl/Model/ExpiredShortUrlsConditions.php +++ b/module/Core/src/ShortUrl/Model/ExpiredShortUrlsConditions.php @@ -10,14 +10,6 @@ final readonly class ExpiredShortUrlsConditions { } - public static function fromQuery(array $query): self - { - return new self( - pastValidUntil: (bool) ($query['pastValidUntil'] ?? true), - maxVisitsReached: (bool) ($query['maxVisitsReached'] ?? true), - ); - } - public function hasConditions(): bool { return $this->pastValidUntil || $this->maxVisitsReached; diff --git a/module/Core/src/ShortUrl/Repository/ExpiredShortUrlsRepository.php b/module/Core/src/ShortUrl/Repository/ExpiredShortUrlsRepository.php index 6d8aa0df..0b796971 100644 --- a/module/Core/src/ShortUrl/Repository/ExpiredShortUrlsRepository.php +++ b/module/Core/src/ShortUrl/Repository/ExpiredShortUrlsRepository.php @@ -18,7 +18,7 @@ class ExpiredShortUrlsRepository extends EntitySpecificationRepository implement /** * @inheritDoc */ - public function delete(ExpiredShortUrlsConditions $conditions = new ExpiredShortUrlsConditions()): int + public function delete(ExpiredShortUrlsConditions $conditions): int { $qb = $this->getEntityManager()->createQueryBuilder(); $qb->delete(ShortUrl::class, 's'); @@ -29,7 +29,7 @@ class ExpiredShortUrlsRepository extends EntitySpecificationRepository implement /** * @inheritDoc */ - public function dryCount(ExpiredShortUrlsConditions $conditions = new ExpiredShortUrlsConditions()): int + public function dryCount(ExpiredShortUrlsConditions $conditions): int { $qb = $this->getEntityManager()->createQueryBuilder(); $qb->select('COUNT(s.id)') diff --git a/module/Core/src/ShortUrl/Repository/ExpiredShortUrlsRepositoryInterface.php b/module/Core/src/ShortUrl/Repository/ExpiredShortUrlsRepositoryInterface.php index e82c3e43..96032065 100644 --- a/module/Core/src/ShortUrl/Repository/ExpiredShortUrlsRepositoryInterface.php +++ b/module/Core/src/ShortUrl/Repository/ExpiredShortUrlsRepositoryInterface.php @@ -11,10 +11,10 @@ interface ExpiredShortUrlsRepositoryInterface /** * Delete expired short URLs matching provided conditions */ - public function delete(ExpiredShortUrlsConditions $conditions = new ExpiredShortUrlsConditions()): int; + public function delete(ExpiredShortUrlsConditions $conditions): int; /** * Count how many expired short URLs would be deleted for provided conditions */ - public function dryCount(ExpiredShortUrlsConditions $conditions = new ExpiredShortUrlsConditions()): int; + public function dryCount(ExpiredShortUrlsConditions $conditions): int; } diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php index b639ace4..8aac9b73 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php @@ -16,7 +16,6 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode; use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsCountFiltering; use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsListFiltering; use Shlinkio\Shlink\Core\Visit\Entity\ShortUrlVisitsCount; -use Shlinkio\Shlink\Core\Visit\Entity\Visit; use function Shlinkio\Shlink\Core\ArrayUtils\map; use function sprintf; diff --git a/module/Core/test/ShortUrl/DeleteShortUrlServiceTest.php b/module/Core/test/ShortUrl/DeleteShortUrlServiceTest.php index 3ac9897c..4788818e 100644 --- a/module/Core/test/ShortUrl/DeleteShortUrlServiceTest.php +++ b/module/Core/test/ShortUrl/DeleteShortUrlServiceTest.php @@ -13,7 +13,9 @@ use Shlinkio\Shlink\Core\Exception\DeleteShortUrlException; use Shlinkio\Shlink\Core\Options\DeleteShortUrlsOptions; use Shlinkio\Shlink\Core\ShortUrl\DeleteShortUrlService; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; +use Shlinkio\Shlink\Core\ShortUrl\Model\ExpiredShortUrlsConditions; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; +use Shlinkio\Shlink\Core\ShortUrl\Repository\ExpiredShortUrlsRepository; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\Visitor; @@ -26,6 +28,7 @@ class DeleteShortUrlServiceTest extends TestCase { private MockObject & EntityManagerInterface $em; private MockObject & ShortUrlResolverInterface $urlResolver; + private MockObject & ExpiredShortUrlsRepository $expiredShortUrlsRepository; private string $shortCode; protected function setUp(): void @@ -39,6 +42,8 @@ class DeleteShortUrlServiceTest extends TestCase $this->urlResolver = $this->createMock(ShortUrlResolverInterface::class); $this->urlResolver->method('resolveShortUrl')->willReturn($shortUrl); + + $this->expiredShortUrlsRepository = $this->createMock(ExpiredShortUrlsRepository::class); } #[Test] @@ -94,11 +99,33 @@ class DeleteShortUrlServiceTest extends TestCase $service->deleteByShortCode(ShortUrlIdentifier::fromShortCodeAndDomain($this->shortCode)); } + #[Test] + public function deleteExpiredShortUrlsDelegatesToRepository(): void + { + $conditions = new ExpiredShortUrlsConditions(); + $this->expiredShortUrlsRepository->expects($this->once())->method('delete')->with($conditions)->willReturn(5); + + $result = $this->createService()->deleteExpiredShortUrls($conditions); + + self::assertEquals(5, $result); + } + + #[Test] + public function countExpiredShortUrlsDelegatesToRepository(): void + { + $conditions = new ExpiredShortUrlsConditions(); + $this->expiredShortUrlsRepository->expects($this->once())->method('dryCount')->with($conditions)->willReturn(2); + + $result = $this->createService()->countExpiredShortUrls($conditions); + + self::assertEquals(2, $result); + } + private function createService(bool $checkVisitsThreshold = true, int $visitsThreshold = 5): DeleteShortUrlService { return new DeleteShortUrlService($this->em, new DeleteShortUrlsOptions( $visitsThreshold, $checkVisitsThreshold, - ), $this->urlResolver); + ), $this->urlResolver, $this->expiredShortUrlsRepository); } } From 527d28ad81b78b0d812f0e88b146705208dbe9cc Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 3 Apr 2024 19:18:56 +0200 Subject: [PATCH 4/6] Add DeleteExpiredShortUrlsCommand test --- CHANGELOG.md | 5 ++ .../DeleteExpiredShortUrlsCommandTest.php | 90 +++++++++++++++++++ 2 files changed, 95 insertions(+) create mode 100644 module/CLI/test/Command/ShortUrl/DeleteExpiredShortUrlsCommandTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 07d4e3ac..4dc88849 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this Previously, this was exposed only for orphan visits, since this can be an arbitrary value for those. * [#2077](https://github.com/shlinkio/shlink/issues/2077) When sending visits to Matomo, the short URL title is now used as document title in matomo. +* [#2059](https://github.com/shlinkio/shlink/issues/2059) Add new `short-url:delete-expired` command that can be used to programmatically delete expired short URLs. + + Expired short URLs are those that have a `calidUntil` date in the past, or optionally, that have reached the max amount of visits. + + This command can be run periodically by those who create many disposable URLs which are valid only for a period of time, and then can be deleted to save space. ### Changed * [#2034](https://github.com/shlinkio/shlink/issues/2034) Modernize entities, using constructor property promotion and readonly wherever possible. diff --git a/module/CLI/test/Command/ShortUrl/DeleteExpiredShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/DeleteExpiredShortUrlsCommandTest.php new file mode 100644 index 00000000..5930a93c --- /dev/null +++ b/module/CLI/test/Command/ShortUrl/DeleteExpiredShortUrlsCommandTest.php @@ -0,0 +1,90 @@ +service = $this->createMock(DeleteShortUrlServiceInterface::class); + $this->commandTester = CliTestUtils::testerForCommand(new DeleteExpiredShortUrlsCommand($this->service)); + } + + #[Test] + public function warningIsDisplayedAndExecutionCanBeCancelled(): void + { + $this->service->expects($this->never())->method('countExpiredShortUrls'); + $this->service->expects($this->never())->method('deleteExpiredShortUrls'); + + $this->commandTester->setInputs(['n']); + $this->commandTester->execute([]); + $output = $this->commandTester->getDisplay(); + $status = $this->commandTester->getStatusCode(); + + self::assertStringContainsString('Careful!', $output); + self::assertEquals(ExitCode::EXIT_WARNING, $status); + } + + #[Test] + #[TestWith([[], true])] + #[TestWith([['--force' => true], false])] + #[TestWith([['-f' => true], false])] + public function deletionIsExecutedByDefault(array $input, bool $expectsWarning): void + { + $this->service->expects($this->never())->method('countExpiredShortUrls'); + $this->service->expects($this->once())->method('deleteExpiredShortUrls')->willReturn(5); + + $this->commandTester->setInputs(['y']); + $this->commandTester->execute($input); + $output = $this->commandTester->getDisplay(); + $status = $this->commandTester->getStatusCode(); + + if ($expectsWarning) { + self::assertStringContainsString('Careful!', $output); + } else { + self::assertStringNotContainsString('Careful!', $output); + } + self::assertStringContainsString('5 expired short URLs have been deleted', $output); + self::assertEquals(ExitCode::EXIT_SUCCESS, $status); + } + + #[Test] + public function countIsExecutedDuringDryRun(): void + { + $this->service->expects($this->once())->method('countExpiredShortUrls')->willReturn(38); + $this->service->expects($this->never())->method('deleteExpiredShortUrls'); + + $this->commandTester->execute(['--dry-run' => true]); + $output = $this->commandTester->getDisplay(); + $status = $this->commandTester->getStatusCode(); + + self::assertStringNotContainsString('Careful!', $output); + self::assertStringContainsString('There are 38 expired short URLs matching provided conditions', $output); + self::assertEquals(ExitCode::EXIT_SUCCESS, $status); + } + + #[Test] + #[TestWith([[], new ExpiredShortUrlsConditions()])] + #[TestWith([['--evaluate-max-visits' => true], new ExpiredShortUrlsConditions(maxVisitsReached: true)])] + public function providesExpectedConditionsToService(array $extraInput, ExpiredShortUrlsConditions $conditions): void + { + $this->service->expects($this->once())->method('countExpiredShortUrls')->with($conditions)->willReturn(4); + $this->commandTester->execute(['--dry-run' => true, ...$extraInput]); + } +} From 38819965602d52d399677687fa1c9a901ecc0198 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 3 Apr 2024 19:20:38 +0200 Subject: [PATCH 5/6] Migrate from docker-compose to docker compose in CI pipelines --- .github/workflows/ci-db-tests.yml | 4 ++-- .github/workflows/ci-tests.yml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci-db-tests.yml b/.github/workflows/ci-db-tests.yml index dd797e83..8cea11f7 100644 --- a/.github/workflows/ci-db-tests.yml +++ b/.github/workflows/ci-db-tests.yml @@ -23,7 +23,7 @@ jobs: run: sudo ./data/infra/ci/install-ms-odbc.sh - name: Start database server if: ${{ inputs.platform != 'sqlite:ci' }} - run: docker-compose -f docker-compose.yml -f docker-compose.ci.yml up -d shlink_db_${{ inputs.platform }} + run: docker compose -f docker-compose.yml -f docker-compose.ci.yml up -d shlink_db_${{ inputs.platform }} - uses: './.github/actions/ci-setup' with: php-version: ${{ matrix.php-version }} @@ -31,7 +31,7 @@ jobs: extensions-cache-key: db-tests-extensions-${{ matrix.php-version }}-${{ inputs.platform }} - name: Create test database if: ${{ inputs.platform == 'ms' }} - run: docker-compose exec -T shlink_db_ms /opt/mssql-tools/bin/sqlcmd -S localhost -U sa -P 'Passw0rd!' -Q "CREATE DATABASE shlink_test;" + run: docker compose exec -T shlink_db_ms /opt/mssql-tools/bin/sqlcmd -S localhost -U sa -P 'Passw0rd!' -Q "CREATE DATABASE shlink_test;" - name: Run tests run: composer test:db:${{ inputs.platform }} - name: Upload code coverage diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index ea26ccd7..d2cf4d9a 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -20,10 +20,10 @@ jobs: - uses: actions/checkout@v4 - name: Start postgres database server if: ${{ inputs.test-group == 'api' }} - run: docker-compose -f docker-compose.yml -f docker-compose.ci.yml up -d shlink_db_postgres + run: docker compose -f docker-compose.yml -f docker-compose.ci.yml up -d shlink_db_postgres - name: Start maria database server if: ${{ inputs.test-group == 'cli' }} - run: docker-compose -f docker-compose.yml -f docker-compose.ci.yml up -d shlink_db_maria + run: docker compose -f docker-compose.yml -f docker-compose.ci.yml up -d shlink_db_maria - uses: './.github/actions/ci-setup' with: php-version: ${{ matrix.php-version }} From b7db676cba848a282c7f004f0b217b2a851f8a65 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 3 Apr 2024 19:21:39 +0200 Subject: [PATCH 6/6] Test non-interactivity on DeleteExpiredShortUrlsCommand --- CHANGELOG.md | 2 +- .../ShortUrl/DeleteExpiredShortUrlsCommandTest.php | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4dc88849..a397f3af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#2077](https://github.com/shlinkio/shlink/issues/2077) When sending visits to Matomo, the short URL title is now used as document title in matomo. * [#2059](https://github.com/shlinkio/shlink/issues/2059) Add new `short-url:delete-expired` command that can be used to programmatically delete expired short URLs. - Expired short URLs are those that have a `calidUntil` date in the past, or optionally, that have reached the max amount of visits. + Expired short URLs are those that have a `validUntil` date in the past, or optionally, that have reached the max amount of visits. This command can be run periodically by those who create many disposable URLs which are valid only for a period of time, and then can be deleted to save space. diff --git a/module/CLI/test/Command/ShortUrl/DeleteExpiredShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/DeleteExpiredShortUrlsCommandTest.php index 5930a93c..ea580064 100644 --- a/module/CLI/test/Command/ShortUrl/DeleteExpiredShortUrlsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/DeleteExpiredShortUrlsCommandTest.php @@ -42,16 +42,17 @@ class DeleteExpiredShortUrlsCommandTest extends TestCase } #[Test] - #[TestWith([[], true])] - #[TestWith([['--force' => true], false])] - #[TestWith([['-f' => true], false])] - public function deletionIsExecutedByDefault(array $input, bool $expectsWarning): void + #[TestWith([[], [], true])] + #[TestWith([['--force' => true], [], false])] + #[TestWith([['-f' => true], [], false])] + #[TestWith([[], ['interactive' => false], false])] + public function deletionIsExecutedByDefault(array $input, array $options, bool $expectsWarning): void { $this->service->expects($this->never())->method('countExpiredShortUrls'); $this->service->expects($this->once())->method('deleteExpiredShortUrls')->willReturn(5); $this->commandTester->setInputs(['y']); - $this->commandTester->execute($input); + $this->commandTester->execute($input, $options); $output = $this->commandTester->getDisplay(); $status = $this->commandTester->getStatusCode();