Refactored method in ShortUrlsRepository

CHANGELOG.md

@@ -32,6 +32,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 * [#1300](https://github.com/shlinkio/shlink/issues/1300) Changed default ordering for short URLs list, returning always from newest to oldest.
 * [#1299](https://github.com/shlinkio/shlink/issues/1299) Updated to the latest base docker images, based in PHP 8.1.1, and bumped openswoole to v4.9.1.
 * [#1282](https://github.com/shlinkio/shlink/issues/1282) Env vars now have precedence over installer options.
+* [#1328](https://github.com/shlinkio/shlink/issues/1328) Refactored ShortUrlsRepository to use DTOs in methods with too many arguments.
 
 ### Deprecated
 * [#1315](https://github.com/shlinkio/shlink/issues/1315) Deprecated `GET /tags?withStats=true` endpoint. Use `GET /tags/stats` instead.

module/Core/src/Repository/ShortUrlRepository.php

@@ -127,7 +127,7 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU
         return $qb;
     }
 
-    public function findOneWithDomainFallback(string $shortCode, ?string $domain = null): ?ShortUrl
+    public function findOneWithDomainFallback(ShortUrlIdentifier $identifier): ?ShortUrl
     {
         // When ordering DESC, Postgres puts nulls at the beginning while the rest of supported DB engines put them at
         // the bottom
@@ -146,8 +146,8 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU
         $query = $this->getEntityManager()->createQuery($dql);
         $query->setMaxResults(1)
               ->setParameters([
-                  'shortCode' => $shortCode,
-                  'domain' => $domain,
+                  'shortCode' => $identifier->shortCode(),
+                  'domain' => $identifier->domain(),
               ]);
 
         // Since we ordered by domain, we will have first the URL matching provided domain, followed by the one

module/Core/src/Repository/ShortUrlRepositoryInterface.php

@@ -20,8 +20,7 @@ interface ShortUrlRepositoryInterface extends ObjectRepository, EntitySpecificat
 
     public function countList(ShortUrlsCountFiltering $filtering): int;
 
-    // TODO Use ShortUrlIdentifier here
-    public function findOneWithDomainFallback(string $shortCode, ?string $domain = null): ?ShortUrl;
+    public function findOneWithDomainFallback(ShortUrlIdentifier $identifier): ?ShortUrl;
 
     public function findOne(ShortUrlIdentifier $identifier, ?Specification $spec = null): ?ShortUrl;
 

module/Core/src/Service/ShortUrl/ShortUrlResolver.php

@@ -39,7 +39,7 @@ class ShortUrlResolver implements ShortUrlResolverInterface
     {
         /** @var ShortUrlRepository $shortUrlRepo */
         $shortUrlRepo = $this->em->getRepository(ShortUrl::class);
-        $shortUrl = $shortUrlRepo->findOneWithDomainFallback($identifier->shortCode(), $identifier->domain());
+        $shortUrl = $shortUrlRepo->findOneWithDomainFallback($identifier);
         if (! $shortUrl?->isEnabled()) {
             throw ShortUrlNotFoundException::fromNotFound($identifier);
         }

module/Core/test-db/Repository/ShortUrlRepositoryTest.php

@@ -57,25 +57,32 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
 
         $this->getEntityManager()->flush();
 
-        self::assertSame($regularOne, $this->repo->findOneWithDomainFallback($regularOne->getShortCode()));
         self::assertSame($regularOne, $this->repo->findOneWithDomainFallback(
-            $withDomainDuplicatingRegular->getShortCode(),
+            ShortUrlIdentifier::fromShortCodeAndDomain($regularOne->getShortCode()),
+        ));
+        self::assertSame($regularOne, $this->repo->findOneWithDomainFallback(
+            ShortUrlIdentifier::fromShortCodeAndDomain($withDomainDuplicatingRegular->getShortCode()),
         ));
         self::assertSame($withDomain, $this->repo->findOneWithDomainFallback(
-            $withDomain->getShortCode(),
-            'example.com',
+            ShortUrlIdentifier::fromShortCodeAndDomain($withDomain->getShortCode(), 'example.com'),
         ));
         self::assertSame(
             $withDomainDuplicatingRegular,
-            $this->repo->findOneWithDomainFallback($withDomainDuplicatingRegular->getShortCode(), 'doma.in'),
+            $this->repo->findOneWithDomainFallback(
+                ShortUrlIdentifier::fromShortCodeAndDomain($withDomainDuplicatingRegular->getShortCode(), 'doma.in'),
+            ),
         );
-        self::assertSame(
-            $regularOne,
-            $this->repo->findOneWithDomainFallback($withDomainDuplicatingRegular->getShortCode(), 'other-domain.com'),
-        );
-        self::assertNull($this->repo->findOneWithDomainFallback('invalid'));
-        self::assertNull($this->repo->findOneWithDomainFallback($withDomain->getShortCode()));
-        self::assertNull($this->repo->findOneWithDomainFallback($withDomain->getShortCode(), 'other-domain.com'));
+        self::assertSame($regularOne, $this->repo->findOneWithDomainFallback(ShortUrlIdentifier::fromShortCodeAndDomain(
+            $withDomainDuplicatingRegular->getShortCode(),
+            'other-domain.com',
+        )));
+        self::assertNull($this->repo->findOneWithDomainFallback(ShortUrlIdentifier::fromShortCodeAndDomain('invalid')));
+        self::assertNull($this->repo->findOneWithDomainFallback(
+            ShortUrlIdentifier::fromShortCodeAndDomain($withDomain->getShortCode()),
+        ));
+        self::assertNull($this->repo->findOneWithDomainFallback(
+            ShortUrlIdentifier::fromShortCodeAndDomain($withDomain->getShortCode(), 'other-domain.com'),
+        ));
     }
 
     /** @test */

module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php

@@ -86,7 +86,9 @@ class ShortUrlResolverTest extends TestCase
         $shortCode = $shortUrl->getShortCode();
 
         $repo = $this->prophesize(ShortUrlRepositoryInterface::class);
-        $findOneByShortCode = $repo->findOneWithDomainFallback($shortCode, null)->willReturn($shortUrl);
+        $findOneByShortCode = $repo->findOneWithDomainFallback(
+            ShortUrlIdentifier::fromShortCodeAndDomain($shortCode),
+        )->willReturn($shortUrl);
         $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
 
         $result = $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode));
@@ -105,7 +107,9 @@ class ShortUrlResolverTest extends TestCase
         $shortCode = $shortUrl->getShortCode();
 
         $repo = $this->prophesize(ShortUrlRepositoryInterface::class);
-        $findOneByShortCode = $repo->findOneWithDomainFallback($shortCode, null)->willReturn($shortUrl);
+        $findOneByShortCode = $repo->findOneWithDomainFallback(
+            ShortUrlIdentifier::fromShortCodeAndDomain($shortCode),
+        )->willReturn($shortUrl);
         $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
 
         $this->expectException(ShortUrlNotFoundException::class);