Merge pull request #1570 from doctrine/DDC-2524
[RFC] Tests around reported cases over DDC-2524
This commit is contained in:
Guilherme Blanco
commit
9b77ba2c1a
@ -1,4 +1,5 @@
|
||||
<?php
|
||||
|
||||
/*
|
||||
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
|
||||
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
|
||||
@ -20,137 +21,163 @@
|
||||
namespace Doctrine\ORM\Internal;
|
||||
|
||||
/**
|
||||
* The CommitOrderCalculator is used by the UnitOfWork to sort out the
|
||||
* correct order in which changes to entities need to be persisted.
|
||||
* CommitOrderCalculator implements topological sorting, which is an ordering
|
||||
* algorithm for directed graphs (DG) and/or directed acyclic graphs (DAG) by
|
||||
* using a depth-first searching (DFS) to traverse the graph built in memory.
|
||||
* This algorithm have a linear running time based on nodes (V) and dependency
|
||||
* between the nodes (E), resulting in a computational complexity of O(V + E).
|
||||
*
|
||||
* @since 2.0
|
||||
* @author Roman Borschel <roman@code-factory.org>
|
||||
* @author Guilherme Blanco <guilhermeblanco@hotmail.com>
|
||||
* @author Roman Borschel <roman@code-factory.org>
|
||||
*/
|
||||
class CommitOrderCalculator
|
||||
{
|
||||
const NOT_VISITED = 1;
|
||||
const IN_PROGRESS = 2;
|
||||
const VISITED = 3;
|
||||
const NOT_VISITED = 0;
|
||||
const IN_PROGRESS = 1;
|
||||
const VISITED = 2;
|
||||
|
||||
/**
|
||||
* @var array
|
||||
* Matrix of nodes (aka. vertex).
|
||||
* Keys are provided hashes and values are the node definition objects.
|
||||
*
|
||||
* The node state definition contains the following properties:
|
||||
*
|
||||
* - <b>state</b> (integer)
|
||||
* Whether the node is NOT_VISITED or IN_PROGRESS
|
||||
*
|
||||
* - <b>value</b> (object)
|
||||
* Actual node value
|
||||
*
|
||||
* - <b>dependencyList</b> (array<string>)
|
||||
* Map of node dependencies defined as hashes.
|
||||
*
|
||||
* @var array<stdClass>
|
||||
*/
|
||||
private $_nodeStates = array();
|
||||
private $nodeList = array();
|
||||
|
||||
/**
|
||||
* The nodes to sort.
|
||||
* Volatile variable holding calculated nodes during sorting process.
|
||||
*
|
||||
* @var array
|
||||
*/
|
||||
private $_classes = array();
|
||||
private $sortedNodeList = array();
|
||||
|
||||
/**
|
||||
* @var array
|
||||
*/
|
||||
private $_relatedClasses = array();
|
||||
|
||||
/**
|
||||
* @var array
|
||||
*/
|
||||
private $_sorted = array();
|
||||
|
||||
/**
|
||||
* Clears the current graph.
|
||||
* Checks for node (vertex) existence in graph.
|
||||
*
|
||||
* @return void
|
||||
* @param string $hash
|
||||
*
|
||||
* @return boolean
|
||||
*/
|
||||
public function clear()
|
||||
public function hasNode($hash)
|
||||
{
|
||||
$this->_classes = array();
|
||||
$this->_relatedClasses = array();
|
||||
return isset($this->nodeList[$hash]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets a valid commit order for all current nodes.
|
||||
* Adds a new node (vertex) to the graph, assigning its hash and value.
|
||||
*
|
||||
* Uses a depth-first search (DFS) to traverse the graph.
|
||||
* @param string $hash
|
||||
* @param object $node
|
||||
*
|
||||
* @return void
|
||||
*/
|
||||
public function addNode($hash, $node)
|
||||
{
|
||||
$vertex = new \stdClass();
|
||||
|
||||
$vertex->hash = $hash;
|
||||
$vertex->state = self::NOT_VISITED;
|
||||
$vertex->value = $node;
|
||||
$vertex->dependencyList = array();
|
||||
|
||||
$this->nodeList[$hash] = $vertex;
|
||||
}
|
||||
|
||||
/**
|
||||
* Adds a new dependency (edge) to the graph using their hashes.
|
||||
*
|
||||
* @param string $fromHash
|
||||
* @param string $toHash
|
||||
* @param integer $weight
|
||||
*
|
||||
* @return void
|
||||
*/
|
||||
public function addDependency($fromHash, $toHash, $weight)
|
||||
{
|
||||
$vertex = $this->nodeList[$fromHash];
|
||||
$edge = new \stdClass();
|
||||
|
||||
$edge->from = $fromHash;
|
||||
$edge->to = $toHash;
|
||||
$edge->weight = $weight;
|
||||
|
||||
$vertex->dependencyList[$toHash] = $edge;
|
||||
}
|
||||
|
||||
/**
|
||||
* Return a valid order list of all current nodes.
|
||||
* The desired topological sorting is the reverse post order of these searches.
|
||||
*
|
||||
* @return array The list of ordered classes.
|
||||
* {@internal Highly performance-sensitive method.}
|
||||
*
|
||||
* @return array
|
||||
*/
|
||||
public function getCommitOrder()
|
||||
public function sort()
|
||||
{
|
||||
// Check whether we need to do anything. 0 or 1 node is easy.
|
||||
$nodeCount = count($this->_classes);
|
||||
|
||||
if ($nodeCount <= 1) {
|
||||
return ($nodeCount == 1) ? array_values($this->_classes) : array();
|
||||
foreach ($this->nodeList as $vertex) {
|
||||
if ($vertex->state !== self::NOT_VISITED) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Init
|
||||
foreach ($this->_classes as $node) {
|
||||
$this->_nodeStates[$node->name] = self::NOT_VISITED;
|
||||
$this->visit($vertex);
|
||||
}
|
||||
|
||||
// Go
|
||||
foreach ($this->_classes as $node) {
|
||||
if ($this->_nodeStates[$node->name] == self::NOT_VISITED) {
|
||||
$this->_visitNode($node);
|
||||
}
|
||||
}
|
||||
$sortedList = $this->sortedNodeList;
|
||||
|
||||
$sorted = array_reverse($this->_sorted);
|
||||
$this->nodeList = array();
|
||||
$this->sortedNodeList = array();
|
||||
|
||||
$this->_sorted = $this->_nodeStates = array();
|
||||
|
||||
return $sorted;
|
||||
return array_reverse($sortedList);
|
||||
}
|
||||
|
||||
/**
|
||||
* @param \Doctrine\ORM\Mapping\ClassMetadata $node
|
||||
* Visit a given node definition for reordering.
|
||||
*
|
||||
* @return void
|
||||
*/
|
||||
private function _visitNode($node)
|
||||
{
|
||||
$this->_nodeStates[$node->name] = self::IN_PROGRESS;
|
||||
|
||||
if (isset($this->_relatedClasses[$node->name])) {
|
||||
foreach ($this->_relatedClasses[$node->name] as $relatedNode) {
|
||||
if ($this->_nodeStates[$relatedNode->name] == self::NOT_VISITED) {
|
||||
$this->_visitNode($relatedNode);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
$this->_nodeStates[$node->name] = self::VISITED;
|
||||
$this->_sorted[] = $node;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param \Doctrine\ORM\Mapping\ClassMetadata $fromClass
|
||||
* @param \Doctrine\ORM\Mapping\ClassMetadata $toClass
|
||||
* {@internal Highly performance-sensitive method.}
|
||||
*
|
||||
* @return void
|
||||
* @param \stdClass $vertex
|
||||
*/
|
||||
public function addDependency($fromClass, $toClass)
|
||||
private function visit($vertex)
|
||||
{
|
||||
$this->_relatedClasses[$fromClass->name][] = $toClass;
|
||||
$vertex->state = self::IN_PROGRESS;
|
||||
|
||||
foreach ($vertex->dependencyList as $edge) {
|
||||
$adjacentVertex = $this->nodeList[$edge->to];
|
||||
|
||||
switch ($adjacentVertex->state) {
|
||||
case self::VISITED:
|
||||
// Do nothing, since node was already visited
|
||||
break;
|
||||
|
||||
case self::IN_PROGRESS:
|
||||
if ($adjacentVertex->dependencyList[$vertex->hash]->weight < $edge->weight) {
|
||||
$adjacentVertex->state = self::VISITED;
|
||||
|
||||
$this->sortedNodeList[] = $adjacentVertex->value;
|
||||
}
|
||||
break;
|
||||
|
||||
case self::NOT_VISITED:
|
||||
$this->visit($adjacentVertex);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @param string $className
|
||||
*
|
||||
* @return bool
|
||||
*/
|
||||
public function hasClass($className)
|
||||
{
|
||||
return isset($this->_classes[$className]);
|
||||
}
|
||||
if ($vertex->state !== self::VISITED) {
|
||||
$vertex->state = self::VISITED;
|
||||
|
||||
/**
|
||||
* @param \Doctrine\ORM\Mapping\ClassMetadata $class
|
||||
*
|
||||
* @return void
|
||||
*/
|
||||
public function addClass($class)
|
||||
{
|
||||
$this->_classes[$class->name] = $class;
|
||||
$this->sortedNodeList[] = $vertex->value;
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -212,14 +212,6 @@ class UnitOfWork implements PropertyChangedListener
|
||||
*/
|
||||
private $em;
|
||||
|
||||
/**
|
||||
* The calculator used to calculate the order in which changes to
|
||||
* entities need to be written to the database.
|
||||
*
|
||||
* @var \Doctrine\ORM\Internal\CommitOrderCalculator
|
||||
*/
|
||||
private $commitOrderCalculator;
|
||||
|
||||
/**
|
||||
* The entity persister instances used to persist entity instances.
|
||||
*
|
||||
@ -1138,11 +1130,11 @@ class UnitOfWork implements PropertyChangedListener
|
||||
foreach ($entityChangeSet as $entity) {
|
||||
$class = $this->em->getClassMetadata(get_class($entity));
|
||||
|
||||
if ($calc->hasClass($class->name)) {
|
||||
if ($calc->hasNode($class->name)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$calc->addClass($class);
|
||||
$calc->addNode($class->name, $class);
|
||||
|
||||
$newNodes[] = $class;
|
||||
}
|
||||
@ -1156,13 +1148,16 @@ class UnitOfWork implements PropertyChangedListener
|
||||
|
||||
$targetClass = $this->em->getClassMetadata($assoc['targetEntity']);
|
||||
|
||||
if ( ! $calc->hasClass($targetClass->name)) {
|
||||
$calc->addClass($targetClass);
|
||||
if ( ! $calc->hasNode($targetClass->name)) {
|
||||
$calc->addNode($targetClass->name, $targetClass);
|
||||
|
||||
$newNodes[] = $targetClass;
|
||||
}
|
||||
|
||||
$calc->addDependency($targetClass, $class);
|
||||
$joinColumns = reset($assoc['joinColumns']);
|
||||
$isNullable = isset($joinColumns['nullable']) ? $joinColumns['nullable'] : false;
|
||||
|
||||
$calc->addDependency($targetClass->name, $class->name, $isNullable ? 0 : 1);
|
||||
|
||||
// If the target class has mapped subclasses, these share the same dependency.
|
||||
if ( ! $targetClass->subClasses) {
|
||||
@ -1172,18 +1167,18 @@ class UnitOfWork implements PropertyChangedListener
|
||||
foreach ($targetClass->subClasses as $subClassName) {
|
||||
$targetSubClass = $this->em->getClassMetadata($subClassName);
|
||||
|
||||
if ( ! $calc->hasClass($subClassName)) {
|
||||
$calc->addClass($targetSubClass);
|
||||
if ( ! $calc->hasNode($subClassName)) {
|
||||
$calc->addNode($targetSubClass->name, $targetSubClass);
|
||||
|
||||
$newNodes[] = $targetSubClass;
|
||||
}
|
||||
|
||||
$calc->addDependency($targetSubClass, $class);
|
||||
$calc->addDependency($targetSubClass->name, $class->name, 1);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return $calc->getCommitOrder();
|
||||
return $calc->sort();
|
||||
}
|
||||
|
||||
/**
|
||||
@ -2354,11 +2349,7 @@ class UnitOfWork implements PropertyChangedListener
|
||||
*/
|
||||
public function getCommitOrderCalculator()
|
||||
{
|
||||
if ($this->commitOrderCalculator === null) {
|
||||
$this->commitOrderCalculator = new Internal\CommitOrderCalculator;
|
||||
}
|
||||
|
||||
return $this->commitOrderCalculator;
|
||||
return new Internal\CommitOrderCalculator();
|
||||
}
|
||||
|
||||
/**
|
||||
@ -2386,10 +2377,6 @@ class UnitOfWork implements PropertyChangedListener
|
||||
$this->readOnlyObjects =
|
||||
$this->visitedCollections =
|
||||
$this->orphanRemovals = array();
|
||||
|
||||
if ($this->commitOrderCalculator !== null) {
|
||||
$this->commitOrderCalculator->clear();
|
||||
}
|
||||
} else {
|
||||
$visited = array();
|
||||
|
||||
|
||||
@ -2,6 +2,7 @@
|
||||
|
||||
namespace Doctrine\Tests\ORM;
|
||||
|
||||
use Doctrine\ORM\Internal\CommitOrderCalculator;
|
||||
use Doctrine\ORM\Mapping\ClassMetadata;
|
||||
|
||||
/**
|
||||
@ -17,7 +18,7 @@ class CommitOrderCalculatorTest extends \Doctrine\Tests\OrmTestCase
|
||||
|
||||
protected function setUp()
|
||||
{
|
||||
$this->_calc = new \Doctrine\ORM\Internal\CommitOrderCalculator();
|
||||
$this->_calc = new CommitOrderCalculator();
|
||||
}
|
||||
|
||||
public function testCommitOrdering1()
|
||||
@ -28,21 +29,41 @@ class CommitOrderCalculatorTest extends \Doctrine\Tests\OrmTestCase
|
||||
$class4 = new ClassMetadata(__NAMESPACE__ . '\NodeClass4');
|
||||
$class5 = new ClassMetadata(__NAMESPACE__ . '\NodeClass5');
|
||||
|
||||
$this->_calc->addClass($class1);
|
||||
$this->_calc->addClass($class2);
|
||||
$this->_calc->addClass($class3);
|
||||
$this->_calc->addClass($class4);
|
||||
$this->_calc->addClass($class5);
|
||||
$this->_calc->addNode($class1->name, $class1);
|
||||
$this->_calc->addNode($class2->name, $class2);
|
||||
$this->_calc->addNode($class3->name, $class3);
|
||||
$this->_calc->addNode($class4->name, $class4);
|
||||
$this->_calc->addNode($class5->name, $class5);
|
||||
|
||||
$this->_calc->addDependency($class1, $class2);
|
||||
$this->_calc->addDependency($class2, $class3);
|
||||
$this->_calc->addDependency($class3, $class4);
|
||||
$this->_calc->addDependency($class5, $class1);
|
||||
$this->_calc->addDependency($class1->name, $class2->name, 1);
|
||||
$this->_calc->addDependency($class2->name, $class3->name, 1);
|
||||
$this->_calc->addDependency($class3->name, $class4->name, 1);
|
||||
$this->_calc->addDependency($class5->name, $class1->name, 1);
|
||||
|
||||
$sorted = $this->_calc->getCommitOrder();
|
||||
$sorted = $this->_calc->sort();
|
||||
|
||||
// There is only 1 valid ordering for this constellation
|
||||
$correctOrder = array($class5, $class1, $class2, $class3, $class4);
|
||||
|
||||
$this->assertSame($correctOrder, $sorted);
|
||||
}
|
||||
|
||||
public function testCommitOrdering2()
|
||||
{
|
||||
$class1 = new ClassMetadata(__NAMESPACE__ . '\NodeClass1');
|
||||
$class2 = new ClassMetadata(__NAMESPACE__ . '\NodeClass2');
|
||||
|
||||
$this->_calc->addNode($class1->name, $class1);
|
||||
$this->_calc->addNode($class2->name, $class2);
|
||||
|
||||
$this->_calc->addDependency($class1->name, $class2->name, 0);
|
||||
$this->_calc->addDependency($class2->name, $class1->name, 1);
|
||||
|
||||
$sorted = $this->_calc->sort();
|
||||
|
||||
// There is only 1 valid ordering for this constellation
|
||||
$correctOrder = array($class2, $class1);
|
||||
|
||||
$this->assertSame($correctOrder, $sorted);
|
||||
}
|
||||
}
|
||||
|
||||
155
tests/Doctrine/Tests/ORM/Functional/CascadeRemoveOrderTest.php
Normal file
155
tests/Doctrine/Tests/ORM/Functional/CascadeRemoveOrderTest.php
Normal file
@ -0,0 +1,155 @@
|
||||
<?php
|
||||
|
||||
namespace Doctrine\Tests\ORM\Functional;
|
||||
|
||||
use Doctrine\Common\Collections\ArrayCollection;
|
||||
|
||||
/**
|
||||
* @group CascadeRemoveOrderTest
|
||||
*/
|
||||
class CascadeRemoveOrderTest extends \Doctrine\Tests\OrmFunctionalTestCase
|
||||
{
|
||||
protected function setUp()
|
||||
{
|
||||
parent::setUp();
|
||||
|
||||
$this->_schemaTool->createSchema(array(
|
||||
$this->_em->getClassMetadata(__NAMESPACE__ . '\CascadeRemoveOrderEntityO'),
|
||||
$this->_em->getClassMetadata(__NAMESPACE__ . '\CascadeRemoveOrderEntityG'),
|
||||
));
|
||||
}
|
||||
|
||||
protected function tearDown()
|
||||
{
|
||||
parent::tearDown();
|
||||
|
||||
$this->_schemaTool->dropSchema(array(
|
||||
$this->_em->getClassMetadata(__NAMESPACE__ . '\CascadeRemoveOrderEntityO'),
|
||||
$this->_em->getClassMetadata(__NAMESPACE__ . '\CascadeRemoveOrderEntityG'),
|
||||
));
|
||||
}
|
||||
|
||||
public function testSingle()
|
||||
{
|
||||
$eO = new CascadeRemoveOrderEntityO();
|
||||
$eG = new CascadeRemoveOrderEntityG($eO);
|
||||
|
||||
$this->_em->persist($eO);
|
||||
$this->_em->flush();
|
||||
$this->_em->clear();
|
||||
|
||||
$eOloaded = $this->_em->find('Doctrine\Tests\ORM\Functional\CascadeRemoveOrderEntityO', $eO->getId());
|
||||
|
||||
$this->_em->remove($eOloaded);
|
||||
$this->_em->flush();
|
||||
}
|
||||
|
||||
public function testMany()
|
||||
{
|
||||
$eO = new CascadeRemoveOrderEntityO();
|
||||
$eG1 = new CascadeRemoveOrderEntityG($eO);
|
||||
$eG2 = new CascadeRemoveOrderEntityG($eO);
|
||||
$eG3 = new CascadeRemoveOrderEntityG($eO);
|
||||
|
||||
$eO->setOneToOneG($eG2);
|
||||
|
||||
$this->_em->persist($eO);
|
||||
$this->_em->flush();
|
||||
$this->_em->clear();
|
||||
|
||||
$eOloaded = $this->_em->find('Doctrine\Tests\ORM\Functional\CascadeRemoveOrderEntityO', $eO->getId());
|
||||
|
||||
$this->_em->remove($eOloaded);
|
||||
$this->_em->flush();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @Entity
|
||||
*/
|
||||
class CascadeRemoveOrderEntityO
|
||||
{
|
||||
/**
|
||||
* @Id @Column(type="integer")
|
||||
* @GeneratedValue
|
||||
*/
|
||||
private $id;
|
||||
|
||||
/**
|
||||
* @OneToOne(targetEntity="Doctrine\Tests\ORM\Functional\CascadeRemoveOrderEntityG")
|
||||
* @JoinColumn(nullable=true, onDelete="SET NULL")
|
||||
*/
|
||||
private $oneToOneG;
|
||||
|
||||
/**
|
||||
* @OneToMany(
|
||||
* targetEntity="Doctrine\Tests\ORM\Functional\CascadeRemoveOrderEntityG",
|
||||
* mappedBy="ownerO",
|
||||
* cascade={"persist", "remove"}
|
||||
* )
|
||||
*/
|
||||
private $oneToManyG;
|
||||
|
||||
|
||||
public function __construct()
|
||||
{
|
||||
$this->oneToManyG = new ArrayCollection();
|
||||
}
|
||||
|
||||
public function getId()
|
||||
{
|
||||
return $this->id;
|
||||
}
|
||||
|
||||
public function setOneToOneG(CascadeRemoveOrderEntityG $eG)
|
||||
{
|
||||
$this->oneToOneG = $eG;
|
||||
}
|
||||
|
||||
public function getOneToOneG()
|
||||
{
|
||||
return $this->oneToOneG;
|
||||
}
|
||||
|
||||
public function addOneToManyG(CascadeRemoveOrderEntityG $eG)
|
||||
{
|
||||
$this->oneToManyG->add($eG);
|
||||
}
|
||||
|
||||
public function getOneToManyGs()
|
||||
{
|
||||
return $this->oneToManyG->toArray();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @Entity
|
||||
*/
|
||||
class CascadeRemoveOrderEntityG
|
||||
{
|
||||
/**
|
||||
* @Id @Column(type="integer")
|
||||
* @GeneratedValue
|
||||
*/
|
||||
private $id;
|
||||
|
||||
/**
|
||||
* @ManyToOne(
|
||||
* targetEntity="Doctrine\Tests\ORM\Functional\CascadeRemoveOrderEntityO",
|
||||
* inversedBy="oneToMany"
|
||||
* )
|
||||
*/
|
||||
private $ownerO;
|
||||
|
||||
public function __construct(CascadeRemoveOrderEntityO $eO, $position=1)
|
||||
{
|
||||
$this->position = $position;
|
||||
$this->ownerO= $eO;
|
||||
$this->ownerO->addOneToManyG($this);
|
||||
}
|
||||
|
||||
public function getId()
|
||||
{
|
||||
return $this->id;
|
||||
}
|
||||
}
|
||||
@ -97,6 +97,27 @@ class OneToManyOrphanRemovalTest extends \Doctrine\Tests\OrmFunctionalTestCase
|
||||
$this->assertEquals(1, count($result), 'CmsPhonenumber should be removed by orphanRemoval');
|
||||
}
|
||||
|
||||
/**
|
||||
* @group DDC-2524
|
||||
*/
|
||||
public function testOrphanRemovalClearCollectionAndAddNew()
|
||||
{
|
||||
$user = $this->_em->find('Doctrine\Tests\Models\CMS\CmsUser', $this->userId);
|
||||
$newPhone = new CmsPhonenumber();
|
||||
|
||||
$newPhone->phonenumber = '654321';
|
||||
|
||||
$user->getPhonenumbers()->clear();
|
||||
$user->addPhonenumber($newPhone);
|
||||
|
||||
$this->_em->flush();
|
||||
|
||||
$query = $this->_em->createQuery('SELECT p FROM Doctrine\Tests\Models\CMS\CmsPhonenumber p');
|
||||
$result = $query->getResult();
|
||||
|
||||
$this->assertEquals(1, count($result), 'Old CmsPhonenumbers should be removed by orphanRemoval and new one added');
|
||||
}
|
||||
|
||||
/**
|
||||
* @group DDC-1496
|
||||
*/
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user