From a72dc16d85c330aec54a23f49d9f8ce024aa00c8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya <alejandrocelaya@gmail.com> Date: Fri, 27 Nov 2020 17:05:13 +0100 Subject: [PATCH 1/5] #917 --- module/Core/config/routes.config.php | 12 +++++++++++- module/Core/src/Action/QrCodeAction.php | 11 +++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/module/Core/config/routes.config.php b/module/Core/config/routes.config.php index 82abef30..a95e8e96 100644 --- a/module/Core/config/routes.config.php +++ b/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, ], diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php index 4a8b7db5..0fb5089b 100644 --- a/module/Core/src/Action/QrCodeAction.php +++ b/module/Core/src/Action/QrCodeAction.php @@ -48,11 +48,15 @@ class QrCodeAction implements MiddlewareInterface return $handler->handle($request); } + $query = $request->getQueryParams(); + // Size attribute is deprecated + $size = $this->normalizeSize($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; } From fe59a5ad8606a07738003d580cdda2248a831c7b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya <alejandrocelaya@gmail.com> Date: Fri, 27 Nov 2020 17:16:46 +0100 Subject: [PATCH 2/5] #917 Fixed cast to int on QR code action --- module/Core/src/Action/QrCodeAction.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php index 0fb5089b..4cdecf97 100644 --- a/module/Core/src/Action/QrCodeAction.php +++ b/module/Core/src/Action/QrCodeAction.php @@ -50,7 +50,7 @@ class QrCodeAction implements MiddlewareInterface $query = $request->getQueryParams(); // Size attribute is deprecated - $size = $this->normalizeSize($request->getAttribute('size', $query['size'] ?? self::DEFAULT_SIZE)); + $size = $this->normalizeSize((int) $request->getAttribute('size', $query['size'] ?? self::DEFAULT_SIZE)); $qrCode = new QrCode($shortUrl->toString($this->domainConfig)); $qrCode->setSize($size); From 4f1ab977a1cd6a6c91be1a1bede5a06efc6672b1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya <alejandrocelaya@gmail.com> Date: Fri, 27 Nov 2020 17:42:33 +0100 Subject: [PATCH 3/5] #917 Added tests covering the different ways to provide sizes to the QR codes --- .../src/Action/AbstractTrackingAction.php | 2 +- module/Core/src/Action/QrCodeAction.php | 2 +- module/Core/test/Action/QrCodeActionTest.php | 46 +++++++++++++------ 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php index b121ae3a..86eb197b 100644 --- a/module/Core/src/Action/AbstractTrackingAction.php +++ b/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 diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php index 4cdecf97..919682d5 100644 --- a/module/Core/src/Action/QrCodeAction.php +++ b/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 diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index 1237585c..76daa406 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/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, + ]; + } } From c13adb04ef43274c15e0ed7aa7b38bccada7791c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya <alejandrocelaya@gmail.com> Date: Fri, 27 Nov 2020 17:47:52 +0100 Subject: [PATCH 4/5] #917 Documented QR endpoint with query size and path size --- docs/swagger/paths/{shortCode}_qr-code.json | 2 +- .../paths/{shortCode}_qr-code_{size}.json | 66 +++++++++++++++++++ docs/swagger/swagger.json | 3 + 3 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 docs/swagger/paths/{shortCode}_qr-code_{size}.json diff --git a/docs/swagger/paths/{shortCode}_qr-code.json b/docs/swagger/paths/{shortCode}_qr-code.json index a3fdaffb..3714f802 100644 --- a/docs/swagger/paths/{shortCode}_qr-code.json +++ b/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": { diff --git a/docs/swagger/paths/{shortCode}_qr-code_{size}.json b/docs/swagger/paths/{shortCode}_qr-code_{size}.json new file mode 100644 index 00000000..fb5dd33e --- /dev/null +++ b/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" + } + } + } + } + } + } +} diff --git a/docs/swagger/swagger.json b/docs/swagger/swagger.json index 8dc04412..dc834905 100644 --- a/docs/swagger/swagger.json +++ b/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" } } } From cfdf2f9480bfcf614206058c40331048719f7d8d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya <alejandrocelaya@gmail.com> Date: Fri, 27 Nov 2020 17:50:09 +0100 Subject: [PATCH 5/5] #917 Updated changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06adfb76..4336c93d 100644 --- a/CHANGELOG.md +++ b/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*