From bfb54189b8e231528570a9877cf46db7a8aec8e7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 16 Jan 2022 15:34:07 +0100 Subject: [PATCH 1/3] Moved some config to the proper namespace, now that config is no longer part of the public contract --- config/autoload/redirects.global.php | 3 +- config/autoload/webhooks.global.php | 5 +-- module/Core/config/dependencies.config.php | 6 ++- module/Core/src/Options/RedirectOptions.php | 45 +++++++++++++++++++ .../Core/src/Options/UrlShortenerOptions.php | 34 -------------- module/Core/src/Options/WebhookOptions.php | 10 ++--- .../Core/src/Util/RedirectResponseHelper.php | 4 +- .../NotifyVisitToWebHooksTest.php | 2 +- .../test/Util/RedirectResponseHelperTest.php | 6 +-- 9 files changed, 63 insertions(+), 52 deletions(-) create mode 100644 module/Core/src/Options/RedirectOptions.php diff --git a/config/autoload/redirects.global.php b/config/autoload/redirects.global.php index e38b9c25..20cde8eb 100644 --- a/config/autoload/redirects.global.php +++ b/config/autoload/redirects.global.php @@ -15,8 +15,7 @@ return [ 'base_url' => env('DEFAULT_BASE_URL_REDIRECT'), ], - 'url_shortener' => [ - // TODO Move these options to their own config namespace. Maybe "redirects". + '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), ], diff --git a/config/autoload/webhooks.global.php b/config/autoload/webhooks.global.php index fb946028..6bbbfbda 100644 --- a/config/autoload/webhooks.global.php +++ b/config/autoload/webhooks.global.php @@ -9,9 +9,8 @@ return (static function (): array { return [ - 'url_shortener' => [ - // TODO Move these options to their own config namespace - 'visits_webhooks' => $webhooks === null ? [] : explode(',', $webhooks), + 'visits_webhooks' => [ + 'webhooks' => $webhooks === null ? [] : explode(',', $webhooks), 'notify_orphan_visits_to_webhooks' => (bool) env('NOTIFY_ORPHAN_VISITS_TO_WEBHOOKS', false), ], diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index fdfecef9..90931c11 100644 --- a/module/Core/config/dependencies.config.php +++ b/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, @@ -86,10 +87,11 @@ 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, @@ -123,7 +125,7 @@ return [ 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'], diff --git a/module/Core/src/Options/RedirectOptions.php b/module/Core/src/Options/RedirectOptions.php new file mode 100644 index 00000000..5479c59b --- /dev/null +++ b/module/Core/src/Options/RedirectOptions.php @@ -0,0 +1,45 @@ +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; + } +} diff --git a/module/Core/src/Options/UrlShortenerOptions.php b/module/Core/src/Options/UrlShortenerOptions.php index ecbbb590..775254ce 100644 --- a/module/Core/src/Options/UrlShortenerOptions.php +++ b/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; diff --git a/module/Core/src/Options/WebhookOptions.php b/module/Core/src/Options/WebhookOptions.php index c86789b2..6eb07692 100644 --- a/module/Core/src/Options/WebhookOptions.php +++ b/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 diff --git a/module/Core/src/Util/RedirectResponseHelper.php b/module/Core/src/Util/RedirectResponseHelper.php index 5f9edf99..312c2a95 100644 --- a/module/Core/src/Util/RedirectResponseHelper.php +++ b/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) { } diff --git a/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php b/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php index 99609bb4..56324e40 100644 --- a/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php +++ b/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']), diff --git a/module/Core/test/Util/RedirectResponseHelperTest.php b/module/Core/test/Util/RedirectResponseHelperTest.php index eb26768f..651d4bc7 100644 --- a/module/Core/test/Util/RedirectResponseHelperTest.php +++ b/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); } From 77fee1390fd5b3925741faf585d5d37f6c0b90f5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 16 Jan 2022 15:41:20 +0100 Subject: [PATCH 2/3] Renamed class to a more appropriate name --- module/Core/config/dependencies.config.php | 8 ++++---- module/Core/src/Importer/ImportedLinksProcessor.php | 4 ++-- ...{ShortCodeHelper.php => ShortCodeUniquenessHelper.php} | 2 +- ...terface.php => ShortCodeUniquenessHelperInterface.php} | 2 +- module/Core/src/Service/UrlShortener.php | 4 ++-- module/Core/test/Importer/ImportedLinksProcessorTest.php | 4 ++-- ...deHelperTest.php => ShortCodeUniquenessHelperTest.php} | 8 ++++---- module/Core/test/Service/UrlShortenerTest.php | 4 ++-- 8 files changed, 18 insertions(+), 18 deletions(-) rename module/Core/src/Service/ShortUrl/{ShortCodeHelper.php => ShortCodeUniquenessHelper.php} (90%) rename module/Core/src/Service/ShortUrl/{ShortCodeHelperInterface.php => ShortCodeUniquenessHelperInterface.php} (72%) rename module/Core/test/Service/ShortUrl/{ShortCodeHelperTest.php => ShortCodeUniquenessHelperTest.php} (91%) diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 90931c11..516ad8a1 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -33,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, @@ -97,7 +97,7 @@ return [ ShortUrl\Helper\ShortUrlTitleResolutionHelper::class, 'em', ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class, - Service\ShortUrl\ShortCodeHelper::class, + Service\ShortUrl\ShortCodeUniquenessHelper::class, ], Visit\VisitsTracker::class => [ 'em', @@ -120,7 +120,7 @@ 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], @@ -165,7 +165,7 @@ return [ Importer\ImportedLinksProcessor::class => [ 'em', ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class, - Service\ShortUrl\ShortCodeHelper::class, + Service\ShortUrl\ShortCodeUniquenessHelper::class, Util\DoctrineBatchHelper::class, ], diff --git a/module/Core/src/Importer/ImportedLinksProcessor.php b/module/Core/src/Importer/ImportedLinksProcessor.php index cddfbb88..fe4f24df 100644 --- a/module/Core/src/Importer/ImportedLinksProcessor.php +++ b/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); diff --git a/module/Core/src/Service/ShortUrl/ShortCodeHelper.php b/module/Core/src/Service/ShortUrl/ShortCodeUniquenessHelper.php similarity index 90% rename from module/Core/src/Service/ShortUrl/ShortCodeHelper.php rename to module/Core/src/Service/ShortUrl/ShortCodeUniquenessHelper.php index 5bb992c5..461a14b6 100644 --- a/module/Core/src/Service/ShortUrl/ShortCodeHelper.php +++ b/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) { diff --git a/module/Core/src/Service/ShortUrl/ShortCodeHelperInterface.php b/module/Core/src/Service/ShortUrl/ShortCodeUniquenessHelperInterface.php similarity index 72% rename from module/Core/src/Service/ShortUrl/ShortCodeHelperInterface.php rename to module/Core/src/Service/ShortUrl/ShortCodeUniquenessHelperInterface.php index a020a30c..975a2b8b 100644 --- a/module/Core/src/Service/ShortUrl/ShortCodeHelperInterface.php +++ b/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; } diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php index 41779715..8fa54493 100644 --- a/module/Core/src/Service/UrlShortener.php +++ b/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, ) { } diff --git a/module/Core/test/Importer/ImportedLinksProcessorTest.php b/module/Core/test/Importer/ImportedLinksProcessorTest.php index 1a4a4de1..70662bb1 100644 --- a/module/Core/test/Importer/ImportedLinksProcessorTest.php +++ b/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); diff --git a/module/Core/test/Service/ShortUrl/ShortCodeHelperTest.php b/module/Core/test/Service/ShortUrl/ShortCodeUniquenessHelperTest.php similarity index 91% rename from module/Core/test/Service/ShortUrl/ShortCodeHelperTest.php rename to module/Core/test/Service/ShortUrl/ShortCodeUniquenessHelperTest.php index b30f8cab..7e962dc8 100644 --- a/module/Core/test/Service/ShortUrl/ShortCodeHelperTest.php +++ b/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'); diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php index 6bac432e..bdd508b4 100644 --- a/module/Core/test/Service/UrlShortenerTest.php +++ b/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( From 492eba3a8ba20c9e42dbddd6da58a9bae4369d81 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 16 Jan 2022 15:54:22 +0100 Subject: [PATCH 3/3] Fixed duplicated slashes generated in path when doing not-found redirects with placeholders --- module/Core/src/Config/NotFoundRedirectResolver.php | 5 ++++- module/Core/test/Config/NotFoundRedirectResolverTest.php | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/module/Core/src/Config/NotFoundRedirectResolver.php b/module/Core/src/Config/NotFoundRedirectResolver.php index 531254f7..caa100c3 100644 --- a/module/Core/src/Config/NotFoundRedirectResolver.php +++ b/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 diff --git a/module/Core/test/Config/NotFoundRedirectResolverTest.php b/module/Core/test/Config/NotFoundRedirectResolverTest.php index 0dc25768..aa98d102 100644 --- a/module/Core/test/Config/NotFoundRedirectResolverTest.php +++ b/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', ]; }