Used filesystem check instead of cache check for preview generation

composer.json

@@ -28,6 +28,7 @@
         "guzzlehttp/guzzle": "^6.2",
         "symfony/console": "^3.0",
         "symfony/process": "^3.0",
+        "symfony/filesystem": "^3.0",
         "firebase/php-jwt": "^4.0",
         "monolog/monolog": "^1.21",
         "theorchard/monolog-cascade": "^0.4",

module/CLI/src/Command/Shortcode/GeneratePreviewCommand.php

@@ -78,10 +78,12 @@ class GeneratePreviewCommand extends Command
             $this->previewGenerator->generatePreview($url);
             $output->writeln($this->translator->translate(' <info>Success!</info>'));
         } catch (PreviewGenerationException $e) {
-            $output->writeln([
+            $messages = [' <error>' . $this->translator->translate('Error') . '</error>'];
-                ' <error>' . $this->translator->translate('Error') . '</error>',
+            if ($output->isVerbose()) {
-                '<error>' . $e->__toString() . '</error>',
+                $messages[] = '<error>' . $e->__toString() . '</error>';
-            ]);
+            }
+
+            $output->writeln($messages);
         }
     }
 }

module/Common/config/dependencies.config.php

@@ -4,27 +4,28 @@ use Doctrine\Common\Cache\Cache;
 use Doctrine\ORM\EntityManager;
 use Monolog\Logger;
 use Psr\Log\LoggerInterface;
-use Shlinkio\Shlink\Common\Factory\CacheFactory;
+use Shlinkio\Shlink\Common\Factory;
-use Shlinkio\Shlink\Common\Factory\EntityManagerFactory;
-use Shlinkio\Shlink\Common\Factory\LoggerFactory;
-use Shlinkio\Shlink\Common\Factory\TranslatorFactory;
 use Shlinkio\Shlink\Common\Image;
 use Shlinkio\Shlink\Common\Middleware\LocaleMiddleware;
 use Shlinkio\Shlink\Common\Service;
 use Shlinkio\Shlink\Common\Twig\Extension\TranslatorExtension;
+use Symfony\Component\Filesystem\Filesystem;
 use Zend\I18n\Translator\Translator;
 use Zend\ServiceManager\Factory\InvokableFactory;
 
 return [
 
     'dependencies' => [
+        'invokables' => [
+            Filesystem::class => Filesystem::class,
+        ],
         'factories' => [
-            EntityManager::class => EntityManagerFactory::class,
+            EntityManager::class => Factory\EntityManagerFactory::class,
             GuzzleHttp\Client::class => InvokableFactory::class,
-            Cache::class => CacheFactory::class,
+            Cache::class => Factory\CacheFactory::class,
-            'Logger_Shlink' => LoggerFactory::class,
+            'Logger_Shlink' => Factory\LoggerFactory::class,
 
-            Translator::class => TranslatorFactory::class,
+            Translator::class => Factory\TranslatorFactory::class,
             TranslatorExtension::class => AnnotatedFactory::class,
             LocaleMiddleware::class => AnnotatedFactory::class,
 

module/Common/src/Service/PreviewGenerator.php

@@ -2,18 +2,14 @@
 namespace Shlinkio\Shlink\Common\Service;
 
 use Acelaya\ZsmAnnotatedServices\Annotation\Inject;
-use Doctrine\Common\Cache\Cache;
 use mikehaertl\wkhtmlto\Image;
 use Shlinkio\Shlink\Common\Exception\PreviewGenerationException;
 use Shlinkio\Shlink\Common\Image\ImageBuilder;
 use Shlinkio\Shlink\Common\Image\ImageBuilderInterface;
+use Symfony\Component\Filesystem\Filesystem;
 
 class PreviewGenerator implements PreviewGeneratorInterface
 {
-    /**
-     * @var Cache
-     */
-    private $cache;
     /**
      * @var string
      */
@@ -22,20 +18,24 @@ class PreviewGenerator implements PreviewGeneratorInterface
      * @var ImageBuilderInterface
      */
     private $imageBuilder;
+    /**
+     * @var Filesystem
+     */
+    private $filesystem;
 
     /**
      * PreviewGenerator constructor.
      * @param ImageBuilderInterface $imageBuilder
-     * @param Cache $cache
+     * @param Filesystem $filesystem
      * @param string $location
      *
-     * @Inject({ImageBuilder::class, Cache::class, "config.preview_generation.files_location"})
+     * @Inject({ImageBuilder::class, Filesystem::class, "config.preview_generation.files_location"})
      */
-    public function __construct(ImageBuilderInterface $imageBuilder, Cache $cache, $location)
+    public function __construct(ImageBuilderInterface $imageBuilder, Filesystem $filesystem, $location)
     {
-        $this->cache = $cache;
         $this->location = $location;
         $this->imageBuilder = $imageBuilder;
+        $this->filesystem = $filesystem;
     }
 
     /**
@@ -50,22 +50,21 @@ class PreviewGenerator implements PreviewGeneratorInterface
         /** @var Image $image */
         $image = $this->imageBuilder->build(Image::class, ['url' => $url]);
 
+        // If the file already exists, return its path
         $cacheId = sprintf('preview_%s.%s', urlencode($url), $image->type);
-        if ($this->cache->contains($cacheId)) {
+        $path = $this->location . '/' . $cacheId;
-            return $this->cache->fetch($cacheId);
+        if ($this->filesystem->exists($path)) {
+            return $path;
         }
 
-        $path = $this->location . '/' . $cacheId;
+        // Save and check if an error occurred
         $image->saveAs($path);
-
-        // Check if an error occurred
         $error = $image->getError();
         if (! empty($error)) {
             throw PreviewGenerationException::fromImageError($error);
         }
 
         // Cache the path and return it
-        $this->cache->save($cacheId, $path);
         return $path;
     }
 }

module/Common/test/Service/PreviewGeneratorTest.php

@@ -1,13 +1,13 @@
 <?php
 namespace ShlinkioTest\Shlink\Common\Service;
 
-use Doctrine\Common\Cache\ArrayCache;
 use mikehaertl\wkhtmlto\Image;
 use PHPUnit_Framework_TestCase as TestCase;
 use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Common\Image\ImageBuilder;
 use Shlinkio\Shlink\Common\Service\PreviewGenerator;
+use Symfony\Component\Filesystem\Filesystem;
 use Zend\ServiceManager\ServiceManager;
 
 class PreviewGeneratorTest extends TestCase
@@ -21,49 +21,51 @@ class PreviewGeneratorTest extends TestCase
      */
     protected $image;
     /**
-     * @var ArrayCache
+     * @var ObjectProphecy
      */
-    protected $cache;
+    protected $filesystem;
 
     public function setUp()
     {
         $this->image = $this->prophesize(Image::class);
-        $this->cache = new ArrayCache();
+        $this->filesystem = $this->prophesize(Filesystem::class);
+
         $this->generator = new PreviewGenerator(new ImageBuilder(new ServiceManager(), [
             'factories' => [
                 Image::class => function () {
                     return $this->image->reveal();
                 },
             ]
-        ]), $this->cache, 'dir');
+        ]), $this->filesystem->reveal(), 'dir');
     }
 
     /**
      * @test
      */
-    public function alreadyCachedElementsAreNotProcessed()
+    public function alreadyProcessedElementsAreNotProcessed()
     {
         $url = 'http://foo.com';
-        $this->cache->save(sprintf('preview_%s.png', urlencode($url)), 'dir/foo.png');
+        $this->filesystem->exists(sprintf('dir/preview_%s.png', urlencode($url)))->willReturn(true)
+                                                                                 ->shouldBeCalledTimes(1);
         $this->image->saveAs(Argument::cetera())->shouldBeCalledTimes(0);
-        $this->assertEquals('dir/foo.png', $this->generator->generatePreview($url));
+        $this->assertEquals(sprintf('dir/preview_%s.png', urlencode($url)), $this->generator->generatePreview($url));
     }
 
     /**
      * @test
      */
-    public function nonCachedElementsAreProcessedAndThenCached()
+    public function nonProcessedElementsAreProcessed()
     {
         $url = 'http://foo.com';
         $cacheId = sprintf('preview_%s.png', urlencode($url));
         $expectedPath = 'dir/' . $cacheId;
 
+        $this->filesystem->exists(sprintf('dir/preview_%s.png', urlencode($url)))->willReturn(false)
+                                                                                 ->shouldBeCalledTimes(1);
+
         $this->image->saveAs($expectedPath)->shouldBeCalledTimes(1);
         $this->image->getError()->willReturn('')->shouldBeCalledTimes(1);
-
-        $this->assertFalse($this->cache->contains($cacheId));
         $this->assertEquals($expectedPath, $this->generator->generatePreview($url));
-        $this->assertTrue($this->cache->contains($cacheId));
     }
 
     /**
@@ -76,6 +78,9 @@ class PreviewGeneratorTest extends TestCase
         $cacheId = sprintf('preview_%s.png', urlencode($url));
         $expectedPath = 'dir/' . $cacheId;
 
+        $this->filesystem->exists(sprintf('dir/preview_%s.png', urlencode($url)))->willReturn(false)
+                                                                                 ->shouldBeCalledTimes(1);
+
         $this->image->saveAs($expectedPath)->shouldBeCalledTimes(1);
         $this->image->getError()->willReturn('Error!!')->shouldBeCalledTimes(1);