10000 Fix style & optimise http client usage · symfony/symfony@aa997d7 · GitHub
[go: up one dir, main page]
Skip to content

Commit aa997d7

Browse files
GromNaNGromNaN
committed
Fix style & optimise http client usage
1 parent b027a5c commit aa997d7

File tree

11 files changed

+47
-36
lines changed

11 files changed

+47
-36
lines changed

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

Lines 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

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\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

Lines 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

Lines 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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
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

Lines 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

Lines 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

Lines 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

Lines 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

Lines 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

Lines 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