Merge pull request #2242 from acelaya-forks/feature/avoid-n-plus-1-do…

…main Feature/avoid n plus 1 domain
shlinkio · Oct 30, 2024 · 5b98c44 · 5b98c44
2 parents 0387a8c + ee32d1a
commit 5b98c44
Show file tree

Hide file tree

Showing 13 changed files with 48 additions and 39 deletions.
diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php
@@ -13,7 +13,7 @@
 use Shlinkio\Shlink\Core\Domain\Entity\Domain;
 use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlsParams;
-use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary;
+use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithDeps;
 use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode;
 use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlsParamsInputFilter;
 use Shlinkio\Shlink\Core\ShortUrl\ShortUrlListServiceInterface;
@@ -186,7 +186,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 
     /**
      * @param array<string, callable(array $serializedShortUrl, ShortUrl $shortUrl): ?string> $columnsMap
-     * @return Paginator<ShortUrlWithVisitsSummary>
+     * @return Paginator<ShortUrlWithDeps>
      */
     private function renderPage(
         OutputInterface $output,
@@ -196,7 +196,7 @@ private function renderPage(
     ): Paginator {
         $shortUrls = $this->shortUrlService->listShortUrls($params);
 
-        $rows = map([...$shortUrls], function (ShortUrlWithVisitsSummary $shortUrl) use ($columnsMap) {
+        $rows = map([...$shortUrls], function (ShortUrlWithDeps $shortUrl) use ($columnsMap) {
             $serializedShortUrl = $this->transformer->transform($shortUrl);
             return map($columnsMap, fn (callable $call) => $call($serializedShortUrl, $shortUrl->shortUrl));
         });

diff --git a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php
@@ -16,7 +16,7 @@
 use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier;
 use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation;
 use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlsParams;
-use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary;
+use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithDeps;
 use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode;
 use Shlinkio\Shlink\Core\ShortUrl\ShortUrlListServiceInterface;
 use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformer;
@@ -48,7 +48,7 @@ public function loadingMorePagesCallsListMoreTimes(): void
         // The paginator will return more than one page
         $data = [];
         for ($i = 0; $i < 50; $i++) {
-            $data[] = ShortUrlWithVisitsSummary::fromShortUrl(ShortUrl::withLongUrl('https://url_' . $i));
+            $data[] = ShortUrlWithDeps::fromShortUrl(ShortUrl::withLongUrl('https://url_' . $i));
         }
 
         $this->shortUrlService->expects($this->exactly(3))->method('listShortUrls')->withAnyParameters()
@@ -70,7 +70,7 @@ public function havingMorePagesButAnsweringNoCallsListJustOnce(): void
         // The paginator will return more than one page
         $data = [];
         for ($i = 0; $i < 30; $i++) {
-            $data[] = ShortUrlWithVisitsSummary::fromShortUrl(ShortUrl::withLongUrl('https://url_' . $i));
+            $data[] = ShortUrlWithDeps::fromShortUrl(ShortUrl::withLongUrl('https://url_' . $i));
         }
 
         $this->shortUrlService->expects($this->once())->method('listShortUrls')->with(
@@ -112,7 +112,7 @@ public function provideOptionalFlagsMakesNewColumnsToBeIncluded(
         $this->shortUrlService->expects($this->once())->method('listShortUrls')->with(
             ShortUrlsParams::empty(),
         )->willReturn(new Paginator(new ArrayAdapter([
-            ShortUrlWithVisitsSummary::fromShortUrl(
+            ShortUrlWithDeps::fromShortUrl(
                 ShortUrl::create(ShortUrlCreation::fromRawData([
                     'longUrl' => 'https://foo.com',
                     'tags' => ['foo', 'bar', 'baz'],

diff --git a/module/Core/src/ShortUrl/Helper/ShortUrlStringifier.php b/module/Core/src/ShortUrl/Helper/ShortUrlStringifier.php
@@ -7,6 +7,7 @@
 use Laminas\Diactoros\Uri;
 use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions;
 use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
+use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier;
 
 use function sprintf;
 
@@ -18,19 +19,20 @@ public function __construct(
     ) {
     }
 
-    public function stringify(ShortUrl $shortUrl): string
+    public function stringify(ShortUrl|ShortUrlIdentifier $shortUrl): string
     {
+        $shortUrlIdentifier = $shortUrl instanceof ShortUrl ? ShortUrlIdentifier::fromShortUrl($shortUrl) : $shortUrl;
         $uriWithoutShortCode = (new Uri())->withScheme($this->urlShortenerOptions->schema)
-                                          ->withHost($this->resolveDomain($shortUrl))
+                                          ->withHost($this->resolveDomain($shortUrlIdentifier))
                                           ->withPath($this->basePath)
                                           ->__toString();
 
         // The short code needs to be appended to avoid it from being URL-encoded
-        return sprintf('%s/%s', $uriWithoutShortCode, $shortUrl->getShortCode());
+        return sprintf('%s/%s', $uriWithoutShortCode, $shortUrlIdentifier->shortCode);
     }
 
-    private function resolveDomain(ShortUrl $shortUrl): string
+    private function resolveDomain(ShortUrlIdentifier $shortUrlIdentifier): string
     {
-        return $shortUrl->getDomain()?->authority ?? $this->urlShortenerOptions->defaultDomain;
+        return $shortUrlIdentifier->domain ?? $this->urlShortenerOptions->defaultDomain;
     }
 }
diff --git a/module/Core/src/ShortUrl/Helper/ShortUrlStringifierInterface.php b/module/Core/src/ShortUrl/Helper/ShortUrlStringifierInterface.php
@@ -5,8 +5,9 @@
 namespace Shlinkio\Shlink\Core\ShortUrl\Helper;
 
 use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
+use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier;
 
 interface ShortUrlStringifierInterface
 {
-    public function stringify(ShortUrl $shortUrl): string;
+    public function stringify(ShortUrl|ShortUrlIdentifier $shortUrl): string;
 }
diff --git a/module/Core/src/ShortUrl/Model/ShortUrlIdentifier.php b/module/Core/src/ShortUrl/Model/ShortUrlIdentifier.php
@@ -33,10 +33,8 @@ public static function fromRedirectRequest(ServerRequestInterface $request): sel
 
     public static function fromShortUrl(ShortUrl $shortUrl): self
     {
-        $domain = $shortUrl->getDomain();
-        $domainAuthority = $domain?->authority;
-
-        return new self($shortUrl->getShortCode(), $domainAuthority);
+        $domain = $shortUrl->getDomain()?->authority;
+        return new self($shortUrl->getShortCode(), $domain);
     }
 
     public static function fromShortCodeAndDomain(string $shortCode, string|null $domain = null): self

diff --git a/...rtUrl/Model/ShortUrlWithVisitsSummary.php → ...e/src/ShortUrl/Model/ShortUrlWithDeps.php b/...rtUrl/Model/ShortUrlWithVisitsSummary.php → ...e/src/ShortUrl/Model/ShortUrlWithDeps.php
@@ -7,12 +7,12 @@
 use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Visit\Model\VisitsSummary;
 
-final readonly class ShortUrlWithVisitsSummary
+final readonly class ShortUrlWithDeps
 {
     private function __construct(
         public ShortUrl $shortUrl,
+        private string|null $authority,
         private VisitsSummary|null $visitsSummary = null,
-        private string|null $authority = null,
     ) {
     }
 
@@ -23,17 +23,22 @@ public static function fromArray(array $data): self
     {
         return new self(
             shortUrl: $data['shortUrl'],
+            authority: $data['authority'] ?? null,
             visitsSummary: VisitsSummary::fromTotalAndNonBots(
                 total: (int) $data['visits'],
                 nonBots: (int) $data['nonBotVisits'],
             ),
-            authority: $data['authority'] ?? null,
         );
     }
 
     public static function fromShortUrl(ShortUrl $shortUrl): self
     {
-        return new self($shortUrl);
+        return new self($shortUrl, authority: $shortUrl->getDomain()?->authority);
+    }
+
+    public function toIdentifier(): ShortUrlIdentifier
+    {
+        return ShortUrlIdentifier::fromShortCodeAndDomain($this->shortUrl->getShortCode(), $this->authority);
     }
 
     public function toArray(): array

diff --git a/module/Core/src/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapter.php b/module/Core/src/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapter.php
@@ -6,13 +6,13 @@
 
 use Pagerfanta\Adapter\AdapterInterface;
 use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlsParams;
-use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary;
+use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithDeps;
 use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsCountFiltering;
 use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsListFiltering;
 use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlListRepositoryInterface;
 use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
-/** @implements AdapterInterface<ShortUrlWithVisitsSummary> */
+/** @implements AdapterInterface<ShortUrlWithDeps> */
 readonly class ShortUrlRepositoryAdapter implements AdapterInterface
 {
     public function __construct(

diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php
@@ -12,7 +12,7 @@
 use Shlinkio\Shlink\Core\Domain\Entity\Domain;
 use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\ShortUrl\Model\OrderableField;
-use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary;
+use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithDeps;
 use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode;
 use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsCountFiltering;
 use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsListFiltering;
@@ -25,7 +25,7 @@
 class ShortUrlListRepository extends EntitySpecificationRepository implements ShortUrlListRepositoryInterface
 {
     /**
-     * @return ShortUrlWithVisitsSummary[]
+     * @return ShortUrlWithDeps[]
      */
     public function findList(ShortUrlsListFiltering $filtering): array
     {
@@ -59,7 +59,7 @@ public function findList(ShortUrlsListFiltering $filtering): array
 
         /** @var array{shortUrl: ShortUrl, visits: string, nonBotVisits: string, authority: string|null}[] $result */
         $result = $qb->getQuery()->getResult();
-        return map($result, static fn (array $s) => ShortUrlWithVisitsSummary::fromArray($s));
+        return map($result, static fn (array $s) => ShortUrlWithDeps::fromArray($s));
     }
 
     private function processOrderByForList(QueryBuilder $qb, ShortUrlsListFiltering $filtering): void

diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlListRepositoryInterface.php b/module/Core/src/ShortUrl/Repository/ShortUrlListRepositoryInterface.php
@@ -4,14 +4,14 @@
 
 namespace Shlinkio\Shlink\Core\ShortUrl\Repository;
 
-use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary;
+use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithDeps;
 use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsCountFiltering;
 use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsListFiltering;
 
 interface ShortUrlListRepositoryInterface
 {
     /**
-     * @return ShortUrlWithVisitsSummary[]
+     * @return ShortUrlWithDeps[]
      */
     public function findList(ShortUrlsListFiltering $filtering): array;
 

diff --git a/module/Core/src/ShortUrl/ShortUrlListServiceInterface.php b/module/Core/src/ShortUrl/ShortUrlListServiceInterface.php
@@ -6,13 +6,13 @@
 
 use Shlinkio\Shlink\Common\Paginator\Paginator;
 use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlsParams;
-use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary;
+use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithDeps;
 use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
 interface ShortUrlListServiceInterface
 {
     /**
-     * @return Paginator<ShortUrlWithVisitsSummary>
+     * @return Paginator<ShortUrlWithDeps>
      */
     public function listShortUrls(ShortUrlsParams $params, ApiKey|null $apiKey = null): Paginator;
 }
diff --git a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php
@@ -6,20 +6,23 @@
 
 use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface;
-use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary;
+use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier;
+use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithDeps;
 
 readonly class ShortUrlDataTransformer implements ShortUrlDataTransformerInterface
 {
     public function __construct(private ShortUrlStringifierInterface $stringifier)
     {
     }
 
-    public function transform(ShortUrlWithVisitsSummary|ShortUrl $data): array
+    public function transform(ShortUrlWithDeps|ShortUrl $shortUrl): array
     {
-        $shortUrl = $data instanceof ShortUrlWithVisitsSummary ? $data->shortUrl : $data;
+        $shortUrlIdentifier = $shortUrl instanceof ShortUrl
+            ? ShortUrlIdentifier::fromShortUrl($shortUrl)
+            : $shortUrl->toIdentifier();
         return [
-            'shortUrl' => $this->stringifier->stringify($shortUrl),
-            ...$data->toArray(),
+            'shortUrl' => $this->stringifier->stringify($shortUrlIdentifier),
+            ...$shortUrl->toArray(),
         ];
     }
 }
diff --git a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformerInterface.php b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformerInterface.php
@@ -5,9 +5,9 @@
 namespace Shlinkio\Shlink\Core\ShortUrl\Transformer;
 
 use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
-use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary;
+use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithDeps;
 
 interface ShortUrlDataTransformerInterface
 {
-    public function transform(ShortUrlWithVisitsSummary|ShortUrl $data): array;
+    public function transform(ShortUrlWithDeps|ShortUrl $shortUrl): array;
 }
diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php
@@ -14,7 +14,7 @@
 use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\ShortUrl\Model\OrderableField;
 use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation;
-use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary;
+use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithDeps;
 use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode;
 use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsCountFiltering;
 use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsListFiltering;
@@ -97,7 +97,7 @@ public function findListProperlyFiltersResult(): void
         $result = $this->repo->findList(new ShortUrlsListFiltering(searchTerm: 'bar'));
         self::assertCount(2, $result);
         self::assertEquals(2, $this->repo->countList(new ShortUrlsCountFiltering('bar')));
-        self::assertContains($foo, map($result, fn (ShortUrlWithVisitsSummary $s) => $s->shortUrl));
+        self::assertContains($foo, map($result, fn (ShortUrlWithDeps $s) => $s->shortUrl));
 
         $result = $this->repo->findList(new ShortUrlsListFiltering());
         self::assertCount(3, $result);