From 9729b39429a2e49f4b1f9eb68752e76fadc20b22 Mon Sep 17 00:00:00 2001 From: Wouter Brinkman Date: Mon, 28 Aug 2023 10:27:30 +0200 Subject: [PATCH 1/9] Add support for JsonManifestVersionStrategy --- .../Compiler/AssetsVersionCompilerPass.php | 34 +++++++++--- Resources/config/imagine_twig_mode_lazy.xml | 1 + Templating/LazyFilterRuntime.php | 54 ++++++++++++++----- Tests/Templating/LazyFilterRuntimeTest.php | 20 +++++++ composer.json | 1 + 5 files changed, 90 insertions(+), 20 deletions(-) diff --git a/DependencyInjection/Compiler/AssetsVersionCompilerPass.php b/DependencyInjection/Compiler/AssetsVersionCompilerPass.php index fba063ec0..a76ef58d2 100644 --- a/DependencyInjection/Compiler/AssetsVersionCompilerPass.php +++ b/DependencyInjection/Compiler/AssetsVersionCompilerPass.php @@ -11,6 +11,7 @@ namespace Liip\ImagineBundle\DependencyInjection\Compiler; +use Symfony\Component\Asset\VersionStrategy\JsonManifestVersionStrategy; use Symfony\Component\Asset\VersionStrategy\StaticVersionStrategy; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -18,20 +19,22 @@ * Inject the Symfony framework assets version parameter to the * LiipImagineBundle twig extension if possible. * - * We extract the version parameter from the StaticVersionStrategy service - * definition. If anything is not as expected, we log a warning and do nothing. + * We extract either: + * - the version parameter from the StaticVersionStrategy service + * - the json manifest from the JsonManifestVersionStrategy service + * If anything is not as expected, we log a warning and do nothing. * * The expectation is for the user to configure the assets version in liip * imagine for custom setups. * - * Anything other than StaticVersionStrategy needs to be implemented by the - * user in CacheResolveEvent event listeners. + * Anything other than StaticVersionStrategy or JsonManifestVersionStrategy needs + * to be implemented by the user in CacheResolveEvent event listeners. */ class AssetsVersionCompilerPass extends AbstractCompilerPass { public function process(ContainerBuilder $container): void { - if (!class_exists(StaticVersionStrategy::class) + if (!class_exists(StaticVersionStrategy::class) || !class_exists(JsonManifestVersionStrategy::class) // this application has no asset version configured || !$container->has('assets._version__default') // we are not using the new LazyFilterRuntime @@ -46,13 +49,14 @@ public function process(ContainerBuilder $container): void } $versionStrategyDefinition = $container->findDefinition('assets._version__default'); - if (!is_a($versionStrategyDefinition->getClass(), StaticVersionStrategy::class, true)) { + if (!is_a($versionStrategyDefinition->getClass(), StaticVersionStrategy::class, true) && !is_a($versionStrategyDefinition->getClass(), JsonManifestVersionStrategy::class, true)) { $this->log($container, 'Symfony assets versioning strategy "'.$versionStrategyDefinition->getClass().'" not automatically supported. Configure liip_imagine.twig.assets_version if you have problems with assets versioning'); return; } $version = $versionStrategyDefinition->getArgument(0); $format = $versionStrategyDefinition->getArgument(1); + $format = $container->resolveEnvPlaceholders($format); if ($format && !$this->str_ends_with($format, '?%%s')) { $this->log($container, 'Can not handle assets versioning with custom format "'.$format.'". asset twig function can likely not be used with the imagine_filter'); @@ -61,6 +65,24 @@ public function process(ContainerBuilder $container): void } $runtimeDefinition->setArgument(1, $version); + + if (is_a($versionStrategyDefinition->getClass(), JsonManifestVersionStrategy::class, true)) { + $jsonManifest = file_get_contents($version); + + if (!is_string($jsonManifest)) { + $this->log($container, 'Can not handle assets versioning with "'.$versionStrategyDefinition->getClass().'". The manifest file at "'.$version.' " could not be read'); + + return; + } + $jsonManifest = \json_decode($jsonManifest, true); + if (!is_array($jsonManifest)) { + $this->log($container, 'Can not handle assets versioning with "'.$versionStrategyDefinition->getClass().'". The manifest file at "'.$version.' " does not contain valid JSON'); + + return; + } + $runtimeDefinition->setArgument(1, null); + $runtimeDefinition->setArgument(2, $jsonManifest); + } } /** diff --git a/Resources/config/imagine_twig_mode_lazy.xml b/Resources/config/imagine_twig_mode_lazy.xml index 9e4a9f1bb..ac9ec6f19 100644 --- a/Resources/config/imagine_twig_mode_lazy.xml +++ b/Resources/config/imagine_twig_mode_lazy.xml @@ -14,6 +14,7 @@ null + null diff --git a/Templating/LazyFilterRuntime.php b/Templating/LazyFilterRuntime.php index 59a300e59..aed5e637c 100644 --- a/Templating/LazyFilterRuntime.php +++ b/Templating/LazyFilterRuntime.php @@ -29,10 +29,16 @@ final class LazyFilterRuntime implements RuntimeExtensionInterface */ private $assetVersion; - public function __construct(CacheManager $cache, string $assetVersion = null) + /** + * @var array|null + */ + private $jsonManifest; + + public function __construct(CacheManager $cache, string $assetVersion = null, array $jsonManifest = null) { $this->cache = $cache; $this->assetVersion = $assetVersion; + $this->jsonManifest = $jsonManifest; } /** @@ -41,9 +47,9 @@ public function __construct(CacheManager $cache, string $assetVersion = null) public function filter(string $path, string $filter, array $config = [], string $resolver = null, int $referenceType = UrlGeneratorInterface::ABSOLUTE_URL): string { $path = $this->cleanPath($path); - $path = $this->cache->getBrowserPath($path, $filter, $config, $resolver, $referenceType); + $resolvedPath = $this->cache->getBrowserPath($path, $filter, $config, $resolver, $referenceType); - return $this->appendAssetVersion($path); + return $this->appendAssetVersion($resolvedPath, $path); } /** @@ -57,33 +63,53 @@ public function filterCache(string $path, string $filter, array $config = [], st if (\count($config)) { $path = $this->cache->getRuntimePath($path, $config); } - $path = $this->cache->resolve($path, $filter, $resolver); + $resolvedPath = $this->cache->resolve($path, $filter, $resolver); - return $this->appendAssetVersion($path); + return $this->appendAssetVersion($resolvedPath, $path); } private function cleanPath(string $path): string { - if (!$this->assetVersion) { + if (!$this->assetVersion && !$this->jsonManifest) { return $path; } - $start = mb_strrpos($path, $this->assetVersion); - if (mb_strlen($path) - mb_strlen($this->assetVersion) === $start) { - return rtrim(mb_substr($path, 0, $start), '?'); + if ($this->assetVersion) { + $start = mb_strrpos($path, $this->assetVersion); + if (mb_strlen($path) - mb_strlen($this->assetVersion) === $start) { + return rtrim(mb_substr($path, 0, $start), '?'); + } + } elseif ($this->jsonManifest) { + $asset = array_search($path, $this->jsonManifest, true); + if ($asset) { + return $asset; + } } return $path; } - private function appendAssetVersion(string $path): string + private function appendAssetVersion(string $resolvedPath, string $path): string { - if (!$this->assetVersion) { - return $path; + if (!$this->assetVersion && !$this->jsonManifest) { + return $resolvedPath; } - $separator = false !== mb_strpos($path, '?') ? '&' : '?'; + if ($this->assetVersion) { + $separator = false !== mb_strpos($resolvedPath, '?') ? '&' : '?'; + + return $resolvedPath.$separator.$this->assetVersion; + } elseif ($this->jsonManifest) { + $manifestVersion = array_key_exists($path, $this->jsonManifest) ? $this->jsonManifest[$path] : null; + if ($manifestVersion) { + $resolvedPath = str_replace($path, $resolvedPath, $manifestVersion); + + if (str_starts_with($manifestVersion, '/') && !str_starts_with($resolvedPath, '/')) { + $resolvedPath = '/' . $resolvedPath; + } + } + } - return $path.$separator.$this->assetVersion; + return $resolvedPath; } } diff --git a/Tests/Templating/LazyFilterRuntimeTest.php b/Tests/Templating/LazyFilterRuntimeTest.php index 0167feb42..38abb1870 100644 --- a/Tests/Templating/LazyFilterRuntimeTest.php +++ b/Tests/Templating/LazyFilterRuntimeTest.php @@ -23,6 +23,7 @@ class LazyFilterRuntimeTest extends AbstractTest { private const FILTER = 'thumbnail'; private const VERSION = 'v2'; + private const JSON_MANIFEST = ['image/cats.png' => '/image/cats.png?v2']; /** * @var LazyFilterRuntime @@ -101,6 +102,25 @@ public function testDifferentVersion(): void $this->assertSame($cachePath.'?'.self::VERSION, $actualPath); } + public function testJsonManifestVersionHandling(): void + { + $this->runtime = new LazyFilterRuntime($this->manager, null, self::JSON_MANIFEST); + + $sourcePath = array_keys(self::JSON_MANIFEST)[0]; + $versionedPath = array_values(self::JSON_MANIFEST)[0]; + + $cachePath = str_replace('image/', 'image/cache/' . self::FILTER . '/', $sourcePath); + $expectedPath = str_replace('image/', 'image/cache/' . self::FILTER . '/', $versionedPath); + + $this->manager + ->expects($this->once()) + ->method('getBrowserPath') + ->with($sourcePath, self::FILTER) + ->willReturn($cachePath); + + $this->assertSame($expectedPath, $this->runtime->filter($versionedPath, self::FILTER)); + } + public function testInvokeFilterCacheMethod(): void { $expectedInputPath = 'thePathToTheImage'; diff --git a/composer.json b/composer.json index 55dbcad2a..866b8fdce 100644 --- a/composer.json +++ b/composer.json @@ -53,6 +53,7 @@ }, "suggest": { "ext-exif": "required to read EXIF metadata from images", + "ext-json": "required to read JSON manifest versioning", "ext-gd": "required to use gd driver", "ext-gmagick": "required to use gmagick driver", "ext-imagick": "required to use imagick driver", From eb3132fc88a3cf829ae9b2fa6ec2a2c07ffcb967 Mon Sep 17 00:00:00 2001 From: Wouter Brinkman Date: Mon, 28 Aug 2023 14:03:16 +0200 Subject: [PATCH 2/9] Fix test --- Templating/LazyFilterRuntime.php | 6 +----- Tests/Templating/LazyFilterRuntimeTest.php | 9 ++++++--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/Templating/LazyFilterRuntime.php b/Templating/LazyFilterRuntime.php index aed5e637c..e4559a7a9 100644 --- a/Templating/LazyFilterRuntime.php +++ b/Templating/LazyFilterRuntime.php @@ -102,11 +102,7 @@ private function appendAssetVersion(string $resolvedPath, string $path): string } elseif ($this->jsonManifest) { $manifestVersion = array_key_exists($path, $this->jsonManifest) ? $this->jsonManifest[$path] : null; if ($manifestVersion) { - $resolvedPath = str_replace($path, $resolvedPath, $manifestVersion); - - if (str_starts_with($manifestVersion, '/') && !str_starts_with($resolvedPath, '/')) { - $resolvedPath = '/' . $resolvedPath; - } + $resolvedPath = str_replace($path, $manifestVersion, $resolvedPath); } } diff --git a/Tests/Templating/LazyFilterRuntimeTest.php b/Tests/Templating/LazyFilterRuntimeTest.php index 38abb1870..0f86f9f10 100644 --- a/Tests/Templating/LazyFilterRuntimeTest.php +++ b/Tests/Templating/LazyFilterRuntimeTest.php @@ -15,6 +15,7 @@ use Liip\ImagineBundle\Templating\LazyFilterRuntime; use Liip\ImagineBundle\Tests\AbstractTest; use PHPUnit\Framework\MockObject\MockObject; +use Symfony\Component\Routing\Generator\UrlGeneratorInterface; /** * @covers \Liip\ImagineBundle\Templating\LazyFilterRuntime @@ -109,8 +110,8 @@ public function testJsonManifestVersionHandling(): void $sourcePath = array_keys(self::JSON_MANIFEST)[0]; $versionedPath = array_values(self::JSON_MANIFEST)[0]; - $cachePath = str_replace('image/', 'image/cache/' . self::FILTER . '/', $sourcePath); - $expectedPath = str_replace('image/', 'image/cache/' . self::FILTER . '/', $versionedPath); + $cachePath = 'image/cache/' . self::FILTER . '/' . $sourcePath; + $expectedPath = 'image/cache/' . self::FILTER . '/' . $versionedPath; $this->manager ->expects($this->once()) @@ -118,7 +119,9 @@ public function testJsonManifestVersionHandling(): void ->with($sourcePath, self::FILTER) ->willReturn($cachePath); - $this->assertSame($expectedPath, $this->runtime->filter($versionedPath, self::FILTER)); + $actualPath = $this->runtime->filter($versionedPath, self::FILTER); + + $this->assertSame($expectedPath, $actualPath); } public function testInvokeFilterCacheMethod(): void From a244035e5bcfaa8434ce0710ad19e79b6de54d07 Mon Sep 17 00:00:00 2001 From: Wouter Brinkman Date: Mon, 28 Aug 2023 10:27:30 +0200 Subject: [PATCH 3/9] Add support for JsonManifestVersionStrategy --- .../Compiler/AssetsVersionCompilerPass.php | 34 +++++++++--- Resources/config/imagine_twig_mode_lazy.xml | 1 + Templating/LazyFilterRuntime.php | 54 ++++++++++++++----- Tests/Templating/LazyFilterRuntimeTest.php | 20 +++++++ composer.json | 1 + 5 files changed, 90 insertions(+), 20 deletions(-) diff --git a/DependencyInjection/Compiler/AssetsVersionCompilerPass.php b/DependencyInjection/Compiler/AssetsVersionCompilerPass.php index fba063ec0..a76ef58d2 100644 --- a/DependencyInjection/Compiler/AssetsVersionCompilerPass.php +++ b/DependencyInjection/Compiler/AssetsVersionCompilerPass.php @@ -11,6 +11,7 @@ namespace Liip\ImagineBundle\DependencyInjection\Compiler; +use Symfony\Component\Asset\VersionStrategy\JsonManifestVersionStrategy; use Symfony\Component\Asset\VersionStrategy\StaticVersionStrategy; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -18,20 +19,22 @@ * Inject the Symfony framework assets version parameter to the * LiipImagineBundle twig extension if possible. * - * We extract the version parameter from the StaticVersionStrategy service - * definition. If anything is not as expected, we log a warning and do nothing. + * We extract either: + * - the version parameter from the StaticVersionStrategy service + * - the json manifest from the JsonManifestVersionStrategy service + * If anything is not as expected, we log a warning and do nothing. * * The expectation is for the user to configure the assets version in liip * imagine for custom setups. * - * Anything other than StaticVersionStrategy needs to be implemented by the - * user in CacheResolveEvent event listeners. + * Anything other than StaticVersionStrategy or JsonManifestVersionStrategy needs + * to be implemented by the user in CacheResolveEvent event listeners. */ class AssetsVersionCompilerPass extends AbstractCompilerPass { public function process(ContainerBuilder $container): void { - if (!class_exists(StaticVersionStrategy::class) + if (!class_exists(StaticVersionStrategy::class) || !class_exists(JsonManifestVersionStrategy::class) // this application has no asset version configured || !$container->has('assets._version__default') // we are not using the new LazyFilterRuntime @@ -46,13 +49,14 @@ public function process(ContainerBuilder $container): void } $versionStrategyDefinition = $container->findDefinition('assets._version__default'); - if (!is_a($versionStrategyDefinition->getClass(), StaticVersionStrategy::class, true)) { + if (!is_a($versionStrategyDefinition->getClass(), StaticVersionStrategy::class, true) && !is_a($versionStrategyDefinition->getClass(), JsonManifestVersionStrategy::class, true)) { $this->log($container, 'Symfony assets versioning strategy "'.$versionStrategyDefinition->getClass().'" not automatically supported. Configure liip_imagine.twig.assets_version if you have problems with assets versioning'); return; } $version = $versionStrategyDefinition->getArgument(0); $format = $versionStrategyDefinition->getArgument(1); + $format = $container->resolveEnvPlaceholders($format); if ($format && !$this->str_ends_with($format, '?%%s')) { $this->log($container, 'Can not handle assets versioning with custom format "'.$format.'". asset twig function can likely not be used with the imagine_filter'); @@ -61,6 +65,24 @@ public function process(ContainerBuilder $container): void } $runtimeDefinition->setArgument(1, $version); + + if (is_a($versionStrategyDefinition->getClass(), JsonManifestVersionStrategy::class, true)) { + $jsonManifest = file_get_contents($version); + + if (!is_string($jsonManifest)) { + $this->log($container, 'Can not handle assets versioning with "'.$versionStrategyDefinition->getClass().'". The manifest file at "'.$version.' " could not be read'); + + return; + } + $jsonManifest = \json_decode($jsonManifest, true); + if (!is_array($jsonManifest)) { + $this->log($container, 'Can not handle assets versioning with "'.$versionStrategyDefinition->getClass().'". The manifest file at "'.$version.' " does not contain valid JSON'); + + return; + } + $runtimeDefinition->setArgument(1, null); + $runtimeDefinition->setArgument(2, $jsonManifest); + } } /** diff --git a/Resources/config/imagine_twig_mode_lazy.xml b/Resources/config/imagine_twig_mode_lazy.xml index 9e4a9f1bb..ac9ec6f19 100644 --- a/Resources/config/imagine_twig_mode_lazy.xml +++ b/Resources/config/imagine_twig_mode_lazy.xml @@ -14,6 +14,7 @@ null + null diff --git a/Templating/LazyFilterRuntime.php b/Templating/LazyFilterRuntime.php index 59a300e59..aed5e637c 100644 --- a/Templating/LazyFilterRuntime.php +++ b/Templating/LazyFilterRuntime.php @@ -29,10 +29,16 @@ final class LazyFilterRuntime implements RuntimeExtensionInterface */ private $assetVersion; - public function __construct(CacheManager $cache, string $assetVersion = null) + /** + * @var array|null + */ + private $jsonManifest; + + public function __construct(CacheManager $cache, string $assetVersion = null, array $jsonManifest = null) { $this->cache = $cache; $this->assetVersion = $assetVersion; + $this->jsonManifest = $jsonManifest; } /** @@ -41,9 +47,9 @@ public function __construct(CacheManager $cache, string $assetVersion = null) public function filter(string $path, string $filter, array $config = [], string $resolver = null, int $referenceType = UrlGeneratorInterface::ABSOLUTE_URL): string { $path = $this->cleanPath($path); - $path = $this->cache->getBrowserPath($path, $filter, $config, $resolver, $referenceType); + $resolvedPath = $this->cache->getBrowserPath($path, $filter, $config, $resolver, $referenceType); - return $this->appendAssetVersion($path); + return $this->appendAssetVersion($resolvedPath, $path); } /** @@ -57,33 +63,53 @@ public function filterCache(string $path, string $filter, array $config = [], st if (\count($config)) { $path = $this->cache->getRuntimePath($path, $config); } - $path = $this->cache->resolve($path, $filter, $resolver); + $resolvedPath = $this->cache->resolve($path, $filter, $resolver); - return $this->appendAssetVersion($path); + return $this->appendAssetVersion($resolvedPath, $path); } private function cleanPath(string $path): string { - if (!$this->assetVersion) { + if (!$this->assetVersion && !$this->jsonManifest) { return $path; } - $start = mb_strrpos($path, $this->assetVersion); - if (mb_strlen($path) - mb_strlen($this->assetVersion) === $start) { - return rtrim(mb_substr($path, 0, $start), '?'); + if ($this->assetVersion) { + $start = mb_strrpos($path, $this->assetVersion); + if (mb_strlen($path) - mb_strlen($this->assetVersion) === $start) { + return rtrim(mb_substr($path, 0, $start), '?'); + } + } elseif ($this->jsonManifest) { + $asset = array_search($path, $this->jsonManifest, true); + if ($asset) { + return $asset; + } } return $path; } - private function appendAssetVersion(string $path): string + private function appendAssetVersion(string $resolvedPath, string $path): string { - if (!$this->assetVersion) { - return $path; + if (!$this->assetVersion && !$this->jsonManifest) { + return $resolvedPath; } - $separator = false !== mb_strpos($path, '?') ? '&' : '?'; + if ($this->assetVersion) { + $separator = false !== mb_strpos($resolvedPath, '?') ? '&' : '?'; + + return $resolvedPath.$separator.$this->assetVersion; + } elseif ($this->jsonManifest) { + $manifestVersion = array_key_exists($path, $this->jsonManifest) ? $this->jsonManifest[$path] : null; + if ($manifestVersion) { + $resolvedPath = str_replace($path, $resolvedPath, $manifestVersion); + + if (str_starts_with($manifestVersion, '/') && !str_starts_with($resolvedPath, '/')) { + $resolvedPath = '/' . $resolvedPath; + } + } + } - return $path.$separator.$this->assetVersion; + return $resolvedPath; } } diff --git a/Tests/Templating/LazyFilterRuntimeTest.php b/Tests/Templating/LazyFilterRuntimeTest.php index 0167feb42..38abb1870 100644 --- a/Tests/Templating/LazyFilterRuntimeTest.php +++ b/Tests/Templating/LazyFilterRuntimeTest.php @@ -23,6 +23,7 @@ class LazyFilterRuntimeTest extends AbstractTest { private const FILTER = 'thumbnail'; private const VERSION = 'v2'; + private const JSON_MANIFEST = ['image/cats.png' => '/image/cats.png?v2']; /** * @var LazyFilterRuntime @@ -101,6 +102,25 @@ public function testDifferentVersion(): void $this->assertSame($cachePath.'?'.self::VERSION, $actualPath); } + public function testJsonManifestVersionHandling(): void + { + $this->runtime = new LazyFilterRuntime($this->manager, null, self::JSON_MANIFEST); + + $sourcePath = array_keys(self::JSON_MANIFEST)[0]; + $versionedPath = array_values(self::JSON_MANIFEST)[0]; + + $cachePath = str_replace('image/', 'image/cache/' . self::FILTER . '/', $sourcePath); + $expectedPath = str_replace('image/', 'image/cache/' . self::FILTER . '/', $versionedPath); + + $this->manager + ->expects($this->once()) + ->method('getBrowserPath') + ->with($sourcePath, self::FILTER) + ->willReturn($cachePath); + + $this->assertSame($expectedPath, $this->runtime->filter($versionedPath, self::FILTER)); + } + public function testInvokeFilterCacheMethod(): void { $expectedInputPath = 'thePathToTheImage'; diff --git a/composer.json b/composer.json index 55dbcad2a..866b8fdce 100644 --- a/composer.json +++ b/composer.json @@ -53,6 +53,7 @@ }, "suggest": { "ext-exif": "required to read EXIF metadata from images", + "ext-json": "required to read JSON manifest versioning", "ext-gd": "required to use gd driver", "ext-gmagick": "required to use gmagick driver", "ext-imagick": "required to use imagick driver", From 995155401b87539c9a5c7702ec618d4fc745fed2 Mon Sep 17 00:00:00 2001 From: Wouter Brinkman Date: Mon, 28 Aug 2023 14:03:16 +0200 Subject: [PATCH 4/9] Fix test --- Templating/LazyFilterRuntime.php | 6 +----- Tests/Templating/LazyFilterRuntimeTest.php | 9 ++++++--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/Templating/LazyFilterRuntime.php b/Templating/LazyFilterRuntime.php index aed5e637c..e4559a7a9 100644 --- a/Templating/LazyFilterRuntime.php +++ b/Templating/LazyFilterRuntime.php @@ -102,11 +102,7 @@ private function appendAssetVersion(string $resolvedPath, string $path): string } elseif ($this->jsonManifest) { $manifestVersion = array_key_exists($path, $this->jsonManifest) ? $this->jsonManifest[$path] : null; if ($manifestVersion) { - $resolvedPath = str_replace($path, $resolvedPath, $manifestVersion); - - if (str_starts_with($manifestVersion, '/') && !str_starts_with($resolvedPath, '/')) { - $resolvedPath = '/' . $resolvedPath; - } + $resolvedPath = str_replace($path, $manifestVersion, $resolvedPath); } } diff --git a/Tests/Templating/LazyFilterRuntimeTest.php b/Tests/Templating/LazyFilterRuntimeTest.php index 38abb1870..0f86f9f10 100644 --- a/Tests/Templating/LazyFilterRuntimeTest.php +++ b/Tests/Templating/LazyFilterRuntimeTest.php @@ -15,6 +15,7 @@ use Liip\ImagineBundle\Templating\LazyFilterRuntime; use Liip\ImagineBundle\Tests\AbstractTest; use PHPUnit\Framework\MockObject\MockObject; +use Symfony\Component\Routing\Generator\UrlGeneratorInterface; /** * @covers \Liip\ImagineBundle\Templating\LazyFilterRuntime @@ -109,8 +110,8 @@ public function testJsonManifestVersionHandling(): void $sourcePath = array_keys(self::JSON_MANIFEST)[0]; $versionedPath = array_values(self::JSON_MANIFEST)[0]; - $cachePath = str_replace('image/', 'image/cache/' . self::FILTER . '/', $sourcePath); - $expectedPath = str_replace('image/', 'image/cache/' . self::FILTER . '/', $versionedPath); + $cachePath = 'image/cache/' . self::FILTER . '/' . $sourcePath; + $expectedPath = 'image/cache/' . self::FILTER . '/' . $versionedPath; $this->manager ->expects($this->once()) @@ -118,7 +119,9 @@ public function testJsonManifestVersionHandling(): void ->with($sourcePath, self::FILTER) ->willReturn($cachePath); - $this->assertSame($expectedPath, $this->runtime->filter($versionedPath, self::FILTER)); + $actualPath = $this->runtime->filter($versionedPath, self::FILTER); + + $this->assertSame($expectedPath, $actualPath); } public function testInvokeFilterCacheMethod(): void From 2e6532434c5fe0cda3c2539461ede844e8e4c3c4 Mon Sep 17 00:00:00 2001 From: Wouter Brinkman Date: Mon, 4 Sep 2023 09:23:43 +0200 Subject: [PATCH 5/9] Codestyle --- .../Compiler/AssetsVersionCompilerPass.php | 9 ++++----- Templating/LazyFilterRuntime.php | 2 +- Tests/Templating/LazyFilterRuntimeTest.php | 5 ++--- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/DependencyInjection/Compiler/AssetsVersionCompilerPass.php b/DependencyInjection/Compiler/AssetsVersionCompilerPass.php index a76ef58d2..e6b9286a2 100644 --- a/DependencyInjection/Compiler/AssetsVersionCompilerPass.php +++ b/DependencyInjection/Compiler/AssetsVersionCompilerPass.php @@ -56,7 +56,6 @@ public function process(ContainerBuilder $container): void } $version = $versionStrategyDefinition->getArgument(0); $format = $versionStrategyDefinition->getArgument(1); - $format = $container->resolveEnvPlaceholders($format); if ($format && !$this->str_ends_with($format, '?%%s')) { $this->log($container, 'Can not handle assets versioning with custom format "'.$format.'". asset twig function can likely not be used with the imagine_filter'); @@ -67,15 +66,15 @@ public function process(ContainerBuilder $container): void $runtimeDefinition->setArgument(1, $version); if (is_a($versionStrategyDefinition->getClass(), JsonManifestVersionStrategy::class, true)) { - $jsonManifest = file_get_contents($version); + $jsonManifestString = file_get_contents($version); - if (!is_string($jsonManifest)) { + if (!\is_string($jsonManifestString)) { $this->log($container, 'Can not handle assets versioning with "'.$versionStrategyDefinition->getClass().'". The manifest file at "'.$version.' " could not be read'); return; } - $jsonManifest = \json_decode($jsonManifest, true); - if (!is_array($jsonManifest)) { + $jsonManifest = json_decode($jsonManifestString, true); + if (!\is_array($jsonManifest)) { $this->log($container, 'Can not handle assets versioning with "'.$versionStrategyDefinition->getClass().'". The manifest file at "'.$version.' " does not contain valid JSON'); return; diff --git a/Templating/LazyFilterRuntime.php b/Templating/LazyFilterRuntime.php index e4559a7a9..b3655abe8 100644 --- a/Templating/LazyFilterRuntime.php +++ b/Templating/LazyFilterRuntime.php @@ -100,7 +100,7 @@ private function appendAssetVersion(string $resolvedPath, string $path): string return $resolvedPath.$separator.$this->assetVersion; } elseif ($this->jsonManifest) { - $manifestVersion = array_key_exists($path, $this->jsonManifest) ? $this->jsonManifest[$path] : null; + $manifestVersion = \array_key_exists($path, $this->jsonManifest) ? $this->jsonManifest[$path] : null; if ($manifestVersion) { $resolvedPath = str_replace($path, $manifestVersion, $resolvedPath); } diff --git a/Tests/Templating/LazyFilterRuntimeTest.php b/Tests/Templating/LazyFilterRuntimeTest.php index 0f86f9f10..97474ca43 100644 --- a/Tests/Templating/LazyFilterRuntimeTest.php +++ b/Tests/Templating/LazyFilterRuntimeTest.php @@ -15,7 +15,6 @@ use Liip\ImagineBundle\Templating\LazyFilterRuntime; use Liip\ImagineBundle\Tests\AbstractTest; use PHPUnit\Framework\MockObject\MockObject; -use Symfony\Component\Routing\Generator\UrlGeneratorInterface; /** * @covers \Liip\ImagineBundle\Templating\LazyFilterRuntime @@ -110,8 +109,8 @@ public function testJsonManifestVersionHandling(): void $sourcePath = array_keys(self::JSON_MANIFEST)[0]; $versionedPath = array_values(self::JSON_MANIFEST)[0]; - $cachePath = 'image/cache/' . self::FILTER . '/' . $sourcePath; - $expectedPath = 'image/cache/' . self::FILTER . '/' . $versionedPath; + $cachePath = 'image/cache/'.self::FILTER.'/'.$sourcePath; + $expectedPath = 'image/cache/'.self::FILTER.'/'.$versionedPath; $this->manager ->expects($this->once()) From 44fefafc5cbb66c8fd8bfa7da396837a6fe30c83 Mon Sep 17 00:00:00 2001 From: Wouter Brinkman Date: Mon, 4 Sep 2023 11:18:46 +0200 Subject: [PATCH 6/9] Speed up searching the array using array_flip & array_key_exists, clean up if/elseif --- Templating/LazyFilterRuntime.php | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/Templating/LazyFilterRuntime.php b/Templating/LazyFilterRuntime.php index b3655abe8..fdec903d3 100644 --- a/Templating/LazyFilterRuntime.php +++ b/Templating/LazyFilterRuntime.php @@ -34,11 +34,17 @@ final class LazyFilterRuntime implements RuntimeExtensionInterface */ private $jsonManifest; + /** + * @var array|null + */ + private $jsonManifestLookup; + public function __construct(CacheManager $cache, string $assetVersion = null, array $jsonManifest = null) { $this->cache = $cache; $this->assetVersion = $assetVersion; $this->jsonManifest = $jsonManifest; + $this->jsonManifestLookup = $jsonManifest ? array_flip($jsonManifest) : null; } /** @@ -79,10 +85,11 @@ private function cleanPath(string $path): string if (mb_strlen($path) - mb_strlen($this->assetVersion) === $start) { return rtrim(mb_substr($path, 0, $start), '?'); } - } elseif ($this->jsonManifest) { - $asset = array_search($path, $this->jsonManifest, true); - if ($asset) { - return $asset; + } + + if ($this->jsonManifest) { + if (\array_key_exists($path, $this->jsonManifestLookup)) { + return $this->jsonManifestLookup[$path]; } } @@ -99,11 +106,10 @@ private function appendAssetVersion(string $resolvedPath, string $path): string $separator = false !== mb_strpos($resolvedPath, '?') ? '&' : '?'; return $resolvedPath.$separator.$this->assetVersion; - } elseif ($this->jsonManifest) { - $manifestVersion = \array_key_exists($path, $this->jsonManifest) ? $this->jsonManifest[$path] : null; - if ($manifestVersion) { - $resolvedPath = str_replace($path, $manifestVersion, $resolvedPath); - } + } + + if (\array_key_exists($path, $this->jsonManifest)) { + $resolvedPath = str_replace($path, $this->jsonManifest[$path], $resolvedPath); } return $resolvedPath; From c9951ad697c7dc3c40d31c65469b79a375655fc7 Mon Sep 17 00:00:00 2001 From: Wouter Brinkman Date: Mon, 4 Sep 2023 11:51:36 +0200 Subject: [PATCH 7/9] Revert checking for the Asset component --- DependencyInjection/Compiler/AssetsVersionCompilerPass.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DependencyInjection/Compiler/AssetsVersionCompilerPass.php b/DependencyInjection/Compiler/AssetsVersionCompilerPass.php index e6b9286a2..afd34dc35 100644 --- a/DependencyInjection/Compiler/AssetsVersionCompilerPass.php +++ b/DependencyInjection/Compiler/AssetsVersionCompilerPass.php @@ -34,7 +34,7 @@ class AssetsVersionCompilerPass extends AbstractCompilerPass { public function process(ContainerBuilder $container): void { - if (!class_exists(StaticVersionStrategy::class) || !class_exists(JsonManifestVersionStrategy::class) + if (!class_exists(StaticVersionStrategy::class) // this application has no asset version configured || !$container->has('assets._version__default') // we are not using the new LazyFilterRuntime From 62dbb65e6457ff1c9ef763ba0005d1e57d34f9f9 Mon Sep 17 00:00:00 2001 From: Wouter Brinkman Date: Wed, 6 Sep 2023 14:49:46 +0200 Subject: [PATCH 8/9] Handle cases where the key might not have a prefixed slash --- Templating/LazyFilterRuntime.php | 5 +++- Tests/Templating/LazyFilterRuntimeTest.php | 29 ++++++++++++++++------ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/Templating/LazyFilterRuntime.php b/Templating/LazyFilterRuntime.php index fdec903d3..cc41e4144 100644 --- a/Templating/LazyFilterRuntime.php +++ b/Templating/LazyFilterRuntime.php @@ -109,7 +109,10 @@ private function appendAssetVersion(string $resolvedPath, string $path): string } if (\array_key_exists($path, $this->jsonManifest)) { - $resolvedPath = str_replace($path, $this->jsonManifest[$path], $resolvedPath); + $prefixedSlash = '/' !== mb_substr($path, 0, 1) && '/' === mb_substr($this->jsonManifest[$path], 0, 1); + $versionedPath = $prefixedSlash ? mb_substr($this->jsonManifest[$path], 1) : $this->jsonManifest[$path]; + + $resolvedPath = str_replace($path, $versionedPath, $resolvedPath); } return $resolvedPath; diff --git a/Tests/Templating/LazyFilterRuntimeTest.php b/Tests/Templating/LazyFilterRuntimeTest.php index 97474ca43..6e55b9ef5 100644 --- a/Tests/Templating/LazyFilterRuntimeTest.php +++ b/Tests/Templating/LazyFilterRuntimeTest.php @@ -23,7 +23,12 @@ class LazyFilterRuntimeTest extends AbstractTest { private const FILTER = 'thumbnail'; private const VERSION = 'v2'; - private const JSON_MANIFEST = ['image/cats.png' => '/image/cats.png?v2']; + private const JSON_MANIFEST = [ + 'image/cats.png' => '/image/cats.png?v=bc321bd12a', + 'image/dogs.png' => '/image/dogs.ac38d2a1bc.png', + '/image/cows.png' => '/image/cows.png?v=a5de32a2c4', + '/image/sheep.png' => '/image/sheep.7ca26b36af.png', + ]; /** * @var LazyFilterRuntime @@ -102,15 +107,15 @@ public function testDifferentVersion(): void $this->assertSame($cachePath.'?'.self::VERSION, $actualPath); } - public function testJsonManifestVersionHandling(): void + /** + * @dataProvider provideJsonManifest + */ + public function testJsonManifestVersionHandling(string $sourcePath, string $versionedPath): void { $this->runtime = new LazyFilterRuntime($this->manager, null, self::JSON_MANIFEST); - $sourcePath = array_keys(self::JSON_MANIFEST)[0]; - $versionedPath = array_values(self::JSON_MANIFEST)[0]; - - $cachePath = 'image/cache/'.self::FILTER.'/'.$sourcePath; - $expectedPath = 'image/cache/'.self::FILTER.'/'.$versionedPath; + $cachePath = 'image/cache/'.self::FILTER.'/'.('/' === (mb_substr($sourcePath, 0, 1)) ? mb_substr($sourcePath, 1) : $sourcePath); + $expectedPath = 'image/cache/'.self::FILTER.'/'.('/' === (mb_substr($versionedPath, 0, 1)) ? mb_substr($versionedPath, 1) : $versionedPath); $this->manager ->expects($this->once()) @@ -123,6 +128,16 @@ public function testJsonManifestVersionHandling(): void $this->assertSame($expectedPath, $actualPath); } + public function provideJsonManifest(): array + { + return [ + 'query parameter, no slash' => [array_keys(self::JSON_MANIFEST)[0], array_values(self::JSON_MANIFEST)[0]], + 'in filename, no slash' => [array_keys(self::JSON_MANIFEST)[1], array_values(self::JSON_MANIFEST)[1]], + 'query parameter, slash' => [array_keys(self::JSON_MANIFEST)[2], array_values(self::JSON_MANIFEST)[2]], + 'in filename, slash' => [array_keys(self::JSON_MANIFEST)[3], array_values(self::JSON_MANIFEST)[3]], + ]; + } + public function testInvokeFilterCacheMethod(): void { $expectedInputPath = 'thePathToTheImage'; From 909c339cb3bcf20601cb398309bf0f9602cb95a0 Mon Sep 17 00:00:00 2001 From: Wouter Brinkman Date: Fri, 15 Sep 2023 08:50:59 +0200 Subject: [PATCH 9/9] Update documentation --- Resources/doc/asset-versioning.rst | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Resources/doc/asset-versioning.rst b/Resources/doc/asset-versioning.rst index 997ce44fc..b73315998 100644 --- a/Resources/doc/asset-versioning.rst +++ b/Resources/doc/asset-versioning.rst @@ -17,6 +17,11 @@ setting for ``framework.assets.version``. It strips the version from the file name and appends it to the resulting image URL so that the file is found and cache busting is used. +Since LiipImagineBundle version 2.12, we integrate with the configuration +setting for ``framework.assets.json_manifest_path``. The manifest file is used +to lookup the location of the actual file, and append the versioning string to +the resulting image URL so that cache busting is used. + Cache Busting ~~~~~~~~~~~~~ @@ -25,6 +30,12 @@ versioning to bust the cache of your images. This can help for example after you changed the settings of a filter set. If you use ``framework.assets.version``, change the asset version in that case. +If you use ``framework.assets.json_manifest_path``, then rebuild the manifest +in your asset pipeline. Note that your versioning string might be calculated +using a content hash. Changing a filter setting in these cases will *not* bust +the previous cache. Either rename your filter in that case or use a different +versioning strategy within your asset pipeline that ensures a new hash for each +build. If you do not use Symfony asset versioning, set the ``liip_imagine.twig.assets_version`` parameter. Note that you still need to clear/refresh the cached images to have them updated, the asset version is only