From f49e9064cdf4cbd885088ce7dabc0ccf828de066 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 8 Aug 2016 10:02:52 +0200 Subject: [PATCH] Added cache adapter to the UrlShortener service to cache shortcode-url maps --- module/Core/src/Action/RedirectAction.php | 6 ++-- module/Core/src/Service/UrlShortener.php | 24 ++++++++++++-- module/Core/test/Service/UrlShortenerTest.php | 33 ++++++++++++++++--- .../Rest/test/Action/GetVisitsActionTest.php | 2 +- .../Rest/test/Action/ResolveUrlActionTest.php | 2 +- 5 files changed, 56 insertions(+), 11 deletions(-) diff --git a/module/Core/src/Action/RedirectAction.php b/module/Core/src/Action/RedirectAction.php index 031aa0fe..626ae27e 100644 --- a/module/Core/src/Action/RedirectAction.php +++ b/module/Core/src/Action/RedirectAction.php @@ -18,14 +18,14 @@ class RedirectAction implements MiddlewareInterface */ private $urlShortener; /** - * @var VisitsTracker|VisitsTrackerInterface + * @var VisitsTrackerInterface */ private $visitTracker; /** * RedirectMiddleware constructor. - * @param UrlShortenerInterface|UrlShortener $urlShortener - * @param VisitsTrackerInterface|VisitsTracker $visitTracker + * @param UrlShortenerInterface $urlShortener + * @param VisitsTrackerInterface $visitTracker * * @Inject({UrlShortener::class, VisitsTracker::class}) */ diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php index 01a8d4b6..356667e1 100644 --- a/module/Core/src/Service/UrlShortener.php +++ b/module/Core/src/Service/UrlShortener.php @@ -2,6 +2,7 @@ namespace Shlinkio\Shlink\Core\Service; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; +use Doctrine\Common\Cache\Cache; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\ORMException; use GuzzleHttp\ClientInterface; @@ -28,23 +29,30 @@ class UrlShortener implements UrlShortenerInterface * @var string */ private $chars; + /** + * @var Cache + */ + private $cache; /** * UrlShortener constructor. * @param ClientInterface $httpClient * @param EntityManagerInterface $em + * @param Cache $cache * @param string $chars * - * @Inject({"httpClient", "em", "config.url_shortener.shortcode_chars"}) + * @Inject({"httpClient", "em", Cache::class, "config.url_shortener.shortcode_chars"}) */ public function __construct( ClientInterface $httpClient, EntityManagerInterface $em, + Cache $cache, $chars = self::DEFAULT_CHARS ) { $this->httpClient = $httpClient; $this->em = $em; $this->chars = empty($chars) ? self::DEFAULT_CHARS : $chars; + $this->cache = $cache; } /** @@ -140,6 +148,11 @@ class UrlShortener implements UrlShortenerInterface */ public function shortCodeToUrl($shortCode) { + // Check if the short code => URL map is already cached + if ($this->cache->contains($shortCode)) { + return $this->cache->fetch($shortCode); + } + // Validate short code format if (! preg_match('|[' . $this->chars . "]+|", $shortCode)) { throw InvalidShortCodeException::fromShortCode($shortCode, $this->chars); @@ -149,6 +162,13 @@ class UrlShortener implements UrlShortenerInterface $shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy([ 'shortCode' => $shortCode, ]); - return isset($shortUrl) ? $shortUrl->getOriginalUrl() : null; + // Cache the shortcode + if (isset($shortUrl)) { + $url = $shortUrl->getOriginalUrl(); + $this->cache->save($shortCode, $url); + return $url; + } + + return null; } } diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php index 8298a8cc..90886016 100644 --- a/module/Core/test/Service/UrlShortenerTest.php +++ b/module/Core/test/Service/UrlShortenerTest.php @@ -1,6 +1,8 @@ findOneBy(Argument::any())->willReturn(null); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $this->urlShortener = new UrlShortener($this->httpClient->reveal(), $this->em->reveal()); + $this->cache = new ArrayCache(); + + $this->urlShortener = new UrlShortener($this->httpClient->reveal(), $this->em->reveal(), $this->cache); } /** @@ -112,16 +120,19 @@ class UrlShortenerTest extends TestCase public function shortCodeIsProperlyParsed() { // 12C1c -> 10 + $shortCode = '12C1c'; $shortUrl = new ShortUrl(); - $shortUrl->setShortCode('12C1c') + $shortUrl->setShortCode($shortCode) ->setOriginalUrl('expected_url'); $repo = $this->prophesize(ObjectRepository::class); - $repo->findOneBy(['shortCode' => '12C1c'])->willReturn($shortUrl); + $repo->findOneBy(['shortCode' => $shortCode])->willReturn($shortUrl); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $url = $this->urlShortener->shortCodeToUrl('12C1c'); + $this->assertFalse($this->cache->contains($shortCode)); + $url = $this->urlShortener->shortCodeToUrl($shortCode); $this->assertEquals($shortUrl->getOriginalUrl(), $url); + $this->assertTrue($this->cache->contains($shortCode)); } /** @@ -132,4 +143,18 @@ class UrlShortenerTest extends TestCase { $this->urlShortener->shortCodeToUrl('&/('); } + + /** + * @test + */ + public function cachedShortCodeDoesNotHitDatabase() + { + $shortCode = '12C1c'; + $expectedUrl = 'expected_url'; + $this->cache->save($shortCode, $expectedUrl); + $this->em->getRepository(ShortUrl::class)->willReturn(null)->shouldBeCalledTimes(0); + + $url = $this->urlShortener->shortCodeToUrl($shortCode); + $this->assertEquals($expectedUrl, $url); + } } diff --git a/module/Rest/test/Action/GetVisitsActionTest.php b/module/Rest/test/Action/GetVisitsActionTest.php index 901c549e..7e222269 100644 --- a/module/Rest/test/Action/GetVisitsActionTest.php +++ b/module/Rest/test/Action/GetVisitsActionTest.php @@ -59,7 +59,7 @@ class GetVisitsActionTest extends TestCase ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), new Response() ); - $this->assertEquals(400, $response->getStatusCode()); + $this->assertEquals(404, $response->getStatusCode()); } /** diff --git a/module/Rest/test/Action/ResolveUrlActionTest.php b/module/Rest/test/Action/ResolveUrlActionTest.php index 0bd3bded..07db655a 100644 --- a/module/Rest/test/Action/ResolveUrlActionTest.php +++ b/module/Rest/test/Action/ResolveUrlActionTest.php @@ -39,7 +39,7 @@ class ResolveUrlActionTest extends TestCase $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); $response = $this->action->__invoke($request, new Response()); - $this->assertEquals(400, $response->getStatusCode()); + $this->assertEquals(404, $response->getStatusCode()); $this->assertTrue(strpos($response->getBody()->getContents(), RestUtils::INVALID_ARGUMENT_ERROR) > 0); }