Moved check for URL validation config option to the UrlValidator itself

module/Core/config/dependencies.config.php

@@ -51,7 +51,7 @@ return [
         Options\NotFoundRedirectOptions::class => ['config.not_found_redirects'],
         Options\UrlShortenerOptions::class => ['config.url_shortener'],
 
-        Service\UrlShortener::class => [Util\UrlValidator::class, 'em', Options\UrlShortenerOptions::class],
+        Service\UrlShortener::class => [Util\UrlValidator::class, 'em'],
         Service\VisitsTracker::class => ['em', EventDispatcherInterface::class],
         Service\ShortUrlService::class => ['em', Service\ShortUrl\ShortUrlResolver::class],
         Service\VisitService::class => ['em'],
@@ -63,7 +63,7 @@ return [
         ],
         Service\ShortUrl\ShortUrlResolver::class => ['em'],
 
-        Util\UrlValidator::class => ['httpClient'],
+        Util\UrlValidator::class => ['httpClient', Options\UrlShortenerOptions::class],
 
         Action\RedirectAction::class => [
             Service\ShortUrl\ShortUrlResolver::class,

module/Core/src/Service/UrlShortener.php

@@ -11,7 +11,6 @@ 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\Options\UrlShortenerOptions;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
 use Shlinkio\Shlink\Core\Util\TagManagerTrait;
 use Shlinkio\Shlink\Core\Util\UrlValidatorInterface;
@@ -24,17 +23,14 @@ class UrlShortener implements UrlShortenerInterface
     use TagManagerTrait;
 
     private EntityManagerInterface $em;
-    private UrlShortenerOptions $options;
     private UrlValidatorInterface $urlValidator;
 
     public function __construct(
         UrlValidatorInterface $urlValidator,
-        EntityManagerInterface $em,
+        EntityManagerInterface $em
-        UrlShortenerOptions $options
     ) {
         $this->urlValidator = $urlValidator;
         $this->em = $em;
-        $this->options = $options;
     }
 
     /**
@@ -53,11 +49,7 @@ class UrlShortener implements UrlShortenerInterface
             return $existingShortUrl;
         }
 
-        // If the URL validation is enabled, check that the URL actually exists
+        $this->urlValidator->validateUrl($url);
-        if ($this->options->isUrlValidationEnabled()) {
-            $this->urlValidator->validateUrl($url);
-        }
-
         $this->em->beginTransaction();
         $shortUrl = new ShortUrl($url, $meta, new PersistenceDomainResolver($this->em));
         $shortUrl->setTags($this->tagNamesToEntities($this->em, $tags));

module/Core/src/Util/UrlValidator.php

@@ -9,16 +9,19 @@ use GuzzleHttp\ClientInterface;
 use GuzzleHttp\Exception\GuzzleException;
 use GuzzleHttp\RequestOptions;
 use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
+use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
 
 class UrlValidator implements UrlValidatorInterface, RequestMethodInterface
 {
     private const MAX_REDIRECTS = 15;
 
     private ClientInterface $httpClient;
+    private UrlShortenerOptions $options;
 
-    public function __construct(ClientInterface $httpClient)
+    public function __construct(ClientInterface $httpClient, UrlShortenerOptions $options)
     {
         $this->httpClient = $httpClient;
+        $this->options = $options;
     }
 
     /**
@@ -26,6 +29,11 @@ class UrlValidator implements UrlValidatorInterface, RequestMethodInterface
      */
     public function validateUrl(string $url): void
     {
+        // If the URL validation is not enabled, skip check
+        if (! $this->options->isUrlValidationEnabled()) {
+            return;
+        }
+
         try {
             $this->httpClient->request(self::METHOD_GET, $url, [
                 RequestOptions::ALLOW_REDIRECTS => ['max' => self::MAX_REDIRECTS],

module/Core/test/Service/UrlShortenerTest.php

@@ -17,7 +17,6 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Tag;
 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\Service\UrlShortener;
 use Shlinkio\Shlink\Core\Util\UrlValidatorInterface;
@@ -33,6 +32,10 @@ class UrlShortenerTest extends TestCase
     public function setUp(): void
     {
         $this->urlValidator = $this->prophesize(UrlValidatorInterface::class);
+        $this->urlValidator->validateUrl('http://foobar.com/12345/hello?foo=bar')->will(
+            function (): void {
+            },
+        );
 
         $this->em = $this->prophesize(EntityManagerInterface::class);
         $conn = $this->prophesize(Connection::class);
@@ -50,16 +53,7 @@ class UrlShortenerTest extends TestCase
         $repo->shortCodeIsInUse(Argument::cetera())->willReturn(false);
         $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
 
-        $this->setUrlShortener(false);
+        $this->urlShortener = new UrlShortener($this->urlValidator->reveal(), $this->em->reveal());
-    }
-
-    private function setUrlShortener(bool $urlValidationEnabled): void
-    {
-        $this->urlShortener = new UrlShortener(
-            $this->urlValidator->reveal(),
-            $this->em->reveal(),
-            new UrlShortenerOptions(['validate_url' => $urlValidationEnabled]),
-        );
     }
 
     /** @test */
@@ -119,24 +113,6 @@ class UrlShortenerTest extends TestCase
         );
     }
 
-    /** @test */
-    public function validatorIsCalledWhenUrlValidationIsEnabled(): void
-    {
-        $this->setUrlShortener(true);
-        $validateUrl = $this->urlValidator->validateUrl('http://foobar.com/12345/hello?foo=bar')->will(
-            function (): void {
-            },
-        );
-
-        $this->urlShortener->urlToShortCode(
-            new Uri('http://foobar.com/12345/hello?foo=bar'),
-            [],
-            ShortUrlMeta::createEmpty(),
-        );
-
-        $validateUrl->shouldHaveBeenCalledOnce();
-    }
-
     /** @test */
     public function exceptionIsThrownWhenNonUniqueSlugIsProvided(): void
     {
@@ -175,6 +151,7 @@ class UrlShortenerTest extends TestCase
         $findExisting->shouldHaveBeenCalledOnce();
         $getRepo->shouldHaveBeenCalledOnce();
         $this->em->persist(Argument::cetera())->shouldNotHaveBeenCalled();
+        $this->urlValidator->validateUrl(Argument::cetera())->shouldNotHaveBeenCalled();
         $this->assertSame($expected, $result);
     }
 

module/Core/test/Util/UrlValidatorTest.php

@@ -13,17 +13,20 @@ use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
+use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
 use Shlinkio\Shlink\Core\Util\UrlValidator;
 
 class UrlValidatorTest extends TestCase
 {
     private UrlValidator $urlValidator;
     private ObjectProphecy $httpClient;
+    private UrlShortenerOptions $options;
 
     public function setUp(): void
     {
         $this->httpClient = $this->prophesize(ClientInterface::class);
-        $this->urlValidator = new UrlValidator($this->httpClient->reveal());
+        $this->options = new UrlShortenerOptions(['validate_url' => true]);
+        $this->urlValidator = new UrlValidator($this->httpClient->reveal(), $this->options);
     }
 
     /** @test */
@@ -52,4 +55,15 @@ class UrlValidatorTest extends TestCase
 
         $request->shouldHaveBeenCalledOnce();
     }
+
+    /** @test */
+    public function noCheckIsPerformedWhenUrlValidationIsDisabled(): void
+    {
+        $request = $this->httpClient->request(Argument::cetera())->willReturn(new Response());
+        $this->options->validateUrl = false;
+
+        $this->urlValidator->validateUrl('');
+
+        $request->shouldNotHaveBeenCalled();
+    }
 }