Merge pull request #721 from acelaya-forks/feature/qr-code-links

Feature/qr code links

CHANGELOG.md

@@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org).
 
-## [Unreleased]
+## 2.1.3 - 2020-04-09
 
 #### Added
 
@@ -28,6 +28,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 * [#711](https://github.com/shlinkio/shlink/issues/711) Fixed `HEAD` requests returning a duplicated `Content-Length` header.
 * [#716](https://github.com/shlinkio/shlink/issues/716) Fixed Twitter not properly displaying preview for final long URL.
 * [#717](https://github.com/shlinkio/shlink/issues/717) Fixed DB connection expiring on task workers when using swoole.
+* [#705](https://github.com/shlinkio/shlink/issues/705) Fixed how the short URL domain is inferred when generating QR codes, making sure the configured domain is respected even if the request is performed using a different one, and only when a custom domain is used, then that one is used instead.
 
 
 ## 2.1.2 - 2020-03-29

module/Core/config/dependencies.config.php

@@ -4,9 +4,7 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Core;
 
-use Doctrine\Common\Cache\Cache;
 use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory;
-use Mezzio\Router\RouterInterface;
 use Mezzio\Template\TemplateRendererInterface;
 use Psr\EventDispatcher\EventDispatcherInterface;
 use Shlinkio\Shlink\Core\Domain\Resolver;
@@ -39,8 +37,6 @@ return [
             Action\PixelAction::class => ConfigAbstractFactory::class,
             Action\QrCodeAction::class => ConfigAbstractFactory::class,
 
-            Middleware\QrCodeCacheMiddleware::class => ConfigAbstractFactory::class,
-
             Resolver\PersistenceDomainResolver::class => ConfigAbstractFactory::class,
         ],
     ],
@@ -81,13 +77,11 @@ return [
             'Logger_Shlink',
         ],
         Action\QrCodeAction::class => [
-            RouterInterface::class,
             Service\ShortUrl\ShortUrlResolver::class,
+            'config.url_shortener.domain',
             'Logger_Shlink',
         ],
 
-        Middleware\QrCodeCacheMiddleware::class => [Cache::class],
-
         Resolver\PersistenceDomainResolver::class => ['em'],
     ],
 

module/Core/config/routes.config.php

@@ -5,7 +5,6 @@ declare(strict_types=1);
 use Fig\Http\Message\RequestMethodInterface as RequestMethod;
 use RKA\Middleware\IpAddress;
 use Shlinkio\Shlink\Core\Action;
-use Shlinkio\Shlink\Core\Middleware;
 
 return [
 
@@ -32,7 +31,6 @@ return [
             'name' => Action\QrCodeAction::class,
             'path' => '/{shortCode}/qr-code[/{size:[0-9]+}]',
             'middleware' => [
-                Middleware\QrCodeCacheMiddleware::class,
                 Action\QrCodeAction::class,
             ],
             'allowed_methods' => [RequestMethod::METHOD_GET],

module/Core/src/Action/QrCodeAction.php

@@ -5,7 +5,6 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Core\Action;
 
 use Endroid\QrCode\QrCode;
-use Mezzio\Router\RouterInterface;
 use Psr\Http\Message\ResponseInterface as Response;
 use Psr\Http\Message\ServerRequestInterface as Request;
 use Psr\Http\Server\MiddlewareInterface;
@@ -23,17 +22,17 @@ class QrCodeAction implements MiddlewareInterface
     private const MIN_SIZE = 50;
     private const MAX_SIZE = 1000;
 
-    private RouterInterface $router;
     private ShortUrlResolverInterface $urlResolver;
+    private array $domainConfig;
     private LoggerInterface $logger;
 
     public function __construct(
-        RouterInterface $router,
         ShortUrlResolverInterface $urlResolver,
+        array $domainConfig,
         ?LoggerInterface $logger = null
     ) {
-        $this->router = $router;
         $this->urlResolver = $urlResolver;
+        $this->domainConfig = $domainConfig;
         $this->logger = $logger ?: new NullLogger();
     }
 
@@ -42,23 +41,19 @@ class QrCodeAction implements MiddlewareInterface
         $identifier = ShortUrlIdentifier::fromRedirectRequest($request);
 
         try {
-            $this->urlResolver->resolveEnabledShortUrl($identifier);
+            $shortUrl = $this->urlResolver->resolveEnabledShortUrl($identifier);
         } catch (ShortUrlNotFoundException $e) {
             $this->logger->warning('An error occurred while creating QR code. {e}', ['e' => $e]);
             return $handler->handle($request);
         }
 
-        $path = $this->router->generateUri(RedirectAction::class, ['shortCode' => $identifier->shortCode()]);
+        $qrCode = new QrCode($shortUrl->toString($this->domainConfig));
-        $size = $this->getSizeParam($request);
+        $qrCode->setSize($this->getSizeParam($request));
-
-        $qrCode = new QrCode((string) $request->getUri()->withPath($path)->withQuery(''));
-        $qrCode->setSize($size);
         $qrCode->setMargin(0);
+
         return new QrCodeResponse($qrCode);
     }
 
-    /**
-     */
     private function getSizeParam(Request $request): int
     {
         $size = (int) $request->getAttribute('size', self::DEFAULT_SIZE);

module/Core/src/Middleware/QrCodeCacheMiddleware.php

@@ -1,50 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Shlinkio\Shlink\Core\Middleware;
-
-use Doctrine\Common\Cache\Cache;
-use Laminas\Diactoros\Response as DiactResp;
-use Psr\Http\Message\ResponseInterface as Response;
-use Psr\Http\Message\ServerRequestInterface as Request;
-use Psr\Http\Server\MiddlewareInterface;
-use Psr\Http\Server\RequestHandlerInterface;
-
-class QrCodeCacheMiddleware implements MiddlewareInterface
-{
-    private Cache $cache;
-
-    public function __construct(Cache $cache)
-    {
-        $this->cache = $cache;
-    }
-
-    /**
-     * Process an incoming server request and return a response, optionally delegating
-     * to the next middleware component to create the response.
-     *
-     *
-     */
-    public function process(Request $request, RequestHandlerInterface $handler): Response
-    {
-        $cacheKey = $request->getUri()->getPath();
-
-        // If this QR code is already cached, just return it
-        if ($this->cache->contains($cacheKey)) {
-            $qrData = $this->cache->fetch($cacheKey);
-            $response = new DiactResp();
-            $response->getBody()->write($qrData['body']);
-            return $response->withHeader('Content-Type', $qrData['content-type']);
-        }
-
-        // If not, call the next middleware and cache it
-        /** @var Response $resp */
-        $resp = $handler->handle($request);
-        $this->cache->save($cacheKey, [
-            'body' => $resp->getBody()->__toString(),
-            'content-type' => $resp->getHeaderLine('Content-Type'),
-        ]);
-        return $resp;
-    }
-}

module/Core/test/Action/QrCodeActionTest.php

@@ -30,7 +30,7 @@ class QrCodeActionTest extends TestCase
 
         $this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class);
 
-        $this->action = new QrCodeAction($router->reveal(), $this->urlResolver->reveal());
+        $this->action = new QrCodeAction($this->urlResolver->reveal(), ['domain' => 'doma.in']);
     }
 
     /** @test */

module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php

@@ -1,55 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace ShlinkioTest\Shlink\Core\Middleware;
-
-use Doctrine\Common\Cache\ArrayCache;
-use Doctrine\Common\Cache\Cache;
-use Laminas\Diactoros\Response;
-use Laminas\Diactoros\ServerRequest;
-use Laminas\Diactoros\Uri;
-use PHPUnit\Framework\TestCase;
-use Prophecy\Argument;
-use Psr\Http\Server\RequestHandlerInterface;
-use Shlinkio\Shlink\Core\Middleware\QrCodeCacheMiddleware;
-
-class QrCodeCacheMiddlewareTest extends TestCase
-{
-    private QrCodeCacheMiddleware $middleware;
-    private Cache $cache;
-
-    public function setUp(): void
-    {
-        $this->cache = new ArrayCache();
-        $this->middleware = new QrCodeCacheMiddleware($this->cache);
-    }
-
-    /** @test */
-    public function noCachedPathFallsBackToNextMiddleware(): void
-    {
-        $delegate = $this->prophesize(RequestHandlerInterface::class);
-        $delegate->handle(Argument::any())->willReturn(new Response())->shouldBeCalledOnce();
-
-        $this->middleware->process((new ServerRequest())->withUri(new Uri('/foo/bar')), $delegate->reveal());
-
-        $this->assertTrue($this->cache->contains('/foo/bar'));
-    }
-
-    /** @test */
-    public function cachedPathReturnsCacheContent(): void
-    {
-        $isCalled = false;
-        $uri = (new Uri())->withPath('/foo');
-        $this->cache->save('/foo', ['body' => 'the body', 'content-type' => 'image/png']);
-        $delegate = $this->prophesize(RequestHandlerInterface::class);
-
-        $resp = $this->middleware->process((new ServerRequest())->withUri($uri), $delegate->reveal());
-
-        $this->assertFalse($isCalled);
-        $resp->getBody()->rewind();
-        $this->assertEquals('the body', $resp->getBody()->getContents());
-        $this->assertEquals('image/png', $resp->getHeaderLine('Content-Type'));
-        $delegate->handle(Argument::any())->shouldHaveBeenCalledTimes(0);
-    }
-}