diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php index 48f597f3..fcaaa5dc 100644 --- a/module/Core/src/Entity/ShortUrl.php +++ b/module/Core/src/Entity/ShortUrl.php @@ -175,7 +175,7 @@ class ShortUrl extends AbstractEntity public function getVisitsCount(): int { - return count($this->visits); + return \count($this->visits); } /** diff --git a/module/Core/src/Exception/DeleteShortUrlException.php b/module/Core/src/Exception/DeleteShortUrlException.php new file mode 100644 index 00000000..8e2914d7 --- /dev/null +++ b/module/Core/src/Exception/DeleteShortUrlException.php @@ -0,0 +1,34 @@ +visitsThreshold = $visitsThreshold; + parent::__construct($message, $code, $previous); + } + + public static function fromVisitsThreshold(int $threshold, string $shortCode): self + { + return new self($threshold, \sprintf( + 'Impossible to delete short URL with short code "%s" since it has more than "%s" visits.', + $shortCode, + $threshold + )); + } + + public function getVisitsThreshold(): int + { + return $this->visitsThreshold; + } +} diff --git a/module/Core/src/Exception/InvalidShortCodeException.php b/module/Core/src/Exception/InvalidShortCodeException.php index d2201f1d..c8123a74 100644 --- a/module/Core/src/Exception/InvalidShortCodeException.php +++ b/module/Core/src/Exception/InvalidShortCodeException.php @@ -7,9 +7,9 @@ class InvalidShortCodeException extends RuntimeException { public static function fromCharset($shortCode, $charSet, \Exception $previous = null) { - $code = isset($previous) ? $previous->getCode() : -1; + $code = $previous !== null ? $previous->getCode() : -1; return new static( - sprintf('Provided short code "%s" does not match the char set "%s"', $shortCode, $charSet), + \sprintf('Provided short code "%s" does not match the char set "%s"', $shortCode, $charSet), $code, $previous ); @@ -17,6 +17,6 @@ class InvalidShortCodeException extends RuntimeException public static function fromNotFoundShortCode($shortCode) { - return new static(sprintf('Provided short code "%s" does not belong to a short URL', $shortCode)); + return new static(\sprintf('Provided short code "%s" does not belong to a short URL', $shortCode)); } } diff --git a/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php b/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php new file mode 100644 index 00000000..4f7607eb --- /dev/null +++ b/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php @@ -0,0 +1,56 @@ +em = $em; + $this->deleteShortUrlsOptions = $deleteShortUrlsOptions; + } + + /** + * @throws Exception\InvalidShortCodeException + * @throws Exception\DeleteShortUrlException + */ + public function deleteByShortCode(string $shortCode): void + { + $shortUrl = $this->findByShortCode($this->em, $shortCode); + if ($this->isThresholdReached($shortUrl)) { + throw Exception\DeleteShortUrlException::fromVisitsThreshold( + $this->deleteShortUrlsOptions->getVisitsThreshold(), + $shortUrl->getShortCode() + ); + } + + $this->em->remove($shortUrl); + $this->em->flush(); + } + + private function isThresholdReached(ShortUrl $shortUrl): bool + { + if (! $this->deleteShortUrlsOptions->doCheckVisitsThreshold()) { + return false; + } + + return $shortUrl->getVisitsCount() >= $this->deleteShortUrlsOptions->getVisitsThreshold(); + } +} diff --git a/module/Core/src/Service/ShortUrl/DeleteShortUrlServiceInterface.php b/module/Core/src/Service/ShortUrl/DeleteShortUrlServiceInterface.php new file mode 100644 index 00000000..4afbb15a --- /dev/null +++ b/module/Core/src/Service/ShortUrl/DeleteShortUrlServiceInterface.php @@ -0,0 +1,15 @@ +getRepository(ShortUrl::class)->findOneBy([ + 'shortCode' => $shortCode, + ]); + if ($shortUrl === null) { + throw InvalidShortCodeException::fromNotFoundShortCode($shortCode); + } + + return $shortUrl; + } +} diff --git a/module/Core/src/Service/ShortUrlService.php b/module/Core/src/Service/ShortUrlService.php index b96adff0..91e64bf1 100644 --- a/module/Core/src/Service/ShortUrlService.php +++ b/module/Core/src/Service/ShortUrlService.php @@ -9,11 +9,13 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; +use Shlinkio\Shlink\Core\Service\ShortUrl\FindShortCodeTrait; use Shlinkio\Shlink\Core\Util\TagManagerTrait; use Zend\Paginator\Paginator; class ShortUrlService implements ShortUrlServiceInterface { + use FindShortCodeTrait; use TagManagerTrait; /** @@ -48,7 +50,7 @@ class ShortUrlService implements ShortUrlServiceInterface */ public function setTagsByShortCode(string $shortCode, array $tags = []): ShortUrl { - $shortUrl = $this->findByShortCode($shortCode); + $shortUrl = $this->findByShortCode($this->em, $shortCode); $shortUrl->setTags($this->tagNamesToEntities($this->em, $tags)); $this->em->flush(); @@ -60,7 +62,7 @@ class ShortUrlService implements ShortUrlServiceInterface */ public function updateMetadataByShortCode(string $shortCode, ShortUrlMeta $shortCodeMeta): ShortUrl { - $shortUrl = $this->findByShortCode($shortCode); + $shortUrl = $this->findByShortCode($this->em, $shortCode); if ($shortCodeMeta->hasValidSince()) { $shortUrl->setValidSince($shortCodeMeta->getValidSince()); } @@ -77,31 +79,4 @@ class ShortUrlService implements ShortUrlServiceInterface return $shortUrl; } - - /** - * @throws InvalidShortCodeException - */ - public function deleteByShortCode(string $shortCode): void - { - $this->em->remove($this->findByShortCode($shortCode)); - $this->em->flush(); - } - - /** - * @param string $shortCode - * @return ShortUrl - * @throws InvalidShortCodeException - */ - private function findByShortCode(string $shortCode): ShortUrl - { - /** @var ShortUrl|null $shortUrl */ - $shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy([ - 'shortCode' => $shortCode, - ]); - if ($shortUrl === null) { - throw InvalidShortCodeException::fromNotFoundShortCode($shortCode); - } - - return $shortUrl; - } } diff --git a/module/Core/src/Service/ShortUrlServiceInterface.php b/module/Core/src/Service/ShortUrlServiceInterface.php index d616648c..c6c61126 100644 --- a/module/Core/src/Service/ShortUrlServiceInterface.php +++ b/module/Core/src/Service/ShortUrlServiceInterface.php @@ -27,9 +27,4 @@ interface ShortUrlServiceInterface * @throws InvalidShortCodeException */ public function updateMetadataByShortCode(string $shortCode, ShortUrlMeta $shortCodeMeta): ShortUrl; - - /** - * @throws InvalidShortCodeException - */ - public function deleteByShortCode(string $shortCode): void; } diff --git a/module/Core/test/Exception/DeleteShortUrlExceptionTest.php b/module/Core/test/Exception/DeleteShortUrlExceptionTest.php new file mode 100644 index 00000000..6412233e --- /dev/null +++ b/module/Core/test/Exception/DeleteShortUrlExceptionTest.php @@ -0,0 +1,61 @@ +assertEquals($expectedMessage, $e->getMessage()); + } + + public function provideMessages(): array + { + return [ + [ + 50, + 'abc123', + 'Impossible to delete short URL with short code "abc123" since it has more than "50" visits.', + ], + [ + 33, + 'def456', + 'Impossible to delete short URL with short code "def456" since it has more than "33" visits.', + ], + [ + 5713, + 'foobar', + 'Impossible to delete short URL with short code "foobar" since it has more than "5713" visits.', + ], + ]; + } + + /** + * @test + * @dataProvider provideThresholds + */ + public function visitsThresholdIsProperlyReturned(int $threshold) + { + $e = new DeleteShortUrlException($threshold); + $this->assertEquals($threshold, $e->getVisitsThreshold()); + } + + public function provideThresholds(): array + { + return \array_map(function (int $number) { + return [$number]; + }, \range(5, 50, 5)); + } +} diff --git a/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php b/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php new file mode 100644 index 00000000..727ad187 --- /dev/null +++ b/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php @@ -0,0 +1,97 @@ +setShortCode('abc123') + ->setVisits(new ArrayCollection(\array_map(function () { + return new Visit(); + }, \range(0, 10)))); + + $this->em = $this->prophesize(EntityManagerInterface::class); + + $repo = $this->prophesize(ShortUrlRepositoryInterface::class); + $repo->findOneBy(Argument::type('array'))->willReturn($shortUrl); + $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); + } + + /** + * @test + */ + public function deleteByShortCodeThrowsExceptionWhenThresholdIsReached() + { + $service = $this->createService(); + + $this->expectException(DeleteShortUrlException::class); + $this->expectExceptionMessage( + 'Impossible to delete short URL with short code "abc123" since it has more than "5" visits.' + ); + + $service->deleteByShortCode('abc123'); + } + + /** + * @test + */ + public function deleteByShortCodeDeletesUrlWhenThresholdIsReachedButCheckIsDisabled() + { + $service = $this->createService(false); + + $remove = $this->em->remove(Argument::type(ShortUrl::class))->willReturn(null); + $flush = $this->em->flush()->willReturn(null); + + $service->deleteByShortCode('abc123'); + + $remove->shouldHaveBeenCalledTimes(1); + $flush->shouldHaveBeenCalledTimes(1); + } + + /** + * @test + */ + public function deleteByShortCodeDeletesUrlWhenThresholdIsNotReached() + { + $service = $this->createService(true, 100); + + $remove = $this->em->remove(Argument::type(ShortUrl::class))->willReturn(null); + $flush = $this->em->flush()->willReturn(null); + + $service->deleteByShortCode('abc123'); + + $remove->shouldHaveBeenCalledTimes(1); + $flush->shouldHaveBeenCalledTimes(1); + } + + private function createService(bool $checkVisitsThreshold = true, int $visitsThreshold = 5): DeleteShortUrlService + { + return new DeleteShortUrlService($this->em->reveal(), new DeleteShortUrlsOptions([ + 'visitsThreshold' => $visitsThreshold, + 'checkVisitsThreshold' => $checkVisitsThreshold, + ])); + } +}