8000 [Asset] Handle URL in json_manifest_path · symfony/symfony@b6b5d59 · GitHub
[go: up one dir, main page]

Skip to content

Commit b6b5d59

Browse files
committed
[Asset] Handle URL in json_manifest_path
Download the manifest using the HttpClient
1 parent 3f177be commit b6b5d59

File tree

11 files changed

+48
-78
lines changed

11 files changed

+48
-78
lines changed

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -585,7 +585,6 @@ private function addAssetsSection(ArrayNodeDefinition $rootNode)
585585
->scalarNode('version')->defaultNull()->end()
586586
->scalarNode('version_format')->defaultValue('%%s?%%s')->end()
587587
->scalarNode('json_manifest_path')->defaultNull()->end()
588-
->scalarNode('json_manifest_url')->defaultNull()->end()
589588
->scalarNode('base_path')->defaultValue('')->end()
590589
->arrayNode('base_urls')
591590
->requiresAtLeastOneElement()
@@ -611,24 +610,6 @@ private function addAssetsSection(ArrayNodeDefinition $rootNode)
611610
})
612611
->thenInvalid('You cannot use both "version" and "json_manifest_path" at the same time under "assets".')
613612
->end()
614-
->validate()
615-
->ifTrue(function ($v) {
616-
return isset($v['version_strategy']) && isset($v['json_manifest_url']);
617-
})
618-
->thenInvalid('You cannot use both "version_strategy" and "json_manifest_url" at the same time under "assets" packages.')
619-
->end()
620-
->validate()
621-
->ifTrue(function ($v) {
622-
return isset($v['version']) && isset($v['json_manifest_url']);
623-
})
624-
->thenInvalid('You cannot use both "version" and "json_manifest_url" at the same time under "assets" packages.')
625-
->end()
626-
->validate()
627-
->ifTrue(function ($v) {
628-
return isset($v['json_manifest_path']) && isset($v['json_manifest_url']);
629-
})
630-
->thenInvalid('You cannot use both "json_manifest_path" and "json_manifest_url" at the same time under "assets" packages.')
631-
->end()
632613
->fixXmlConfig('package')
633614
->children()
634615
->arrayNode('packages')
@@ -646,7 +627,6 @@ private function addAssetsSection(ArrayNodeDefinition $rootNode)
646627
->end()
647628
->scalarNode('version_format')->defaultNull()->end()
648629
->scalarNode('json_manifest_path')->defaultNull()->end()
649-
->scalarNode('json_manifest_url')->defaultNull()->end()
650630
->scalarNode('base_path')->defaultValue('')->end()
651631
->arrayNode('base_urls')
652632
->requiresAtLeastOneElement()
@@ -672,24 +652,6 @@ private function addAssetsSection(ArrayNodeDefinition $rootNode)
672652
})
673653
->thenInvalid('You cannot use both "version" and "json_manifest_path" at the same time under "assets" packages.')
674654
->end()
675-
->validate()
676-
->ifTrue(function ($v) {
677-
return isset($v['version_strategy']) && isset($v['json_manifest_url']);
678-
})
679-
->thenInvalid('You cannot use both "version_strategy" and "json_manifest_url" at the same time under "assets" packages.')
680-
->end()
681-
->validate()
682-
->ifTrue(function ($v) {
683-
return isset($v['version']) && isset($v['json_manifest_url']);
684-
})
685-
->thenInvalid('You cannot use both "version" and "json_manifest_url" at the same time under "assets" packages.')
686-
->end()
687-
->validate()
688-
->ifTrue(function ($v) {
689-
return isset($v['json_manifest_path']) && isset($v['json_manifest_url']);
690-
})
691-
->thenInvalid('You cannot use both "json_manifest_path" and "json_manifest_url" at the same time under "assets" packages.')
692-
->end()
693655
->end()
694656
->end()
695657
->end()

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -984,7 +984,7 @@ private function registerAssetsConfiguration(array $config, ContainerBuilder $co
984984
if ($config['version_strategy']) {
985985
$defaultVersion = new Reference($config['version_strategy']);
986986
} else {
987-
$defaultVersion = $this->createVersion($container, $config['version'], $config['version_format'], $config['json_manifest_path'], $config['json_manifest_url'], '_default');
987+
$defaultVersion = $this->createVersion($container, $config['version'], $config['version_format'], $config['json_manifest_path'], '_default');
988988
}
989989

990990
$defaultPackage = $this->createPackageDefinition($config['base_path'], $config['base_urls'], $defaultVersion);
@@ -994,14 +994,14 @@ private function registerAssetsConfiguration(array $config, ContainerBuilder $co
994994
foreach ($config['packages'] as $name => $package) {
995995
if (null !== $package['version_strategy']) {
996996
$version = new Reference($package['version_strategy']);
997-
} elseif (!\array_key_exists('version', $package) && null === $package['json_manifest_path'] && null === $package['json_manifest_url']) {
998-
// if neither version nor json_manifest_path nor json_manifest_url are specified, use the default
997+
} elseif (!\array_key_exists('version', $package) && null === $package['json_manifest_path']) {
998+
// if neither version nor json_manifest_path are specified, use the default
999999
$version = $defaultVersion;
10001000
} else {
10011001
// let format fallback to main version_format
10021002
$format = $package['version_format'] ?: $config['version_format'];
10031003
$version = isset($package['version']) ? $package['version'] : null;
1004-
$version = $this->createVersion($container, $version, $format, $package['json_manifest_path'], $package['json_manifest_url'], $name);
1004+
$version = $this->createVersion($container, $version, $format, $package['json_manifest_path'], $name);
10051005
}
10061006

10071007
$container->setDefinition('assets._package_'.$name, $this->createPackageDefinition($package['base_path'], $package['base_urls'], $version));
@@ -1034,7 +1034,7 @@ private function createPackageDefinition(?string $basePath, array $baseUrls, Ref
10341034
return $package;
10351035
}
10361036

1037-
private function createVersion(ContainerBuilder $container, ?string $version, ?string $format, ?string $jsonManifestPath, ?string $jsonManifestUrl, string $name): Reference
1037+
private function createVersion(ContainerBuilder $container, ?string $version, ?string $format, ?string $jsonManifestPath, string $name): Reference
10381038
{
10391039
// Configuration prevents $version and $jsonManifestPath from being set
10401040
if (null !== $version) {
@@ -1049,16 +1049,13 @@ private function createVersion(ContainerBuilder $container, ?string $version, ?s
10491049
}
10501050

10511051
if (null !== $jsonManifestPath) {
1052-
$def = new ChildDefinition('assets.json_manifest_version_strategy');
1053-
$def->replaceArgument(0, $jsonManifestPath);
1054-
$container->setDefinition('assets._version_'.$name, $def);
1055-
1056-
return new Reference('assets._version_'.$name);
1057-
}
1052+
$definitionName = 'assets.json_manifest_version_strategy';
1053+
if (0 === strpos(parse_url($jsonManifestPath, PHP_URL_SCHEME), 'http')) {
1054+
$definitionName = 'assets.remote_json_manifest_version_strategy';
1055+
}
10581056

1059-
if (null !== $jsonManifestUrl) {
1060-
$def = new ChildDefinition('assets.remote_json_manifest_version_strategy');
1061-
$def->replaceArgument(0, $jsonManifestUrl);
1057+
$def = new ChildDefinition($definitionName);
1058+
$def->replaceArgument(0, $jsonManifestPath);
10621059
$container->setDefinition('assets._version_'.$name, $def);
10631060

10641061
return new Reference('assets._version_'.$name);

src/Symfony/Bundle/FrameworkBundle/Resources/config/assets.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353

5454
<service id="assets.remote_json_manifest_version_strategy" class="Symfony\Component\Asset\VersionStrategy\RemoteJsonManifestVersionStrat 1241 egy" abstract="true">
5555
<argument /> <!-- manifest url -->
56-
<argument type="service" id="http_client"/>
56+
<argument type="service" id="http_client" />
5757
</service>
5858
</services>
5959
</container>

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/assets.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@
2727
'json_manifest_strategy' => [
2828
'json_manifest_path' => '/path/to/manifest.json',
2929
],
30+
'remote_manifest' => [
31+
'json_manifest_path' => 'https://cdn.example.com/manifest.json',
32+
],
3033
],
3134
],
3235
]);

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/assets.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
<framework:base-url>https://bar_version_strategy.example.com</framework:base-url>
2323
</framework:package>
2424
<framework:package name="json_manifest_strategy" json-manifest-path="/path/to/manifest.json" />
25+
<framework:package name="remote_manifest" json-manifest-path="https://cdn.example.com/manifest.json" />
2526
</framework:assets>
2627
</framework:config>
2728
</container>

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/assets.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,5 @@ framework:
1919
version_strategy: assets.custom_version_strategy
2020
json_manifest_strategy:
2121
json_manifest_path: '/path/to/manifest.json'
22+
remote_manifest:
23+
json_manifest_path: 'https://cdn.example.com/manifest.json'

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -535,7 +535,7 @@ public function testAssets()
535535

536536
// packages
537537
$packages = $packages->getArgument(1);
538-
$this->assertCount(6, $packages);
538+
$this->assertCount(7, $packages);
539539

540540
$package = $container->getDefinition((string) $packages['images_path']);
541541
$this->assertPathPackage($container, $package, '/foo', 'SomeVersionScheme', '%%s?version=%%s');
@@ -556,6 +556,11 @@ public function testAssets()
556556
$versionStrategy = $container->getDefinition((string) $package->getArgument(1));
557557
$this->assertEquals('assets.json_manifest_version_strategy', $versionStrategy->getParent());
558558
$this->assertEquals('/path/to/manifest.json', $versionStrategy->getArgument(0));
559+
560+
$package = $container->getDefinition($packages['remote_manifest']);
561+
$versionStrategy = $container->getDefinition($package->getArgument(1));
562+
$this->assertSame('assets.remote_json_manifest_version_strategy', $versionStrategy->getParent());
563+
$this->assertSame('https://cdn.example.com/manifest.json', $versionStrategy->getArgument(0));
559564
}
560565

561566
public function testAssetsDefaultVersionStrategyAsService()

src/Symfony/Bundle/FrameworkBundle/composer.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
"require-dev": {
3333
"doctrine/annotations": "~1.7",
3434
"doctrine/cache": "~1.0",
35-
"symfony/asset": "^4.4|^5.0",
35+
"symfony/asset": "^5.1",
3636
"symfony/browser-kit": "^4.4|^5.0",
3737
"symfony/console": "^4.4|^5.0",
3838
"symfony/css-selector": "^4.4|^5.0",
@@ -68,7 +68,7 @@
6868
"phpdocumentor/reflection-docblock": "<3.0",
6969
"phpdocumentor/type-resolver": "<0.2.1",
7070
"phpunit/phpunit": "<5.4.3",
71-
"symfony/asset": "<4.4",
71+
"symfony/asset": "<5.1",
7272
"symfony/browser-kit": "<4.4",
7373
"symfony/console": "<4.4",
7474
"symfony/dotenv": "<5.1",

src/Symfony/Component/Asset/Tests/VersionStrategy/RemoteJsonManifestVersionStrategyTest.php

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

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\Asset\VersionStrategy\RemoteJsonManifestVersionStrategy;
16+
use Symfony\Component\HttpClient\Exception\JsonException;
1617
use Symfony\Component\HttpClient\MockHttpClient;
1718
use Symfony\Component\HttpClient\Response\MockResponse;
1819

@@ -22,50 +23,51 @@ public function testGetVersion()
2223
{
2324
$strategy = $this->createStrategy('https://cdn.example.com/manifest-valid.json');
2425

25-
$this->assertEquals('main.123abc.js', $strategy->getVersion('main.js'));
26+
$this->assertSame('main.123abc.js', $strategy->getVersion('main.js'));
2627
}
2728

2829
public function testApplyVersion()
2930
{
3031
$strategy = $this->createStrategy('https://cdn.example.com/manifest-valid.json');
3132

32-
$this->assertEquals('css/styles.555def.css', $strategy->getVersion('css/styles.css'));
33+
$this->assertSame('css/styles.555def.css', $strategy->getVersion('css/styles.css'));
3334
}
3435

3536
public function testApplyVersionWhenKeyDoesNotExistInManifest()
3637
{
3738
$strategy = $this->createStrategy('https://cdn.example.com/manifest-valid.json');
3839

39-
$this->assertEquals('css/other.css', $strategy->getVersion('css/other.css'));
40+
$this->assertSame('css/other.css', $strategy->getVersion('css/other.css'));
4041
}
4142

4243
public function testMissingManifestFileThrowsException()
4344
{
4445
$this->expectException('RuntimeException');
46+
$this->expectExceptionMessage('HTTP 404 returned for "https://cdn.example.com/non-existent-file.json"');
4547
$strategy = $this->createStrategy('https://cdn.example.com/non-existent-file.json');
4648
$strategy->getVersion('main.js');
4749
}
4850

4951
public function testManifestFileWithBadJSONThrowsException()
5052
{
51-
$this->expectException('RuntimeException');
52-
$this->expectExceptionMessage('Error parsing JSON');
53+
$this->expectException(JsonException::class);
54+
$this->expectExceptionMessage('Syntax error');
5355
$strategy = $this->createStrategy('https://cdn.example.com/manifest-invalid.json');
5456
$strategy->getVersion('main.js');
5557
}
5658

5759
private function createStrategy($manifestUrl)
5860
{
59-
$this->httpClient = new MockHttpClient(function ($method, $url, $options) {
61+
$httpClient = new MockHttpClient(function ($method, $url, $options) {
6062
$filename = __DIR__.'/../fixtures/'.basename($url);
6163

6264
if (file_exists($filename)) {
63-
return new MockResponse(file_get_contents($filename));
65+
return new MockResponse(file_get_contents($filename), ['http_headers' => ['content-type' => 'application/json']]);
6466
}
6567

6668
return new MockResponse('{}', ['http_code' => 404]);
6769
});
6870

69-
return new RemoteJsonManifestVersionStrategy($manifestUrl, $this->httpClient);
71+
return new RemoteJsonManifestVersionStrategy($manifestUrl, $httpClient);
7072
}
7173
}

src/Symfony/Component/Asset/VersionStrategy/RemoteJsonManifestVersionStrategy.php

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Asset\VersionStrategy;
1313

1414
use Symfony\Contracts\HttpClient\HttpClientInterface;
15+
use Symfony\Contracts\HttpClient\ResponseInterface;
1516

1617
/**
1718
* Reads the versioned path of an asset from a remote JSON manifest file.
@@ -27,14 +28,16 @@
2728
class RemoteJsonManifestVersionStrategy implements VersionStrategyInterface
2829
{
2930
private $manifestData;
30-
private $httpResponse;
31+
private $manifestUrl;
32+
private $httpClient;
3133

3234
/**
33-
* @param string $manifestUrl Absolute url to the manifest file
35+
* @param string $manifestUrl Absolute URL to the manifest file
3436
*/
3537
public function __construct(string $manifestUrl, HttpClientInterface $httpClient)
3638
{
37-
$this->httpResponse = $httpClient->request('GET', $manifestUrl);
39+
$this->manifestUrl = $manifestUrl;
40+
$this->httpClient = $httpClient;
3841
}
3942

4043
/**
@@ -44,23 +47,17 @@ public function __construct(string $manifestUrl, HttpClientInterface $httpClient
4447
*/
4548
public function getVersion(string $path)
4649
{
47-
return $this->applyVersion($path);
50+
$this->applyVersion($path);
4851
}
4952

5053
public function applyVersion(string $path)
51-
{
52-
return $this->getManifestPath($path) ?: $path;
53-
}
54-
55-
private function getManifestPath(string $path): ?string
5654
{
5755
if (null === $this->manifestData) {
58-
$this->manifestData = json_decode($this->httpResponse->getContent(), true);
59-
if (0 < json_last_error()) {
60-
throw new \RuntimeException(sprintf('Error parsing JSON from asset manifest file "%s" - %s', $this->httpResponse->getInfo()['url'], json_last_error_msg()));
61-
}
56+
$this->manifestData = $this->httpClient->request('GET', $this->manifestUrl, [
57+
'headers' => ['accept' => 'application/json'],
58+
])->toArray();
6259
}
6360

64-
return isset($this->manifestData[$path]) ? $this->manifestData[$path] : null;
61+
return $this->manifestData[$path] ?? $path;
6562
}
6663
}

src/Symfony/Component/Asset/composer.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
"symfony/http-foundation": ""
2323
},
2424
"require-dev": {
25+
"symfony/http-client": "^4.4|^5.0",
2526
"symfony/http-foundation": "^4.4|^5.0",
2627
"symfony/http-kernel": "^4.4|^5.0"
2728
},

0 commit comments

Comments
 (0)
0