diff --git a/core/lib/Drupal/Core/Asset/AssetCollector.php b/core/lib/Drupal/Core/Asset/AssetCollector.php index 941340d..c338482 100644 --- a/core/lib/Drupal/Core/Asset/AssetCollector.php +++ b/core/lib/Drupal/Core/Asset/AssetCollector.php @@ -69,16 +69,16 @@ class AssetCollector { protected $assetDefaults = array(); - protected $methodMap = array( + protected $classMap = array( 'css' => array( - 'file' => 'createStylesheetFileAsset', - 'external' => 'createStylesheetExternalAsset', - 'string' => 'createStylesheetStringAsset', + 'file' => 'Drupal\\Core\\Asset\\StylesheetFileAsset', + 'external' => 'Drupal\\Core\\Asset\\StylesheetExternalAsset', + 'string' => 'Drupal\\Core\\Asset\\StylesheetStringAsset', ), 'js' => array( - 'file' => 'createJavascriptFileAsset', - 'external' => 'createJavascriptExternalAsset', - 'string' => 'createJavascriptStringAsset', + 'file' => 'Drupal\\Core\\Asset\\JavascriptFileAsset', + 'external' => 'Drupal\\Core\\Asset\\JavascriptExternalAsset', + 'string' => 'Drupal\\Core\\Asset\\JavascriptStringAsset', ), ); @@ -112,80 +112,39 @@ public function add(AssetInterface $asset) { * @param array $filters * ??? * - * @return ??? - * - * @todo Document. + * @return \Drupal\Core\Asset\AssetInterface */ public function create($asset_type, $source_type, $data, $options = array(), $filters = array()) { - return call_user_func(array($this, $this->methodMap[$asset_type][$source_type]), $data, $options, $filters); - } - - public function setBag(AssetBagInterface $bag) { - if ($this->isLocked()) { - throw new \Exception('The collector instance is locked. A new bag cannot assigned on a locked collector.'); + if (!isset($this->classMap[$asset_type])) { + throw new \InvalidArgumentException('Only assets of type "js" or "css" are allowed.'); } - $this->bag = $bag; - } - - public function clearBag() { - if ($this->isLocked()) { - throw new \Exception('The collector instance is locked. Bags cannot be cleared on a locked collector.'); + if (!isset($this->classMap[$asset_type][$source_type])) { + throw new \InvalidArgumentException('Only sources of type "file", "string", or "external" are allowed.'); } - $this->bag = NULL; - } - public function createStylesheetFileAsset($path, $options = array(), $filters = array()) { - $asset = new StylesheetFileAsset($path, $options, $filters); - $asset->setDefaults($this->getDefaults('css')); - if (!empty($this->bag)) { - $this->add($asset); - } - return $asset; - } + $class = $this->classMap[$asset_type][$source_type]; + $asset = new $class($data, $options, $filters); + $asset->setDefaults($this->getDefaults($asset_type)); - public function createStylesheetStringAsset($data, $options = array(), $filters = array()) { - $asset = new StylesheetStringAsset($data, $options, $filters); - $asset->setDefaults($this->getDefaults('css')); if (!empty($this->bag)) { $this->add($asset); } - return $asset; - } - public function createStylesheetExternalAsset($url, $options = array(), $filters = array()) { - $asset = new StylesheetExternalAsset($url, $options, $filters); - $asset->setDefaults($this->getDefaults('css')); - if (!empty($this->bag)) { - $this->add($asset); - } return $asset; } - public function createJavascriptFileAsset($path, $options = array(), $filters = array()) { - $asset = new JavascriptFileAsset($path, $options, $filters); - $asset->setDefaults($this->getDefaults('js')); - if (!empty($this->bag)) { - $this->add($asset); - } - return $asset; - } - - public function createJavascriptStringAsset($data, $options = array(), $filters = array()) { - $asset = new JavascriptStringAsset($data, $options, $filters); - $asset->setDefaults($this->getDefaults('js')); - if (!empty($this->bag)) { - $this->add($asset); + public function setBag(AssetBagInterface $bag) { + if ($this->isLocked()) { + throw new \Exception('The collector instance is locked. A new bag cannot assigned on a locked collector.'); } - return $asset; + $this->bag = $bag; } - public function createJavascriptExternalAsset($url, $options = array(), $filters = array()) { - $asset = new JavascriptExternalAsset($url, $options, $filters); - $asset->setDefaults($this->getDefaults('js')); - if (!empty($this->bag)) { - $this->add($asset); + public function clearBag() { + if ($this->isLocked()) { + throw new \Exception('The collector instance is locked. Bags cannot be cleared on a locked collector.'); } - return $asset; + $this->bag = NULL; } public function createJavascriptSetting() { diff --git a/core/tests/Drupal/Tests/Core/Asset/AssetCollectorTest.php b/core/tests/Drupal/Tests/Core/Asset/AssetCollectorTest.php index 9ec4ac0..479adf9 100644 --- a/core/tests/Drupal/Tests/Core/Asset/AssetCollectorTest.php +++ b/core/tests/Drupal/Tests/Core/Asset/AssetCollectorTest.php @@ -80,7 +80,7 @@ public function setUp() { */ public function testMetadataInjection() { // Test a single value first - $asset = $this->collector->createStylesheetFileAsset('foo', array('group' => CSS_AGGREGATE_THEME)); + $asset = $this->collector->create('css', 'file', 'foo', array('group' => CSS_AGGREGATE_THEME)); $this->assertEquals(CSS_AGGREGATE_THEME, $asset['group'], 'Collector injected user-passed parameters into the created asset.'); // TODO is it worth testing multiple params? what about weird ones, like weight? @@ -102,9 +102,7 @@ public function testAssetsImplicitlyArriveInInjectedBag() { $this->collector->setBag($bag); $asset2 = $this->collector->create('css', 'file', 'bar'); - $this->assertContains($asset2, $bag->getCss(), 'Asset created via generic method was implicitly added to bag.'); - $asset3 = $this->collector->createStylesheetFileAsset('baz'); - $this->assertContains($asset3, $bag->getCss(), 'Asset created via specific method was implicitly added to bag.'); + $this->assertContains($asset2, $bag->getCss(), 'Created asset was implicitly added to bag.'); } /** @@ -119,7 +117,7 @@ public function testClearBag() { } public function testLock() { - $this->assertTrue($this->collector->lock($this), 'Collector locked succesfully.'); + $this->assertTrue($this->collector->lock($this), 'Collector locked successfully.'); $this->assertTrue($this->collector->isLocked(), 'Collector accurately reports that it is locked via isLocked() method.'); } @@ -192,59 +190,42 @@ public function testGetNonexistentDefault() { } public function testDefaultPropagation() { - // Test that defaults are correctly applied when passing through both - // the generic and specific factory methods. + // Test that defaults are correctly applied by the factory. $this->collector->setDefaults('css', array('every_page' => TRUE, 'group' => CSS_AGGREGATE_THEME)); $css1 = $this->collector->create('css', 'file', 'foo'); $this->assertTrue($css1['every_page'], 'Correct default propagated for "every_page" property.'); $this->assertEquals(CSS_AGGREGATE_THEME, $css1['group'], 'Correct default propagated for "group" property.'); - $css2 = $this->collector->createStylesheetFileAsset('foo'); - $this->assertTrue($css2['every_page'], 'Correct default propagated for "every_page" property.'); - $this->assertEquals(CSS_AGGREGATE_THEME, $css2['group'], 'Correct default propagated for "group" property.'); - // TODO bother testing js? it seems logically redundant } public function testCreateStylesheetFileAsset() { $css_file1 = $this->collector->create('css', 'file', 'foo'); - $css_file2 = $this->collector->createStylesheetFileAsset('foo'); - $this->assertInstanceOf('\Drupal\Core\Asset\StylesheetFileAsset', $css_file1, 'Collector correctly created a StylesheetFileAsset instance through the generic method.'); - $this->assertInstanceOf('\Drupal\Core\Asset\StylesheetFileAsset', $css_file2, 'Collector correctly created a StylesheetFileAsset instance through the specific method.'); + $this->assertInstanceOf('\Drupal\Core\Asset\StylesheetFileAsset', $css_file1, 'Collector correctly created a StylesheetFileAsset instance.'); } public function testCreateStylesheetExternalAsset() { $css_external1 = $this->collector->create('css', 'external', 'http://foo.bar/path/to/asset.css'); - $css_external2 = $this->collector->createStylesheetExternalAsset('http://foo.bar/path/to/asset.css'); - $this->assertInstanceOf('\Drupal\Core\Asset\StylesheetExternalAsset', $css_external1, 'Collector correctly created a StylesheetExternalAsset instance through the generic method.'); - $this->assertInstanceOf('\Drupal\Core\Asset\StylesheetExternalAsset', $css_external2, 'Collector correctly created a StylesheetExternalAsset instance through the specific method.'); + $this->assertInstanceOf('\Drupal\Core\Asset\StylesheetExternalAsset', $css_external1, 'Collector correctly created a StylesheetExternalAsset instance.'); } public function testCreateStylesheetStringAsset() { $css_string1 = $this->collector->create('css', 'string', 'foo'); - $css_string2 = $this->collector->createStylesheetStringAsset('foo'); - $this->assertInstanceOf('\Drupal\Core\Asset\StylesheetStringAsset', $css_string1, 'Collector correctly created a StylesheetStringAsset instance through the generic method.'); - $this->assertInstanceOf('\Drupal\Core\Asset\StylesheetStringAsset', $css_string2, 'Collector correctly created a StylesheetStringAsset instance through the specific method.'); + $this->assertInstanceOf('\Drupal\Core\Asset\StylesheetStringAsset', $css_string1, 'Collector correctly created a StylesheetStringAsset instance .'); } public function testCreateJavascriptFileAsset() { $js_file1 = $this->collector->create('js', 'file', 'foo'); - $js_file2 = $this->collector->createJavascriptFileAsset('foo'); - $this->assertInstanceOf('\Drupal\Core\Asset\JavascriptFileAsset', $js_file1, 'Collector correctly created a JavascriptFileAsset instance through the generic method.'); - $this->assertInstanceOf('\Drupal\Core\Asset\JavascriptFileAsset', $js_file2, 'Collector correctly created a JavascriptFileAsset instance through the specific method.'); + $this->assertInstanceOf('\Drupal\Core\Asset\JavascriptFileAsset', $js_file1, 'Collector correctly created a JavascriptFileAsset instance .'); } public function testCreateJavascriptExternalAsset() { $js_external1 = $this->collector->create('js', 'external', 'http://foo.bar/path/to/asset.js'); - $js_external2 = $this->collector->createJavascriptExternalAsset('http://foo.bar/path/to/asset.js'); - $this->assertInstanceOf('\Drupal\Core\Asset\JavascriptExternalAsset', $js_external1, 'Collector correctly created a JavascriptExternalAsset instance through the generic method.'); - $this->assertInstanceOf('\Drupal\Core\Asset\JavascriptExternalAsset', $js_external2, 'Collector correctly created a JavascriptExternalAsset instance through the specific method.'); + $this->assertInstanceOf('\Drupal\Core\Asset\JavascriptExternalAsset', $js_external1, 'Collector correctly created a JavascriptExternalAsset instance .'); } public function testCreateJavascriptStringAsset() { $js_string1 = $this->collector->create('js', 'string', 'foo'); - $js_string2 = $this->collector->createJavascriptStringAsset('foo'); - $this->assertInstanceOf('\Drupal\Core\Asset\JavascriptStringAsset', $js_string1, 'Collector correctly created a JavascriptStringAsset instance through the generic method.'); - $this->assertInstanceOf('\Drupal\Core\Asset\JavascriptStringAsset', $js_string2, 'Collector correctly created a JavascriptStringAsset instance through the specific method.'); + $this->assertInstanceOf('\Drupal\Core\Asset\JavascriptStringAsset', $js_string1, 'Collector correctly created a JavascriptStringAsset instance .'); } } \ No newline at end of file