Skip to content

Commit

Permalink
Merge pull request #240 from elpiafo/fix_sorter
Browse files Browse the repository at this point in the history 
Fix sorter issue
  • Loading branch information
@mikeSimonson
mikeSimonson authored Sep 20, 2016
2 parents e454a78 + 4b97c4c commit 17fa5bf
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 70 deletions.
6 changes: 4 additions & 2 deletions lib/Doctrine/Common/DataFixtures/Purger/ORMPurger.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,9 @@ private function getCommitOrder(EntityManagerInterface $em, array $classes)
$sorter = new TopologicalSorter();

foreach ($classes as $class) {
$sorter->addNode($class->name, $class);
if ( ! $sorter->hasNode($class->name)) {
$sorter->addNode($class->name, $class);
}

// $class before its parents
foreach ($class->parentClasses as $parentClass) {
Expand Down Expand Up @@ -207,7 +209,7 @@ private function getCommitOrder(EntityManagerInterface $em, array $classes)
}
}

return $sorter->sort();
return array_reverse($sorter->sort());
}

/**
Expand Down
42 changes: 30 additions & 12 deletions lib/Doctrine/Common/DataFixtures/Sorter/TopologicalSorter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,23 @@ class TopologicalSorter
*/
private $sortedNodeList = [];

/**
* Allow or not cyclic dependencies
*
* @var boolean
*/
private $allowCyclicDependencies;

/**
* Construct TopologicalSorter object
*
* @param boolean $allowCyclicDependencies
*/
public function __construct($allowCyclicDependencies = true)
{
$this->allowCyclicDependencies = $allowCyclicDependencies;
}

/**
* Adds a new node (vertex) to the graph, assigning its hash and value.
*
Expand Down Expand Up @@ -154,19 +171,20 @@ private function visit(Vertex $definition)
switch ($childDefinition->state) {
case Vertex::VISITED:
break;

case Vertex::IN_PROGRESS:
throw new CircularReferenceException(
sprintf(
'Graph contains cyclic dependency between the classes "%s" and'
.' "%s". An example of this problem would be the following: '
.'Class C has class B as its dependency. Then, class B has class A has its dependency. '
.'Finally, class A has class C as its dependency.',
$definition->value->getName(),
$childDefinition->value->getName()
)
);

if ( ! $this->allowCyclicDependencies) {
throw new CircularReferenceException(
sprintf(
'Graph contains cyclic dependency between the classes "%s" and'
.' "%s". An example of this problem would be the following: '
.'Class C has class B as its dependency. Then, class B has class A has its dependency. '
.'Finally, class A has class C as its dependency.',
$definition->value->getName(),
$childDefinition->value->getName()
)
);
}
break;
case Vertex::NOT_VISITED:
$this->visit($childDefinition);
}
Expand Down
132 changes: 76 additions & 56 deletions tests/Doctrine/Tests/Common/DataFixtures/Sorter/TopologicalSorterTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
use Doctrine\Common\DataFixtures\Exception\CircularReferenceException;
use Doctrine\Common\DataFixtures\Sorter\TopologicalSorter;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\Tests\Mock;

/**
* TopologicalSorter tests.
Expand All @@ -38,125 +37,146 @@
*/
class TopologicalSorterTest extends \PHPUnit_Framework_TestCase
{
/**
* @var \Doctrine\Common\DataFixtures\Sorter\TopologicalSorter
*/
private $sorter;

/**
* {@inheritdoc}
*/
public function setUp()
{
$this->sorter = new TopologicalSorter();
}

public function testSuccessSortLinearDependency()
{
$sorter = new TopologicalSorter();

$node1 = new ClassMetadata(1);
$node2 = new ClassMetadata(2);
$node3 = new ClassMetadata(3);
$node4 = new ClassMetadata(4);
$node5 = new ClassMetadata(5);

$this->sorter->addNode('1', $node1);
$this->sorter->addNode('2', $node2);
$this->sorter->addNode('3', $node3);
$this->sorter->addNode('4', $node4);
$this->sorter->addNode('5', $node5);
$sorter->addNode('1', $node1);
$sorter->addNode('2', $node2);
$sorter->addNode('3', $node3);
$sorter->addNode('4', $node4);
$sorter->addNode('5', $node5);

$this->sorter->addDependency('1', '2');
$this->sorter->addDependency('2', '3');
$this->sorter->addDependency('3', '4');
$this->sorter->addDependency('5', '1');
$sorter->addDependency('1', '2');
$sorter->addDependency('2', '3');
$sorter->addDependency('3', '4');
$sorter->addDependency('5', '1');

$sortedList = $this->sorter->sort();
$sortedList = $sorter->sort();
$correctList = array($node4, $node3, $node2, $node1, $node5);

self::assertSame($correctList, $sortedList);
}

public function testSuccessSortMultiDependency()
{
$sorter = new TopologicalSorter();

$node1 = new ClassMetadata(1);
$node2 = new ClassMetadata(2);
$node3 = new ClassMetadata(3);
$node4 = new ClassMetadata(4);
$node5 = new ClassMetadata(5);

$this->sorter->addNode('1', $node1);
$this->sorter->addNode('2', $node2);
$this->sorter->addNode('3', $node3);
$this->sorter->addNode('4', $node4);
$this->sorter->addNode('5', $node5);
$sorter->addNode('1', $node1);
$sorter->addNode('2', $node2);
$sorter->addNode('3', $node3);
$sorter->addNode('4', $node4);
$sorter->addNode('5', $node5);

$this->sorter->addDependency('3', '2');
$this->sorter->addDependency('3', '4');
$this->sorter->addDependency('3', '5');
$this->sorter->addDependency('4', '1');
$this->sorter->addDependency('5', '1');
$sorter->addDependency('3', '2');
$sorter->addDependency('3', '4');
$sorter->addDependency('3', '5');
$sorter->addDependency('4', '1');
$sorter->addDependency('5', '1');

$sortedList = $this->sorter->sort();
$sortedList = $sorter->sort();
$correctList = array($node1, $node2, $node4, $node5, $node3);

self::assertSame($correctList, $sortedList);
}

public function testSortCyclicDependency()
{
$sorter = new TopologicalSorter();

$node1 = new ClassMetadata(1);
$node2 = new ClassMetadata(2);
$node3 = new ClassMetadata(3);

$sorter->addNode('1', $node1);
$sorter->addNode('2', $node2);
$sorter->addNode('3', $node3);

$sorter->addDependency('1', '2');
$sorter->addDependency('2', '3');
$sorter->addDependency('3', '1');

$sortedList = $sorter->sort();
$correctList = array($node3, $node2, $node1);

self::assertSame($correctList, $sortedList);

$sorter->sort();
}

public function testFailureSortCyclicDependency()
{
$sorter = new TopologicalSorter(false);

$node1 = new ClassMetadata(1);
$node2 = new ClassMetadata(2);
$node3 = new ClassMetadata(3);

$this->sorter->addNode('1', $node1);
$this->sorter->addNode('2', $node2);
$this->sorter->addNode('3', $node3);
$sorter->addNode('1', $node1);
$sorter->addNode('2', $node2);
$sorter->addNode('3', $node3);

$this->sorter->addDependency('1', '2');
$this->sorter->addDependency('2', '3');
$this->sorter->addDependency('3', '1');
$sorter->addDependency('1', '2');
$sorter->addDependency('2', '3');
$sorter->addDependency('3', '1');

$this->expectException(CircularReferenceException::class);

$this->sorter->sort();
$sorter->sort();
}

public function testNoFailureOnSelfReferencingDependency()
{
$sorter = new TopologicalSorter();

$node1 = new ClassMetadata(1);
$node2 = new ClassMetadata(2);
$node3 = new ClassMetadata(3);
$node4 = new ClassMetadata(4);
$node5 = new ClassMetadata(5);

$this->sorter->addNode('1', $node1);
$this->sorter->addNode('2', $node2);
$this->sorter->addNode('3', $node3);
$this->sorter->addNode('4', $node4);
$this->sorter->addNode('5', $node5);
$sorter->addNode('1', $node1);
$sorter->addNode('2', $node2);
$sorter->addNode('3', $node3);
$sorter->addNode('4', $node4);
$sorter->addNode('5', $node5);

$this->sorter->addDependency('1', '2');
$this->sorter->addDependency('1', '1');
$this->sorter->addDependency('2', '3');
$this->sorter->addDependency('3', '4');
$this->sorter->addDependency('5', '1');
$sorter->addDependency('1', '2');
$sorter->addDependency('1', '1');
$sorter->addDependency('2', '3');
$sorter->addDependency('3', '4');
$sorter->addDependency('5', '1');

$sortedList = $this->sorter->sort();
$sortedList = $sorter->sort();
$correctList = array($node4, $node3, $node2, $node1, $node5);

self::assertSame($correctList, $sortedList);
}

public function testFailureSortMissingDependency()
{
$sorter = new TopologicalSorter();

$node1 = new ClassMetadata(1);

$this->sorter->addNode('1', $node1);
$sorter->addNode('1', $node1);

$this->sorter->addDependency('1', '2');
$sorter->addDependency('1', '2');

$this->expectException(\RuntimeException::class);

$this->sorter->sort();
$sorter->sort();
}
}

0 comments on commit 17fa5bf

Please sign in to comment.