Added cache adapter to the UrlShortener service to cache shortcode-url maps

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})
      */

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;
     }
 }

module/Core/test/Service/UrlShortenerTest.php

@@ -1,6 +1,8 @@
 <?php
 namespace ShlinkioTest\Shlink\Core\Service;
 
+use Doctrine\Common\Cache\ArrayCache;
+use Doctrine\Common\Cache\Cache;
 use Doctrine\Common\Persistence\ObjectRepository;
 use Doctrine\DBAL\Connection;
 use Doctrine\ORM\EntityManagerInterface;
@@ -29,6 +31,10 @@ class UrlShortenerTest extends TestCase
      * @var ObjectProphecy
      */
     protected $httpClient;
+    /**
+     * @var Cache
+     */
+    protected $cache;
 
     public function setUp()
     {
@@ -50,7 +56,9 @@ class UrlShortenerTest extends TestCase
         $repo->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);
+    }
 }

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());
     }
 
     /**

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);
     }