diff --git a/Binary/Loader/FileSystemLoader.php b/Binary/Loader/FileSystemLoader.php
index 16cbbdfda..ef7104b0f 100644
--- a/Binary/Loader/FileSystemLoader.php
+++ b/Binary/Loader/FileSystemLoader.php
@@ -36,31 +36,54 @@ class FileSystemLoader implements LoaderInterface
protected $locator;
/**
+ * This method will continue to support two prior, deprecated signitures for the duration of the 1.x
+ * release. The currently documented signiture will be the only valid usage once 2.0 is release. You
+ * can reference PR-963 {@see https://github.com/liip/LiipImagineBundle/pull/963} for more information.
+ *
* @param MimeTypeGuesserInterface $mimeGuesser
* @param ExtensionGuesserInterface $extensionGuesser
- * @param string[] $dataRoots
+ * @param LocatorInterface
*/
- public function __construct(
- MimeTypeGuesserInterface $mimeGuesser,
- ExtensionGuesserInterface $extensionGuesser,
- $dataRoots
- /* LocatorInterface $locator */
- ) {
+ public function __construct(MimeTypeGuesserInterface $mimeGuesser, ExtensionGuesserInterface $extensionGuesser, $locator)
+ {
$this->mimeTypeGuesser = $mimeGuesser;
$this->extensionGuesser = $extensionGuesser;
- if (count($dataRoots) === 0) {
- throw new InvalidArgumentException('One or more data root paths must be specified.');
- }
+ if ($locator instanceof LocatorInterface) { // post-1.9.0 behavior
+ $this->locator = $locator;
+ } elseif (is_array($locator) || is_string($locator)) { // pre-1.9.0 behaviour
+ if (count((array) $locator) === 0) {
+ throw new InvalidArgumentException('One or more data root paths must be specified.');
+ }
- if (func_num_args() >= 4 && false === ($this->locator = func_get_arg(3)) instanceof LocatorInterface) {
- throw new \InvalidArgumentException(sprintf('Method %s() expects a LocatorInterface for the forth argument.', __METHOD__));
- } elseif (func_num_args() < 4) {
- @trigger_error(sprintf('Method %s() will have a forth `LocatorInterface $locator` argument in version 2.0. Not defining it is deprecated since version 1.7.2', __METHOD__), E_USER_DEPRECATED);
- $this->locator = new FileSystemLocator();
- }
+ if (func_num_args() >= 4) {
+ if (func_get_arg(3) instanceof LocatorInterface) {
+ @trigger_error(sprintf(
+ 'Passing a LocatorInterface as fourth parameter to %s() is deprecated. It needs to be the '.
+ 'third parameter. The previous third parameter (data roots) is removed and the data roots must '.
+ 'now be passed as a constructor argument to the LocatorInterface passed to this method.', __METHOD__
+ ), E_USER_DEPRECATED);
- $this->locator->setOptions(array('roots' => (array) $dataRoots));
+ $this->locator = func_get_arg(3);
+ $this->locator->setOptions(array('roots' => (array) $locator));
+ } else {
+ throw new \InvalidArgumentException(sprintf(
+ 'Unknown call to %s(). Please check the method signature.', __METHOD__
+ ));
+ }
+ } else {
+ @trigger_error(sprintf(
+ 'Method %s() will expect the third parameter to be a LocatorInterface in version 2.0. Defining '.
+ 'data roots instead is deprecated since version 1.9.0', __METHOD__
+ ), E_USER_DEPRECATED);
+
+ $this->locator = new FileSystemLocator((array) $locator);
+ }
+ } else { // invalid behavior
+ throw new \InvalidArgumentException(sprintf(
+ 'Method %s() expects a LocatorInterface for the third argument.', __METHOD__
+ ));
+ }
}
/**
diff --git a/Binary/Locator/FileSystemLocator.php b/Binary/Locator/FileSystemLocator.php
index ad1a802a9..7ba6c295b 100644
--- a/Binary/Locator/FileSystemLocator.php
+++ b/Binary/Locator/FileSystemLocator.php
@@ -24,6 +24,16 @@ class FileSystemLocator implements LocatorInterface
private $roots = array();
/**
+ * @param string[] $roots
+ */
+ public function __construct(array $roots = array())
+ {
+ $this->roots = array_map(array($this, 'sanitizeRootPath'), $roots);
+ }
+
+ /**
+ * @deprecated Since version 0.9.0, use __construct(array $roots) instead
+ *
* @param array[] $options
*/
public function setOptions(array $options = array())
@@ -37,6 +47,11 @@ public function setOptions(array $options = array())
throw new InvalidArgumentException(sprintf('Invalid options provided to %s()', __METHOD__), null, $e);
}
+ @trigger_error(
+ sprintf('%s() is deprecated. Pass the data roots to the constructor instead.', __METHOD__),
+ E_USER_DEPRECATED
+ );
+
$this->roots = array_map(array($this, 'sanitizeRootPath'), (array) $options['roots']);
}
diff --git a/DependencyInjection/Factory/Loader/FileSystemLoaderFactory.php b/DependencyInjection/Factory/Loader/FileSystemLoaderFactory.php
index ccc517a65..03fd37a52 100644
--- a/DependencyInjection/Factory/Loader/FileSystemLoaderFactory.php
+++ b/DependencyInjection/Factory/Loader/FileSystemLoaderFactory.php
@@ -12,10 +12,11 @@
namespace Liip\ImagineBundle\DependencyInjection\Factory\Loader;
use Liip\ImagineBundle\Exception\InvalidArgumentException;
-use Liip\ImagineBundle\Utility\Framework\SymfonyFramework;
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
+use Symfony\Component\DependencyInjection\ChildDefinition;
use Symfony\Component\DependencyInjection\ContainerBuilder;
-use Symfony\Component\DependencyInjection\ContainerInterface;
+use Symfony\Component\DependencyInjection\Definition;
+use Symfony\Component\DependencyInjection\DefinitionDecorator;
use Symfony\Component\DependencyInjection\Reference;
class FileSystemLoaderFactory extends AbstractLoaderFactory
@@ -26,8 +27,7 @@ class FileSystemLoaderFactory extends AbstractLoaderFactory
public function create(ContainerBuilder $container, $loaderName, array $config)
{
$definition = $this->getChildLoaderDefinition();
- $definition->replaceArgument(2, $this->resolveDataRoots($config['data_root'], $config['bundle_resources'], $container));
- $definition->replaceArgument(3, $this->createLocatorReference($config['locator']));
+ $definition->replaceArgument(2, new Reference($this->setupLocator($loaderName, $config, $container)));
return $this->setTaggedLoaderDefinition($loaderName, $definition, $container);
}
@@ -165,18 +165,34 @@ private function getBundlePathsUsingNamedObj(array $classes)
}
/**
- * @param string $reference
+ * @param string $loaderName
+ * @param array $config
+ * @param ContainerBuilder $container
*
- * @return Reference
+ * @return string
*/
- private function createLocatorReference($reference)
+ private function setupLocator($loaderName, array $config, ContainerBuilder $container)
{
- $name = sprintf('liip_imagine.binary.locator.%s', $reference);
+ $locator = $this->getLocatorDefinition($config);
+ $locator->replaceArgument(0, $this->resolveDataRoots($config['data_root'], $config['bundle_resources'], $container));
- if (SymfonyFramework::hasDefinitionSharing()) {
- return new Reference($name);
- }
+ $locatorId = sprintf('liip_imagine.binary.locator.%s.%s', $config['locator'], $loaderName);
+ $container->setDefinition($locatorId, $locator);
+
+ return $locatorId;
+ }
+
+ /**
+ * @param array $config
+ *
+ * @return Definition
+ */
+ private function getLocatorDefinition(array $config)
+ {
+ $key = sprintf('liip_imagine.binary.locator.%s', $config['locator']);
+ $def = class_exists('\Symfony\Component\DependencyInjection\ChildDefinition') ?
+ new ChildDefinition($key) : new DefinitionDecorator($key);
- return new Reference($name, ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, false);
+ return $def;
}
}
diff --git a/Resources/config/imagine.xml b/Resources/config/imagine.xml
index 060dc6bb3..a0c819505 100644
--- a/Resources/config/imagine.xml
+++ b/Resources/config/imagine.xml
@@ -257,10 +257,12 @@
+
+
diff --git a/Tests/Binary/Loader/FileSystemLoaderTest.php b/Tests/Binary/Loader/FileSystemLoaderTest.php
index 7d0b00396..5e7508bf9 100644
--- a/Tests/Binary/Loader/FileSystemLoaderTest.php
+++ b/Tests/Binary/Loader/FileSystemLoaderTest.php
@@ -128,7 +128,7 @@ public function testMultipleRootLoadCases($root, $path)
/**
* @expectedException \InvalidArgumentException
- * @expectedExceptionMessageRegExp {Method .+ expects a LocatorInterface for the forth argument}
+ * @expectedExceptionMessageRegExp {Unknown call to [^(]+__construct\(\)\. Please check the method signature\.}
*/
public function testThrowsIfConstructionArgumentsInvalid()
{
@@ -146,7 +146,7 @@ public function testThrowsIfConstructionArgumentsInvalid()
*/
public function testThrowsIfZeroCountRootPathArray()
{
- $this->getFileSystemLoader(array());
+ new FileSystemLoader(MimeTypeGuesser::getInstance(), ExtensionGuesser::getInstance(), array());
}
/**
@@ -203,14 +203,6 @@ public function testThrowsIfFileDoesNotExist()
$this->getFileSystemLoader()->find('fileNotExist');
}
- /**
- * @return FileSystemLocator
- */
- private function getFileSystemLocator()
- {
- return new FileSystemLocator();
- }
-
/**
* @return string[]
*/
@@ -230,11 +222,20 @@ private function getFileSystemLoader($root = null, LocatorInterface $locator = n
return new FileSystemLoader(
MimeTypeGuesser::getInstance(),
ExtensionGuesser::getInstance(),
- null !== $root ? $root : $this->getDefaultDataRoots(),
- null !== $locator ? $locator : $this->getFileSystemLocator()
+ null !== $locator ? $locator : $this->getFileSystemLocator(null !== $root ? $root : $this->getDefaultDataRoots())
);
}
+ /**
+ * @param string|string[] $roots
+ *
+ * @return FileSystemLocator
+ */
+ private function getFileSystemLocator($roots)
+ {
+ return new FileSystemLocator((array) $roots);
+ }
+
/**
* @param FileBinary|mixed $return
* @param string|null $message
diff --git a/Tests/Binary/Locator/AbstractFileSystemLocatorTest.php b/Tests/Binary/Locator/AbstractFileSystemLocatorTest.php
index 20583495d..81c0759c2 100644
--- a/Tests/Binary/Locator/AbstractFileSystemLocatorTest.php
+++ b/Tests/Binary/Locator/AbstractFileSystemLocatorTest.php
@@ -17,11 +17,11 @@
abstract class AbstractFileSystemLocatorTest extends \PHPUnit_Framework_TestCase
{
/**
- * @param string[]|string $paths
+ * @param string[]|string $roots
*
* @return LocatorInterface
*/
- abstract protected function getFileSystemLocator($paths);
+ abstract protected function getFileSystemLocator($roots);
public function testImplementsLocatorInterface()
{
diff --git a/Tests/Binary/Locator/FileSystemInsecureLocatorTest.php b/Tests/Binary/Locator/FileSystemInsecureLocatorTest.php
index 1827e4f4d..1d4d6080b 100644
--- a/Tests/Binary/Locator/FileSystemInsecureLocatorTest.php
+++ b/Tests/Binary/Locator/FileSystemInsecureLocatorTest.php
@@ -19,17 +19,6 @@
*/
class FileSystemInsecureLocatorTest extends AbstractFileSystemLocatorTest
{
- /**
- * @return LocatorInterface
- */
- protected function getFileSystemLocator($paths)
- {
- $locator = new FileSystemInsecureLocator();
- $locator->setOptions(array('roots' => (array) $paths));
-
- return $locator;
- }
-
public function testLoadsOnSymbolicLinks()
{
$loader = $this->getFileSystemLocator($root = realpath(__DIR__.'/../../Fixtures/FileSystemLocator/root-02'));
@@ -90,4 +79,14 @@ public static function provideMultipleRootLoadCases()
return array(array($prepend[mt_rand(0, count($prepend) - 1)], $params[0]), $params[1]);
}, static::provideLoadCases());
}
+
+ /**
+ * @param string|string[] $roots
+ *
+ * @return LocatorInterface
+ */
+ protected function getFileSystemLocator($roots)
+ {
+ return new FileSystemInsecureLocator((array) $roots);
+ }
}
diff --git a/Tests/Binary/Locator/FileSystemLocatorTest.php b/Tests/Binary/Locator/FileSystemLocatorTest.php
index c91493627..42bc25c0d 100644
--- a/Tests/Binary/Locator/FileSystemLocatorTest.php
+++ b/Tests/Binary/Locator/FileSystemLocatorTest.php
@@ -19,17 +19,6 @@
*/
class FileSystemLocatorTest extends AbstractFileSystemLocatorTest
{
- /**
- * @return LocatorInterface
- */
- protected function getFileSystemLocator($paths)
- {
- $locator = new FileSystemLocator();
- $locator->setOptions(array('roots' => (array) $paths));
-
- return $locator;
- }
-
/**
* @expectedException \Liip\ImagineBundle\Exception\Binary\Loader\NotLoadableException
* @expectedExceptionMessage Source image invalid
@@ -85,4 +74,14 @@ public static function provideMultipleRootLoadCases()
return array(array($prepend[mt_rand(0, count($prepend) - 1)], $params[0]), $params[1]);
}, static::provideLoadCases());
}
+
+ /**
+ * @param string|string[] $roots
+ *
+ * @return LocatorInterface
+ */
+ protected function getFileSystemLocator($roots)
+ {
+ return new FileSystemLocator((array) $roots);
+ }
}
diff --git a/Tests/DependencyInjection/Factory/Loader/FileSystemLoaderFactoryTest.php b/Tests/DependencyInjection/Factory/Loader/FileSystemLoaderFactoryTest.php
index 6bc8f8fc8..288a04286 100644
--- a/Tests/DependencyInjection/Factory/Loader/FileSystemLoaderFactoryTest.php
+++ b/Tests/DependencyInjection/Factory/Loader/FileSystemLoaderFactoryTest.php
@@ -63,7 +63,8 @@ public function testCreateLoaderDefinitionOnCreate()
$this->assertInstanceOfChildDefinition($loaderDefinition);
$this->assertEquals('liip_imagine.binary.loader.prototype.filesystem', $loaderDefinition->getParent());
- $this->assertEquals(array('theDataRoot'), $loaderDefinition->getArgument(2));
+ $locatorReference = $container->getDefinition('liip_imagine.binary.loader.the_loader_name')->getArgument(2);
+ $this->assertEquals(array('theDataRoot'), $container->getDefinition((string) $locatorReference)->getArgument(0));
}
public function testCreateLoaderDefinitionOnCreateWithBundlesEnabledUsingMetadata()
@@ -98,7 +99,8 @@ public function testCreateLoaderDefinitionOnCreateWithBundlesEnabledUsingMetadat
'LiipBarBundle' => $barBundleRootPath.'/Resources/public',
);
- $this->assertEquals($expected, $container->getDefinition('liip_imagine.binary.loader.the_loader_name')->getArgument(2));
+ $locatorReference = $container->getDefinition('liip_imagine.binary.loader.the_loader_name')->getArgument(2);
+ $this->assertEquals($expected, $container->getDefinition((string) $locatorReference)->getArgument(0));
}
public function testCreateLoaderDefinitionOnCreateWithBundlesEnabledUsingMetadataAndBlacklisting()
@@ -134,7 +136,8 @@ public function testCreateLoaderDefinitionOnCreateWithBundlesEnabledUsingMetadat
'LiipBarBundle' => $barBundleRootPath.'/Resources/public',
);
- $this->assertEquals($expected, $container->getDefinition('liip_imagine.binary.loader.the_loader_name')->getArgument(2));
+ $locatorReference = $container->getDefinition('liip_imagine.binary.loader.the_loader_name')->getArgument(2);
+ $this->assertEquals($expected, $container->getDefinition((string) $locatorReference)->getArgument(0));
}
public function testCreateLoaderDefinitionOnCreateWithBundlesEnabledUsingMetadataAndWhitelisting()
@@ -170,7 +173,8 @@ public function testCreateLoaderDefinitionOnCreateWithBundlesEnabledUsingMetadat
'LiipFooBundle' => $fooBundleRootPath.'/Resources/public',
);
- $this->assertEquals($expected, $container->getDefinition('liip_imagine.binary.loader.the_loader_name')->getArgument(2));
+ $locatorReference = $container->getDefinition('liip_imagine.binary.loader.the_loader_name')->getArgument(2);
+ $this->assertEquals($expected, $container->getDefinition((string) $locatorReference)->getArgument(0));
}
public function testCreateLoaderDefinitionOnCreateWithBundlesEnabledUsingNamedObj()
@@ -201,7 +205,8 @@ public function testCreateLoaderDefinitionOnCreateWithBundlesEnabledUsingNamedOb
'LiipBarBundle' => $barBundleRootPath.'/Resources/public',
);
- $this->assertEquals($expected, $container->getDefinition('liip_imagine.binary.loader.the_loader_name')->getArgument(2));
+ $locatorReference = $container->getDefinition('liip_imagine.binary.loader.the_loader_name')->getArgument(2);
+ $this->assertEquals($expected, $container->getDefinition((string) $locatorReference)->getArgument(0));
}
/**
diff --git a/UPGRADE.md b/UPGRADE.md
index ea6737044..8cbea6cea 100644
--- a/UPGRADE.md
+++ b/UPGRADE.md
@@ -1,5 +1,12 @@
# Upgrade
+## 1.9.0
+
+ - __[Data Loader]__ The arguments for the `FileSystemLoader` class constructor have changed. Passing an array of roots
+ as the third parameter and an (optional) `LocatorInterace` as the fourth parameter is deprecated. A `LocatorInterface`
+ should now be passed as third parameter, and the array of data roots to the `LocatorInterface::__construct()` method
+ directly. All prior signatures will continue to work until `2.0` is release.
+
## 1.8.0
- __[Routing]__ The `Resources/config/routing.xml` file has been deprecated and will be removed in `2.0`. Use the new