diff --git a/core/lib/Drupal/Core/Asset/Collection/AssetLibrary.php b/core/lib/Drupal/Core/Asset/Collection/AssetLibrary.php index 6b67baf..ce08b07 100644 --- a/core/lib/Drupal/Core/Asset/Collection/AssetLibrary.php +++ b/core/lib/Drupal/Core/Asset/Collection/AssetLibrary.php @@ -19,7 +19,7 @@ * The primary role of an asset library is to be declared as a dependency by * other assets (including assets declared by other libraries). */ -class AssetLibrary extends AssetCollection implements DependencyInterface, RelativePositionInterface { +class AssetLibrary extends AssetCollection implements DependencyInterface { /** * The asset library's title. @@ -176,78 +176,6 @@ public function clearDependencies() { } /** - * {@inheritdoc} - */ - public function after($asset) { - $this->attemptWrite(__METHOD__); - if (!($asset instanceof AssetInterface || is_string($asset))) { - throw new \InvalidArgumentException('Ordering information must be declared using either an asset string id or the full AssetInterface object.'); - } - - $this->predecessors[] = $asset; - return $this; - } - - /** - * {@inheritdoc} - */ - public function hasPredecessors() { - return !empty($this->predecessors); - } - - /** - * {@inheritdoc} - */ - public function getPredecessors() { - return $this->predecessors; - } - - /** - * {@inheritdoc} - */ - public function clearPredecessors() { - $this->attemptWrite(__METHOD__); - $this->predecessors = array(); - return $this; - } - - /** - * {@inheritdoc} - */ - public function before($asset) { - $this->attemptWrite(__METHOD__); - if (!($asset instanceof AssetInterface || is_string($asset))) { - throw new \InvalidArgumentException('Ordering information must be declared using either an asset string id or the full AssetInterface object.'); - } - - $this->successors[] = $asset; - return $this; - } - - /** - * {@inheritdoc} - */ - public function hasSuccessors() { - return !empty($this->successors); - } - - /** - * {@inheritdoc} - */ - public function getSuccessors() { - return $this->successors; - } - - /** - * {@inheritdoc} - */ - public function clearSuccessors() { - $this->attemptWrite(__METHOD__); - $this->successors = array(); - return $this; - } - - /** * Checks if the asset library is frozen, throws an exception if it is. */ protected function attemptWrite($method) { diff --git a/core/lib/Drupal/Core/Asset/GroupSort/AssetGraph.php b/core/lib/Drupal/Core/Asset/GroupSort/AssetGraph.php index 93d027c..20a6954 100644 --- a/core/lib/Drupal/Core/Asset/GroupSort/AssetGraph.php +++ b/core/lib/Drupal/Core/Asset/GroupSort/AssetGraph.php @@ -16,28 +16,32 @@ * An extension of the DirectedAdjacencyGraph concept designed specifically for * Drupal's asset management use case. * - * Drupal allows for two types of positioning declarations: + * Drupal allows for two types of ordering declarations: * * - Dependencies, which guarantee that dependent asset must be present and - * that it must precede the asset declaring it as a dependency. - * - Ordering, which can guarantee that asset A will be either preceded or + * that it must precede the asset declaring it as a dependency. Expressed by + * methods on \Drupal\Core\Asset\DependencyInterface. + * - Positioning, which can guarantee that asset A will be either preceded or * succeeded by asset B, but does NOT guarantee that B will be present. + * Expressed by methods on \Drupal\Core\Asset\RelativePositionInterface. * - * The impact of a dependency can be calculated myopically (without knowledge of - * the full set), as a dependency inherently guarantees the presence of the - * other vertex in the set. + * The first, dependencies, are NOT dealt with by AssetGraph; dependency + * resolution requires collaboration with other fixed services. For that, + * @see \Drupal\Core\Asset\Collection\AssetCollection::resolveLibraries() * - * For ordering, however, the full set must be inspected to determine whether or - * not the other asset is already present. If it is, a directed edge can be - * declared; if it is not, then …. + * AssetGraph deals only with positioning data. As asset vertices are added to + * the graph via addVertex(), AssetGraph checks their predecessor and successor + * lists. If an asset in either of those lists is already present in the graph, + * then AssetGraph will automatically create a directed edge between the two. If + * a vertex from those lists is not already present, then a 'watch' is + * created for it, such that if that vertex is added at a later time then the + * appropriate directed edge will be created automatically. * - * This class eases the process of determining what to do with ordering - * declarations by implementing a more sophisticated addVertex() mechanism, - * which incrementally sets up (and triggers) watches for any ordering - * declarations that have not yet been realized. + * This makes it much easier for calling code to construct the correct graph - + * it needs merely add all the asset vertices one by one, and the correct graph + * is guaranteed to be created. * * TODO add stuff that tracks data about unresolved successors/predecessors - * TODO finish the ordering paragraph */ class AssetGraph extends DirectedAdjacencyList { diff --git a/core/tests/Drupal/Tests/Core/Asset/AssetLibraryRepositoryTest.php b/core/tests/Drupal/Tests/Core/Asset/AssetLibraryRepositoryTest.php index c8c2960..657e89f 100644 --- a/core/tests/Drupal/Tests/Core/Asset/AssetLibraryRepositoryTest.php +++ b/core/tests/Drupal/Tests/Core/Asset/AssetLibraryRepositoryTest.php @@ -197,7 +197,7 @@ public function testResolveDependencies() { ->will($this->returnValue(TRUE)); $main_asset->expects($this->exactly(2)) ->method('getDependencyInfo') - ->will($this->returnValue(array('foo/bar'))); + ->will($this->returnValue(array('foo/bar', 'foo/baz'))); $main_asset->expects($this->once()) ->method('after')->with($compatible_dep); @@ -208,12 +208,10 @@ public function testResolveDependencies() { $library1->expects($this->once()) ->method('getDependencyInfo') ->will($this->returnValue(array('foo/baz', 'qux/bing'))); - $library1->expects($this->once()) - ->method('after')->with($lib_dep); $it = new \ArrayIterator(array($compatible_dep, $incompatible_dep)); - $library1->expects($this->once()) + $library1->expects($this->any()) ->method('getIterator') ->will($this->returnValue($it)); @@ -221,29 +219,30 @@ public function testResolveDependencies() { $library2->expects($this->once()) ->method('getIterator') ->will($this->returnValue(new \ArrayIterator(array()))); + // Never to ensure resolution is non-recursive $library2->expects($this->never()) ->method('hasDependencies'); $library3 = $this->getMock('Drupal\Core\Asset\Collection\AssetLibrary'); - $library3->expects($this->once()) + // Never because !$library1 instanceof RelativePositionInterface + $library3->expects($this->never()) ->method('getIterator') ->will($this->returnValue(new \ArrayIterator(array($lib_dep)))); + // Never to ensure resolution is non-recursive $library3->expects($this->never()) ->method('hasDependencies') ->will($this->returnValue(array('qux/quark'))); - $library4 = $this->getMock('Drupal\Core\Asset\Collection\AssetLibrary'); $repository->set('foo/bar', $library1); $repository->set('foo/baz', $library2); $repository->set('qux/bing', $library3); - $repository->set('qux/quark', $library4); // Ensure no auto-attach when the second param turns it off. - $this->assertEquals(array($library1), $repository->resolveDependencies($main_asset, FALSE)); + $this->assertEquals(array($library1, $library2), $repository->resolveDependencies($main_asset, FALSE)); // Now, let it auto-attach. - $this->assertEquals(array($library1), $repository->resolveDependencies($main_asset)); + $this->assertEquals(array($library1, $library2), $repository->resolveDependencies($main_asset)); // The correctness of $main_asset's predecessor data is guaranteed by the // method counts on the mock; no direct validation is necessary. diff --git a/core/tests/Drupal/Tests/Core/Asset/Collection/AssetLibraryTest.php b/core/tests/Drupal/Tests/Core/Asset/Collection/AssetLibraryTest.php index bf94ea3..6a36f5e 100644 --- a/core/tests/Drupal/Tests/Core/Asset/Collection/AssetLibraryTest.php +++ b/core/tests/Drupal/Tests/Core/Asset/Collection/AssetLibraryTest.php @@ -107,123 +107,6 @@ public function testClearDependencies() { } /** - * @covers ::after - */ - public function testAfter() { - $library = $this->getLibraryFixture(); - $dep = $this->createStubFileAsset(); - - $this->assertSame($library, $library->after('foo')); - $this->assertSame($library, $library->after($dep)); - - $this->assertAttributeContains($dep, 'predecessors', $library); - - $invalid = array(0, 1.1, fopen(__FILE__, 'r'), TRUE, array(), new \stdClass); - - try { - foreach ($invalid as $val) { - $library->after($val); - $this->fail('Was able to create an ordering relationship with an inappropriate value.'); - } - } catch (\InvalidArgumentException $e) {} - } - - /** - * @depends testAfter - * @covers ::hasPredecessors - */ - public function testHasPredecessors() { - $library = $this->getLibraryFixture(); - $this->assertFalse($library->hasPredecessors()); - - $library->after('foo'); - $this->assertTrue($library->hasPredecessors()); - } - - /** - * @depends testAfter - * @covers ::getPredecessors - */ - public function testGetPredecessors() { - $library = $this->getLibraryFixture(); - $this->assertEmpty($library->getPredecessors()); - - $library->after('foo'); - $this->assertEquals(array('foo'), $library->getPredecessors()); - } - - /** - * @depends testAfter - * @depends testHasPredecessors - * @covers ::clearPredecessors - */ - public function testClearPredecessors() { - $library = $this->getLibraryFixture(); - $library->after('foo'); - - $this->assertSame($library, $library->clearPredecessors()); - $this->assertFalse($library->hasPredecessors()); - } - - /** - * @covers ::before - */ - public function testBefore() { - $library = $this->getLibraryFixture(); - $dep = $this->createStubFileAsset(); - - $this->assertSame($library, $library->before('foo')); - $this->assertSame($library, $library->before($dep)); - - $this->assertAttributeContains($dep, 'successors', $library); - - $invalid = array(0, 1.1, fopen(__FILE__, 'r'), TRUE, array(), new \stdClass); - - try { - foreach ($invalid as $val) { - $library->after($val); - $this->fail('Was able to create an ordering relationship with an inappropriate value.'); - } - } catch (\InvalidArgumentException $e) {} - } - - /** - * @depends testBefore - * @covers ::hasSuccessors - */ - public function testHasSuccessors() { - $library = $this->getLibraryFixture(); - $this->assertFalse($library->hasSuccessors()); - - $library->before('foo'); - $this->assertTrue($library->hasSuccessors()); - } - - /** - * @depends testBefore - * @covers ::getSuccessors - */ - public function testGetSuccessors() { - $library = $this->getLibraryFixture(); - $this->assertEmpty($library->getSuccessors()); - - $library->before('foo'); - $this->assertEquals(array('foo'), $library->getSuccessors()); - } - - /** - * @depends testBefore - * @covers ::clearSuccessors - */ - public function testClearSuccessors() { - $library = $this->getLibraryFixture(); - $library->before('foo'); - - $this->assertSame($library, $library->clearSuccessors()); - $this->assertFalse($library->hasSuccessors()); - } - - /** * Tests that all methods that should be disabled by freezing the collection * correctly trigger an exception. * @@ -239,10 +122,6 @@ public function testExceptionOnWriteWhenFrozen() { 'setWebsite' => array('foo'), 'addDependency' => array('foo/bar'), 'clearDependencies' => array(function() {}), - 'after' => array('foo'), - 'clearPredecessors' => array(), - 'before' => array('foo'), - 'clearSuccessors' => array(), ); // No exception before freeze