feature symfony#57879 [AssetMapper] Truncate public digests to 8 characters (smnandre)

fabpot · fabpot · commit 0862610acc65 · 2024-08-04T16:36:12.000+02:00
This PR was squashed before being merged into the 7.2 branch. Discussion ---------- [AssetMapper] Truncate public digests to 8 characters | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | Fix #... | License | MIT ⚠️ 2024-08-03 Update: **Changes Made Based on Feedback** This pull request (PR) now shortens the public digest of mapped assets to **8 characters**. The digest is used in the filenames of deployed assets, leading to a tangible reduction in HTML size, particularly in the generated import map. -- **Original message:** I've been wanting to use shorter digests in my mapped assets / importmap URL's. This PR adds an hashAlgorithm argument to the MappedAssetFactory to do so. I'm not sure this is enough/good choice, as we need some requirement regarding the digests (7+ caracters, alnum only) But adding an entire AssetHasherInterface would be probably overkill, so i'm _**very open**_ for feedback / ideas there. (I did not put FrameworkBundle modifications in this PR, as the discussion will probably have an impact on what's to do) Commits ------- e1ae985 [AssetMapper] Truncate public digests to 8 characters
diff --git a/src/Symfony/Component/AssetMapper/CHANGELOG.md b/src/Symfony/Component/AssetMapper/CHANGELOG.md
@@ -1,6 +1,11 @@
 CHANGELOG
 =========
 
+7.2
+---
+
+ * Shorten the public digest of mapped assets to 7 characters
+
 7.1
 ---
 
diff --git a/src/Symfony/Component/AssetMapper/Factory/MappedAssetFactory.php b/src/Symfony/Component/AssetMapper/Factory/MappedAssetFactory.php
@@ -24,6 +24,7 @@
 class MappedAssetFactory implements MappedAssetFactoryInterface
 {
     private const PREDIGESTED_REGEX = '/-([0-9a-zA-Z]{7,128}\.digested)/';
+    private const PUBLIC_DIGEST_LENGTH = 7;
 
     private array $assetsCache = [];
     private array $assetsBeingCreated = [];
@@ -89,10 +90,7 @@ private function getDigest(MappedAsset $asset, ?string $content): array
             return [hash('xxh128', $content), false];
         }
 
-        return [
-            hash_file('xxh128', $asset->sourcePath),
-            false,
-        ];
+        return [hash_file('xxh128', $asset->sourcePath), false];
     }
 
     private function compileContent(MappedAsset $asset): ?string
@@ -119,6 +117,7 @@ private function getPublicPath(MappedAsset $asset, ?string $content): ?string
             return $this->assetsPathResolver->resolvePublicPath($asset->logicalPath);
         }
 
+        $digest = substr(base64_encode($digest), 0, self::PUBLIC_DIGEST_LENGTH);
         $digestedPath = preg_replace_callback('/\.(\w+)$/', fn ($matches) => "-{$digest}{$matches[0]}", $asset->logicalPath);
 
         return $this->assetsPathResolver->resolvePublicPath($digestedPath);
diff --git a/src/Symfony/Component/AssetMapper/Tests/AssetMapperDevServerSubscriberFunctionalTest.php b/src/Symfony/Component/AssetMapper/Tests/AssetMapperDevServerSubscriberFunctionalTest.php
@@ -21,7 +21,7 @@ public function testGettingAssetWorks()
     {
         $client = static::createClient();
 
-        $client->request('GET', '/assets/file1-b3445cb7a86a0795a7af7f2004498aef.css');
+        $client->request('GET', '/assets/file1-YjM0NDV.css');
         $response = $client->getResponse();
         $this->assertSame(200, $response->getStatusCode());
         $this->assertInstanceOf(BinaryFileResponse::class, $response);
@@ -39,7 +39,7 @@ public function testGettingAssetWithNonAsciiFilenameWorks()
     {
         $client = static::createClient();
 
-        $client->request('GET', '/assets/voilà-6344422da690fcc471f23f7a8966cd1c.css');
+        $client->request('GET', '/assets/voilà-NjM0NDQ.css');
         $response = $client->getResponse();
         $this->assertSame(200, $response->getStatusCode());
         $this->assertSame(<<<EOF
diff --git a/src/Symfony/Component/AssetMapper/Tests/Command/AssetMapperCompileCommandTest.php b/src/Symfony/Component/AssetMapper/Tests/Command/AssetMapperCompileCommandTest.php
@@ -59,12 +59,12 @@ public function testAssetsAreCompiled()
         // match Compiling \d+ assets
         $this->assertMatchesRegularExpression('/Compiled \d+ assets/', $tester->getDisplay());
 
-        $this->assertFileExists($targetBuildDir.'/subdir/file5-f4fdc37375c7f5f2629c5659a0579967.js');
+        $this->assertFileExists($targetBuildDir.'/subdir/file5-ZjRmZGM.js');
         $this->assertSame(<<<EOF
         import '../file4.js';
         console.log('file5.js');
 
-        EOF, $this->filesystem->readFile($targetBuildDir.'/subdir/file5-f4fdc37375c7f5f2629c5659a0579967.js'));
+        EOF, $this->filesystem->readFile($targetBuildDir.'/subdir/file5-ZjRmZGM.js'));
 
         $finder = new Finder();
         $finder->in($targetBuildDir)->files();
diff --git a/src/Symfony/Component/AssetMapper/Tests/Factory/MappedAssetFactoryTest.php b/src/Symfony/Component/AssetMapper/Tests/Factory/MappedAssetFactoryTest.php
@@ -19,6 +19,7 @@
 use Symfony\Component\AssetMapper\Compiler\CssAssetUrlCompiler;
 use Symfony\Component\AssetMapper\Compiler\JavaScriptImportPathCompiler;
 use Symfony\Component\AssetMapper\Exception\CircularAssetsException;
+use Symfony\Component\AssetMapper\Exception\InvalidArgumentException;
 use Symfony\Component\AssetMapper\Factory\MappedAssetFactory;
 use Symfony\Component\AssetMapper\ImportMap\ImportMapConfigReader;
 use Symfony\Component\AssetMapper\MappedAsset;
diff --git a/src/Symfony/Component/AssetMapper/Tests/MapperAwareAssetPackageIntegrationTest.php b/src/Symfony/Component/AssetMapper/Tests/MapperAwareAssetPackageIntegrationTest.php
@@ -37,7 +37,7 @@ public function testDefaultAssetPackageIsDecorated()
     {
         $packages = $this->kernel->getContainer()->get('public.assets.packages');
         \assert($packages instanceof Packages);
-        $this->assertSame('/assets/file1-b3445cb7a86a0795a7af7f2004498aef.css', $packages->getUrl('file1.css'));
+        $this->assertSame('/assets/file1-YjM0NDV.css', $packages->getUrl('file1.css'));
         $this->assertSame('/non-existent.css', $packages->getUrl('non-existent.css'));
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -24,6 +24,7 @@`
`24`	`24`	`class MappedAssetFactory implements MappedAssetFactoryInterface`
`25`	`25`	`{`
`26`	`26`	`private const PREDIGESTED_REGEX = '/-([0-9a-zA-Z]{7,128}\.digested)/';`
	`27`	`+ private const PUBLIC_DIGEST_LENGTH = 7;`
`27`	`28`
`28`	`29`	`private array $assetsCache = [];`
`29`	`30`	`private array $assetsBeingCreated = [];`
`@@ -89,10 +90,7 @@ private function getDigest(MappedAsset $asset, ?string $content): array`
`89`	`90`	`return [hash('xxh128', $content), false];`
`90`	`91`	`}`
`91`	`92`
`92`		`- return [`
`93`		`- hash_file('xxh128', $asset->sourcePath),`
`94`		`- false,`
`95`		`- ];`
	`93`	`+ return [hash_file('xxh128', $asset->sourcePath), false];`
`96`	`94`	`}`
`97`	`95`
`98`	`96`	`private function compileContent(MappedAsset $asset): ?string`
`@@ -119,6 +117,7 @@ private function getPublicPath(MappedAsset $asset, ?string $content): ?string`
`119`	`117`	`return $this->assetsPathResolver->resolvePublicPath($asset->logicalPath);`
`120`	`118`	`}`
`121`	`119`
	`120`	`+ $digest = substr(base64_encode($digest), 0, self::PUBLIC_DIGEST_LENGTH);`
`122`	`121`	`$digestedPath = preg_replace_callback('/\.(\w+)$/', fn ($matches) => "-{$digest}{$matches[0]}", $asset->logicalPath);`
`123`	`122`
`124`	`123`	`return $this->assetsPathResolver->resolvePublicPath($digestedPath);`
Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,7 @@ public function testDefaultAssetPackageIsDecorated()`
`37`	`37`	`{`
`38`	`38`	`$packages = $this->kernel->getContainer()->get('public.assets.packages');`
`39`	`39`	`\assert($packages instanceof Packages);`
`40`		`- $this->assertSame('/assets/file1-b3445cb7a86a0795a7af7f2004498aef.css', $packages->getUrl('file1.css'));`
	`40`	`+ $this->assertSame('/assets/file1-YjM0NDV.css', $packages->getUrl('file1.css'));`
`41`	`41`	`$this->assertSame('/non-existent.css', $packages->getUrl('non-existent.css'));`
`42`	`42`	`}`
`43`	`43`	`}`