locator services must be non-shared

liip · Feb 12, 2017 · 0695ad0 · 0695ad0
1 parent 94eebdf
commit 0695ad0
Show file tree

Hide file tree

Showing 14 changed files with 359 additions and 86 deletions.
diff --git a/Binary/Locator/FileSystemLocator.php b/Binary/Locator/FileSystemLocator.php
@@ -21,7 +21,7 @@ class FileSystemLocator implements LocatorInterface
     /**
      * @var string[]
      */
-    protected $roots;
+    private $roots;
 
     /**
      * @param array[] $options

diff --git a/DependencyInjection/Compiler/LocatorsCompilerPass.php b/DependencyInjection/Compiler/LocatorsCompilerPass.php
@@ -0,0 +1,41 @@
+<?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 Liip\ImagineBundle\Utility\Framework\SymfonyFramework;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
+
+class LocatorsCompilerPass extends AbstractCompilerPass
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function process(ContainerBuilder $container)
+    {
+        foreach (array_keys($container->findTaggedServiceIds('liip_imagine.binary.locator')) as $id) {
+            $this->disableSharedDefinition($container->getDefinition($id));
+        }
+    }
+
+    /**
+     * @param Definition $definition
+     */
+    private function disableSharedDefinition(Definition $definition)
+    {
+        if (SymfonyFramework::hasDefinitionSharedToggle()) {
+            $definition->setShared(false);
+        } else {
+            $definition->setScope('prototype');
+        }
+    }
+}
diff --git a/DependencyInjection/Factory/Loader/FileSystemLoaderFactory.php b/DependencyInjection/Factory/Loader/FileSystemLoaderFactory.php
@@ -11,8 +11,10 @@
 
 namespace Liip\ImagineBundle\DependencyInjection\Factory\Loader;
 
+use Liip\ImagineBundle\Utility\Framework\SymfonyFramework;
 use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\DependencyInjection\Reference;
 
 class FileSystemLoaderFactory extends AbstractLoaderFactory
@@ -24,7 +26,7 @@ public function create(ContainerBuilder $container, $loaderName, array $config)
     {
         $definition = $this->getChildLoaderDefinition();
         $definition->replaceArgument(2, $config['data_root']);
-        $definition->replaceArgument(3, new Reference(sprintf('liip_imagine.binary.locator.%s', $config['locator'])));
+        $definition->replaceArgument(3, $this->createLocatorReference($config['locator']));
 
         return $this->setTaggedLoaderDefinition($loaderName, $definition, $container);
     }
@@ -61,4 +63,18 @@ public function addConfiguration(ArrayNodeDefinition $builder)
                 ->end()
             ->end();
     }
+
+    /**
+     * @param string $reference
+     *
+     * @return Reference
+     */
+    private function createLocatorReference($reference)
+    {
+        return new Reference(
+            sprintf('liip_imagine.binary.locator.%s', $reference),
+            ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE,
+            SymfonyFramework::hasDefinitionSharedToggle()
+        );
+    }
 }
diff --git a/LiipImagineBundle.php b/LiipImagineBundle.php
@@ -13,6 +13,7 @@
 
 use Liip\ImagineBundle\DependencyInjection\Compiler\FiltersCompilerPass;
 use Liip\ImagineBundle\DependencyInjection\Compiler\LoadersCompilerPass;
+use Liip\ImagineBundle\DependencyInjection\Compiler\LocatorsCompilerPass;
 use Liip\ImagineBundle\DependencyInjection\Compiler\MetadataReaderCompilerPass;
 use Liip\ImagineBundle\DependencyInjection\Compiler\PostProcessorsCompilerPass;
 use Liip\ImagineBundle\DependencyInjection\Compiler\ResolversCompilerPass;
@@ -35,6 +36,7 @@ public function build(ContainerBuilder $container)
     {
         parent::build($container);
 
+        $container->addCompilerPass(new LocatorsCompilerPass());
         $container->addCompilerPass(new LoadersCompilerPass());
         $container->addCompilerPass(new FiltersCompilerPass());
         $container->addCompilerPass(new PostProcessorsCompilerPass());

diff --git a/Resources/config/imagine.xml b/Resources/config/imagine.xml
@@ -247,9 +247,11 @@
         <!-- Data loader locators -->
 
         <service id="liip_imagine.binary.locator.filesystem" class="%liip_imagine.binary.locator.filesystem.class%" public="false">
+            <tag name="liip_imagine.binary.locator" />
         </service>
 
         <service id="liip_imagine.binary.locator.filesystem_insecure" class="%liip_imagine.binary.locator.filesystem_insecure.class%" public="false">
+            <tag name="liip_imagine.binary.locator" />
         </service>
 
         <!-- Cache resolver -->

diff --git a/Tests/DependencyInjection/Compiler/LocatorsCompilerPassTest.php b/Tests/DependencyInjection/Compiler/LocatorsCompilerPassTest.php
@@ -0,0 +1,51 @@
+<?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\Tests\DependencyInjection\Compiler;
+
+use Liip\ImagineBundle\DependencyInjection\Compiler\LocatorsCompilerPass;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
+
+/**
+ * @covers \Liip\ImagineBundle\DependencyInjection\Compiler\AbstractCompilerPass
+ * @covers \Liip\ImagineBundle\DependencyInjection\Compiler\LocatorsCompilerPass
+ */
+class LocatorsCompilerPassTest extends \PHPUnit_Framework_TestCase
+{
+    public function testProcess()
+    {
+        $locatorDefinition = new Definition();
+        $locatorDefinition->addTag('liip_imagine.binary.locator', array(
+            'shared' => true,
+        ));
+
+        $container = new ContainerBuilder();
+        $container->setDefinition('liip_imagine.binary.locator.foo', $locatorDefinition);
+
+        $pass = new LocatorsCompilerPass();
+
+        //guard
+        if (method_exists($locatorDefinition, 'isShared')) {
+            $this->assertTrue($locatorDefinition->isShared());
+        } else {
+            $this->assertSame('container', $locatorDefinition->getScope());
+        }
+
+        $pass->process($container);
+
+        if (method_exists($locatorDefinition, 'isShared')) {
+            $this->assertFalse($locatorDefinition->isShared());
+        } else {
+            $this->assertSame('prototype', $locatorDefinition->getScope());
+        }
+    }
+}
diff --git a/Tests/DependencyInjection/Factory/Resolver/AwsS3ResolverFactoryTest.php b/Tests/DependencyInjection/Factory/Resolver/AwsS3ResolverFactoryTest.php
@@ -12,13 +12,13 @@
 namespace Liip\ImagineBundle\Tests\DependencyInjection\Factory\Resolver;
 
 use Liip\ImagineBundle\DependencyInjection\Factory\Resolver\AwsS3ResolverFactory;
+use Liip\ImagineBundle\Utility\Framework\SymfonyFramework;
 use Symfony\Component\Config\Definition\Builder\TreeBuilder;
 use Symfony\Component\Config\Definition\Processor;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
-use Symfony\Component\HttpKernel\Kernel;
 
 /**
- * @covers Liip\ImagineBundle\DependencyInjection\Factory\Resolver\AwsS3ResolverFactory<extended>
+ * @covers \Liip\ImagineBundle\DependencyInjection\Factory\Resolver\AwsS3ResolverFactory<extended>
  */
 class AwsS3ResolverFactoryTest extends \Phpunit_Framework_TestCase
 {
@@ -122,7 +122,7 @@ public function testCreateS3ClientDefinitionOnCreate()
 
     public function testCreateS3ClientDefinitionWithFactoryOnCreate()
     {
-        if (version_compare(Kernel::VERSION_ID, '20600') < 0) {
+        if (SymfonyFramework::isKernelLessThan(2, 6)) {
             $this->markTestSkipped('No need to test on symfony < 2.6');
         }
 
@@ -147,7 +147,7 @@ public function testCreateS3ClientDefinitionWithFactoryOnCreate()
 
     public function testLegacyCreateS3ClientDefinitionWithFactoryOnCreate()
     {
-        if (version_compare(Kernel::VERSION_ID, '20600') >= 0) {
+        if (SymfonyFramework::isKernelGreaterThanOrEqualTo(2, 6)) {
             $this->markTestSkipped('No need to test on symfony >= 2.6');
         }
 

diff --git a/Tests/DependencyInjection/LiipImagineExtensionTest.php b/Tests/DependencyInjection/LiipImagineExtensionTest.php
@@ -15,15 +15,16 @@
 use Liip\ImagineBundle\DependencyInjection\Factory\Resolver\WebPathResolverFactory;
 use Liip\ImagineBundle\DependencyInjection\LiipImagineExtension;
 use Liip\ImagineBundle\Tests\AbstractTest;
+use Liip\ImagineBundle\Utility\Framework\SymfonyFramework;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\ContainerInterface;
+use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Reference;
-use Symfony\Component\HttpKernel\Kernel;
 use Symfony\Component\Yaml\Parser;
 
 /**
- * @covers Liip\ImagineBundle\DependencyInjection\Configuration
- * @covers Liip\ImagineBundle\DependencyInjection\LiipImagineExtension
+ * @covers \Liip\ImagineBundle\DependencyInjection\Configuration
+ * @covers \Liip\ImagineBundle\DependencyInjection\LiipImagineExtension
  */
 class LiipImagineExtensionTest extends AbstractTest
 {
@@ -77,7 +78,7 @@ public function testCustomRouteRequirements()
      */
     public function testFactoriesConfiguration($service, $factory)
     {
-        if (version_compare(Kernel::VERSION_ID, '20600') < 0) {
+        if (SymfonyFramework::isKernelLessThan(2, 6)) {
             $this->markTestSkipped('No need to test on symfony < 2.6');
         }
 
@@ -88,11 +89,12 @@ public function testFactoriesConfiguration($service, $factory)
     }
 
     /**
+     * @legacy
      * @dataProvider factoriesProvider
      */
     public function testLegacyFactoriesConfiguration($service, $factory)
     {
-        if (version_compare(Kernel::VERSION_ID, '20600') >= 0) {
+        if (SymfonyFramework::isKernelGreaterThanOrEqualTo(2, 6)) {
             $this->markTestSkipped('No need to test on symfony >= 2.6');
         }
 
@@ -117,9 +119,6 @@ public function factoriesProvider()
         );
     }
 
-    /**
-     * @return ContainerBuilder
-     */
     protected function createEmptyConfiguration()
     {
         $this->containerBuilder = new ContainerBuilder();
@@ -130,9 +129,6 @@ protected function createEmptyConfiguration()
         $this->assertTrue($this->containerBuilder instanceof ContainerBuilder);
     }
 
-    /**
-     * @return ContainerBuilder
-     */
     protected function createFullConfiguration()
     {
         $this->containerBuilder = new ContainerBuilder();
@@ -183,29 +179,39 @@ protected function getFullConfig()
         return $parser->parse($yaml);
     }
 
+    /**
+     * @param string $value
+     * @param string $key
+     */
     private function assertAlias($value, $key)
     {
         $this->assertEquals($value, (string) $this->containerBuilder->getAlias($key), sprintf('%s alias is correct', $key));
     }
 
+    /**
+     * @param string $value
+     * @param string $key
+     */
     private function assertParameter($value, $key)
     {
         $this->assertEquals($value, $this->containerBuilder->getParameter($key), sprintf('%s parameter is correct', $key));
     }
 
+    /**
+     * @param string $id
+     */
     private function assertHasDefinition($id)
     {
         $this->assertTrue(($this->containerBuilder->hasDefinition($id) ?: $this->containerBuilder->hasAlias($id)));
     }
 
-    private function assertNotHasDefinition($id)
-    {
-        $this->assertFalse(($this->containerBuilder->hasDefinition($id) ?: $this->containerBuilder->hasAlias($id)));
-    }
-
-    private function assertDICConstructorArguments($definition, $args)
+    /**
+     * @param Definition $definition
+     * @param array      $arguments
+     */
+    private function assertDICConstructorArguments(Definition $definition, array $arguments)
     {
-        $this->assertEquals($args, $definition->getArguments(), "Expected and actual DIC Service constructor arguments of definition '".$definition->getClass()."' don't match.");
+        $this->assertEquals($arguments, $definition->getArguments(), "Expected and actual DIC Service constructor arguments of definition '".$definition->getClass()."' don't match.");
     }
 
     protected function tearDown()

diff --git a/Tests/Form/Type/ImageTypeTest.php b/Tests/Form/Type/ImageTypeTest.php
@@ -12,12 +12,12 @@
 namespace Liip\ImagineBundle\Tests\Form\Type;
 
 use Liip\ImagineBundle\Form\Type\ImageType;
+use Liip\ImagineBundle\Utility\Framework\SymfonyFramework;
 use Symfony\Component\Form\FormView;
-use Symfony\Component\HttpKernel\Kernel;
 use Symfony\Component\OptionsResolver\OptionsResolver;
 
 /**
- * @covers Liip\ImagineBundle\Form\Type\ImageType
+ * @covers \Liip\ImagineBundle\Form\Type\ImageType
  */
 class ImageTypeTest extends \PHPUnit_Framework_TestCase
 {
@@ -37,7 +37,7 @@ public function testGetParent()
 
     public function testConfigureOptions()
     {
-        if (version_compare(Kernel::VERSION_ID, '20600') < 0) {
+        if (SymfonyFramework::isKernelLessThan(2, 6)) {
             $this->markTestSkipped('No need to test on symfony < 2.6');
         }
 
@@ -57,7 +57,7 @@ public function testConfigureOptions()
 
     public function testLegacySetDefaultOptions()
     {
-        if (version_compare(Kernel::VERSION_ID, '20600') >= 0) {
+        if (SymfonyFramework::isKernelGreaterThanOrEqualTo(2, 6)) {
             $this->markTestSkipped('No need to test on symfony >= 2.6');
         }