Skip to content

[Asset] [DX] Option to make asset manifests strict on missing item #38495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 25, 2021
Merged

[Asset] [DX] Option to make asset manifests strict on missing item #38495

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,10 @@ private function addAssetsSection(ArrayNodeDefinition $rootNode, callable $enabl
->{$enableIfStandalone('symfony/asset', Package::class)}()
->fixXmlConfig('base_url')
->children()
->booleanNode('strict_mode')
->info('Throw an exception if an entry is missing from the manifest.json')
->defaultFalse()
->end()
->scalarNode('version_strategy')->defaultNull()->end()
->scalarNode('version')->defaultNull()->end()
->scalarNode('version_format')->defaultValue('%%s?%%s')->end()
Expand Down Expand Up @@ -733,6 +737,10 @@ private function addAssetsSection(ArrayNodeDefinition $rootNode, callable $enabl
->prototype('array')
->fixXmlConfig('base_url')
->children()
->booleanNode('strict_mode')
->info('Throw an exception if an entry is missing from the manifest.json')
->defaultFalse()
->end()
->scalarNode('version_strategy')->defaultNull()->end()
->scalarNode('version')
->beforeNormalization()
Expand Down
7 changes: 4 additions & 3 deletions src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1141,7 +1141,7 @@ private function registerAssetsConfiguration(array $config, ContainerBuilder $co
if ($config['version_strategy']) {
$defaultVersion = new Reference($config['version_strategy']);
} else {
$defaultVersion = $this->createVersion($container, $config['version'], $config['version_format'], $config['json_manifest_path'], '_default');
$defaultVersion = $this->createVersion($container, $config['version'], $config['version_format'], $config['json_manifest_path'], '_default', $config['strict_mode']);
}

$defaultPackage = $this->createPackageDefinition($config['base_path'], $config['base_urls'], $defaultVersion);
Expand All @@ -1157,7 +1157,7 @@ private function registerAssetsConfiguration(array $config, ContainerBuilder $co
// let format fallback to main version_format
$format = $package['version_format'] ?: $config['version_format'];
$version = $package['version'] ?? null;
$version = $this->createVersion($container, $version, $format, $package['json_manifest_path'], $name);
$version = $this->createVersion($container, $version, $format, $package['json_manifest_path'], $name, $package['strict_mode']);
}

$packageDefinition = $this->createPackageDefinition($package['base_path'], $package['base_urls'], $version)
Expand Down Expand Up @@ -1186,7 +1186,7 @@ private function createPackageDefinition(?string $basePath, array $baseUrls, Ref
return $package;
}

private function createVersion(ContainerBuilder $container, ?string $version, ?string $format, ?string $jsonManifestPath, string $name): Reference
private function createVersion(ContainerBuilder $container, ?string $version, ?string $format, ?string $jsonManifestPath, string $name, bool $strictMode): Reference
{
// Configuration prevents $version and $jsonManifestPath from being set
if (null !== $version) {
Expand All @@ -1203,6 +1203,7 @@ private function createVersion(ContainerBuilder $container, ?string $version, ?s
if (null !== $jsonManifestPath) {
$def = new ChildDefinition('assets.json_manifest_version_strategy');
$def->replaceArgument(0, $jsonManifestPath);
$def->replaceArgument(2, $strictMode);
$container->setDefinition('assets._version_'.$name, $def);

return new Reference('assets._version_'.$name);
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/assets.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
->args([
abstract_arg('manifest path'),
service('http_client')->nullOnInvalid(),
false,
])

->set('assets.remote_json_manifest_version_strategy', RemoteJsonManifestVersionStrategy::class)
Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@
<xsd:attribute name="version" type="xsd:string" />
<xsd:attribute name="version-format" type="xsd:string" />
<xsd:attribute name="json-manifest-path" type="xsd:string" />
<xsd:attribute name="strict-mode" type="xsd:boolean" />
</xsd:complexType>

<xsd:complexType name="package">
Expand All @@ -168,6 +169,7 @@
<xsd:attribute name="version" type="xsd:string" />
<xsd:attribute name="version-format" type="xsd:string" />
<xsd:attribute name="json-manifest-path" type="xsd:string" />
<xsd:attribute name="strict-mode" type="xsd:boolean" />
</xsd:complexType>

<xsd:complexType name="translator">
Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ public function testAssetsCanBeEnabled()
'base_urls' => [],
'packages' => [],
'json_manifest_path' => null,
'strict_mode' => false,
];

$this->assertEquals($defaultConfig, $config['assets']);
Expand Down Expand Up @@ -489,6 +490,7 @@ protected static function getBundleDefaultConfig()
'base_urls' => [],
'packages' => [],
'json_manifest_path' => null,
'strict_mode' => false,
],
'cache' => [
'pools' => [],
Expand Down
4 changes: 4 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/assets.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@
'env_manifest' => [
'json_manifest_path' => '%env(env_manifest)%',
],
'strict_manifest_strategy' => [
'json_manifest_path' => '/path/to/manifest.json',
'strict_mode' => true,
],
],
],
]);
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/assets.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
<framework:package name="remote_manifest" json-manifest-path="https://cdn.example.com/manifest.json" />
<framework:package name="var_manifest" json-manifest-path="%var_json_manifest_path%" />
<framework:package name="env_manifest" json-manifest-path="%env(env_manifest)%" />
<framework:package name="strict_manifest_strategy" json-manifest-path="/path/to/manifest.json" strict-mode="true" />
</framework:assets>
</framework:config>

Expand Down
3 changes: 3 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/assets.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ framework:
json_manifest_path: '%var_json_manifest_path%'
env_manifest:
json_manifest_path: '%env(env_manifest)%'
strict_manifest_strategy:
json_manifest_path: '/path/to/manifest.json'
strict_mode: true

parameters:
var_json_manifest_path: 'https://cdn.example.com/manifest.json'
Expand Down
11 changes: 10 additions & 1 deletion src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ public function testAssets()

// packages
$packageTags = $container->findTaggedServiceIds('assets.package');
$this->assertCount(9, $packageTags);
$this->assertCount(10, $packageTags);

$packages = [];
foreach ($packageTags as $serviceId => $tagAttributes) {
Expand All @@ -658,6 +658,7 @@ public function testAssets()
$versionStrategy = $container->getDefinition((string) $package->getArgument(1));
$this->assertEquals('assets.json_manifest_version_strategy', $versionStrategy->getParent());
$this->assertEquals('/path/to/manifest.json', $versionStrategy->getArgument(0));
$this->assertFalse($versionStrategy->getArgument(2));

$package = $container->getDefinition($packages['remote_manifest']);
$versionStrategy = $container->getDefinition($package->getArgument(1));
Expand All @@ -668,11 +669,19 @@ public function testAssets()
$versionStrategy = $container->getDefinition($package->getArgument(1));
$this->assertSame('assets.json_manifest_version_strategy', $versionStrategy->getParent());
$this->assertSame('https://cdn.example.com/manifest.json', $versionStrategy->getArgument(0));
$this->assertFalse($versionStrategy->getArgument(2));

$package = $container->getDefinition($packages['env_manifest']);
$versionStrategy = $container->getDefinition($package->getArgument(1));
$this->assertSame('assets.json_manifest_version_strategy', $versionStrategy->getParent());
$this->assertStringMatchesFormat('env_%s', $versionStrategy->getArgument(0));
$this->assertFalse($versionStrategy->getArgument(2));

$package = $container->getDefinition((string) $packages['strict_manifest_strategy']);
$versionStrategy = $container->getDefinition((string) $package->getArgument(1));
$this->assertEquals('assets.json_manifest_version_strategy', $versionStrategy->getParent());
$this->assertEquals('/path/to/manifest.json', $versionStrategy->getArgument(0));
$this->assertTrue($versionStrategy->getArgument(2));
}

public function testAssetsDefaultVersionStrategyAsService()
Expand Down
41 changes: 41 additions & 0 deletions src/Symfony/Component/Asset/Exception/AssetNotFoundException.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Asset\Exception;

/**
* Represents an asset not found in a manifest.
*/
class AssetNotFoundException extends RuntimeException
{
private $alternatives;

/**
* @param string $message Exception message to throw
* @param array $alternatives List of similar defined names
* @param int $code Exception code
* @param \Throwable $previous Previous exception used for the exception chaining
*/
public function __construct(string $message, array $alternatives = [], int $code = 0, \Throwable $previous = null)
{
parent::__construct($message, $code, $previous);

$this->alternatives = $alternatives;
}

/**
* @return array A list of similar defined names
*/
public function getAlternatives(): array
{
return $this->alternatives;
}
}
19 changes: 19 additions & 0 deletions src/Symfony/Component/Asset/Exception/RuntimeException.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Asset\Exception;

/**
* Base RuntimeException for the Asset component.
*/
class RuntimeException extends \RuntimeException implements ExceptionInterface
{
}
44 changes: 37 additions & 7 deletions src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,51 +12,64 @@
namespace Symfony\Component\Asset\Tests\VersionStrategy;

use PHPUnit\Framework\TestCase;
use Symfony\Component\Asset\Exception\AssetNotFoundException;
use Symfony\Component\Asset\Exception\RuntimeException;
use Symfony\Component\Asset\VersionStrategy\JsonManifestVersionStrategy;
use Symfony\Component\HttpClient\MockHttpClient;
use Symfony\Component\HttpClient\Response\MockResponse;

class JsonManifestVersionStrategyTest extends TestCase
{
/**
* @dataProvider ProvideValidStrategies
* @dataProvider provideValidStrategies
*/
public function testGetVersion(JsonManifestVersionStrategy $strategy)
{
$this->assertSame('main.123abc.js', $strategy->getVersion('main.js'));
}

/**
* @dataProvider ProvideValidStrategies
* @dataProvider provideValidStrategies
*/
public function testApplyVersion(JsonManifestVersionStrategy $strategy)
{
$this->assertSame('css/styles.555def.css', $strategy->applyVersion('css/styles.css'));
}

/**
* @dataProvider ProvideValidStrategies
* @dataProvider provideValidStrategies
*/
public function testApplyVersionWhenKeyDoesNotExistInManifest(JsonManifestVersionStrategy $strategy)
{
$this->assertSame('css/other.css', $strategy->applyVersion('css/other.css'));
}

/**
* @dataProvider ProvideMissingStrategies
* @dataProvider provideStrictStrategies
*/
public function testStrictExceptionWhenKeyDoesNotExistInManifest(JsonManifestVersionStrategy $strategy, $path, $message)
{
$this->expectException(AssetNotFoundException::class);
$this->expectExceptionMessageMatches($message);

$strategy->getVersion($path);
}

/**
* @dataProvider provideMissingStrategies
*/
public function testMissingManifestFileThrowsException(JsonManifestVersionStrategy $strategy)
{
$this->expectException(\RuntimeException::class);
$this->expectException(RuntimeException::class);
$strategy->getVersion('main.js');
}

/**
* @dataProvider ProvideInvalidStrategies
* @dataProvider provideInvalidStrategies
*/
public function testManifestFileWithBadJSONThrowsException(JsonManifestVersionStrategy $strategy)
{
$this->expectException(\RuntimeException::class);
$this->expectException(RuntimeException::class);
$this->expectExceptionMessage('Error parsing JSON');
$strategy->getVersion('main.js');
}
Expand Down Expand Up @@ -100,4 +113,21 @@ public function provideStrategies(string $manifestPath)

yield [new JsonManifestVersionStrategy(__DIR__.'/../fixtures/'.$manifestPath)];
}

public function provideStrictStrategies()
{
$strategy = new JsonManifestVersionStrategy(__DIR__.'/../fixtures/manifest-valid.json', null, true);

yield [
$strategy,
'css/styles.555def.css',
'~Asset "css/styles.555def.css" not found in manifest "(.*)/manifest-valid.json"\. Did you mean one of these\? "css/styles.css", "css/style.css".~',
];

yield [
$strategy,
'img/avatar.png',
'~Asset "img/avatar.png" not found in manifest "(.*)/manifest-valid.json"\.~',
];
}
}
4 changes: 3 additions & 1 deletion src/Symfony/Component/Asset/Tests/fixtures/manifest-valid.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
{
"main.js": "main.123abc.js",
"css/styles.css": "css/styles.555def.css"
"css/styles.css": "css/styles.555def.css",
"css/style.css": "css/style.abcdef.css",
"main/home.css": "main/home.css"
}
Loading