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 aa997d7

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

File tree

11 files changed

+47
-36
lines changed
Filter options

11 files changed

+47
-36
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
+13-13Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -530,35 +530,35 @@ public function testAssets()
530530
$packages = $container->getDefinition('assets.packages');
531531

532532
// default package
533-
$defaultPackage = $container->getDefinition((string) $packages->getArgument(0));
533+
$defaultPackage = $container->getDefinition($packages->getArgument(0));
534534
$this->assertUrlPackage($container, $defaultPackage, ['http://cdn.example.com'], 'SomeVersionScheme', '%%s?version=%%s');
535535

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

540-
$package = $container->getDefinition((string) $packages['images_path']);
540+
$package = $container->getDefinition($packages['images_path']);
541541
$this->assertPathPackage($container, $package, '/foo', 'SomeVersionScheme', '%%s?version=%%s');
542542

543-
$package = $container->getDefinition((string) $packages['images']);
543+
$package = $container->getDefinition($packages['images']);
544544
$this->assertUrlPackage($container, $package, ['http://images1.example.com', 'http://images2.example.com'], '1.0.0', '%%s?version=%%s');
545545

546-
$package = $container->getDefinition((string) $packages['foo']);
546+
$package = $container->getDefinition($packages['foo']);
547547
$this->assertPathPackage($container, $package, '', '1.0.0', '%%s-%%s');
548548

549-
$package = $container->getDefinition((string) $packages['bar']);
549+
$package = $container->getDefinition($packages['bar']);
550550
$this->assertUrlPackage($container, $package, ['https://bar2.example.com'], 'SomeVersionScheme', '%%s?version=%%s');
551551

552-
$package = $container->getDefinition((string) $packages['bar_version_strategy']);
552+
$package = $container->getDefinition($packages['bar_version_strategy']);
553553
$this->assertEquals('assets.custom_version_strategy', (string) $package->getArgument(1));
554554

555-
$package = $container->getDefinition((string) $packages['json_manifest_strategy']);
556-
$versionStrategy = $container->getDefinition((string) $package->getArgument(1));
555+
$package = $container->getDefinition($packages['json_manifest_strategy']);
556+
$versionStrategy = $container->getDefinition($package->getArgument(1));
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));
560+
$package = $container->getDefinition($packages['remote_manifest']);
561+
$versionStrategy = $container->getDefinition($package->getArgument(1));
562562
$this->assertEquals('assets.remote_json_manifest_version_strategy', $versionStrategy->getParent());
563563
$this->assertEquals('https://cdn.example.com/manifest.json', $versionStrategy->getArgument(0));
564564
}
@@ -569,7 +569,7 @@ public function testAssetsDefaultVersionStrategyAsService()
569569
$packages = $container->getDefinition('assets.packages');
570570

571571
// default package
572-
$defaultPackage = $container->getDefinition((string) $packages->getArgument(0));
572+
$defaultPackage = $container->getDefinition($packages->getArgument(0));
573573
$this->assertEquals('assets.custom_version_strategy', (string) $defaultPackage->getArgument(1));
574574
}
575575

@@ -643,7 +643,7 @@ public function testMessengerRouting()
643643

644644
$sendersMapping = $senderLocatorDefinition->getArgument(0);
645645
$this->assertEquals(['amqp', 'messenger.transport.audit'], $sendersMapping[DummyMessage::class]);
646-
$sendersLocator = $container->getDefinition((string) $senderLocatorDefinition->getArgument(1));
646+
$sendersLocator = $container->getDefinition($senderLocatorDefinition->getArgument(1));
647647
$this->assertSame(['amqp', 'audit', 'messenger.transport.amqp', 'messenger.transport.audit'], array_keys($sendersLocator->getArgument(0)));
648648
$this->assertEquals(new Reference('messenger.transport.amqp'), $sendersLocator->getArgument(0)['amqp']->getValues()[0]);
649649
$this->assertEquals(new Reference('messenger.transport.audit'), $sendersLocator->getArgument(0)['messenger.transport.audit']->getValues()[0]);
@@ -1511,7 +1511,7 @@ private function assertUrlPackage(ContainerBuilder $container, ChildDefinition $
15111511

15121512
private function assertVersionStrategy(ContainerBuilder $container, Reference $reference, $version, $format)
15131513
{
1514-
$versionStrategy = $container->getDefinition((string) $reference);
1514+
$versionStrategy = $container->getDefinition($reference);
15151515
if (null === $version) {
15161516
$this->assertEquals('assets.empty_version_strategy', (string) $reference);
15171517
} else {

‎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/EmptyVersionStrategyTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Asset/Tests/VersionStrategy/EmptyVersionStrategyTest.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,6 @@ public function testApplyVersion()
2929
$emptyVersionStrategy = new EmptyVersionStrategy();
3030
$path = 'test-path';
3131

32-
$this->assertEquals($path, $emptyVersionStrategy->applyVersion($path));
32+
$this->assertSame($path, $emptyVersionStrategy->applyVersion($path));
3333
}
3434
}

‎src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php
+3-3Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,21 +20,21 @@ public function testGetVersion()
2020
{
2121
$strategy = $this->createStrategy('manifest-valid.json');
2222

23-
$this->assertEquals('main.123abc.js', $strategy->getVersion('main.js'));
23+
$this->assertSame('main.123abc.js', $strategy->getVersion('main.js'));
2424
}
2525

2626
public function testApplyVersion()
2727
{
2828
$strategy = $this->createStrategy('manifest-valid.json');
2929

30-
$this->assertEquals('css/styles.555def.css', $strategy->getVersion('css/styles.css'));
30+
$this->assertSame('css/styles.555def.css', $strategy->getVersion('css/styles.css'));
3131
}
3232

3333
public function testApplyVersionWhenKeyDoesNotExistInManifest()
3434
{
3535
$strategy = $this->createStrategy('manifest-valid.json');
3636

37-
$this->assertEquals('css/other.css', $strategy->getVersion('css/other.css'));
37+
$this->assertSame('css/other.css', $strategy->getVersion('css/other.css'));
3838
}
3939

4040
public function testMissingManifestFileThrowsException()

‎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/Tests/VersionStrategy/StaticVersionStrategyTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Asset/Tests/VersionStrategy/StaticVersionStrategyTest.php
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public function testGetVersion()
2121
$version = 'v1';
2222
$path = 'test-path';
2323
$staticVersionStrategy = new StaticVersionStrategy($version);
24-
$this->assertEquals($version, $staticVersionStrategy->getVersion($path));
24+
$this->assertSame($version, $staticVersionStrategy->getVersion($path));
2525
}
2626

2727
/**
@@ -31,7 +31,7 @@ public function testApplyVersion($path, $version, $format)
3131
{
3232
$staticVersionStrategy = new StaticVersionStrategy($version, $format);
3333
$formatted = sprintf($format ?: '%s?%s', $path, $version);
34-
$this->assertEquals($formatted, $staticVersionStrategy->applyVersion($path));
34+
$this->assertSame($formatted, $staticVersionStrategy->applyVersion($path));
3535
}
3636

3737
public function getConfigs()

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,6 @@ private function getManifestPath(string $path): ?string
6363
}
6464
}
6565

66-
return isset($this->manifestData[$path]) ? $this->manifestData[$path] : null;
66+
return $this->manifestData[$path] ?? null;
6767
}
6868
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Asset/VersionStrategy/RemoteJsonManifestVersionStrategy.php
+15-5Lines changed: 15 additions & 5 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
/**
3339
* @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.