Added helper service to avoid code duplication when resolving short URLs titles

module/Core/config/dependencies.config.php

@@ -44,6 +44,7 @@ return [
 
             ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class => ConfigAbstractFactory::class,
             ShortUrl\Helper\ShortUrlStringifier::class => ConfigAbstractFactory::class,
+            ShortUrl\Helper\ShortUrlTitleResolutionHelper::class => ConfigAbstractFactory::class,
             ShortUrl\Transformer\ShortUrlDataTransformer::class => ConfigAbstractFactory::class,
 
             Mercure\MercureUpdatesGenerator::class => ConfigAbstractFactory::class,
@@ -69,7 +70,7 @@ return [
         Options\UrlShortenerOptions::class => ['config.url_shortener'],
 
         Service\UrlShortener::class => [
-            Util\UrlValidator::class,
+            ShortUrl\Helper\ShortUrlTitleResolutionHelper::class,
             'em',
             ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class,
             Service\ShortUrl\ShortCodeHelper::class,
@@ -82,7 +83,7 @@ return [
         Service\ShortUrlService::class => [
             'em',
             Service\ShortUrl\ShortUrlResolver::class,
-            Util\UrlValidator::class,
+            ShortUrl\Helper\ShortUrlTitleResolutionHelper::class,
             ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class,
         ],
         Visit\VisitLocator::class => ['em'],
@@ -122,6 +123,7 @@ return [
 
         ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class => ['em'],
         ShortUrl\Helper\ShortUrlStringifier::class => ['config.url_shortener.domain', 'config.router.base_path'],
+        ShortUrl\Helper\ShortUrlTitleResolutionHelper::class => [Util\UrlValidator::class],
         ShortUrl\Transformer\ShortUrlDataTransformer::class => [ShortUrl\Helper\ShortUrlStringifier::class],
 
         Mercure\MercureUpdatesGenerator::class => [ShortUrl\Transformer\ShortUrlDataTransformer::class],

module/Core/src/Model/ShortUrlEdit.php

@@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\Model;
 
 use Cake\Chronos\Chronos;
 use Shlinkio\Shlink\Core\Exception\ValidationException;
+use Shlinkio\Shlink\Core\ShortUrl\Helper\TitleResolutionModelInterface;
 use Shlinkio\Shlink\Core\Validation\ShortUrlInputFilter;
 
 use function array_key_exists;
@@ -13,7 +14,7 @@ use function Shlinkio\Shlink\Core\getOptionalBoolFromInputFilter;
 use function Shlinkio\Shlink\Core\getOptionalIntFromInputFilter;
 use function Shlinkio\Shlink\Core\parseDateField;
 
-final class ShortUrlEdit
+final class ShortUrlEdit implements TitleResolutionModelInterface
 {
     private bool $longUrlPropWasProvided = false;
     private ?string $longUrl = null;
@@ -75,6 +76,11 @@ final class ShortUrlEdit
         return $this->longUrl;
     }
 
+    public function getLongUrl(): string
+    {
+        return $this->longUrl() ?? '';
+    }
+
     public function longUrlWasProvided(): bool
     {
         return $this->longUrlPropWasProvided && $this->longUrl !== null;
@@ -133,6 +139,11 @@ final class ShortUrlEdit
         return $this->titlePropWasProvided;
     }
 
+    public function hasTitle(): bool
+    {
+        return $this->titleWasProvided();
+    }
+
     public function titleWasAutoResolved(): bool
     {
         return $this->titleWasAutoResolved;

module/Core/src/Model/ShortUrlMeta.php

@@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\Model;
 
 use Cake\Chronos\Chronos;
 use Shlinkio\Shlink\Core\Exception\ValidationException;
+use Shlinkio\Shlink\Core\ShortUrl\Helper\TitleResolutionModelInterface;
 use Shlinkio\Shlink\Core\Validation\ShortUrlInputFilter;
 use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
@@ -15,7 +16,7 @@ use function Shlinkio\Shlink\Core\parseDateField;
 
 use const Shlinkio\Shlink\Core\DEFAULT_SHORT_CODES_LENGTH;
 
-final class ShortUrlMeta
+final class ShortUrlMeta implements TitleResolutionModelInterface
 {
     private string $longUrl;
     private ?Chronos $validSince = null;

module/Core/src/Service/ShortUrlService.php

@@ -15,26 +15,26 @@ use Shlinkio\Shlink\Core\Model\ShortUrlsParams;
 use Shlinkio\Shlink\Core\Paginator\Adapter\ShortUrlRepositoryAdapter;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
 use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface;
+use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlTitleResolutionHelperInterface;
 use Shlinkio\Shlink\Core\ShortUrl\Resolver\ShortUrlRelationResolverInterface;
-use Shlinkio\Shlink\Core\Util\UrlValidatorInterface;
 use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
 class ShortUrlService implements ShortUrlServiceInterface
 {
     private ORM\EntityManagerInterface $em;
     private ShortUrlResolverInterface $urlResolver;
-    private UrlValidatorInterface $urlValidator;
+    private ShortUrlTitleResolutionHelperInterface $titleResolutionHelper;
     private ShortUrlRelationResolverInterface $relationResolver;
 
     public function __construct(
         ORM\EntityManagerInterface $em,
         ShortUrlResolverInterface $urlResolver,
-        UrlValidatorInterface $urlValidator,
+        ShortUrlTitleResolutionHelperInterface $titleResolutionHelper,
         ShortUrlRelationResolverInterface $relationResolver
     ) {
         $this->em = $em;
         $this->urlResolver = $urlResolver;
-        $this->urlValidator = $urlValidator;
+        $this->titleResolutionHelper = $titleResolutionHelper;
         $this->relationResolver = $relationResolver;
     }
 
@@ -62,7 +62,8 @@ class ShortUrlService implements ShortUrlServiceInterface
         ?ApiKey $apiKey = null
     ): ShortUrl {
         if ($shortUrlEdit->longUrlWasProvided()) {
-            $shortUrlEdit = $this->processTitleAndValidateUrl($shortUrlEdit);
+            /** @var ShortUrlEdit $shortUrlEdit */
+            $shortUrlEdit = $this->titleResolutionHelper->processTitleAndValidateUrl($shortUrlEdit);
         }
 
         $shortUrl = $this->urlResolver->resolveShortUrl($identifier, $apiKey);
@@ -72,15 +73,4 @@ class ShortUrlService implements ShortUrlServiceInterface
 
         return $shortUrl;
     }
-
-    private function processTitleAndValidateUrl(ShortUrlEdit $shortUrlEdit): ShortUrlEdit
-    {
-        if ($shortUrlEdit->titleWasProvided()) {
-            $this->urlValidator->validateUrl($shortUrlEdit->longUrl(), $shortUrlEdit->doValidateUrl());
-            return $shortUrlEdit;
-        }
-
-        $title = $this->urlValidator->validateUrlWithTitle($shortUrlEdit->longUrl(), $shortUrlEdit->doValidateUrl());
-        return $title === null ? $shortUrlEdit : $shortUrlEdit->withResolvedTitle($title);
-    }
 }

module/Core/src/Service/UrlShortener.php

@@ -11,23 +11,23 @@ use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException;
 use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface;
 use Shlinkio\Shlink\Core\Service\ShortUrl\ShortCodeHelperInterface;
+use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlTitleResolutionHelperInterface;
 use Shlinkio\Shlink\Core\ShortUrl\Resolver\ShortUrlRelationResolverInterface;
-use Shlinkio\Shlink\Core\Util\UrlValidatorInterface;
 
 class UrlShortener implements UrlShortenerInterface
 {
     private EntityManagerInterface $em;
-    private UrlValidatorInterface $urlValidator;
+    private ShortUrlTitleResolutionHelperInterface $titleResolutionHelper;
     private ShortUrlRelationResolverInterface $relationResolver;
     private ShortCodeHelperInterface $shortCodeHelper;
 
     public function __construct(
-        UrlValidatorInterface $urlValidator,
+        ShortUrlTitleResolutionHelperInterface $titleResolutionHelper,
         EntityManagerInterface $em,
         ShortUrlRelationResolverInterface $relationResolver,
         ShortCodeHelperInterface $shortCodeHelper
     ) {
-        $this->urlValidator = $urlValidator;
+        $this->titleResolutionHelper = $titleResolutionHelper;
         $this->em = $em;
         $this->relationResolver = $relationResolver;
         $this->shortCodeHelper = $shortCodeHelper;
@@ -45,7 +45,8 @@ class UrlShortener implements UrlShortenerInterface
             return $existingShortUrl;
         }
 
-        $meta = $this->processTitleAndValidateUrl($meta);
+        /** @var ShortUrlMeta $meta */
+        $meta = $this->titleResolutionHelper->processTitleAndValidateUrl($meta);
 
         return $this->em->transactional(function () use ($meta) {
             $shortUrl = ShortUrl::fromMeta($meta, $this->relationResolver);
@@ -82,15 +83,4 @@ class UrlShortener implements UrlShortenerInterface
             throw NonUniqueSlugException::fromSlug($shortUrlToBeCreated->getShortCode(), $domainAuthority);
         }
     }
-
-    private function processTitleAndValidateUrl(ShortUrlMeta $meta): ShortUrlMeta
-    {
-        if ($meta->hasTitle()) {
-            $this->urlValidator->validateUrl($meta->getLongUrl(), $meta->doValidateUrl());
-            return $meta;
-        }
-
-        $title = $this->urlValidator->validateUrlWithTitle($meta->getLongUrl(), $meta->doValidateUrl());
-        return $title === null ? $meta : $meta->withResolvedTitle($title);
-    }
 }

module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php

@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\ShortUrl\Helper;
+
+use Shlinkio\Shlink\Core\Util\UrlValidatorInterface;
+
+class ShortUrlTitleResolutionHelper implements ShortUrlTitleResolutionHelperInterface
+{
+    private UrlValidatorInterface $urlValidator;
+
+    public function __construct(UrlValidatorInterface $urlValidator)
+    {
+        $this->urlValidator = $urlValidator;
+    }
+
+    public function processTitleAndValidateUrl(TitleResolutionModelInterface $data): TitleResolutionModelInterface
+    {
+        if ($data->hasTitle()) {
+            $this->urlValidator->validateUrl($data->getLongUrl(), $data->doValidateUrl());
+            return $data;
+        }
+
+        $title = $this->urlValidator->validateUrlWithTitle($data->getLongUrl(), $data->doValidateUrl());
+        return $title === null ? $data : $data->withResolvedTitle($title);
+    }
+}

module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelperInterface.php

@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\ShortUrl\Helper;
+
+use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
+
+interface ShortUrlTitleResolutionHelperInterface
+{
+    /**
+     * @throws InvalidUrlException
+     */
+    public function processTitleAndValidateUrl(TitleResolutionModelInterface $data): TitleResolutionModelInterface;
+}

module/Core/src/ShortUrl/Helper/TitleResolutionModelInterface.php

@@ -0,0 +1,16 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\ShortUrl\Helper;
+
+interface TitleResolutionModelInterface
+{
+    public function hasTitle(): bool;
+
+    public function getLongUrl(): string;
+
+    public function doValidateUrl(): ?bool;
+
+    public function withResolvedTitle(string $title): self;
+}

module/Core/test/Service/ShortUrlServiceTest.php

@@ -17,8 +17,8 @@ use Shlinkio\Shlink\Core\Model\ShortUrlsParams;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
 use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface;
 use Shlinkio\Shlink\Core\Service\ShortUrlService;
+use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlTitleResolutionHelperInterface;
 use Shlinkio\Shlink\Core\ShortUrl\Resolver\SimpleShortUrlRelationResolver;
-use Shlinkio\Shlink\Core\Util\UrlValidatorInterface;
 use Shlinkio\Shlink\Rest\Entity\ApiKey;
 use ShlinkioTest\Shlink\Core\Util\ApiKeyHelpersTrait;
 
@@ -32,7 +32,7 @@ class ShortUrlServiceTest extends TestCase
     private ShortUrlService $service;
     private ObjectProphecy $em;
     private ObjectProphecy $urlResolver;
-    private ObjectProphecy $urlValidator;
+    private ObjectProphecy $titleResolutionHelper;
 
     public function setUp(): void
     {
@@ -41,12 +41,12 @@ class ShortUrlServiceTest extends TestCase
         $this->em->flush()->willReturn(null);
 
         $this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class);
-        $this->urlValidator = $this->prophesize(UrlValidatorInterface::class);
+        $this->titleResolutionHelper = $this->prophesize(ShortUrlTitleResolutionHelperInterface::class);
 
         $this->service = new ShortUrlService(
             $this->em->reveal(),
             $this->urlResolver->reveal(),
-            $this->urlValidator->reveal(),
+            $this->titleResolutionHelper->reveal(),
             new SimpleShortUrlRelationResolver(),
         );
     }
@@ -93,6 +93,10 @@ class ShortUrlServiceTest extends TestCase
         )->willReturn($shortUrl);
         $flush = $this->em->flush()->willReturn(null);
 
+        $processTitle = $this->titleResolutionHelper->processTitleAndValidateUrl($shortUrlEdit)->willReturn(
+            $shortUrlEdit,
+        );
+
         $result = $this->service->updateShortUrl(new ShortUrlIdentifier('abc123'), $shortUrlEdit, $apiKey);
 
         self::assertSame($shortUrl, $result);
@@ -102,10 +106,7 @@ class ShortUrlServiceTest extends TestCase
         self::assertEquals($shortUrlEdit->longUrl() ?? $originalLongUrl, $shortUrl->getLongUrl());
         $findShortUrl->shouldHaveBeenCalled();
         $flush->shouldHaveBeenCalled();
-        $this->urlValidator->validateUrlWithTitle(
-            $shortUrlEdit->longUrl(),
-            $shortUrlEdit->doValidateUrl(),
-        )->shouldHaveBeenCalledTimes($expectedValidateCalls);
+        $processTitle->shouldHaveBeenCalledTimes($expectedValidateCalls);
     }
 
     public function provideShortUrlEdits(): iterable

module/Core/test/Service/UrlShortenerTest.php

@@ -16,8 +16,8 @@ use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
 use Shlinkio\Shlink\Core\Service\ShortUrl\ShortCodeHelperInterface;
 use Shlinkio\Shlink\Core\Service\UrlShortener;
+use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlTitleResolutionHelperInterface;
 use Shlinkio\Shlink\Core\ShortUrl\Resolver\SimpleShortUrlRelationResolver;
-use Shlinkio\Shlink\Core\Util\UrlValidatorInterface;
 
 class UrlShortenerTest extends TestCase
 {
@@ -25,12 +25,13 @@ class UrlShortenerTest extends TestCase
 
     private UrlShortener $urlShortener;
     private ObjectProphecy $em;
-    private ObjectProphecy $urlValidator;
+    private ObjectProphecy $titleResolutionHelper;
     private ObjectProphecy $shortCodeHelper;
 
     public function setUp(): void
     {
-        $this->urlValidator = $this->prophesize(UrlValidatorInterface::class);
+        $this->titleResolutionHelper = $this->prophesize(ShortUrlTitleResolutionHelperInterface::class);
+        $this->titleResolutionHelper->processTitleAndValidateUrl(Argument::cetera())->willReturnArgument();
 
         $this->em = $this->prophesize(EntityManagerInterface::class);
         $this->em->persist(Argument::any())->will(function ($arguments): void {
@@ -52,47 +53,22 @@ class UrlShortenerTest extends TestCase
         $this->shortCodeHelper->ensureShortCodeUniqueness(Argument::cetera())->willReturn(true);
 
         $this->urlShortener = new UrlShortener(
-            $this->urlValidator->reveal(),
+            $this->titleResolutionHelper->reveal(),
             $this->em->reveal(),
             new SimpleShortUrlRelationResolver(),
             $this->shortCodeHelper->reveal(),
         );
     }
 
-    /**
-     * @test
-     * @dataProvider provideTitles
-     */
-    public function urlIsProperlyShortened(?string $title, int $validateWithTitleCallsNum, int $validateCallsNum): void
+    /** @test */
+    public function urlIsProperlyShortened(): void
     {
         $longUrl = 'http://foobar.com/12345/hello?foo=bar';
-        $shortUrl = $this->urlShortener->shorten(ShortUrlMeta::fromRawData(['longUrl' => $longUrl, 'title' => $title]));
+        $meta = ShortUrlMeta::fromRawData(['longUrl' => $longUrl]);
+        $shortUrl = $this->urlShortener->shorten($meta);
 
-        self::assertEquals('http://foobar.com/12345/hello?foo=bar', $shortUrl->getLongUrl());
-        $this->urlValidator->validateUrlWithTitle($longUrl, null)->shouldHaveBeenCalledTimes(
-            $validateWithTitleCallsNum,
-        );
-        $this->urlValidator->validateUrl($longUrl, null)->shouldHaveBeenCalledTimes($validateCallsNum);
-    }
-
-    /**
-     * @test
-     * @dataProvider provideTitles
-     */
-    public function urlIsProperlyShortenedWithExpectedResolvedTitle(?string $title): void
-    {
-        $validateWithTitle = $this->urlValidator->validateUrlWithTitle(Argument::cetera())->willReturn($title);
-
-        $shortUrl = $this->urlShortener->shorten(ShortUrlMeta::fromRawData(['longUrl' => 'foo']));
-
-        self::assertEquals($title, $shortUrl->getTitle());
-        $validateWithTitle->shouldHaveBeenCalledOnce();
-    }
-
-    public function provideTitles(): iterable
-    {
-        yield 'no title' => [null, 1, 0];
-        yield 'title' => ['link title', 0, 1];
+        self::assertEquals($longUrl, $shortUrl->getLongUrl());
+        $this->titleResolutionHelper->processTitleAndValidateUrl($meta)->shouldHaveBeenCalledOnce();
     }
 
     /** @test */
@@ -123,8 +99,7 @@ class UrlShortenerTest extends TestCase
         $findExisting->shouldHaveBeenCalledOnce();
         $getRepo->shouldHaveBeenCalledOnce();
         $this->em->persist(Argument::cetera())->shouldNotHaveBeenCalled();
-        $this->urlValidator->validateUrl(Argument::cetera())->shouldNotHaveBeenCalled();
-        $this->urlValidator->validateUrlWithTitle(Argument::cetera())->shouldNotHaveBeenCalled();
+        $this->titleResolutionHelper->processTitleAndValidateUrl(Argument::cetera())->shouldNotHaveBeenCalled();
         self::assertSame($expected, $result);
     }
 

module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php

@@ -0,0 +1,49 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Core\ShortUrl\Helper;
+
+use PHPUnit\Framework\TestCase;
+use Prophecy\PhpUnit\ProphecyTrait;
+use Prophecy\Prophecy\ObjectProphecy;
+use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
+use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlTitleResolutionHelper;
+use Shlinkio\Shlink\Core\Util\UrlValidatorInterface;
+
+class ShortUrlTitleResolutionHelperTest extends TestCase
+{
+    use ProphecyTrait;
+
+    private ShortUrlTitleResolutionHelper $helper;
+    private ObjectProphecy $urlValidator;
+
+    protected function setUp(): void
+    {
+        $this->urlValidator = $this->prophesize(UrlValidatorInterface::class);
+        $this->helper = new ShortUrlTitleResolutionHelper($this->urlValidator->reveal());
+    }
+
+    /**
+     * @test
+     * @dataProvider provideTitles
+     */
+    public function urlIsProperlyShortened(?string $title, int $validateWithTitleCallsNum, int $validateCallsNum): void
+    {
+        $longUrl = 'http://foobar.com/12345/hello?foo=bar';
+        $this->helper->processTitleAndValidateUrl(
+            ShortUrlMeta::fromRawData(['longUrl' => $longUrl, 'title' => $title]),
+        );
+
+        $this->urlValidator->validateUrlWithTitle($longUrl, null)->shouldHaveBeenCalledTimes(
+            $validateWithTitleCallsNum,
+        );
+        $this->urlValidator->validateUrl($longUrl, null)->shouldHaveBeenCalledTimes($validateCallsNum);
+    }
+
+    public function provideTitles(): iterable
+    {
+        yield 'no title' => [null, 1, 0];
+        yield 'title' => ['link title', 0, 1];
+    }
+}