From 99b7c77997b07b1c0f8ea46c83a7f03f9155d00d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 9 Aug 2016 10:24:42 +0200 Subject: [PATCH 1/5] Created action to generate QR codes --- composer.json | 3 +- module/Common/src/Response/QrCodeResponse.php | 35 ++++++ module/Core/config/dependencies.config.php | 5 +- module/Core/config/routes.config.php | 10 +- module/Core/src/Action/QrCodeAction.php | 113 ++++++++++++++++++ 5 files changed, 161 insertions(+), 5 deletions(-) create mode 100644 module/Common/src/Response/QrCodeResponse.php create mode 100644 module/Core/src/Action/QrCodeAction.php diff --git a/composer.json b/composer.json index 9ea805ba..1f9bf7bd 100644 --- a/composer.json +++ b/composer.json @@ -28,7 +28,8 @@ "symfony/console": "^3.0", "firebase/php-jwt": "^4.0", "monolog/monolog": "^1.21", - "theorchard/monolog-cascade": "^0.4" + "theorchard/monolog-cascade": "^0.4", + "endroid/qrcode": "^1.7" }, "require-dev": { "phpunit/phpunit": "^5.0", diff --git a/module/Common/src/Response/QrCodeResponse.php b/module/Common/src/Response/QrCodeResponse.php new file mode 100644 index 00000000..e19e4578 --- /dev/null +++ b/module/Common/src/Response/QrCodeResponse.php @@ -0,0 +1,35 @@ +createBody($qrCode), + $status, + $this->injectContentType($qrCode->getContentType(), $headers) + ); + } + + /** + * Create the message body. + * + * @param QrCode $qrCode + * @return StreamInterface + */ + private function createBody(QrCode $qrCode) + { + $body = new Stream('php://temp', 'wb+'); + $body->write($qrCode->get()); + $body->rewind(); + return $body; + } +} diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 27983069..eb6168f9 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -1,6 +1,6 @@ AnnotatedFactory::class, // Middleware - RedirectAction::class => AnnotatedFactory::class, + Action\RedirectAction::class => AnnotatedFactory::class, + Action\QrCodeAction::class => AnnotatedFactory::class, ], ], diff --git a/module/Core/config/routes.config.php b/module/Core/config/routes.config.php index 4d9a85e5..9b499f71 100644 --- a/module/Core/config/routes.config.php +++ b/module/Core/config/routes.config.php @@ -1,5 +1,5 @@ 'long-url-redirect', 'path' => '/{shortCode}', - 'middleware' => RedirectAction::class, + 'middleware' => Action\RedirectAction::class, + 'allowed_methods' => ['GET'], + ], + [ + 'name' => 'short-url-qr-code', + 'path' => '/qr/{shortCode}[/{size:[0-9]+}]', + 'middleware' => Action\QrCodeAction::class, 'allowed_methods' => ['GET'], ], ], diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php new file mode 100644 index 00000000..5d0549eb --- /dev/null +++ b/module/Core/src/Action/QrCodeAction.php @@ -0,0 +1,113 @@ +router = $router; + $this->urlShortener = $urlShortener; + $this->logger = $logger ?: new NullLogger(); + } + + /** + * Process an incoming request and/or response. + * + * Accepts a server-side request and a response instance, and does + * something with them. + * + * If the response is not complete and/or further processing would not + * interfere with the work done in the middleware, or if the middleware + * wants to delegate to another process, it can use the `$out` callable + * if present. + * + * If the middleware does not return a value, execution of the current + * request is considered complete, and the response instance provided will + * be considered the response to return. + * + * Alternately, the middleware may return a response instance. + * + * Often, middleware will `return $out();`, with the assumption that a + * later middleware will return a response. + * + * @param Request $request + * @param Response $response + * @param null|callable $out + * @return null|Response + */ + public function __invoke(Request $request, Response $response, callable $out = null) + { + // Make sure the short URL exists for this short code + $shortCode = $request->getAttribute('shortCode'); + try { + $shortUrl = $this->urlShortener->shortCodeToUrl($shortCode); + if (! isset($shortUrl)) { + return $out($request, $response->withStatus(404), 'Not Found'); + } + } catch (InvalidShortCodeException $e) { + $this->logger->warning('Tried to create a QR code with an invalid short code' . PHP_EOL . $e); + return $out($request, $response->withStatus(404), 'Not Found'); + } + + $path = $this->router->generateUri('long-url-redirect', ['shortCode' => $shortCode]); + $size = $this->getSizeParam($request); + + $qrCode = new QrCode($request->getUri()->withPath($path)->withQuery('')); + $qrCode->setSize($size) + ->setPadding(0); + return new QrCodeResponse($qrCode); + } + + /** + * @param Request $request + * @return int + */ + protected function getSizeParam(Request $request) + { + $size = intval($request->getAttribute('size', 300)); + if ($size < 50) { + return 50; + } elseif ($size > 1000) { + return 1000; + } + + return $size; + } +} From 8eb279fd288d1345bf966e9cdc5bdad9fca23109 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 9 Aug 2016 13:32:33 +0200 Subject: [PATCH 2/5] Updated UrlShortener to namespace the cache entries --- module/Core/src/Service/UrlShortener.php | 7 ++++--- module/Core/test/Service/UrlShortenerTest.php | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php index 30b700cd..ed87b604 100644 --- a/module/Core/src/Service/UrlShortener.php +++ b/module/Core/src/Service/UrlShortener.php @@ -148,9 +148,10 @@ class UrlShortener implements UrlShortenerInterface */ public function shortCodeToUrl($shortCode) { + $cacheKey = sprintf('%s_longUrl', $shortCode); // Check if the short code => URL map is already cached - if ($this->cache->contains($shortCode)) { - return $this->cache->fetch($shortCode); + if ($this->cache->contains($cacheKey)) { + return $this->cache->fetch($cacheKey); } // Validate short code format @@ -165,7 +166,7 @@ class UrlShortener implements UrlShortenerInterface // Cache the shortcode if (isset($shortUrl)) { $url = $shortUrl->getOriginalUrl(); - $this->cache->save($shortCode, $url); + $this->cache->save($cacheKey, $url); return $url; } diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php index 90886016..4cf7da1d 100644 --- a/module/Core/test/Service/UrlShortenerTest.php +++ b/module/Core/test/Service/UrlShortenerTest.php @@ -129,10 +129,10 @@ class UrlShortenerTest extends TestCase $repo->findOneBy(['shortCode' => $shortCode])->willReturn($shortUrl); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $this->assertFalse($this->cache->contains($shortCode)); + $this->assertFalse($this->cache->contains($shortCode . '_longUrl')); $url = $this->urlShortener->shortCodeToUrl($shortCode); $this->assertEquals($shortUrl->getOriginalUrl(), $url); - $this->assertTrue($this->cache->contains($shortCode)); + $this->assertTrue($this->cache->contains($shortCode . '_longUrl')); } /** @@ -151,7 +151,7 @@ class UrlShortenerTest extends TestCase { $shortCode = '12C1c'; $expectedUrl = 'expected_url'; - $this->cache->save($shortCode, $expectedUrl); + $this->cache->save($shortCode . '_longUrl', $expectedUrl); $this->em->getRepository(ShortUrl::class)->willReturn(null)->shouldBeCalledTimes(0); $url = $this->urlShortener->shortCodeToUrl($shortCode); From 18084433c7969e39f34fd360842622b4a08ad556 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 9 Aug 2016 13:41:30 +0200 Subject: [PATCH 3/5] Created middleware to cache generated QR codes --- module/Common/src/Factory/CacheFactory.php | 1 + module/Core/config/dependencies.config.php | 2 + module/Core/config/routes.config.php | 6 +- .../src/Middleware/QrCodeCacheMiddleware.php | 73 +++++++++++++++++++ 4 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 module/Core/src/Middleware/QrCodeCacheMiddleware.php diff --git a/module/Common/src/Factory/CacheFactory.php b/module/Common/src/Factory/CacheFactory.php index c866b980..710c5962 100644 --- a/module/Common/src/Factory/CacheFactory.php +++ b/module/Common/src/Factory/CacheFactory.php @@ -3,6 +3,7 @@ namespace Shlinkio\Shlink\Common\Factory; use Doctrine\Common\Cache\ApcuCache; use Doctrine\Common\Cache\ArrayCache; +use Doctrine\Common\Cache\FilesystemCache; use Interop\Container\ContainerInterface; use Interop\Container\Exception\ContainerException; use Zend\ServiceManager\Exception\ServiceNotCreatedException; diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index eb6168f9..6a5596ee 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -1,6 +1,7 @@ AnnotatedFactory::class, Action\QrCodeAction::class => AnnotatedFactory::class, + Middleware\QrCodeCacheMiddleware::class => AnnotatedFactory::class, ], ], diff --git a/module/Core/config/routes.config.php b/module/Core/config/routes.config.php index 9b499f71..9b0c071f 100644 --- a/module/Core/config/routes.config.php +++ b/module/Core/config/routes.config.php @@ -1,5 +1,6 @@ 'short-url-qr-code', 'path' => '/qr/{shortCode}[/{size:[0-9]+}]', - 'middleware' => Action\QrCodeAction::class, + 'middleware' => [ + Middleware\QrCodeCacheMiddleware::class, + Action\QrCodeAction::class, + ], 'allowed_methods' => ['GET'], ], ], diff --git a/module/Core/src/Middleware/QrCodeCacheMiddleware.php b/module/Core/src/Middleware/QrCodeCacheMiddleware.php new file mode 100644 index 00000000..d0f132bb --- /dev/null +++ b/module/Core/src/Middleware/QrCodeCacheMiddleware.php @@ -0,0 +1,73 @@ +cache = $cache; + } + + /** + * Process an incoming request and/or response. + * + * Accepts a server-side request and a response instance, and does + * something with them. + * + * If the response is not complete and/or further processing would not + * interfere with the work done in the middleware, or if the middleware + * wants to delegate to another process, it can use the `$out` callable + * if present. + * + * If the middleware does not return a value, execution of the current + * request is considered complete, and the response instance provided will + * be considered the response to return. + * + * Alternately, the middleware may return a response instance. + * + * Often, middleware will `return $out();`, with the assumption that a + * later middleware will return a response. + * + * @param Request $request + * @param Response $response + * @param null|callable $out + * @return null|Response + */ + public function __invoke(Request $request, Response $response, callable $out = null) + { + $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->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 = $out($request, $response); + $this->cache->save($cacheKey, [ + 'body' => $resp->getBody()->__toString(), + 'content-type' => $resp->getHeaderLine('Content-Type'), + ]); + return $resp; + } +} From 12410e82d824f1f1db8fb1f851729e4afbbfa7c8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 9 Aug 2016 14:18:20 +0200 Subject: [PATCH 4/5] Created tests for QrCode middlewares --- module/Core/test/Action/QrCodeActionTest.php | 93 +++++++++++++++++++ .../Middleware/QrCodeCacheMiddlewareTest.php | 69 ++++++++++++++ 2 files changed, 162 insertions(+) create mode 100644 module/Core/test/Action/QrCodeActionTest.php create mode 100644 module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php new file mode 100644 index 00000000..d4924724 --- /dev/null +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -0,0 +1,93 @@ +prophesize(RouterInterface::class); + $router->generateUri(Argument::cetera())->willReturn('/foo/bar'); + + $this->urlShortener = $this->prophesize(UrlShortener::class); + + $this->action = new QrCodeAction($router->reveal(), $this->urlShortener->reveal()); + } + + /** + * @test + */ + public function aNonexistentShortCodeWillReturnNotFoundResponse() + { + $shortCode = 'abc123'; + $this->urlShortener->shortCodeToUrl($shortCode)->willReturn(null)->shouldBeCalledTimes(1); + + $resp = $this->action->__invoke( + ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), + new Response(), + function ($req, $resp) { + return $resp; + } + ); + $this->assertEquals(404, $resp->getStatusCode()); + } + + /** + * @test + */ + public function anInvalidShortCodeWillReturnNotFoundResponse() + { + $shortCode = 'abc123'; + $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(InvalidShortCodeException::class) + ->shouldBeCalledTimes(1); + + $resp = $this->action->__invoke( + ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), + new Response(), + function ($req, $resp) { + return $resp; + } + ); + $this->assertEquals(404, $resp->getStatusCode()); + } + + /** + * @test + */ + public function aCorrectRequestReturnsTheQrCodeResponse() + { + $shortCode = 'abc123'; + $this->urlShortener->shortCodeToUrl($shortCode)->willReturn(new ShortUrl())->shouldBeCalledTimes(1); + + $resp = $this->action->__invoke( + ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), + new Response(), + function ($req, $resp) { + return $resp; + } + ); + + $this->assertInstanceOf(QrCodeResponse::class, $resp); + $this->assertEquals(200, $resp->getStatusCode()); + } +} diff --git a/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php b/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php new file mode 100644 index 00000000..64aac3e0 --- /dev/null +++ b/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php @@ -0,0 +1,69 @@ +cache = new ArrayCache(); + $this->middleware = new QrCodeCacheMiddleware($this->cache); + } + + /** + * @test + */ + public function noCachedPathFallbacksToNextMiddleware() + { + $isCalled = false; + $this->middleware->__invoke( + ServerRequestFactory::fromGlobals(), + new Response(), + function ($req, $resp) use (&$isCalled) { + $isCalled = true; + return $resp; + } + ); + $this->assertTrue($isCalled); + } + + /** + * @test + */ + public function cachedPathReturnsCacheContent() + { + $isCalled = false; + $uri = (new Uri())->withPath('/foo'); + $this->cache->save('/foo', ['body' => 'the body', 'content-type' => 'image/png']); + + $resp = $this->middleware->__invoke( + ServerRequestFactory::fromGlobals()->withUri($uri), + new Response(), + function ($req, $resp) use (&$isCalled) { + $isCalled = true; + return $resp; + } + ); + + $this->assertFalse($isCalled); + $resp->getBody()->rewind(); + $this->assertEquals('the body', $resp->getBody()->getContents()); + $this->assertEquals('image/png', $resp->getHeaderLine('Content-Type')); + } +} From 90cef7d4d904f318c8b6b8b03ccdd9cc9bc932d4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 9 Aug 2016 14:19:46 +0200 Subject: [PATCH 5/5] Removed unused import --- module/Common/src/Factory/CacheFactory.php | 1 - 1 file changed, 1 deletion(-) diff --git a/module/Common/src/Factory/CacheFactory.php b/module/Common/src/Factory/CacheFactory.php index 710c5962..c866b980 100644 --- a/module/Common/src/Factory/CacheFactory.php +++ b/module/Common/src/Factory/CacheFactory.php @@ -3,7 +3,6 @@ namespace Shlinkio\Shlink\Common\Factory; use Doctrine\Common\Cache\ApcuCache; use Doctrine\Common\Cache\ArrayCache; -use Doctrine\Common\Cache\FilesystemCache; use Interop\Container\ContainerInterface; use Interop\Container\Exception\ContainerException; use Zend\ServiceManager\Exception\ServiceNotCreatedException;