Skip to content

Commit

Permalink
Tests around reported cases over DDC-2524
Browse files Browse the repository at this point in the history
  • Loading branch information
guilhermeblanco committed Nov 27, 2015
1 parent 57b22bd commit 8ea62b9
Show file tree
Hide file tree
Showing 5 changed files with 333 additions and 122 deletions.
197 changes: 112 additions & 85 deletions lib/Doctrine/ORM/Internal/CommitOrderCalculator.php
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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>
* @since 2.0
* @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
*/
private $_nodeStates = array();

/**
* The nodes to sort.
* Matrix of nodes (aka. vertex).
* Keys are provided hashes and values are the node definition objects.
*
* @var array
*/
private $_classes = array();

/**
* @var array
* 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 $_relatedClasses = array();
private $nodeList = array();

/**
* Volatile variable holding calculated nodes during sorting process.
*
* @var array
*/
private $_sorted = array();
private $sortedNodeList = 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.
* The desired topological sorting is the reverse postorder of these searches.
* @param string $hash
* @param object $node
*
* @return array The list of ordered classes.
* @return void
*/
public function getCommitOrder()
public function addNode($hash, $node)
{
// Check whether we need to do anything. 0 or 1 node is easy.
$nodeCount = count($this->_classes);
$vertex = new \stdClass();

if ($nodeCount <= 1) {
return ($nodeCount == 1) ? array_values($this->_classes) : array();
}

// Init
foreach ($this->_classes as $node) {
$this->_nodeStates[$node->name] = self::NOT_VISITED;
}
$vertex->hash = $hash;
$vertex->state = self::NOT_VISITED;
$vertex->value = $node;
$vertex->dependencyList = array();

// Go
foreach ($this->_classes as $node) {
if ($this->_nodeStates[$node->name] == self::NOT_VISITED) {
$this->_visitNode($node);
}
}

$sorted = array_reverse($this->_sorted);

$this->_sorted = $this->_nodeStates = array();

return $sorted;
$this->nodeList[$hash] = $vertex;
}

/**
* @param \Doctrine\ORM\Mapping\ClassMetadata $node
* Adds a new dependency (edge) to the graph using their hashes.
*
* @param string $fromHash
* @param string $toHash
* @param integer $weight
*
* @return void
*/
private function _visitNode($node)
public function addDependency($fromHash, $toHash, $weight)
{
$this->_nodeStates[$node->name] = self::IN_PROGRESS;
$vertex = $this->nodeList[$fromHash];
$edge = new \stdClass();

if (isset($this->_relatedClasses[$node->name])) {
foreach ($this->_relatedClasses[$node->name] as $relatedNode) {
if ($this->_nodeStates[$relatedNode->name] == self::NOT_VISITED) {
$this->_visitNode($relatedNode);
}
}
}
$edge->from = $fromHash;
$edge->to = $toHash;
$edge->weight = $weight;

$this->_nodeStates[$node->name] = self::VISITED;
$this->_sorted[] = $node;
$vertex->dependencyList[$toHash] = $edge;
}

/**
* @param \Doctrine\ORM\Mapping\ClassMetadata $fromClass
* @param \Doctrine\ORM\Mapping\ClassMetadata $toClass
* Return a valid order list of all current nodes.
* The desired topological sorting is the reverse post order of these searches.
*
* @return void
*/
public function addDependency($fromClass, $toClass)
{
$this->_relatedClasses[$fromClass->name][] = $toClass;
}

/**
* @param string $className
* {@internal Highly performance-sensitive method.}
*
* @return bool
* @return array
*/
public function hasClass($className)
public function sort()
{
return isset($this->_classes[$className]);
foreach ($this->nodeList as $vertex) {
if ($vertex->state !== self::NOT_VISITED) {
continue;
}

$this->visit($vertex);
}

$sortedList = $this->sortedNodeList;

$this->nodeList = array();
$this->sortedNodeList = array();

return array_reverse($sortedList);
}

/**
* @param \Doctrine\ORM\Mapping\ClassMetadata $class
* Visit a given node definition for reordering.
*
* @return void
* {@internal Highly performance-sensitive method.}
*
* @param \stdClass $vertex
*/
public function addClass($class)
private function visit($vertex)
{
$this->_classes[$class->name] = $class;
$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);
}
}

if ($vertex->state !== self::VISITED) {
$vertex->state = self::VISITED;

$this->sortedNodeList[] = $vertex->value;
}
}
}
}
39 changes: 13 additions & 26 deletions lib/Doctrine/ORM/UnitOfWork.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -1138,11 +1130,11 @@ private function getCommitOrder(array $entityChangeSet = null)
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;
}
Expand All @@ -1156,13 +1148,16 @@ private function getCommitOrder(array $entityChangeSet = null)

$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) {
Expand All @@ -1172,18 +1167,18 @@ private function getCommitOrder(array $entityChangeSet = null)
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();
}

/**
Expand Down Expand Up @@ -2354,11 +2349,7 @@ public function lock($entity, $lockMode, $lockVersion = null)
*/
public function getCommitOrderCalculator()
{
if ($this->commitOrderCalculator === null) {
$this->commitOrderCalculator = new Internal\CommitOrderCalculator;
}

return $this->commitOrderCalculator;
return new Internal\CommitOrderCalculator();
}

/**
Expand Down Expand Up @@ -2386,10 +2377,6 @@ public function clear($entityName = null)
$this->readOnlyObjects =
$this->visitedCollections =
$this->orphanRemovals = array();

if ($this->commitOrderCalculator !== null) {
$this->commitOrderCalculator->clear();
}
} else {
$visited = array();

Expand Down
Loading

0 comments on commit 8ea62b9

Please sign in to comment.