From e028d8ea316a28a72778fbd4e25f83fb267192b5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 18 Mar 2024 22:09:15 +0100 Subject: [PATCH] Move logic to serialize ShortUrls to entity itself --- .../src/ShortUrl/DeleteShortUrlService.php | 8 +-- module/Core/src/ShortUrl/Entity/ShortUrl.php | 72 ++++++++----------- .../Transformer/ShortUrlDataTransformer.php | 35 +-------- .../PublishingUpdatesGeneratorTest.php | 2 +- .../test/ShortUrl/ShortUrlServiceTest.php | 8 ++- 5 files changed, 42 insertions(+), 83 deletions(-) diff --git a/module/Core/src/ShortUrl/DeleteShortUrlService.php b/module/Core/src/ShortUrl/DeleteShortUrlService.php index 16fe6ac0..2a39e695 100644 --- a/module/Core/src/ShortUrl/DeleteShortUrlService.php +++ b/module/Core/src/ShortUrl/DeleteShortUrlService.php @@ -43,10 +43,8 @@ class DeleteShortUrlService implements DeleteShortUrlServiceInterface private function isThresholdReached(ShortUrl $shortUrl): bool { - if (! $this->deleteShortUrlsOptions->checkVisitsThreshold) { - return false; - } - - return $shortUrl->getVisitsCount() >= $this->deleteShortUrlsOptions->visitsThreshold; + return $this->deleteShortUrlsOptions->checkVisitsThreshold && $shortUrl->reachedVisits( + $this->deleteShortUrlsOptions->visitsThreshold, + ); } } diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php index ef703e54..474d5afc 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -20,10 +20,12 @@ use Shlinkio\Shlink\Core\ShortUrl\Resolver\ShortUrlRelationResolverInterface; use Shlinkio\Shlink\Core\ShortUrl\Resolver\SimpleShortUrlRelationResolver; use Shlinkio\Shlink\Core\Tag\Entity\Tag; use Shlinkio\Shlink\Core\Visit\Entity\Visit; +use Shlinkio\Shlink\Core\Visit\Model\VisitsSummary; use Shlinkio\Shlink\Core\Visit\Model\VisitType; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use Shlinkio\Shlink\Rest\Entity\ApiKey; +use function array_map; use function count; use function Shlinkio\Shlink\Core\generateRandomShortCode; use function Shlinkio\Shlink\Core\normalizeDate; @@ -187,33 +189,9 @@ class ShortUrl extends AbstractEntity return $this->domain; } - /** - * @return Collection - */ - public function getTags(): Collection + public function reachedVisits(int $visitsAmount): bool { - return $this->tags; - } - - public function getValidSince(): ?Chronos - { - return $this->validSince; - } - - public function getValidUntil(): ?Chronos - { - return $this->validUntil; - } - - public function getVisitsCount(): int - { - return count($this->visits); - } - - public function nonBotVisitsCount(): int - { - $criteria = Criteria::create()->where(Criteria::expr()->eq('potentialBot', false)); - return count($this->visits->matching($criteria)); + return count($this->visits) >= $visitsAmount; } public function mostRecentImportedVisitDate(): ?Chronos @@ -236,21 +214,6 @@ class ShortUrl extends AbstractEntity return $this; } - public function getMaxVisits(): ?int - { - return $this->maxVisits; - } - - public function title(): ?string - { - return $this->title; - } - - public function crawlable(): bool - { - return $this->crawlable; - } - public function forwardQuery(): bool { return $this->forwardQuery; @@ -276,7 +239,7 @@ class ShortUrl extends AbstractEntity public function isEnabled(): bool { - $maxVisitsReached = $this->maxVisits !== null && $this->getVisitsCount() >= $this->maxVisits; + $maxVisitsReached = $this->maxVisits !== null && $this->reachedVisits($this->maxVisits); if ($maxVisitsReached) { return false; } @@ -294,4 +257,29 @@ class ShortUrl extends AbstractEntity return true; } + + public function toArray(): array + { + return [ + 'shortCode' => $this->shortCode, + 'longUrl' => $this->longUrl, + 'dateCreated' => $this->dateCreated->toAtomString(), + 'tags' => array_map(static fn (Tag $tag) => $tag->__toString(), $this->tags->toArray()), + 'meta' => [ + 'validSince' => $this->validSince?->toAtomString(), + 'validUntil' => $this->validUntil?->toAtomString(), + 'maxVisits' => $this->maxVisits, + ], + 'domain' => $this->domain, + 'title' => $this->title, + 'crawlable' => $this->crawlable, + 'forwardQuery' => $this->forwardQuery, + 'visitsSummary' => VisitsSummary::fromTotalAndNonBots( + count($this->visits), + count($this->visits->matching( + Criteria::create()->where(Criteria::expr()->eq('potentialBot', false)), + )), + ), + ]; + } } diff --git a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php index ea694c61..09b9436b 100644 --- a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php +++ b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php @@ -7,14 +7,10 @@ namespace Shlinkio\Shlink\Core\ShortUrl\Transformer; use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; -use Shlinkio\Shlink\Core\Tag\Entity\Tag; -use Shlinkio\Shlink\Core\Visit\Model\VisitsSummary; -use function array_map; - -class ShortUrlDataTransformer implements DataTransformerInterface +readonly class ShortUrlDataTransformer implements DataTransformerInterface { - public function __construct(private readonly ShortUrlStringifierInterface $stringifier) + public function __construct(private ShortUrlStringifierInterface $stringifier) { } @@ -24,33 +20,8 @@ class ShortUrlDataTransformer implements DataTransformerInterface public function transform($shortUrl): array // phpcs:ignore { return [ - 'shortCode' => $shortUrl->getShortCode(), 'shortUrl' => $this->stringifier->stringify($shortUrl), - 'longUrl' => $shortUrl->getLongUrl(), - 'dateCreated' => $shortUrl->getDateCreated()->toAtomString(), - 'tags' => array_map(static fn (Tag $tag) => $tag->__toString(), $shortUrl->getTags()->toArray()), - 'meta' => $this->buildMeta($shortUrl), - 'domain' => $shortUrl->getDomain(), - 'title' => $shortUrl->title(), - 'crawlable' => $shortUrl->crawlable(), - 'forwardQuery' => $shortUrl->forwardQuery(), - 'visitsSummary' => VisitsSummary::fromTotalAndNonBots( - $shortUrl->getVisitsCount(), - $shortUrl->nonBotVisitsCount(), - ), - ]; - } - - private function buildMeta(ShortUrl $shortUrl): array - { - $validSince = $shortUrl->getValidSince(); - $validUntil = $shortUrl->getValidUntil(); - $maxVisits = $shortUrl->getMaxVisits(); - - return [ - 'validSince' => $validSince?->toAtomString(), - 'validUntil' => $validUntil?->toAtomString(), - 'maxVisits' => $maxVisits, + ...$shortUrl->toArray(), ]; } } diff --git a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php index 75faa25e..94802cae 100644 --- a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php +++ b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php @@ -132,7 +132,7 @@ class PublishingUpdatesGeneratorTest extends TestCase 'maxVisits' => null, ], 'domain' => null, - 'title' => $shortUrl->title(), + 'title' => 'The title', 'crawlable' => false, 'forwardQuery' => true, 'visitsSummary' => VisitsSummary::fromTotalAndNonBots(0, 0), diff --git a/module/Core/test/ShortUrl/ShortUrlServiceTest.php b/module/Core/test/ShortUrl/ShortUrlServiceTest.php index ae73ba33..669015a0 100644 --- a/module/Core/test/ShortUrl/ShortUrlServiceTest.php +++ b/module/Core/test/ShortUrl/ShortUrlServiceTest.php @@ -70,9 +70,11 @@ class ShortUrlServiceTest extends TestCase ); self::assertSame($shortUrl, $result); - self::assertEquals($shortUrlEdit->validSince, $shortUrl->getValidSince()); - self::assertEquals($shortUrlEdit->validUntil, $shortUrl->getValidUntil()); - self::assertEquals($shortUrlEdit->maxVisits, $shortUrl->getMaxVisits()); + ['validSince' => $since, 'validUntil' => $until, 'maxVisits' => $maxVisits] = $shortUrl->toArray()['meta']; + + self::assertEquals($shortUrlEdit->validSince?->toAtomString(), $since); + self::assertEquals($shortUrlEdit->validUntil?->toAtomString(), $until); + self::assertEquals($shortUrlEdit->maxVisits, $maxVisits); self::assertEquals($shortUrlEdit->longUrl ?? $originalLongUrl, $shortUrl->getLongUrl()); }