diff --git a/CHANGELOG.md b/CHANGELOG.md index e128fe1d0..0304dbd37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ for a given releases. Unreleased, upcoming changes will be updated here periodic ## 3.x (unreleased) +- Allow Flysystem resolvers to cache resolved URLs with a PSR-6 cache pool [#1663](https://github.com/liip/LiipImagineBundle/pull/1663). - Dropped support for PHP templating [#1549](https://github.com/liip/LiipImagineBundle/pull/1549). - Dropped support for old PHP versions, minimum version is now 8.1 [#1549](https://github.com/liip/LiipImagineBundle/pull/1549). - Removed legacy `AmazonS3Resolver` for the outdated `amazonwebservices/aws-sdk-for-php`, use `AwsS3Resolver` with `aws/aws-sdk-php` instead [#1549](https://github.com/liip/LiipImagineBundle/pull/1549). diff --git a/doc/cache-resolver/flysystem.rst b/doc/cache-resolver/flysystem.rst index f59f43619..384e9a114 100644 --- a/doc/cache-resolver/flysystem.rst +++ b/doc/cache-resolver/flysystem.rst @@ -43,6 +43,7 @@ Using `OneupFlysystemBundle`_, a basic configuration might look as follows: root_url: "https://images.example.com" cache_prefix: media/cache visibility: public + cache: cache.app oneup_flysystem: adapters: @@ -68,6 +69,8 @@ There are several configuration options available: The visibility is applied, when the objects are stored on a flysystem filesystem. You will most probably want to leave the default or explicitly set ``public``. Default value: ``public`` +* ``cache``: an optional PSR-6 cache pool service used to cache resolved URLs and + avoid repeated remote filesystem lookups. Default value: ``false`` Usage ----- diff --git a/src/DependencyInjection/Factory/Resolver/FlysystemResolverFactory.php b/src/DependencyInjection/Factory/Resolver/FlysystemResolverFactory.php index cd3f4c528..5f6a7415a 100644 --- a/src/DependencyInjection/Factory/Resolver/FlysystemResolverFactory.php +++ b/src/DependencyInjection/Factory/Resolver/FlysystemResolverFactory.php @@ -18,6 +18,14 @@ class FlysystemResolverFactory extends AbstractResolverFactory { + private const CACHE_ARGUMENT_INDEX = 0; + private const CACHE_CONFIG_KEY = 'cache'; + private const CACHED_RESOLVER_SUFFIX = '.cached'; + private const PSR_CACHE_RESOLVER_NAME = 'psr_cache'; + private const RESOLVER_ARGUMENT_INDEX = 1; + private const RESOLVER_TAG = 'liip_imagine.cache.resolver'; + private const RESOLVER_TAG_KEY = 'resolver'; + public function create(ContainerBuilder $container, string $name, array $config): string { $resolverDefinition = $this->getChildResolverDefinition($this->getChildResolverName()); @@ -25,13 +33,25 @@ public function create(ContainerBuilder $container, string $name, array $config) $resolverDefinition->replaceArgument(2, $config['root_url']); $resolverDefinition->replaceArgument(3, $config['cache_prefix']); $resolverDefinition->replaceArgument(4, $config['visibility']); - $resolverDefinition->addTag('liip_imagine.cache.resolver', [ - 'resolver' => $name, - ]); - - $resolverId = 'liip_imagine.cache.resolver.'.$name; + $resolverId = static::$namePrefix.'.'.$name; $container->setDefinition($resolverId, $resolverDefinition); + if ($config[self::CACHE_CONFIG_KEY]) { + $cachedResolverId = $resolverId.self::CACHED_RESOLVER_SUFFIX; + + $container->setDefinition($cachedResolverId, $resolverDefinition); + + $cacheResolverDefinition = $this->getChildResolverDefinition(self::PSR_CACHE_RESOLVER_NAME); + $cacheResolverDefinition->replaceArgument(self::CACHE_ARGUMENT_INDEX, new Reference($config[self::CACHE_CONFIG_KEY])); + $cacheResolverDefinition->replaceArgument(self::RESOLVER_ARGUMENT_INDEX, new Reference($cachedResolverId)); + + $container->setDefinition($resolverId, $cacheResolverDefinition); + } + + $container->getDefinition($resolverId)->addTag(self::RESOLVER_TAG, [ + self::RESOLVER_TAG_KEY => $name, + ]); + return $resolverId; } @@ -55,6 +75,9 @@ public function addConfiguration(ArrayNodeDefinition $builder): void ->isRequired() ->cannotBeEmpty() ->end() + ->scalarNode(self::CACHE_CONFIG_KEY) + ->defaultFalse() + ->end() ->enumNode('visibility') ->values(['public', 'private', 'noPredefinedVisibility']) ->defaultValue('public') diff --git a/tests/DependencyInjection/Factory/Resolver/FlysystemResolverFactoryTest.php b/tests/DependencyInjection/Factory/Resolver/FlysystemResolverFactoryTest.php index 554e6fd4f..ef59b5e34 100644 --- a/tests/DependencyInjection/Factory/Resolver/FlysystemResolverFactoryTest.php +++ b/tests/DependencyInjection/Factory/Resolver/FlysystemResolverFactoryTest.php @@ -18,12 +18,24 @@ use Symfony\Component\Config\Definition\Processor; use Symfony\Component\DependencyInjection\ChildDefinition; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Reference; /** * @covers \Liip\ImagineBundle\DependencyInjection\Factory\Resolver\FlysystemResolverFactory */ class FlysystemResolverFactoryTest extends TestCase { + private const CACHE_PREFIX = 'theCachePrefix'; + private const CACHE_SERVICE_ID = 'the_cache_service_id'; + private const CACHED_RESOLVER_ID = 'liip_imagine.cache.resolver.the_resolver_name.cached'; + private const FILESYSTEM_SERVICE_ID = 'flyfilesystemservice'; + private const FLYSYSTEM_RESOLVER_PARENT = 'liip_imagine.cache.resolver.prototype.flysystem2'; + private const PSR_CACHE_RESOLVER_PARENT = 'liip_imagine.cache.resolver.prototype.psr_cache'; + private const RESOLVER_ID = 'liip_imagine.cache.resolver.the_resolver_name'; + private const RESOLVER_NAME = 'the_resolver_name'; + private const RESOLVER_TAG = 'liip_imagine.cache.resolver'; + private const ROOT_URL = 'http://images.example.com'; + public function testImplementsResolverFactoryInterface(): void { $rc = new \ReflectionClass(FlysystemResolverFactory::class); @@ -51,30 +63,61 @@ public function testCreateResolverDefinitionOnCreate(): void $resolver = new FlysystemResolverFactory(); - $resolver->create($container, 'the_resolver_name', [ - 'filesystem_service' => 'flyfilesystemservice', - 'root_url' => 'http://images.example.com', - 'cache_prefix' => 'theCachePrefix', + $resolver->create($container, self::RESOLVER_NAME, [ + 'filesystem_service' => self::FILESYSTEM_SERVICE_ID, + 'root_url' => self::ROOT_URL, + 'cache_prefix' => self::CACHE_PREFIX, 'visibility' => 'public', + 'cache' => false, ]); - $this->assertTrue($container->hasDefinition('liip_imagine.cache.resolver.the_resolver_name')); + $this->assertTrue($container->hasDefinition(self::RESOLVER_ID)); - $resolverDefinition = $container->getDefinition('liip_imagine.cache.resolver.the_resolver_name'); + $resolverDefinition = $container->getDefinition(self::RESOLVER_ID); $this->assertInstanceOf(ChildDefinition::class, $resolverDefinition); - $resolverName = 'liip_imagine.cache.resolver.prototype.flysystem2'; - $this->assertSame($resolverName, $resolverDefinition->getParent()); + $this->assertSame(self::FLYSYSTEM_RESOLVER_PARENT, $resolverDefinition->getParent()); - $this->assertSame('http://images.example.com', $resolverDefinition->getArgument(2)); - $this->assertSame('theCachePrefix', $resolverDefinition->getArgument(3)); + $this->assertSame(self::ROOT_URL, $resolverDefinition->getArgument(2)); + $this->assertSame(self::CACHE_PREFIX, $resolverDefinition->getArgument(3)); $this->assertSame('public', $resolverDefinition->getArgument(4)); } + public function testWrapResolverWithPsrCacheOnCreate(): void + { + $container = new ContainerBuilder(); + + $resolver = new FlysystemResolverFactory(); + + $resolver->create($container, self::RESOLVER_NAME, [ + 'filesystem_service' => self::FILESYSTEM_SERVICE_ID, + 'root_url' => self::ROOT_URL, + 'cache_prefix' => self::CACHE_PREFIX, + 'visibility' => 'public', + 'cache' => self::CACHE_SERVICE_ID, + ]); + + $this->assertTrue($container->hasDefinition(self::CACHED_RESOLVER_ID)); + $cachedResolverDefinition = $container->getDefinition(self::CACHED_RESOLVER_ID); + $this->assertInstanceOf(ChildDefinition::class, $cachedResolverDefinition); + $this->assertSame(self::FLYSYSTEM_RESOLVER_PARENT, $cachedResolverDefinition->getParent()); + + $resolverDefinition = $container->getDefinition(self::RESOLVER_ID); + $this->assertInstanceOf(ChildDefinition::class, $resolverDefinition); + $this->assertSame(self::PSR_CACHE_RESOLVER_PARENT, $resolverDefinition->getParent()); + + $this->assertInstanceOf(Reference::class, $resolverDefinition->getArgument(0)); + $this->assertSame(self::CACHE_SERVICE_ID, (string) $resolverDefinition->getArgument(0)); + + $this->assertInstanceOf(Reference::class, $resolverDefinition->getArgument(1)); + $this->assertSame(self::CACHED_RESOLVER_ID, (string) $resolverDefinition->getArgument(1)); + $this->assertSame([['resolver' => self::RESOLVER_NAME]], $resolverDefinition->getTag(self::RESOLVER_TAG)); + } + public function testProcessCorrectlyOptionsOnAddConfiguration(): void { - $expectedRootUrl = 'http://images.example.com'; - $expectedCachePrefix = 'theCachePrefix'; - $expectedFlysystemService = 'flyfilesystemservice'; + $expectedRootUrl = self::ROOT_URL; + $expectedCachePrefix = self::CACHE_PREFIX; + $expectedFlysystemService = self::FILESYSTEM_SERVICE_ID; $expectedVisibility = 'public'; $treeBuilder = new TreeBuilder('flysystem'); @@ -101,6 +144,9 @@ public function testProcessCorrectlyOptionsOnAddConfiguration(): void $this->assertArrayHasKey('visibility', $config); $this->assertSame($expectedVisibility, $config['visibility']); + + $this->assertArrayHasKey('cache', $config); + $this->assertFalse($config['cache']); } public function testAddDefaultOptionsIfNotSetOnAddConfiguration(): void diff --git a/tests/Functional/Imagine/Cache/Resolver/CachedFlysystemResolverTest.php b/tests/Functional/Imagine/Cache/Resolver/CachedFlysystemResolverTest.php new file mode 100644 index 000000000..b822737e3 --- /dev/null +++ b/tests/Functional/Imagine/Cache/Resolver/CachedFlysystemResolverTest.php @@ -0,0 +1,34 @@ +createClient(); + + $resolver = self::$kernel->getContainer()->get(self::RESOLVER_SERVICE_ID); + + $this->assertInstanceOf(PsrCacheResolver::class, $resolver); + $this->assertInstanceOf(FlysystemV2Resolver::class, $this->getPrivateProperty($resolver, 'resolver')); + } +} diff --git a/tests/Functional/app/AppKernel.php b/tests/Functional/app/AppKernel.php index 4d97beb7f..b09954d0b 100644 --- a/tests/Functional/app/AppKernel.php +++ b/tests/Functional/app/AppKernel.php @@ -59,6 +59,9 @@ public function registerContainerConfiguration(LoaderInterface $loader): void $container ->setAlias('test.liip_imagine.filter.manager', 'liip_imagine.filter.manager') ->setPublic(true); + $container + ->setAlias('test.liip_imagine.cache.resolver.cached_flysystem', 'liip_imagine.cache.resolver.cached_flysystem') + ->setPublic(true); }); } } diff --git a/tests/Functional/app/config/config.yml b/tests/Functional/app/config/config.yml index dcfa5f5e3..139c080ab 100644 --- a/tests/Functional/app/config/config.yml +++ b/tests/Functional/app/config/config.yml @@ -6,6 +6,12 @@ parameters: services: logger: class: \Psr\Log\NullLogger + test.flysystem.adapter: + class: League\Flysystem\Local\LocalFilesystemAdapter + arguments: ["%kernel.cache_dir%/flysystem"] + test.flysystem: + class: League\Flysystem\Filesystem + arguments: ['@test.flysystem.adapter'] framework: @@ -68,6 +74,12 @@ liip_imagine: resolvers: + cached_flysystem: + flysystem: + filesystem_service: test.flysystem + root_url: "https://images.example.com" + cache: cache.app + default: web_path: web_root: "%kernel.project_dir%/public"