Refactored UrlShortener public method to receibe DTOs instead of primitive params

module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php

@@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\CLI\Command\ShortUrl;
 use Cake\Chronos\Chronos;
 use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
 use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException;
+use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
 use Shlinkio\Shlink\Core\Service\UrlShortenerInterface;
 use Shlinkio\Shlink\Core\Util\ShortUrlBuilderTrait;
 use Symfony\Component\Console\Command\Command;
@@ -116,10 +117,12 @@ class GenerateShortUrlCommand extends Command
             $shortCode = $this->urlShortener->urlToShortCode(
                 new Uri($longUrl),
                 $tags,
-                $this->getOptionalDate($input, 'validSince'),
+                ShortUrlMeta::createFromParams(
-                $this->getOptionalDate($input, 'validUntil'),
+                    $this->getOptionalDate($input, 'validSince'),
-                $customSlug,
+                    $this->getOptionalDate($input, 'validUntil'),
-                $maxVisits !== null ? (int) $maxVisits : null
+                    $customSlug,
+                    $maxVisits !== null ? (int) $maxVisits : null
+                )
             )->getShortCode();
             $shortUrl = $this->buildShortUrl($this->domainConfig, $shortCode);
 

module/Core/src/Model/CreateShortUrlData.php

@@ -17,11 +17,11 @@ final class CreateShortUrlData
     public function __construct(
         UriInterface $longUrl,
         array $tags = [],
-        ShortUrlMeta $meta = null
+        ?ShortUrlMeta $meta = null
     ) {
         $this->longUrl = $longUrl;
         $this->tags = $tags;
-        $this->meta = $meta ?? ShortUrlMeta::createFromParams();
+        $this->meta = $meta ?? ShortUrlMeta::createEmpty();
     }
 
     /**

module/Core/src/Model/ShortUrlMeta.php

@@ -138,4 +138,12 @@ final class ShortUrlMeta
     {
         return $this->maxVisits !== null;
     }
+
+    public function withCustomSlug(string $customSlug): self
+    {
+        $clone = clone $this;
+        $clone->customSlug = $customSlug;
+
+        return $clone;
+    }
 }

module/Core/src/Service/UrlShortener.php

@@ -3,7 +3,6 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Core\Service;
 
-use Cake\Chronos\Chronos;
 use Cocur\Slugify\SlugifyInterface;
 use Doctrine\ORM\EntityManagerInterface;
 use GuzzleHttp\ClientInterface;
@@ -53,41 +52,35 @@ class UrlShortener implements UrlShortenerInterface
     }
 
     /**
+     * @param string[] $tags
      * @throws NonUniqueSlugException
      * @throws InvalidUrlException
      * @throws RuntimeException
      */
-    public function urlToShortCode(
+    public function urlToShortCode(UriInterface $url, array $tags, ShortUrlMeta $meta): ShortUrl
-        UriInterface $url,
+    {
-        array $tags = [],
-        ?Chronos $validSince = null,
-        ?Chronos $validUntil = null,
-        ?string $customSlug = null,
-        ?int $maxVisits = null
-    ): ShortUrl {
         // If the URL validation is enabled, check that the URL actually exists
         if ($this->options->isUrlValidationEnabled()) {
             $this->checkUrlExists($url);
         }
-        $customSlug = $this->processCustomSlug($customSlug);
+        $meta = $this->processCustomSlug($meta);
 
         // Transactionally insert the short url, then generate the short code and finally update the short code
         try {
             $this->em->beginTransaction();
 
             // First, create the short URL with an empty short code
-            $shortUrl = new ShortUrl(
+            $shortUrl = new ShortUrl((string) $url, $meta);
-                (string) $url,
-                ShortUrlMeta::createFromParams($validSince, $validUntil, null, $maxVisits)
-            );
             $this->em->persist($shortUrl);
             $this->em->flush();
 
-            // Generate the short code and persist it
+            // Generate the short code and persist it if no custom slug was provided
-            // TODO Somehow provide the logic to calculate the shortCode to avoid the need of a setter
+            if (! $meta->hasCustomSlug()) {
-            $shortCode = $customSlug ?? $this->convertAutoincrementIdToShortCode((float) $shortUrl->getId());
+                // TODO Somehow provide the logic to calculate the shortCode to avoid the need of a setter
-            $shortUrl->setShortCode($shortCode)
+                $shortCode = $this->convertAutoincrementIdToShortCode((float) $shortUrl->getId());
-                     ->setTags($this->tagNamesToEntities($this->em, $tags));
+                $shortUrl->setShortCode($shortCode);
+            }
+            $shortUrl->setTags($this->tagNamesToEntities($this->em, $tags));
             $this->em->flush();
 
             $this->em->commit();
@@ -130,25 +123,27 @@ class UrlShortener implements UrlShortenerInterface
         return $chars[(int) $id] . $code;
     }
 
-    private function processCustomSlug(?string $customSlug): ?string
+    private function processCustomSlug(ShortUrlMeta $meta): ?ShortUrlMeta
     {
-        if ($customSlug === null) {
+        if (! $meta->hasCustomSlug()) {
-            return null;
+            return $meta;
         }
 
-        // If a custom slug was provided, make sure it's unique
+        // FIXME If the slug was generated while filtering the value originally, we would not need an immutable setter
-        $customSlug = $this->slugger->slugify($customSlug);
+        //       in ShortUrlMeta
-        $shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy(['shortCode' => $customSlug]);
+        $customSlug = $this->slugger->slugify($meta->getCustomSlug());
-        if ($shortUrl !== null) {
+
+        /** @var ShortUrlRepository $repo */
+        $repo = $this->em->getRepository(ShortUrl::class);
+        $shortUrlsCount = $repo->count(['shortCode' => $customSlug]);
+        if ($shortUrlsCount > 0) {
             throw NonUniqueSlugException::fromSlug($customSlug);
         }
 
-        return $customSlug;
+        return $meta->withCustomSlug($customSlug);
     }
 
     /**
-     * Tries to find the mapped URL for provided short code. Returns null if not found
-     *
      * @throws InvalidShortCodeException
      * @throws EntityDoesNotExistException
      */

module/Core/src/Service/UrlShortenerInterface.php

@@ -3,7 +3,6 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Core\Service;
 
-use Cake\Chronos\Chronos;
 use Psr\Http\Message\UriInterface;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
@@ -11,26 +10,19 @@ use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
 use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
 use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException;
 use Shlinkio\Shlink\Core\Exception\RuntimeException;
+use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
 
 interface UrlShortenerInterface
 {
     /**
+     * @param string[] $tags
      * @throws NonUniqueSlugException
      * @throws InvalidUrlException
      * @throws RuntimeException
      */
-    public function urlToShortCode(
+    public function urlToShortCode(UriInterface $url, array $tags, ShortUrlMeta $meta): ShortUrl;
-        UriInterface $url,
-        array $tags = [],
-        ?Chronos $validSince = null,
-        ?Chronos $validUntil = null,
-        ?string $customSlug = null,
-        ?int $maxVisits = null
-    ): ShortUrl;
 
     /**
-     * Tries to find the mapped URL for provided short code. Returns null if not found
-     *
      * @throws InvalidShortCodeException
      * @throws EntityDoesNotExistException
      */

module/Core/test/Service/UrlShortenerTest.php

@@ -4,7 +4,6 @@ declare(strict_types=1);
 namespace ShlinkioTest\Shlink\Core\Service;
 
 use Cocur\Slugify\SlugifyInterface;
-use Doctrine\Common\Persistence\ObjectRepository;
 use Doctrine\DBAL\Connection;
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\ORMException;
@@ -17,7 +16,9 @@ use Prophecy\Prophecy\MethodProphecy;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException;
+use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
 use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
+use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface;
 use Shlinkio\Shlink\Core\Service\UrlShortener;
 use Zend\Diactoros\Uri;
@@ -49,8 +50,8 @@ class UrlShortenerTest extends TestCase
             $shortUrl = $arguments[0];
             $shortUrl->setId('10');
         });
-        $repo = $this->prophesize(ObjectRepository::class);
+        $repo = $this->prophesize(ShortUrlRepository::class);
-        $repo->findOneBy(Argument::any())->willReturn(null);
+        $repo->count(Argument::any())->willReturn(0);
         $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
 
         $this->slugger = $this->prophesize(SlugifyInterface::class);
@@ -74,7 +75,11 @@ class UrlShortenerTest extends TestCase
     public function urlIsProperlyShortened()
     {
         // 10 -> 12C1c
-        $shortUrl = $this->urlShortener->urlToShortCode(new Uri('http://foobar.com/12345/hello?foo=bar'));
+        $shortUrl = $this->urlShortener->urlToShortCode(
+            new Uri('http://foobar.com/12345/hello?foo=bar'),
+            [],
+            ShortUrlMeta::createEmpty()
+        );
         $this->assertEquals('12C1c', $shortUrl->getShortCode());
     }
 
@@ -91,7 +96,11 @@ class UrlShortenerTest extends TestCase
         $this->em->close()->shouldBeCalledOnce();
 
         $this->em->flush()->willThrow(new ORMException());
-        $this->urlShortener->urlToShortCode(new Uri('http://foobar.com/12345/hello?foo=bar'));
+        $this->urlShortener->urlToShortCode(
+            new Uri('http://foobar.com/12345/hello?foo=bar'),
+            [],
+            ShortUrlMeta::createEmpty()
+        );
     }
 
     /**
@@ -105,7 +114,11 @@ class UrlShortenerTest extends TestCase
         $this->httpClient->request(Argument::cetera())->willThrow(
             new ClientException('', $this->prophesize(Request::class)->reveal())
         );
-        $this->urlShortener->urlToShortCode(new Uri('http://foobar.com/12345/hello?foo=bar'));
+        $this->urlShortener->urlToShortCode(
+            new Uri('http://foobar.com/12345/hello?foo=bar'),
+            [],
+            ShortUrlMeta::createEmpty()
+        );
     }
 
     /**
@@ -119,9 +132,7 @@ class UrlShortenerTest extends TestCase
         $this->urlShortener->urlToShortCode(
             new Uri('http://foobar.com/12345/hello?foo=bar'),
             [],
-            null,
+            ShortUrlMeta::createFromRawData(['customSlug' => 'custom-slug'])
-            null,
-            'custom-slug'
         );
 
         $slugify->shouldHaveBeenCalledOnce();
@@ -135,24 +146,21 @@ class UrlShortenerTest extends TestCase
         /** @var MethodProphecy $slugify */
         $slugify = $this->slugger->slugify('custom-slug')->willReturnArgument();
 
-        $repo = $this->prophesize(ShortUrlRepositoryInterface::class);
+        $repo = $this->prophesize(ShortUrlRepository::class);
-        /** @var MethodProphecy $findBySlug */
+        $countBySlug = $repo->count(['shortCode' => 'custom-slug'])->willReturn(1);
-        $findBySlug = $repo->findOneBy(['shortCode' => 'custom-slug'])->willReturn(new ShortUrl(''));
         $repo->findOneBy(Argument::cetera())->willReturn(null);
         /** @var MethodProphecy $getRepo */
         $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
 
         $slugify->shouldBeCalledOnce();
-        $findBySlug->shouldBeCalledOnce();
+        $countBySlug->shouldBeCalledOnce();
         $getRepo->shouldBeCalled();
         $this->expectException(NonUniqueSlugException::class);
 
         $this->urlShortener->urlToShortCode(
             new Uri('http://foobar.com/12345/hello?foo=bar'),
             [],
-            null,
+            ShortUrlMeta::createFromRawData(['customSlug' => 'custom-slug'])
-            null,
-            'custom-slug'
         );
     }
 

module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php

@@ -53,17 +53,9 @@ abstract class AbstractCreateShortUrlAction extends AbstractRestAction
 
         $longUrl = $shortUrlData->getLongUrl();
         $shortUrlMeta = $shortUrlData->getMeta();
-        $customSlug = $shortUrlMeta->getCustomSlug();
 
         try {
-            $shortUrl = $this->urlShortener->urlToShortCode(
+            $shortUrl = $this->urlShortener->urlToShortCode($longUrl, $shortUrlData->getTags(), $shortUrlMeta);
-                $longUrl,
-                $shortUrlData->getTags(),
-                $shortUrlMeta->getValidSince(),
-                $shortUrlMeta->getValidUntil(),
-                $customSlug,
-                $shortUrlMeta->getMaxVisits()
-            );
             $transformer = new ShortUrlDataTransformer($this->domainConfig);
 
             return new JsonResponse($transformer->transform($shortUrl));
@@ -74,6 +66,7 @@ abstract class AbstractCreateShortUrlAction extends AbstractRestAction
                 'message' => sprintf('Provided URL %s is invalid. Try with a different one.', $longUrl),
             ], self::STATUS_BAD_REQUEST);
         } catch (NonUniqueSlugException $e) {
+            $customSlug = $shortUrlMeta->getCustomSlug();
             $this->logger->warning('Provided non-unique slug. {e}', ['e' => $e]);
             return new JsonResponse([
                 'error' => RestUtils::getRestErrorCodeFromException($e),

module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php

@@ -10,6 +10,7 @@ use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
 use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException;
+use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
 use Shlinkio\Shlink\Core\Service\UrlShortener;
 use Shlinkio\Shlink\Rest\Action\ShortUrl\CreateShortUrlAction;
 use Shlinkio\Shlink\Rest\Util\RestUtils;
@@ -86,9 +87,7 @@ class CreateShortUrlActionTest extends TestCase
         $this->urlShortener->urlToShortCode(
             Argument::type(Uri::class),
             Argument::type('array'),
-            null,
+            ShortUrlMeta::createFromRawData(['customSlug' => 'foo']),
-            null,
-            'foo',
             Argument::cetera()
         )->willThrow(NonUniqueSlugException::class)->shouldBeCalledOnce();
 

module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php

@@ -9,6 +9,7 @@ use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
 use Psr\Http\Message\UriInterface;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
+use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
 use Shlinkio\Shlink\Core\Service\UrlShortenerInterface;
 use Shlinkio\Shlink\Rest\Action\ShortUrl\SingleStepCreateShortUrlAction;
 use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface;
@@ -91,10 +92,7 @@ class SingleStepCreateShortUrlActionTest extends TestCase
                 return $argument;
             }),
             [],
-            null,
+            ShortUrlMeta::createEmpty()
-            null,
-            null,
-            null
         )->willReturn(new ShortUrl(''));
 
         $resp = $this->action->handle($request);