From fac519699aa13944c6a9e122078d2eeddee96b84 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 18 Aug 2016 13:20:57 +0200 Subject: [PATCH] Used filesystem check instead of cache check for preview generation --- composer.json | 1 + .../Shortcode/GeneratePreviewCommand.php | 10 ++++--- module/Common/config/dependencies.config.php | 17 ++++++----- .../Common/src/Service/PreviewGenerator.php | 29 +++++++++---------- .../test/Service/PreviewGeneratorTest.php | 29 +++++++++++-------- 5 files changed, 47 insertions(+), 39 deletions(-) diff --git a/composer.json b/composer.json index a1e8061b..923d6f32 100644 --- a/composer.json +++ b/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", diff --git a/module/CLI/src/Command/Shortcode/GeneratePreviewCommand.php b/module/CLI/src/Command/Shortcode/GeneratePreviewCommand.php index eead3229..358eb034 100644 --- a/module/CLI/src/Command/Shortcode/GeneratePreviewCommand.php +++ b/module/CLI/src/Command/Shortcode/GeneratePreviewCommand.php @@ -78,10 +78,12 @@ class GeneratePreviewCommand extends Command $this->previewGenerator->generatePreview($url); $output->writeln($this->translator->translate(' Success!')); } catch (PreviewGenerationException $e) { - $output->writeln([ - ' ' . $this->translator->translate('Error') . '', - '' . $e->__toString() . '', - ]); + $messages = [' ' . $this->translator->translate('Error') . '']; + if ($output->isVerbose()) { + $messages[] = '' . $e->__toString() . ''; + } + + $output->writeln($messages); } } } diff --git a/module/Common/config/dependencies.config.php b/module/Common/config/dependencies.config.php index a36dddb6..c995b14d 100644 --- a/module/Common/config/dependencies.config.php +++ b/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\EntityManagerFactory; -use Shlinkio\Shlink\Common\Factory\LoggerFactory; -use Shlinkio\Shlink\Common\Factory\TranslatorFactory; +use Shlinkio\Shlink\Common\Factory; 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, - 'Logger_Shlink' => LoggerFactory::class, + Cache::class => Factory\CacheFactory::class, + 'Logger_Shlink' => Factory\LoggerFactory::class, - Translator::class => TranslatorFactory::class, + Translator::class => Factory\TranslatorFactory::class, TranslatorExtension::class => AnnotatedFactory::class, LocaleMiddleware::class => AnnotatedFactory::class, diff --git a/module/Common/src/Service/PreviewGenerator.php b/module/Common/src/Service/PreviewGenerator.php index 5129e256..03fa392f 100644 --- a/module/Common/src/Service/PreviewGenerator.php +++ b/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)) { - return $this->cache->fetch($cacheId); + $path = $this->location . '/' . $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; } } diff --git a/module/Common/test/Service/PreviewGeneratorTest.php b/module/Common/test/Service/PreviewGeneratorTest.php index c9a07706..60b5d024 100644 --- a/module/Common/test/Service/PreviewGeneratorTest.php +++ b/module/Common/test/Service/PreviewGeneratorTest.php @@ -1,13 +1,13 @@ 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);