From 60c68c914bfa4af80c02c52a019d5def9542e9ed Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Thu, 18 Aug 2016 11:16:56 +0200
Subject: [PATCH] Managed error while generating URL previews by throwing an
 exception

---
 config/autoload/phpwkhtmltopdf.global.php      |  3 ++-
 .../Exception/PreviewGenerationException.php   | 10 ++++++++++
 module/Common/src/Service/PreviewGenerator.php | 15 ++++++++++++---
 .../src/Service/PreviewGeneratorInterface.php  |  3 +++
 .../test/Service/PreviewGeneratorTest.php      | 18 ++++++++++++++++++
 module/Core/src/Action/PreviewAction.php       |  3 +++
 6 files changed, 48 insertions(+), 4 deletions(-)
 create mode 100644 module/Common/src/Exception/PreviewGenerationException.php

diff --git a/config/autoload/phpwkhtmltopdf.global.php b/config/autoload/phpwkhtmltopdf.global.php
index 6029b973..e10d0300 100644
--- a/config/autoload/phpwkhtmltopdf.global.php
+++ b/config/autoload/phpwkhtmltopdf.global.php
@@ -2,9 +2,10 @@
 return [
 
     'phpwkhtmltopdf' => [
+        'files_location' => 'data/cache',
         'images' => [
             'binary' => 'bin/wkhtmltoimage',
-            'files_location' => 'data/cache/previews',
+            'type' => 'jpg',
         ],
     ],
 
diff --git a/module/Common/src/Exception/PreviewGenerationException.php b/module/Common/src/Exception/PreviewGenerationException.php
new file mode 100644
index 00000000..ac205495
--- /dev/null
+++ b/module/Common/src/Exception/PreviewGenerationException.php
@@ -0,0 +1,10 @@
+<?php
+namespace Shlinkio\Shlink\Common\Exception;
+
+class PreviewGenerationException extends RuntimeException
+{
+    public static function fromImageError($error)
+    {
+        return new self(sprintf('Error generating a preview image with error: %s', $error));
+    }
+}
diff --git a/module/Common/src/Service/PreviewGenerator.php b/module/Common/src/Service/PreviewGenerator.php
index 0a902967..07ecba76 100644
--- a/module/Common/src/Service/PreviewGenerator.php
+++ b/module/Common/src/Service/PreviewGenerator.php
@@ -4,6 +4,7 @@ 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;
 
 class PreviewGenerator implements PreviewGeneratorInterface
 {
@@ -26,7 +27,7 @@ class PreviewGenerator implements PreviewGeneratorInterface
      * @param Cache $cache
      * @param string $location
      *
-     * @Inject({Image::class, Cache::class, "config.phpwkhtmltopdf.images.files_location"})
+     * @Inject({Image::class, Cache::class, "config.phpwkhtmltopdf.files_location"})
      */
     public function __construct(Image $image, Cache $cache, $location)
     {
@@ -40,10 +41,11 @@ class PreviewGenerator implements PreviewGeneratorInterface
      *
      * @param string $url
      * @return string
+     * @throws PreviewGenerationException
      */
     public function generatePreview($url)
     {
-        $cacheId = sprintf('preview_%s.png', urlencode($url));
+        $cacheId = sprintf('preview_%s.%s', urlencode($url), $this->image->type);
         if ($this->cache->contains($cacheId)) {
             return $this->cache->fetch($cacheId);
         }
@@ -51,8 +53,15 @@ class PreviewGenerator implements PreviewGeneratorInterface
         $path = $this->location . '/' . $cacheId;
         $this->image->setPage($url);
         $this->image->saveAs($path);
-        $this->cache->save($cacheId, $path);
 
+        // Check if an error occurred
+        $error = $this->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/src/Service/PreviewGeneratorInterface.php b/module/Common/src/Service/PreviewGeneratorInterface.php
index 3029eb97..2e7ea0aa 100644
--- a/module/Common/src/Service/PreviewGeneratorInterface.php
+++ b/module/Common/src/Service/PreviewGeneratorInterface.php
@@ -1,6 +1,8 @@
 <?php
 namespace Shlinkio\Shlink\Common\Service;
 
+use Shlinkio\Shlink\Common\Exception\PreviewGenerationException;
+
 interface PreviewGeneratorInterface
 {
     /**
@@ -8,6 +10,7 @@ interface PreviewGeneratorInterface
      *
      * @param string $url
      * @return string
+     * @throws PreviewGenerationException
      */
     public function generatePreview($url);
 }
diff --git a/module/Common/test/Service/PreviewGeneratorTest.php b/module/Common/test/Service/PreviewGeneratorTest.php
index 6cb6763a..b699293f 100644
--- a/module/Common/test/Service/PreviewGeneratorTest.php
+++ b/module/Common/test/Service/PreviewGeneratorTest.php
@@ -52,9 +52,27 @@ class PreviewGeneratorTest extends TestCase
 
         $this->image->setPage($url)->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));
     }
+
+    /**
+     * @test
+     * @expectedException \Shlinkio\Shlink\Common\Exception\PreviewGenerationException
+     */
+    public function errorWhileGeneratingPreviewThrowsException()
+    {
+        $url = 'http://foo.com';
+        $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);
+
+        $this->generator->generatePreview($url);
+    }
 }
diff --git a/module/Core/src/Action/PreviewAction.php b/module/Core/src/Action/PreviewAction.php
index 3c030e50..4c21501c 100644
--- a/module/Core/src/Action/PreviewAction.php
+++ b/module/Core/src/Action/PreviewAction.php
@@ -4,6 +4,7 @@ namespace Shlinkio\Shlink\Core\Action;
 use Acelaya\ZsmAnnotatedServices\Annotation\Inject;
 use Psr\Http\Message\ResponseInterface as Response;
 use Psr\Http\Message\ServerRequestInterface as Request;
+use Shlinkio\Shlink\Common\Exception\PreviewGenerationException;
 use Shlinkio\Shlink\Common\Service\PreviewGenerator;
 use Shlinkio\Shlink\Common\Service\PreviewGeneratorInterface;
 use Shlinkio\Shlink\Common\Util\ResponseUtilsTrait;
@@ -77,6 +78,8 @@ class PreviewAction implements MiddlewareInterface
             return $this->generateImageResponse($imagePath);
         } catch (InvalidShortCodeException $e) {
             return $out($request, $response->withStatus(404), 'Not found');
+        } catch (PreviewGenerationException $e) {
+            return $out($request, $response->withStatus(500), 'Preview generation error');
         }
     }
 }