Skip to content

Commit 1731694

Browse files
fabpotfabpot
committed
Merge branch '6.4' into 7.0
* 6.4: [Form] Fix merging form data and files (ter) [Intl] Update the ICU data to 74.1 [Scheduler] Use MockClock [AssetMapper] Fixing memory bug where we stored way more file content than needed [AssetMapper] jsdelivr "no version" import syntax [HtmlSanitizer] Consider `width` attribute as safe [DoctrineBridge] Fix exception message [Security][Validator] Missing translations for Luxembourgish
2 parents abd5914 + 0398887 commit 1731694

11 files changed

+64
-62
lines changed

‎Command/AssetMapperCompileCommand.php‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,9 @@ private function shortenPath(string $path): string
104104

105105
private function createManifestAndWriteFiles(SymfonyStyle $io): array
106106
{
107-
$allAssets = $this->assetMapper->allAssets();
108-
109107
$io->comment(sprintf('Compiling and writing asset files to <info>%s</info>', $this->shortenPath($this->assetsFilesystem->getDestinationPath())));
110108
$manifest = [];
111-
foreach ($allAssets as $asset) {
109+
foreach ($this->assetMapper->allAssets() as $asset) {
112110
if (null !== $asset->content) {
113111
// The original content has been modified by the AssetMapperCompiler
114112
$this->assetsFilesystem->write($asset->publicPath, $asset->content);

‎Command/VersionProblemCommandTrait.php‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,6 @@ private function renderVersionProblems(ImportMapVersionChecker $importMapVersion
2929
continue;
3030
}
3131

32-
if (null === $problem->requiredVersionConstraint) {
33-
$output->writeln(sprintf('[warning] <info>%s</info> appears to import <info>%s</info> but this is not listed as a dependency of <info>%s</info>. This is odd and could be a misconfiguration of that package.', $problem->packageName, $problem->dependencyPackageName, $problem->packageName));
34-
35-
continue;
36-
}
37-
3832
$output->writeln(sprintf('[warning] <info>%s</info> requires <info>%s</info>@<comment>%s</comment> but version <comment>%s</comment> is installed.', $problem->packageName, $problem->dependencyPackageName, $problem->requiredVersionConstraint, $problem->installedVersion));
3933
}
4034
}

‎Factory/MappedAssetFactory.php‎

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ class MappedAssetFactory implements MappedAssetFactoryInterface
2626

2727
private array $assetsCache = [];
2828
private array $assetsBeingCreated = [];
29-
private array $fileContentsCache = [];
3029

3130
public function __construct(
3231
private readonly PublicAssetsPathResolverInterface $assetsPathResolver,
@@ -46,14 +45,15 @@ public function createMappedAsset(string $logicalPath, string $sourcePath): ?Map
4645
$isVendor = $this->isVendor($sourcePath);
4746
$asset = new MappedAsset($logicalPath, $sourcePath, $this->assetsPathResolver->resolvePublicPath($logicalPath), isVendor: $isVendor);
4847

49-
[$digest, $isPredigested] = $this->getDigest($asset);
48+
$content = $this->compileContent($asset);
49+
[$digest, $isPredigested] = $this->getDigest($asset, $content);
5050

5151
$asset = new MappedAsset(
5252
$asset->logicalPath,
5353
$asset->sourcePath,
5454
$asset->publicPathWithoutDigest,
55-
$this->getPublicPath($asset),
56-
$this->compileContent($asset),
55+
$this->getPublicPath($asset, $content),
56+
$content,
5757
$digest,
5858
$isPredigested,
5959
$isVendor,
@@ -75,15 +75,15 @@ public function createMappedAsset(string $logicalPath, string $sourcePath): ?Map
7575
*
7676
* @return array{0: string, 1: bool}
7777
*/
78-
private function getDigest(MappedAsset $asset): array
78+
private function getDigest(MappedAsset $asset, ?string $content): array
7979
{
8080
// check for a pre-digested file
8181
if (preg_match(self::PREDIGESTED_REGEX, $asset->logicalPath, $matches)) {
8282
return [$matches[1], true];
8383
}
8484

8585
// Use the compiled content if any
86-
if (null !== $content = $this->compileContent($asset)) {
86+
if (null !== $content) {
8787
return [hash('xxh128', $content), false];
8888
}
8989

@@ -95,27 +95,23 @@ private function getDigest(MappedAsset $asset): array
9595

9696
private function compileContent(MappedAsset $asset): ?string
9797
{
98-
if (\array_key_exists($asset->logicalPath, $this->fileContentsCache)) {
99-
return $this->fileContentsCache[$asset->logicalPath];
100-
}
101-
10298
if (!is_file($asset->sourcePath)) {
10399
throw new RuntimeException(sprintf('Asset source path "%s" could not be found.', $asset->sourcePath));
104100
}
105101

106102
if (!$this->compiler->supports($asset)) {
107-
return $this->fileContentsCache[$asset->logicalPath] = null;
103+
return null;
108104
}
109105

110106
$content = file_get_contents($asset->sourcePath);
111-
$content = $this->compiler->compile($content, $asset);
107+
$compiled = $this->compiler->compile($content, $asset);
112108

113-
return $this->fileContentsCache[$asset->logicalPath] = $content;
109+
return $compiled !== $content ? $compiled : null;
114110
}
115111

116-
private function getPublicPath(MappedAsset $asset): ?string
112+
private function getPublicPath(MappedAsset $asset, ?string $content): ?string
117113
{
118-
[$digest, $isPredigested] = $this->getDigest($asset);
114+
[$digest, $isPredigested] = $this->getDigest($asset, $content);
119115

120116
if ($isPredigested) {
121117
return $this->assetsPathResolver->resolvePublicPath($asset->logicalPath);

‎ImportMap/ImportMapVersionChecker.php‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,14 +87,13 @@ public function checkVersions(): array
8787
}
8888

8989
$dependencyPackageName = $entries->get($dependencyName)->getPackageName();
90-
$dependencyVersionConstraint = $packageDependencies[$dependencyPackageName] ?? null;
91-
92-
if (null === $dependencyVersionConstraint) {
93-
$problems[] = new PackageVersionProblem($packageName, $dependencyPackageName, $dependencyVersionConstraint, $entries->get($dependencyName)->version);
9490

91+
if (!isset($packageDependencies[$dependencyPackageName])) {
9592
continue;
9693
}
9794

95+
$dependencyVersionConstraint = $packageDependencies[$dependencyPackageName];
96+
9897
if (!$this->isVersionSatisfied($dependencyVersionConstraint, $entries->get($dependencyName)->version)) {
9998
$problems[] = new PackageVersionProblem($packageName, $dependencyPackageName, $dependencyVersionConstraint, $entries->get($dependencyName)->version);
10099
}

‎ImportMap/PackageVersionProblem.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ final class PackageVersionProblem
1616
public function __construct(
1717
public readonly string $packageName,
1818
public readonly string $dependencyPackageName,
19-
public readonly ?string $requiredVersionConstraint,
19+
public readonly string $requiredVersionConstraint,
2020
public readonly ?string $installedVersion
2121
) {
2222
}

‎ImportMap/Resolver/JsDelivrEsmResolver.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ final class JsDelivrEsmResolver implements PackageResolverInterface
2626
public const URL_PATTERN_DIST = self::URL_PATTERN_DIST_CSS.'/+esm';
2727
public const URL_PATTERN_ENTRYPOINT = 'https://data.jsdelivr.com/v1/packages/npm/%s@%s/entrypoints';
2828

29-
public const IMPORT_REGEX = '{from"/npm/((?:@[^/]+/)?[^@]+)@([^/]+)((?:/[^/]+)*?)/\+esm"}';
29+
public const IMPORT_REGEX = '{from"/npm/((?:@[^/]+/)?[^@]+?)(?:@([^/]+))?((?:/[^/]+)*?)/\+esm"}';
3030

3131
private HttpClientInterface $httpClient;
3232

@@ -222,7 +222,7 @@ private function fetchPackageRequirementsFromImports(string $content): array
222222
preg_match_all(self::IMPORT_REGEX, $content, $matches);
223223
$dependencies = [];
224224
foreach ($matches[1] as $index => $packageName) {
225-
$version = $matches[2][$index];
225+
$version = $matches[2][$index] ?: null;
226226
$packageName .= $matches[3][$index]; // add the path if any
227227

228228
$dependencies[] = new PackageRequireOptions($packageName, $version);

‎MappedAsset.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ final class MappedAsset
2626
public readonly string $publicExtension;
2727

2828
/**
29-
* The final content of this asset, if different from the source.
29+
* The final content of this asset if different from the sourcePath.
30+
*
31+
* If null, the content should be read from the sourcePath.
3032
*/
3133
public readonly ?string $content;
3234

‎Tests/AssetMapperDevServerSubscriberFunctionalTest.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
1515
use Symfony\Component\AssetMapper\Tests\Fixtures\AssetMapperTestAppKernel;
16+
use Symfony\Component\HttpFoundation\BinaryFileResponse;
1617

1718
class AssetMapperDevServerSubscriberFunctionalTest extends WebTestCase
1819
{
@@ -23,11 +24,12 @@ public function testGettingAssetWorks()
2324
$client->request('GET', '/assets/file1-b3445cb7a86a0795a7af7f2004498aef.css');
2425
$response = $client->getResponse();
2526
$this->assertSame(200, $response->getStatusCode());
27+
$this->assertInstanceOf(BinaryFileResponse::class, $response);
2628
$this->assertSame(<<<EOF
2729
/* file1.css */
2830
body {}
2931
30-
EOF, $response->getContent());
32+
EOF, $response->getFile()->getContent());
3133
$this->assertSame('"b3445cb7a86a0795a7af7f2004498aef"', $response->headers->get('ETag'));
3234
$this->assertSame('immutable, max-age=604800, public', $response->headers->get('Cache-Control'));
3335
$this->assertTrue($response->headers->has('X-Assets-Dev'));
@@ -59,11 +61,12 @@ public function testPreDigestedAssetIsReturned()
5961
$client->request('GET', '/assets/already-abcdefVWXYZ0123456789.digested.css');
6062
$response = $client->getResponse();
6163
$this->assertSame(200, $response->getStatusCode());
64+
$this->assertInstanceOf(BinaryFileResponse::class, $response);
6265
$this->assertSame(<<<EOF
6366
/* already-abcdefVWXYZ0123456789.digested.css */
6467
body {}
6568
66-
EOF, $response->getContent());
69+
EOF, $response->getFile()->getContent());
6770
}
6871

6972
protected static function getKernelClass(): string

‎Tests/Factory/MappedAssetFactoryTest.php‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,22 @@ public function testCreateMappedAssetRespectsPreDigestedPaths()
4848
$this->assertSame('/final-assets/already-abcdefVWXYZ0123456789.digested.css', $asset->publicPathWithoutDigest);
4949
}
5050

51-
public function testCreateMappedAssetWithContentBasic()
51+
public function testCreateMappedAssetWithContentThatChanged()
5252
{
53-
$assetMapper = $this->createFactory();
54-
$expected = <<<EOF
55-
/* file1.css */
56-
body {}
53+
$file1Compiler = new class() implements AssetCompilerInterface {
54+
public function supports(MappedAsset $asset): bool
55+
{
56+
return true;
57+
}
5758

58-
EOF;
59+
public function compile(string $content, MappedAsset $asset, AssetMapperInterface $assetMapper): string
60+
{
61+
return 'totally changed';
62+
}
63+
};
64+
65+
$assetMapper = $this->createFactory($file1Compiler);
66+
$expected = 'totally changed';
5967

6068
$asset = $assetMapper->createMappedAsset('file1.css', __DIR__.'/../Fixtures/dir1/file1.css');
6169
$this->assertSame($expected, $asset->content);
@@ -65,6 +73,14 @@ public function testCreateMappedAssetWithContentBasic()
6573
$this->assertSame($expected, $asset->content);
6674
}
6775

76+
public function testCreateMappedAssetWithContentThatDoesNotChange()
77+
{
78+
$assetMapper = $this->createFactory();
79+
$asset = $assetMapper->createMappedAsset('file1.css', __DIR__.'/../Fixtures/dir1/file1.css');
80+
// null content because the final content matches the file source
81+
$this->assertNull($asset->content);
82+
}
83+
6884
public function testCreateMappedAssetWithContentErrorsOnCircularReferences()
6985
{
7086
$factory = $this->createFactory();

‎Tests/ImportMap/ImportMapVersionCheckerTest.php‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -220,28 +220,6 @@ public static function getCheckVersionsTests()
220220
],
221221
];
222222

223-
yield 'single that imports something that is not required by the package' => [
224-
[
225-
self::createRemoteEntry('foo', version: '1.0.0'),
226-
self::createRemoteEntry('bar', version: '1.5.0'),
227-
],
228-
[
229-
'foo' => ['bar'],
230-
'bar' => [],
231-
],
232-
[
233-
[
234-
'url' => '/foo/1.0.0',
235-
'response' => [
236-
'dependencies' => [],
237-
],
238-
],
239-
],
240-
[
241-
new PackageVersionProblem('foo', 'bar', null, '1.5.0'),
242-
],
243-
];
244-
245223
yield 'single with npm-style constraint' => [
246224
[
247225
self::createRemoteEntry('foo', version: '1.0.0'),

0 commit comments

Comments
 (0)