Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/2.x' into 2-to-3
Browse files Browse the repository at this point in the history
  • Loading branch information
dbu committed Jun 19, 2024
2 parents 68bcdb4 + 0c95437 commit 123aae8
Show file tree
Hide file tree
Showing 7 changed files with 207 additions and 19 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ for a given releases. Unreleased, upcoming changes will be updated here periodic

# 2.x

## [2.13.0](https://github.com/liip/LiipImagineBundle/tree/2.13.0)

- Support JsonManifestVersionStrategy that was added in Symfony 6.

## [2.12.3](https://github.com/liip/LiipImagineBundle/tree/2.12.3)

- Add alias for `Imagine\Image\ImagineInterface` to help autowiring.
Expand Down
3 changes: 3 additions & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
"phpstan/phpstan-symfony": "^1.0",
"psr/cache": "^3.0",
"psr/log": "^1.0|^2.0|^3.0",
"symfony/asset": "^6.4|^7.0",
"symfony/browser-kit": "^6.4|^7.0",
"symfony/cache": "^6.4|^7.0",
"symfony/console": "^6.4|^7.0",
Expand All @@ -54,6 +55,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",
Expand All @@ -64,6 +66,7 @@
"league/flysystem": "required to use FlySystem data loader or cache resolver",
"monolog/monolog": "A psr/log compatible logger is required to enable logging",
"rokka/imagine-vips": "required to use 'vips' driver",
"symfony/asset": "If you want to use asset versioning",
"symfony/messenger": "If you like to process images in background"
},
"config": {
Expand Down
11 changes: 11 additions & 0 deletions doc/asset-versioning.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.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.

Cache Busting
~~~~~~~~~~~~~

Expand All @@ -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
Expand Down
33 changes: 28 additions & 5 deletions src/DependencyInjection/Compiler/AssetsVersionCompilerPass.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,24 @@

namespace Liip\ImagineBundle\DependencyInjection\Compiler;

use Symfony\Component\Asset\VersionStrategy\JsonManifestVersionStrategy;
use Symfony\Component\Asset\VersionStrategy\StaticVersionStrategy;
use Symfony\Component\DependencyInjection\ContainerBuilder;

/**
* 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
{
Expand All @@ -46,7 +49,9 @@ 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;
Expand All @@ -61,5 +66,23 @@ public function process(ContainerBuilder $container): void
}

$runtimeDefinition->setArgument(1, $version);

if (is_a($versionStrategyDefinition->getClass(), JsonManifestVersionStrategy::class, true)) {
$jsonManifestString = file_get_contents($version);

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($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;
}
$runtimeDefinition->setArgument(1, null);
$runtimeDefinition->setArgument(2, $jsonManifest);
}
}
}
1 change: 1 addition & 0 deletions src/Resources/config/imagine_twig_mode_lazy.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
<tag name="twig.runtime" />
<argument type="service" id="liip_imagine.cache.manager" />
<argument>null</argument>
<argument>null</argument>
</service>

</services>
Expand Down
104 changes: 90 additions & 14 deletions src/Templating/LazyFilterRuntime.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,22 @@ final class LazyFilterRuntime implements RuntimeExtensionInterface
*/
private ?string $assetVersion;

public function __construct(CacheManager $cache, ?string $assetVersion = null)
/**
* @var array|null
*/
private $jsonManifest;

Check failure on line 30 in src/Templating/LazyFilterRuntime.php

View workflow job for this annotation

GitHub Actions / phpstan

Property Liip\ImagineBundle\Templating\LazyFilterRuntime::$jsonManifest type has no value type specified in iterable type array.

Check failure on line 30 in src/Templating/LazyFilterRuntime.php

View workflow job for this annotation

GitHub Actions / phpstan

Property Liip\ImagineBundle\Templating\LazyFilterRuntime::$jsonManifest type has no value type specified in iterable type array.

/**
* @var array|null
*/
private $jsonManifestLookup;

Check failure on line 35 in src/Templating/LazyFilterRuntime.php

View workflow job for this annotation

GitHub Actions / phpstan

Property Liip\ImagineBundle\Templating\LazyFilterRuntime::$jsonManifestLookup type has no value type specified in iterable type array.

Check failure on line 35 in src/Templating/LazyFilterRuntime.php

View workflow job for this annotation

GitHub Actions / phpstan

Property Liip\ImagineBundle\Templating\LazyFilterRuntime::$jsonManifestLookup type has no value type specified in iterable type array.

public function __construct(CacheManager $cache, ?string $assetVersion = null, ?array $jsonManifest = null)

Check failure on line 37 in src/Templating/LazyFilterRuntime.php

View workflow job for this annotation

GitHub Actions / phpstan

Method Liip\ImagineBundle\Templating\LazyFilterRuntime::__construct() has parameter $jsonManifest with no value type specified in iterable type array.

Check failure on line 37 in src/Templating/LazyFilterRuntime.php

View workflow job for this annotation

GitHub Actions / phpstan

Method Liip\ImagineBundle\Templating\LazyFilterRuntime::__construct() has parameter $jsonManifest with no value type specified in iterable type array.
{
$this->cache = $cache;
$this->assetVersion = $assetVersion;
$this->jsonManifest = $jsonManifest;
$this->jsonManifestLookup = $jsonManifest ? array_flip($jsonManifest) : null;
}

/**
Expand All @@ -36,9 +48,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);
}

/**
Expand All @@ -52,33 +64,97 @@ public function filterCache(string $path, string $filter, array $config = [], ?s
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), '?');
}
}

if ($this->jsonManifest) {
if (\array_key_exists($path, $this->jsonManifestLookup)) {
return $this->jsonManifestLookup[$path];
}
}

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;
}

if ($this->assetVersion) {
$separator = false !== mb_strpos($resolvedPath, '?') ? '&' : '?';

return $resolvedPath.$separator.$this->assetVersion;
}

if (\array_key_exists($path, $this->jsonManifest)) {
$prefixedSlash = 0 !== mb_strpos($path, '/') && 0 === mb_strpos($this->jsonManifest[$path], '/');
$versionedPath = $prefixedSlash ? mb_substr($this->jsonManifest[$path], 1) : $this->jsonManifest[$path];

$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

Check failure on line 130 in src/Templating/LazyFilterRuntime.php

View workflow job for this annotation

GitHub Actions / phpstan

Method Liip\ImagineBundle\Templating\LazyFilterRuntime::captureVersion() return type has no value type specified in iterable type array.

Check failure on line 130 in src/Templating/LazyFilterRuntime.php

View workflow job for this annotation

GitHub Actions / phpstan

Method Liip\ImagineBundle\Templating\LazyFilterRuntime::captureVersion() return type has no value type specified in iterable type array.
{
$originalLength = mb_strlen($originalFilename);
$versionedLength = mb_strlen($versionedFilename);

for ($i = 0; $i < $originalLength && $i < $versionedLength; ++$i) {
if ($originalFilename[$i] !== $versionedFilename[$i]) {
break;
}
}

$version = mb_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 > mb_strlen($resolvedFilename)) {
return $resolvedFilename;
}

$separator = false !== mb_strpos($path, '?') ? '&' : '?';
$firstPart = mb_substr($resolvedFilename, 0, $position);
$secondPart = mb_substr($resolvedFilename, $position);

return $path.$separator.$this->assetVersion;
return $firstPart.$version.$secondPart;
}
}
70 changes: 70 additions & 0 deletions tests/Templating/LazyFilterRuntimeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ class LazyFilterRuntimeTest extends AbstractTest
{
private const FILTER = 'thumbnail';
private const VERSION = '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',
];

private LazyFilterRuntime $runtime;
/**
Expand Down Expand Up @@ -101,6 +107,70 @@ public function testDifferentVersion(): void
$this->assertSame($cachePath.'?'.self::VERSION, $actualPath);
}

/**
* @dataProvider provideJsonManifest
*/
public function testJsonManifestVersionHandling(string $sourcePath, string $versionedPath): 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);
$expectedPath = 'image/cache/'.self::FILTER.'/'.('/' === (mb_substr($versionedPath, 0, 1)) ? mb_substr($versionedPath, 1) : $versionedPath);

$this->manager
->expects($this->once())
->method('getBrowserPath')
->with($sourcePath, self::FILTER)
->willReturn($cachePath);

$actualPath = $this->runtime->filter($versionedPath, self::FILTER);

$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 [
'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 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';
Expand Down

0 comments on commit 123aae8

Please sign in to comment.