Merge pull request #923 from acelaya-forks/feature/qr-codes-query-size

Feature/qr codes query size

CHANGELOG.md

@@ -12,7 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 * [#912](https://github.com/shlinkio/shlink/issues/912) Changed error templates to be plain html files, removing the dependency on `league/plates` package.
 
 ### Deprecated
-* *Nothing*
+* [#917](https://github.com/shlinkio/shlink/issues/917) Deprecated `/{shortCode}/qr-code/{size}` URL, in favor of providing the size in the query instead, `/{shortCode}/qr-code?size={size}`.
 
 ### Removed
 * *Nothing*

docs/swagger/paths/{shortCode}_qr-code.json

@@ -18,7 +18,7 @@
             },
             {
                 "name": "size",
-                "in": "path",
+                "in": "query",
                 "description": "The size of the image to be returned.",
                 "required": false,
                 "schema": {

docs/swagger/paths/{shortCode}_qr-code_{size}.json

@@ -0,0 +1,66 @@
+{
+    "get": {
+        "operationId": "shortUrlQrCodeSize",
+        "deprecated": true,
+        "tags": [
+            "URL Shortener"
+        ],
+        "summary": "Short URL QR code",
+        "description": "Generates a QR code image pointing to a short URL",
+        "parameters": [
+            {
+                "name": "shortCode",
+                "in": "path",
+                "description": "The short code to resolve.",
+                "required": true,
+                "schema": {
+                    "type": "string"
+                }
+            },
+            {
+                "name": "size",
+                "in": "path",
+                "description": "The size of the image to be returned.",
+                "required": false,
+                "schema": {
+                    "type": "integer",
+                    "minimum": 50,
+                    "maximum": 1000,
+                    "default": 300
+                }
+            },
+            {
+                "name": "format",
+                "in": "query",
+                "description": "The format for the QR code image, being valid values png and svg. Not providing the param or providing any other value will fall back to png.",
+                "required": false,
+                "schema": {
+                    "type": "string",
+                    "enum": [
+                        "png",
+                        "svg"
+                    ]
+                }
+            }
+        ],
+        "responses": {
+            "200": {
+                "description": "QR code in PNG format",
+                "content": {
+                    "image/png": {
+                        "schema": {
+                            "type": "string",
+                            "format": "binary"
+                        }
+                    },
+                    "image/svg+xml": {
+                        "schema": {
+                            "type": "string",
+                            "format": "binary"
+                        }
+                    }
+                }
+            }
+        }
+    }
+}

docs/swagger/swagger.json

@@ -116,6 +116,9 @@
         },
         "/{shortCode}/qr-code": {
             "$ref": "paths/{shortCode}_qr-code.json"
+        },
+        "/{shortCode}/qr-code/{size}": {
+            "$ref": "paths/{shortCode}_qr-code_{size}.json"
         }
     }
 }

module/Core/config/routes.config.php

@@ -29,7 +29,17 @@ return [
         ],
         [
             'name' => Action\QrCodeAction::class,
-            'path' => '/{shortCode}/qr-code[/{size:[0-9]+}]',
+            'path' => '/{shortCode}/qr-code',
+            'middleware' => [
+                Action\QrCodeAction::class,
+            ],
+            'allowed_methods' => [RequestMethod::METHOD_GET],
+        ],
+
+        // Deprecated
+        [
+            'name' => 'old_' . Action\QrCodeAction::class,
+            'path' => '/{shortCode}/qr-code/{size:[0-9]+}',
             'middleware' => [
                 Action\QrCodeAction::class,
             ],

module/Core/src/Action/AbstractTrackingAction.php

@@ -41,7 +41,7 @@ abstract class AbstractTrackingAction implements MiddlewareInterface, RequestMet
         $this->urlResolver = $urlResolver;
         $this->visitTracker = $visitTracker;
         $this->appOptions = $appOptions;
-        $this->logger = $logger ?: new NullLogger();
+        $this->logger = $logger ?? new NullLogger();
     }
 
     public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface

module/Core/src/Action/QrCodeAction.php

@@ -34,7 +34,7 @@ class QrCodeAction implements MiddlewareInterface
     ) {
         $this->urlResolver = $urlResolver;
         $this->domainConfig = $domainConfig;
-        $this->logger = $logger ?: new NullLogger();
+        $this->logger = $logger ?? new NullLogger();
     }
 
     public function process(Request $request, RequestHandlerInterface $handler): Response
@@ -48,11 +48,15 @@ class QrCodeAction implements MiddlewareInterface
             return $handler->handle($request);
         }
 
+        $query = $request->getQueryParams();
+        // Size attribute is deprecated
+        $size = $this->normalizeSize((int) $request->getAttribute('size', $query['size'] ?? self::DEFAULT_SIZE));
+
         $qrCode = new QrCode($shortUrl->toString($this->domainConfig));
-        $qrCode->setSize($this->getSizeParam($request));
+        $qrCode->setSize($size);
         $qrCode->setMargin(0);
 
-        $format = $request->getQueryParams()['format'] ?? 'png';
+        $format = $query['format'] ?? 'png';
         if ($format === 'svg') {
             $qrCode->setWriter(new SvgWriter());
         }
@@ -60,9 +64,8 @@ class QrCodeAction implements MiddlewareInterface
         return new QrCodeResponse($qrCode);
     }
 
-    private function getSizeParam(Request $request): int
+    private function normalizeSize(int $size): int
     {
-        $size = (int) $request->getAttribute('size', self::DEFAULT_SIZE);
         if ($size < self::MIN_SIZE) {
             return self::MIN_SIZE;
         }

module/Core/test/Action/QrCodeActionTest.php

@@ -6,11 +6,13 @@ namespace ShlinkioTest\Shlink\Core\Action;
 
 use Laminas\Diactoros\Response;
 use Laminas\Diactoros\ServerRequest;
+use Laminas\Diactoros\ServerRequestFactory;
 use Mezzio\Router\RouterInterface;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\PhpUnit\ProphecyTrait;
 use Prophecy\Prophecy\ObjectProphecy;
+use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\RequestHandlerInterface;
 use Shlinkio\Shlink\Common\Response\QrCodeResponse;
 use Shlinkio\Shlink\Core\Action\QrCodeAction;
@@ -19,6 +21,8 @@ use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException;
 use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
 use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface;
 
+use function getimagesizefromstring;
+
 class QrCodeActionTest extends TestCase
 {
     use ProphecyTrait;
@@ -51,21 +55,6 @@ class QrCodeActionTest extends TestCase
         $process->shouldHaveBeenCalledOnce();
     }
 
-    /** @test */
-    public function anInvalidShortCodeWillReturnNotFoundResponse(): void
-    {
-        $shortCode = 'abc123';
-        $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode, ''))
-            ->willThrow(ShortUrlNotFoundException::class)
-            ->shouldBeCalledOnce();
-        $delegate = $this->prophesize(RequestHandlerInterface::class);
-        $process = $delegate->handle(Argument::any())->willReturn(new Response());
-
-        $this->action->process((new ServerRequest())->withAttribute('shortCode', $shortCode), $delegate->reveal());
-
-        $process->shouldHaveBeenCalledOnce();
-    }
-
     /** @test */
     public function aCorrectRequestReturnsTheQrCodeResponse(): void
     {
@@ -110,4 +99,31 @@ class QrCodeActionTest extends TestCase
         yield 'svg format' => [['format' => 'svg'], 'image/svg+xml'];
         yield 'unsupported format' => [['format' => 'jpg'], 'image/png'];
     }
+
+    /**
+     * @test
+     * @dataProvider provideRequestsWithSize
+     */
+    public function imageIsReturnedWithExpectedSize(ServerRequestInterface $req, int $expectedSize): void
+    {
+        $code = 'abc123';
+        $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($code, ''))->willReturn(new ShortUrl(''));
+        $delegate = $this->prophesize(RequestHandlerInterface::class);
+
+        $resp = $this->action->process($req->withAttribute('shortCode', $code), $delegate->reveal());
+        [$size] = getimagesizefromstring((string) $resp->getBody());
+
+        self::assertEquals($expectedSize, $size);
+    }
+
+    public function provideRequestsWithSize(): iterable
+    {
+        yield 'no size' => [ServerRequestFactory::fromGlobals(), 300];
+        yield 'size in attr' => [ServerRequestFactory::fromGlobals()->withAttribute('size', '400'), 400];
+        yield 'size in query' => [ServerRequestFactory::fromGlobals()->withQueryParams(['size' => '123']), 123];
+        yield 'size in query and attr' => [
+            ServerRequestFactory::fromGlobals()->withAttribute('size', '350')->withQueryParams(['size' => '123']),
+            350,
+        ];
+    }
 }