From 4e4a81c3461920a17845ca94bcc2d67addf05d9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Deruss=C3=A9?= Date: Sun, 13 Dec 2020 19:17:49 +0100 Subject: [PATCH] Allow env variables in `json_manifest_path` --- UPGRADE-5.3.md | 5 ++ UPGRADE-6.0.md | 5 ++ .../FrameworkExtension.php | 7 +- .../Resources/config/assets.php | 2 + .../Fixtures/php/assets.php | 9 +++ .../Fixtures/xml/assets.xml | 7 ++ .../Fixtures/yml/assets.yml | 8 +++ .../FrameworkExtensionTest.php | 14 +++- .../Bundle/FrameworkBundle/composer.json | 4 +- src/Symfony/Component/Asset/CHANGELOG.md | 5 ++ .../JsonManifestVersionStrategyTest.php | 66 ++++++++++++++----- .../RemoteJsonManifestVersionStrategyTest.php | 3 + .../JsonManifestVersionStrategy.php | 29 ++++++-- .../RemoteJsonManifestVersionStrategy.php | 4 ++ src/Symfony/Component/Asset/composer.json | 3 +- 15 files changed, 138 insertions(+), 33 deletions(-) diff --git a/UPGRADE-5.3.md b/UPGRADE-5.3.md index 922a1b0ad8..2ccc3bc039 100644 --- a/UPGRADE-5.3.md +++ b/UPGRADE-5.3.md @@ -1,6 +1,11 @@ UPGRADE FROM 5.2 to 5.3 ======================= +Asset +----- + + * Deprecated `RemoteJsonManifestVersionStrategy`, use `JsonManifestVersionStrategy` instead. + Form ---- diff --git a/UPGRADE-6.0.md b/UPGRADE-6.0.md index 0413478ff7..a72f4731ef 100644 --- a/UPGRADE-6.0.md +++ b/UPGRADE-6.0.md @@ -1,6 +1,11 @@ UPGRADE FROM 5.x to 6.0 ======================= +Asset +----- + + * Removed `RemoteJsonManifestVersionStrategy`, use `JsonManifestVersionStrategy` instead. + Config ------ diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 51af309978..7a2506e2ee 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -1129,12 +1129,7 @@ class FrameworkExtension extends Extension } if (null !== $jsonManifestPath) { - $definitionName = 'assets.json_manifest_version_strategy'; - if (0 === strpos(parse_url($jsonManifestPath, \PHP_URL_SCHEME), 'http')) { - $definitionName = 'assets.remote_json_manifest_version_strategy'; - } - - $def = new ChildDefinition($definitionName); + $def = new ChildDefinition('assets.json_manifest_version_strategy'); $def->replaceArgument(0, $jsonManifestPath); $container->setDefinition('assets._version_'.$name, $def); diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/assets.php b/src/Symfony/Bundle/FrameworkBundle/Resources/config/assets.php index 28f7bba6a4..db889670c2 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/assets.php +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/assets.php @@ -77,10 +77,12 @@ return static function (ContainerConfigurator $container) { ->abstract() ->args([ abstract_arg('manifest path'), + service('http_client'), ]) ->set('assets.remote_json_manifest_version_strategy', RemoteJsonManifestVersionStrategy::class) ->abstract() + ->deprecate('symfony/framework-bundle', '5.3', 'The "%service_id%" service is deprecated, use "assets.json_manifest_version_strategy" instead.') ->args([ abstract_arg('manifest url'), service('http_client'), diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/assets.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/assets.php index ef2fd77013..ab16a52e21 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/assets.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/assets.php @@ -30,6 +30,15 @@ $container->loadFromExtension('framework', [ 'remote_manifest' => [ 'json_manifest_path' => 'https://cdn.example.com/manifest.json', ], + 'var_manifest' => [ + 'json_manifest_path' => '%var_json_manifest_path%', + ], + 'env_manifest' => [ + 'json_manifest_path' => '%env(env_manifest)%', + ], ], ], ]); + +$container->setParameter('var_json_manifest_path', 'https://cdn.example.com/manifest.json'); +$container->setParameter('env(env_manifest)', 'https://cdn.example.com/manifest.json'); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/assets.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/assets.xml index 24bfdc6456..ae0e0e099b 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/assets.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/assets.xml @@ -23,6 +23,13 @@ + + + + + https://cdn.example.com/manifest.json + https://cdn.example.com/manifest.json + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/assets.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/assets.yml index 4a4a57bc43..ab9eb1b610 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/assets.yml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/assets.yml @@ -21,3 +21,11 @@ framework: json_manifest_path: '/path/to/manifest.json' remote_manifest: json_manifest_path: 'https://cdn.example.com/manifest.json' + var_manifest: + json_manifest_path: '%var_json_manifest_path%' + env_manifest: + json_manifest_path: '%env(env_manifest)%' + +parameters: + var_json_manifest_path: 'https://cdn.example.com/manifest.json' + env(env_manifest): https://cdn.example.com/manifest.json diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index 6d4beaaccb..63538ebff8 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -597,7 +597,7 @@ abstract class FrameworkExtensionTest extends TestCase // packages $packages = $packages->getArgument(1); - $this->assertCount(7, $packages); + $this->assertCount(9, $packages); $package = $container->getDefinition((string) $packages['images_path']); $this->assertPathPackage($container, $package, '/foo', 'SomeVersionScheme', '%%s?version=%%s'); @@ -621,8 +621,18 @@ abstract class FrameworkExtensionTest extends TestCase $package = $container->getDefinition($packages['remote_manifest']); $versionStrategy = $container->getDefinition($package->getArgument(1)); - $this->assertSame('assets.remote_json_manifest_version_strategy', $versionStrategy->getParent()); + $this->assertSame('assets.json_manifest_version_strategy', $versionStrategy->getParent()); $this->assertSame('https://cdn.example.com/manifest.json', $versionStrategy->getArgument(0)); + + $package = $container->getDefinition($packages['var_manifest']); + $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)); + + $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)); } public function testAssetsDefaultVersionStrategyAsService() diff --git a/src/Symfony/Bundle/FrameworkBundle/composer.json b/src/Symfony/Bundle/FrameworkBundle/composer.json index 8f8dae611f..22d582d7ec 100644 --- a/src/Symfony/Bundle/FrameworkBundle/composer.json +++ b/src/Symfony/Bundle/FrameworkBundle/composer.json @@ -34,7 +34,7 @@ "require-dev": { "doctrine/annotations": "~1.7", "doctrine/cache": "~1.0", - "symfony/asset": "^5.1", + "symfony/asset": "^5.3", "symfony/browser-kit": "^4.4|^5.0", "symfony/console": "^5.2", "symfony/css-selector": "^4.4|^5.0", @@ -71,7 +71,7 @@ "phpdocumentor/reflection-docblock": "<3.0", "phpdocumentor/type-resolver": "<0.2.1", "phpunit/phpunit": "<5.4.3", - "symfony/asset": "<5.1", + "symfony/asset": "<5.3", "symfony/browser-kit": "<4.4", "symfony/console": "<5.2", "symfony/dotenv": "<5.1", diff --git a/src/Symfony/Component/Asset/CHANGELOG.md b/src/Symfony/Component/Asset/CHANGELOG.md index 9df5fc14d0..5fd63351cf 100644 --- a/src/Symfony/Component/Asset/CHANGELOG.md +++ b/src/Symfony/Component/Asset/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +5.3.0 +----- + + * deprecated `RemoteJsonManifestVersionStrategy`, use `JsonManifestVersionStrategy` instead. + 5.1.0 ----- diff --git a/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php b/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php index 7f2d44c4a2..924fb33686 100644 --- a/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php +++ b/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php @@ -13,47 +13,83 @@ namespace Symfony\Component\Asset\Tests\VersionStrategy; use PHPUnit\Framework\TestCase; use Symfony\Component\Asset\VersionStrategy\JsonManifestVersionStrategy; +use Symfony\Component\HttpClient\MockHttpClient; +use Symfony\Component\HttpClient\Response\MockResponse; class JsonManifestVersionStrategyTest extends TestCase { - public function testGetVersion() + /** + * @dataProvider ProvideValidStrategies + */ + public function testGetVersion(JsonManifestVersionStrategy $strategy) { - $strategy = $this->createStrategy('manifest-valid.json'); - $this->assertSame('main.123abc.js', $strategy->getVersion('main.js')); } - public function testApplyVersion() + /** + * @dataProvider ProvideValidStrategies + */ + public function testApplyVersion(JsonManifestVersionStrategy $strategy) { - $strategy = $this->createStrategy('manifest-valid.json'); - $this->assertSame('css/styles.555def.css', $strategy->applyVersion('css/styles.css')); } - public function testApplyVersionWhenKeyDoesNotExistInManifest() + /** + * @dataProvider ProvideValidStrategies + */ + public function testApplyVersionWhenKeyDoesNotExistInManifest(JsonManifestVersionStrategy $strategy) { - $strategy = $this->createStrategy('manifest-valid.json'); - $this->assertSame('css/other.css', $strategy->applyVersion('css/other.css')); } - public function testMissingManifestFileThrowsException() + /** + * @dataProvider ProvideMissingStrategies + */ + public function testMissingManifestFileThrowsException(JsonManifestVersionStrategy $strategy) { $this->expectException('RuntimeException'); - $strategy = $this->createStrategy('non-existent-file.json'); $strategy->getVersion('main.js'); } - public function testManifestFileWithBadJSONThrowsException() + /** + * @dataProvider ProvideInvalidStrategies + */ + public function testManifestFileWithBadJSONThrowsException(JsonManifestVersionStrategy $strategy) { $this->expectException('RuntimeException'); $this->expectExceptionMessage('Error parsing JSON'); - $strategy = $this->createStrategy('manifest-invalid.json'); $strategy->getVersion('main.js'); } - private function createStrategy($manifestFilename) + public function provideValidStrategies() { - return new JsonManifestVersionStrategy(__DIR__.'/../fixtures/'.$manifestFilename); + yield from $this->provideStrategies('manifest-valid.json'); + } + + public function provideInvalidStrategies() + { + yield from $this->provideStrategies('manifest-invalid.json'); + } + + public function provideMissingStrategies() + { + yield from $this->provideStrategies('non-existent-file.json'); + } + + public function provideStrategies(string $manifestPath) + { + $httpClient = new MockHttpClient(function ($method, $url, $options) { + $filename = __DIR__.'/../fixtures/'.basename($url); + + if (file_exists($filename)) { + return new MockResponse(file_get_contents($filename), ['http_headers' => ['content-type' => 'application/json']]); + } + + return new MockResponse('{}', ['http_code' => 404]); + }); + + yield [new JsonManifestVersionStrategy('https://cdn.example.com/'.$manifestPath, $httpClient)]; + + yield [new JsonManifestVersionStrategy(__DIR__.'/../fixtures/'.$manifestPath)]; } } diff --git a/src/Symfony/Component/Asset/Tests/VersionStrategy/RemoteJsonManifestVersionStrategyTest.php b/src/Symfony/Component/Asset/Tests/VersionStrategy/RemoteJsonManifestVersionStrategyTest.php index cc153c5487..1ea8f100e7 100644 --- a/src/Symfony/Component/Asset/Tests/VersionStrategy/RemoteJsonManifestVersionStrategyTest.php +++ b/src/Symfony/Component/Asset/Tests/VersionStrategy/RemoteJsonManifestVersionStrategyTest.php @@ -17,6 +17,9 @@ use Symfony\Component\HttpClient\Exception\JsonException; use Symfony\Component\HttpClient\MockHttpClient; use Symfony\Component\HttpClient\Response\MockResponse; +/** + * @group legacy + */ class RemoteJsonManifestVersionStrategyTest extends TestCase { public function testGetVersion() diff --git a/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php b/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php index c4de580d0b..3314d9e17c 100644 --- a/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php +++ b/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php @@ -11,6 +11,9 @@ namespace Symfony\Component\Asset\VersionStrategy; +use Symfony\Contracts\HttpClient\Exception\DecodingExceptionInterface; +use Symfony\Contracts\HttpClient\HttpClientInterface; + /** * Reads the versioned path of an asset from a JSON manifest file. * @@ -26,13 +29,15 @@ class JsonManifestVersionStrategy implements VersionStrategyInterface { private $manifestPath; private $manifestData; + private $httpClient; /** * @param string $manifestPath Absolute path to the manifest file */ - public function __construct(string $manifestPath) + public function __construct(string $manifestPath, HttpClientInterface $httpClient = null) { $this->manifestPath = $manifestPath; + $this->httpClient = $httpClient; } /** @@ -53,13 +58,23 @@ class JsonManifestVersionStrategy implements VersionStrategyInterface private function getManifestPath(string $path): ?string { if (null === $this->manifestData) { - if (!is_file($this->manifestPath)) { - throw new \RuntimeException(sprintf('Asset manifest file "%s" does not exist.', $this->manifestPath)); - } + if (null !== $this->httpClient && 0 === strpos(parse_url($this->manifestPath, \PHP_URL_SCHEME), 'http')) { + try { + $this->manifestData = $this->httpClient->request('GET', $this->manifestPath, [ + 'headers' => ['accept' => 'application/json'], + ])->toArray(); + } catch (DecodingExceptionInterface $e) { + throw new \RuntimeException(sprintf('Error parsing JSON from asset manifest URL "%s".', $this->manifestPath), 0, $e); + } + } else { + if (!is_file($this->manifestPath)) { + throw new \RuntimeException(sprintf('Asset manifest file "%s" does not exist.', $this->manifestPath)); + } - $this->manifestData = json_decode(file_get_contents($this->manifestPath), true); - if (0 < json_last_error()) { - throw new \RuntimeException(sprintf('Error parsing JSON from asset manifest file "%s": ', $this->manifestPath).json_last_error_msg()); + $this->manifestData = json_decode(file_get_contents($this->manifestPath), true); + if (0 < json_last_error()) { + throw new \RuntimeException(sprintf('Error parsing JSON from asset manifest file "%s": ', $this->manifestPath).json_last_error_msg()); + } } } diff --git a/src/Symfony/Component/Asset/VersionStrategy/RemoteJsonManifestVersionStrategy.php b/src/Symfony/Component/Asset/VersionStrategy/RemoteJsonManifestVersionStrategy.php index db45b3b7ec..cc6170a27e 100644 --- a/src/Symfony/Component/Asset/VersionStrategy/RemoteJsonManifestVersionStrategy.php +++ b/src/Symfony/Component/Asset/VersionStrategy/RemoteJsonManifestVersionStrategy.php @@ -13,6 +13,8 @@ namespace Symfony\Component\Asset\VersionStrategy; use Symfony\Contracts\HttpClient\HttpClientInterface; +trigger_deprecation('symfony/asset', '5.3', 'The "%s" class is deprecated, use "%s" instead.', RemoteJsonManifestVersionStrategy::class, JsonManifestVersionStrategy::class); + /** * Reads the versioned path of an asset from a remote JSON manifest file. * @@ -23,6 +25,8 @@ use Symfony\Contracts\HttpClient\HttpClientInterface; * } * * You could then ask for the version of "main.js" or "css/styles.css". + * + * @deprecated since Symfony 5.3, use JsonManifestVersionStrategy instead. */ class RemoteJsonManifestVersionStrategy implements VersionStrategyInterface { diff --git a/src/Symfony/Component/Asset/composer.json b/src/Symfony/Component/Asset/composer.json index 65295f8797..79b0b1d6e9 100644 --- a/src/Symfony/Component/Asset/composer.json +++ b/src/Symfony/Component/Asset/composer.json @@ -16,7 +16,8 @@ } ], "require": { - "php": ">=7.2.5" + "php": ">=7.2.5", + "symfony/deprecation-contracts": "^2.1" }, "suggest": { "symfony/http-foundation": ""