Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 10dd62a

Browse filesBrowse files
GromNaNGromNaN
committed
Fix style & optimise http client usage
1 parent b027a5c commit 10dd62a
Copy full SHA for 10dd62a

File tree

7 files changed

+32
-21
lines changed
Filter options

7 files changed

+32
-21
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1046,7 +1046,7 @@ private function createVersion(ContainerBuilder $container, ?string $version, ?s
10461046

10471047
if (null !== $jsonManifestPath) {
10481048
$definitionName = 'assets.json_manifest_version_strategy';
1049-
if ('http' === substr(parse_url($jsonManifestPath, \PHP_URL_SCHEME), 0, 4)) {
1049+
if (0 === strpos(parse_url($jsonManifestPath, PHP_URL_SCHEME), 'http')) {
10501050
$definitionName = 'assets.remote_json_manifest_version_strategy';
10511051
}
10521052

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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/Resources/config/assets.xml
+1-1Lines 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\RemoteJsonManifestVersionStrategy" 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/FrameworkExtensionTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -557,10 +557,10 @@ public function testAssets()
557557
$this->assertEquals('assets.json_manifest_version_strategy', $versionStrategy->getParent());
558558
$this->assertEquals('/path/to/manifest.json', $versionStrategy->getArgument(0));
559559

560-
$package = $container->getDefinition((string) $packages['remote_manifest']);
561-
$versionStrategy = $container->getDefinition((string) $package->getArgument(1));
562-
$this->assertEquals('assets.remote_json_manifest_version_strategy', $versionStrategy->getParent());
563-
$this->assertEquals('https://cdn.example.com/manifest.json', $versionStrategy->getArgument(0));
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));
564564
}
565565

566566
public function testAssetsDefaultVersionStrategyAsService()

‎src/Symfony/Bundle/FrameworkBundle/composer.json

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/composer.json
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Asset/Tests/VersionStrategy/RemoteJsonManifestVersionStrategyTest.php
+7-6Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,41 +22,42 @@ public function testGetVersion()
2222
{
2323
$strategy = $this->createStrategy('https://cdn.example.com/manifest-valid.json');
2424

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

2828
public function testApplyVersion()
2929
{
3030
$strategy = $this->createStrategy('https://cdn.example.com/manifest-valid.json');
3131

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

3535
public function testApplyVersionWhenKeyDoesNotExistInManifest()
3636
{
3737
$strategy = $this->createStrategy('https://cdn.example.com/manifest-valid.json');
3838

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

4242
public function testMissingManifestFileThrowsException()
4343
{
4444
$this->expectException('RuntimeException');
45+
$this->expectExceptionMessage('Error loading asset manifest from URL "https://cdn.example.com/non-existent-file.json"');
4546
$strategy = $this->createStrategy('https://cdn.example.com/non-existent-file.json');
4647
$strategy->getVersion('main.js');
4748
}
4849

4950
public function testManifestFileWithBadJSONThrowsException()
5051
{
5152
$this->expectException('RuntimeException');
52-
$this->expectExceptionMessage('Error parsing JSON');
53+
$this->expectExceptionMessage('Error loading asset manifest from URL "https://cdn.example.com/manifest-invalid.json"');
5354
$strategy = $this->createStrategy('https://cdn.example.com/manifest-invalid.json');
5455
$strategy->getVersion('main.js');
5556
}
5657

5758
private function createStrategy($manifestUrl)
5859
{
59-
$this->httpClient = new MockHttpClient(function ($method, $url, $options) {
60+
$httpClient = new MockHttpClient(function ($method, $url, $options) {
6061
$filename = __DIR__.'/../fixtures/'.basename($url);
6162

6263
if (file_exists($filename)) {
@@ -66,6 +67,6 @@ private function createStrategy($manifestUrl)
6667
return new MockResponse('{}', ['http_code' => 404]);
6768
});
6869

69-
return new RemoteJsonManifestVersionStrategy($manifestUrl, $this->httpClient);
70+
return new RemoteJsonManifestVersionStrategy($manifestUrl, $httpClient);
7071
}
7172
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Asset/VersionStrategy/RemoteJsonManifestVersionStrategy.php
+16-6Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@
1111

1212
namespace Symfony\Component\Asset\VersionStrategy;
1313

14+
use Symfony\Contracts\HttpClient\Exception\ExceptionInterface;
1415
use Symfony\Contracts\HttpClient\HttpClientInterface;
16+
use Symfony\Contracts\HttpClient\ResponseInterface;
1517

1618
/**
1719
* Reads the versioned path of an asset from a remote JSON manifest file.
@@ -27,14 +29,21 @@
2729
class RemoteJsonManifestVersionStrategy implements VersionStrategyInterface
2830
{
2931
private $manifestData;
32+
33+
/**
34+
* @var ResponseInterface
35+
*/
3036
private $httpResponse;
3137

3238
/**
33-
* @param string $manifestUrl Absolute url to the manifest file
39+
* @param string $manifestUrl Absolute URL to the manifest file
3440
*/
3541
public function __construct(string $manifestUrl, HttpClientInterface $httpClient)
3642
{
37-
$this->httpResponse = $httpClient->request('GET', $manifestUrl);
43+
$this->httpResponse = $httpClient->request('GET', $manifestUrl, [
44+
'headers' => ['accept' => 'application/json'],
45+
'buffer' => true,
46+
]);
3847
}
3948

4049
/**
@@ -55,12 +64,13 @@ public function applyVersion(string $path)
5564
private function getManifestPath(string $path): ?string
5665
{
5766
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()));
67+
try {
68+
$this->manifestData = $this->httpResponse->toArray();
69+
} catch (ExceptionInterface $e) {
70+
throw new \RuntimeException(sprintf('Error loading asset manifest from URL "%s"', $this->httpResponse->getInfo()['url']), 0, $e);
6171
}
6272
}
6373

64-
return isset($this->manifestData[$path]) ? $this->manifestData[$path] : null;
74+
return $this->manifestData[$path] ?? null;
6575
}
6676
}

‎src/Symfony/Component/Asset/composer.json

Copy file name to clipboardExpand all lines: src/Symfony/Component/Asset/composer.json
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@
2222
"symfony/http-foundation": ""
2323
},
2424
"require-dev": {
25+
"symfony/http-client": "^4.4|^5.0",
2526
"symfony/http-foundation": "^4.4|^5.0",
26-
"symfony/http-kernel": "^4.4|^5.0",
27-
"symfony/http-client": "^4.4|^5.0"
27+
"symfony/http-kernel": "^4.4|^5.0"
2828
},
2929
"autoload": {
3030
"psr-4": { "Symfony\\Component\\Asset\\": "" },

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.