Merge pull request #792 from acelaya-forks/feature/configurable-redirect

Feature/configurable redirect

CHANGELOG.md

@@ -8,7 +8,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 
 #### Added
 
-* *Nothing*
+* [#746](https://github.com/shlinkio/shlink/issues/746) Allowed to configure the kind of redirect you want to use for your short URLs. You can either set:
+
+    * `302` redirects: Default behavior. Visitors always hit the server.
+    * `301` redirects: Better for SEO. Visitors hit the server the first time and then cache the redirect.
+
+    When selecting 301 redirects, you can also configure the time redirects are cached, to mitigate deviations in stats.
+
+* [#709](https://github.com/shlinkio/shlink/issues/709) Added multi-architecture builds for the docker image.
 
 #### Changed
 
@@ -31,7 +38,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 
 #### Added
 
-* [#709](https://github.com/shlinkio/shlink/issues/709) Added multi-architecture builds for the docker image.
+* *Nothing*
 
 #### Changed
 

composer.json

@@ -52,7 +52,7 @@
         "shlinkio/shlink-common": "^3.1.0",
         "shlinkio/shlink-config": "^1.0",
         "shlinkio/shlink-event-dispatcher": "^1.4",
-        "shlinkio/shlink-installer": "^5.0.0",
+        "shlinkio/shlink-installer": "^5.1.0",
         "shlinkio/shlink-ip-geolocation": "^1.4",
         "symfony/console": "^5.1",
         "symfony/filesystem": "^5.1",

config/autoload/installer.global.php

@@ -37,6 +37,8 @@ return [
             Option\Mercure\MercureJwtSecretConfigOption::class,
             Option\UrlShortener\GeoLiteLicenseKeyConfigOption::class,
             Option\UrlShortener\IpAnonymizationConfigOption::class,
+            Option\UrlShortener\RedirectStatusCodeConfigOption::class,
+            Option\UrlShortener\RedirectCacheLifeTimeConfigOption::class,
         ],
 
         'installation_commands' => [

config/autoload/swoole.local.php.dist

@@ -2,9 +2,6 @@
 
 declare(strict_types=1);
 
-use Laminas\ServiceManager\Factory\InvokableFactory;
-use Mezzio\Swoole\HotCodeReload\FileWatcher\InotifyFileWatcher;
-
 return [
 
     'mezzio-swoole' => [
@@ -13,10 +10,4 @@ return [
         ],
     ],
 
-    'dependencies' => [
-        'factories' => [
-            InotifyFileWatcher::class => InvokableFactory::class,
-        ],
-    ],
-
 ];

config/autoload/url-shortener.global.php

@@ -2,6 +2,8 @@
 
 declare(strict_types=1);
 
+use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_CACHE_LIFETIME;
+use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_STATUS_CODE;
 use const Shlinkio\Shlink\Core\DEFAULT_SHORT_CODES_LENGTH;
 
 return [
@@ -15,6 +17,8 @@ return [
         'anonymize_remote_addr' => true,
         'visits_webhooks' => [],
         'default_short_codes_length' => DEFAULT_SHORT_CODES_LENGTH,
+        'redirect_status_code' => DEFAULT_REDIRECT_STATUS_CODE,
+        'redirect_cache_lifetime' => DEFAULT_REDIRECT_CACHE_LIFETIME,
     ],
 
 ];

config/cli-config.php

@@ -4,11 +4,10 @@ declare(strict_types=1);
 
 use Doctrine\ORM\EntityManager;
 use Doctrine\ORM\Tools\Console\ConsoleRunner;
-use Laminas\ServiceManager\ServiceManager;
 use Psr\Container\ContainerInterface;
 
 return (function () {
-    /** @var ContainerInterface|ServiceManager $container */
+    /** @var ContainerInterface $container */
     $container = include __DIR__ . '/container.php';
     $em = $container->get(EntityManager::class);
 

docker/README.md

@@ -174,6 +174,8 @@ This is the complete list of supported env vars:
 * `MERCURE_INTERNAL_HUB_URL`: An internal URL for a mercure hub. Will be used only when publishing updates to mercure, and does not need to be public. If this is not provided but `MERCURE_PUBLIC_HUB_URL` was, the former one will be used to publish updates.
 * `MERCURE_JWT_SECRET`: The secret key that was provided to the mercure hub server, in order to be able to generate valid JWTs for publishing/subscribing to that server.
 * `ANONYMIZE_REMOTE_ADDR`: Tells if IP addresses from visitors should be obfuscated before storing them in the database. Default value is `true`. **Careful!** Setting this to `false` will make your Shlink instance no longer be in compliance with the GDPR and other similar data protection regulations. 
+* `REDIRECT_STATUS_CODE`: Either **301** or **302**. Used to determine if redirects from short to long URLs should be used with a 301 or 302 status. Defaults to 302.
+* `REDIRECT_CACHE_LIFETIME`: Allows to set the amount of seconds that redirects should be cached when redirect status is 301. Default values is 30. 
 
 An example using all env vars could look like this:
 
@@ -206,6 +208,8 @@ docker run \
     -e "MERCURE_INTERNAL_HUB_URL=http://my-mercure-hub.prod.svc.cluster.local" \
     -e MERCURE_JWT_SECRET=super_secret_key \
     -e ANONYMIZE_REMOTE_ADDR=false \
+    -e REDIRECT_STATUS_CODE=301 \
+    -e REDIRECT_CACHE_LIFETIME=90 \
     shlinkio/shlink:stable
 ```
 
@@ -251,7 +255,9 @@ The whole configuration should have this format, but it can be split into multip
     "mercure_public_hub_url": "https://example.com",
     "mercure_internal_hub_url": "http://my-mercure-hub.prod.svc.cluster.local",
     "mercure_jwt_secret": "super_secret_key",
-    "anonymize_remote_addr": false
+    "anonymize_remote_addr": false,
+    "redirect_status_code": 301,
+    "redirect_cache_lifetime": 90
 }
 ```
 

docker/config/shlink_in_docker.local.php

@@ -11,6 +11,9 @@ use function explode;
 use function Functional\contains;
 use function Shlinkio\Shlink\Common\env;
 
+use const Shlinkio\Shlink\Core\DEFAULT_DELETE_SHORT_URL_THRESHOLD;
+use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_CACHE_LIFETIME;
+use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_STATUS_CODE;
 use const Shlinkio\Shlink\Core\DEFAULT_SHORT_CODES_LENGTH;
 use const Shlinkio\Shlink\Core\MIN_SHORT_CODES_LENGTH;
 
@@ -104,7 +107,7 @@ return [
 
     'delete_short_urls' => [
         'check_visits_threshold' => true,
-        'visits_threshold' => (int) env('DELETE_SHORT_URL_THRESHOLD', 15),
+        'visits_threshold' => (int) env('DELETE_SHORT_URL_THRESHOLD', DEFAULT_DELETE_SHORT_URL_THRESHOLD),
     ],
 
     'entity_manager' => [
@@ -120,6 +123,8 @@ return [
         'anonymize_remote_addr' => (bool) env('ANONYMIZE_REMOTE_ADDR', true),
         'visits_webhooks' => $helper->getVisitsWebhooks(),
         'default_short_codes_length' => $helper->getDefaultShortCodesLength(),
+        'redirect_status_code' => (int) env('REDIRECT_STATUS_CODE', DEFAULT_REDIRECT_STATUS_CODE),
+        'redirect_cache_lifetime' => (int) env('REDIRECT_CACHE_LIFETIME', DEFAULT_REDIRECT_CACHE_LIFETIME),
     ],
 
     'not_found_redirects' => $helper->getNotFoundRedirectsConfig(),

module/Core/config/dependencies.config.php

@@ -76,6 +76,7 @@ return [
             Service\ShortUrl\ShortUrlResolver::class,
             Service\VisitsTracker::class,
             Options\AppOptions::class,
+            Options\UrlShortenerOptions::class,
             'Logger_Shlink',
         ],
         Action\PixelAction::class => [

module/Core/functions/functions.php

@@ -6,12 +6,16 @@ namespace Shlinkio\Shlink\Core;
 
 use Cake\Chronos\Chronos;
 use DateTimeInterface;
+use Fig\Http\Message\StatusCodeInterface;
 use PUGX\Shortid\Factory as ShortIdFactory;
 
 use function sprintf;
 
+const DEFAULT_DELETE_SHORT_URL_THRESHOLD = 15;
 const DEFAULT_SHORT_CODES_LENGTH = 5;
 const MIN_SHORT_CODES_LENGTH = 4;
+const DEFAULT_REDIRECT_STATUS_CODE = StatusCodeInterface::STATUS_FOUND;
+const DEFAULT_REDIRECT_CACHE_LIFETIME = 30;
 const LOCAL_LOCK_FACTORY = 'Shlinkio\Shlink\LocalLockFactory';
 
 function generateRandomShortCode(int $length): string

module/Core/src/Action/RedirectAction.php

@@ -4,18 +4,41 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Core\Action;
 
+use Fig\Http\Message\StatusCodeInterface;
 use Laminas\Diactoros\Response\RedirectResponse;
 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\Options;
+use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface;
+use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface;
 
-class RedirectAction extends AbstractTrackingAction
+use function sprintf;
+
+class RedirectAction extends AbstractTrackingAction implements StatusCodeInterface
 {
+    private Options\UrlShortenerOptions $urlShortenerOptions;
+
+    public function __construct(
+        ShortUrlResolverInterface $urlResolver,
+        VisitsTrackerInterface $visitTracker,
+        Options\AppOptions $appOptions,
+        Options\UrlShortenerOptions $urlShortenerOptions,
+        ?LoggerInterface $logger = null
+    ) {
+        parent::__construct($urlResolver, $visitTracker, $appOptions, $logger);
+        $this->urlShortenerOptions = $urlShortenerOptions;
+    }
+
     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);
+        $statusCode = $this->urlShortenerOptions->redirectStatusCode();
+        $headers = $statusCode === self::STATUS_FOUND ? [] : [
+            'Cache-Control' => sprintf('private,max-age=%s', $this->urlShortenerOptions->redirectCacheLifetime()),
+        ];
+
+        return new RedirectResponse($longUrl, $statusCode, $headers);
     }
 
     protected function createErrorResp(ServerRequestInterface $request, RequestHandlerInterface $handler): Response

module/Core/src/Config/SimplifiedConfigParser.php

@@ -38,6 +38,8 @@ class SimplifiedConfigParser
         'mercure_internal_hub_url' => ['mercure', 'internal_hub_url'],
         'mercure_jwt_secret' => ['mercure', 'jwt_secret'],
         'anonymize_remote_addr' => ['url_shortener', 'anonymize_remote_addr'],
+        'redirect_status_code' => ['url_shortener', 'redirect_status_code'],
+        'redirect_cache_lifetime' => ['url_shortener', 'redirect_cache_lifetime'],
     ];
     private const SIMPLIFIED_CONFIG_SIDE_EFFECTS = [
         'delete_short_url_threshold' => [

module/Core/src/Options/DeleteShortUrlsOptions.php

@@ -6,9 +6,11 @@ namespace Shlinkio\Shlink\Core\Options;
 
 use Laminas\Stdlib\AbstractOptions;
 
+use const Shlinkio\Shlink\Core\DEFAULT_DELETE_SHORT_URL_THRESHOLD;
+
 class DeleteShortUrlsOptions extends AbstractOptions
 {
-    private int $visitsThreshold = 15;
+    private int $visitsThreshold = DEFAULT_DELETE_SHORT_URL_THRESHOLD;
     private bool $checkVisitsThreshold = true;
 
     public function getVisitsThreshold(): int

module/Core/src/Options/UrlShortenerOptions.php

@@ -6,20 +6,53 @@ namespace Shlinkio\Shlink\Core\Options;
 
 use Laminas\Stdlib\AbstractOptions;
 
+use function Functional\contains;
+
+use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_CACHE_LIFETIME;
+use const Shlinkio\Shlink\Core\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;
 
     public function isUrlValidationEnabled(): bool
     {
         return $this->validateUrl;
     }
 
-    protected function setValidateUrl(bool $validateUrl): self
+    protected function setValidateUrl(bool $validateUrl): void
     {
         $this->validateUrl = $validateUrl;
-        return $this;
+    }
+
+    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/test/Action/RedirectActionTest.php

@@ -27,16 +27,19 @@ class RedirectActionTest extends TestCase
     private RedirectAction $action;
     private ObjectProphecy $urlResolver;
     private ObjectProphecy $visitTracker;
+    private Options\UrlShortenerOptions $shortenerOpts;
 
     public function setUp(): void
     {
         $this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class);
         $this->visitTracker = $this->prophesize(VisitsTrackerInterface::class);
+        $this->shortenerOpts = new Options\UrlShortenerOptions();
 
         $this->action = new RedirectAction(
             $this->urlResolver->reveal(),
             $this->visitTracker->reveal(),
             new Options\AppOptions(['disableTrackParam' => 'foobar']),
+            $this->shortenerOpts,
         );
     }
 
@@ -48,8 +51,9 @@ class RedirectActionTest extends TestCase
     {
         $shortCode = 'abc123';
         $shortUrl = new ShortUrl('http://domain.com/foo/bar?some=thing');
-        $shortCodeToUrl = $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode, ''))
-            ->willReturn($shortUrl);
+        $shortCodeToUrl = $this->urlResolver->resolveEnabledShortUrl(
+            new ShortUrlIdentifier($shortCode, ''),
+        )->willReturn($shortUrl);
         $track = $this->visitTracker->track(Argument::cetera())->will(function (): void {
         });
 
@@ -110,4 +114,40 @@ class RedirectActionTest extends TestCase
 
         $track->shouldNotHaveBeenCalled();
     }
+
+    /**
+     * @test
+     * @dataProvider provideRedirectConfigs
+     */
+    public function expectedStatusCodeAndCacheIsReturnedBasedOnConfig(
+        int $configuredStatus,
+        int $configuredLifetime,
+        int $expectedStatus,
+        ?string $expectedCacheControl
+    ): void {
+        $this->shortenerOpts->redirectStatusCode = $configuredStatus;
+        $this->shortenerOpts->redirectCacheLifetime = $configuredLifetime;
+
+        $shortUrl = new ShortUrl('http://domain.com/foo/bar');
+        $shortCode = $shortUrl->getShortCode();
+        $this->urlResolver->resolveEnabledShortUrl(Argument::cetera())->willReturn($shortUrl);
+
+        $request = (new ServerRequest())->withAttribute('shortCode', $shortCode);
+        $response = $this->action->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal());
+
+        $this->assertInstanceOf(Response\RedirectResponse::class, $response);
+        $this->assertEquals($expectedStatus, $response->getStatusCode());
+        $this->assertEquals($response->hasHeader('Cache-Control'), $expectedCacheControl !== null);
+        $this->assertEquals($response->getHeaderLine('Cache-Control'), $expectedCacheControl ?? '');
+    }
+
+    public function provideRedirectConfigs(): iterable
+    {
+        yield 'status 302' => [302, 20, 302, null];
+        yield 'status over 302' => [400, 20, 302, null];
+        yield 'status below 301' => [201, 20, 302, null];
+        yield 'status 301 with valid expiration' => [301, 20, 301, 'private,max-age=20'];
+        yield 'status 301 with zero expiration' => [301, 0, 301, 'private,max-age=30'];
+        yield 'status 301 with negative expiration' => [301, -20, 301, 'private,max-age=30'];
+    }
 }

module/Core/test/Config/SimplifiedConfigParserTest.php

@@ -65,6 +65,8 @@ class SimplifiedConfigParserTest extends TestCase
             'mercure_internal_hub_url' => 'internal_url',
             'mercure_jwt_secret' => 'super_secret_value',
             'anonymize_remote_addr' => false,
+            'redirect_status_code' => 301,
+            'redirect_cache_lifetime' => 90,
         ];
         $expected = [
             'app_options' => [
@@ -94,6 +96,8 @@ class SimplifiedConfigParserTest extends TestCase
                 ],
                 'default_short_codes_length' => 8,
                 'anonymize_remote_addr' => false,
+                'redirect_status_code' => 301,
+                'redirect_cache_lifetime' => 90,
             ],
 
             'delete_short_urls' => [