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* 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" } } } 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/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 4a8b7db5..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 @@ -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; } 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, + ]; + } }