Skip to content

Commit

Permalink
Merge pull request #171 from uwej711/reorder_children
Browse files Browse the repository at this point in the history
Implement reorder - please review and comment
  • Loading branch information
lsmith77 committed Sep 10, 2012
2 parents 059f3de + 9442ebd commit 8cd7a38
Show file tree
Hide file tree
Showing 4 changed files with 307 additions and 31 deletions.
56 changes: 39 additions & 17 deletions lib/Doctrine/ODM/PHPCR/DocumentManager.php
Expand Up @@ -539,7 +539,7 @@ public function getDocumentsByQuery(QueryInterface $query, $className = null)
public function persist($document)
{
if (!is_object($document)) {
throw new \InvalidArgumentException(gettype($document));
throw new \InvalidArgumentException('Parameter $document needs to be an object, '.gettype($document).' given');
}

$this->errorIfClosed();
Expand All @@ -559,7 +559,7 @@ public function persist($document)
public function bindTranslation($document, $locale)
{
if (!is_object($document)) {
throw new \InvalidArgumentException(gettype($document));
throw new \InvalidArgumentException('Parameter $document needs to be an object, '.gettype($document).' given');
}

$this->errorIfClosed();
Expand All @@ -579,7 +579,7 @@ public function bindTranslation($document, $locale)
public function getLocalesFor($document)
{
if (!is_object($document)) {
throw new \InvalidArgumentException(gettype($document));
throw new \InvalidArgumentException('Parameter $document needs to be an object, '.gettype($document).' given');
}

$this->errorIfClosed();
Expand All @@ -600,7 +600,7 @@ public function getLocalesFor($document)
public function move($document, $targetPath)
{
if (!is_object($document)) {
throw new \InvalidArgumentException(gettype($document));
throw new \InvalidArgumentException('Parameter $document needs to be an object, '.gettype($document).' given');
}

if (strpos($targetPath, '/') !== 0) {
Expand All @@ -611,6 +611,28 @@ public function move($document, $targetPath)
$this->unitOfWork->scheduleMove($document, $targetPath);
}

/**
* Reorder a child of the given document
*
* Note that this does not update the fields with @Child/Children annotations.
* If you want to continue working with the manager after a reorder, you are probably
* safest calling DocumentManager::clear and re-loading the documents you need to use.
*
* @param object $document an already registered document
* @param string $srcName the nodename of the child to be reordered
* @param string $targetName the nodename of the target of the reordering
* @param boolean $before insert before or after the target
*/
public function reorder($document, $srcName, $targetName, $before)
{
if (!is_object($document)) {
throw new \InvalidArgumentException('Parameter $document needs to be an object, '.gettype($document).' given');
}

$this->errorIfClosed();
$this->unitOfWork->scheduleReorder($document, $srcName, $targetName, $before);
}

/**
* Remove the previously persisted document and all its children from the tree
*
Expand All @@ -623,7 +645,7 @@ public function move($document, $targetPath)
public function remove($document)
{
if (!is_object($document)) {
throw new \InvalidArgumentException(gettype($document));
throw new \InvalidArgumentException('Parameter $document needs to be an object, '.gettype($document).' given');
}

$this->errorIfClosed();
Expand All @@ -640,7 +662,7 @@ public function remove($document)
public function merge($document)
{
if (!is_object($document)) {
throw new \InvalidArgumentException(gettype($document));
throw new \InvalidArgumentException('Parameter $document needs to be an object, '.gettype($document).' given');
}

throw new \BadMethodCallException(__METHOD__.' not yet implemented');
Expand All @@ -662,7 +684,7 @@ public function merge($document)
public function detach($document)
{
if (!is_object($document)) {
throw new \InvalidArgumentException(gettype($document));
throw new \InvalidArgumentException('Parameter $document needs to be an object, '.gettype($document).' given');
}

$this->errorIfClosed();
Expand All @@ -678,7 +700,7 @@ public function detach($document)
public function refresh($document)
{
if (!is_object($document)) {
throw new \InvalidArgumentException(gettype($document));
throw new \InvalidArgumentException('Parameter $document needs to be an object, '.gettype($document).' given');
}

$this->errorIfClosed();
Expand All @@ -704,7 +726,7 @@ public function refresh($document)
public function getChildren($document, $filter = null, $fetchDepth = null)
{
if (!is_object($document)) {
throw new \InvalidArgumentException(gettype($document));
throw new \InvalidArgumentException('Parameter $document needs to be an object, '.gettype($document).' given');
}

$this->errorIfClosed();
Expand All @@ -725,7 +747,7 @@ public function getChildren($document, $filter = null, $fetchDepth = null)
public function getReferrers($document, $type = null, $name = null)
{
if (!is_object($document)) {
throw new \InvalidArgumentException(gettype($document));
throw new \InvalidArgumentException('Parameter $document needs to be an object, '.gettype($document).' given');
}

$this->errorIfClosed();
Expand All @@ -741,7 +763,7 @@ public function getReferrers($document, $type = null, $name = null)
public function flush($document = null)
{
if (null !== $document && !is_object($document) && !is_array($document)) {
throw new \InvalidArgumentException(gettype($document));
throw new \InvalidArgumentException('Parameter $document needs to be an object, '.gettype($document).' given');
}

$this->errorIfClosed();
Expand Down Expand Up @@ -793,7 +815,7 @@ public function getReference($documentName, $id)
public function checkin($document)
{
if (!is_object($document)) {
throw new \InvalidArgumentException(gettype($document));
throw new \InvalidArgumentException('Parameter $document needs to be an object, '.gettype($document).' given');
}

$this->errorIfClosed();
Expand All @@ -808,7 +830,7 @@ public function checkin($document)
public function checkout($document)
{
if (!is_object($document)) {
throw new \InvalidArgumentException(gettype($document));
throw new \InvalidArgumentException('Parameter $document needs to be an object, '.gettype($document).' given');
}

$this->errorIfClosed();
Expand All @@ -827,7 +849,7 @@ public function checkout($document)
public function checkpoint($document)
{
if (!is_object($document)) {
throw new \InvalidArgumentException(gettype($document));
throw new \InvalidArgumentException('Parameter $document needs to be an object, '.gettype($document).' given');
}

$this->errorIfClosed();
Expand Down Expand Up @@ -887,7 +909,7 @@ public function removeVersion($documentVersion)
public function getAllLinearVersions($document, $limit = -1)
{
if (!is_object($document)) {
throw new \InvalidArgumentException(gettype($document));
throw new \InvalidArgumentException('Parameter $document needs to be an object, '.gettype($document).' given');
}

$this->errorIfClosed();
Expand Down Expand Up @@ -928,7 +950,7 @@ public function findVersionByName($className, $id, $versionName)
public function contains($document)
{
if (!is_object($document)) {
throw new \InvalidArgumentException(gettype($document));
throw new \InvalidArgumentException('Parameter $document needs to be an object, '.gettype($document).' given');
}

return $this->unitOfWork->contains($document);
Expand Down Expand Up @@ -983,7 +1005,7 @@ public function close()
public function initializeObject($document)
{
if (!is_object($document)) {
throw new \InvalidArgumentException(gettype($document));
throw new \InvalidArgumentException('Parameter $document needs to be an object, '.gettype($document).' given');
}

$this->unitOfWork->initializeObject($document);
Expand Down
94 changes: 81 additions & 13 deletions lib/Doctrine/ODM/PHPCR/UnitOfWork.php
Expand Up @@ -56,7 +56,6 @@ class UnitOfWork
const STATE_MANAGED = 2;
const STATE_REMOVED = 3;
const STATE_DETACHED = 4;
const STATE_MOVED = 5;

/**
* @var DocumentManager
Expand Down Expand Up @@ -140,6 +139,11 @@ class UnitOfWork
*/
private $scheduledMoves = array();

/**
* @var array
*/
private $scheduledReorders = array();

/**
* @var array
*/
Expand Down Expand Up @@ -452,7 +456,7 @@ public function refreshDocumentForProxy($className, Proxy $document)
public function bindTranslation($document, $locale)
{
$state = $this->getDocumentState($document);
if ($state !== self::STATE_MANAGED && $state !== self::STATE_MOVED) {
if ($state !== self::STATE_MANAGED) {
throw new \InvalidArgumentException('Document has to be managed to be able to bind a translation '.self::objToStr($document, $this->dm));
}

Expand Down Expand Up @@ -515,10 +519,6 @@ private function doScheduleInsert($document, &$visited, $overrideIdGenerator = n
case self::STATE_MANAGED:
// TODO: Change Tracking Deferred Explicit
break;
case self::STATE_MOVED:
unset($this->scheduledMoves[$oid]);
$this->setDocumentState($oid, self::STATE_MANAGED);
break;
case self::STATE_REMOVED:
unset($this->scheduledRemovals[$oid]);
$this->setDocumentState($oid, self::STATE_MANAGED);
Expand Down Expand Up @@ -628,7 +628,27 @@ public function scheduleMove($document, $targetPath)
}

$this->scheduledMoves[$oid] = array($document, $targetPath);
$this->setDocumentState($oid, self::STATE_MOVED);
$this->setDocumentState($oid, self::STATE_MANAGED);
}

public function scheduleReorder($document, $srcName, $targetName, $before)
{
$oid = spl_object_hash($document);

$state = $this->getDocumentState($document);
switch ($state) {
case self::STATE_NEW:
unset($this->scheduledInserts[$oid]);
break;
case self::STATE_REMOVED:
unset($this->scheduledRemovals[$oid]);
break;
case self::STATE_DETACHED:
throw new \InvalidArgumentException('Detached document passed to reorder(): '.self::objToStr($document, $this->dm));
}

$this->scheduledReorders[$oid] = array($document, $srcName, $targetName, $before);
$this->setDocumentState($oid, self::STATE_MANAGED);
}

public function scheduleRemove($document)
Expand All @@ -640,8 +660,9 @@ public function scheduleRemove($document)
case self::STATE_NEW:
unset($this->scheduledInserts[$oid]);
break;
case self::STATE_MOVED:
case self::STATE_MANAGED:
unset($this->scheduledMoves[$oid]);
unset($this->scheduledReorders[$oid]);
break;
case self::STATE_DETACHED:
throw new \InvalidArgumentException('Detached document passed to remove(): '.self::objToStr($document, $this->dm));
Expand Down Expand Up @@ -722,7 +743,7 @@ private function getDocumentState($document)
private function computeSingleDocumentChangeSet($document)
{
$state = $this->getDocumentState($document);
if ($state !== self::STATE_MANAGED && $state !== self::STATE_MOVED) {
if ($state !== self::STATE_MANAGED) {
throw new \InvalidArgumentException('Document has to be managed or moved for single computation '.self::objToStr($document, $this->dm));
}

Expand Down Expand Up @@ -752,7 +773,7 @@ private function computeChangeSets()
{
foreach ($this->identityMap as $document) {
$state = $this->getDocumentState($document);
if ($state === self::STATE_MANAGED || $state === self::STATE_MOVED) {
if ($state === self::STATE_MANAGED) {
$class = $this->dm->getClassMetadata(get_class($document));
$this->computeChangeSet($class, $document);
}
Expand Down Expand Up @@ -862,7 +883,7 @@ private function computeChangeSet(ClassMetadata $class, $document)
$parentClass = $this->dm->getClassMetadata(get_class($parent));
$state = $this->getDocumentState($parent);

if ($state === self::STATE_MANAGED || $state === self::STATE_MOVED) {
if ($state === self::STATE_MANAGED) {
$this->computeChangeSet($parentClass, $parent);
}
}
Expand Down Expand Up @@ -1143,7 +1164,7 @@ public function persistNew($class, $document, $overrideIdGenerator = null, $pare
if ($this->evm->hasListeners(Event::prePersist)) {
$this->evm->dispatchEvent(Event::prePersist, new LifecycleEventArgs($document, $this->dm));
}

$generator = $overrideIdGenerator ? $overrideIdGenerator : $class->idGenerator;

$id = $this->getIdGenerator($generator)->generate($document, $class, $this->dm, $parent);
Expand Down Expand Up @@ -1186,7 +1207,6 @@ private function doDetach($document, array &$visited)
$state = $this->getDocumentState($document);
switch ($state) {
case self::STATE_MANAGED:
case self::STATE_MOVED:
$this->unregisterDocument($document);
break;
case self::STATE_NEW:
Expand Down Expand Up @@ -1271,6 +1291,8 @@ public function commit($document = null)

$this->executeRemovals($this->scheduledRemovals);

$this->executeReorders($this->scheduledReorders);

$this->session->save();

if (!empty($this->scheduledMoves)) {
Expand Down Expand Up @@ -1309,6 +1331,7 @@ public function commit($document = null)
$this->scheduledAssociationUpdates =
$this->scheduledRemovals =
$this->scheduledMoves =
$this->scheduledReorders =
$this->scheduledInserts =
$this->visitedCollections =
$this->documentChangesets =
Expand Down Expand Up @@ -1640,6 +1663,49 @@ private function executeMoves($documents)
}
}

/**
* Execute reorderings
*
* @param $documents
*/
private function executeReorders($documents)
{
foreach ($documents as $oid => $value) {
if (!$this->contains($oid)) {
continue;
}
list($parent, $src, $target, $before) = $value;

$parentNode = $this->session->getNode($this->getDocumentId($parent));
$children = $parentNode->getNodes();

// check for src and target ...
$dest = $target;
if (isset($children[$src]) && isset($children[$target])) {
// there is no orderAfter, so we need to find the child after target to use it in orderBefore
if (!$before) {
$dest = null;
$found = false;
foreach ($children as $name => $child) {
if ($name === $target) {
$found = true;
} elseif ($found) {
$dest = $name;
break;
}
}
}
$parentNode->orderBefore($src, $dest);
// set all children collection to initialized = false to force reload after reordering
$class = $this->dm->getClassMetadata(get_class($parent));
foreach ($class->childrenMappings as $childrenMapping) {
$children = $class->reflFields[$childrenMapping['fieldName']]->getValue($parent);
$children->setInitialized(false);
}
}
}
}

/**
* Executes all document removals
*
Expand Down Expand Up @@ -1839,6 +1905,7 @@ private function unregisterDocument($document)
unset($this->scheduledRemovals[$oid],
$this->scheduledUpdates[$oid],
$this->scheduledMoves[$oid],
$this->scheduledReorders[$oid],
$this->scheduledInserts[$oid],
$this->scheduledAssociationUpdates[$oid],
$this->originalData[$oid],
Expand Down Expand Up @@ -2020,6 +2087,7 @@ public function clear()
$this->scheduledAssociationUpdates =
$this->scheduledInserts =
$this->scheduledMoves =
$this->scheduledReorders =
$this->scheduledRemovals =
$this->visitedCollections =
$this->documentHistory =
Expand Down
2 changes: 1 addition & 1 deletion tests/Doctrine/Tests/ODM/PHPCR/Functional/MoveTest.php
Expand Up @@ -79,7 +79,7 @@ public function testMoveWithPersist()
$this->dm->flush();
$this->dm->clear();

$user = $this->dm->find($this->type, '/functional/lsmith');
$user = $this->dm->find($this->type, '/functional/user2');
$this->assertNotNull($user, 'User must exist');
}

Expand Down

0 comments on commit 8cd7a38

Please sign in to comment.