From 9729b39429a2e49f4b1f9eb68752e76fadc20b22 Mon Sep 17 00:00:00 2001 From: Wouter Brinkman Date: Mon, 28 Aug 2023 10:27:30 +0200 Subject: [PATCH 01/12] 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 02/12] 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 03/12] 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 04/12] 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 05/12] 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 06/12] 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 07/12] 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 08/12] 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 0edec019f322ea33c479df184e198b4ebafd830a Mon Sep 17 00:00:00 2001 From: Wouter Brinkman Date: Tue, 11 Jun 2024 12:59:59 +0200 Subject: [PATCH 09/12] Rewrite the version string to our resolved path. Handles file extension changes --- Templating/LazyFilterRuntime.php | 52 +++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/Templating/LazyFilterRuntime.php b/Templating/LazyFilterRuntime.php index cc41e4144..eb4e40357 100644 --- a/Templating/LazyFilterRuntime.php +++ b/Templating/LazyFilterRuntime.php @@ -108,13 +108,63 @@ private function appendAssetVersion(string $resolvedPath, string $path): string return $resolvedPath.$separator.$this->assetVersion; } + + if (\array_key_exists($path, $this->jsonManifest)) { + $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); + $originalExt = pathinfo($path, PATHINFO_EXTENSION); + $resolvedExt = pathinfo($resolvedPath, PATHINFO_EXTENSION); + + if ($originalExt !== $resolvedExt) { + $path = str_replace('.'.$originalExt, '.'.$resolvedExt, $path); + $versionedPath = str_replace('.'.$originalExt, '.'.$resolvedExt, $versionedPath); + } + + $versioning = $this->captureVersion(pathinfo($path, PATHINFO_BASENAME), pathinfo($versionedPath, PATHINFO_BASENAME)); + $resolvedFilename = pathinfo($resolvedPath, PATHINFO_BASENAME); + $resolvedDir = pathinfo($resolvedPath, PATHINFO_DIRNAME); + $resolvedPath = $resolvedDir.'/'.$this->insertVersion($resolvedFilename, $versioning['version'], $versioning['position']); } return $resolvedPath; } + + /** + * Capture the versioning string from the versioned filename + */ + private function captureVersion(string $originalFilename, string $versionedFilename): array + { + $originalLength = strlen($originalFilename); + $versionedLength = strlen($versionedFilename); + + for ($i = 0; $i < $originalLength && $i < $versionedLength; $i++) { + if ($originalFilename[$i] !== $versionedFilename[$i]) { + break; + } + } + + $version = substr($versionedFilename, $i, $versionedLength - $originalLength); + + return ['version' => $version, 'position' => $i]; + } + + /** + * Insert the version string into our resolved filename + */ + private function insertVersion(string $resolvedFilename, string $version, int $position): string + { + if ($position < 0 || $position > strlen($resolvedFilename)) { + return $resolvedFilename; + } + + $firstPart = substr($resolvedFilename, 0, $position); + $secondPart = substr($resolvedFilename, $position); + + $versionedFilename = $firstPart . $version . $secondPart; + + return $versionedFilename; + } } From 411cfaa1dfeb886fd933541838e3643bdb41cdd7 Mon Sep 17 00:00:00 2001 From: Wouter Brinkman Date: Tue, 11 Jun 2024 13:25:14 +0200 Subject: [PATCH 10/12] Add test for extension change --- Tests/Templating/LazyFilterRuntimeTest.php | 33 ++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/Tests/Templating/LazyFilterRuntimeTest.php b/Tests/Templating/LazyFilterRuntimeTest.php index 6e55b9ef5..a89cc03b4 100644 --- a/Tests/Templating/LazyFilterRuntimeTest.php +++ b/Tests/Templating/LazyFilterRuntimeTest.php @@ -128,6 +128,29 @@ public function testJsonManifestVersionHandling(string $sourcePath, string $vers $this->assertSame($expectedPath, $actualPath); } + /** + * @dataProvider provideJsonManifestSwapExt + */ + public function testJsonManifestVersionHandlingWithExtensionSwapping(string $sourcePath, string $versionedPath, $originalExt, $newExt): void + { + $this->runtime = new LazyFilterRuntime($this->manager, null, self::JSON_MANIFEST); + + $cachePath = 'image/cache/'.self::FILTER.'/'.('/' === (mb_substr($sourcePath, 0, 1)) ? mb_substr($sourcePath, 1) : $sourcePath); + $cachePath = str_replace('.'.$originalExt, '.'.$newExt, $cachePath); + $expectedPath = 'image/cache/'.self::FILTER.'/'.('/' === (mb_substr($versionedPath, 0, 1)) ? mb_substr($versionedPath, 1) : $versionedPath); + $expectedPath = str_replace('.'.$originalExt, '.'.$newExt, $expectedPath); + + $this->manager + ->expects($this->once()) + ->method('getBrowserPath') + ->with($sourcePath, self::FILTER) + ->willReturn($cachePath); + + $actualPath = $this->runtime->filter($versionedPath, self::FILTER); + + $this->assertSame($expectedPath, $actualPath); + } + public function provideJsonManifest(): array { return [ @@ -138,6 +161,16 @@ public function provideJsonManifest(): array ]; } + public function provideJsonManifestSwapExt(): array + { + return [ + 'query parameter, no slash' => [array_keys(self::JSON_MANIFEST)[0], array_values(self::JSON_MANIFEST)[0], 'png', 'webp'], + 'in filename, no slash' => [array_keys(self::JSON_MANIFEST)[1], array_values(self::JSON_MANIFEST)[1], 'png', 'webp'], + 'query parameter, slash' => [array_keys(self::JSON_MANIFEST)[2], array_values(self::JSON_MANIFEST)[2], 'png', 'webp'], + 'in filename, slash' => [array_keys(self::JSON_MANIFEST)[3], array_values(self::JSON_MANIFEST)[3], 'png', 'webp'], + ]; + } + public function testInvokeFilterCacheMethod(): void { $expectedInputPath = 'thePathToTheImage'; From 945b29b8397d0db32a4bc2164730cf63a4319179 Mon Sep 17 00:00:00 2001 From: Wouter Brinkman Date: Tue, 11 Jun 2024 13:29:57 +0200 Subject: [PATCH 11/12] Update documentation to the next version --- Resources/doc/asset-versioning.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Resources/doc/asset-versioning.rst b/Resources/doc/asset-versioning.rst index b73315998..18dbc4033 100644 --- a/Resources/doc/asset-versioning.rst +++ b/Resources/doc/asset-versioning.rst @@ -17,7 +17,7 @@ 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 +Since LiipImagineBundle version 2.13, 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. From b5b5569ce7a37c89d6897064bdc205721038cf82 Mon Sep 17 00:00:00 2001 From: Wouter Brinkman Date: Tue, 11 Jun 2024 13:35:57 +0200 Subject: [PATCH 12/12] Update changelog with versioning --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3eea964d..634e014fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ This file contains a complete enumeration of all [pull requests](https://github. for a given releases. Unreleased, upcoming changes will be updated here periodically; reference the next release on our [milestones](https://github.com/liip/LiipImagineBundle/milestones) page for the latest changes. -## [2.12.0](https://github.com/liip/LiipImagineBundle/tree/2.12.0) +## [2.13.0](https://github.com/liip/LiipImagineBundle/tree/2.13.0) - Support JsonManifestVersionStrategy that was added in Symfony 6.