From efe655f880ec7e8d873bea5c8aa88dfaa4305237 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 4 Aug 2022 17:03:08 +0200 Subject: [PATCH] Enhanced ExtraPathRedirectMiddleware so that it supports multi-segment slugs --- .../ExtraPathRedirectMiddleware.php | 54 +++++++++------ .../ExtraPathRedirectMiddlewareTest.php | 69 ++++++++++++++----- 2 files changed, 87 insertions(+), 36 deletions(-) diff --git a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php index 44cab54c..bb350aa2 100644 --- a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php +++ b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php @@ -18,8 +18,10 @@ use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlRedirectionBuilderInterface; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; -use function array_pad; +use function array_slice; +use function count; use function explode; +use function implode; use function sprintf; use function trim; @@ -42,21 +44,7 @@ class ExtraPathRedirectMiddleware implements MiddlewareInterface return $handler->handle($request); } - $uri = $request->getUri(); - $query = $request->getQueryParams(); - [$potentialShortCode, $extraPath] = $this->resolvePotentialShortCodeAndExtraPath($uri); - $identifier = ShortUrlIdentifier::fromShortCodeAndDomain($potentialShortCode, $uri->getAuthority()); - - try { - // TODO Try pieces of the URL in order to match multi-segment slugs too - $shortUrl = $this->resolver->resolveEnabledShortUrl($identifier); - $this->requestTracker->trackIfApplicable($shortUrl, $request); - - $longUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $query, $extraPath); - return $this->redirectResponseHelper->buildRedirectResponse($longUrl); - } catch (ShortUrlNotFoundException) { - return $handler->handle($request); - } + return $this->tryToResolveRedirect($request, $handler); } private function shouldApplyLogic(?NotFoundType $notFoundType): bool @@ -74,14 +62,40 @@ class ExtraPathRedirectMiddleware implements MiddlewareInterface ); } + private function tryToResolveRedirect( + ServerRequestInterface $request, + RequestHandlerInterface $handler, + int $shortCodeSegments = 1, + ): ResponseInterface { + $uri = $request->getUri(); + $query = $request->getQueryParams(); + [$potentialShortCode, $extraPath] = $this->resolvePotentialShortCodeAndExtraPath($uri, $shortCodeSegments); + $identifier = ShortUrlIdentifier::fromShortCodeAndDomain($potentialShortCode, $uri->getAuthority()); + + try { + $shortUrl = $this->resolver->resolveEnabledShortUrl($identifier); + $this->requestTracker->trackIfApplicable($shortUrl, $request); + + $longUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $query, $extraPath); + return $this->redirectResponseHelper->buildRedirectResponse($longUrl); + } catch (ShortUrlNotFoundException) { + if ($extraPath === null || ! $this->urlShortenerOptions->multiSegmentSlugsEnabled()) { + return $handler->handle($request); + } + + return $this->tryToResolveRedirect($request, $handler, $shortCodeSegments + 1); + } + } + /** * @return array{0: string, 1: string|null} */ - private function resolvePotentialShortCodeAndExtraPath(UriInterface $uri): array + private function resolvePotentialShortCodeAndExtraPath(UriInterface $uri, int $shortCodeSegments): array { - $pathParts = explode('/', trim($uri->getPath(), '/'), 2); - [$potentialShortCode, $extraPath] = array_pad($pathParts, 2, null); + $parts = explode('/', trim($uri->getPath(), '/')); + $shortCode = array_slice($parts, 0, $shortCodeSegments); + $extraPath = array_slice($parts, $shortCodeSegments); - return [$potentialShortCode, $extraPath === null ? null : sprintf('/%s', $extraPath)]; + return [implode('/', $shortCode), count($extraPath) > 0 ? sprintf('/%s', implode('/', $extraPath)) : null]; } } diff --git a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php index f4e31c5d..4099faea 100644 --- a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php +++ b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php @@ -28,6 +28,8 @@ use Shlinkio\Shlink\Core\ShortUrl\Middleware\ExtraPathRedirectMiddleware; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; +use function str_starts_with; + class ExtraPathRedirectMiddlewareTest extends TestCase { use ProphecyTrait; @@ -74,6 +76,7 @@ class ExtraPathRedirectMiddlewareTest extends TestCase $this->middleware->process($request, $this->handler->reveal()); + $this->handler->handle($request)->shouldHaveBeenCalledOnce(); $this->resolver->resolveEnabledShortUrl(Argument::cetera())->shouldNotHaveBeenCalled(); $this->requestTracker->trackIfApplicable(Argument::cetera())->shouldNotHaveBeenCalled(); $this->redirectionBuilder->buildShortUrlRedirect(Argument::cetera())->shouldNotHaveBeenCalled(); @@ -112,49 +115,83 @@ class ExtraPathRedirectMiddlewareTest extends TestCase ]; } - /** @test */ - public function handlerIsCalledWhenNoShortUrlIsFound(): void - { + /** + * @test + * @dataProvider provideResolves + */ + public function handlerIsCalledWhenNoShortUrlIsFoundAfterExpectedAmountOfIterations( + bool $multiSegmentEnabled, + int $expectedResolveCalls, + ): void { + $this->options->multiSegmentSlugsEnabled = $multiSegmentEnabled; + $type = $this->prophesize(NotFoundType::class); $type->isRegularNotFound()->willReturn(true); + $type->isInvalidShortUrl()->willReturn(true); $request = ServerRequestFactory::fromGlobals()->withAttribute(NotFoundType::class, $type->reveal()) ->withUri(new Uri('/shortCode/bar/baz')); - $resolve = $this->resolver->resolveEnabledShortUrl(Argument::cetera())->willThrow( - ShortUrlNotFoundException::class, - ); + $resolve = $this->resolver->resolveEnabledShortUrl( + Argument::that(fn (ShortUrlIdentifier $identifier) => str_starts_with($identifier->shortCode, 'shortCode')), + )->willThrow(ShortUrlNotFoundException::class); $this->middleware->process($request, $this->handler->reveal()); - $resolve->shouldHaveBeenCalledOnce(); + $resolve->shouldHaveBeenCalledTimes($expectedResolveCalls); $this->requestTracker->trackIfApplicable(Argument::cetera())->shouldNotHaveBeenCalled(); $this->redirectionBuilder->buildShortUrlRedirect(Argument::cetera())->shouldNotHaveBeenCalled(); $this->redirectResponseHelper->buildRedirectResponse(Argument::cetera())->shouldNotHaveBeenCalled(); } - /** @test */ - public function visitIsTrackedAndRedirectIsReturnedWhenShortUrlIsFound(): void - { + /** + * @test + * @dataProvider provideResolves + */ + public function visitIsTrackedAndRedirectIsReturnedWhenShortUrlIsFoundAfterExpectedAmountOfIterations( + bool $multiSegmentEnabled, + int $expectedResolveCalls, + ?string $expectedExtraPath, + ): void { + $this->options->multiSegmentSlugsEnabled = $multiSegmentEnabled; + $type = $this->prophesize(NotFoundType::class); $type->isRegularNotFound()->willReturn(true); + $type->isInvalidShortUrl()->willReturn(true); $request = ServerRequestFactory::fromGlobals()->withAttribute(NotFoundType::class, $type->reveal()) ->withUri(new Uri('https://doma.in/shortCode/bar/baz')); $shortUrl = ShortUrl::withLongUrl(''); - $identifier = ShortUrlIdentifier::fromShortCodeAndDomain('shortCode', 'doma.in'); - - $resolve = $this->resolver->resolveEnabledShortUrl($identifier)->willReturn($shortUrl); - $buildLongUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, [], '/bar/baz')->willReturn( - 'the_built_long_url', + $identifier = Argument::that( + fn (ShortUrlIdentifier $identifier) => str_starts_with($identifier->shortCode, 'shortCode'), ); + + $currentIteration = 1; + $resolve = $this->resolver->resolveEnabledShortUrl($identifier)->will( + function () use ($shortUrl, &$currentIteration, $expectedResolveCalls): ShortUrl { + if ($expectedResolveCalls === $currentIteration) { + return $shortUrl; + } + + $currentIteration++; + throw ShortUrlNotFoundException::fromNotFound(ShortUrlIdentifier::fromShortUrl($shortUrl)); + }, + ); + $buildLongUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, [], $expectedExtraPath) + ->willReturn('the_built_long_url'); $buildResp = $this->redirectResponseHelper->buildRedirectResponse('the_built_long_url')->willReturn( new RedirectResponse(''), ); $this->middleware->process($request, $this->handler->reveal()); - $resolve->shouldHaveBeenCalledOnce(); + $resolve->shouldHaveBeenCalledTimes($expectedResolveCalls); $buildLongUrl->shouldHaveBeenCalledOnce(); $buildResp->shouldHaveBeenCalledOnce(); $this->requestTracker->trackIfApplicable($shortUrl, $request)->shouldHaveBeenCalledOnce(); } + + public function provideResolves(): iterable + { + yield [false, 1, '/bar/baz']; + yield [true, 3, null]; + } }