Merge pull request #579 from trsteel88/error-logging

Update how missing filters are logged

Author: makasim

Controller/ImagineController.php

@@ -3,11 +3,13 @@
 namespace Liip\ImagineBundle\Controller;
 
 use Imagine\Exception\RuntimeException;
+use Liip\ImagineBundle\Exception\Imagine\Filter\NonExistingFilterException;
 use Liip\ImagineBundle\Imagine\Cache\CacheManager;
 use Liip\ImagineBundle\Imagine\Data\DataManager;
 use Liip\ImagineBundle\Imagine\Filter\FilterManager;
 use Liip\ImagineBundle\Exception\Binary\Loader\NotLoadableException;
 use Liip\ImagineBundle\Imagine\Cache\SignerInterface;
+use Psr\Log\LoggerInterface;
 use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
@@ -35,6 +37,11 @@ class ImagineController
      */
     protected $signer;
 
+    /**
+     * @var LoggerInterface
+     */
+    protected $logger;
+
     /**
      * @param DataManager     $dataManager
      * @param FilterManager   $filterManager
@@ -45,12 +52,14 @@ public function __construct(
         DataManager $dataManager,
         FilterManager $filterManager,
         CacheManager $cacheManager,
-        SignerInterface $signer
+        SignerInterface $signer,
+        LoggerInterface $logger = null
     ) {
         $this->dataManager = $dataManager;
         $this->filterManager = $filterManager;
         $this->cacheManager = $cacheManager;
         $this->signer = $signer;
+        $this->logger = $logger;
     }
 
     /**
@@ -87,6 +96,14 @@ public function filterAction(Request $request, $path, $filter)
             }
 
             return new RedirectResponse($this->cacheManager->resolve($path, $filter), 301);
+        } catch (NonExistingFilterException $e) {
+            $message = sprintf('Could not locate filter "%s" for path "%s". Message was "%s"', $filter, $path, $e->getMessage());
+
+            if (null !== $this->logger) {
+                $this->logger->debug($message);
+            }
+
+            throw new NotFoundHttpException($message, $e);
         } catch (RuntimeException $e) {
             throw new \RuntimeException(sprintf('Unable to create image for path "%s" and filter "%s". Message was "%s"', $path, $filter, $e->getMessage()), 0, $e);
         }
@@ -140,6 +157,14 @@ public function filterRuntimeAction(Request $request, $hash, $path, $filter)
             );
 
             return new RedirectResponse($this->cacheManager->resolve($rcPath, $filter), 301);
+        } catch (NonExistingFilterException $e) {
+            $message = sprintf('Could not locate filter "%s" for path "%s". Message was "%s"', $filter, $hash.'/'.$path, $e->getMessage());
+
+            if (null !== $this->logger) {
+                $this->logger->debug($message);
+            }
+
+            throw new NotFoundHttpException($message, $e);
         } catch (RuntimeException $e) {
             throw new \RuntimeException(sprintf('Unable to create image for path "%s" and filter "%s". Message was "%s"', $hash.'/'.$path, $filter, $e->getMessage()), 0, $e);
         }

Exception/Imagine/Filter/NonExistingFilterException.php

@@ -0,0 +1,10 @@
+<?php
+
+namespace Liip\ImagineBundle\Exception\Imagine\Filter;
+
+use Liip\ImagineBundle\Exception\ExceptionInterface;
+
+class NonExistingFilterException extends \RuntimeException implements ExceptionInterface
+{
+
+}

Imagine/Filter/FilterConfiguration.php

@@ -2,6 +2,8 @@
 
 namespace Liip\ImagineBundle\Imagine\Filter;
 
+use Liip\ImagineBundle\Exception\Imagine\Filter\NonExistingFilterException;
+
 class FilterConfiguration
 {
     /**
@@ -24,12 +26,12 @@ public function __construct(array $filters = array())
      *
      * @return array
      *
-     * @throws \RuntimeException
+     * @throws NonExistingFilterException
      */
     public function get($filter)
     {
         if (false == array_key_exists($filter, $this->filters)) {
-            throw new \RuntimeException(sprintf('Could not find configuration for a filter: %s', $filter));
+            throw new NonExistingFilterException(sprintf('Could not find configuration for a filter: %s', $filter));
         }
 
         return $this->filters[$filter];

Resources/config/imagine.xml

@@ -109,6 +109,7 @@
             <argument type="service" id="liip_imagine.filter.manager" />
             <argument type="service" id="liip_imagine.cache.manager" />
             <argument type="service" id="liip_imagine.cache.signer" />
+            <argument type="service" id="logger" on-invalid="ignore" />
         </service>
 
         <service id="liip_imagine.meta_data.reader" class="%liip_imagine.meta_data.reader.class%" public="false" />

Tests/Controller/ImagineControllerTest.php

@@ -18,7 +18,8 @@ public function testCouldBeConstructedWithExpectedServices()
             $this->createDataManagerMock(),
             $this->createFilterManagerMock(),
             $this->createCacheManagerMock(),
-            $this->createSignerMock()
+            $this->createSignerMock(),
+            $this->createLoggerMock()
         );
     }
 
@@ -53,4 +54,12 @@ protected function createSignerMock()
     {
         return $this->getMock('Liip\ImagineBundle\Imagine\Cache\Signer', array(), array(), '', false);
     }
+
+    /**
+     * @return \PHPUnit_Framework_MockObject_MockObject|\Psr\Log\LoggerInterface
+     */
+    protected function createLoggerMock()
+    {
+        return $this->getMock('Psr\Log\LoggerInterface');
+    }
 }

Tests/DependencyInjection/LiipImagineExtensionTest.php

@@ -7,6 +7,7 @@
 use Liip\ImagineBundle\Tests\AbstractTest;
 use Liip\ImagineBundle\DependencyInjection\LiipImagineExtension;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\Yaml\Parser;
 
@@ -45,6 +46,7 @@ public function testLoadWithDefaults()
                 new Reference('liip_imagine.filter.manager'),
                 new Reference('liip_imagine.cache.manager'),
                 new Reference('liip_imagine.cache.signer'),
+                new Reference('logger', ContainerInterface::IGNORE_ON_INVALID_REFERENCE),
             )
         );
     }

Tests/Functional/Controller/ImagineControllerTest.php

@@ -102,6 +102,14 @@ public function testShouldThrowNotFoundHttpExceptionIfFileNotExists()
         $this->client->request('GET', '/media/cache/resolve/thumbnail_web_path/images/shrodinger_cats_which_not_exist.jpeg');
     }
 
+    /**
+     * @expectedException \Symfony\Component\HttpKernel\Exception\NotFoundHttpException
+     */
+    public function testInvalidFilterShouldThrowNotFoundHttpException()
+    {
+        $this->client->request('GET', '/media/cache/resolve/invalid-filter/images/cats.jpeg');
+    }
+
     public function testShouldResolveWithCustomFiltersPopulatingCacheFirst()
     {
         /** @var Signer $signer */