diff --git a/lib/Doctrine/ODM/PHPCR/.DocumentManager.php.swl b/lib/Doctrine/ODM/PHPCR/.DocumentManager.php.swl new file mode 100644 index 000000000..b4e58930d Binary files /dev/null and b/lib/Doctrine/ODM/PHPCR/.DocumentManager.php.swl differ diff --git a/lib/Doctrine/ODM/PHPCR/DocumentManager.php b/lib/Doctrine/ODM/PHPCR/DocumentManager.php index 9241e92e0..eb44168e4 100644 --- a/lib/Doctrine/ODM/PHPCR/DocumentManager.php +++ b/lib/Doctrine/ODM/PHPCR/DocumentManager.php @@ -389,7 +389,9 @@ public function findMany($className, array $ids) } $nodes = $this->session->getNodes($ids); - $hints = array('fallback' => true); + $hints = array( + 'fallback' => true, + ); $documents = $this->unitOfWork->getOrCreateDocuments($className, $nodes, $hints); return new ArrayCollection($documents); diff --git a/lib/Doctrine/ODM/PHPCR/Queue/TreeOperation.php b/lib/Doctrine/ODM/PHPCR/Queue/TreeOperation.php new file mode 100644 index 000000000..6d9451fa9 --- /dev/null +++ b/lib/Doctrine/ODM/PHPCR/Queue/TreeOperation.php @@ -0,0 +1,104 @@ +. + */ + +namespace Doctrine\ODM\PHPCR\Queue; + +/** + * Represents a horizontal tree operation, encompassing + * MOVE, REMOVE and INSERT operations. + * + * @author Daniel Leech + */ +class TreeOperation +{ + const OP_MOVE = 'move'; + const OP_REMOVE = 'remove'; + const OP_INSERT = 'insert'; + + private $oid; + private $type; + private $args; + private $valid = true; + + /** + * @param mixed $type + * @param mixed $oid + * @param mixed $args + */ + public function __construct($type, $oid, $args) + { + $this->oid = $oid; + $this->type = $type; + $this->args = $args; + } + + /** + * Return the SPL object ID which maps + * to the document on which the operaton should be + * performed. + * + * @return string + */ + public function getOid() + { + return $this->oid; + } + + /** + * Return the type of operation, one of + * the self::OP_* constants. + * + * @return string + */ + public function getType() + { + return $this->type; + } + + /** + * Return the arguments for the operation + * + * @return mixed + */ + public function getArgs() + { + return $this->args; + } + + /** + * Invalidate this item so that it will not be processed. + * Invalidation is quicker than removing the item from the queue. + * + * @param boolean + */ + public function invalidate() + { + $this->valid = false; + } + + /** + * Return true if this item should be processed + * + * @return boolean + */ + public function isValid() + { + return $this->valid; + } +} diff --git a/lib/Doctrine/ODM/PHPCR/Queue/TreeOperationBatch.php b/lib/Doctrine/ODM/PHPCR/Queue/TreeOperationBatch.php new file mode 100644 index 000000000..86a32728b --- /dev/null +++ b/lib/Doctrine/ODM/PHPCR/Queue/TreeOperationBatch.php @@ -0,0 +1,75 @@ +. + */ + +namespace Doctrine\ODM\PHPCR\Queue; + +/** + * Represents a batch of operations of the same type + * + * @author Daniel Leech + */ +class TreeOperationBatch +{ + private $type; + private $schedule = array(); + + /** + * @param mixed $type + */ + public function __construct($type) + { + $this->type = $type; + } + + /** + * Return the operation schedule, e.g. + * + * array( + * $oid => $args + * ) + * + * @return array + */ + public function getSchedule() + { + return $this->schedule; + } + + /** + * Schedule an operation + * + * @param string $oid + * @param array $args + */ + public function schedule($oid, $args) + { + $this->schedule[$oid] = $args; + } + + /** + * Return the operation type, one of the + * TreeOperation::OP_* constants. + * + * @return string + */ + public function getType() + { + return $this->type; + } +} diff --git a/lib/Doctrine/ODM/PHPCR/Queue/TreeOperationQueue.php b/lib/Doctrine/ODM/PHPCR/Queue/TreeOperationQueue.php new file mode 100644 index 000000000..1ee4dd713 --- /dev/null +++ b/lib/Doctrine/ODM/PHPCR/Queue/TreeOperationQueue.php @@ -0,0 +1,169 @@ +. + */ + +namespace Doctrine\ODM\PHPCR\Queue; + +use Doctrine\ODM\PHPCR\Queue\TreeOperation; +use Doctrine\ODM\PHPCR\Queue\TreeOperationBatch; + +/** + * The tree operation queue + * + * @author Daniel Leech + */ +class TreeOperationQueue +{ + private $queue = array(); + + /** + * Push a new operation onto the queue + * + * @param TreeOperation $operation + */ + public function push(TreeOperation $operation) + { + $this->queue[] = $operation; + } + + /** + * Partition into contiguous sets of operations + * + * @return TreeOperationBatch[] + */ + public function getBatches() + { + $type = null; + $batches = array(); + + foreach ($this->queue as $operation) { + if (false === $operation->isValid()) { + continue; + } + + if ($operation->getType() !== $type) { + $batch = new TreeOperationBatch($operation->getType()); + $type = $operation->getType(); + $batches[] = $batch; + } + + $batch->schedule( + $operation->getOid(), + $operation->getArgs() + ); + } + + return $batches; + } + + /** + * Clear the queue + */ + public function clear() + { + $this->queue = array(); + } + + /** + * Return the entire schedule for the given operation type + * + * @param string $type + * + * @return array + */ + public function getSchedule($type) + { + $schedule = array(); + + foreach ($this->queue as $operation) { + if (false === $operation->isValid()) { + continue; + } + + if ($type !== $operation->getType()) { + continue; + } + + $schedule[$operation->getOid()] = $operation->getArgs(); + } + + return $schedule; + } + + /** + * Return true if the spl object id is scheduled for the given type + * + * @param string $type + * @param string $oid + * + * @return boolean + */ + public function isQueued($type, $oid) + { + foreach ($this->queue as $operation) { + if (false === $operation->isValid()) { + continue; + } + + if ($operation->getType() !== $type) { + continue; + } + + if ($oid == $operation->getOid()) { + return true; + } + } + + return false; + } + + /** + * Remove all operations with the given spl object ID and type + * + * @param string $type + * @param string $oid + */ + public function unqueue($type, $oid) + { + foreach ($this->queue as $operation) { + if ($operation->getType() !== $type) { + continue; + } + + if ($operation->getOid() !== $oid) { + continue; + } + + $operation->invalidate(); + } + } + + /** + * Remove all references to the given spl object ID + * + * @param $oid string + */ + public function unregister($oid) + { + foreach ($this->queue as $operation) { + if ($operation->getOid() == $oid) { + $operation->invalidate(); + } + } + } +} diff --git a/lib/Doctrine/ODM/PHPCR/UnitOfWork.php b/lib/Doctrine/ODM/PHPCR/UnitOfWork.php index 8ddd01814..c3648eaf7 100644 --- a/lib/Doctrine/ODM/PHPCR/UnitOfWork.php +++ b/lib/Doctrine/ODM/PHPCR/UnitOfWork.php @@ -57,6 +57,8 @@ use PHPCR\Util\NodeHelper; use Jackalope\Session as JackalopeSession; +use Doctrine\ODM\PHPCR\Queue\TreeOperation; +use Doctrine\ODM\PHPCR\Queue\TreeOperationQueue; /** * Unit of work class @@ -168,20 +170,6 @@ class UnitOfWork */ private $scheduledUpdates = array(); - /** - * List of documents that will be inserted on next flush - * oid => document - * @var array - */ - private $scheduledInserts = array(); - - /** - * List of documents that will be moved on next flush - * oid => array(document, target path) - * @var array - */ - private $scheduledMoves = array(); - /** * List of parent documents that have children that will be reordered on next flush * parent oid => list of array with records array(parent document, srcName, targetName, before) with @@ -194,13 +182,6 @@ class UnitOfWork */ private $scheduledReorders = array(); - /** - * List of documents that will be removed on next flush - * oid => document - * @var array - */ - private $scheduledRemovals = array(); - /** * @var array */ @@ -260,6 +241,11 @@ class UnitOfWork */ private $useFetchDepth; + /** + * @var TreeOperationQueue + */ + private $treeOpQueue; + /** * @param DocumentManager $dm */ @@ -269,6 +255,7 @@ public function __construct(DocumentManager $dm) $this->session = $dm->getPhpcrSession(); $this->eventListenersInvoker = new ListenersInvoker($dm); $this->eventManager = $dm->getEventManager(); + $this->treeOpQueue = new TreeOperationQueue(); $config = $dm->getConfiguration(); $this->documentClassMapper = $config->getDocumentClassMapper(); @@ -813,7 +800,7 @@ private function doScheduleInsert($document, &$visited, $overrideIdGenerator = n // TODO: Change Tracking Deferred Explicit break; case self::STATE_REMOVED: - unset($this->scheduledRemovals[$oid]); + $this->treeOpQueue->unqueue(TreeOperation::OP_REMOVE, $oid); $this->setDocumentState($oid, self::STATE_MANAGED); break; case self::STATE_DETACHED: @@ -927,16 +914,22 @@ public function scheduleMove($document, $targetPath) switch ($state) { case self::STATE_NEW: - unset($this->scheduledInserts[$oid]); break; case self::STATE_REMOVED: - unset($this->scheduledRemovals[$oid]); + $this->treeOpQueue->unqueue(TreeOperation::OP_REMOVE, $oid); break; case self::STATE_DETACHED: throw new InvalidArgumentException('Detached document passed to move(): '.self::objToStr($document, $this->dm)); } - $this->scheduledMoves[$oid] = array($document, $targetPath); + $this->treeOpQueue->push( + new TreeOperation( + TreeOperation::OP_MOVE, + $oid, + array($document, $targetPath) + ) + ); + $this->setDocumentState($oid, self::STATE_MANAGED); } @@ -975,17 +968,20 @@ private function doRemove($document, &$visited) $state = $this->getDocumentState($document); switch ($state) { case self::STATE_NEW: - unset($this->scheduledInserts[$oid]); - break; 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)); } - $this->scheduledRemovals[$oid] = $document; + $this->treeOpQueue->push( + new TreeOperation( + TreeOperation::OP_REMOVE, + $oid, + $document + ) + ); + $this->setDocumentState($oid, self::STATE_REMOVED); $class = $this->dm->getClassMetadata(get_class($document)); @@ -1100,7 +1096,7 @@ public function getDocumentState($document) */ public function isScheduledForInsert($document) { - return isset($this->scheduledInserts[spl_object_hash($document)]); + return $this->treeOpQueue->isQueued(TreeOperation::OP_INSERT); } /** @@ -1115,7 +1111,8 @@ public function computeSingleDocumentChangeSet($document) throw new InvalidArgumentException('Document has to be managed for single computation '.self::objToStr($document, $this->dm)); } - foreach ($this->scheduledInserts as $insertedDocument) { + $insertSchedule = $this->treeOpQueue->getSchedule(TreeOperation::OP_INSERT); + foreach ($insertSchedule as $insertedDocument) { $class = $this->dm->getClassMetadata(get_class($insertedDocument)); $this->computeChangeSet($class, $insertedDocument); } @@ -1126,7 +1123,7 @@ public function computeSingleDocumentChangeSet($document) } $oid = spl_object_hash($document); - if (!isset($this->scheduledInserts[$oid])) { + if (!isset($insertSchedule[$oid])) { $class = $this->dm->getClassMetadata(get_class($document)); $this->computeChangeSet($class, $document); } @@ -1577,7 +1574,6 @@ public function computeChangeSet(ClassMetadata $class, $document) if ($isNew) { $this->documentChangesets[$oid]['fields'] = $fields; - $this->scheduledInserts[$oid] = $document; return; } @@ -1769,6 +1765,14 @@ public function persistNew(ClassMetadata $class, $document, $overrideIdGenerator $id = $generator->generate($document, $class, $this->dm, $parent); $this->registerDocument($document, $id); + $this->treeOpQueue->push( + new TreeOperation( + TreeOperation::OP_INSERT, + spl_object_hash($document), + $document + ) + ); + if (!$generator instanceof AssignedIdGenerator) { $class->setIdentifierValue($document, $id); } @@ -2181,13 +2185,13 @@ public function commit($document = null) } $this->invokeGlobalEvent(Event::onFlush, new ManagerEventArgs($this->dm)); + $opBatches = $this->treeOpQueue->getBatches(); - if (empty($this->scheduledInserts) + if ( + empty($opBatches) && empty($this->scheduledUpdates) - && empty($this->scheduledRemovals) && empty($this->scheduledReorders) && empty($this->documentTranslations) - && empty($this->scheduledMoves) ) { $this->invokeGlobalEvent(Event::postFlush, new ManagerEventArgs($this->dm)); $this->changesetComputed = array(); @@ -2211,15 +2215,23 @@ public function commit($document = null) } try { - $this->executeInserts($this->scheduledInserts); - - $this->executeUpdates($this->scheduledUpdates); - $this->executeRemovals($this->scheduledRemovals); + foreach ($opBatches as $opBatch) { + switch ($opBatch->getType()) { + case TreeOperation::OP_INSERT: + $this->executeInserts($opBatch->getSchedule()); + continue; + case TreeOperation::OP_REMOVE: + $this->executeRemovals($opBatch->getSchedule()); + continue; + case TreeOperation::OP_MOVE: + $this->executeMoves($opBatch->getSchedule()); + continue; + } + } $this->executeReorders($this->scheduledReorders); - - $this->executeMoves($this->scheduledMoves); + $this->executeUpdates($this->scheduledUpdates); $this->session->save(); @@ -2259,11 +2271,9 @@ public function commit($document = null) } } + $this->treeOpQueue->clear(); $this->scheduledUpdates = - $this->scheduledRemovals = - $this->scheduledMoves = $this->scheduledReorders = - $this->scheduledInserts = $this->visitedCollections = $this->documentChangesets = $this->changesetComputed = array(); @@ -2281,10 +2291,6 @@ private function executeInserts($documents) // sort the documents to insert parents first but maintain child order $oids = array(); foreach ($documents as $oid => $document) { - if (!$this->contains($oid)) { - continue; - } - $oids[$oid] = $this->getDocumentId($document); } @@ -2318,7 +2324,9 @@ private function executeInserts($documents) && !$class->isNullable($fieldName) && !$this->isAutocreatedProperty($class, $fieldName) ) { - throw new PHPCRException(sprintf('Field "%s" of class "%s" is not nullable', $fieldName, $class->name)); + if (!$this->treeOpQueue->isQueued(TreeOperation::OP_REMOVE, $oid)) { + throw new PHPCRException(sprintf('Field "%s" of class "%s" is not nullable', $fieldName, $class->name)); + } } } @@ -3026,11 +3034,11 @@ private function unregisterDocument($document) unset($this->identityMap[$this->documentIds[$oid]]); } - unset($this->scheduledRemovals[$oid], + $this->treeOpQueue->unregister($oid); + + unset( $this->scheduledUpdates[$oid], - $this->scheduledMoves[$oid], $this->scheduledReorders[$oid], - $this->scheduledInserts[$oid], $this->originalData[$oid], $this->originalTranslatedData[$oid], $this->documentIds[$oid], @@ -3075,7 +3083,7 @@ public function contains($document) { $oid = is_object($document) ? spl_object_hash($document) : $document; - return isset($this->documentIds[$oid]) && !isset($this->scheduledRemovals[$oid]); + return isset($this->documentIds[$oid]) && !$this->treeOpQueue->isQueued(TreeOperation::OP_REMOVE, $oid); } /** @@ -3177,14 +3185,13 @@ public function clear() $this->documentChangesets = $this->changesetComputed = $this->scheduledUpdates = - $this->scheduledInserts = - $this->scheduledMoves = $this->scheduledReorders = - $this->scheduledRemovals = $this->visitedCollections = $this->documentHistory = $this->documentVersion = array(); + $this->treeOpQueue->clear(); + $this->invokeGlobalEvent(Event::onClear, new OnClearEventArgs($this->dm)); $this->session->refresh(false); @@ -3818,7 +3825,7 @@ public function getScheduledUpdates() */ public function getScheduledInserts() { - return $this->scheduledInserts; + return $this->treeOpQueue->getSchedule(TreeOperation::OP_INSERT); } /** @@ -3828,7 +3835,7 @@ public function getScheduledInserts() */ public function getScheduledMoves() { - return $this->scheduledMoves; + return $this->treeOpQueue->getSchedule(TreeOperation::OP_MOVE); } /** @@ -3848,7 +3855,7 @@ public function getScheduledReorders() */ public function getScheduledRemovals() { - return $this->scheduledRemovals; + return $this->treeOpQueue->getSchedule(TreeOperation::OP_REMOVE); } /** diff --git a/tests/Doctrine/Tests/ODM/PHPCR/Functional/BasicCrudTest.php b/tests/Doctrine/Tests/ODM/PHPCR/Functional/BasicCrudTest.php index 047d7f527..97781b8ae 100644 --- a/tests/Doctrine/Tests/ODM/PHPCR/Functional/BasicCrudTest.php +++ b/tests/Doctrine/Tests/ODM/PHPCR/Functional/BasicCrudTest.php @@ -325,6 +325,32 @@ public function testRemoveAndInsertAfterFlush() $this->assertEquals($user->username, $userNew->username); } + public function testInsertMoveDelete() + { + $this->dm->clear(); + + $user = new User2(); + $user->username = "test"; + $user->id = '/functional/user1'; + $this->dm->persist($user); + + $user2 = new User2(); + $user2->username = "test"; + $user2->id = '/functional/user2'; + $this->dm->persist($user2); + + $subuser = new User2(); + $subuser->username = "test"; + $subuser->id = '/functional/user2/subuser'; + $this->dm->persist($subuser); + $this->dm->move($subuser, '/functional/user1/subuser'); + $this->dm->remove($user2); + $this->dm->flush(); + + $subuser = $this->dm->find(null, '/functional/user1/subuser'); + $this->assertNotNull($subuser); + } + public function testRemoveAndReinsert() { $this->dm->clear(); diff --git a/tests/Doctrine/Tests/ODM/PHPCR/Queue/TreeOperationBatchTest.php b/tests/Doctrine/Tests/ODM/PHPCR/Queue/TreeOperationBatchTest.php new file mode 100644 index 000000000..1fad5ce1b --- /dev/null +++ b/tests/Doctrine/Tests/ODM/PHPCR/Queue/TreeOperationBatchTest.php @@ -0,0 +1,33 @@ +batch = new TreeOperationBatch(TreeOperation::OP_MOVE); + } + + public function testGetters() + { + $this->assertEquals(TreeOperation::OP_MOVE, $this->batch->getType()); + } + + public function testSchedule() + { + $this->batch->schedule('1234', 'arg'); + $this->batch->schedule('4321', 'arg'); + + $this->assertEquals(array( + '1234' => 'arg', + '4321' => 'arg', + ), $this->batch->getSchedule()); + } +} + diff --git a/tests/Doctrine/Tests/ODM/PHPCR/Queue/TreeOperationQueueTest.php b/tests/Doctrine/Tests/ODM/PHPCR/Queue/TreeOperationQueueTest.php new file mode 100644 index 000000000..90f1714c6 --- /dev/null +++ b/tests/Doctrine/Tests/ODM/PHPCR/Queue/TreeOperationQueueTest.php @@ -0,0 +1,138 @@ +queue = new TreeOperationQueue(); + } + + public function provideGetBatches() + { + return array( + array( + array( + TreeOperation::OP_MOVE, + TreeOperation::OP_INSERT, + TreeOperation::OP_REMOVE, + ), + 3 + ), + array( + array( + TreeOperation::OP_REMOVE, + ), + 1 + ), + array( + array( + TreeOperation::OP_REMOVE, + TreeOperation::OP_REMOVE, + TreeOperation::OP_INSERT, + TreeOperation::OP_MOVE, + TreeOperation::OP_REMOVE, + ), + 4 + ) + ); + } + + /** + * @dataProvider provideGetBatches + */ + public function testGetBatches($operations, $expectedNbBatches) + { + foreach ($operations as $operation) { + $this->queue->push(new TreeOperation($operation, '1234', 'arg')); + } + + $batches = $this->queue->getBatches(); + + $this->assertCount($expectedNbBatches, $batches); + } + + public function provideSchedule() + { + return array( + array( + array( + array( + TreeOperation::OP_MOVE, + 1, + 'arg1', + true + ), + array( + TreeOperation::OP_MOVE, + 2, + 'arg2', + false + ), + array( + TreeOperation::OP_REMOVE, + 6, + 'arg3', + true + ), + array( + TreeOperation::OP_MOVE, + 3, + 'arg3', + true + ), + ), + TreeOperation::OP_MOVE, + array( + '1' => 'arg1', + '3' => 'arg3', + ), + ) + ); + } + + /** + * @dataProvider provideSchedule + */ + public function testSchedule($operations, $targetType, $expectedSchedule) + { + foreach ($operations as $operation) { + $treeOperation = new TreeOperation($operation[0], $operation[1], $operation[2]); + if (!$operation[3]) { + $treeOperation->invalidate(); + } + + $this->queue->push($treeOperation); + } + + $this->assertEquals($expectedSchedule, $this->queue->getSchedule($targetType)); + } + + public function testIsQueued() + { + $this->queue->push(new TreeOperation(TreeOperation::OP_MOVE, '1234', 'arg')); + $this->assertTrue($this->queue->isQueued(TreeOperation::OP_MOVE, '1234')); + $this->assertFalse($this->queue->isQueued(TreeOperation::OP_MOVE, '4321')); + $this->assertFalse($this->queue->isQueued(TreeOperation::OP_INSERT, '1234')); + } + + public function testUnqueue() + { + $this->queue->push(new TreeOperation(TreeOperation::OP_MOVE, '1234', 'arg')); + $this->queue->unqueue(TreeOperation::OP_MOVE, '1234'); + $this->assertFalse($this->queue->isQueued(TreeOperation::OP_MOVE, '1234')); + } + + public function testUnregister() + { + $this->queue->push(new TreeOperation(TreeOperation::OP_MOVE, '1234', 'arg')); + $this->queue->unregister('1234'); + $this->assertFalse($this->queue->isQueued(TreeOperation::OP_MOVE, '1234')); + } +} diff --git a/tests/Doctrine/Tests/ODM/PHPCR/Queue/TreeOperationTest.php b/tests/Doctrine/Tests/ODM/PHPCR/Queue/TreeOperationTest.php new file mode 100644 index 000000000..133ee9956 --- /dev/null +++ b/tests/Doctrine/Tests/ODM/PHPCR/Queue/TreeOperationTest.php @@ -0,0 +1,29 @@ +treeOperation = new TreeOperation(TreeOperation::OP_MOVE, '1234', 'arg'); + } + + public function testGetters() + { + $this->assertEquals('1234', $this->treeOperation->getOid()); + $this->assertEquals('arg', $this->treeOperation->getArgs()); + $this->assertEquals(TreeOperation::OP_MOVE, $this->treeOperation->getType()); + } + + public function testInvalidate() + { + $this->assertTrue($this->treeOperation->isValid()); + $this->treeOperation->invalidate(); + $this->assertFalse($this->treeOperation->isValid()); + } +}