Moved all logic to redirect to specific URLs when a 404 is found to the NotFoundHandler

module/Core/config/dependencies.config.php

@@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core;
 
 use Doctrine\Common\Cache\Cache;
 use Psr\EventDispatcher\EventDispatcherInterface;
+use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions;
 use Shlinkio\Shlink\Core\Response\NotFoundHandler;
 use Shlinkio\Shlink\PreviewGenerator\Service\PreviewGenerator;
 use Zend\Expressive\Router\RouterInterface;
@@ -40,7 +41,11 @@ return [
     ],
 
     ConfigAbstractFactory::class => [
-        NotFoundHandler::class => [TemplateRendererInterface::class],
+        NotFoundHandler::class => [
+            TemplateRendererInterface::class,
+            NotFoundRedirectOptions::class,
+            'config.router.base_path',
+        ],
 
         Options\AppOptions::class => ['config.app_options'],
         Options\DeleteShortUrlsOptions::class => ['config.delete_short_urls'],
@@ -58,7 +63,6 @@ return [
             Service\UrlShortener::class,
             Service\VisitsTracker::class,
             Options\AppOptions::class,
-            Options\NotFoundRedirectOptions::class,
             'Logger_Shlink',
         ],
         Action\PixelAction::class => [

module/Core/src/Action/RedirectAction.php

@@ -7,11 +7,8 @@ namespace Shlinkio\Shlink\Core\Action;
 use Psr\Http\Message\ResponseInterface as Response;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\RequestHandlerInterface;
-use Psr\Log\LoggerInterface;
 use Shlinkio\Shlink\Core\Action\Util\ErrorResponseBuilderTrait;
 use Shlinkio\Shlink\Core\Options;
-use Shlinkio\Shlink\Core\Service\UrlShortenerInterface;
-use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface;
 use Zend\Diactoros\Response\RedirectResponse;
 
 class RedirectAction extends AbstractTrackingAction
@@ -21,17 +18,6 @@ class RedirectAction extends AbstractTrackingAction
     /** @var Options\NotFoundRedirectOptions */
     private $redirectOptions;
 
-    public function __construct(
-        UrlShortenerInterface $urlShortener,
-        VisitsTrackerInterface $visitTracker,
-        Options\AppOptions $appOptions,
-        Options\NotFoundRedirectOptions $redirectOptions,
-        ?LoggerInterface $logger = null
-    ) {
-        parent::__construct($urlShortener, $visitTracker, $appOptions, $logger);
-        $this->redirectOptions = $redirectOptions;
-    }
-
     protected function createSuccessResp(string $longUrl): Response
     {
         // Return a redirect response to the long URL.
@@ -39,14 +25,8 @@ class RedirectAction extends AbstractTrackingAction
         return new RedirectResponse($longUrl);
     }
 
-    protected function createErrorResp(
-        ServerRequestInterface $request,
-        RequestHandlerInterface $handler
-    ): Response {
-        if ($this->redirectOptions->hasInvalidShortUrlRedirect()) {
-            return new RedirectResponse($this->redirectOptions->getInvalidShortUrlRedirect());
-        }
-
+    protected function createErrorResp(ServerRequestInterface $request, RequestHandlerInterface $handler): Response
+    {
         return $this->buildErrorResponse($request, $handler);
     }
 }

module/Core/src/Response/NotFoundHandler.php

@@ -5,15 +5,19 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Core\Response;
 
 use Fig\Http\Message\StatusCodeInterface;
+use InvalidArgumentException;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\RequestHandlerInterface;
+use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions;
 use Zend\Diactoros\Response;
+use Zend\Expressive\Router\RouteResult;
 use Zend\Expressive\Template\TemplateRendererInterface;
 
 use function array_shift;
 use function explode;
 use function Functional\contains;
+use function rtrim;
 
 class NotFoundHandler implements RequestHandlerInterface
 {
@@ -23,11 +27,21 @@ class NotFoundHandler implements RequestHandlerInterface
     private $renderer;
     /** @var string */
     private $defaultTemplate;
+    /** @var NotFoundRedirectOptions */
+    private $redirectOptions;
+    /** @var string */
+    private $shlinkBasePath;
 
-    public function __construct(TemplateRendererInterface $renderer, string $defaultTemplate = 'ShlinkCore::error/404')
-    {
+    public function __construct(
+        TemplateRendererInterface $renderer,
+        NotFoundRedirectOptions $redirectOptions,
+        string $shlinkBasePath,
+        string $defaultTemplate = 'ShlinkCore::error/404'
+    ) {
         $this->renderer = $renderer;
         $this->defaultTemplate = $defaultTemplate;
+        $this->redirectOptions = $redirectOptions;
+        $this->shlinkBasePath = $shlinkBasePath;
     }
 
     /**
@@ -36,10 +50,15 @@ class NotFoundHandler implements RequestHandlerInterface
      * @param ServerRequestInterface $request
      *
      * @return ResponseInterface
-     * @throws \InvalidArgumentException
+     * @throws InvalidArgumentException
      */
     public function handle(ServerRequestInterface $request): ResponseInterface
     {
+        $redirectResponse = $this->createRedirectResponse($request);
+        if ($redirectResponse !== null) {
+            return $redirectResponse;
+        }
+
         $accepts = explode(',', $request->getHeaderLine('Accept'));
         $accept = array_shift($accepts);
         $status = StatusCodeInterface::STATUS_NOT_FOUND;
@@ -55,4 +74,29 @@ class NotFoundHandler implements RequestHandlerInterface
         $notFoundTemplate = $request->getAttribute(self::NOT_FOUND_TEMPLATE, $this->defaultTemplate);
         return new Response\HtmlResponse($this->renderer->render($notFoundTemplate), $status);
     }
+
+    private function createRedirectResponse(ServerRequestInterface $request): ?ResponseInterface
+    {
+        /** @var RouteResult $routeResult */
+        $routeResult = $request->getAttribute(RouteResult::class, RouteResult::fromRouteFailure(null));
+        $isBaseUrl = rtrim($request->getUri()->getPath(), '/') === $this->shlinkBasePath;
+
+        if ($isBaseUrl && $this->redirectOptions->hasBaseUrlRedirect()) {
+            return new Response\RedirectResponse($this->redirectOptions->getBaseUrlRedirect());
+        }
+
+        if (!$isBaseUrl && $routeResult->isFailure() && $this->redirectOptions->hasRegular404Redirect()) {
+            return new Response\RedirectResponse($this->redirectOptions->getRegular404Redirect());
+        }
+
+        if (
+            $routeResult->isSuccess() &&
+            $routeResult->getMatchedRouteName() === 'long-url-redirect' &&
+            $this->redirectOptions->hasInvalidShortUrlRedirect()
+        ) {
+            return new Response\RedirectResponse($this->redirectOptions->getInvalidShortUrlRedirect());
+        }
+
+        return null;
+    }
 }

module/Core/test/Action/RedirectActionTest.php

@@ -25,20 +25,16 @@ class RedirectActionTest extends TestCase
     private $urlShortener;
     /** @var ObjectProphecy */
     private $visitTracker;
-    /** @var Options\NotFoundRedirectOptions */
-    private $redirectOptions;
 
     public function setUp(): void
     {
         $this->urlShortener = $this->prophesize(UrlShortener::class);
         $this->visitTracker = $this->prophesize(VisitsTracker::class);
-        $this->redirectOptions = new Options\NotFoundRedirectOptions();
 
         $this->action = new RedirectAction(
             $this->urlShortener->reveal(),
             $this->visitTracker->reveal(),
-            new Options\AppOptions(['disableTrackParam' => 'foobar']),
-            $this->redirectOptions
+            new Options\AppOptions(['disableTrackParam' => 'foobar'])
         );
     }
 
@@ -78,28 +74,6 @@ class RedirectActionTest extends TestCase
         $handle->shouldHaveBeenCalledOnce();
     }
 
-    /** @test */
-    public function redirectToCustomUrlIsReturnedIfConfiguredSoAndShortUrlIsNotFound(): void
-    {
-        $shortCode = 'abc123';
-        $shortCodeToUrl = $this->urlShortener->shortCodeToUrl($shortCode, '')->willThrow(
-            EntityDoesNotExistException::class
-        );
-
-        $handler = $this->prophesize(RequestHandlerInterface::class);
-        $handle = $handler->handle(Argument::any())->willReturn(new Response());
-
-        $this->redirectOptions->invalidShortUrl = 'https://shlink.io';
-
-        $request = (new ServerRequest())->withAttribute('shortCode', $shortCode);
-        $resp = $this->action->process($request, $handler->reveal());
-
-        $this->assertEquals(302, $resp->getStatusCode());
-        $this->assertEquals('https://shlink.io', $resp->getHeaderLine('Location'));
-        $shortCodeToUrl->shouldHaveBeenCalledOnce();
-        $handle->shouldNotHaveBeenCalled();
-    }
-
     /** @test */
     public function visitIsNotTrackedIfDisableParamIsProvided(): void
     {

module/Core/test/Response/NotFoundHandlerTest.php

@@ -7,9 +7,16 @@ namespace ShlinkioTest\Shlink\Core\Response;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
+use Psr\Http\Message\ServerRequestInterface;
+use Psr\Http\Server\MiddlewareInterface;
+use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions;
 use Shlinkio\Shlink\Core\Response\NotFoundHandler;
 use Zend\Diactoros\Response;
 use Zend\Diactoros\ServerRequest;
+use Zend\Diactoros\ServerRequestFactory;
+use Zend\Diactoros\Uri;
+use Zend\Expressive\Router\Route;
+use Zend\Expressive\Router\RouteResult;
 use Zend\Expressive\Template\TemplateRendererInterface;
 
 class NotFoundHandlerTest extends TestCase
@@ -18,11 +25,15 @@ class NotFoundHandlerTest extends TestCase
     private $delegate;
     /** @var ObjectProphecy */
     private $renderer;
+    /** @var NotFoundRedirectOptions */
+    private $redirectOptions;
 
     public function setUp(): void
     {
         $this->renderer = $this->prophesize(TemplateRendererInterface::class);
-        $this->delegate = new NotFoundHandler($this->renderer->reveal());
+        $this->redirectOptions = new NotFoundRedirectOptions();
+
+        $this->delegate = new NotFoundHandler($this->renderer->reveal(), $this->redirectOptions, '');
     }
 
     /**
@@ -47,4 +58,55 @@ class NotFoundHandlerTest extends TestCase
         yield 'application/x-json' => [Response\JsonResponse::class, 'application/x-json', 0];
         yield 'text/html' => [Response\HtmlResponse::class, 'text/html', 1];
     }
+
+    /**
+     * @test
+     * @dataProvider provideRedirects
+     */
+    public function expectedRedirectionIsReturnedDependingOnTheCase(
+        ServerRequestInterface $request,
+        string $expectedRedirectTo
+    ): void {
+        $this->redirectOptions->invalidShortUrl = 'invalidShortUrl';
+        $this->redirectOptions->regular404 = 'regular404';
+        $this->redirectOptions->baseUrl = 'baseUrl';
+
+        $resp = $this->delegate->handle($request);
+
+        $this->assertInstanceOf(Response\RedirectResponse::class, $resp);
+        $this->assertEquals($expectedRedirectTo, $resp->getHeaderLine('Location'));
+        $this->renderer->render(Argument::cetera())->shouldNotHaveBeenCalled();
+    }
+
+    public function provideRedirects(): iterable
+    {
+        yield 'base URL with trailing slash' => [
+            ServerRequestFactory::fromGlobals()->withUri(new Uri('/')),
+            'baseUrl',
+        ];
+        yield 'base URL without trailing slash' => [
+            ServerRequestFactory::fromGlobals()->withUri(new Uri('')),
+            'baseUrl',
+        ];
+        yield 'regular 404' => [
+            ServerRequestFactory::fromGlobals()->withUri(new Uri('/foo/bar')),
+            'regular404',
+        ];
+        yield 'invalid short URL' => [
+            ServerRequestFactory::fromGlobals()
+                ->withAttribute(
+                    RouteResult::class,
+                    RouteResult::fromRoute(
+                        new Route(
+                            '',
+                            $this->prophesize(MiddlewareInterface::class)->reveal(),
+                            ['GET'],
+                            'long-url-redirect'
+                        )
+                    )
+                )
+                ->withUri(new Uri('/abc123')),
+            'invalidShortUrl',
+        ];
+    }
 }