From 99419306eb2792eb582f7c6838fd49007853934c Mon Sep 17 00:00:00 2001 From: Rob Frawley 2nd Date: Wed, 16 May 2018 21:41:05 -0400 Subject: [PATCH] add chain data loader implementation Signed-off-by: Rob Frawley 2nd --- Binary/Loader/ChainLoader.php | 75 ++++++++ .../Factory/Loader/ChainLoaderFactory.php | 66 +++++++ LiipImagineBundle.php | 2 + Resources/config/imagine.xml | 4 + Resources/doc/data-loader/chain.rst | 57 ++++++ Tests/Binary/Loader/ChainLoaderTest.php | 172 ++++++++++++++++++ .../Factory/Loader/ChainLoaderFactoryTest.php | 94 ++++++++++ .../Loader/FileSystemLoaderFactoryTest.php | 10 +- .../Loader/FlysystemLoaderFactoryTest.php | 4 +- .../Loader/StreamLoaderFactoryTest.php | 2 +- .../Binary/Loader/ChainLoaderTest.php | 42 +++++ Tests/Functional/app/config/config.yml | 8 + Tests/LiipImagineBundleTest.php | 18 ++ 13 files changed, 543 insertions(+), 11 deletions(-) create mode 100644 Binary/Loader/ChainLoader.php create mode 100644 DependencyInjection/Factory/Loader/ChainLoaderFactory.php create mode 100644 Resources/doc/data-loader/chain.rst create mode 100644 Tests/Binary/Loader/ChainLoaderTest.php create mode 100644 Tests/DependencyInjection/Factory/Loader/ChainLoaderFactoryTest.php create mode 100644 Tests/Functional/Binary/Loader/ChainLoaderTest.php diff --git a/Binary/Loader/ChainLoader.php b/Binary/Loader/ChainLoader.php new file mode 100644 index 000000000..b1fb93f3e --- /dev/null +++ b/Binary/Loader/ChainLoader.php @@ -0,0 +1,75 @@ +loaders = array_filter($loaders, function ($loader) { + return $loader instanceof LoaderInterface; + }); + } + + /** + * {@inheritdoc} + */ + public function find($path) + { + $exceptions = []; + + foreach ($this->loaders as $loader) { + try { + return $loader->find($path); + } catch (\Exception $e) { + $exceptions[$e->getMessage()] = $loader; + } + } + + throw new NotLoadableException(self::getLoaderExceptionMessage($path, $exceptions, $this->loaders)); + } + + /** + * @param string $path + * @param \Exception[] $exceptions + * @param array $loaders + * + * @return string + */ + private static function getLoaderExceptionMessage(string $path, array $exceptions, array $loaders): string + { + array_walk($loaders, function (LoaderInterface &$loader, string $name): void { + $loader = sprintf('%s=[%s]', (new \ReflectionObject($loader))->getShortName(), $name); + }); + + array_walk($exceptions, function (LoaderInterface &$loader, string $message): void { + $loader = sprintf('%s=[%s]', (new \ReflectionObject($loader))->getShortName(), $message); + }); + + return vsprintf('Source image not resolvable "%s" using "%s" %d loaders (internal exceptions: %s).', [ + $path, + implode(', ', $loaders), + count($loaders), + implode(', ', $exceptions), + ]); + } +} diff --git a/DependencyInjection/Factory/Loader/ChainLoaderFactory.php b/DependencyInjection/Factory/Loader/ChainLoaderFactory.php new file mode 100644 index 000000000..afb995200 --- /dev/null +++ b/DependencyInjection/Factory/Loader/ChainLoaderFactory.php @@ -0,0 +1,66 @@ +getChildLoaderDefinition(); + $definition->replaceArgument(0, $this->createLoaderReferences($config['loaders'])); + + return $this->setTaggedLoaderDefinition($loaderName, $definition, $container); + } + + /** + * {@inheritdoc} + */ + public function getName(): string + { + return 'chain'; + } + + /** + * {@inheritdoc} + */ + public function addConfiguration(ArrayNodeDefinition $builder): void + { + $builder + ->children() + ->arrayNode('loaders') + ->isRequired() + ->prototype('scalar') + ->cannotBeEmpty() + ->end() + ->end() + ->end(); + } + + /** + * @param string[] $loaders + * + * @return string[] + */ + private function createLoaderReferences(array $loaders): array + { + return array_combine($loaders, array_map(function ($name) { + return new Reference(sprintf('liip_imagine.binary.loader.%s', $name)); + }, $loaders)); + } +} diff --git a/LiipImagineBundle.php b/LiipImagineBundle.php index ebda57c88..90f56e4ec 100644 --- a/LiipImagineBundle.php +++ b/LiipImagineBundle.php @@ -18,6 +18,7 @@ use Liip\ImagineBundle\DependencyInjection\Compiler\MetadataReaderCompilerPass; use Liip\ImagineBundle\DependencyInjection\Compiler\PostProcessorsCompilerPass; use Liip\ImagineBundle\DependencyInjection\Compiler\ResolversCompilerPass; +use Liip\ImagineBundle\DependencyInjection\Factory\Loader\ChainLoaderFactory; use Liip\ImagineBundle\DependencyInjection\Factory\Loader\FileSystemLoaderFactory; use Liip\ImagineBundle\DependencyInjection\Factory\Loader\FlysystemLoaderFactory; use Liip\ImagineBundle\DependencyInjection\Factory\Loader\StreamLoaderFactory; @@ -59,5 +60,6 @@ public function build(ContainerBuilder $container) $extension->addLoaderFactory(new StreamLoaderFactory()); $extension->addLoaderFactory(new FileSystemLoaderFactory()); $extension->addLoaderFactory(new FlysystemLoaderFactory()); + $extension->addLoaderFactory(new ChainLoaderFactory()); } } diff --git a/Resources/config/imagine.xml b/Resources/config/imagine.xml index b260e0d1a..ef2b9a488 100644 --- a/Resources/config/imagine.xml +++ b/Resources/config/imagine.xml @@ -190,6 +190,10 @@ + + + + diff --git a/Resources/doc/data-loader/chain.rst b/Resources/doc/data-loader/chain.rst new file mode 100644 index 000000000..cfa905a3c --- /dev/null +++ b/Resources/doc/data-loader/chain.rst @@ -0,0 +1,57 @@ + +.. _data-loaders-chain: + +Chain Loader +============ + +The ``Chain`` data loader doesn't load the image binary itself; instead +it allows for loading the image binary using any number of other +configured data loaders. For example, if you configured both a +:ref:`filesystem ` and +:ref:`flysystem ` data loader, this loader can +be defined to load from both in a defined order, returning the image +binary from the first that responds. + +.. tip:: + + This loader iterates over the data loaders in the order they are + configured in the chain definition, returning an image binary from + the first loader that supports the passed file path. This means if + a file exists in more than one loader, the file will be returned + using the first one defined in your configuration file for this + chain loader. + + + +Configuration +------------- + +As this loader leverages any number of other configured loaders, its +configuration is relatively simple; it supports only a ``loaders`` +option that accepts an array of other configured loader names: + +.. code-block:: yaml + + # app/config/config.yml + + liip_imagine: + loaders: + foo: + filesystem: + # configure filesystem loader + + bar: + flysystem: + # configure flysystem loader + + baz: + stream: + # configure stream loader + + qux: + chain: + # use the "foo", "bar", and "baz" loaders + loaders: + - foo + - bar + - baz diff --git a/Tests/Binary/Loader/ChainLoaderTest.php b/Tests/Binary/Loader/ChainLoaderTest.php new file mode 100644 index 000000000..2c5c66ab3 --- /dev/null +++ b/Tests/Binary/Loader/ChainLoaderTest.php @@ -0,0 +1,172 @@ +assertInstanceOf(LoaderInterface::class, $this->getChainLoader()); + } + + /** + * @return array[] + */ + public static function provideLoadCases(): array + { + $file = pathinfo(__FILE__, PATHINFO_BASENAME); + + return [ + [ + __DIR__, + $file, + ], + [ + __DIR__.'/', + $file, + ], + [ + __DIR__, '/'. + $file, + ], + [ + __DIR__.'/../../Binary/Loader', + '/'.$file, + ], + [ + realpath(__DIR__.'/..'), + 'Loader/'.$file, + ], + [ + __DIR__.'/../', + '/Loader/../../Binary/Loader/'.$file, + ], + ]; + } + + /** + * @dataProvider provideLoadCases + * + * @param string $root + * @param string $path + */ + public function testLoad(string $root, string $path): void + { + $this->assertValidLoaderFindReturn($this->getChainLoader([$root])->find($path)); + } + + /** + * @return array[] + */ + public function provideInvalidPathsData(): array + { + return [ + ['../Loader/../../Binary/Loader/../../../Resources/config/routing.yaml'], + ['../../Binary/'], + ]; + } + + /** + * @dataProvider provideInvalidPathsData + * + * @param string $path + */ + public function testThrowsIfFileDoesNotExist(string $path): void + { + $this->expectException(NotLoadableException::class); + $this->expectExceptionMessageRegExp('{Source image not resolvable "[^"]+" using "FileSystemLoader=\[foo\]" 1 loaders}'); + + $this->getChainLoader()->find($path); + } + + /** + * @dataProvider provideInvalidPathsData + * + * @param string $path + */ + public function testThrowsIfFileDoesNotExistWithMultipleLoaders(string $path): void + { + $this->expectException(NotLoadableException::class); + $this->expectExceptionMessageRegExp('{Source image not resolvable "[^"]+" using "FileSystemLoader=\[foo\], FileSystemLoader=\[bar\]" 2 loaders \(internal exceptions: FileSystemLoader=\[.+\], FileSystemLoader=\[.+\]\)\.}'); + + $this->getChainLoader([], [ + 'foo' => new FileSystemLoader( + MimeTypeGuesser::getInstance(), + ExtensionGuesser::getInstance(), + $this->getFileSystemLocator([ + realpath(__DIR__.'/../../'), + ]) + ), + 'bar' => new FileSystemLoader( + MimeTypeGuesser::getInstance(), + ExtensionGuesser::getInstance(), + $this->getFileSystemLocator([ + realpath(__DIR__.'/../../../'), + ]) + ), + ])->find($path); + } + + /** + * @param string[] $paths + * + * @return FileSystemLocator + */ + private function getFileSystemLocator(array $paths = []): FileSystemLocator + { + return new FileSystemLocator($paths); + } + + /** + * @param string[] $paths + * @param FileSystemLoader[] $loaders + * + * @return ChainLoader + */ + private function getChainLoader(array $paths = [], array $loaders = null): ChainLoader + { + if (null === $loaders) { + $loaders = [ + 'foo' => new FileSystemLoader( + MimeTypeGuesser::getInstance(), + ExtensionGuesser::getInstance(), + $this->getFileSystemLocator($paths ?: [__DIR__]) + ), + ]; + } + + return new ChainLoader($loaders); + } + + /** + * @param FileBinary|mixed $return + * @param string|null $message + */ + private function assertValidLoaderFindReturn($return, string $message = null): void + { + $this->assertInstanceOf(FileBinary::class, $return, $message); + $this->assertStringStartsWith('text/', $return->getMimeType(), $message); + } +} diff --git a/Tests/DependencyInjection/Factory/Loader/ChainLoaderFactoryTest.php b/Tests/DependencyInjection/Factory/Loader/ChainLoaderFactoryTest.php new file mode 100644 index 000000000..43b5a0a5b --- /dev/null +++ b/Tests/DependencyInjection/Factory/Loader/ChainLoaderFactoryTest.php @@ -0,0 +1,94 @@ +assertInstanceOf(LoaderFactoryInterface::class, new ChainLoaderFactory()); + } + + public function testReturnsExpectedName(): void + { + $this->assertSame('chain', (new ChainLoaderFactory())->getName()); + } + + public function testCreateLoaderDefinition(): void + { + $container = new ContainerBuilder(); + + $loader = new ChainLoaderFactory(); + $loader->create($container, 'the_loader_name', [ + 'loaders' => [ + 'foo', + 'bar', + 'baz', + ], + ]); + + $this->assertTrue($container->hasDefinition('liip_imagine.binary.loader.the_loader_name')); + + /** @var ChildDefinition $loaderDefinition */ + $loaderDefinition = $container->getDefinition('liip_imagine.binary.loader.the_loader_name'); + + $this->assertInstanceOfChildDefinition($loaderDefinition); + $this->assertSame('liip_imagine.binary.loader.prototype.chain', $loaderDefinition->getParent()); + + foreach ($loaderDefinition->getArgument(0) as $reference) { + $this->assertInstanceOf('\Symfony\Component\DependencyInjection\Reference', $reference); + } + } + + public function testProcessOptionsOnAddConfiguration(): void + { + $treeBuilder = new TreeBuilder(); + $rootNode = $treeBuilder->root('chain', 'array'); + + $loader = new ChainLoaderFactory(); + $loader->addConfiguration($rootNode); + + $config = $this->processConfigTree($treeBuilder, [ + 'chain' => [ + 'loaders' => [ + 'foo', + 'bar', + ], + ], + ]); + + $this->assertArrayHasKey('loaders', $config); + $this->assertSame(['foo', 'bar'], $config['loaders']); + } + + /** + * @param TreeBuilder $treeBuilder + * @param array $configs + * + * @return array + */ + private function processConfigTree(TreeBuilder $treeBuilder, array $configs): array + { + return (new Processor())->process($treeBuilder->buildTree(), $configs); + } +} diff --git a/Tests/DependencyInjection/Factory/Loader/FileSystemLoaderFactoryTest.php b/Tests/DependencyInjection/Factory/Loader/FileSystemLoaderFactoryTest.php index 801ce952f..876297b40 100644 --- a/Tests/DependencyInjection/Factory/Loader/FileSystemLoaderFactoryTest.php +++ b/Tests/DependencyInjection/Factory/Loader/FileSystemLoaderFactoryTest.php @@ -22,22 +22,18 @@ use Symfony\Component\DependencyInjection\ContainerBuilder; /** - * @covers \Liip\ImagineBundle\DependencyInjection\Factory\Loader\FileSystemLoaderFactory + * @covers \Liip\ImagineBundle\DependencyInjection\Factory\Loader\FileSystemLoaderFactory */ class FileSystemLoaderFactoryTest extends FactoryTestCase { public function testImplementsLoaderFactoryInterface() { - $rc = new \ReflectionClass(FileSystemLoaderFactory::class); - - $this->assertTrue($rc->implementsInterface(LoaderFactoryInterface::class)); + $this->assertInstanceOf(LoaderFactoryInterface::class, new FileSystemLoaderFactory()); } public function testCouldBeConstructedWithoutAnyArguments() { - $loader = new FileSystemLoaderFactory(); - - $this->assertInstanceOf(FileSystemLoaderFactory::class, $loader); + $this->assertInstanceOf(FileSystemLoaderFactory::class, new FileSystemLoaderFactory()); } public function testReturnExpectedName() diff --git a/Tests/DependencyInjection/Factory/Loader/FlysystemLoaderFactoryTest.php b/Tests/DependencyInjection/Factory/Loader/FlysystemLoaderFactoryTest.php index 1f834fba2..a8cefa45c 100644 --- a/Tests/DependencyInjection/Factory/Loader/FlysystemLoaderFactoryTest.php +++ b/Tests/DependencyInjection/Factory/Loader/FlysystemLoaderFactoryTest.php @@ -21,9 +21,7 @@ use Symfony\Component\DependencyInjection\ContainerBuilder; /** - * @requires PHP 5.4 - * - * @covers \Liip\ImagineBundle\DependencyInjection\Factory\Loader\FlysystemLoaderFactory + * @covers \Liip\ImagineBundle\DependencyInjection\Factory\Loader\FlysystemLoaderFactory */ class FlysystemLoaderFactoryTest extends TestCase { diff --git a/Tests/DependencyInjection/Factory/Loader/StreamLoaderFactoryTest.php b/Tests/DependencyInjection/Factory/Loader/StreamLoaderFactoryTest.php index 4bce6d56d..5a3f7a7a1 100644 --- a/Tests/DependencyInjection/Factory/Loader/StreamLoaderFactoryTest.php +++ b/Tests/DependencyInjection/Factory/Loader/StreamLoaderFactoryTest.php @@ -20,7 +20,7 @@ use Symfony\Component\DependencyInjection\ContainerBuilder; /** - * @covers \Liip\ImagineBundle\DependencyInjection\Factory\Loader\StreamLoaderFactory + * @covers \Liip\ImagineBundle\DependencyInjection\Factory\Loader\StreamLoaderFactory */ class StreamLoaderFactoryTest extends TestCase { diff --git a/Tests/Functional/Binary/Loader/ChainLoaderTest.php b/Tests/Functional/Binary/Loader/ChainLoaderTest.php new file mode 100644 index 000000000..914503e66 --- /dev/null +++ b/Tests/Functional/Binary/Loader/ChainLoaderTest.php @@ -0,0 +1,42 @@ +getLoader('baz'); + + foreach (['images/cats.jpeg', 'images/cats2.jpeg', 'file.ext', 'bar-bundle-file.ext', 'foo-bundle-file.ext'] as $file) { + $this->assertNotNull($loader->find($file)); + } + } + + /** + * @param string $name + * + * @return ChainLoader|object + */ + private function getLoader(string $name): ChainLoader + { + return $this->getService(sprintf('liip_imagine.binary.loader.%s', $name)); + } +} diff --git a/Tests/Functional/app/config/config.yml b/Tests/Functional/app/config/config.yml index 172c38d35..f55be851c 100644 --- a/Tests/Functional/app/config/config.yml +++ b/Tests/Functional/app/config/config.yml @@ -39,6 +39,14 @@ liip_imagine: filesystem: data_root: "%kernel.root_dir%/../../Fixtures/FileSystemLocator/root-02" + baz: + chain: + loaders: + - foo + - bar + - default + - bundles_all + bundles_all: filesystem: data_root: ~ diff --git a/Tests/LiipImagineBundleTest.php b/Tests/LiipImagineBundleTest.php index 10a92d62c..169a197f8 100644 --- a/Tests/LiipImagineBundleTest.php +++ b/Tests/LiipImagineBundleTest.php @@ -15,6 +15,7 @@ use Liip\ImagineBundle\DependencyInjection\Compiler\LoadersCompilerPass; use Liip\ImagineBundle\DependencyInjection\Compiler\PostProcessorsCompilerPass; use Liip\ImagineBundle\DependencyInjection\Compiler\ResolversCompilerPass; +use Liip\ImagineBundle\DependencyInjection\Factory\Loader\ChainLoaderFactory; use Liip\ImagineBundle\DependencyInjection\Factory\Loader\FileSystemLoaderFactory; use Liip\ImagineBundle\DependencyInjection\Factory\Loader\FlysystemLoaderFactory; use Liip\ImagineBundle\DependencyInjection\Factory\Loader\StreamLoaderFactory; @@ -161,6 +162,23 @@ public function testAddFlysystemResolverFactoryOnBuild() $bundle->build($containerMock); } + public function testAddChainLoaderFactoryOnBuild() + { + $extensionMock = $this->createLiipImagineExtensionMock(); + $extensionMock + ->expects($this->at(6)) + ->method('addLoaderFactory') + ->with($this->isInstanceOf(ChainLoaderFactory::class)); + $containerMock = $this->createContainerBuilderMock(); + $containerMock + ->expects($this->atLeastOnce()) + ->method('getExtension') + ->with('liip_imagine') + ->will($this->returnValue($extensionMock)); + $bundle = new LiipImagineBundle(); + $bundle->build($containerMock); + } + public function testAddStreamLoaderFactoryOnBuild() { $extensionMock = $this->createLiipImagineExtensionMock();