container compiler pass logging and metadata reader cleanup

liip · Feb 1, 2017 · 77f1c1d · 77f1c1d
1 parent 7c0f500
commit 77f1c1d
Show file tree

Hide file tree

Showing 11 changed files with 183 additions and 93 deletions.
diff --git a/DependencyInjection/Compiler/AbstractCompilerPass.php b/DependencyInjection/Compiler/AbstractCompilerPass.php
@@ -0,0 +1,36 @@
+<?php
+
+/*
+ * This file is part of the `liip/LiipImagineBundle` project.
+ *
+ * (c) https://github.com/liip/LiipImagineBundle/graphs/contributors
+ *
+ * For the full copyright and license information, please view the LICENSE.md
+ * file that was distributed with this source code.
+ */
+
+namespace Liip\ImagineBundle\DependencyInjection\Compiler;
+
+use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+abstract class AbstractCompilerPass implements CompilerPassInterface
+{
+    /**
+     * {@inheritdoc}
+     */
+    protected function log(ContainerBuilder $container, $message, array $replacements = array())
+    {
+        if (count($replacements) > 0) {
+            $message = vsprintf($message, $replacements);
+        }
+
+        if (method_exists($container, 'log')) {
+            $container->log($this, $message);
+        } else {
+            $compiler = $container->getCompiler();
+            $formatter = $compiler->getLoggingFormatter();
+            $compiler->addLogMessage($formatter->format($this, $message));
+        }
+    }
+}
diff --git a/DependencyInjection/Compiler/FiltersCompilerPass.php b/DependencyInjection/Compiler/FiltersCompilerPass.php
@@ -11,11 +11,10 @@
 
 namespace Liip\ImagineBundle\DependencyInjection\Compiler;
 
-use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Reference;
 
-class FiltersCompilerPass implements CompilerPassInterface
+class FiltersCompilerPass extends AbstractCompilerPass
 {
     /**
      * {@inheritdoc}
@@ -29,6 +28,7 @@ public function process(ContainerBuilder $container)
 
             foreach ($tags as $id => $tag) {
                 $manager->addMethodCall('addLoader', array($tag[0]['loader'], new Reference($id)));
+                $this->log($container, 'Registered imagine-bimdle filter loader: %s', array($id));
             }
         }
     }

diff --git a/DependencyInjection/Compiler/LoadersCompilerPass.php b/DependencyInjection/Compiler/LoadersCompilerPass.php
@@ -11,11 +11,10 @@
 
 namespace Liip\ImagineBundle\DependencyInjection\Compiler;
 
-use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Reference;
 
-class LoadersCompilerPass implements CompilerPassInterface
+class LoadersCompilerPass extends AbstractCompilerPass
 {
     /**
      * {@inheritdoc}
@@ -29,6 +28,7 @@ public function process(ContainerBuilder $container)
 
             foreach ($tags as $id => $tag) {
                 $manager->addMethodCall('addLoader', array($tag[0]['loader'], new Reference($id)));
+                $this->log($container, 'Registered imagine-bimdle binary loader: %s', array($id));
             }
         }
     }

diff --git a/DependencyInjection/Compiler/MetadataReaderCompilerPass.php b/DependencyInjection/Compiler/MetadataReaderCompilerPass.php
@@ -11,30 +11,44 @@
 
 namespace Liip\ImagineBundle\DependencyInjection\Compiler;
 
-use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 
 /**
- * By default, a metadata reader based on the exif php extension is used.
- * This compiler pass checks if the extension is loaded an switches to a simpler
- * implementation if not.
+ * Be default, a metadata reader that requires the "exif" PHP extension is used. This compiler pass checks if the
+ * extension is loaded or not, and switches to a metadata reader (that does not rely on "exif") if not.
  */
-class MetadataReaderCompilerPass implements CompilerPassInterface
+class MetadataReaderCompilerPass extends AbstractCompilerPass
 {
-    const METADATA_READER_PARAM = 'liip_imagine.meta_data.reader.class';
+    /**
+     * @var string
+     */
+    private static $metadataReaderParameter = 'liip_imagine.meta_data.reader.class';
 
-    const DEFAULT_METADATA_READER_CLASS = 'Imagine\Image\Metadata\DefaultMetadataReader';
+    /**
+     * @var string
+     */
+    private static $metadataReaderDefaultClass = 'Imagine\Image\Metadata\DefaultMetadataReader';
 
-    const EXIF_METADATA_READER_CLASS = 'Imagine\Image\Metadata\ExifMetadataReader';
+    /**
+     * @var string
+     */
+    private static $metadataReaderExifClass = 'Imagine\Image\Metadata\ExifMetadataReader';
 
     /**
      * {@inheritdoc}
      */
     public function process(ContainerBuilder $container)
     {
-        if (!$this->extExifIsAvailable() && $this->isDefaultMetadataReader($container)) {
-            $container->setParameter(self::METADATA_READER_PARAM, self::DEFAULT_METADATA_READER_CLASS);
-            $this->logMetadataReaderReplaced($container);
+        if (!$this->isExifExtensionLoaded() && $this->isExifMetadataReaderSet($container)) {
+            $container->setParameter(self::$metadataReaderParameter, self::$metadataReaderDefaultClass);
+            $message = 'Overwrote "%s" parameter value from "%s" to "%s" due to missing "exif" extension '
+                .'(installing the "exif" extension is highly recommended; you may experience degraded '
+                .'metadata handling without it)';
+            $this->log($container, $message, array(
+                self::$metadataReaderParameter,
+                self::$metadataReaderExifClass,
+                self::$metadataReaderDefaultClass,
+            ));
         }
     }
 
@@ -43,32 +57,15 @@ public function process(ContainerBuilder $container)
      *
      * @return bool
      */
-    protected function isDefaultMetadataReader(ContainerBuilder $container)
-    {
-        $currentMetadataReaderParameter = $container->getParameter(self::METADATA_READER_PARAM);
-
-        return $currentMetadataReaderParameter === self::EXIF_METADATA_READER_CLASS;
-    }
-
-    /**
-     * @param ContainerBuilder $container
-     */
-    protected function logMetadataReaderReplaced(ContainerBuilder $container)
+    private function isExifMetadataReaderSet(ContainerBuilder $container)
     {
-        $compiler = $container->getCompiler();
-        $formatter = $compiler->getLoggingFormatter();
-        $message = 'Automatically replaced Imagine ExifMetadataReader with DefaultMetadataReader; '.
-            'you might experience issues with LiipImagineBundle; reason: PHP extension "exif" is missing; solution: '.
-            'for advanced metadata extraction install the PHP extension "exif" or set a custom MetadataReader '.
-            'through the "liip_imagine.meta_data.reader.class" parameter';
-
-        $compiler->addLogMessage($formatter->format($this, $message));
+        return $container->getParameter(self::$metadataReaderParameter) === self::$metadataReaderExifClass;
     }
 
     /**
      * @return bool
      */
-    protected function extExifIsAvailable()
+    protected function isExifExtensionLoaded()
     {
         return extension_loaded('exif');
     }

diff --git a/DependencyInjection/Compiler/PostProcessorsCompilerPass.php b/DependencyInjection/Compiler/PostProcessorsCompilerPass.php
@@ -11,7 +11,6 @@
 
 namespace Liip\ImagineBundle\DependencyInjection\Compiler;
 
-use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Reference;
 
@@ -20,7 +19,7 @@
  *
  * @author Konstantin Tjuterev <[email protected]>
  */
-class PostProcessorsCompilerPass implements CompilerPassInterface
+class PostProcessorsCompilerPass extends AbstractCompilerPass
 {
     /**
      * {@inheritdoc}
@@ -34,6 +33,7 @@ public function process(ContainerBuilder $container)
 
             foreach ($tags as $id => $tag) {
                 $manager->addMethodCall('addPostProcessor', array($tag[0]['post_processor'], new Reference($id)));
+                $this->log($container, 'Registered imagine-bimdle filter post-processor: %s', array($id));
             }
         }
     }

diff --git a/DependencyInjection/Compiler/ResolversCompilerPass.php b/DependencyInjection/Compiler/ResolversCompilerPass.php
@@ -11,11 +11,10 @@
 
 namespace Liip\ImagineBundle\DependencyInjection\Compiler;
 
-use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Reference;
 
-class ResolversCompilerPass implements CompilerPassInterface
+class ResolversCompilerPass extends AbstractCompilerPass
 {
     /**
      * {@inheritdoc}
@@ -29,6 +28,7 @@ public function process(ContainerBuilder $container)
 
             foreach ($tags as $id => $tag) {
                 $manager->addMethodCall('addResolver', array($tag[0]['resolver'], new Reference($id)));
+                $this->log($container, 'Registered imagine-bimdle cache resolver: %s', array($id));
             }
         }
     }

diff --git a/Tests/DependencyInjection/Compiler/FiltersCompilerPassTest.php b/Tests/DependencyInjection/Compiler/FiltersCompilerPassTest.php
@@ -16,7 +16,8 @@
 use Symfony\Component\DependencyInjection\Definition;
 
 /**
- * @covers Liip\ImagineBundle\DependencyInjection\Compiler\FiltersCompilerPass
+ * @covers \Liip\ImagineBundle\DependencyInjection\Compiler\AbstractCompilerPass
+ * @covers \Liip\ImagineBundle\DependencyInjection\Compiler\FiltersCompilerPass
  */
 class FiltersCompilerPassTest extends \PHPUnit_Framework_TestCase
 {

diff --git a/Tests/DependencyInjection/Compiler/LoadersCompilerPassTest.php b/Tests/DependencyInjection/Compiler/LoadersCompilerPassTest.php
@@ -16,7 +16,8 @@
 use Symfony\Component\DependencyInjection\Definition;
 
 /**
- * @covers Liip\ImagineBundle\DependencyInjection\Compiler\LoadersCompilerPass
+ * @covers \Liip\ImagineBundle\DependencyInjection\Compiler\AbstractCompilerPass
+ * @covers \Liip\ImagineBundle\DependencyInjection\Compiler\LoadersCompilerPass
  */
 class LoadersCompilerPassTest extends \PHPUnit_Framework_TestCase
 {

diff --git a/Tests/DependencyInjection/Compiler/MetadataReaderCompilerPassTest.php b/Tests/DependencyInjection/Compiler/MetadataReaderCompilerPassTest.php
@@ -15,71 +15,83 @@
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 
 /**
- * @covers Liip\ImagineBundle\DependencyInjection\Compiler\MetadataReaderCompilerPass
+ * @covers \Liip\ImagineBundle\DependencyInjection\Compiler\AbstractCompilerPass
+ * @covers \Liip\ImagineBundle\DependencyInjection\Compiler\MetadataReaderCompilerPass
  */
 class MetadataReaderCompilerPassTest extends \PHPUnit_Framework_TestCase
 {
+    /**
+     * @param \ReflectionClass $r
+     * @param string           $p
+     *
+     * @return string
+     */
+    private static function getPrivateStaticProperty(\ReflectionClass $r, $p)
+    {
+        $property = $r->getProperty($p);
+        $property->setAccessible(true);
+
+        return $property->getValue();
+    }
+
+    /**
+     * @return mixed[]
+     */
+    private static function getReaderParamAndDefaultAndExifValues()
+    {
+        $r = new \ReflectionClass('Liip\ImagineBundle\DependencyInjection\Compiler\MetadataReaderCompilerPass');
+
+        return [
+            static::getPrivateStaticProperty($r, 'metadataReaderParameter'),
+            static::getPrivateStaticProperty($r, 'metadataReaderExifClass'),
+            static::getPrivateStaticProperty($r, 'metadataReaderDefaultClass'),
+        ];
+    }
+
+    /**
+     * @param bool $return
+     *
+     * @return \PHPUnit_Framework_MockObject_MockObject|MetadataReaderCompilerPass
+     */
+    private function getMetadataReaderCompilerPass($return)
+    {
+        $mock = $this->getMockBuilder('Liip\ImagineBundle\DependencyInjection\Compiler\MetadataReaderCompilerPass')
+            ->setMethods(array('isExifExtensionLoaded'))
+            ->getMock();
+
+        $mock
+            ->expects($this->atLeastOnce())
+            ->method('isExifExtensionLoaded')
+            ->willReturn($return);
+
+        return $mock;
+    }
+
     public function testProcessWithoutExtExifAddsDefaultReader()
     {
+        list($metadataParameter, $metadataExifClass, $metadataDefaultClass) = static::getReaderParamAndDefaultAndExifValues();
+
         $container = new ContainerBuilder();
-        $container->setParameter(
-            MetadataReaderCompilerPass::METADATA_READER_PARAM,
-            MetadataReaderCompilerPass::EXIF_METADATA_READER_CLASS
-        );
-
-        /** @var MetadataReaderCompilerPass $pass */
-        $pass = $this->getMock(
-            'Liip\ImagineBundle\DependencyInjection\Compiler\MetadataReaderCompilerPass',
-            array('extExifIsAvailable'),
-            array()
-        );
-        $pass->expects($this->once())
-            ->method('extExifIsAvailable')
-            ->willReturn(false);
-
-        //guard
-        $this->assertEquals(
-            MetadataReaderCompilerPass::EXIF_METADATA_READER_CLASS,
-            $container->getParameter(MetadataReaderCompilerPass::METADATA_READER_PARAM)
-        );
+        $container->setParameter($metadataParameter, $metadataExifClass);
 
-        $pass->process($container);
+        $pass = $this->getMetadataReaderCompilerPass(false);
+        $this->assertEquals($metadataExifClass, $container->getParameter($metadataParameter));
 
-        $this->assertEquals(
-            MetadataReaderCompilerPass::DEFAULT_METADATA_READER_CLASS,
-            $container->getParameter(MetadataReaderCompilerPass::METADATA_READER_PARAM)
-        );
+        $pass->process($container);
+        $this->assertEquals($metadataDefaultClass, $container->getParameter($metadataParameter));
     }
 
     public function testProcessWithExtExifKeepsExifReader()
     {
+        list($metadataParameter, $metadataExifClass) = static::getReaderParamAndDefaultAndExifValues();
+
         $container = new ContainerBuilder();
-        $container->setParameter(
-            MetadataReaderCompilerPass::METADATA_READER_PARAM,
-            MetadataReaderCompilerPass::EXIF_METADATA_READER_CLASS
-        );
-
-        /** @var MetadataReaderCompilerPass $pass */
-        $pass = $this->getMock(
-            'Liip\ImagineBundle\DependencyInjection\Compiler\MetadataReaderCompilerPass',
-            array('extExifIsAvailable'),
-            array()
-        );
-        $pass->expects($this->once())
-            ->method('extExifIsAvailable')
-            ->willReturn(true);
-
-        //guard
-        $this->assertEquals(
-            MetadataReaderCompilerPass::EXIF_METADATA_READER_CLASS,
-            $container->getParameter(MetadataReaderCompilerPass::METADATA_READER_PARAM)
-        );
+        $container->setParameter($metadataParameter, $metadataExifClass);
 
-        $pass->process($container);
+        $pass = static::getMetadataReaderCompilerPass(true);
+        $this->assertEquals($metadataExifClass, $container->getParameter($metadataParameter));
 
-        $this->assertEquals(
-            MetadataReaderCompilerPass::EXIF_METADATA_READER_CLASS,
-            $container->getParameter(MetadataReaderCompilerPass::METADATA_READER_PARAM)
-        );
+        $pass->process($container);
+        $this->assertEquals($metadataExifClass, $container->getParameter($metadataParameter));
     }
 }