Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flysystem v2 support #1357

Merged
merged 9 commits into from
Feb 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Binary/Loader/FlysystemV2Loader.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public function find($path)
$extension
);
} catch (FilesystemException $exception) {
throw new NotLoadableException(sprintf('Source image "%s" not found.', $path), null, $exception);
throw new NotLoadableException(sprintf('Source image "%s" not found.', $path), 0, $exception);
}
}

Expand Down
6 changes: 4 additions & 2 deletions DependencyInjection/Factory/Loader/AbstractLoaderFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@ abstract class AbstractLoaderFactory implements LoaderFactoryInterface
protected static $namePrefix = 'liip_imagine.binary.loader';

/**
* @param string|null $name
*
* @return ChildDefinition
*/
final protected function getChildLoaderDefinition()
final protected function getChildLoaderDefinition($name = null)
{
return new ChildDefinition(sprintf('%s.prototype.%s', static::$namePrefix, $this->getName()));
return new ChildDefinition(sprintf('%s.prototype.%s', static::$namePrefix, $name ?: $this->getName()));
}

/**
Expand Down
15 changes: 14 additions & 1 deletion DependencyInjection/Factory/Loader/FlysystemLoaderFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Liip\ImagineBundle\DependencyInjection\Factory\Loader;

use League\Flysystem\FilesystemOperator;
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Reference;
Expand All @@ -22,7 +23,7 @@ class FlysystemLoaderFactory extends AbstractLoaderFactory
*/
public function create(ContainerBuilder $container, $loaderName, array $config)
{
$definition = $this->getChildLoaderDefinition();
$definition = $this->getChildLoaderDefinition($this->getChildLoaderName());

if ($container->hasDefinition('liip_imagine.mime_types')) {
$mimeTypes = $container->getDefinition('liip_imagine.mime_types');
Expand Down Expand Up @@ -55,4 +56,16 @@ public function addConfiguration(ArrayNodeDefinition $builder)
->end()
->end();
}

/**
* @return string|null
*/
private function getChildLoaderName()
{
if (interface_exists(FilesystemOperator::class)) {
return 'flysystem2';
}

return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Liip\ImagineBundle\DependencyInjection\Factory\Resolver;

use League\Flysystem\FilesystemOperator;
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Reference;
Expand All @@ -22,7 +23,7 @@ class FlysystemResolverFactory extends AbstractResolverFactory
*/
public function create(ContainerBuilder $container, $resolverName, array $config)
{
$resolverDefinition = $this->getChildResolverDefinition();
$resolverDefinition = $this->getChildResolverDefinition($this->getChildResolverName());
$resolverDefinition->replaceArgument(0, new Reference($config['filesystem_service']));
$resolverDefinition->replaceArgument(2, $config['root_url']);
$resolverDefinition->replaceArgument(3, $config['cache_prefix']);
Expand Down Expand Up @@ -69,4 +70,16 @@ public function addConfiguration(ArrayNodeDefinition $builder)
->end()
->end();
}

/**
* @return string|null
*/
private function getChildResolverName()
{
if (interface_exists(FilesystemOperator::class)) {
return 'flysystem2';
}

return null;
}
}
13 changes: 13 additions & 0 deletions Resources/config/imagine.xml
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,11 @@
<argument><!-- will be injected by FlysystemLoaderFactory --></argument>
</service>

<service id="liip_imagine.binary.loader.prototype.flysystem2" class="Liip\ImagineBundle\Binary\Loader\FlysystemV2Loader" abstract="true">
<argument type="service" id="liip_imagine.extension_guesser" />
<argument><!-- will be injected by FlysystemV2LoaderFactory --></argument>
</service>

<service id="liip_imagine.binary.loader.prototype.chain" class="Liip\ImagineBundle\Binary\Loader\ChainLoader" abstract="true">
<argument><!-- will be injected by ChainLoaderFactory --></argument>
</service>
Expand Down Expand Up @@ -350,6 +355,14 @@
<argument><!-- will be injected by a ResolverFactory --></argument>
</service>

<service id="liip_imagine.cache.resolver.prototype.flysystem2" class="Liip\ImagineBundle\Imagine\Cache\Resolver\FlysystemV2Resolver" public="true" abstract="true">
<argument><!-- will be injected by a ResolverFactory --></argument>
<argument type="service" id="router.request_context" />
<argument><!-- will be injected by a ResolverFactory --></argument>
<argument><!-- will be injected by a ResolverFactory --></argument>
<argument><!-- will be injected by a ResolverFactory --></argument>
</service>

<service id="liip_imagine.cache.resolver.prototype.proxy" class="Liip\ImagineBundle\Imagine\Cache\Resolver\ProxyResolver" public="true" abstract="true">
<argument><!-- will be injected by AwsS3ResolverFactory --></argument>
<argument><!-- will be injected by AwsS3ResolverFactory --></argument>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
use Symfony\Component\DependencyInjection\Reference;

/**
* @covers \Liip\ImagineBundle\DependencyInjection\Factory\Loader\ChainLoaderFactory
* @covers \Liip\ImagineBundle\DependencyInjection\Factory\Loader\ChainLoaderFactory<extended>
*/
class ChainLoaderFactoryTest extends FactoryTestCase
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
use Symfony\Component\DependencyInjection\ContainerBuilder;

/**
* @covers \Liip\ImagineBundle\DependencyInjection\Factory\Loader\FileSystemLoaderFactory
* @covers \Liip\ImagineBundle\DependencyInjection\Factory\Loader\FileSystemLoaderFactory<extended>
*/
class FileSystemLoaderFactoryTest extends FactoryTestCase
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@

namespace Liip\ImagineBundle\Tests\DependencyInjection\Factory\Loader;

use League\Flysystem\Filesystem;
use League\Flysystem\FilesystemInterface;
use League\Flysystem\FilesystemOperator;
use Liip\ImagineBundle\DependencyInjection\Factory\Loader\FlysystemLoaderFactory;
use Liip\ImagineBundle\DependencyInjection\Factory\Loader\LoaderFactoryInterface;
use PHPUnit\Framework\TestCase;
Expand All @@ -21,15 +22,17 @@
use Symfony\Component\DependencyInjection\ContainerBuilder;

/**
* @covers \Liip\ImagineBundle\DependencyInjection\Factory\Loader\FlysystemLoaderFactory
* @covers \Liip\ImagineBundle\DependencyInjection\Factory\Loader\FlysystemLoaderFactory<extended>
*/
class FlysystemLoaderFactoryTest extends TestCase
{
protected function setUp(): void
{
parent::setUp();

if (!class_exists(Filesystem::class)) {
if (!interface_exists(FilesystemInterface::class)
&& !interface_exists(FilesystemOperator::class)
) {
$this->markTestSkipped('Requires the league/flysystem package.');
}
}
Expand Down Expand Up @@ -69,7 +72,12 @@ public function testCreateLoaderDefinitionOnCreate(): void

$loaderDefinition = $container->getDefinition('liip_imagine.binary.loader.the_loader_name');
$this->assertInstanceOf(ChildDefinition::class, $loaderDefinition);
$this->assertSame('liip_imagine.binary.loader.prototype.flysystem', $loaderDefinition->getParent());
if (interface_exists(FilesystemOperator::class)) {
$loaderName = 'liip_imagine.binary.loader.prototype.flysystem2';
} else {
$loaderName = 'liip_imagine.binary.loader.prototype.flysystem';
}
$this->assertSame($loaderName, $loaderDefinition->getParent());

$reference = $loaderDefinition->getArgument(1);
$this->assertSame('flyfilesystemservice', (string) $reference);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
use Symfony\Component\DependencyInjection\ContainerBuilder;

/**
* @covers \Liip\ImagineBundle\DependencyInjection\Factory\Loader\StreamLoaderFactory
* @covers \Liip\ImagineBundle\DependencyInjection\Factory\Loader\StreamLoaderFactory<extended>
*/
class StreamLoaderFactoryTest extends TestCase
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@

namespace Liip\ImagineBundle\Tests\DependencyInjection\Factory\Resolver;

use League\Flysystem\Filesystem;
use League\Flysystem\FilesystemInterface;
use League\Flysystem\FilesystemOperator;
use Liip\ImagineBundle\DependencyInjection\Factory\Resolver\FlysystemResolverFactory;
use Liip\ImagineBundle\DependencyInjection\Factory\Resolver\ResolverFactoryInterface;
use PHPUnit\Framework\TestCase;
Expand All @@ -29,7 +30,9 @@ protected function setUp(): void
{
parent::setUp();

if (!class_exists(Filesystem::class)) {
if (!interface_exists(FilesystemInterface::class)
&& !interface_exists(FilesystemOperator::class)
) {
$this->markTestSkipped('Requires the league/flysystem package.');
}
}
Expand Down Expand Up @@ -72,7 +75,12 @@ public function testCreateResolverDefinitionOnCreate(): void

$resolverDefinition = $container->getDefinition('liip_imagine.cache.resolver.the_resolver_name');
$this->assertInstanceOf(ChildDefinition::class, $resolverDefinition);
$this->assertSame('liip_imagine.cache.resolver.prototype.flysystem', $resolverDefinition->getParent());
if (interface_exists(FilesystemOperator::class)) {
$resolverName = 'liip_imagine.cache.resolver.prototype.flysystem2';
} else {
$resolverName = 'liip_imagine.cache.resolver.prototype.flysystem';
}
$this->assertSame($resolverName, $resolverDefinition->getParent());

$this->assertSame('http://images.example.com', $resolverDefinition->getArgument(2));
$this->assertSame('theCachePrefix', $resolverDefinition->getArgument(3));
Expand Down
30 changes: 11 additions & 19 deletions Tests/Imagine/Cache/Resolver/FlysystemV2ResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ public function testCreateObjectInAdapter(): void
$fs = $this->createFlySystemMock();
$fs
->expects($this->once())
->method('write')
->willReturn(true);
->method('write');

$resolver = new FlysystemV2Resolver($fs, new RequestContext(), 'http://images.example.com');

Expand Down Expand Up @@ -136,8 +135,7 @@ public function testRemoveCacheForPathAndFilterOnRemove(): void
$fs
->expects($this->once())
->method('delete')
->with('media/cache/thumb/some-folder/path.jpg')
->willReturn(true);
->with('media/cache/thumb/some-folder/path.jpg');

$resolver = new FlysystemV2Resolver($fs, new RequestContext(), 'http://images.example.com');
$resolver->remove(['some-folder/path.jpg'], ['thumb']);
Expand All @@ -154,8 +152,7 @@ public function testRemoveCacheForSomePathsAndFilterOnRemove(): void
$fs
->expects($this->at(1))
->method('delete')
->with('media/cache/thumb/pathOne.jpg')
->willReturn(true);
->with('media/cache/thumb/pathOne.jpg');
$fs
->expects($this->at(2))
->method('fileExists')
Expand All @@ -164,8 +161,7 @@ public function testRemoveCacheForSomePathsAndFilterOnRemove(): void
$fs
->expects($this->at(3))
->method('delete')
->with('media/cache/thumb/pathTwo.jpg')
->willReturn(true);
->with('media/cache/thumb/pathTwo.jpg');

$resolver = new FlysystemV2Resolver($fs, new RequestContext(), 'http://images.example.com');
$resolver->remove(
Expand All @@ -185,8 +181,7 @@ public function testRemoveCacheForSomePathsAndSomeFiltersOnRemove(): void
$fs
->expects($this->at(1))
->method('delete')
->with('media/cache/filterOne/pathOne.jpg')
->willReturn(true);
->with('media/cache/filterOne/pathOne.jpg');
$fs
->expects($this->at(2))
->method('fileExists')
Expand All @@ -195,8 +190,7 @@ public function testRemoveCacheForSomePathsAndSomeFiltersOnRemove(): void
$fs
->expects($this->at(3))
->method('delete')
->with('media/cache/filterTwo/pathOne.jpg')
->willReturn(true);
->with('media/cache/filterTwo/pathOne.jpg');
$fs
->expects($this->at(4))
->method('fileExists')
Expand All @@ -205,8 +199,7 @@ public function testRemoveCacheForSomePathsAndSomeFiltersOnRemove(): void
$fs
->expects($this->at(5))
->method('delete')
->with('media/cache/filterOne/pathTwo.jpg')
->willReturn(true);
->with('media/cache/filterOne/pathTwo.jpg');
$fs
->expects($this->at(6))
->method('fileExists')
Expand All @@ -215,8 +208,7 @@ public function testRemoveCacheForSomePathsAndSomeFiltersOnRemove(): void
$fs
->expects($this->at(7))
->method('delete')
->with('media/cache/filterTwo/pathTwo.jpg')
->willReturn(true);
->with('media/cache/filterTwo/pathTwo.jpg');

$resolver = new FlysystemV2Resolver($fs, new RequestContext(), 'http://images.example.com');
$resolver->remove(
Expand All @@ -230,7 +222,7 @@ public function testDoNothingWhenObjectNotExistForPathAndFilterOnRemove(): void
$fs = $this->createFlySystemMock();
$fs
->expects($this->once())
->method('has')
->method('fileExists')
->with('media/cache/thumb/some-folder/path.jpg')
->willReturn(false);
$fs
Expand Down Expand Up @@ -263,11 +255,11 @@ public function testRemoveCacheForSomeFiltersOnRemove(): void
$fs = $this->createFlySystemMock();
$fs
->expects($this->at(0))
->method('deleteDir')
->method('deleteDirectory')
->with('media/cache/theFilterOne');
$fs
->expects($this->at(1))
->method('deleteDir')
->method('deleteDirectory')
->with('media/cache/theFilterTwo');

$resolver = new FlysystemV2Resolver($fs, new RequestContext(), 'http://images.example.com');
Expand Down
3 changes: 1 addition & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@
"symfony/phpunit-bridge": "^5.2",
"symfony/templating": "^3.4|^4.3|^5.0",
"symfony/validator": "^3.4|^4.3|^5.0",
"symfony/yaml": "^3.4|^4.3|^5.0",
"friendsofphp/php-cs-fixer": "*"
"symfony/yaml": "^3.4|^4.3|^5.0"
},
"suggest": {
"ext-exif": "required to read EXIF metadata from images",
Expand Down