Merge pull request #2069 from acelaya-forks/feature/short-url-simplification

Move logic to serialize ShortUrls to entity itself

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 $this->deleteShortUrlsOptions->checkVisitsThreshold && $shortUrl->reachedVisits(
-            return false;
+            $this->deleteShortUrlsOptions->visitsThreshold,
-        }
+        );
-
-        return $shortUrl->getVisitsCount() >= $this->deleteShortUrlsOptions->visitsThreshold;
     }
 }

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;
     }
 
-    /**
+    public function reachedVisits(int $visitsAmount): bool
-     * @return Collection<int, Tag>
-     */
-    public function getTags(): Collection
     {
-        return $this->tags;
+        return count($this->visits) >= $visitsAmount;
-    }
-
-    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));
     }
 
     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)),
+                )),
+            ),
+        ];
+    }
 }

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;
+readonly class ShortUrlDataTransformer implements DataTransformerInterface
-
-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(),
+            ...$shortUrl->toArray(),
-            '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,
         ];
     }
 }

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),

module/Core/test/ShortUrl/ShortUrlServiceTest.php

@@ -70,9 +70,11 @@ class ShortUrlServiceTest extends TestCase
         );
 
         self::assertSame($shortUrl, $result);
-        self::assertEquals($shortUrlEdit->validSince, $shortUrl->getValidSince());
+        ['validSince' => $since, 'validUntil' => $until, 'maxVisits' => $maxVisits] = $shortUrl->toArray()['meta'];
-        self::assertEquals($shortUrlEdit->validUntil, $shortUrl->getValidUntil());
+
-        self::assertEquals($shortUrlEdit->maxVisits, $shortUrl->getMaxVisits());
+        self::assertEquals($shortUrlEdit->validSince?->toAtomString(), $since);
+        self::assertEquals($shortUrlEdit->validUntil?->toAtomString(), $until);
+        self::assertEquals($shortUrlEdit->maxVisits, $maxVisits);
         self::assertEquals($shortUrlEdit->longUrl ?? $originalLongUrl, $shortUrl->getLongUrl());
     }