Merge pull request #252 from acelaya/feature/redirect-not-found

Feature/redirect not found

.travis.yml

@@ -26,11 +26,11 @@ install:
 
 script:
   - mkdir build
-  - composer ci
+  - composer check
 
 after_success:
   - rm -f build/clover.xml
-  - vendor/bin/phpcov merge build --clover build/clover.xml
+  - phpdbg -qrr vendor/bin/phpcov merge build --clover build/clover.xml
   - wget https://scrutinizer-ci.com/ocular.phar
   - php ocular.phar code-coverage:upload --format=php-clover build/clover.xml
 

CHANGELOG.md

@@ -8,7 +8,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 #### Added
 
-* *Nothing*
+* [#236](https://github.com/shlinkio/shlink/issues/236) Added option to define a redirection to a custom URL when a user hits an invalid short URL.
+
+    It only affects URLs matched as "short URL" where the short code is invalid, not any 404 that happens in the app. For example, a request to the path `/foo/bar` will keep returning a 404.
+
+    This new option will be asked by the installer both for new shlink installations and for any previous shlink version which is updated.
 
 #### Changed
 

composer.json

@@ -88,17 +88,15 @@
     },
     "scripts": {
         "check": [
-            "@cs",
-            "@stan",
-            "@test",
-            "@infect"
-        ],
-        "ci": [
             "@cs",
             "@stan",
             "@test:ci",
             "@infect:ci"
         ],
+        "ci": [
+            "echo \"This command is DEPRECATED. Use check instead\"",
+            "@check"
+        ],
 
         "cs": "phpcs",
         "cs:fix": "phpcbf",

config/autoload/url-shortener.global.php

@@ -13,6 +13,10 @@ return [
         ],
         'shortcode_chars' => env('SHORTCODE_CHARS', UrlShortener::DEFAULT_CHARS),
         'validate_url' => true,
+        'not_found_short_url' => [
+            'enable_redirection' => false,
+            'redirect_to' => null,
+        ],
     ],
 
 ];

module/Core/config/dependencies.config.php

@@ -16,8 +16,9 @@ return [
 
     'dependencies' => [
         'factories' => [
-            Options\AppOptions::class => Options\AppOptionsFactory::class,
+            Options\AppOptions::class => ConfigAbstractFactory::class,
-            Options\DeleteShortUrlsOptions::class => Options\DeleteShortUrlsOptionsFactory::class,
+            Options\DeleteShortUrlsOptions::class => ConfigAbstractFactory::class,
+            Options\NotFoundShortUrlOptions::class => ConfigAbstractFactory::class,
             NotFoundHandler::class => ConfigAbstractFactory::class,
 
             // Services
@@ -40,6 +41,10 @@ return [
     ConfigAbstractFactory::class => [
         NotFoundHandler::class => [TemplateRendererInterface::class],
 
+        Options\AppOptions::class => ['config.app_options'],
+        Options\DeleteShortUrlsOptions::class => ['config.delete_short_urls'],
+        Options\NotFoundShortUrlOptions::class => ['config.url_shortener.not_found_short_url'],
+
         // Services
         Service\UrlShortener::class => [
             'httpClient',
@@ -58,6 +63,7 @@ return [
             Service\UrlShortener::class,
             Service\VisitsTracker::class,
             Options\AppOptions::class,
+            Options\NotFoundShortUrlOptions::class,
             'Logger_Shlink',
         ],
         Action\PixelAction::class => [

module/Core/src/Action/AbstractTrackingAction.php

@@ -9,7 +9,6 @@ use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
-use Shlinkio\Shlink\Core\Action\Util\ErrorResponseBuilderTrait;
 use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
 use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
 use Shlinkio\Shlink\Core\Model\Visitor;
@@ -20,8 +19,6 @@ use function array_key_exists;
 
 abstract class AbstractTrackingAction implements MiddlewareInterface
 {
-    use ErrorResponseBuilderTrait;
-
     /**
      * @var UrlShortenerInterface
      */
@@ -74,12 +71,17 @@ abstract class AbstractTrackingAction implements MiddlewareInterface
                 $this->visitTracker->track($shortCode, Visitor::fromRequest($request));
             }
 
-            return $this->createResp($url->getLongUrl());
+            return $this->createSuccessResp($url->getLongUrl());
         } catch (InvalidShortCodeException | EntityDoesNotExistException $e) {
             $this->logger->warning('An error occurred while tracking short code. {e}', ['e' => $e]);
-            return $this->buildErrorResponse($request, $handler);
+            return $this->createErrorResp($request, $handler);
         }
     }
 
-    abstract protected function createResp(string $longUrl): ResponseInterface;
+    abstract protected function createSuccessResp(string $longUrl): ResponseInterface;
+
+    abstract protected function createErrorResp(
+        ServerRequestInterface $request,
+        RequestHandlerInterface $handler
+    ): ResponseInterface;
 }

module/Core/src/Action/PixelAction.php

@@ -4,12 +4,21 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Core\Action;
 
 use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\ServerRequestInterface;
+use Psr\Http\Server\RequestHandlerInterface;
 use Shlinkio\Shlink\Common\Response\PixelResponse;
 
 class PixelAction extends AbstractTrackingAction
 {
-    protected function createResp(string $longUrl): ResponseInterface
+    protected function createSuccessResp(string $longUrl): ResponseInterface
     {
         return new PixelResponse();
     }
+
+    protected function createErrorResp(
+        ServerRequestInterface $request,
+        RequestHandlerInterface $handler
+    ): ResponseInterface {
+        return new PixelResponse();
+    }
 }

module/Core/src/Action/RedirectAction.php

@@ -4,14 +4,50 @@ declare(strict_types=1);
 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
 {
-    protected function createResp(string $longUrl): Response
+    use ErrorResponseBuilderTrait;
+
+    /**
+     * @var Options\NotFoundShortUrlOptions
+     */
+    private $notFoundOptions;
+
+    public function __construct(
+        UrlShortenerInterface $urlShortener,
+        VisitsTrackerInterface $visitTracker,
+        Options\AppOptions $appOptions,
+        Options\NotFoundShortUrlOptions $notFoundOptions,
+        LoggerInterface $logger = null
+    ) {
+        parent::__construct($urlShortener, $visitTracker, $appOptions, $logger);
+        $this->notFoundOptions = $notFoundOptions;
+    }
+
+    protected function createSuccessResp(string $longUrl): Response
     {
         // Return a redirect response to the long URL.
         // Use a temporary redirect to make sure browsers always hit the server for analytics purposes
         return new RedirectResponse($longUrl);
     }
+
+    protected function createErrorResp(
+        ServerRequestInterface $request,
+        RequestHandlerInterface $handler
+    ): Response {
+        if ($this->notFoundOptions->isRedirectionEnabled()) {
+            return new RedirectResponse($this->notFoundOptions->getRedirectTo());
+        }
+
+        return $this->buildErrorResponse($request, $handler);
+    }
 }

module/Core/src/Options/AppOptionsFactory.php

@@ -1,31 +0,0 @@
-<?php
-declare(strict_types=1);
-
-namespace Shlinkio\Shlink\Core\Options;
-
-use Interop\Container\ContainerInterface;
-use Interop\Container\Exception\ContainerException;
-use Zend\ServiceManager\Exception\ServiceNotCreatedException;
-use Zend\ServiceManager\Exception\ServiceNotFoundException;
-use Zend\ServiceManager\Factory\FactoryInterface;
-
-class AppOptionsFactory implements FactoryInterface
-{
-    /**
-     * Create an object
-     *
-     * @param  ContainerInterface $container
-     * @param  string $requestedName
-     * @param  null|array $options
-     * @return object
-     * @throws ServiceNotFoundException if unable to resolve the service.
-     * @throws ServiceNotCreatedException if an exception is raised when
-     *     creating a service.
-     * @throws ContainerException if any other error occurs
-     */
-    public function __invoke(ContainerInterface $container, $requestedName, array $options = null)
-    {
-        $config = $container->has('config') ? $container->get('config') : [];
-        return new AppOptions($config['app_options'] ?? []);
-    }
-}

module/Core/src/Options/DeleteShortUrlsOptionsFactory.php

@@ -1,31 +0,0 @@
-<?php
-declare(strict_types=1);
-
-namespace Shlinkio\Shlink\Core\Options;
-
-use Interop\Container\ContainerInterface;
-use Interop\Container\Exception\ContainerException;
-use Zend\ServiceManager\Exception\ServiceNotCreatedException;
-use Zend\ServiceManager\Exception\ServiceNotFoundException;
-use Zend\ServiceManager\Factory\FactoryInterface;
-
-class DeleteShortUrlsOptionsFactory implements FactoryInterface
-{
-    /**
-     * Create an object
-     *
-     * @param  ContainerInterface $container
-     * @param  string $requestedName
-     * @param  null|array $options
-     * @return object
-     * @throws ServiceNotFoundException if unable to resolve the service.
-     * @throws ServiceNotCreatedException if an exception is raised when
-     *     creating a service.
-     * @throws ContainerException if any other error occurs
-     */
-    public function __invoke(ContainerInterface $container, $requestedName, array $options = null)
-    {
-        $config = $container->has('config') ? $container->get('config') : [];
-        return new DeleteShortUrlsOptions($config['delete_short_urls'] ?? []);
-    }
-}

module/Core/src/Options/NotFoundShortUrlOptions.php

@@ -0,0 +1,40 @@
+<?php
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Options;
+
+use Zend\Stdlib\AbstractOptions;
+
+class NotFoundShortUrlOptions extends AbstractOptions
+{
+    /**
+     * @var bool
+     */
+    private $enableRedirection = false;
+    /**
+     * @var string|null
+     */
+    private $redirectTo;
+
+    public function isRedirectionEnabled(): bool
+    {
+        return $this->enableRedirection;
+    }
+
+    protected function setEnableRedirection(bool $enableRedirection = true): self
+    {
+        $this->enableRedirection = $enableRedirection;
+        return $this;
+    }
+
+    public function getRedirectTo(): string
+    {
+        return $this->redirectTo ?? '';
+    }
+
+    protected function setRedirectTo(?string $redirectTo): self
+    {
+        $this->redirectTo = $redirectTo;
+        return $this;
+    }
+}

module/Core/test/Action/RedirectActionTest.php

@@ -5,13 +5,12 @@ namespace ShlinkioTest\Shlink\Core\Action;
 
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
-use Prophecy\Prophecy\MethodProphecy;
 use Prophecy\Prophecy\ObjectProphecy;
 use Psr\Http\Server\RequestHandlerInterface;
 use Shlinkio\Shlink\Core\Action\RedirectAction;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
-use Shlinkio\Shlink\Core\Options\AppOptions;
+use Shlinkio\Shlink\Core\Options;
 use Shlinkio\Shlink\Core\Service\UrlShortener;
 use Shlinkio\Shlink\Core\Service\VisitsTracker;
 use ShlinkioTest\Shlink\Common\Util\TestUtils;
@@ -23,25 +22,31 @@ class RedirectActionTest extends TestCase
     /**
      * @var RedirectAction
      */
-    protected $action;
+    private $action;
     /**
      * @var ObjectProphecy
      */
-    protected $urlShortener;
+    private $urlShortener;
     /**
      * @var ObjectProphecy
      */
-    protected $visitTracker;
+    private $visitTracker;
+    /**
+     * @var Options\NotFoundShortUrlOptions
+     */
+    private $notFoundOptions;
 
     public function setUp()
     {
         $this->urlShortener = $this->prophesize(UrlShortener::class);
         $this->visitTracker = $this->prophesize(VisitsTracker::class);
+        $this->notFoundOptions = new Options\NotFoundShortUrlOptions();
 
         $this->action = new RedirectAction(
             $this->urlShortener->reveal(),
             $this->visitTracker->reveal(),
-            new AppOptions(['disableTrackParam' => 'foobar'])
+            new Options\AppOptions(['disableTrackParam' => 'foobar']),
+            $this->notFoundOptions
         );
     }
 
@@ -76,14 +81,38 @@ class RedirectActionTest extends TestCase
                                                        ->shouldBeCalledTimes(1);
         $this->visitTracker->track(Argument::cetera())->shouldNotBeCalled();
 
-        $delegate = $this->prophesize(RequestHandlerInterface::class);
+        $handler = $this->prophesize(RequestHandlerInterface::class);
-        /** @var MethodProphecy $process */
+        $handle = $handler->handle(Argument::any())->willReturn(new Response());
-        $process = $delegate->handle(Argument::any())->willReturn(new Response());
 
         $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode);
-        $this->action->process($request, $delegate->reveal());
+        $this->action->process($request, $handler->reveal());
 
-        $process->shouldHaveBeenCalledTimes(1);
+        $handle->shouldHaveBeenCalledTimes(1);
+    }
+
+    /**
+     * @test
+     */
+    public function redirectToCustomUrlIsReturnedIfConfiguredSoAndShortUrlIsNotFound()
+    {
+        $shortCode = 'abc123';
+        $shortCodeToUrl = $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(
+            EntityDoesNotExistException::class
+        );
+
+        $handler = $this->prophesize(RequestHandlerInterface::class);
+        $handle = $handler->handle(Argument::any())->willReturn(new Response());
+
+        $this->notFoundOptions->enableRedirection = true;
+        $this->notFoundOptions->redirectTo = 'https://shlink.io';
+
+        $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode);
+        $resp = $this->action->process($request, $handler->reveal());
+
+        $this->assertEquals(302, $resp->getStatusCode());
+        $this->assertEquals('https://shlink.io', $resp->getHeaderLine('Location'));
+        $shortCodeToUrl->shouldHaveBeenCalledTimes(1);
+        $handle->shouldNotHaveBeenCalled();
     }
 
     /**

module/Core/test/Options/AppOptionsFactoryTest.php

@@ -1,31 +0,0 @@
-<?php
-declare(strict_types=1);
-
-namespace ShlinkioTest\Shlink\Core\Options;
-
-use PHPUnit\Framework\TestCase;
-use Shlinkio\Shlink\Core\Options\AppOptions;
-use Shlinkio\Shlink\Core\Options\AppOptionsFactory;
-use Zend\ServiceManager\ServiceManager;
-
-class AppOptionsFactoryTest extends TestCase
-{
-    /**
-     * @var AppOptionsFactory
-     */
-    protected $factory;
-
-    public function setUp()
-    {
-        $this->factory = new AppOptionsFactory();
-    }
-
-    /**
-     * @test
-     */
-    public function serviceIsCreated()
-    {
-        $instance = $this->factory->__invoke(new ServiceManager([]), '');
-        $this->assertInstanceOf(AppOptions::class, $instance);
-    }
-}

module/Installer/src/Config/Plugin/UrlShortenerConfigCustomizer.php

@@ -19,11 +19,15 @@ class UrlShortenerConfigCustomizer implements ConfigCustomizerInterface
     public const HOSTNAME = 'HOSTNAME';
     public const CHARS = 'CHARS';
     public const VALIDATE_URL = 'VALIDATE_URL';
+    public const ENABLE_NOT_FOUND_REDIRECTION = 'ENABLE_NOT_FOUND_REDIRECTION';
+    public const NOT_FOUND_REDIRECT_TO = 'NOT_FOUND_REDIRECT_TO';
     private const EXPECTED_KEYS = [
         self::SCHEMA,
         self::HOSTNAME,
         self::CHARS,
         self::VALIDATE_URL,
+        self::ENABLE_NOT_FOUND_REDIRECTION,
+        self::NOT_FOUND_REDIRECT_TO,
     ];
 
     public function process(SymfonyStyle $io, CustomizableAppConfig $appConfig): void
@@ -38,6 +42,11 @@ class UrlShortenerConfigCustomizer implements ConfigCustomizerInterface
 
         $io->title('URL SHORTENER');
         foreach ($keysToAskFor as $key) {
+            // Skip not found redirect URL when the user decided not to redirect
+            if ($key === self::NOT_FOUND_REDIRECT_TO && ! $urlShortener[self::ENABLE_NOT_FOUND_REDIRECTION]) {
+                continue;
+            }
+
             $urlShortener[$key] = $this->ask($io, $key);
         }
         $appConfig->setUrlShortener($urlShortener);
@@ -60,6 +69,18 @@ class UrlShortenerConfigCustomizer implements ConfigCustomizerInterface
                 ) ?: str_shuffle(UrlShortener::DEFAULT_CHARS);
             case self::VALIDATE_URL:
                 return $io->confirm('Do you want to validate long urls by 200 HTTP status code on response');
+            case self::ENABLE_NOT_FOUND_REDIRECTION:
+                return $io->confirm(
+                    'Do you want to enable a redirection to a custom URL when a user hits an invalid short URL? ' .
+                    '(If not enabled, the user will see a default "404 not found" page)',
+                    false
+                );
+            case self::NOT_FOUND_REDIRECT_TO:
+                return $this->askRequired(
+                    $io,
+                    'redirect URL',
+                    'Custom URL to redirect to when a user hits an invalid short URL'
+                );
         }
 
         return '';

module/Installer/src/Model/CustomizableAppConfig.php

@@ -146,6 +146,16 @@ final class CustomizableAppConfig implements ArraySerializableInterface
             UrlShortenerConfigCustomizer::HOSTNAME => ['url_shortener', 'domain', 'hostname'],
             UrlShortenerConfigCustomizer::CHARS => ['url_shortener', 'shortcode_chars'],
             UrlShortenerConfigCustomizer::VALIDATE_URL => ['url_shortener', 'validate_url'],
+            UrlShortenerConfigCustomizer::ENABLE_NOT_FOUND_REDIRECTION => [
+                'url_shortener',
+                'not_found_short_url',
+                'enable_redirection',
+            ],
+            UrlShortenerConfigCustomizer::NOT_FOUND_REDIRECT_TO => [
+                'url_shortener',
+                'not_found_short_url',
+                'redirect_to',
+            ],
         ], $pathCollection));
     }
 
@@ -191,6 +201,11 @@ final class CustomizableAppConfig implements ArraySerializableInterface
                 ],
                 'shortcode_chars' => $this->urlShortener[UrlShortenerConfigCustomizer::CHARS] ?? '',
                 'validate_url' => $this->urlShortener[UrlShortenerConfigCustomizer::VALIDATE_URL] ?? true,
+                'not_found_short_url' => [
+                    'enable_redirection' =>
+                        $this->urlShortener[UrlShortenerConfigCustomizer::ENABLE_NOT_FOUND_REDIRECTION] ?? false,
+                    'redirect_to' => $this->urlShortener[UrlShortenerConfigCustomizer::NOT_FOUND_REDIRECT_TO] ?? null,
+                ],
             ],
         ];
 

module/Installer/test/Config/Plugin/UrlShortenerConfigCustomizerTest.php

@@ -46,10 +46,12 @@ class UrlShortenerConfigCustomizerTest extends TestCase
             'HOSTNAME' => 'asked',
             'CHARS' => 'asked',
             'VALIDATE_URL' => true,
+            'ENABLE_NOT_FOUND_REDIRECTION' => true,
+            'NOT_FOUND_REDIRECT_TO' => 'asked',
         ], $config->getUrlShortener());
-        $ask->shouldHaveBeenCalledTimes(2);
+        $ask->shouldHaveBeenCalledTimes(3);
         $choice->shouldHaveBeenCalledTimes(1);
-        $confirm->shouldHaveBeenCalledTimes(1);
+        $confirm->shouldHaveBeenCalledTimes(2);
     }
 
     /**
@@ -64,6 +66,8 @@ class UrlShortenerConfigCustomizerTest extends TestCase
         $config->setUrlShortener([
             'SCHEMA' => 'foo',
             'HOSTNAME' => 'foo',
+            'ENABLE_NOT_FOUND_REDIRECTION' => true,
+            'NOT_FOUND_REDIRECT_TO' => 'foo',
         ]);
 
         $this->plugin->process($this->io->reveal(), $config);
@@ -73,6 +77,8 @@ class UrlShortenerConfigCustomizerTest extends TestCase
             'HOSTNAME' => 'foo',
             'CHARS' => 'asked',
             'VALIDATE_URL' => false,
+            'ENABLE_NOT_FOUND_REDIRECTION' => true,
+            'NOT_FOUND_REDIRECT_TO' => 'foo',
         ], $config->getUrlShortener());
         $choice->shouldNotHaveBeenCalled();
         $ask->shouldHaveBeenCalledTimes(1);
@@ -94,6 +100,8 @@ class UrlShortenerConfigCustomizerTest extends TestCase
             'HOSTNAME' => 'foo',
             'CHARS' => 'foo',
             'VALIDATE_URL' => true,
+            'ENABLE_NOT_FOUND_REDIRECTION' => true,
+            'NOT_FOUND_REDIRECT_TO' => 'foo',
         ]);
 
         $this->plugin->process($this->io->reveal(), $config);
@@ -103,9 +111,41 @@ class UrlShortenerConfigCustomizerTest extends TestCase
             'HOSTNAME' => 'foo',
             'CHARS' => 'foo',
             'VALIDATE_URL' => true,
+            'ENABLE_NOT_FOUND_REDIRECTION' => true,
+            'NOT_FOUND_REDIRECT_TO' => 'foo',
         ], $config->getUrlShortener());
         $choice->shouldNotHaveBeenCalled();
         $ask->shouldNotHaveBeenCalled();
         $confirm->shouldNotHaveBeenCalled();
     }
+
+    /**
+     * @test
+     */
+    public function redirectUrlOptionIsNotAskedIfAnswerToPreviousQuestionIsNo()
+    {
+        $ask = $this->io->ask(Argument::cetera())->willReturn('asked');
+        $confirm = $this->io->confirm(Argument::cetera())->willReturn(false);
+
+        $config = new CustomizableAppConfig();
+        $config->setUrlShortener([
+            'SCHEMA' => 'foo',
+            'HOSTNAME' => 'foo',
+            'CHARS' => 'foo',
+            'VALIDATE_URL' => true,
+        ]);
+
+        $this->plugin->process($this->io->reveal(), $config);
+
+        $this->assertTrue($config->hasUrlShortener());
+        $this->assertEquals([
+            'SCHEMA' => 'foo',
+            'HOSTNAME' => 'foo',
+            'CHARS' => 'foo',
+            'VALIDATE_URL' => true,
+            'ENABLE_NOT_FOUND_REDIRECTION' => false,
+        ], $config->getUrlShortener());
+        $ask->shouldNotHaveBeenCalled();
+        $confirm->shouldHaveBeenCalledTimes(1);
+    }
 }