From 2c91ded51426f9cf26f92b40aed49965ba9425fb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 18 Aug 2016 12:21:26 +0200 Subject: [PATCH] Improved PreviewGenerator by composing an ImageBuilder that creates new Image objects fore each URL --- config/autoload/phpwkhtmltopdf.global.php | 1 - config/autoload/preview-generation.global.php | 8 ++ module/CLI/config/cli.config.php | 1 + module/CLI/config/dependencies.config.php | 1 + .../Shortcode/GeneratePreviewCommand.php | 87 +++++++++++++++++++ module/Common/config/dependencies.config.php | 6 +- module/Common/src/Image/ImageBuilder.php | 10 +++ .../Common/src/Image/ImageBuilderFactory.php | 31 +++++++ .../src/Image/ImageBuilderInterface.php | 8 ++ module/Common/src/Image/ImageFactory.php | 8 +- .../Common/src/Service/PreviewGenerator.php | 28 +++--- .../test/Service/PreviewGeneratorTest.php | 12 ++- 12 files changed, 181 insertions(+), 20 deletions(-) create mode 100644 config/autoload/preview-generation.global.php create mode 100644 module/CLI/src/Command/Shortcode/GeneratePreviewCommand.php create mode 100644 module/Common/src/Image/ImageBuilder.php create mode 100644 module/Common/src/Image/ImageBuilderFactory.php create mode 100644 module/Common/src/Image/ImageBuilderInterface.php diff --git a/config/autoload/phpwkhtmltopdf.global.php b/config/autoload/phpwkhtmltopdf.global.php index e10d0300..f7c29f16 100644 --- a/config/autoload/phpwkhtmltopdf.global.php +++ b/config/autoload/phpwkhtmltopdf.global.php @@ -2,7 +2,6 @@ return [ 'phpwkhtmltopdf' => [ - 'files_location' => 'data/cache', 'images' => [ 'binary' => 'bin/wkhtmltoimage', 'type' => 'jpg', diff --git a/config/autoload/preview-generation.global.php b/config/autoload/preview-generation.global.php new file mode 100644 index 00000000..b4f14da3 --- /dev/null +++ b/config/autoload/preview-generation.global.php @@ -0,0 +1,8 @@ + [ + 'files_location' => 'data/cache', + ], + +]; diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index 31c4e460..8bd88607 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -10,6 +10,7 @@ return [ Command\Shortcode\ResolveUrlCommand::class, Command\Shortcode\ListShortcodesCommand::class, Command\Shortcode\GetVisitsCommand::class, + Command\Shortcode\GeneratePreviewCommand::class, Command\Visit\ProcessVisitsCommand::class, Command\Config\GenerateCharsetCommand::class, Command\Config\GenerateSecretCommand::class, diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index ebc607c8..00e56607 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -14,6 +14,7 @@ return [ Command\Shortcode\ResolveUrlCommand::class => AnnotatedFactory::class, Command\Shortcode\ListShortcodesCommand::class => AnnotatedFactory::class, Command\Shortcode\GetVisitsCommand::class => AnnotatedFactory::class, + Command\Shortcode\GeneratePreviewCommand::class => AnnotatedFactory::class, Command\Visit\ProcessVisitsCommand::class => AnnotatedFactory::class, Command\Config\GenerateCharsetCommand::class => AnnotatedFactory::class, Command\Config\GenerateSecretCommand::class => AnnotatedFactory::class, diff --git a/module/CLI/src/Command/Shortcode/GeneratePreviewCommand.php b/module/CLI/src/Command/Shortcode/GeneratePreviewCommand.php new file mode 100644 index 00000000..eead3229 --- /dev/null +++ b/module/CLI/src/Command/Shortcode/GeneratePreviewCommand.php @@ -0,0 +1,87 @@ +previewGenerator = $previewGenerator; + $this->translator = $translator; + $this->shortUrlService = $shortUrlService; + parent::__construct(null); + } + + public function configure() + { + $this->setName('shortcode:process-previews') + ->setDescription( + $this->translator->translate( + 'Processes and generates the previews for every URL, improving performance for later web requests.' + ) + ); + } + + public function execute(InputInterface $input, OutputInterface $output) + { + $page = 1; + do { + $shortUrls = $this->shortUrlService->listShortUrls($page); + $page += 1; + + foreach ($shortUrls as $shortUrl) { + $this->processUrl($shortUrl->getOriginalUrl(), $output); + } + } while ($page <= $shortUrls->count()); + + $output->writeln('' . $this->translator->translate('Finished processing all URLs') . ''); + } + + protected function processUrl($url, OutputInterface $output) + { + try { + $output->write(sprintf($this->translator->translate('Processing URL %s...'), $url)); + $this->previewGenerator->generatePreview($url); + $output->writeln($this->translator->translate(' Success!')); + } catch (PreviewGenerationException $e) { + $output->writeln([ + ' ' . $this->translator->translate('Error') . '', + '' . $e->__toString() . '', + ]); + } + } +} diff --git a/module/Common/config/dependencies.config.php b/module/Common/config/dependencies.config.php index 6188d4ad..a36dddb6 100644 --- a/module/Common/config/dependencies.config.php +++ b/module/Common/config/dependencies.config.php @@ -2,14 +2,13 @@ use Acelaya\ZsmAnnotatedServices\Factory\V3\AnnotatedFactory; use Doctrine\Common\Cache\Cache; use Doctrine\ORM\EntityManager; -use mikehaertl\wkhtmlto\Image; 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\Image\ImageFactory; +use Shlinkio\Shlink\Common\Image; use Shlinkio\Shlink\Common\Middleware\LocaleMiddleware; use Shlinkio\Shlink\Common\Service; use Shlinkio\Shlink\Common\Twig\Extension\TranslatorExtension; @@ -24,12 +23,13 @@ return [ GuzzleHttp\Client::class => InvokableFactory::class, Cache::class => CacheFactory::class, 'Logger_Shlink' => LoggerFactory::class, - Image::class => ImageFactory::class, Translator::class => TranslatorFactory::class, TranslatorExtension::class => AnnotatedFactory::class, LocaleMiddleware::class => AnnotatedFactory::class, + Image\ImageBuilder::class => Image\ImageBuilderFactory::class, + Service\IpLocationResolver::class => AnnotatedFactory::class, Service\PreviewGenerator::class => AnnotatedFactory::class, ], diff --git a/module/Common/src/Image/ImageBuilder.php b/module/Common/src/Image/ImageBuilder.php new file mode 100644 index 00000000..0e4de841 --- /dev/null +++ b/module/Common/src/Image/ImageBuilder.php @@ -0,0 +1,10 @@ + [ + Image::class => ImageFactory::class, + ]]); + } +} diff --git a/module/Common/src/Image/ImageBuilderInterface.php b/module/Common/src/Image/ImageBuilderInterface.php new file mode 100644 index 00000000..a1479a81 --- /dev/null +++ b/module/Common/src/Image/ImageBuilderInterface.php @@ -0,0 +1,8 @@ +get('config')['phpwkhtmltopdf']; - return new Image(isset($config['images']) ? $config['images'] : null); + $image = new Image(isset($config['images']) ? $config['images'] : null); + + if (isset($options['url'])) { + $image->setPage($options['url']); + } + + return $image; } } diff --git a/module/Common/src/Service/PreviewGenerator.php b/module/Common/src/Service/PreviewGenerator.php index 07ecba76..5129e256 100644 --- a/module/Common/src/Service/PreviewGenerator.php +++ b/module/Common/src/Service/PreviewGenerator.php @@ -5,13 +5,11 @@ 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; class PreviewGenerator implements PreviewGeneratorInterface { - /** - * @var Image - */ - private $image; /** * @var Cache */ @@ -20,20 +18,24 @@ class PreviewGenerator implements PreviewGeneratorInterface * @var string */ private $location; + /** + * @var ImageBuilderInterface + */ + private $imageBuilder; /** * PreviewGenerator constructor. - * @param Image $image + * @param ImageBuilderInterface $imageBuilder * @param Cache $cache * @param string $location * - * @Inject({Image::class, Cache::class, "config.phpwkhtmltopdf.files_location"}) + * @Inject({ImageBuilder::class, Cache::class, "config.preview_generation.files_location"}) */ - public function __construct(Image $image, Cache $cache, $location) + public function __construct(ImageBuilderInterface $imageBuilder, Cache $cache, $location) { - $this->image = $image; $this->cache = $cache; $this->location = $location; + $this->imageBuilder = $imageBuilder; } /** @@ -45,17 +47,19 @@ class PreviewGenerator implements PreviewGeneratorInterface */ public function generatePreview($url) { - $cacheId = sprintf('preview_%s.%s', urlencode($url), $this->image->type); + /** @var Image $image */ + $image = $this->imageBuilder->build(Image::class, ['url' => $url]); + + $cacheId = sprintf('preview_%s.%s', urlencode($url), $image->type); if ($this->cache->contains($cacheId)) { return $this->cache->fetch($cacheId); } $path = $this->location . '/' . $cacheId; - $this->image->setPage($url); - $this->image->saveAs($path); + $image->saveAs($path); // Check if an error occurred - $error = $this->image->getError(); + $error = $image->getError(); if (! empty($error)) { throw PreviewGenerationException::fromImageError($error); } diff --git a/module/Common/test/Service/PreviewGeneratorTest.php b/module/Common/test/Service/PreviewGeneratorTest.php index b699293f..c9a07706 100644 --- a/module/Common/test/Service/PreviewGeneratorTest.php +++ b/module/Common/test/Service/PreviewGeneratorTest.php @@ -6,7 +6,9 @@ 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 Zend\ServiceManager\ServiceManager; class PreviewGeneratorTest extends TestCase { @@ -27,7 +29,13 @@ class PreviewGeneratorTest extends TestCase { $this->image = $this->prophesize(Image::class); $this->cache = new ArrayCache(); - $this->generator = new PreviewGenerator($this->image->reveal(), $this->cache, 'dir'); + $this->generator = new PreviewGenerator(new ImageBuilder(new ServiceManager(), [ + 'factories' => [ + Image::class => function () { + return $this->image->reveal(); + }, + ] + ]), $this->cache, 'dir'); } /** @@ -50,7 +58,6 @@ class PreviewGeneratorTest extends TestCase $cacheId = sprintf('preview_%s.png', urlencode($url)); $expectedPath = 'dir/' . $cacheId; - $this->image->setPage($url)->shouldBeCalledTimes(1); $this->image->saveAs($expectedPath)->shouldBeCalledTimes(1); $this->image->getError()->willReturn('')->shouldBeCalledTimes(1); @@ -69,7 +76,6 @@ class PreviewGeneratorTest extends TestCase $cacheId = sprintf('preview_%s.png', urlencode($url)); $expectedPath = 'dir/' . $cacheId; - $this->image->setPage($url)->shouldBeCalledTimes(1); $this->image->saveAs($expectedPath)->shouldBeCalledTimes(1); $this->image->getError()->willReturn('Error!!')->shouldBeCalledTimes(1);