From e41cf49dbe395345f7ffe3970528c5e3e8e891f4 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 17 Mar 2017 15:10:06 -0400 Subject: [PATCH 01/14] [Asset][FrameworkBundle] Adding a new version strategy that reads from a JSON manifest file --- .../Bundle/FrameworkBundle/CHANGELOG.md | 2 + .../DependencyInjection/Configuration.php | 14 ++++ .../FrameworkExtension.php | 23 +++++- .../Resources/config/assets.xml | 4 + .../Resources/config/schema/symfony-1.0.xsd | 2 + .../Fixtures/php/assets.php | 4 + .../Fixtures/xml/assets.xml | 1 + .../Fixtures/yml/assets.yml | 3 + .../FrameworkExtensionTest.php | 7 +- src/Symfony/Component/Asset/CHANGELOG.md | 5 ++ .../JsonManifestVersionStrategyTest.php | 64 ++++++++++++++++ .../Tests/fixtures/manifest-invalid.json | 4 + .../Asset/Tests/fixtures/manifest-valid.json | 4 + .../JsonManifestVersionStrategy.php | 73 +++++++++++++++++++ 14 files changed, 207 insertions(+), 3 deletions(-) create mode 100644 src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php create mode 100644 src/Symfony/Component/Asset/Tests/fixtures/manifest-invalid.json create mode 100644 src/Symfony/Component/Asset/Tests/fixtures/manifest-valid.json create mode 100644 src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md index e86ca4ce2ea59..50f33d26bfd62 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md @@ -4,6 +4,8 @@ CHANGELOG 3.3.0 ----- + * A new version_strategy called json_manifest was added that reads a JSON manifest + to load the correct, versioned paths of assets. * Added support for the `controller.service_arguments` tag, for injecting services into controllers' actions * Deprecated `cache:clear` with warmup (always call it with `--no-warmup`) * Deprecated the "framework.trusted_proxies" configuration option and the corresponding "kernel.trusted_proxies" parameter diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index 8f63bd12cfa7d..8beb4bf191f8f 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -551,6 +551,7 @@ private function addAssetsSection(ArrayNodeDefinition $rootNode) ->scalarNode('version_strategy')->defaultNull()->end() ->scalarNode('version')->defaultNull()->end() ->scalarNode('version_format')->defaultValue('%%s?%%s')->end() + ->scalarNode('manifest_path')->defaultNull()->end() ->scalarNode('base_path')->defaultValue('')->end() ->arrayNode('base_urls') ->requiresAtLeastOneElement() @@ -567,6 +568,12 @@ private function addAssetsSection(ArrayNodeDefinition $rootNode) }) ->thenInvalid('You cannot use both "version_strategy" and "version" at the same time under "assets".') ->end() + ->validate() + ->ifTrue(function ($v) { + return isset($v['version_strategy']) && $v['version_strategy'] == 'json_manifest' && !isset($v['manifest_path']); + }) + ->thenInvalid('You must configure the "manifest_path" key in order to use the "json_manifest" "version_strategy".') + ->end() ->fixXmlConfig('package') ->children() ->arrayNode('packages') @@ -582,6 +589,7 @@ private function addAssetsSection(ArrayNodeDefinition $rootNode) ->end() ->end() ->scalarNode('version_format')->defaultNull()->end() + ->scalarNode('manifest_path')->defaultNull()->end() ->scalarNode('base_path')->defaultValue('')->end() ->arrayNode('base_urls') ->requiresAtLeastOneElement() @@ -598,6 +606,12 @@ private function addAssetsSection(ArrayNodeDefinition $rootNode) }) ->thenInvalid('You cannot use both "version_strategy" and "version" at the same time under "assets" packages.') ->end() + ->validate() + ->ifTrue(function ($v) { + return isset($v['version_strategy']) && $v['version_strategy'] == 'json_manifest' && !isset($v['manifest_path']); + }) + ->thenInvalid('You must configure the "manifest_path" key in order to use the "json_manifest" "version_strategy".') + ->end() ->end() ->end() ->end() diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 0edc31daac9e5..c9a352c10f2d7 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -808,7 +808,11 @@ private function registerAssetsConfiguration(array $config, ContainerBuilder $co $defaultVersion = null; if ($config['version_strategy']) { - $defaultVersion = new Reference($config['version_strategy']); + if ($config['version_strategy'] == 'json_manifest') { + $defaultVersion = $this->createJsonManifestVersion($container, $config['manifest_path'], '_default'); + } else { + $defaultVersion = new Reference($config['version_strategy']); + } } else { $defaultVersion = $this->createVersion($container, $config['version'], $config['version_format'], '_default'); } @@ -819,7 +823,11 @@ private function registerAssetsConfiguration(array $config, ContainerBuilder $co $namedPackages = array(); foreach ($config['packages'] as $name => $package) { if (null !== $package['version_strategy']) { - $version = new Reference($package['version_strategy']); + if ($package['version_strategy'] == 'json_manifest') { + $version = $this->createJsonManifestVersion($container, $package['manifest_path'], $name); + } else { + $version = new Reference($package['version_strategy']); + } } elseif (!array_key_exists('version', $package)) { $version = $defaultVersion; } else { @@ -872,6 +880,17 @@ private function createVersion(ContainerBuilder $container, $version, $format, $ return new Reference('assets._version_'.$name); } + private function createJsonManifestVersion(ContainerBuilder $container, $manifestPath, $name) + { + $def = new ChildDefinition('assets.json_manifest_version_strategy'); + $def + ->replaceArgument(0, $manifestPath) + ; + $container->setDefinition('assets._version_'.$name, $def); + + return new Reference('assets._version_'.$name); + } + /** * Loads the translator configuration. * diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/assets.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/assets.xml index 028a3d5d6af2d..385b7e307040d 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/assets.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/assets.xml @@ -39,6 +39,10 @@ + + + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd index 2ec8540ef213a..b8818cdd33a58 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd @@ -131,6 +131,7 @@ + @@ -143,6 +144,7 @@ + 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 3cfc5c1bc0cc4..92ec3b1ececbd 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/assets.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/assets.php @@ -24,6 +24,10 @@ 'base_urls' => array('https://bar2.example.com'), 'version_strategy' => 'assets.custom_version_strategy', ), + 'json_manifest_strategy' => array( + 'version_strategy' => 'json_manifest', + 'manifest_path' => '/path/to/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 b99952b2923d3..fb075474a2b51 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/assets.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/assets.xml @@ -21,6 +21,7 @@ https://bar_version_strategy.example.com + 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 73cd9234e7060..d34154e8317ae 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/assets.yml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/assets.yml @@ -17,3 +17,6 @@ framework: bar_version_strategy: base_urls: ["https://bar_version_strategy.example.com"] version_strategy: assets.custom_version_strategy + json_manifest_strategy: + version_strategy: json_manifest + manifest_path: '/path/to/manifest.json' diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index e3956b1fb576d..51f97a59a248d 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -374,7 +374,7 @@ public function testAssets() // packages $packages = $packages->getArgument(1); - $this->assertCount(5, $packages); + $this->assertCount(6, $packages); $package = $container->getDefinition((string) $packages['images_path']); $this->assertPathPackage($container, $package, '/foo', 'SomeVersionScheme', '%%s?version=%%s'); @@ -390,6 +390,11 @@ public function testAssets() $package = $container->getDefinition((string) $packages['bar_version_strategy']); $this->assertEquals('assets.custom_version_strategy', (string) $package->getArgument(1)); + + $package = $container->getDefinition((string) $packages['json_manifest_strategy']); + $versionStrategy = $container->getDefinition((string) $package->getArgument(1)); + $this->assertEquals('assets.json_manifest_version_strategy', $versionStrategy->getParent()); + $this->assertEquals('/path/to/manifest.json', $versionStrategy->getArgument(0)); } public function testAssetsDefaultVersionStrategyAsService() diff --git a/src/Symfony/Component/Asset/CHANGELOG.md b/src/Symfony/Component/Asset/CHANGELOG.md index 619a423402ada..56d722d55b4f0 100644 --- a/src/Symfony/Component/Asset/CHANGELOG.md +++ b/src/Symfony/Component/Asset/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +3.3.0 +----- + * JsonManifestVersionStrategy was added as a way to read final, + versioned paths from a JSON manifest file. + 2.7.0 ----- diff --git a/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php b/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php new file mode 100644 index 0000000000000..803bd84156fb5 --- /dev/null +++ b/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php @@ -0,0 +1,64 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Asset\Tests\VersionStrategy; + +use PHPUnit\Framework\TestCase; +use Symfony\Component\Asset\VersionStrategy\JsonManifestVersionStrategy; + +class JsonManifestVersionStrategyTest extends TestCase +{ + public function testGetVersion() + { + $strategy = $this->createStrategy('manifest-valid.json'); + + $this->assertEquals('main.123abc.js', $strategy->getVersion('main.js')); + } + + public function testApplyVersion() + { + $strategy = $this->createStrategy('manifest-valid.json'); + + $this->assertEquals('css/styles.555def.css', $strategy->getVersion('css/styles.css')); + } + + public function testApplyVersionWhenKeyDoesNotExistInManifest() + { + $strategy = $this->createStrategy('manifest-valid.json'); + + $this->assertEquals('css/other.css', $strategy->getVersion('css/other.css')); + } + + /** + * @expectedException \RuntimeException + */ + public function testMissingManifestFileThrowsException() + { + $strategy = $this->createStrategy('non-existent-file.json'); + $strategy->getVersion('main.js'); + } + + /** + * @expectedException \RuntimeException + * @expectedExceptionMessage Error parsing JSON + */ + public function testManifestFileWithBadJSONThrowsException() + { + $strategy = $this->createStrategy('manifest-invalid.json'); + $strategy->getVersion('main.js'); + + } + + private function createStrategy($manifestFilename) + { + return new JsonManifestVersionStrategy(__DIR__.'/../fixtures/'.$manifestFilename); + } +} diff --git a/src/Symfony/Component/Asset/Tests/fixtures/manifest-invalid.json b/src/Symfony/Component/Asset/Tests/fixtures/manifest-invalid.json new file mode 100644 index 0000000000000..949af54eacb9d --- /dev/null +++ b/src/Symfony/Component/Asset/Tests/fixtures/manifest-invalid.json @@ -0,0 +1,4 @@ +{ + "main.js": main.123abc.js", + "css/styles.css": "css/styles.555def.css" +} \ No newline at end of file diff --git a/src/Symfony/Component/Asset/Tests/fixtures/manifest-valid.json b/src/Symfony/Component/Asset/Tests/fixtures/manifest-valid.json new file mode 100644 index 0000000000000..0d72efd6d5ada --- /dev/null +++ b/src/Symfony/Component/Asset/Tests/fixtures/manifest-valid.json @@ -0,0 +1,4 @@ +{ + "main.js": "main.123abc.js", + "css/styles.css": "css/styles.555def.css" +} \ No newline at end of file diff --git a/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php b/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php new file mode 100644 index 0000000000000..d37b6095ff5a7 --- /dev/null +++ b/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php @@ -0,0 +1,73 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Asset\VersionStrategy; + +use Symfony\Component\Asset\Exception\MissingAssetManifestException; + +/** + * Reads the versioned path of an asset from a JSON manifest file. + * + * For example, the manifest file might look like this: + * { + * "main.js": "main.abc123.js", + * "css/styles.css": "css/styles.555abc.css" + * } + * + * You could then as for the version of "main.js" or "css/styles.css". + */ +class JsonManifestVersionStrategy implements VersionStrategyInterface +{ + private $manifestPath; + + private $manifestData; + + public function __construct($manifestPath) + { + $this->manifestPath = $manifestPath; + } + + /** + * With a manifest, we don't really know or care about what + * the version is. Instead, this returns the path to the + * versioned file. + * + * @param string $path + * @return string + */ + public function getVersion($path) + { + return $this->applyVersion($path); + } + + public function applyVersion($path) + { + $manifestPath = $this->getManifestPath($path); + + return $manifestPath ? $manifestPath : $path; + } + + private function getManifestPath($path) + { + if (null === $this->manifestData) { + if (!file_exists($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 < $errorCode = json_last_error()) { + throw new \RuntimeException(sprintf('Error parsing JSON from asset manifest file "%s" - %s', $this->manifestPath, json_last_error_msg())); + } + } + + return isset($this->manifestData[$path]) ? $this->manifestData[$path] : null; + } +} From 1a3d465c4d0b5688eb98aeee31619f335a5462ea Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 17 Mar 2017 15:26:55 -0400 Subject: [PATCH 02/14] fabbot --- .../Tests/DependencyInjection/Fixtures/php/assets.php | 2 +- .../Tests/VersionStrategy/JsonManifestVersionStrategyTest.php | 1 - .../Asset/VersionStrategy/JsonManifestVersionStrategy.php | 3 +-- 3 files changed, 2 insertions(+), 4 deletions(-) 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 92ec3b1ececbd..f79b1dfd44dfe 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/assets.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/assets.php @@ -26,7 +26,7 @@ ), 'json_manifest_strategy' => array( 'version_strategy' => 'json_manifest', - 'manifest_path' => '/path/to/manifest.json' + 'manifest_path' => '/path/to/manifest.json', ), ), ), diff --git a/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php b/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php index 803bd84156fb5..9da2b4ada2856 100644 --- a/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php +++ b/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php @@ -54,7 +54,6 @@ public function testManifestFileWithBadJSONThrowsException() { $strategy = $this->createStrategy('manifest-invalid.json'); $strategy->getVersion('main.js'); - } private function createStrategy($manifestFilename) diff --git a/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php b/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php index d37b6095ff5a7..cc216d7767f30 100644 --- a/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php +++ b/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php @@ -11,8 +11,6 @@ namespace Symfony\Component\Asset\VersionStrategy; -use Symfony\Component\Asset\Exception\MissingAssetManifestException; - /** * Reads the versioned path of an asset from a JSON manifest file. * @@ -41,6 +39,7 @@ public function __construct($manifestPath) * versioned file. * * @param string $path + * * @return string */ public function getVersion($path) From 6d1954940d2a2a3d5db74e952d000279c0bab4ad Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 17 Mar 2017 15:34:05 -0400 Subject: [PATCH 03/14] Removing .json from extension of a file to work around fabbot not liking the invalid JSON --- .../Tests/VersionStrategy/JsonManifestVersionStrategyTest.php | 2 +- .../{manifest-invalid.json => manifest-invalid.json.invalid} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename src/Symfony/Component/Asset/Tests/fixtures/{manifest-invalid.json => manifest-invalid.json.invalid} (100%) diff --git a/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php b/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php index 9da2b4ada2856..8c2189fdb7c44 100644 --- a/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php +++ b/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php @@ -52,7 +52,7 @@ public function testMissingManifestFileThrowsException() */ public function testManifestFileWithBadJSONThrowsException() { - $strategy = $this->createStrategy('manifest-invalid.json'); + $strategy = $this->createStrategy('manifest-invalid.json.invalid'); $strategy->getVersion('main.js'); } diff --git a/src/Symfony/Component/Asset/Tests/fixtures/manifest-invalid.json b/src/Symfony/Component/Asset/Tests/fixtures/manifest-invalid.json.invalid similarity index 100% rename from src/Symfony/Component/Asset/Tests/fixtures/manifest-invalid.json rename to src/Symfony/Component/Asset/Tests/fixtures/manifest-invalid.json.invalid From 36fa8b31e2f896ceafc4142a437d536e3b03d27a Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 17 Mar 2017 15:36:44 -0400 Subject: [PATCH 04/14] Fixing a test --- .../Tests/DependencyInjection/ConfigurationTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php index 6c7a1fd757052..09958a88acfe1 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php @@ -120,6 +120,7 @@ public function testAssetsCanBeEnabled() 'base_path' => '', 'base_urls' => array(), 'packages' => array(), + 'manifest_path' => null, ); $this->assertEquals($defaultConfig, $config['assets']); @@ -274,6 +275,7 @@ protected static function getBundleDefaultConfig() 'base_path' => '', 'base_urls' => array(), 'packages' => array(), + 'manifest_path' => null, ), 'cache' => array( 'pools' => array(), From 0a9914336c9ddb8f7fba2630023e6abbe03c9f26 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 17 Mar 2017 17:39:19 -0400 Subject: [PATCH 05/14] == => === --- .../FrameworkBundle/DependencyInjection/Configuration.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index 8beb4bf191f8f..9ddede28a6bda 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -570,7 +570,7 @@ private function addAssetsSection(ArrayNodeDefinition $rootNode) ->end() ->validate() ->ifTrue(function ($v) { - return isset($v['version_strategy']) && $v['version_strategy'] == 'json_manifest' && !isset($v['manifest_path']); + return isset($v['version_strategy']) && $v['version_strategy'] === 'json_manifest' && !isset($v['manifest_path']); }) ->thenInvalid('You must configure the "manifest_path" key in order to use the "json_manifest" "version_strategy".') ->end() @@ -608,7 +608,7 @@ private function addAssetsSection(ArrayNodeDefinition $rootNode) ->end() ->validate() ->ifTrue(function ($v) { - return isset($v['version_strategy']) && $v['version_strategy'] == 'json_manifest' && !isset($v['manifest_path']); + return isset($v['version_strategy']) && $v['version_strategy'] === 'json_manifest' && !isset($v['manifest_path']); }) ->thenInvalid('You must configure the "manifest_path" key in order to use the "json_manifest" "version_strategy".') ->end() From b2c7d3239bc039289da17bfd27bbebe962dec56f Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 17 Mar 2017 17:40:19 -0400 Subject: [PATCH 06/14] Another attempt at faking out fabbot. Quick fabbot - look over there! (fabbot dislikes invaild JSON) --- .../Tests/VersionStrategy/JsonManifestVersionStrategyTest.php | 2 +- .../{manifest-invalid.json.invalid => manifest-invalid-json} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename src/Symfony/Component/Asset/Tests/fixtures/{manifest-invalid.json.invalid => manifest-invalid-json} (100%) diff --git a/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php b/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php index 8c2189fdb7c44..2980ab23c601a 100644 --- a/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php +++ b/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php @@ -52,7 +52,7 @@ public function testMissingManifestFileThrowsException() */ public function testManifestFileWithBadJSONThrowsException() { - $strategy = $this->createStrategy('manifest-invalid.json.invalid'); + $strategy = $this->createStrategy('manifest-invalid-json'); $strategy->getVersion('main.js'); } diff --git a/src/Symfony/Component/Asset/Tests/fixtures/manifest-invalid.json.invalid b/src/Symfony/Component/Asset/Tests/fixtures/manifest-invalid-json similarity index 100% rename from src/Symfony/Component/Asset/Tests/fixtures/manifest-invalid.json.invalid rename to src/Symfony/Component/Asset/Tests/fixtures/manifest-invalid-json From 9ed9c57c1da636f049bf23d954479f5fef9b1a11 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Wed, 22 Mar 2017 17:35:28 -0400 Subject: [PATCH 07/14] Line breaks and not being clever with invalid - just ending with .json --- .../Tests/VersionStrategy/JsonManifestVersionStrategyTest.php | 2 +- .../fixtures/{manifest-invalid-json => manifest-invalid.json} | 2 +- src/Symfony/Component/Asset/Tests/fixtures/manifest-valid.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename src/Symfony/Component/Asset/Tests/fixtures/{manifest-invalid-json => manifest-invalid.json} (97%) diff --git a/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php b/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php index 2980ab23c601a..9da2b4ada2856 100644 --- a/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php +++ b/src/Symfony/Component/Asset/Tests/VersionStrategy/JsonManifestVersionStrategyTest.php @@ -52,7 +52,7 @@ public function testMissingManifestFileThrowsException() */ public function testManifestFileWithBadJSONThrowsException() { - $strategy = $this->createStrategy('manifest-invalid-json'); + $strategy = $this->createStrategy('manifest-invalid.json'); $strategy->getVersion('main.js'); } diff --git a/src/Symfony/Component/Asset/Tests/fixtures/manifest-invalid-json b/src/Symfony/Component/Asset/Tests/fixtures/manifest-invalid.json similarity index 97% rename from src/Symfony/Component/Asset/Tests/fixtures/manifest-invalid-json rename to src/Symfony/Component/Asset/Tests/fixtures/manifest-invalid.json index 949af54eacb9d..feed937ea1215 100644 --- a/src/Symfony/Component/Asset/Tests/fixtures/manifest-invalid-json +++ b/src/Symfony/Component/Asset/Tests/fixtures/manifest-invalid.json @@ -1,4 +1,4 @@ { "main.js": main.123abc.js", "css/styles.css": "css/styles.555def.css" -} \ No newline at end of file +} diff --git a/src/Symfony/Component/Asset/Tests/fixtures/manifest-valid.json b/src/Symfony/Component/Asset/Tests/fixtures/manifest-valid.json index 0d72efd6d5ada..546a0066d31ee 100644 --- a/src/Symfony/Component/Asset/Tests/fixtures/manifest-valid.json +++ b/src/Symfony/Component/Asset/Tests/fixtures/manifest-valid.json @@ -1,4 +1,4 @@ { "main.js": "main.123abc.js", "css/styles.css": "css/styles.555def.css" -} \ No newline at end of file +} From aa3289609903545c5228768eb7e4d4d421b49323 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Wed, 22 Mar 2017 17:45:09 -0400 Subject: [PATCH 08/14] Tweaks thanks to Stof! --- .../DependencyInjection/FrameworkExtension.php | 5 ++--- .../Asset/VersionStrategy/JsonManifestVersionStrategy.php | 7 +++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index c9a352c10f2d7..607e42a1f9881 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -883,9 +883,8 @@ private function createVersion(ContainerBuilder $container, $version, $format, $ private function createJsonManifestVersion(ContainerBuilder $container, $manifestPath, $name) { $def = new ChildDefinition('assets.json_manifest_version_strategy'); - $def - ->replaceArgument(0, $manifestPath) - ; + $def->replaceArgument(0, $manifestPath); + $def->setPublic(false); $container->setDefinition('assets._version_'.$name, $def); return new Reference('assets._version_'.$name); diff --git a/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php b/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php index cc216d7767f30..e3dfc76755d99 100644 --- a/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php +++ b/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php @@ -28,6 +28,9 @@ class JsonManifestVersionStrategy implements VersionStrategyInterface private $manifestData; + /** + * @param string $manifestPath Absolute path to the manifest file. + */ public function __construct($manifestPath) { $this->manifestPath = $manifestPath; @@ -51,7 +54,7 @@ public function applyVersion($path) { $manifestPath = $this->getManifestPath($path); - return $manifestPath ? $manifestPath : $path; + return $manifestPath ?: $path; } private function getManifestPath($path) @@ -62,7 +65,7 @@ private function getManifestPath($path) } $this->manifestData = json_decode(file_get_contents($this->manifestPath), true); - if (0 < $errorCode = json_last_error()) { + if (0 < json_last_error()) { throw new \RuntimeException(sprintf('Error parsing JSON from asset manifest file "%s" - %s', $this->manifestPath, json_last_error_msg())); } } From 99251c355dc0afc7ad92567432b7b6d8b9b83ee6 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Thu, 23 Mar 2017 06:48:40 -0400 Subject: [PATCH 09/14] Reworking feature so that json_manifest_path is the config key, and is all that's needed to activate This keeps the original behavior of version_strategy: this is always a service id --- .../Bundle/FrameworkBundle/CHANGELOG.md | 4 +- .../DependencyInjection/Configuration.php | 24 ++++-- .../FrameworkExtension.php | 59 +++++++------- .../Resources/config/schema/symfony-1.0.xsd | 4 +- .../DependencyInjection/ConfigurationTest.php | 80 ++++++++++--------- .../Fixtures/php/assets.php | 3 +- .../Fixtures/xml/assets.xml | 2 +- .../Fixtures/yml/assets.yml | 3 +- 8 files changed, 96 insertions(+), 83 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md index 50f33d26bfd62..0203fcbd692eb 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md @@ -4,8 +4,8 @@ CHANGELOG 3.3.0 ----- - * A new version_strategy called json_manifest was added that reads a JSON manifest - to load the correct, versioned paths of assets. + * A new version strategy option called json_manifest_path was added + that allows you to use the JsonManifestVersionStrategy. * Added support for the `controller.service_arguments` tag, for injecting services into controllers' actions * Deprecated `cache:clear` with warmup (always call it with `--no-warmup`) * Deprecated the "framework.trusted_proxies" configuration option and the corresponding "kernel.trusted_proxies" parameter diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index 9ddede28a6bda..de597f113b196 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -551,7 +551,7 @@ private function addAssetsSection(ArrayNodeDefinition $rootNode) ->scalarNode('version_strategy')->defaultNull()->end() ->scalarNode('version')->defaultNull()->end() ->scalarNode('version_format')->defaultValue('%%s?%%s')->end() - ->scalarNode('manifest_path')->defaultNull()->end() + ->scalarNode('json_manifest_path')->defaultNull()->end() ->scalarNode('base_path')->defaultValue('')->end() ->arrayNode('base_urls') ->requiresAtLeastOneElement() @@ -570,9 +570,15 @@ private function addAssetsSection(ArrayNodeDefinition $rootNode) ->end() ->validate() ->ifTrue(function ($v) { - return isset($v['version_strategy']) && $v['version_strategy'] === 'json_manifest' && !isset($v['manifest_path']); + return isset($v['version_strategy']) && isset($v['json_manifest_path']); }) - ->thenInvalid('You must configure the "manifest_path" key in order to use the "json_manifest" "version_strategy".') + ->thenInvalid('You cannot use both "version_strategy" and "json_manifest_path" at the same time under "assets".') + ->end() + ->validate() + ->ifTrue(function ($v) { + return isset($v['version']) && isset($v['json_manifest_path']); + }) + ->thenInvalid('You cannot use both "version" and "json_manifest_path" at the same time under "assets".') ->end() ->fixXmlConfig('package') ->children() @@ -589,7 +595,7 @@ private function addAssetsSection(ArrayNodeDefinition $rootNode) ->end() ->end() ->scalarNode('version_format')->defaultNull()->end() - ->scalarNode('manifest_path')->defaultNull()->end() + ->scalarNode('json_manifest_path')->defaultNull()->end() ->scalarNode('base_path')->defaultValue('')->end() ->arrayNode('base_urls') ->requiresAtLeastOneElement() @@ -608,9 +614,15 @@ private function addAssetsSection(ArrayNodeDefinition $rootNode) ->end() ->validate() ->ifTrue(function ($v) { - return isset($v['version_strategy']) && $v['version_strategy'] === 'json_manifest' && !isset($v['manifest_path']); + return isset($v['version_strategy']) && isset($v['json_manifest_path']); + }) + ->thenInvalid('You cannot use both "version_strategy" and "json_manifest_path" at the same time under "assets" packages.') + ->end() + ->validate() + ->ifTrue(function ($v) { + return isset($v['version']) && isset($v['json_manifest_path']); }) - ->thenInvalid('You must configure the "manifest_path" key in order to use the "json_manifest" "version_strategy".') + ->thenInvalid('You cannot use both "version" and "json_manifest_path" at the same time under "assets" packages.') ->end() ->end() ->end() diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 607e42a1f9881..f39e25db2eef4 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -808,13 +808,9 @@ private function registerAssetsConfiguration(array $config, ContainerBuilder $co $defaultVersion = null; if ($config['version_strategy']) { - if ($config['version_strategy'] == 'json_manifest') { - $defaultVersion = $this->createJsonManifestVersion($container, $config['manifest_path'], '_default'); - } else { - $defaultVersion = new Reference($config['version_strategy']); - } + $defaultVersion = new Reference($config['version_strategy']); } else { - $defaultVersion = $this->createVersion($container, $config['version'], $config['version_format'], '_default'); + $defaultVersion = $this->createVersion($container, $config['version'], $config['version_format'], $config['json_manifest_path'], '_default'); } $defaultPackage = $this->createPackageDefinition($config['base_path'], $config['base_urls'], $defaultVersion); @@ -823,16 +819,15 @@ private function registerAssetsConfiguration(array $config, ContainerBuilder $co $namedPackages = array(); foreach ($config['packages'] as $name => $package) { if (null !== $package['version_strategy']) { - if ($package['version_strategy'] == 'json_manifest') { - $version = $this->createJsonManifestVersion($container, $package['manifest_path'], $name); - } else { - $version = new Reference($package['version_strategy']); - } - } elseif (!array_key_exists('version', $package)) { + $version = new Reference($package['version_strategy']); + } elseif (!array_key_exists('version', $package) && null === $package['json_manifest_path']) { + // if neither version nor json_manifest_path are specified, use the default $version = $defaultVersion; } else { + // let format fallback to main version_format $format = $package['version_format'] ?: $config['version_format']; - $version = $this->createVersion($container, $package['version'], $format, $name); + $version = isset($package['version']) ? $package['version'] : null; + $version = $this->createVersion($container, $version, $format, $package['json_manifest_path'], $name); } $container->setDefinition('assets._package_'.$name, $this->createPackageDefinition($package['base_path'], $package['base_urls'], $version)); @@ -864,30 +859,30 @@ private function createPackageDefinition($basePath, array $baseUrls, Reference $ return $package; } - private function createVersion(ContainerBuilder $container, $version, $format, $name) + private function createVersion(ContainerBuilder $container, $version, $format, $jsonManifestPath, $name) { - if (null === $version) { - return new Reference('assets.empty_version_strategy'); - } + // Configuration prevents $version and $jsonManifestPath from being set + if (null !== $version) { + $def = new ChildDefinition('assets.static_version_strategy'); + $def + ->replaceArgument(0, $version) + ->replaceArgument(1, $format) + ; + $container->setDefinition('assets._version_'.$name, $def); - $def = new ChildDefinition('assets.static_version_strategy'); - $def - ->replaceArgument(0, $version) - ->replaceArgument(1, $format) - ; - $container->setDefinition('assets._version_'.$name, $def); + return new Reference('assets._version_'.$name); + } - return new Reference('assets._version_'.$name); - } + if (null !== $jsonManifestPath) { + $def = new ChildDefinition('assets.json_manifest_version_strategy'); + $def->replaceArgument(0, $jsonManifestPath); + $def->setPublic(false); + $container->setDefinition('assets._version_'.$name, $def); - private function createJsonManifestVersion(ContainerBuilder $container, $manifestPath, $name) - { - $def = new ChildDefinition('assets.json_manifest_version_strategy'); - $def->replaceArgument(0, $manifestPath); - $def->setPublic(false); - $container->setDefinition('assets._version_'.$name, $def); + return new Reference('assets._version_'.$name); + } - return new Reference('assets._version_'.$name); + return new Reference('assets.empty_version_strategy'); } /** diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd index b8818cdd33a58..fc324c24bbff8 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd @@ -131,7 +131,7 @@ - + @@ -144,7 +144,7 @@ - + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php index 09958a88acfe1..79756e0f7ae6c 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php @@ -14,6 +14,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Bundle\FrameworkBundle\DependencyInjection\Configuration; use Symfony\Bundle\FullStack; +use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\Config\Definition\Processor; class ConfigurationTest extends TestCase @@ -120,55 +121,62 @@ public function testAssetsCanBeEnabled() 'base_path' => '', 'base_urls' => array(), 'packages' => array(), - 'manifest_path' => null, + 'json_manifest_path' => null, ); $this->assertEquals($defaultConfig, $config['assets']); } /** - * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException - * @expectedExceptionMessage You cannot use both "version_strategy" and "version" at the same time under "assets". + * @dataProvider provideInvalidAssetConfigurationTests */ - public function testInvalidVersionStrategy() + public function testInvalidAssetsConfiguration(array $assetConfig, $expectedMessage) { + $this->expectException(InvalidConfigurationException::class); + $this->expectExceptionMessage($expectedMessage); + $processor = new Processor(); - $configuration = new Configuration(true); - $processor->processConfiguration($configuration, array( - array( - 'assets' => array( - 'base_urls' => '//example.com', - 'version' => 1, - 'version_strategy' => 'foo', + $configuration = new Configuration(true); + $processor->processConfiguration($configuration, array( + array( + 'assets' => $assetConfig ), - ), - )); + )); } - /** - * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException - * @expectedExceptionMessage You cannot use both "version_strategy" and "version" at the same time under "assets" packages. - */ - public function testInvalidPackageVersionStrategy() + public function provideInvalidAssetConfigurationTests() { - $processor = new Processor(); - $configuration = new Configuration(true); - - $processor->processConfiguration($configuration, array( - array( - 'assets' => array( - 'base_urls' => '//example.com', - 'version' => 1, - 'packages' => array( - 'foo' => array( - 'base_urls' => '//example.com', - 'version' => 1, - 'version_strategy' => 'foo', - ), - ), + // helper to turn config into embedded package config + $createPackageConfig = function(array $packageConfig) { + return array( + 'base_urls' => '//example.com', + 'version' => 1, + 'packages' => array( + 'foo' => $packageConfig, ), - ), - )); + ); + }; + + $config = array( + 'version' => 1, + 'version_strategy' => 'foo', + ); + yield array($config, 'You cannot use both "version_strategy" and "version" at the same time under "assets".'); + yield array($createPackageConfig($config), 'You cannot use both "version_strategy" and "version" at the same time under "assets" packages.'); + + $config = array( + 'json_manifest_path' => '/foo.json', + 'version_strategy' => 'foo', + ); + yield array($config, 'You cannot use both "version_strategy" and "json_manifest_path" at the same time under "assets".'); + yield array($createPackageConfig($config), 'You cannot use both "version_strategy" and "json_manifest_path" at the same time under "assets" packages.'); + + $config = array( + 'json_manifest_path' => '/foo.json', + 'version' => '1', + ); + yield array($config, 'You cannot use both "version" and "json_manifest_path" at the same time under "assets".'); + yield array($createPackageConfig($config), 'You cannot use both "version" and "json_manifest_path" at the same time under "assets" packages.'); } protected static function getBundleDefaultConfig() @@ -275,7 +283,7 @@ protected static function getBundleDefaultConfig() 'base_path' => '', 'base_urls' => array(), 'packages' => array(), - 'manifest_path' => null, + 'json_manifest_path' => null, ), 'cache' => array( 'pools' => array(), 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 f79b1dfd44dfe..dc6bf7bb8df55 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/assets.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/assets.php @@ -25,8 +25,7 @@ 'version_strategy' => 'assets.custom_version_strategy', ), 'json_manifest_strategy' => array( - 'version_strategy' => 'json_manifest', - 'manifest_path' => '/path/to/manifest.json', + 'json_manifest_path' => '/path/to/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 fb075474a2b51..a907a5b967f93 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/assets.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/assets.xml @@ -21,7 +21,7 @@ https://bar_version_strategy.example.com - + 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 d34154e8317ae..a1679e389ddbf 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/assets.yml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/assets.yml @@ -18,5 +18,4 @@ framework: base_urls: ["https://bar_version_strategy.example.com"] version_strategy: assets.custom_version_strategy json_manifest_strategy: - version_strategy: json_manifest - manifest_path: '/path/to/manifest.json' + json_manifest_path: '/path/to/manifest.json' From bebf6743460a9abc80e21dbcbb2e4217b3a039b2 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Thu, 23 Mar 2017 07:08:06 -0400 Subject: [PATCH 10/14] Thanks fabbot! --- .../Tests/DependencyInjection/ConfigurationTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php index 79756e0f7ae6c..bb18d678b59f1 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php @@ -136,10 +136,10 @@ public function testInvalidAssetsConfiguration(array $assetConfig, $expectedMess $this->expectExceptionMessage($expectedMessage); $processor = new Processor(); - $configuration = new Configuration(true); - $processor->processConfiguration($configuration, array( + $configuration = new Configuration(true); + $processor->processConfiguration($configuration, array( array( - 'assets' => $assetConfig + 'assets' => $assetConfig, ), )); } @@ -147,7 +147,7 @@ public function testInvalidAssetsConfiguration(array $assetConfig, $expectedMess public function provideInvalidAssetConfigurationTests() { // helper to turn config into embedded package config - $createPackageConfig = function(array $packageConfig) { + $createPackageConfig = function (array $packageConfig) { return array( 'base_urls' => '//example.com', 'version' => 1, From 5955f1770f1f9cb8e85932f85630748d4eaca836 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Thu, 23 Mar 2017 07:18:55 -0400 Subject: [PATCH 11/14] Making test compatible with older phpunit versions --- .../Tests/DependencyInjection/ConfigurationTest.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php index bb18d678b59f1..38fec3818471b 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php @@ -132,8 +132,12 @@ public function testAssetsCanBeEnabled() */ public function testInvalidAssetsConfiguration(array $assetConfig, $expectedMessage) { - $this->expectException(InvalidConfigurationException::class); - $this->expectExceptionMessage($expectedMessage); + $this->{method_exists($this, $_ = 'expectException') ? $_ : 'setExpectedException'}( + InvalidConfigurationException::class + ); + $this->{method_exists($this, $_ = 'expectExceptionMessage') ? $_ : 'setExpectedExceptionMessage'}( + $expectedMessage + ); $processor = new Processor(); $configuration = new Configuration(true); From 14d50e155746a39919fc314f152802a1f0f44fba Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Thu, 23 Mar 2017 09:03:58 -0400 Subject: [PATCH 12/14] Fixing test - the old way is all inside setExpectedException, the new way is in 2 method calls --- .../Tests/DependencyInjection/ConfigurationTest.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php index 38fec3818471b..99e45584e76f4 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php @@ -133,11 +133,12 @@ public function testAssetsCanBeEnabled() public function testInvalidAssetsConfiguration(array $assetConfig, $expectedMessage) { $this->{method_exists($this, $_ = 'expectException') ? $_ : 'setExpectedException'}( - InvalidConfigurationException::class - ); - $this->{method_exists($this, $_ = 'expectExceptionMessage') ? $_ : 'setExpectedExceptionMessage'}( + InvalidConfigurationException::class, $expectedMessage ); + if (method_exists($this, 'expectExceptionMessage')) { + $this->expectExceptionMessage($expectedMessage); + } $processor = new Processor(); $configuration = new Configuration(true); From ff8869a36f593173c9e8bdfa333f312d3d649530 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Thu, 23 Mar 2017 14:18:51 -0400 Subject: [PATCH 13/14] Tweaks thanks to fabpot --- .php_cs.dist | 2 ++ src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md | 2 +- src/Symfony/Component/Asset/CHANGELOG.md | 2 +- .../VersionStrategy/JsonManifestVersionStrategy.php | 9 ++------- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/.php_cs.dist b/.php_cs.dist index 71c4a35f5883b..ee298bd15678e 100644 --- a/.php_cs.dist +++ b/.php_cs.dist @@ -37,5 +37,7 @@ return PhpCsFixer\Config::create() ->notPath('Symfony/Bundle/FrameworkBundle/Tests/Templating/Helper/Resources/Custom/_name_entry_label.html.php') // explicit heredoc test ->notPath('Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Resources/views/translation.html.php') + // purposefully invalid JSON + ->notPath('Symfony/Component/Asset/Tests/fixtures/manifest-invalid.json') ) ; diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md index 0203fcbd692eb..64fe428e9b252 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md @@ -4,7 +4,7 @@ CHANGELOG 3.3.0 ----- - * A new version strategy option called json_manifest_path was added + * Added a new new version strategy option called json_manifest_path that allows you to use the JsonManifestVersionStrategy. * Added support for the `controller.service_arguments` tag, for injecting services into controllers' actions * Deprecated `cache:clear` with warmup (always call it with `--no-warmup`) diff --git a/src/Symfony/Component/Asset/CHANGELOG.md b/src/Symfony/Component/Asset/CHANGELOG.md index 56d722d55b4f0..aa0b32cfa89f8 100644 --- a/src/Symfony/Component/Asset/CHANGELOG.md +++ b/src/Symfony/Component/Asset/CHANGELOG.md @@ -3,7 +3,7 @@ CHANGELOG 3.3.0 ----- - * JsonManifestVersionStrategy was added as a way to read final, + * Added JsonManifestVersionStrategy as a way to read final, versioned paths from a JSON manifest file. 2.7.0 diff --git a/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php b/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php index e3dfc76755d99..19ab039de364e 100644 --- a/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php +++ b/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php @@ -20,16 +20,15 @@ * "css/styles.css": "css/styles.555abc.css" * } * - * You could then as for the version of "main.js" or "css/styles.css". + * You could then ask for the version of "main.js" or "css/styles.css". */ class JsonManifestVersionStrategy implements VersionStrategyInterface { private $manifestPath; - private $manifestData; /** - * @param string $manifestPath Absolute path to the manifest file. + * @param string $manifestPath Absolute path to the manifest file */ public function __construct($manifestPath) { @@ -40,10 +39,6 @@ public function __construct($manifestPath) * With a manifest, we don't really know or care about what * the version is. Instead, this returns the path to the * versioned file. - * - * @param string $path - * - * @return string */ public function getVersion($path) { From ada2471f27e1a9c2c707cc973ea42260f9c68824 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Thu, 23 Mar 2017 21:05:31 -0400 Subject: [PATCH 14/14] Very minor changes thanks to nicolas-grekas --- src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md | 2 +- .../DependencyInjection/FrameworkExtension.php | 1 - .../Bundle/FrameworkBundle/Resources/config/assets.xml | 2 +- src/Symfony/Component/Asset/CHANGELOG.md | 2 +- .../Asset/VersionStrategy/JsonManifestVersionStrategy.php | 4 +--- 5 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md index 64fe428e9b252..b740b333acbc3 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md @@ -5,7 +5,7 @@ CHANGELOG ----- * Added a new new version strategy option called json_manifest_path - that allows you to use the JsonManifestVersionStrategy. + that allows you to use the `JsonManifestVersionStrategy`. * Added support for the `controller.service_arguments` tag, for injecting services into controllers' actions * Deprecated `cache:clear` with warmup (always call it with `--no-warmup`) * Deprecated the "framework.trusted_proxies" configuration option and the corresponding "kernel.trusted_proxies" parameter diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index f39e25db2eef4..bd2dcd5db4e71 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -876,7 +876,6 @@ private function createVersion(ContainerBuilder $container, $version, $format, $ if (null !== $jsonManifestPath) { $def = new ChildDefinition('assets.json_manifest_version_strategy'); $def->replaceArgument(0, $jsonManifestPath); - $def->setPublic(false); $container->setDefinition('assets._version_'.$name, $def); return new Reference('assets._version_'.$name); diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/assets.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/assets.xml index 385b7e307040d..226e7000dfddd 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/assets.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/assets.xml @@ -39,7 +39,7 @@ - + diff --git a/src/Symfony/Component/Asset/CHANGELOG.md b/src/Symfony/Component/Asset/CHANGELOG.md index aa0b32cfa89f8..72030a9d65b16 100644 --- a/src/Symfony/Component/Asset/CHANGELOG.md +++ b/src/Symfony/Component/Asset/CHANGELOG.md @@ -3,7 +3,7 @@ CHANGELOG 3.3.0 ----- - * Added JsonManifestVersionStrategy as a way to read final, + * Added `JsonManifestVersionStrategy` as a way to read final, versioned paths from a JSON manifest file. 2.7.0 diff --git a/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php b/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php index 19ab039de364e..378ad54346d7f 100644 --- a/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php +++ b/src/Symfony/Component/Asset/VersionStrategy/JsonManifestVersionStrategy.php @@ -47,9 +47,7 @@ public function getVersion($path) public function applyVersion($path) { - $manifestPath = $this->getManifestPath($path); - - return $manifestPath ?: $path; + return $this->getManifestPath($path) ?: $path; } private function getManifestPath($path)