diff --git a/core/lib/Drupal/Core/Asset/AssetLibraryRepository.php b/core/lib/Drupal/Core/Asset/AssetLibraryRepository.php index fbb5cc7..d2974bd 100644 --- a/core/lib/Drupal/Core/Asset/AssetLibraryRepository.php +++ b/core/lib/Drupal/Core/Asset/AssetLibraryRepository.php @@ -105,9 +105,11 @@ public function resolveDependencies(DependencyInterface $asset, $attach = TRUE) foreach ($asset->getDependencyInfo() as $key) { $dependencies[] = $library = $this->get($key); - // Only bother attaching if operating on an asset. + // Only auto-attach if the argument is capable of it. if ($attach && $asset instanceof RelativePositionInterface) { foreach ($library as $libasset) { + // If operating on a proper AssetInterface object, only attach if + // the dependency and the given asset are of the same type. if ($asset instanceof AssetInterface && $asset->getAssetType() !== $libasset->getAssetType()) { continue; diff --git a/core/lib/Drupal/Core/Asset/Collection/AssetCollection.php b/core/lib/Drupal/Core/Asset/Collection/AssetCollection.php index bedeb99..da581df 100644 --- a/core/lib/Drupal/Core/Asset/Collection/AssetCollection.php +++ b/core/lib/Drupal/Core/Asset/Collection/AssetCollection.php @@ -62,6 +62,11 @@ public function mergeCollection(AssetCollectionInterface $collection, $freeze = } } + foreach ($collection->getUnresolvedLibraries() as $library) { + // TODO just cheat and merge these in? + $this->addUnresolvedLibrary($library); + } + if ($freeze) { $collection->freeze(); } diff --git a/core/lib/Drupal/Core/Asset/GroupSort/AssetGraphSorter.php b/core/lib/Drupal/Core/Asset/GroupSort/AssetGraphSorter.php index 9d9599a..1968730 100644 --- a/core/lib/Drupal/Core/Asset/GroupSort/AssetGraphSorter.php +++ b/core/lib/Drupal/Core/Asset/GroupSort/AssetGraphSorter.php @@ -16,6 +16,20 @@ */ abstract class AssetGraphSorter implements AssetGroupSorterInterface { + public function __construct() { + // By default, xdebug prevents a call stack depth of greater than 100 + // function calls as a protection against recursion. The graph traversal + // used here utilizes a deep recursive walker that exceeds this limit in + // most cases - though typically not by much. So, if xdebug is enabled, we + // extend this call stack limit if it is less than 300. Exceeding a max + // stack depth of 300 would require there to be at least 90, but possibly as + // many as around 140, discrete css OR js assets (not combined). Even in the + // most complex of sites, such a high number is unlikely. + if (extension_loaded('xdebug') && ini_get('xdebug.max_nesting_level') < 300) { + ini_set('xdebug.max_nesting_level', 300); + } + } + /** * Creates a queue of starting vertices that will facilitate an ideal TSL. * @@ -25,7 +39,7 @@ * building asset groups: we assume they are more stable and yield the minimal * number of asset groups overall. * - * @param \Drupal\Core\Asset\AssetGraph $graph + * @param \Drupal\Core\Asset\GroupSort\AssetGraph $graph * The graph from which to create a starting queue. * * @return \SplQueue $queue diff --git a/core/lib/Drupal/Core/Asset/GroupSort/CssGraphSorter.php b/core/lib/Drupal/Core/Asset/GroupSort/CssGraphSorter.php index 9faf7fd..b5b4bfc 100644 --- a/core/lib/Drupal/Core/Asset/GroupSort/CssGraphSorter.php +++ b/core/lib/Drupal/Core/Asset/GroupSort/CssGraphSorter.php @@ -74,6 +74,7 @@ public function groupAndSort(AssetCollectionInterface $collection) { // all ordering information. $graph = new AssetGraph(); + // TODO Would probably be better to inject the right collection rather than asking for it here foreach ($collection->getCss() as $asset) { $graph->addVertex($asset); @@ -110,9 +111,7 @@ public function groupAndSort(AssetCollectionInterface $collection) { $visitor = new OptimallyGroupedTSLVisitor($optimal, $optimal_lookup); DepthFirst::traverse($transpose, $visitor, $queue); - $final = $visitor->getTSL(); - $final->reverse(); - return $final; + return $visitor->getTSL()->reverse(); } } diff --git a/core/lib/Drupal/Core/Asset/StringAsset.php b/core/lib/Drupal/Core/Asset/StringAsset.php index 8c1488f..2c690e9 100644 --- a/core/lib/Drupal/Core/Asset/StringAsset.php +++ b/core/lib/Drupal/Core/Asset/StringAsset.php @@ -40,11 +40,11 @@ class StringAsset extends BaseAsset { * Thrown if a non-string is provided as content. */ public function __construct(AssetMetadataInterface $metadata, $content, $filters = array()) { - if (!is_string($content)) { - throw new \InvalidArgumentException('StringAsset requires a string for its content.'); + if (!is_string($content) || empty($content)) { + throw new \InvalidArgumentException('StringAsset requires a non-empty string for its content.'); } - $this->id= empty($content) ? Crypt::randomBytes(32) : hash('sha256', $content); + $this->id = hash('sha256', $content); $this->setContent($content); parent::__construct($metadata, $filters); diff --git a/core/tests/Drupal/Tests/Core/Asset/Collection/AssetCollectionTest.php b/core/tests/Drupal/Tests/Core/Asset/Collection/AssetCollectionTest.php index 82b262e..06e6d0a 100644 --- a/core/tests/Drupal/Tests/Core/Asset/Collection/AssetCollectionTest.php +++ b/core/tests/Drupal/Tests/Core/Asset/Collection/AssetCollectionTest.php @@ -181,35 +181,6 @@ public function testRemoveNonexistentAsset() { } /** - * @depends testAdd - * @covers ::mergeCollection - */ - public function testMergeCollection() { - $coll2 = new AssetCollection(); - $stub1 = $this->createStubFileAsset(); - $stub2 = $this->createStubFileAsset(); - - $coll2->add($stub1); - $this->assertSame($this->collection, $this->collection->mergeCollection($coll2)); - - $this->assertContains($stub1, $this->collection); - $this->assertTrue($coll2->isFrozen()); - - $coll3 = new AssetCollection(); - $coll3->add($stub1); - $coll3->add($stub2); - // Ensure no duplicates, and don't freeze merged bag - $this->assertSame($this->collection, $this->collection->mergeCollection($coll3, FALSE)); - - $contained = array( - $stub1->id() => $stub1, - $stub2->id() => $stub2, - ); - $this->assertEquals($contained, $this->collection->all()); - $this->assertFalse($coll3->isFrozen()); - } - - /** * Tests that all methods that should be disabled by freezing the collection * correctly trigger an exception. * @@ -503,5 +474,41 @@ public function testResolveLibrariesAgain() { $this->assertEquals($expected, $this->collection->all()); } + + /** + * @depends testAdd + * @depends testAddUnresolvedLibrary + * @depends testGetUnresolvedLibraries + * @covers ::mergeCollection + */ + public function testMergeCollection() { + $coll2 = new AssetCollection(); + $stub1 = $this->createStubFileAsset(); + $stub2 = $this->createStubFileAsset(); + + $coll2->add($stub1); + $coll2->addUnresolvedLibrary('foo/bar'); + // Assert same to check fluency + $this->assertSame($this->collection, $this->collection->mergeCollection($coll2)); + + $this->assertEquals(array('foo/bar'), $this->collection->getUnresolvedLibraries()); + $this->assertContains($stub1, $this->collection); + $this->assertTrue($coll2->isFrozen()); + + $coll3 = new AssetCollection(); + $coll3->add($stub1); + $coll3->add($stub2); + $coll3->addUnresolvedLibrary('foo/bar'); + // Ensure no duplicates, and don't freeze merged bag + $this->collection->mergeCollection($coll3, FALSE); + + $this->assertEquals(array('foo/bar'), $this->collection->getUnresolvedLibraries()); + $contained = array( + $stub1->id() => $stub1, + $stub2->id() => $stub2, + ); + $this->assertEquals($contained, $this->collection->all()); + $this->assertFalse($coll3->isFrozen()); + } } diff --git a/core/tests/Drupal/Tests/Core/Asset/StringAssetTest.php b/core/tests/Drupal/Tests/Core/Asset/StringAssetTest.php index 27ee7f3..2568a52 100644 --- a/core/tests/Drupal/Tests/Core/Asset/StringAssetTest.php +++ b/core/tests/Drupal/Tests/Core/Asset/StringAssetTest.php @@ -10,6 +10,7 @@ use Drupal\Core\Asset\StringAsset; /** + * @coversDefaultClass \Drupal\Core\Asset\StringAsset * @group Asset */ class StringAssetTest extends AssetUnitTest { @@ -22,6 +23,9 @@ public static function getInfo() { ); } + /** + * @covers ::__construct + */ public function testInitialCreation() { $meta = $this->createStubAssetMetadata(); $content = 'foo bar baz'; @@ -32,14 +36,26 @@ public function testInitialCreation() { } /** - * @expectedException \InvalidArgumentException + * @covers ::__construct */ - public function testCreateNonString() { + public function testCreateInvalidContent() { $meta = $this->createStubAssetMetadata(); - $asset = new StringAsset($meta, new \stdClass()); + $invalid = array('', 0, 1.1, fopen(__FILE__, 'r'), TRUE, array(), new \stdClass); + + try { + foreach ($invalid as $val) { + new StringAsset($meta, $val); + $varinfo = (gettype($val) == 'string') ? 'an empty string' : 'of type ' . gettype($val); + $this->fail(sprintf('Was able to create a string asset with invalid content; content was %s.', $varinfo)); + } + } catch (\InvalidArgumentException $e) {} } - public function testSetLastModified() { + /** + * @covers ::setLastModified + * @covers ::getLastModified + */ + public function testLastModified() { $meta = $this->createStubAssetMetadata(); $content = 'foo bar baz'; $asset = new StringAsset($meta, $content); @@ -48,18 +64,20 @@ public function testSetLastModified() { $this->assertEquals(100, $asset->getLastModified()); } + /** + * @covers ::id + */ public function testId() { $meta = $this->createStubAssetMetadata(); $content = 'foo bar baz'; $asset = new StringAsset($meta, $content); $this->assertEquals(hash('sha256', $content), $asset->id()); - - $asset = new StringAsset($meta, ''); - // If no content is provided, the id should be a 32-byte random string (ick) - $this->assertEquals(32, strlen($asset->id())); } + /** + * @covers ::load + */ public function testLoad() { $meta = $this->createStubAssetMetadata(); $content = 'foo bar baz'; @@ -70,3 +88,4 @@ public function testLoad() { $this->assertEquals($content, $asset->getContent()); } } +