Merge pull request #1334 from acelaya-forks/feature/tackle-todos

Feature/tackle todos

config/autoload/redirects.global.php

@@ -15,8 +15,7 @@ return [
         'base_url' => env('DEFAULT_BASE_URL_REDIRECT'),
     ],
 
-    'url_shortener' => [
+    'redirects' => [
-        // TODO Move these options to their own config namespace. Maybe "redirects".
         'redirect_status_code' => (int) env('REDIRECT_STATUS_CODE', DEFAULT_REDIRECT_STATUS_CODE),
         'redirect_cache_lifetime' => (int) env('REDIRECT_CACHE_LIFETIME', DEFAULT_REDIRECT_CACHE_LIFETIME),
     ],

config/autoload/webhooks.global.php

@@ -9,9 +9,8 @@ return (static function (): array {
 
     return [
 
-        'url_shortener' => [
+        'visits_webhooks' => [
-            // TODO Move these options to their own config namespace
+            'webhooks' => $webhooks === null ? [] : explode(',', $webhooks),
-            'visits_webhooks' => $webhooks === null ? [] : explode(',', $webhooks),
             'notify_orphan_visits_to_webhooks' => (bool) env('NOTIFY_ORPHAN_VISITS_TO_WEBHOOKS', false),
         ],
 

module/Core/config/dependencies.config.php

@@ -23,6 +23,7 @@ return [
             Options\AppOptions::class => ConfigAbstractFactory::class,
             Options\DeleteShortUrlsOptions::class => ConfigAbstractFactory::class,
             Options\NotFoundRedirectOptions::class => ConfigAbstractFactory::class,
+            Options\RedirectOptions::class => ConfigAbstractFactory::class,
             Options\UrlShortenerOptions::class => ConfigAbstractFactory::class,
             Options\TrackingOptions::class => ConfigAbstractFactory::class,
             Options\QrCodeOptions::class => ConfigAbstractFactory::class,
@@ -32,7 +33,7 @@ return [
             Service\ShortUrlService::class => ConfigAbstractFactory::class,
             Service\ShortUrl\DeleteShortUrlService::class => ConfigAbstractFactory::class,
             Service\ShortUrl\ShortUrlResolver::class => ConfigAbstractFactory::class,
-            Service\ShortUrl\ShortCodeHelper::class => ConfigAbstractFactory::class,
+            Service\ShortUrl\ShortCodeUniquenessHelper::class => ConfigAbstractFactory::class,
 
             Tag\TagService::class => ConfigAbstractFactory::class,
 
@@ -86,16 +87,17 @@ return [
         Options\AppOptions::class => ['config.app_options'],
         Options\DeleteShortUrlsOptions::class => ['config.delete_short_urls'],
         Options\NotFoundRedirectOptions::class => ['config.not_found_redirects'],
+        Options\RedirectOptions::class => ['config.redirects'],
         Options\UrlShortenerOptions::class => ['config.url_shortener'],
         Options\TrackingOptions::class => ['config.tracking'],
         Options\QrCodeOptions::class => ['config.qr_codes'],
-        Options\WebhookOptions::class => ['config.url_shortener'], // TODO This config is currently under url_shortener
+        Options\WebhookOptions::class => ['config.visits_webhooks'],
 
         Service\UrlShortener::class => [
             ShortUrl\Helper\ShortUrlTitleResolutionHelper::class,
             'em',
             ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class,
-            Service\ShortUrl\ShortCodeHelper::class,
+            Service\ShortUrl\ShortCodeUniquenessHelper::class,
         ],
         Visit\VisitsTracker::class => [
             'em',
@@ -118,12 +120,12 @@ return [
             Service\ShortUrl\ShortUrlResolver::class,
         ],
         Service\ShortUrl\ShortUrlResolver::class => ['em'],
-        Service\ShortUrl\ShortCodeHelper::class => ['em'],
+        Service\ShortUrl\ShortCodeUniquenessHelper::class => ['em'],
         Domain\DomainService::class => ['em', 'config.url_shortener.domain.hostname'],
 
         Util\UrlValidator::class => ['httpClient', Options\UrlShortenerOptions::class],
         Util\DoctrineBatchHelper::class => ['em'],
-        Util\RedirectResponseHelper::class => [Options\UrlShortenerOptions::class],
+        Util\RedirectResponseHelper::class => [Options\RedirectOptions::class],
 
         Config\NotFoundRedirectResolver::class => [Util\RedirectResponseHelper::class, 'Logger_Shlink'],
 
@@ -163,7 +165,7 @@ return [
         Importer\ImportedLinksProcessor::class => [
             'em',
             ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class,
-            Service\ShortUrl\ShortCodeHelper::class,
+            Service\ShortUrl\ShortCodeUniquenessHelper::class,
             Util\DoctrineBatchHelper::class,
         ],
 

module/Core/src/Config/NotFoundRedirectResolver.php

@@ -70,7 +70,10 @@ class NotFoundRedirectResolver implements NotFoundRedirectResolverInterface
             $replacePlaceholderForPattern(self::DOMAIN_PLACEHOLDER, $domain, $modifier),
             $replacePlaceholderForPattern(self::ORIGINAL_PATH_PLACEHOLDER, $path, $modifier),
         );
-        $replacePlaceholdersInPath = $replacePlaceholders('\Functional\id');
+        $replacePlaceholdersInPath = compose(
+            $replacePlaceholders('\Functional\id'),
+            static fn (?string $path) => $path === null ? null : str_replace('//', '/', $path), // Fix duplicated bars
+        );
         $replacePlaceholdersInQuery = $replacePlaceholders('\urlencode');
 
         return $redirectUri

module/Core/src/Importer/ImportedLinksProcessor.php

@@ -8,7 +8,7 @@ use Doctrine\ORM\EntityManagerInterface;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface;
-use Shlinkio\Shlink\Core\Service\ShortUrl\ShortCodeHelperInterface;
+use Shlinkio\Shlink\Core\Service\ShortUrl\ShortCodeUniquenessHelperInterface;
 use Shlinkio\Shlink\Core\ShortUrl\Resolver\ShortUrlRelationResolverInterface;
 use Shlinkio\Shlink\Core\Util\DoctrineBatchHelperInterface;
 use Shlinkio\Shlink\Importer\ImportedLinksProcessorInterface;
@@ -25,7 +25,7 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface
     public function __construct(
         private EntityManagerInterface $em,
         private ShortUrlRelationResolverInterface $relationResolver,
-        private ShortCodeHelperInterface $shortCodeHelper,
+        private ShortCodeUniquenessHelperInterface $shortCodeHelper,
         private DoctrineBatchHelperInterface $batchHelper,
     ) {
         $this->shortUrlRepo = $this->em->getRepository(ShortUrl::class);

module/Core/src/Options/RedirectOptions.php

@@ -0,0 +1,45 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Options;
+
+use Laminas\Stdlib\AbstractOptions;
+
+use function Functional\contains;
+
+use const Shlinkio\Shlink\DEFAULT_REDIRECT_CACHE_LIFETIME;
+use const Shlinkio\Shlink\DEFAULT_REDIRECT_STATUS_CODE;
+
+class RedirectOptions extends AbstractOptions
+{
+    private int $redirectStatusCode = DEFAULT_REDIRECT_STATUS_CODE;
+    private int $redirectCacheLifetime = DEFAULT_REDIRECT_CACHE_LIFETIME;
+
+    public function redirectStatusCode(): int
+    {
+        return $this->redirectStatusCode;
+    }
+
+    protected function setRedirectStatusCode(int $redirectStatusCode): void
+    {
+        $this->redirectStatusCode = $this->normalizeRedirectStatusCode($redirectStatusCode);
+    }
+
+    private function normalizeRedirectStatusCode(int $statusCode): int
+    {
+        return contains([301, 302], $statusCode) ? $statusCode : DEFAULT_REDIRECT_STATUS_CODE;
+    }
+
+    public function redirectCacheLifetime(): int
+    {
+        return $this->redirectCacheLifetime;
+    }
+
+    protected function setRedirectCacheLifetime(int $redirectCacheLifetime): void
+    {
+        $this->redirectCacheLifetime = $redirectCacheLifetime > 0
+            ? $redirectCacheLifetime
+            : DEFAULT_REDIRECT_CACHE_LIFETIME;
+    }
+}

module/Core/src/Options/UrlShortenerOptions.php

@@ -6,18 +6,11 @@ namespace Shlinkio\Shlink\Core\Options;
 
 use Laminas\Stdlib\AbstractOptions;
 
-use function Functional\contains;
-
-use const Shlinkio\Shlink\DEFAULT_REDIRECT_CACHE_LIFETIME;
-use const Shlinkio\Shlink\DEFAULT_REDIRECT_STATUS_CODE;
-
 class UrlShortenerOptions extends AbstractOptions
 {
     protected $__strictMode__ = false; // phpcs:ignore
 
     private bool $validateUrl = true;
-    private int $redirectStatusCode = DEFAULT_REDIRECT_STATUS_CODE;
-    private int $redirectCacheLifetime = DEFAULT_REDIRECT_CACHE_LIFETIME;
     private bool $autoResolveTitles = false;
     private bool $appendExtraPath = false;
 
@@ -31,33 +24,6 @@ class UrlShortenerOptions extends AbstractOptions
         $this->validateUrl = $validateUrl;
     }
 
-    public function redirectStatusCode(): int
-    {
-        return $this->redirectStatusCode;
-    }
-
-    protected function setRedirectStatusCode(int $redirectStatusCode): void
-    {
-        $this->redirectStatusCode = $this->normalizeRedirectStatusCode($redirectStatusCode);
-    }
-
-    private function normalizeRedirectStatusCode(int $statusCode): int
-    {
-        return contains([301, 302], $statusCode) ? $statusCode : DEFAULT_REDIRECT_STATUS_CODE;
-    }
-
-    public function redirectCacheLifetime(): int
-    {
-        return $this->redirectCacheLifetime;
-    }
-
-    protected function setRedirectCacheLifetime(int $redirectCacheLifetime): void
-    {
-        $this->redirectCacheLifetime = $redirectCacheLifetime > 0
-            ? $redirectCacheLifetime
-            : DEFAULT_REDIRECT_CACHE_LIFETIME;
-    }
-
     public function autoResolveTitles(): bool
     {
         return $this->autoResolveTitles;

module/Core/src/Options/WebhookOptions.php

@@ -10,22 +10,22 @@ class WebhookOptions extends AbstractOptions
 {
     protected $__strictMode__ = false; // phpcs:ignore
 
-    private array $visitsWebhooks = [];
+    private array $webhooks = [];
     private bool $notifyOrphanVisitsToWebhooks = false;
 
     public function webhooks(): array
     {
-        return $this->visitsWebhooks;
+        return $this->webhooks;
     }
 
     public function hasWebhooks(): bool
     {
-        return ! empty($this->visitsWebhooks);
+        return ! empty($this->webhooks);
     }
 
-    protected function setVisitsWebhooks(array $visitsWebhooks): void
+    protected function setWebhooks(array $webhooks): void
     {
-        $this->visitsWebhooks = $visitsWebhooks;
+        $this->webhooks = $webhooks;
     }
 
     public function notifyOrphanVisits(): bool

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

@@ -9,7 +9,7 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
 
-class ShortCodeHelper implements ShortCodeHelperInterface // TODO Rename to ShortCodeUniquenessHelper
+class ShortCodeUniquenessHelper implements ShortCodeUniquenessHelperInterface
 {
     public function __construct(private EntityManagerInterface $em)
     {

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

@@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\Core\Service\ShortUrl;
 
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 
-interface ShortCodeHelperInterface // TODO Rename to ShortCodeUniquenessHelperInterface
+interface ShortCodeUniquenessHelperInterface
 {
     public function ensureShortCodeUniqueness(ShortUrl $shortUrlToBeCreated, bool $hasCustomSlug): bool;
 }

module/Core/src/Service/UrlShortener.php

@@ -10,7 +10,7 @@ use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
 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\Service\ShortUrl\ShortCodeUniquenessHelperInterface;
 use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlTitleResolutionHelperInterface;
 use Shlinkio\Shlink\Core\ShortUrl\Resolver\ShortUrlRelationResolverInterface;
 
@@ -20,7 +20,7 @@ class UrlShortener implements UrlShortenerInterface
         private ShortUrlTitleResolutionHelperInterface $titleResolutionHelper,
         private EntityManagerInterface $em,
         private ShortUrlRelationResolverInterface $relationResolver,
-        private ShortCodeHelperInterface $shortCodeHelper,
+        private ShortCodeUniquenessHelperInterface $shortCodeHelper,
     ) {
     }
 

module/Core/src/Util/RedirectResponseHelper.php

@@ -7,13 +7,13 @@ namespace Shlinkio\Shlink\Core\Util;
 use Fig\Http\Message\StatusCodeInterface;
 use Laminas\Diactoros\Response\RedirectResponse;
 use Psr\Http\Message\ResponseInterface;
-use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
+use Shlinkio\Shlink\Core\Options\RedirectOptions;
 
 use function sprintf;
 
 class RedirectResponseHelper implements RedirectResponseHelperInterface
 {
-    public function __construct(private UrlShortenerOptions $options)
+    public function __construct(private RedirectOptions $options)
     {
     }
 

module/Core/test/Config/NotFoundRedirectResolverTest.php

@@ -99,7 +99,7 @@ class NotFoundRedirectResolverTest extends TestCase
             new NotFoundRedirectOptions([
                 'regular404' => 'https://redirect-here.com/{ORIGINAL_PATH}/{DOMAIN}/?d={DOMAIN}&p={ORIGINAL_PATH}',
             ]),
-            'https://redirect-here.com//foo/bar/doma.in/?d=doma.in&p=%2Ffoo%2Fbar', // TODO Fix duplicated slash
+            'https://redirect-here.com/foo/bar/doma.in/?d=doma.in&p=%2Ffoo%2Fbar',
         ];
         yield 'invalid short URL' => [
             new Uri('/foo'),
@@ -111,7 +111,7 @@ class NotFoundRedirectResolverTest extends TestCase
             new Uri('/foo'),
             $this->notFoundType($this->requestForRoute(RedirectAction::class)),
             new NotFoundRedirectOptions(['invalidShortUrl' => 'https://redirect-here.com/{ORIGINAL_PATH}']),
-            'https://redirect-here.com//foo', // TODO Fix duplicated slash
+            'https://redirect-here.com/foo',
         ];
     }
 

module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php

@@ -162,7 +162,7 @@ class NotifyVisitToWebHooksTest extends TestCase
             $this->em->reveal(),
             $this->logger->reveal(),
             new WebhookOptions(
-                ['visits_webhooks' => $webhooks, 'notify_orphan_visits_to_webhooks' => $notifyOrphanVisits],
+                ['webhooks' => $webhooks, 'notify_orphan_visits_to_webhooks' => $notifyOrphanVisits],
             ),
             new ShortUrlDataTransformer(new ShortUrlStringifier([])),
             new AppOptions(['name' => 'Shlink', 'version' => '1.2.3']),

module/Core/test/Importer/ImportedLinksProcessorTest.php

@@ -15,7 +15,7 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Importer\ImportedLinksProcessor;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface;
-use Shlinkio\Shlink\Core\Service\ShortUrl\ShortCodeHelperInterface;
+use Shlinkio\Shlink\Core\Service\ShortUrl\ShortCodeUniquenessHelperInterface;
 use Shlinkio\Shlink\Core\ShortUrl\Resolver\SimpleShortUrlRelationResolver;
 use Shlinkio\Shlink\Core\Util\DoctrineBatchHelperInterface;
 use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl;
@@ -46,7 +46,7 @@ class ImportedLinksProcessorTest extends TestCase
         $this->repo = $this->prophesize(ShortUrlRepositoryInterface::class);
         $this->em->getRepository(ShortUrl::class)->willReturn($this->repo->reveal());
 
-        $this->shortCodeHelper = $this->prophesize(ShortCodeHelperInterface::class);
+        $this->shortCodeHelper = $this->prophesize(ShortCodeUniquenessHelperInterface::class);
         $batchHelper = $this->prophesize(DoctrineBatchHelperInterface::class);
         $batchHelper->wrapIterable(Argument::cetera())->willReturnArgument(0);
 

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

@@ -12,20 +12,20 @@ use Shlinkio\Shlink\Core\Entity\Domain;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
-use Shlinkio\Shlink\Core\Service\ShortUrl\ShortCodeHelper;
+use Shlinkio\Shlink\Core\Service\ShortUrl\ShortCodeUniquenessHelper;
 
-class ShortCodeHelperTest extends TestCase
+class ShortCodeUniquenessHelperTest extends TestCase
 {
     use ProphecyTrait;
 
-    private ShortCodeHelper $helper;
+    private ShortCodeUniquenessHelper $helper;
     private ObjectProphecy $em;
     private ObjectProphecy $shortUrl;
 
     protected function setUp(): void
     {
         $this->em = $this->prophesize(EntityManagerInterface::class);
-        $this->helper = new ShortCodeHelper($this->em->reveal());
+        $this->helper = new ShortCodeUniquenessHelper($this->em->reveal());
 
         $this->shortUrl = $this->prophesize(ShortUrl::class);
         $this->shortUrl->getShortCode()->willReturn('abc123');

module/Core/test/Service/UrlShortenerTest.php

@@ -14,7 +14,7 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException;
 use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
-use Shlinkio\Shlink\Core\Service\ShortUrl\ShortCodeHelperInterface;
+use Shlinkio\Shlink\Core\Service\ShortUrl\ShortCodeUniquenessHelperInterface;
 use Shlinkio\Shlink\Core\Service\UrlShortener;
 use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlTitleResolutionHelperInterface;
 use Shlinkio\Shlink\Core\ShortUrl\Resolver\SimpleShortUrlRelationResolver;
@@ -48,7 +48,7 @@ class UrlShortenerTest extends TestCase
         $repo = $this->prophesize(ShortUrlRepository::class);
         $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
 
-        $this->shortCodeHelper = $this->prophesize(ShortCodeHelperInterface::class);
+        $this->shortCodeHelper = $this->prophesize(ShortCodeUniquenessHelperInterface::class);
         $this->shortCodeHelper->ensureShortCodeUniqueness(Argument::cetera())->willReturn(true);
 
         $this->urlShortener = new UrlShortener(

module/Core/test/Util/RedirectResponseHelperTest.php

@@ -6,17 +6,17 @@ namespace ShlinkioTest\Shlink\Core\Util;
 
 use Laminas\Diactoros\Response\RedirectResponse;
 use PHPUnit\Framework\TestCase;
-use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
+use Shlinkio\Shlink\Core\Options\RedirectOptions;
 use Shlinkio\Shlink\Core\Util\RedirectResponseHelper;
 
 class RedirectResponseHelperTest extends TestCase
 {
     private RedirectResponseHelper $helper;
-    private UrlShortenerOptions $shortenerOpts;
+    private RedirectOptions $shortenerOpts;
 
     protected function setUp(): void
     {
-        $this->shortenerOpts = new UrlShortenerOptions();
+        $this->shortenerOpts = new RedirectOptions();
         $this->helper = new RedirectResponseHelper($this->shortenerOpts);
     }