From 0c113faf62b384e255704af46878d5ba4f40dac5 Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Fri, 11 Oct 2013 01:13:32 +0200 Subject: [PATCH 1/6] added unit test for asset bundles --- tests/unit/data/web/assets/.gitignore | 2 + tests/unit/framework/web/AssetBundleTest.php | 84 ++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 tests/unit/data/web/assets/.gitignore create mode 100644 tests/unit/framework/web/AssetBundleTest.php diff --git a/tests/unit/data/web/assets/.gitignore b/tests/unit/data/web/assets/.gitignore new file mode 100644 index 0000000..c96a04f --- /dev/null +++ b/tests/unit/data/web/assets/.gitignore @@ -0,0 +1,2 @@ +* +!.gitignore \ No newline at end of file diff --git a/tests/unit/framework/web/AssetBundleTest.php b/tests/unit/framework/web/AssetBundleTest.php new file mode 100644 index 0000000..60bb7eb --- /dev/null +++ b/tests/unit/framework/web/AssetBundleTest.php @@ -0,0 +1,84 @@ + + */ + +namespace yiiunit\framework\web; + +use Yii; +use yii\base\View; +use yii\web\AssetBundle; +use yii\web\AssetManager; + +/** + * @group web + */ +class AssetBundleTest extends \yiiunit\TestCase +{ + protected function setUp() + { + parent::setUp(); + $this->mockApplication(); + + Yii::setAlias('@testWeb', '/'); + Yii::setAlias('@testWebRoot', '@yiiunit/data/web'); + } + + protected function getView() + { + $view = new View(); + $view->setAssetManager(new AssetManager(array( + 'basePath' => '@testWebRoot/assets', + 'baseUrl' => '@testWeb/assets', + ))); + + return $view; + } + + public function testRegister() + { + $view = $this->getView(); + + $this->assertEmpty($view->assetBundles); + TestJqueryAsset::register($view); + $this->assertEquals(1, count($view->assetBundles)); + $this->assertArrayHasKey('yiiunit\\framework\\web\\TestJqueryAsset', $view->assetBundles); + } + + public function testSimpleDependency() + { + $view = $this->getView(); + + $this->assertEmpty($view->assetBundles); + TestAssetBundle::register($view); + $this->assertEquals(2, count($view->assetBundles)); + $this->assertArrayHasKey('yiiunit\\framework\\web\\TestAssetBundle', $view->assetBundles); + $this->assertArrayHasKey('yiiunit\\framework\\web\\TestJqueryAsset', $view->assetBundles); + } +} + +class TestAssetBundle extends AssetBundle +{ + public $basePath = '@testWebRoot/files'; + public $baseUrl = '@testWeb/files'; + public $css = array( + 'cssFile.css', + ); + public $js = array( + 'jsFile.js', + ); + public $depends = array( + 'yiiunit\\framework\\web\\TestJqueryAsset' + ); +} + +class TestJqueryAsset extends AssetBundle +{ + public $basePath = '@testWebRoot/js'; + public $baseUrl = '@testWeb/js'; + public $js = array( + 'jquery.js', + ); +} \ No newline at end of file From a6784f4ae99a97bd78d73bce7745fdfc534c45b3 Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Fri, 11 Oct 2013 01:14:40 +0200 Subject: [PATCH 2/6] doc fix --- framework/yii/base/View.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/framework/yii/base/View.php b/framework/yii/base/View.php index df0b2b2..9c6a339 100644 --- a/framework/yii/base/View.php +++ b/framework/yii/base/View.php @@ -142,8 +142,8 @@ class View extends Component */ public $dynamicPlaceholders = array(); /** - * @var array list of the registered asset bundles. The keys are the bundle names, and the values - * are booleans indicating whether the bundles have been registered. + * @var AssetBundle[] list of the registered asset bundles. The keys are the bundle names, and the values + * are the registered [[AssetBundle]] objects. * @see registerAssetBundle */ public $assetBundles; From abc1e0c2bb434e628c45accaf44fff378ad36d75 Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Fri, 11 Oct 2013 01:54:05 +0200 Subject: [PATCH 3/6] fix issue #700 adjust depended asset position or throw exception on conflict --- framework/yii/base/View.php | 45 ++++++++-- framework/yii/web/AssetBundle.php | 5 -- tests/unit/data/views/layout.php | 22 +++++ tests/unit/data/views/rawlayout.php | 5 ++ tests/unit/data/views/simple.php | 1 + tests/unit/framework/web/AssetBundleTest.php | 126 +++++++++++++++++++++++++++ 6 files changed, 194 insertions(+), 10 deletions(-) create mode 100644 tests/unit/data/views/layout.php create mode 100644 tests/unit/data/views/rawlayout.php create mode 100644 tests/unit/data/views/simple.php diff --git a/framework/yii/base/View.php b/framework/yii/base/View.php index 9c6a339..1b656d2 100644 --- a/framework/yii/base/View.php +++ b/framework/yii/base/View.php @@ -146,7 +146,7 @@ class View extends Component * are the registered [[AssetBundle]] objects. * @see registerAssetBundle */ - public $assetBundles; + public $assetBundles = array(); /** * @var string the page title */ @@ -523,6 +523,9 @@ class View extends Component $this->trigger(self::EVENT_END_PAGE); $content = ob_get_clean(); + foreach(array_keys($this->assetBundles) as $bundle) { + $this->registerAssetFiles($bundle); + } echo strtr($content, array( self::PH_HEAD => $this->renderHeadHtml(), self::PH_BODY_BEGIN => $this->renderBodyBeginHtml(), @@ -530,7 +533,6 @@ class View extends Component )); unset( - $this->assetBundles, $this->metaTags, $this->linkTags, $this->css, @@ -566,25 +568,58 @@ class View extends Component echo self::PH_HEAD; } + protected function registerAssetFiles($name) + { + if (!isset($this->assetBundles[$name])) { + return; + } + $bundle = $this->assetBundles[$name]; + foreach($bundle->depends as $depName) { + $this->registerAssetFiles($depName); + } + $bundle->registerAssets($this); + unset($this->assetBundles[$name]); + } + /** * Registers the named asset bundle. * All dependent asset bundles will be registered. * @param string $name the name of the asset bundle. + * @param integer|null $position optional parameter to force a minimum Javascript position TODO link to relevant method + * Null means to register on default position. * @return AssetBundle the registered asset bundle instance * @throws InvalidConfigException if the asset bundle does not exist or a circular dependency is detected */ - public function registerAssetBundle($name) + public function registerAssetBundle($name, $position = null) { if (!isset($this->assetBundles[$name])) { $am = $this->getAssetManager(); $bundle = $am->getBundle($name); $this->assetBundles[$name] = false; - $bundle->registerAssets($this); + // register dependencies + $pos = isset($bundle->jsOptions['position']) ? $bundle->jsOptions['position'] : null; + foreach ($bundle->depends as $dep) { + $this->registerAssetBundle($dep, $pos); + } $this->assetBundles[$name] = $bundle; } elseif ($this->assetBundles[$name] === false) { throw new InvalidConfigException("A circular dependency is detected for bundle '$name'."); + } else { + $bundle = $this->assetBundles[$name]; + } + + if ($position !== null) { + $pos = isset($bundle->jsOptions['position']) ? $bundle->jsOptions['position'] : null; + if ($pos === null) { + $bundle->jsOptions['position'] = $pos = $position; + } elseif ($pos > $position) { + throw new InvalidConfigException("A dependend AssetBundle of '$name' requires a higher position but it conflicts with the position set for '$name'!"); + } + foreach ($bundle->depends as $dep) { + $this->registerAssetBundle($dep, $pos); + } } - return $this->assetBundles[$name]; + return $bundle; } /** diff --git a/framework/yii/web/AssetBundle.php b/framework/yii/web/AssetBundle.php index d324ef3..aa2d02b 100644 --- a/framework/yii/web/AssetBundle.php +++ b/framework/yii/web/AssetBundle.php @@ -130,7 +130,6 @@ class AssetBundle extends Object /** * Registers the CSS and JS files with the given view. - * This method will first register all dependent asset bundles. * It will then try to convert non-CSS or JS files (e.g. LESS, Sass) into the corresponding * CSS or JS files using [[AssetManager::converter|asset converter]]. * @param \yii\base\View $view the view that the asset files to be registered with. @@ -139,10 +138,6 @@ class AssetBundle extends Object */ public function registerAssets($view) { - foreach ($this->depends as $name) { - $view->registerAssetBundle($name); - } - $this->publish($view->getAssetManager()); foreach ($this->js as $js) { diff --git a/tests/unit/data/views/layout.php b/tests/unit/data/views/layout.php new file mode 100644 index 0000000..cd9d9d6 --- /dev/null +++ b/tests/unit/data/views/layout.php @@ -0,0 +1,22 @@ + +beginPage(); ?> + + + + Test + head(); ?> + + +beginBody(); ?> + + + +endBody(); ?> + + +endPage(); ?> \ No newline at end of file diff --git a/tests/unit/data/views/rawlayout.php b/tests/unit/data/views/rawlayout.php new file mode 100644 index 0000000..6864642 --- /dev/null +++ b/tests/unit/data/views/rawlayout.php @@ -0,0 +1,5 @@ +beginPage(); ?>1head(); ?>2beginBody(); ?>3endBody(); ?>4endPage(); ?> \ No newline at end of file diff --git a/tests/unit/data/views/simple.php b/tests/unit/data/views/simple.php new file mode 100644 index 0000000..437ba90 --- /dev/null +++ b/tests/unit/data/views/simple.php @@ -0,0 +1 @@ +This is a damn simple view file. \ No newline at end of file diff --git a/tests/unit/framework/web/AssetBundleTest.php b/tests/unit/framework/web/AssetBundleTest.php index 60bb7eb..02cb5ed 100644 --- a/tests/unit/framework/web/AssetBundleTest.php +++ b/tests/unit/framework/web/AssetBundleTest.php @@ -45,6 +45,13 @@ class AssetBundleTest extends \yiiunit\TestCase TestJqueryAsset::register($view); $this->assertEquals(1, count($view->assetBundles)); $this->assertArrayHasKey('yiiunit\\framework\\web\\TestJqueryAsset', $view->assetBundles); + $this->assertTrue($view->assetBundles['yiiunit\\framework\\web\\TestJqueryAsset'] instanceof AssetBundle); + + $expected = << +4 +EOF; + $this->assertEquals($expected, $view->renderFile('@yiiunit/data/views/rawlayout.php')); } public function testSimpleDependency() @@ -56,6 +63,125 @@ class AssetBundleTest extends \yiiunit\TestCase $this->assertEquals(2, count($view->assetBundles)); $this->assertArrayHasKey('yiiunit\\framework\\web\\TestAssetBundle', $view->assetBundles); $this->assertArrayHasKey('yiiunit\\framework\\web\\TestJqueryAsset', $view->assetBundles); + $this->assertTrue($view->assetBundles['yiiunit\\framework\\web\\TestAssetBundle'] instanceof AssetBundle); + $this->assertTrue($view->assetBundles['yiiunit\\framework\\web\\TestJqueryAsset'] instanceof AssetBundle); + + $expected = << +23 + +4 +EOF; + $this->assertEquals($expected, $view->renderFile('@yiiunit/data/views/rawlayout.php')); + } + + public function positionProvider() + { + return array( + array(View::POS_HEAD, true), + array(View::POS_HEAD, false), + array(View::POS_BEGIN, true), + array(View::POS_BEGIN, false), + array(View::POS_END, true), + array(View::POS_END, false), + ); + } + + /** + * @dataProvider positionProvider + */ + public function testPositionDependency($pos, $jqAlreadyRegistered) + { + $view = $this->getView(); + + $view->getAssetManager()->bundles['yiiunit\\framework\\web\\TestAssetBundle'] = array( + 'jsOptions' => array( + 'position' => $pos, + ), + ); + + $this->assertEmpty($view->assetBundles); + if ($jqAlreadyRegistered) { + TestJqueryAsset::register($view); + } + TestAssetBundle::register($view); + $this->assertEquals(2, count($view->assetBundles)); + $this->assertArrayHasKey('yiiunit\\framework\\web\\TestAssetBundle', $view->assetBundles); + $this->assertArrayHasKey('yiiunit\\framework\\web\\TestJqueryAsset', $view->assetBundles); + + $this->assertTrue($view->assetBundles['yiiunit\\framework\\web\\TestAssetBundle'] instanceof AssetBundle); + $this->assertTrue($view->assetBundles['yiiunit\\framework\\web\\TestJqueryAsset'] instanceof AssetBundle); + + $this->assertArrayHasKey('position', $view->assetBundles['yiiunit\\framework\\web\\TestAssetBundle']->jsOptions); + $this->assertEquals($pos, $view->assetBundles['yiiunit\\framework\\web\\TestAssetBundle']->jsOptions['position']); + $this->assertArrayHasKey('position', $view->assetBundles['yiiunit\\framework\\web\\TestJqueryAsset']->jsOptions); + $this->assertEquals($pos, $view->assetBundles['yiiunit\\framework\\web\\TestJqueryAsset']->jsOptions['position']); + + switch($pos) + { + case View::POS_HEAD: + $expected = << + + +234 +EOF; + break; + case View::POS_BEGIN: + $expected = << +2 + +34 +EOF; + break; + default: + case View::POS_END: + $expected = << +23 + +4 +EOF; + break; + } + $this->assertEquals($expected, $view->renderFile('@yiiunit/data/views/rawlayout.php')); + } + + public function positionProvider2() + { + return array( + array(View::POS_BEGIN, true), + array(View::POS_BEGIN, false), + array(View::POS_END, true), + array(View::POS_END, false), + ); + } + + /** + * @dataProvider positionProvider + */ + public function testPositionDependencyConflict($pos, $jqAlreadyRegistered) + { + $view = $this->getView(); + + $view->getAssetManager()->bundles['yiiunit\\framework\\web\\TestAssetBundle'] = array( + 'jsOptions' => array( + 'position' => $pos - 1, + ), + ); + $view->getAssetManager()->bundles['yiiunit\\framework\\web\\TestJqueryAsset'] = array( + 'jsOptions' => array( + 'position' => $pos, + ), + ); + + $this->assertEmpty($view->assetBundles); + if ($jqAlreadyRegistered) { + TestJqueryAsset::register($view); + } + $this->setExpectedException('yii\\base\\InvalidConfigException'); + TestAssetBundle::register($view); } } From a922c41ee63ac4e182042a201c442257f6e35788 Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Fri, 11 Oct 2013 02:14:59 +0200 Subject: [PATCH 4/6] more asset bundle tests and docs --- framework/yii/base/View.php | 37 +++++++++------- tests/unit/framework/web/AssetBundleTest.php | 64 +++++++++++++++++++++++++--- 2 files changed, 81 insertions(+), 20 deletions(-) diff --git a/framework/yii/base/View.php b/framework/yii/base/View.php index 1b656d2..27ede61 100644 --- a/framework/yii/base/View.php +++ b/framework/yii/base/View.php @@ -543,6 +543,24 @@ class View extends Component } /** + * Registers all files provided by an asset bundle including depending bundles files. + * Removes a bundle from [[assetBundles]] once registered. + * @param string $name name of the bundle to register + */ + private function registerAssetFiles($name) + { + if (!isset($this->assetBundles[$name])) { + return; + } + $bundle = $this->assetBundles[$name]; + foreach($bundle->depends as $dep) { + $this->registerAssetFiles($dep); + } + $bundle->registerAssets($this); + unset($this->assetBundles[$name]); + } + + /** * Marks the beginning of an HTML body section. */ public function beginBody() @@ -568,25 +586,14 @@ class View extends Component echo self::PH_HEAD; } - protected function registerAssetFiles($name) - { - if (!isset($this->assetBundles[$name])) { - return; - } - $bundle = $this->assetBundles[$name]; - foreach($bundle->depends as $depName) { - $this->registerAssetFiles($depName); - } - $bundle->registerAssets($this); - unset($this->assetBundles[$name]); - } - /** * Registers the named asset bundle. * All dependent asset bundles will be registered. * @param string $name the name of the asset bundle. - * @param integer|null $position optional parameter to force a minimum Javascript position TODO link to relevant method - * Null means to register on default position. + * @param integer|null $position if set, this forces a minimum position for javascript files. + * This will adjust depending assets javascript file position or fail if requirement can not be met. + * If this is null, asset bundles position settings will not be changed. + * See [[registerJsFile]] for more details on javascript position. * @return AssetBundle the registered asset bundle instance * @throws InvalidConfigException if the asset bundle does not exist or a circular dependency is detected */ diff --git a/tests/unit/framework/web/AssetBundleTest.php b/tests/unit/framework/web/AssetBundleTest.php index 02cb5ed..ce33ce7 100644 --- a/tests/unit/framework/web/AssetBundleTest.php +++ b/tests/unit/framework/web/AssetBundleTest.php @@ -42,10 +42,10 @@ class AssetBundleTest extends \yiiunit\TestCase $view = $this->getView(); $this->assertEmpty($view->assetBundles); - TestJqueryAsset::register($view); + TestSimpleAsset::register($view); $this->assertEquals(1, count($view->assetBundles)); - $this->assertArrayHasKey('yiiunit\\framework\\web\\TestJqueryAsset', $view->assetBundles); - $this->assertTrue($view->assetBundles['yiiunit\\framework\\web\\TestJqueryAsset'] instanceof AssetBundle); + $this->assertArrayHasKey('yiiunit\\framework\\web\\TestSimpleAsset', $view->assetBundles); + $this->assertTrue($view->assetBundles['yiiunit\\framework\\web\\TestSimpleAsset'] instanceof AssetBundle); $expected = << @@ -60,11 +60,13 @@ EOF; $this->assertEmpty($view->assetBundles); TestAssetBundle::register($view); - $this->assertEquals(2, count($view->assetBundles)); + $this->assertEquals(3, count($view->assetBundles)); $this->assertArrayHasKey('yiiunit\\framework\\web\\TestAssetBundle', $view->assetBundles); $this->assertArrayHasKey('yiiunit\\framework\\web\\TestJqueryAsset', $view->assetBundles); + $this->assertArrayHasKey('yiiunit\\framework\\web\\TestAssetLevel3', $view->assetBundles); $this->assertTrue($view->assetBundles['yiiunit\\framework\\web\\TestAssetBundle'] instanceof AssetBundle); $this->assertTrue($view->assetBundles['yiiunit\\framework\\web\\TestJqueryAsset'] instanceof AssetBundle); + $this->assertTrue($view->assetBundles['yiiunit\\framework\\web\\TestAssetLevel3'] instanceof AssetBundle); $expected = << @@ -105,17 +107,21 @@ EOF; TestJqueryAsset::register($view); } TestAssetBundle::register($view); - $this->assertEquals(2, count($view->assetBundles)); + $this->assertEquals(3, count($view->assetBundles)); $this->assertArrayHasKey('yiiunit\\framework\\web\\TestAssetBundle', $view->assetBundles); $this->assertArrayHasKey('yiiunit\\framework\\web\\TestJqueryAsset', $view->assetBundles); + $this->assertArrayHasKey('yiiunit\\framework\\web\\TestAssetLevel3', $view->assetBundles); $this->assertTrue($view->assetBundles['yiiunit\\framework\\web\\TestAssetBundle'] instanceof AssetBundle); $this->assertTrue($view->assetBundles['yiiunit\\framework\\web\\TestJqueryAsset'] instanceof AssetBundle); + $this->assertTrue($view->assetBundles['yiiunit\\framework\\web\\TestAssetLevel3'] instanceof AssetBundle); $this->assertArrayHasKey('position', $view->assetBundles['yiiunit\\framework\\web\\TestAssetBundle']->jsOptions); $this->assertEquals($pos, $view->assetBundles['yiiunit\\framework\\web\\TestAssetBundle']->jsOptions['position']); $this->assertArrayHasKey('position', $view->assetBundles['yiiunit\\framework\\web\\TestJqueryAsset']->jsOptions); $this->assertEquals($pos, $view->assetBundles['yiiunit\\framework\\web\\TestJqueryAsset']->jsOptions['position']); + $this->assertArrayHasKey('position', $view->assetBundles['yiiunit\\framework\\web\\TestAssetLevel3']->jsOptions); + $this->assertEquals($pos, $view->assetBundles['yiiunit\\framework\\web\\TestAssetLevel3']->jsOptions['position']); switch($pos) { @@ -183,6 +189,21 @@ EOF; $this->setExpectedException('yii\\base\\InvalidConfigException'); TestAssetBundle::register($view); } + + public function testCircularDependency() + { + $this->setExpectedException('yii\\base\\InvalidConfigException'); + TestAssetCircleA::register($this->getView()); + } +} + +class TestSimpleAsset extends AssetBundle +{ + public $basePath = '@testWebRoot/js'; + public $baseUrl = '@testWeb/js'; + public $js = array( + 'jquery.js', + ); } class TestAssetBundle extends AssetBundle @@ -207,4 +228,37 @@ class TestJqueryAsset extends AssetBundle public $js = array( 'jquery.js', ); + public $depends = array( + 'yiiunit\\framework\\web\\TestAssetLevel3' + ); +} + +class TestAssetLevel3 extends AssetBundle +{ + public $basePath = '@testWebRoot/js'; + public $baseUrl = '@testWeb/js'; +} + +class TestAssetCircleA extends AssetBundle +{ + public $basePath = '@testWebRoot/js'; + public $baseUrl = '@testWeb/js'; + public $js = array( + 'jquery.js', + ); + public $depends = array( + 'yiiunit\\framework\\web\\TestAssetCircleB' + ); +} + +class TestAssetCircleB extends AssetBundle +{ + public $basePath = '@testWebRoot/js'; + public $baseUrl = '@testWeb/js'; + public $js = array( + 'jquery.js', + ); + public $depends = array( + 'yiiunit\\framework\\web\\TestAssetCircleA' + ); } \ No newline at end of file From 80cf9c8be9d5d29386f725d7900e4b94dba5ccf9 Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Fri, 11 Oct 2013 02:19:32 +0200 Subject: [PATCH 5/6] better error message --- framework/yii/base/View.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/yii/base/View.php b/framework/yii/base/View.php index 27ede61..9071cee 100644 --- a/framework/yii/base/View.php +++ b/framework/yii/base/View.php @@ -620,7 +620,7 @@ class View extends Component if ($pos === null) { $bundle->jsOptions['position'] = $pos = $position; } elseif ($pos > $position) { - throw new InvalidConfigException("A dependend AssetBundle of '$name' requires a higher position but it conflicts with the position set for '$name'!"); + throw new InvalidConfigException("An asset bundle that depends on '$name' has a higher javascript file position configured than '$name'."); } foreach ($bundle->depends as $dep) { $this->registerAssetBundle($dep, $pos); From 72b4f4f71779e1111c2657749ec87b1ee8b4a245 Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Fri, 11 Oct 2013 02:20:51 +0200 Subject: [PATCH 6/6] added comment --- framework/yii/base/View.php | 1 + 1 file changed, 1 insertion(+) diff --git a/framework/yii/base/View.php b/framework/yii/base/View.php index 9071cee..e3dcda8 100644 --- a/framework/yii/base/View.php +++ b/framework/yii/base/View.php @@ -622,6 +622,7 @@ class View extends Component } elseif ($pos > $position) { throw new InvalidConfigException("An asset bundle that depends on '$name' has a higher javascript file position configured than '$name'."); } + // update position for all dependencies foreach ($bundle->depends as $dep) { $this->registerAssetBundle($dep, $pos); }