From 5af6fae93c8ccc5205c1c210c35955c596bd5b98 Mon Sep 17 00:00:00 2001 From: watari Date: Fri, 19 Oct 2018 16:26:32 +0300 Subject: [PATCH 1/8] Implemented CollectionPersister::deleteAll method for simultaneous deletion of collections that is belong to one document. --- .../Persisters/CollectionPersister.php | 31 ++++++++++++++ .../MongoDB/Persisters/DocumentPersister.php | 6 ++- .../Functional/CollectionPersisterTest.php | 41 +++++++++++++++++++ 3 files changed, 77 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php index 687dc6ee53..539408f4c3 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php @@ -68,6 +68,37 @@ public function __construct(DocumentManager $dm, PersistenceBuilder $pb, UnitOfW $this->uow = $uow; } + /** + * Deletes a PersistentCollection instances completely from a document using $unset. If collections belong to the different + * + * @param PersistentCollectionInterface[] $collections + * @param array $options + */ + public function deleteAll(array $collections, array $options) + { + $parents = []; + $unsetPathsMap = []; + + foreach ($collections as $coll) { + $mapping = $coll->getMapping(); + if ($mapping['isInverseSide']) { + continue; // ignore inverse side + } + if (CollectionHelper::isAtomic($mapping['strategy'])) { + throw new \UnexpectedValueException($mapping['strategy'] . ' delete collection strategy should have been handled by DocumentPersister. Please report a bug in issue tracker'); + } + list($propertyPath, $parent) = $this->getPathAndParent($coll); + $oid = \spl_object_hash($parent); + $parents[$oid] = $parent; + $unsetPathsMap[$oid][$propertyPath] = true; + } + + foreach ($unsetPathsMap as $oid => $unsetPaths) { + $query = array('$unset' => $unsetPaths); + $this->executeQuery($parents[$oid], $query, $options); + } + } + /** * Deletes a PersistentCollection instance completely from a document using $unset. * diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php index fd75a66fa0..3a32503a92 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php @@ -1354,12 +1354,16 @@ private function getClassDiscriminatorValues(ClassMetadata $metadata) private function handleCollections($document, $options) { + $collections = []; // Collection deletions (deletions of complete collections) foreach ($this->uow->getScheduledCollections($document) as $coll) { if ($this->uow->isCollectionScheduledForDeletion($coll)) { - $this->cp->delete($coll, $options); + $collections[] = $coll; } } + if (!empty($collections)) { + $this->cp->deleteAll($collections, $options); + } // Collection updates (deleteRows, updateRows, insertRows) foreach ($this->uow->getScheduledCollections($document) as $coll) { if ($this->uow->isCollectionScheduledForUpdate($coll)) { diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php index 2f37a39464..d743f71653 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php @@ -54,6 +54,47 @@ public function testDeleteNestedEmbedMany() $this->assertTrue(isset($check['categories'][1]), 'Test that the category with the children still exists'); } + public function testDeleteAllEmbedMany() + { + $persister = $this->getCollectionPersister(); + $user = $this->getTestUser('jwage'); + $persister->deleteAll([$user->categories], array()); + $user = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(array('username' => 'jwage')); + $this->assertArrayNotHasKey('categories', $user, 'Test that the categories field was deleted'); + } + + public function testDeleteAllReferenceMany() + { + $persister = $this->getCollectionPersister(); + $user = $this->getTestUser('jwage'); + $persister->deleteAll([$user->phonenumbers], array()); + $user = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(array('username' => 'jwage')); + $this->assertArrayNotHasKey('phonenumbers', $user, 'Test that the phonenumbers field was deleted'); + } + + public function testDeleteAllNestedEmbedMany() + { + $persister = $this->getCollectionPersister(); + $user = $this->getTestUser('jwage'); + $persister->deleteAll( + [$user->categories[0]->children[0]->children, $user->categories[0]->children[1]->children], + array() + ); + $check = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(array('username' => 'jwage')); + $this->assertFalse(isset($check['categories']['0']['children'][0]['children'])); + $this->assertFalse(isset($check['categories']['0']['children'][1]['children'])); + $persister->deleteAll( + [$user->categories[0]->children, $user->categories[1]->children], + array() + ); + $check = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(array('username' => 'jwage')); + $this->assertFalse(isset($check['categories'][0]['children']), 'Test that the nested children categories field was deleted'); + $this->assertTrue(isset($check['categories'][0]), 'Test that the category with the children still exists'); + $this->assertFalse(isset($check['categories'][1]['children']), 'Test that the nested children categories field was deleted'); + $this->assertTrue(isset($check['categories'][1]), 'Test that the category with the children still exists'); + } + + public function testDeleteRows() { $persister = $this->getCollectionPersister(); From 1c9af334c36010f05f11f64fd9be4758950af216 Mon Sep 17 00:00:00 2001 From: watari Date: Mon, 22 Oct 2018 10:36:20 +0300 Subject: [PATCH 2/8] Added check for sub-paths before collections removal in CollectionPersister::deleteAll. Fixed style of code. --- .../Persisters/CollectionPersister.php | 53 ++++++++++++++++--- .../MongoDB/Persisters/DocumentPersister.php | 8 +-- .../Functional/CollectionPersisterTest.php | 41 ++++++++++---- 3 files changed, 80 insertions(+), 22 deletions(-) diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php index 539408f4c3..c4eaa631c9 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php @@ -25,6 +25,18 @@ use Doctrine\ODM\MongoDB\PersistentCollection\PersistentCollectionInterface; use Doctrine\ODM\MongoDB\UnitOfWork; use Doctrine\ODM\MongoDB\Utility\CollectionHelper; +use UnexpectedValueException; +use function array_fill_keys; +use function array_keys; +use function array_map; +use function array_reverse; +use function array_unique; +use function array_values; +use function count; +use function get_class; +use function implode; +use function spl_object_hash; +use function strpos; /** * The CollectionPersister is responsible for persisting collections of embedded @@ -72,11 +84,11 @@ public function __construct(DocumentManager $dm, PersistenceBuilder $pb, UnitOfW * Deletes a PersistentCollection instances completely from a document using $unset. If collections belong to the different * * @param PersistentCollectionInterface[] $collections - * @param array $options + * @param array $options */ public function deleteAll(array $collections, array $options) { - $parents = []; + $parents = []; $unsetPathsMap = []; foreach ($collections as $coll) { @@ -85,16 +97,17 @@ public function deleteAll(array $collections, array $options) continue; // ignore inverse side } if (CollectionHelper::isAtomic($mapping['strategy'])) { - throw new \UnexpectedValueException($mapping['strategy'] . ' delete collection strategy should have been handled by DocumentPersister. Please report a bug in issue tracker'); + throw new UnexpectedValueException($mapping['strategy'] . ' delete collection strategy should have been handled by DocumentPersister. Please report a bug in issue tracker'); } - list($propertyPath, $parent) = $this->getPathAndParent($coll); - $oid = \spl_object_hash($parent); - $parents[$oid] = $parent; + list($propertyPath, $parent) = $this->getPathAndParent($coll); + $oid = spl_object_hash($parent); + $parents[$oid] = $parent; $unsetPathsMap[$oid][$propertyPath] = true; } - foreach ($unsetPathsMap as $oid => $unsetPaths) { - $query = array('$unset' => $unsetPaths); + foreach ($unsetPathsMap as $oid => $paths) { + $unsetPaths = array_fill_keys($this->excludeSubPaths(array_keys($paths)), true); + $query = ['$unset' => $unsetPaths]; $this->executeQuery($parents[$oid], $query, $options); } } @@ -315,4 +328,28 @@ private function executeQuery($document, array $newObj, array $options) throw LockException::lockFailed($document); } } + + private function excludeSubPaths(array $paths) + { + $checkedPaths = []; + $pathsAmount = count($paths); + $paths = array_unique($paths); + for ($i = 0; $i < $pathsAmount; $i++) { + $isSubPath = false; + $j = 0; + for (; $j < $pathsAmount; $j++) { + if ($i !== $j && strpos($paths[$i], $paths[$j]) === 0) { + $isSubPath = true; + break; + } + } + if ($isSubPath) { + continue; + } + + $checkedPaths[] = $paths[$i]; + } + + return $checkedPaths; + } } diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php index 3a32503a92..a45a7ce960 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php @@ -1357,11 +1357,13 @@ private function handleCollections($document, $options) $collections = []; // Collection deletions (deletions of complete collections) foreach ($this->uow->getScheduledCollections($document) as $coll) { - if ($this->uow->isCollectionScheduledForDeletion($coll)) { - $collections[] = $coll; + if (! $this->uow->isCollectionScheduledForDeletion($coll)) { + continue; } + + $collections[] = $coll; } - if (!empty($collections)) { + if (! empty($collections)) { $this->cp->deleteAll($collections, $options); } // Collection updates (deleteRows, updateRows, insertRows) diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php index d743f71653..d74525de72 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php @@ -57,43 +57,62 @@ public function testDeleteNestedEmbedMany() public function testDeleteAllEmbedMany() { $persister = $this->getCollectionPersister(); - $user = $this->getTestUser('jwage'); - $persister->deleteAll([$user->categories], array()); - $user = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(array('username' => 'jwage')); + $user = $this->getTestUser('jwage'); + $persister->deleteAll([$user->categories], []); + $user = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(['username' => 'jwage']); $this->assertArrayNotHasKey('categories', $user, 'Test that the categories field was deleted'); } public function testDeleteAllReferenceMany() { $persister = $this->getCollectionPersister(); - $user = $this->getTestUser('jwage'); - $persister->deleteAll([$user->phonenumbers], array()); - $user = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(array('username' => 'jwage')); + $user = $this->getTestUser('jwage'); + $persister->deleteAll([$user->phonenumbers], []); + $user = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(['username' => 'jwage']); $this->assertArrayNotHasKey('phonenumbers', $user, 'Test that the phonenumbers field was deleted'); } public function testDeleteAllNestedEmbedMany() { $persister = $this->getCollectionPersister(); - $user = $this->getTestUser('jwage'); + $user = $this->getTestUser('jwage'); $persister->deleteAll( [$user->categories[0]->children[0]->children, $user->categories[0]->children[1]->children], - array() + [] ); - $check = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(array('username' => 'jwage')); + $check = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(['username' => 'jwage']); $this->assertFalse(isset($check['categories']['0']['children'][0]['children'])); $this->assertFalse(isset($check['categories']['0']['children'][1]['children'])); $persister->deleteAll( [$user->categories[0]->children, $user->categories[1]->children], - array() + [] ); - $check = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(array('username' => 'jwage')); + $check = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(['username' => 'jwage']); $this->assertFalse(isset($check['categories'][0]['children']), 'Test that the nested children categories field was deleted'); $this->assertTrue(isset($check['categories'][0]), 'Test that the category with the children still exists'); $this->assertFalse(isset($check['categories'][1]['children']), 'Test that the nested children categories field was deleted'); $this->assertTrue(isset($check['categories'][1]), 'Test that the category with the children still exists'); } + public function testDeleteAllNestedEmbedManyAndNestedParent() + { + $persister = $this->getCollectionPersister(); + $user = $this->getTestUser('jwage'); + $persister->deleteAll( + [$user->categories[0]->children[0]->children, $user->categories[0]->children[1]->children], + [] + ); + $check = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(['username' => 'jwage']); + $this->assertFalse(isset($check['categories']['0']['children'][0]['children'])); + $this->assertFalse(isset($check['categories']['0']['children'][1]['children'])); + $persister->deleteAll( + [$user->categories[0]->children, $user->categories[0]->children[1]->children, $user->categories], + [] + ); + $check = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(['username' => 'jwage']); + $this->assertFalse(isset($check['categories']), 'Test that the nested categories field was deleted'); + } + public function testDeleteRows() { From 086d11113147d31c94ce6f0794994d7a92b2c465 Mon Sep 17 00:00:00 2001 From: watari Date: Wed, 24 Oct 2018 20:09:18 +0300 Subject: [PATCH 3/8] Updated CollectionPersister logic for handling nested collections scheduled for update - now nested collections for one parent is updated inside one query where it was possible. --- .../Persisters/CollectionPersister.php | 341 ++++++++++++++++++ .../MongoDB/Persisters/DocumentPersister.php | 12 +- 2 files changed, 350 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php index c4eaa631c9..4a54bc5794 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php @@ -19,6 +19,7 @@ namespace Doctrine\ODM\MongoDB\Persisters; +use Closure; use Doctrine\ODM\MongoDB\DocumentManager; use Doctrine\ODM\MongoDB\LockException; use Doctrine\ODM\MongoDB\Mapping\ClassMetadata; @@ -26,7 +27,10 @@ use Doctrine\ODM\MongoDB\UnitOfWork; use Doctrine\ODM\MongoDB\Utility\CollectionHelper; use UnexpectedValueException; +use function array_diff_key; use function array_fill_keys; +use function array_flip; +use function array_intersect_key; use function array_keys; use function array_map; use function array_reverse; @@ -52,6 +56,14 @@ */ class CollectionPersister { + /** + * Validation map that is used for strategy validation in insertCollections method. + */ + const INSERT_STRATEGIES_MAP = [ + ClassMetadata::STORAGE_STRATEGY_PUSH_ALL => true, + ClassMetadata::STORAGE_STRATEGY_ADD_TO_SET => true, + ]; + /** * The DocumentManager instance. * @@ -169,6 +181,54 @@ public function update(PersistentCollectionInterface $coll, array $options) } } + /** + * Updates a list PersistentCollection instances deleting removed rows and inserting new rows. + * + * @param PersistentCollectionInterface[] $collections + * @param array $options + */ + public function updateAll(array $collections, array $options) + { + $setStrategyColls = []; + $addPushStrategyColls = []; + + foreach ($collections as $coll) { + $mapping = $coll->getMapping(); + + if ($mapping['isInverseSide']) { + continue; // ignore inverse side + } + switch ($mapping['strategy']) { + case ClassMetadata::STORAGE_STRATEGY_ATOMIC_SET: + case ClassMetadata::STORAGE_STRATEGY_ATOMIC_SET_ARRAY: + throw new UnexpectedValueException($mapping['strategy'] . ' update collection strategy should have been handled by DocumentPersister. Please report a bug in issue tracker'); + + case ClassMetadata::STORAGE_STRATEGY_SET: + case ClassMetadata::STORAGE_STRATEGY_SET_ARRAY: + $setStrategyColls[] = $coll; + break; + + case ClassMetadata::STORAGE_STRATEGY_ADD_TO_SET: + case ClassMetadata::STORAGE_STRATEGY_PUSH_ALL: + $addPushStrategyColls[] = $coll; + break; + + default: + throw new UnexpectedValueException('Unsupported collection strategy: ' . $mapping['strategy']); + } + } + + if (! empty($setStrategyColls)) { + $this->setCollections($setStrategyColls, $options); + } + if (empty($addPushStrategyColls)) { + return; + } + + $this->deleteCollections($addPushStrategyColls, $options); + $this->insertCollections($addPushStrategyColls, $options); // TODO + } + /** * Sets a PersistentCollection instance. * @@ -190,6 +250,53 @@ private function setCollection(PersistentCollectionInterface $coll, array $optio $this->executeQuery($parent, $query, $options); } + /** + * Sets a list of PersistentCollection instances. + * + * This method is intended to be used with the "set" or "setArray" + * strategies. The "setArray" strategy will ensure that the collection is + * set as a BSON array, which means the collection elements will be + * reindexed numerically before storage. + * + * @param PersistentCollectionInterface[] $collections + * @param array $options + */ + private function setCollections(array $collections, array $options) + { + $parents = []; + $pathCollMap = []; + $pathsMap = []; + foreach ($collections as $coll) { + list($propertyPath, $parent) = $this->getPathAndParent($coll); + $oid = spl_object_hash($parent); + $parents[$oid] = $parent; + $pathCollMap[$oid][$propertyPath] = $coll; + $pathsMap[$oid][] = $propertyPath; + } + + foreach ($pathsMap as $oid => $paths) { + $paths = $this->excludeSubPaths($paths); + /** @var PersistentCollectionInterface[] $setColls */ + $setColls = array_intersect_key($pathCollMap[$oid], array_flip($paths)); + $setPayload = []; + foreach ($setColls as $propertyPath => $coll) { + $coll->initialize(); + $mapping = $coll->getMapping(); + $setData = $this->pb->prepareAssociatedCollectionValue( + $coll, + CollectionHelper::usesSet($mapping['strategy']) + ); + $setPayload[$propertyPath] = $setData; + } + if (empty($setPayload)) { + continue; + } + + $query = ['$set' => $setPayload]; + $this->executeQuery($parents[$oid], $query, $options); + } + } + /** * Deletes removed elements from a PersistentCollection instance. * @@ -226,6 +333,70 @@ private function deleteElements(PersistentCollectionInterface $coll, array $opti $this->executeQuery($parent, array('$pull' => array($propertyPath => null)), $options); } + /** + * Deletes removed elements from a list of PersistentCollection instances. + * + * This method is intended to be used with the "pushAll" and "addToSet" strategies. + * + * @param PersistentCollectionInterface[] $collections + * @param array $options + */ + private function deleteCollections(array $collections, array $options) + { + $parents = []; + $pathCollMap = []; + $pathsMap = []; + $deleteDiffMap = []; + + foreach ($collections as $coll) { + $coll->initialize(); + if (! $this->uow->isCollectionScheduledForUpdate($coll)) { + continue; + } + $deleteDiff = $coll->getDeleteDiff(); + + if (empty($deleteDiff)) { + continue; + } + list($propertyPath, $parent) = $this->getPathAndParent($coll); + + $oid = spl_object_hash($parent); + $parents[$oid] = $parent; + $pathCollMap[$oid][$propertyPath] = $coll; + $pathsMap[$oid][] = $propertyPath; + $deleteDiffMap[$oid][$propertyPath] = $deleteDiff; + } + + foreach ($pathsMap as $oid => $paths) { + $paths = $this->excludeSubPaths($paths); + $deleteColls = array_intersect_key($pathCollMap[$oid], array_flip($paths)); + $unsetPayload = []; + $pullPayload = []; + foreach ($deleteColls as $propertyPath => $coll) { + $deleteDiff = $deleteDiffMap[$oid][$propertyPath]; + foreach ($deleteDiff as $key => $document) { + $unsetPayload[$propertyPath . '.' . $key] = true; + } + $pullPayload[$propertyPath] = null; + } + + if (! empty($unsetPayload)) { + $this->executeQuery($parents[$oid], ['$unset' => $unsetPayload], $options); + } + if (empty($pullPayload)) { + continue; + } + + /** + * @todo This is a hack right now because we don't have a proper way to + * remove an element from an array by its key. Unsetting the key results + * in the element being left in the array as null so we have to pull + * null values. + */ + $this->executeQuery($parents[$oid], ['$pull' => $pullPayload], $options); + } + } + /** * Inserts new elements for a PersistentCollection instance. * @@ -271,6 +442,169 @@ private function insertElements(PersistentCollectionInterface $coll, array $opti $this->executeQuery($parent, $query, $options); } + /** + * Inserts new elements for a list of PersistentCollection instances. + * + * This method is intended to be used with the "pushAll" and "addToSet" strategies. + * + * @param PersistentCollectionInterface[] $collections + * @param array $options + */ + private function insertCollections(array $collections, array $options) + { + $parents = []; + $pushAllPathCollMap = []; + $addToSetPathCollMap = []; + $pushAllPathsMap = []; + $addToSetPathsMap = []; + $diffsMap = []; + + foreach ($collections as $coll) { + $coll->initialize(); + if (! $this->uow->isCollectionScheduledForUpdate($coll)) { + continue; + } + $insertDiff = $coll->getInsertDiff(); + + if (empty($insertDiff)) { + continue; + } + + $mapping = $coll->getMapping(); + $strategy = $mapping['strategy']; + + if (empty(self::INSERT_STRATEGIES_MAP[$strategy])) { + throw new LogicException('Invalid strategy ' . $strategy . ' given for insertCollections'); + } + + list($propertyPath, $parent) = $this->getPathAndParent($coll); + $oid = spl_object_hash($parent); + $parents[$oid] = $parent; + $diffsMap[$oid][$propertyPath] = $insertDiff; + + switch ($strategy) { + case ClassMetadata::STORAGE_STRATEGY_PUSH_ALL: + $pushAllPathCollMap[$oid][$propertyPath] = $coll; + $pushAllPathsMap[$oid][] = $propertyPath; + break; + + case ClassMetadata::STORAGE_STRATEGY_ADD_TO_SET: + $addToSetPathCollMap[$oid][$propertyPath] = $coll; + $addToSetPathsMap[$oid][] = $propertyPath; + break; + + default: + throw new LogicException('Invalid strategy ' . $strategy . ' given for insertCollections'); + } + } + + foreach ($parents as $oid => $parent) { + if (! empty($pushAllPathsMap[$oid])) { + $this->pushAllCollections( + $parent, + $pushAllPathsMap[$oid], + $pushAllPathCollMap[$oid], + $diffsMap[$oid], + $options + ); + } + if (empty($addToSetPathsMap[$oid])) { + continue; + } + + $this->addToSetCollections( + $parent, + $addToSetPathsMap[$oid], + $addToSetPathCollMap[$oid], + $diffsMap[$oid], + $options + ); + } + } + + /** + * Perform collections update for 'pushAll' strategy. + * + * @param object $parent Parent object to which passed collections is belong. + * @param array $collsPaths Paths of collections that is passed. + * @param array $pathCollsMap List of collections indexed by their paths. + * @param array $diffsMap List of collection diffs indexed by collections paths. + * @param array $options + */ + private function pushAllCollections($parent, array $collsPaths, array $pathCollsMap, array $diffsMap, array $options) + { + $pushAllPaths = $this->excludeSubPaths($collsPaths); + /** @var PersistentCollectionInterface[] $pushAllColls */ + $pushAllColls = array_intersect_key($pathCollsMap, array_flip($pushAllPaths)); + $pushAllPayload = []; + foreach ($pushAllColls as $propertyPath => $coll) { + $callback = $this->getValuePrepareCallback($coll); + $value = array_values(array_map($callback, $diffsMap[$propertyPath])); + $pushAllPayload[$propertyPath] = ['$each' => $value]; + } + + if (! empty($pushAllPayload)) { + $this->executeQuery($parent, ['$push' => $pushAllPayload], $options); + } + + $pushAllColls = array_diff_key($pathCollsMap, array_flip($pushAllPaths)); + foreach ($pushAllColls as $propertyPath => $coll) { + $callback = $this->getValuePrepareCallback($coll); + $value = array_values(array_map($callback, $diffsMap[$propertyPath])); + $query = ['$push' => [$propertyPath => ['$each' => $value]]]; + $this->executeQuery($parent, $query, $options); + } + } + + /** + * Perform collections update by 'addToSet' strategy. + * + * @param object $parent Parent object to which passed collections is belong. + * @param array $collsPaths Paths of collections that is passed. + * @param array $pathCollsMap List of collections indexed by their paths. + * @param array $diffsMap List of collection diffs indexed by collections paths. + * @param array $options + */ + private function addToSetCollections($parent, array $collsPaths, array $pathCollsMap, array $diffsMap, array $options) + { + $addToSetPaths = $this->excludeSubPaths($collsPaths); + /** @var PersistentCollectionInterface[] $addToSetColls */ + $addToSetColls = array_intersect_key($pathCollsMap, array_flip($addToSetPaths)); + + $addToSetPayload = []; + foreach ($addToSetColls as $propertyPath => $coll) { + $callback = $this->getValuePrepareCallback($coll); + $value = array_values(array_map($callback, $diffsMap[$propertyPath])); + $addToSetPayload[$propertyPath] = ['$each' => $value]; + } + + if (empty($addToSetPayload)) { + return; + } + + $this->executeQuery($parent, ['$addToSet' => $addToSetPayload], $options); + } + + /** + * Return callback instance for specified collection. This callback will prepare values for query from documents + * that collection contain. + * + * @return Closure + */ + private function getValuePrepareCallback(PersistentCollectionInterface $coll) + { + $mapping = $coll->getMapping(); + if (isset($mapping['embedded'])) { + return function ($v) use ($mapping) { + return $this->pb->prepareEmbeddedDocumentValue($mapping, $v); + }; + } + + return function ($v) use ($mapping) { + return $this->pb->prepareReferencedDocumentValue($mapping, $v); + }; + } + /** * Gets the parent information for a given PersistentCollection. It will * retrieve the top-level persistent Document that the PersistentCollection @@ -329,6 +663,13 @@ private function executeQuery($document, array $newObj, array $options) } } + /** + * Remove from passed paths list all sub-paths. + * + * @param string[] $paths + * + * @return string[] + */ private function excludeSubPaths(array $paths) { $checkedPaths = []; diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php index a45a7ce960..93e05d39dd 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php @@ -1354,8 +1354,8 @@ private function getClassDiscriminatorValues(ClassMetadata $metadata) private function handleCollections($document, $options) { - $collections = []; // Collection deletions (deletions of complete collections) + $collections = []; foreach ($this->uow->getScheduledCollections($document) as $coll) { if (! $this->uow->isCollectionScheduledForDeletion($coll)) { continue; @@ -1367,10 +1367,16 @@ private function handleCollections($document, $options) $this->cp->deleteAll($collections, $options); } // Collection updates (deleteRows, updateRows, insertRows) + $collections = []; foreach ($this->uow->getScheduledCollections($document) as $coll) { - if ($this->uow->isCollectionScheduledForUpdate($coll)) { - $this->cp->update($coll, $options); + if (! $this->uow->isCollectionScheduledForUpdate($coll)) { + continue; } + + $collections[] = $coll; + } + if (! empty($collections)) { + $this->cp->updateAll($collections, $options); } // Take new snapshots from visited collections foreach ($this->uow->getVisitedCollections($document) as $coll) { From 7dea62d9a5c64b06b53842610d98a32bcfb037c2 Mon Sep 17 00:00:00 2001 From: watari Date: Fri, 26 Oct 2018 12:40:03 +0300 Subject: [PATCH 4/8] Reduced complexity of CollectionPersister::excludeSubPaths method. --- .../Persisters/CollectionPersister.php | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php index 4a54bc5794..e556ba6e61 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php @@ -34,11 +34,12 @@ use function array_keys; use function array_map; use function array_reverse; -use function array_unique; use function array_values; use function count; +use function end; use function get_class; use function implode; +use function sort; use function spl_object_hash; use function strpos; @@ -672,25 +673,19 @@ private function executeQuery($document, array $newObj, array $options) */ private function excludeSubPaths(array $paths) { - $checkedPaths = []; - $pathsAmount = count($paths); - $paths = array_unique($paths); - for ($i = 0; $i < $pathsAmount; $i++) { - $isSubPath = false; - $j = 0; - for (; $j < $pathsAmount; $j++) { - if ($i !== $j && strpos($paths[$i], $paths[$j]) === 0) { - $isSubPath = true; - break; - } - } - if ($isSubPath) { + if (empty($paths)) { + return $paths; + } + sort($paths); + $uniquePaths = [$paths[0]]; + for ($i = 1; $i < count($paths); ++$i) { + if (strpos($paths[$i], end($uniquePaths)) === 0) { continue; } - $checkedPaths[] = $paths[$i]; + $uniquePaths[] = $paths[$i]; } - return $checkedPaths; + return $uniquePaths; } } From 99c70c2bad659e50eebf0be3c5de219f2d26c13f Mon Sep 17 00:00:00 2001 From: watari Date: Mon, 12 Nov 2018 20:50:43 +0200 Subject: [PATCH 5/8] Merge remarks implemented. --- .../ODM/MongoDB/Persisters/CollectionPersister.php | 8 ++++---- .../Tests/Functional/CollectionPersisterTest.php | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php index e556ba6e61..5da1ce26c1 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php @@ -104,15 +104,15 @@ public function deleteAll(array $collections, array $options) $parents = []; $unsetPathsMap = []; - foreach ($collections as $coll) { - $mapping = $coll->getMapping(); + foreach ($collections as $collection) { + $mapping = $collection->getMapping(); if ($mapping['isInverseSide']) { continue; // ignore inverse side } if (CollectionHelper::isAtomic($mapping['strategy'])) { throw new UnexpectedValueException($mapping['strategy'] . ' delete collection strategy should have been handled by DocumentPersister. Please report a bug in issue tracker'); } - list($propertyPath, $parent) = $this->getPathAndParent($coll); + list($propertyPath, $parent) = $this->getPathAndParent($collection); $oid = spl_object_hash($parent); $parents[$oid] = $parent; $unsetPathsMap[$oid][$propertyPath] = true; @@ -678,7 +678,7 @@ private function excludeSubPaths(array $paths) } sort($paths); $uniquePaths = [$paths[0]]; - for ($i = 1; $i < count($paths); ++$i) { + for ($i = 1, $count = count($paths); $i < $count; ++$i) { if (strpos($paths[$i], end($uniquePaths)) === 0) { continue; } diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php index d74525de72..26cc00d8f4 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php @@ -59,7 +59,7 @@ public function testDeleteAllEmbedMany() $persister = $this->getCollectionPersister(); $user = $this->getTestUser('jwage'); $persister->deleteAll([$user->categories], []); - $user = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(['username' => 'jwage']); + $user = $this->dm->getDocumentCollection(CollectionPersisterUser::class)->findOne(['username' => 'jwage']); $this->assertArrayNotHasKey('categories', $user, 'Test that the categories field was deleted'); } @@ -68,7 +68,7 @@ public function testDeleteAllReferenceMany() $persister = $this->getCollectionPersister(); $user = $this->getTestUser('jwage'); $persister->deleteAll([$user->phonenumbers], []); - $user = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(['username' => 'jwage']); + $user = $this->dm->getDocumentCollection(CollectionPersisterUser::class)->findOne(['username' => 'jwage']); $this->assertArrayNotHasKey('phonenumbers', $user, 'Test that the phonenumbers field was deleted'); } @@ -80,14 +80,14 @@ public function testDeleteAllNestedEmbedMany() [$user->categories[0]->children[0]->children, $user->categories[0]->children[1]->children], [] ); - $check = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(['username' => 'jwage']); + $check = $this->dm->getDocumentCollection(CollectionPersisterUser::class)->findOne(['username' => 'jwage']); $this->assertFalse(isset($check['categories']['0']['children'][0]['children'])); $this->assertFalse(isset($check['categories']['0']['children'][1]['children'])); $persister->deleteAll( [$user->categories[0]->children, $user->categories[1]->children], [] ); - $check = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(['username' => 'jwage']); + $check = $this->dm->getDocumentCollection(CollectionPersisterUser::class)->findOne(['username' => 'jwage']); $this->assertFalse(isset($check['categories'][0]['children']), 'Test that the nested children categories field was deleted'); $this->assertTrue(isset($check['categories'][0]), 'Test that the category with the children still exists'); $this->assertFalse(isset($check['categories'][1]['children']), 'Test that the nested children categories field was deleted'); @@ -102,14 +102,14 @@ public function testDeleteAllNestedEmbedManyAndNestedParent() [$user->categories[0]->children[0]->children, $user->categories[0]->children[1]->children], [] ); - $check = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(['username' => 'jwage']); + $check = $this->dm->getDocumentCollection(CollectionPersisterUser::class)->findOne(['username' => 'jwage']); $this->assertFalse(isset($check['categories']['0']['children'][0]['children'])); $this->assertFalse(isset($check['categories']['0']['children'][1]['children'])); $persister->deleteAll( [$user->categories[0]->children, $user->categories[0]->children[1]->children, $user->categories], [] ); - $check = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(['username' => 'jwage']); + $check = $this->dm->getDocumentCollection(CollectionPersisterUser::class)->findOne(['username' => 'jwage']); $this->assertFalse(isset($check['categories']), 'Test that the nested categories field was deleted'); } From 01898f85b2204967a0fabc1a567285e3edd8350a Mon Sep 17 00:00:00 2001 From: watari Date: Tue, 27 Nov 2018 18:00:56 +0200 Subject: [PATCH 6/8] Simplified methods for working with nested collections, remove redundant checks --- .../Persisters/CollectionPersister.php | 227 ++++++++---------- .../MongoDB/Persisters/DocumentPersister.php | 4 +- .../Functional/CollectionPersisterTest.php | 8 +- 3 files changed, 108 insertions(+), 131 deletions(-) diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php index 5da1ce26c1..29d82b5f1d 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php @@ -57,19 +57,7 @@ */ class CollectionPersister { - /** - * Validation map that is used for strategy validation in insertCollections method. - */ - const INSERT_STRATEGIES_MAP = [ - ClassMetadata::STORAGE_STRATEGY_PUSH_ALL => true, - ClassMetadata::STORAGE_STRATEGY_ADD_TO_SET => true, - ]; - - /** - * The DocumentManager instance. - * - * @var DocumentManager - */ + /** @var DocumentManager */ private $dm; /** @@ -94,14 +82,14 @@ public function __construct(DocumentManager $dm, PersistenceBuilder $pb, UnitOfW } /** - * Deletes a PersistentCollection instances completely from a document using $unset. If collections belong to the different + * Deletes a PersistentCollection instances completely from a document using $unset. * + * @param object $parent * @param PersistentCollectionInterface[] $collections * @param array $options */ - public function deleteAll(array $collections, array $options) + public function deleteAll($parent, array $collections, array $options) { - $parents = []; $unsetPathsMap = []; foreach ($collections as $collection) { @@ -112,17 +100,17 @@ public function deleteAll(array $collections, array $options) if (CollectionHelper::isAtomic($mapping['strategy'])) { throw new UnexpectedValueException($mapping['strategy'] . ' delete collection strategy should have been handled by DocumentPersister. Please report a bug in issue tracker'); } - list($propertyPath, $parent) = $this->getPathAndParent($collection); - $oid = spl_object_hash($parent); - $parents[$oid] = $parent; - $unsetPathsMap[$oid][$propertyPath] = true; + list($propertyPath) = $this->getPathAndParent($collection); + $unsetPathsMap[$propertyPath] = true; } - foreach ($unsetPathsMap as $oid => $paths) { - $unsetPaths = array_fill_keys($this->excludeSubPaths(array_keys($paths)), true); - $query = ['$unset' => $unsetPaths]; - $this->executeQuery($parents[$oid], $query, $options); + if (empty($unsetPathsMap)) { + return; } + + $unsetPaths = array_fill_keys($this->excludeSubPaths(array_keys($unsetPathsMap)), true); + $query = ['$unset' => $unsetPaths]; + $this->executeQuery($parent, $query, $options); } /** @@ -185,10 +173,11 @@ public function update(PersistentCollectionInterface $coll, array $options) /** * Updates a list PersistentCollection instances deleting removed rows and inserting new rows. * + * @param object $parent * @param PersistentCollectionInterface[] $collections * @param array $options */ - public function updateAll(array $collections, array $options) + public function updateAll($parent, array $collections, array $options) { $setStrategyColls = []; $addPushStrategyColls = []; @@ -220,14 +209,14 @@ public function updateAll(array $collections, array $options) } if (! empty($setStrategyColls)) { - $this->setCollections($setStrategyColls, $options); + $this->setCollections($parent, $setStrategyColls, $options); } if (empty($addPushStrategyColls)) { return; } - $this->deleteCollections($addPushStrategyColls, $options); - $this->insertCollections($addPushStrategyColls, $options); // TODO + $this->deleteCollections($parent, $addPushStrategyColls, $options); + $this->insertCollections($parent, $addPushStrategyColls, $options); } /** @@ -259,43 +248,39 @@ private function setCollection(PersistentCollectionInterface $coll, array $optio * set as a BSON array, which means the collection elements will be * reindexed numerically before storage. * + * @param object $parent * @param PersistentCollectionInterface[] $collections * @param array $options */ - private function setCollections(array $collections, array $options) + private function setCollections($parent, array $collections, array $options) { - $parents = []; $pathCollMap = []; - $pathsMap = []; + $paths = []; foreach ($collections as $coll) { - list($propertyPath, $parent) = $this->getPathAndParent($coll); - $oid = spl_object_hash($parent); - $parents[$oid] = $parent; - $pathCollMap[$oid][$propertyPath] = $coll; - $pathsMap[$oid][] = $propertyPath; - } - - foreach ($pathsMap as $oid => $paths) { - $paths = $this->excludeSubPaths($paths); - /** @var PersistentCollectionInterface[] $setColls */ - $setColls = array_intersect_key($pathCollMap[$oid], array_flip($paths)); - $setPayload = []; - foreach ($setColls as $propertyPath => $coll) { - $coll->initialize(); - $mapping = $coll->getMapping(); - $setData = $this->pb->prepareAssociatedCollectionValue( - $coll, - CollectionHelper::usesSet($mapping['strategy']) - ); - $setPayload[$propertyPath] = $setData; - } - if (empty($setPayload)) { - continue; - } + list($propertyPath) = $this->getPathAndParent($coll); + $pathCollMap[$propertyPath] = $coll; + $paths[] = $propertyPath; + } - $query = ['$set' => $setPayload]; - $this->executeQuery($parents[$oid], $query, $options); + $paths = $this->excludeSubPaths($paths); + /** @var PersistentCollectionInterface[] $setColls */ + $setColls = array_intersect_key($pathCollMap, array_flip($paths)); + $setPayload = []; + foreach ($setColls as $propertyPath => $coll) { + $coll->initialize(); + $mapping = $coll->getMapping(); + $setData = $this->pb->prepareAssociatedCollectionValue( + $coll, + CollectionHelper::usesSet($mapping['strategy']) + ); + $setPayload[$propertyPath] = $setData; } + if (empty($setPayload)) { + return; + } + + $query = ['$set' => $setPayload]; + $this->executeQuery($parent, $query, $options); } /** @@ -339,14 +324,14 @@ private function deleteElements(PersistentCollectionInterface $coll, array $opti * * This method is intended to be used with the "pushAll" and "addToSet" strategies. * + * @param object $parent * @param PersistentCollectionInterface[] $collections * @param array $options */ - private function deleteCollections(array $collections, array $options) + private function deleteCollections($parent, array $collections, array $options) { - $parents = []; $pathCollMap = []; - $pathsMap = []; + $paths = []; $deleteDiffMap = []; foreach ($collections as $coll) { @@ -359,43 +344,39 @@ private function deleteCollections(array $collections, array $options) if (empty($deleteDiff)) { continue; } - list($propertyPath, $parent) = $this->getPathAndParent($coll); - - $oid = spl_object_hash($parent); - $parents[$oid] = $parent; - $pathCollMap[$oid][$propertyPath] = $coll; - $pathsMap[$oid][] = $propertyPath; - $deleteDiffMap[$oid][$propertyPath] = $deleteDiff; - } - - foreach ($pathsMap as $oid => $paths) { - $paths = $this->excludeSubPaths($paths); - $deleteColls = array_intersect_key($pathCollMap[$oid], array_flip($paths)); - $unsetPayload = []; - $pullPayload = []; - foreach ($deleteColls as $propertyPath => $coll) { - $deleteDiff = $deleteDiffMap[$oid][$propertyPath]; - foreach ($deleteDiff as $key => $document) { - $unsetPayload[$propertyPath . '.' . $key] = true; - } - $pullPayload[$propertyPath] = null; - } + list($propertyPath) = $this->getPathAndParent($coll); - if (! empty($unsetPayload)) { - $this->executeQuery($parents[$oid], ['$unset' => $unsetPayload], $options); - } - if (empty($pullPayload)) { - continue; + $pathCollMap[$propertyPath] = $coll; + $paths[] = $propertyPath; + $deleteDiffMap[$propertyPath] = $deleteDiff; + } + + $paths = $this->excludeSubPaths($paths); + $deleteColls = array_intersect_key($pathCollMap, array_flip($paths)); + $unsetPayload = []; + $pullPayload = []; + foreach ($deleteColls as $propertyPath => $coll) { + $deleteDiff = $deleteDiffMap[$propertyPath]; + foreach ($deleteDiff as $key => $document) { + $unsetPayload[$propertyPath . '.' . $key] = true; } + $pullPayload[$propertyPath] = null; + } - /** - * @todo This is a hack right now because we don't have a proper way to - * remove an element from an array by its key. Unsetting the key results - * in the element being left in the array as null so we have to pull - * null values. - */ - $this->executeQuery($parents[$oid], ['$pull' => $pullPayload], $options); + if (! empty($unsetPayload)) { + $this->executeQuery($parent, ['$unset' => $unsetPayload], $options); + } + if (empty($pullPayload)) { + return; } + + /** + * @todo This is a hack right now because we don't have a proper way to + * remove an element from an array by its key. Unsetting the key results + * in the element being left in the array as null so we have to pull + * null values. + */ + $this->executeQuery($parent, ['$pull' => $pullPayload], $options); } /** @@ -448,16 +429,16 @@ private function insertElements(PersistentCollectionInterface $coll, array $opti * * This method is intended to be used with the "pushAll" and "addToSet" strategies. * + * @param object $parent * @param PersistentCollectionInterface[] $collections * @param array $options */ - private function insertCollections(array $collections, array $options) + private function insertCollections($parent, array $collections, array $options) { - $parents = []; $pushAllPathCollMap = []; $addToSetPathCollMap = []; - $pushAllPathsMap = []; - $addToSetPathsMap = []; + $pushAllPaths = []; + $addToSetPaths = []; $diffsMap = []; foreach ($collections as $coll) { @@ -474,24 +455,18 @@ private function insertCollections(array $collections, array $options) $mapping = $coll->getMapping(); $strategy = $mapping['strategy']; - if (empty(self::INSERT_STRATEGIES_MAP[$strategy])) { - throw new LogicException('Invalid strategy ' . $strategy . ' given for insertCollections'); - } - - list($propertyPath, $parent) = $this->getPathAndParent($coll); - $oid = spl_object_hash($parent); - $parents[$oid] = $parent; - $diffsMap[$oid][$propertyPath] = $insertDiff; + list($propertyPath) = $this->getPathAndParent($coll); + $diffsMap[$propertyPath] = $insertDiff; switch ($strategy) { case ClassMetadata::STORAGE_STRATEGY_PUSH_ALL: - $pushAllPathCollMap[$oid][$propertyPath] = $coll; - $pushAllPathsMap[$oid][] = $propertyPath; + $pushAllPathCollMap[$propertyPath] = $coll; + $pushAllPaths[] = $propertyPath; break; case ClassMetadata::STORAGE_STRATEGY_ADD_TO_SET: - $addToSetPathCollMap[$oid][$propertyPath] = $coll; - $addToSetPathsMap[$oid][] = $propertyPath; + $addToSetPathCollMap[$propertyPath] = $coll; + $addToSetPaths[] = $propertyPath; break; default: @@ -499,28 +474,26 @@ private function insertCollections(array $collections, array $options) } } - foreach ($parents as $oid => $parent) { - if (! empty($pushAllPathsMap[$oid])) { - $this->pushAllCollections( - $parent, - $pushAllPathsMap[$oid], - $pushAllPathCollMap[$oid], - $diffsMap[$oid], - $options - ); - } - if (empty($addToSetPathsMap[$oid])) { - continue; - } - - $this->addToSetCollections( + if (! empty($pushAllPaths)) { + $this->pushAllCollections( $parent, - $addToSetPathsMap[$oid], - $addToSetPathCollMap[$oid], - $diffsMap[$oid], + $pushAllPaths, + $pushAllPathCollMap, + $diffsMap, $options ); } + if (empty($addToSetPaths)) { + return; + } + + $this->addToSetCollections( + $parent, + $addToSetPaths, + $addToSetPathCollMap, + $diffsMap, + $options + ); } /** diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php index 93e05d39dd..c35ee55d97 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php @@ -1364,7 +1364,7 @@ private function handleCollections($document, $options) $collections[] = $coll; } if (! empty($collections)) { - $this->cp->deleteAll($collections, $options); + $this->cp->deleteAll($document, $collections, $options); } // Collection updates (deleteRows, updateRows, insertRows) $collections = []; @@ -1376,7 +1376,7 @@ private function handleCollections($document, $options) $collections[] = $coll; } if (! empty($collections)) { - $this->cp->updateAll($collections, $options); + $this->cp->updateAll($document, $collections, $options); } // Take new snapshots from visited collections foreach ($this->uow->getVisitedCollections($document) as $coll) { diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php index 26cc00d8f4..5caed4f1b3 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php @@ -58,7 +58,7 @@ public function testDeleteAllEmbedMany() { $persister = $this->getCollectionPersister(); $user = $this->getTestUser('jwage'); - $persister->deleteAll([$user->categories], []); + $persister->deleteAll($user, [$user->categories], []); $user = $this->dm->getDocumentCollection(CollectionPersisterUser::class)->findOne(['username' => 'jwage']); $this->assertArrayNotHasKey('categories', $user, 'Test that the categories field was deleted'); } @@ -67,7 +67,7 @@ public function testDeleteAllReferenceMany() { $persister = $this->getCollectionPersister(); $user = $this->getTestUser('jwage'); - $persister->deleteAll([$user->phonenumbers], []); + $persister->deleteAll($user, [$user->phonenumbers], []); $user = $this->dm->getDocumentCollection(CollectionPersisterUser::class)->findOne(['username' => 'jwage']); $this->assertArrayNotHasKey('phonenumbers', $user, 'Test that the phonenumbers field was deleted'); } @@ -77,6 +77,7 @@ public function testDeleteAllNestedEmbedMany() $persister = $this->getCollectionPersister(); $user = $this->getTestUser('jwage'); $persister->deleteAll( + $user, [$user->categories[0]->children[0]->children, $user->categories[0]->children[1]->children], [] ); @@ -84,6 +85,7 @@ public function testDeleteAllNestedEmbedMany() $this->assertFalse(isset($check['categories']['0']['children'][0]['children'])); $this->assertFalse(isset($check['categories']['0']['children'][1]['children'])); $persister->deleteAll( + $user, [$user->categories[0]->children, $user->categories[1]->children], [] ); @@ -99,6 +101,7 @@ public function testDeleteAllNestedEmbedManyAndNestedParent() $persister = $this->getCollectionPersister(); $user = $this->getTestUser('jwage'); $persister->deleteAll( + $user, [$user->categories[0]->children[0]->children, $user->categories[0]->children[1]->children], [] ); @@ -106,6 +109,7 @@ public function testDeleteAllNestedEmbedManyAndNestedParent() $this->assertFalse(isset($check['categories']['0']['children'][0]['children'])); $this->assertFalse(isset($check['categories']['0']['children'][1]['children'])); $persister->deleteAll( + $user, [$user->categories[0]->children, $user->categories[0]->children[1]->children, $user->categories], [] ); From fcd68ef151f8446c55575470685bf4c0aba0b6ec Mon Sep 17 00:00:00 2001 From: watari Date: Thu, 29 Nov 2018 23:16:20 +0200 Subject: [PATCH 7/8] Updated existing tests for CollectionPersister for tracking executed queries where it is required. Added missing tests for checking simultaneous embed-many collections updates under different storage strategies. --- .../Functional/CollectionPersisterTest.php | 362 +++++++++++++++++- 1 file changed, 359 insertions(+), 3 deletions(-) diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php index 5caed4f1b3..824f0a57e1 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php @@ -2,13 +2,32 @@ namespace Doctrine\ODM\MongoDB\Tests\Functional; +use Doctrine\Common\Collections\ArrayCollection; use Doctrine\ODM\MongoDB\Tests\BaseTest; use Doctrine\ODM\MongoDB\Persisters\CollectionPersister; use Doctrine\ODM\MongoDB\Persisters\PersistenceBuilder; use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM; +use Doctrine\ODM\MongoDB\Tests\QueryLogger; class CollectionPersisterTest extends BaseTest { + /** + * @var QueryLogger + */ + private $ql; + + protected function getConfiguration() + { + if ( ! isset($this->ql)) { + $this->ql = new QueryLogger(); + } + + $config = parent::getConfiguration(); + $config->setLoggerCallable($this->ql); + + return $config; + } + public function testDeleteReferenceMany() { $persister = $this->getCollectionPersister(); @@ -76,19 +95,23 @@ public function testDeleteAllNestedEmbedMany() { $persister = $this->getCollectionPersister(); $user = $this->getTestUser('jwage'); + $this->ql->clear(); $persister->deleteAll( $user, [$user->categories[0]->children[0]->children, $user->categories[0]->children[1]->children], [] ); + $this->assertCount(1, $this->ql, 'Deletion of several embedded-many collections of one document requires one query'); $check = $this->dm->getDocumentCollection(CollectionPersisterUser::class)->findOne(['username' => 'jwage']); $this->assertFalse(isset($check['categories']['0']['children'][0]['children'])); $this->assertFalse(isset($check['categories']['0']['children'][1]['children'])); + $this->ql->clear(); $persister->deleteAll( $user, [$user->categories[0]->children, $user->categories[1]->children], [] ); + $this->assertCount(1, $this->ql, 'Deletion of several embedded-many collections of one document requires one query'); $check = $this->dm->getDocumentCollection(CollectionPersisterUser::class)->findOne(['username' => 'jwage']); $this->assertFalse(isset($check['categories'][0]['children']), 'Test that the nested children categories field was deleted'); $this->assertTrue(isset($check['categories'][0]), 'Test that the category with the children still exists'); @@ -100,19 +123,23 @@ public function testDeleteAllNestedEmbedManyAndNestedParent() { $persister = $this->getCollectionPersister(); $user = $this->getTestUser('jwage'); + $this->ql->clear(); $persister->deleteAll( $user, [$user->categories[0]->children[0]->children, $user->categories[0]->children[1]->children], [] ); + $this->assertCount(1, $this->ql, 'Deletion of several embedded-many collections of one document requires one query'); $check = $this->dm->getDocumentCollection(CollectionPersisterUser::class)->findOne(['username' => 'jwage']); $this->assertFalse(isset($check['categories']['0']['children'][0]['children'])); $this->assertFalse(isset($check['categories']['0']['children'][1]['children'])); + $this->ql->clear(); $persister->deleteAll( $user, [$user->categories[0]->children, $user->categories[0]->children[1]->children, $user->categories], [] ); + $this->assertCount(1, $this->ql, 'Deletion of several embedded-many collections of one document requires one query'); $check = $this->dm->getDocumentCollection(CollectionPersisterUser::class)->findOne(['username' => 'jwage']); $this->assertFalse(isset($check['categories']), 'Test that the nested categories field was deleted'); } @@ -120,7 +147,6 @@ public function testDeleteAllNestedEmbedManyAndNestedParent() public function testDeleteRows() { - $persister = $this->getCollectionPersister(); $user = $this->getTestUser('jwage'); unset($user->phonenumbers[0]); @@ -132,7 +158,9 @@ public function testDeleteRows() unset($user->categories[0]->children[1]->children[0]); unset($user->categories[0]->children[1]->children[1]); + $this->ql->clear(); $this->dm->flush(); + $this->assertCount(2, $this->ql, 'Modification of several embedded-many collections of one document requires two queries'); $check = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(array('username' => 'jwage')); @@ -147,7 +175,9 @@ public function testDeleteRows() unset($user->categories[0]); unset($user->categories[1]); + $this->ql->clear(); $this->dm->flush(); + $this->assertCount(2, $this->ql, 'Modification of embedded-many collection of one document requires two queries'); $check = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(array('username' => 'jwage')); $this->assertFalse(isset($check['categories'][0])); @@ -159,7 +189,9 @@ public function testInsertRows() $user = $this->getTestUser('jwage'); $user->phonenumbers[] = new CollectionPersisterPhonenumber('6155139185'); $user->phonenumbers[] = new CollectionPersisterPhonenumber('6155139185'); + $this->ql->clear(); $this->dm->flush(); + $this->assertCount(2, $this->ql, 'Modification of embedded-many collection of one document requires two queries'); $check = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(array('username' => 'jwage')); $this->assertCount(4, $check['phonenumbers']); @@ -168,7 +200,13 @@ public function testInsertRows() $user->categories[] = new CollectionPersisterCategory('Test'); $user->categories[] = new CollectionPersisterCategory('Test'); + $this->ql->clear(); $this->dm->flush(); + $this->assertCount( + 1, + $this->ql, + 'Modification of embedded-many collection of one document requires one query since no existing collection elements was removed' + ); $check = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(array('username' => 'jwage')); $this->assertCount(4, $check['categories']); @@ -177,7 +215,13 @@ public function testInsertRows() $user->categories[3]->children[1] = new CollectionPersisterCategory('Test'); $user->categories[3]->children[1]->children[0] = new CollectionPersisterCategory('Test'); $user->categories[3]->children[1]->children[1] = new CollectionPersisterCategory('Test'); + $this->ql->clear(); $this->dm->flush(); + $this->assertCount( + 1, + $this->ql, + 'Modification of embedded-many collection of one document requires one query since no existing collection elements was removed' + ); $check = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterUser')->findOne(array('username' => 'jwage')); $this->assertCount(2, $check['categories'][3]['children']); @@ -230,7 +274,13 @@ public function testNestedEmbedManySetStrategy() $commentA->comments->set('b', $commentAB); $this->dm->persist($post); + $this->ql->clear(); $this->dm->flush(); + $this->assertCount( + 1, + $this->ql, + 'Insertion of embedded-many collection of one document requires no additional queries' + ); $doc = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterPost')->findOne(array('post' => 'postA')); @@ -251,7 +301,13 @@ public function testNestedEmbedManySetStrategy() $commentB->comments->set('a', $commentBA); $this->dm->persist($post); + $this->ql->clear(); $this->dm->flush(); + $this->assertCount( + 1, + $this->ql, + 'Modification of embedded-many collection of one document requires one query since no existing collection elements was removed' + ); $doc = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterPost')->findOne(array('post' => 'postA')); @@ -271,7 +327,13 @@ public function testNestedEmbedManySetStrategy() $commentAB->comment = 'commentAB-modified'; $this->dm->persist($post); + $this->ql->clear(); $this->dm->flush(); + $this->assertCount( + 1, + $this->ql, + 'Modification of embedded-many collection of one document requires one query since no existing collection elements was removed' + ); $doc = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterPost')->findOne(array('post' => 'postA')); @@ -288,7 +350,13 @@ public function testNestedEmbedManySetStrategy() unset($post->comments['b']); $this->dm->persist($post); + $this->ql->clear(); $this->dm->flush(); + $this->assertCount( + 1, + $this->ql, + 'Modification of embedded-many collection of one document requires one query since collection, from which element was removed, have "set" store strategy.' + ); $doc = $this->dm->getDocumentCollection(__NAMESPACE__ . '\CollectionPersisterPost')->findOne(array('post' => 'postA')); @@ -315,6 +383,204 @@ public function testFindBySetStrategyKey() $this->assertSame($post, $this->dm->getRepository(get_class($post))->findOneBy(array('comments.a.by' => 'userA'))); $this->assertSame($post, $this->dm->getRepository(get_class($post))->findOneBy(array('comments.a.comments.b.by' => 'userB'))); } + + public function testPersistSeveralNestedEmbedManySetStrategy() + { + $structure = new CollectionPersisterStructure(); + $structure->set->add(new CollectionPersisterNestedStructure('nested1')); + $structure->set->add(new CollectionPersisterNestedStructure('nested2')); + $structure->set2->add(new CollectionPersisterNestedStructure('nested3')); + $structure->set2->add(new CollectionPersisterNestedStructure('nested4')); + + $this->dm->persist($structure); + $this->dm->flush(); + $this->assertCount( + 1, + $this->ql, + 'Insertion of embedded-many collections of one document by "set" strategy requires no additional queries' + ); + + $structure->set->clear(); + $structure->set->add(new CollectionPersisterNestedStructure('nested5')); + $structure->set2->add(new CollectionPersisterNestedStructure('nested6')); + + $this->ql->clear(); + $this->dm->flush(); + $this->assertCount( + 1, + $this->ql, + 'Modification of embedded-many collections of one document by "set" strategy requires one query' + ); + + $this->assertSame($structure, $this->dm->getRepository(get_class($structure))->findOneBy(['id' => $structure->id])); + } + + public function testPersistSeveralNestedEmbedManySetArrayStrategy() + { + $structure = new CollectionPersisterStructure(); + $structure->setArray->add(new CollectionPersisterNestedStructure('nested1')); + $structure->setArray->add(new CollectionPersisterNestedStructure('nested2')); + $structure->setArray2->add(new CollectionPersisterNestedStructure('nested3')); + $structure->setArray2->add(new CollectionPersisterNestedStructure('nested4')); + + $this->dm->persist($structure); + $this->dm->flush(); + $this->assertCount( + 1, + $this->ql, + 'Insertion of embedded-many collections of one document by "setArray" strategy requires no additional queries' + ); + + $structure->setArray->clear(); + $structure->setArray->add(new CollectionPersisterNestedStructure('nested5')); + $structure->setArray2->add(new CollectionPersisterNestedStructure('nested6')); + + $this->ql->clear(); + $this->dm->flush(); + $this->assertCount( + 1, + $this->ql, + 'Modification of embedded-many collections of one document by "setArray" strategy requires one query' + ); + + $this->assertSame($structure, $this->dm->getRepository(get_class($structure))->findOneBy(['id' => $structure->id])); + } + + public function testPersistSeveralNestedEmbedManyAddToSetStrategy() + { + $structure = new CollectionPersisterStructure(); + $structure->addToSet->add(new CollectionPersisterNestedStructure('nested1')); + $structure->addToSet->add(new CollectionPersisterNestedStructure('nested2')); + $structure->addToSet2->add(new CollectionPersisterNestedStructure('nested3')); + $structure->addToSet2->add(new CollectionPersisterNestedStructure('nested4')); + + $this->dm->persist($structure); + $this->dm->flush(); + $this->assertCount( + 2, + $this->ql, + 'Insertion of embedded-many collections of one document by "addToSet" strategy requires one additional query' + ); + + $structure->addToSet->clear(); + $structure->addToSet->add(new CollectionPersisterNestedStructure('nested5')); + $structure->addToSet2->add(new CollectionPersisterNestedStructure('nested6')); + + $this->ql->clear(); + $this->dm->flush(); + $this->assertCount( + 2, + $this->ql, + 'Modification of embedded-many collections of one document by "addToSet" strategy requires two queries' + ); + + $this->assertSame($structure, $this->dm->getRepository(get_class($structure))->findOneBy(['id' => $structure->id])); + } + + public function testPersistSeveralNestedEmbedManyPushAllStrategy() + { + $structure = new CollectionPersisterStructure(); + $structure->pushAll->add(new CollectionPersisterNestedStructure('nested1')); + $structure->pushAll->add(new CollectionPersisterNestedStructure('nested2')); + $structure->pushAll2->add(new CollectionPersisterNestedStructure('nested3')); + $structure->pushAll2->add(new CollectionPersisterNestedStructure('nested4')); + + $this->dm->persist($structure); + $this->dm->flush(); + $this->assertCount( + 1, + $this->ql, + 'Insertion of embedded-many collections of one document by "pushAll" strategy requires no additional queries' + ); + + $structure->pushAll->add(new CollectionPersisterNestedStructure('nested5')); + $structure->pushAll2->add(new CollectionPersisterNestedStructure('nested6')); + + $this->ql->clear(); + $this->dm->persist($structure); + $this->dm->flush(); + $this->assertCount( + 2, + $this->ql, + 'Modification of embedded-many collections of one document by "pushAll" strategy requires two queries' + ); + + $this->assertSame($structure, $this->dm->getRepository(get_class($structure))->findOneBy(['id' => $structure->id])); + } + + public function testPersistSeveralNestedEmbedManyDifferentStrategies() + { + $structure = new CollectionPersisterStructure(); + $structure->set->add(new CollectionPersisterNestedStructure('nested1')); + $structure->setArray->add(new CollectionPersisterNestedStructure('nested2')); + $structure->pushAll->add(new CollectionPersisterNestedStructure('nested3')); + + $this->dm->persist($structure); + $this->dm->flush(); + $this->assertCount( + 1, + $this->ql, + 'Insertion of embedded-many collections of one document by "set", "setArray" and "pushAll" strategies requires no additional queries' + ); + + $structure->set->remove(0); + $structure->set->add(new CollectionPersisterNestedStructure('nested5')); + $structure->pushAll->remove(0); + $structure->setArray->add(new CollectionPersisterNestedStructure('nested6')); + $structure->setArray->remove(0); + $structure->pushAll->add(new CollectionPersisterNestedStructure('nested7')); + + $this->ql->clear(); + $this->dm->persist($structure); + $this->dm->flush(); + $this->assertCount( + 4, + $this->ql, + 'Modification of embedded-many collections of one document by "set", "setArray" and "pushAll" strategies requires two queries' + ); + + $this->assertSame($structure, $this->dm->getRepository(get_class($structure))->findOneBy(['id' => $structure->id])); + } + + public function testPersistSeveralNestedEmbedManyDifferentStrategiesDeepNesting() + { + $structure = new CollectionPersisterStructure(); + $nested1 = new CollectionPersisterNestedStructure('nested1'); + $nested2 = new CollectionPersisterNestedStructure('nested2'); + $nested3 = new CollectionPersisterNestedStructure('nested3'); + $nested1->setArray->add(new CollectionPersisterNestedStructure('setArray_nested1')); + $nested2->pushAll->add(new CollectionPersisterNestedStructure('pushAll_nested1')); + $nested3->set->add(new CollectionPersisterNestedStructure('set_nested1')); + $structure->set->add($nested1); + $structure->setArray->add($nested2); + $structure->pushAll->add($nested3); + + $this->dm->persist($structure); + $this->dm->flush(); + $this->assertCount( + 1, + $this->ql, + 'Insertion of embedded-many collections of one document by "set", "setArray" and "pushAll" strategies requires no additional queries' + ); + + $structure->set->remove(0); + $structure->set->add(new CollectionPersisterNestedStructure('nested5')); + $structure->setArray->get(0)->set->add(new CollectionPersisterNestedStructure('set_nested1')); + $structure->pushAll->get(0)->set->clear(); + $structure->pushAll->add(new CollectionPersisterNestedStructure('nested5')); + $structure->pushAll->remove(0); + + $this->ql->clear(); + $this->dm->persist($structure); + $this->dm->flush(); + $this->assertCount( + 5, + $this->ql, + 'Modification of embedded-many collections of one document by "set", "setArray" and "pushAll" strategies requires two queries' + ); + + $this->assertSame($structure, $this->dm->getRepository(get_class($structure))->findOneBy(['id' => $structure->id])); + } } /** @ODM\Document(collection="user_collection_persister_test") */ @@ -377,7 +643,7 @@ class CollectionPersisterPost function __construct($post) { - $this->comments = new \Doctrine\Common\Collections\ArrayCollection(); + $this->comments = new ArrayCollection(); $this->post = $post; } @@ -401,8 +667,98 @@ class CollectionPersisterComment function __construct($comment, $by) { - $this->comments = new \Doctrine\Common\Collections\ArrayCollection(); + $this->comments = new ArrayCollection(); $this->comment = $comment; $this->by = $by; } } + +/** @ODM\Document(collection="structure_collection_persister_test") */ +class CollectionPersisterStructure +{ + /** @ODM\Id */ + public $id; + + /** @ODM\EmbedMany(targetDocument=CollectionPersisterNestedStructure::class, strategy="addToSet") */ + public $addToSet; + + /** @ODM\EmbedMany(targetDocument=CollectionPersisterNestedStructure::class, strategy="addToSet") */ + public $addToSet2; + + /** @ODM\EmbedMany(targetDocument=CollectionPersisterNestedStructure::class, strategy="set") */ + public $set; + + /** @ODM\EmbedMany(targetDocument=CollectionPersisterNestedStructure::class, strategy="set") */ + public $set2; + + /** @ODM\EmbedMany(targetDocument=CollectionPersisterNestedStructure::class, strategy="setArray") */ + public $setArray; + + /** @ODM\EmbedMany(targetDocument=CollectionPersisterNestedStructure::class, strategy="setArray") */ + public $setArray2; + + /** @ODM\EmbedMany(targetDocument=CollectionPersisterNestedStructure::class, strategy="pushAll") */ + public $pushAll; + + /** @ODM\EmbedMany(targetDocument=CollectionPersisterNestedStructure::class, strategy="pushAll") */ + public $pushAll2; + + public function __construct() + { + $this->addToSet = new ArrayCollection(); + $this->addToSet2 = new ArrayCollection(); + $this->set = new ArrayCollection(); + $this->set2 = new ArrayCollection(); + $this->setArray = new ArrayCollection(); + $this->setArray2 = new ArrayCollection(); + $this->pushAll = new ArrayCollection(); + $this->pushAll2 = new ArrayCollection(); + } +} + +/** @ODM\EmbeddedDocument */ +class CollectionPersisterNestedStructure +{ + /** @ODM\Id */ + public $id; + + /** @ODM\Field(type="string") */ + public $field; + + /** @ODM\EmbedMany(targetDocument=CollectionPersisterNestedStructure::class, strategy="addToSet") */ + public $addToSet; + + /** @ODM\EmbedMany(targetDocument=CollectionPersisterNestedStructure::class, strategy="addToSet") */ + public $addToSet2; + + /** @ODM\EmbedMany(targetDocument=CollectionPersisterNestedStructure::class, strategy="set") */ + public $set; + + /** @ODM\EmbedMany(targetDocument=CollectionPersisterNestedStructure::class, strategy="set") */ + public $set2; + + /** @ODM\EmbedMany(targetDocument=CollectionPersisterNestedStructure::class, strategy="setArray") */ + public $setArray; + + /** @ODM\EmbedMany(targetDocument=CollectionPersisterNestedStructure::class, strategy="setArray") */ + public $setArray2; + + /** @ODM\EmbedMany(targetDocument=CollectionPersisterNestedStructure::class, strategy="pushAll") */ + public $pushAll; + + /** @ODM\EmbedMany(targetDocument=CollectionPersisterNestedStructure::class, strategy="pushAll") */ + public $pushAll2; + + public function __construct($field) + { + $this->field = $field; + $this->addToSet = new ArrayCollection(); + $this->addToSet2 = new ArrayCollection(); + $this->set = new ArrayCollection(); + $this->set2 = new ArrayCollection(); + $this->setArray = new ArrayCollection(); + $this->setArray2 = new ArrayCollection(); + $this->pushAll = new ArrayCollection(); + $this->pushAll2 = new ArrayCollection(); + } +} From f122613b693c894a053f0a08a9b499305501a8c2 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 4 Feb 2019 07:38:15 +0100 Subject: [PATCH 8/8] Add BC layer for optimized collection operations --- UPGRADE-1.3.md | 7 +++++++ .../ODM/MongoDB/Persisters/CollectionPersister.php | 12 +++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/UPGRADE-1.3.md b/UPGRADE-1.3.md index f1eaadb36a..22cc897366 100644 --- a/UPGRADE-1.3.md +++ b/UPGRADE-1.3.md @@ -86,6 +86,13 @@ cause an exception in 2.0. It is possible to have multiple fields with the same name in the database as long as all but one of them have the `notSaved` option set. +## Persisters + + * The `delete` and `update` methods in + `Doctrine\ODM\MongoDB\Persisters\CollectionPersister` are deprecated. Use + `deleteAll` and `updateAll` instead. The method signatures will be adapted + to match those of `deleteAll` and `updateAll` in 2.0. + ## Proxies * The usage of proxies from Doctrine Common was deprecated and will be replaced diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php index 29d82b5f1d..fb1c6cfbd8 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php @@ -27,6 +27,7 @@ use Doctrine\ODM\MongoDB\UnitOfWork; use Doctrine\ODM\MongoDB\Utility\CollectionHelper; use UnexpectedValueException; +use const E_USER_DEPRECATED; use function array_diff_key; use function array_fill_keys; use function array_flip; @@ -40,8 +41,9 @@ use function get_class; use function implode; use function sort; -use function spl_object_hash; +use function sprintf; use function strpos; +use function trigger_error; /** * The CollectionPersister is responsible for persisting collections of embedded @@ -118,9 +120,13 @@ public function deleteAll($parent, array $collections, array $options) * * @param PersistentCollectionInterface $coll * @param array $options + * + * @deprecated This method will be replaced with the deleteAll method */ public function delete(PersistentCollectionInterface $coll, array $options) { + @trigger_error(sprintf('The "%s" method is deprecated and will be changed to the signature of deleteAll in 2.0.', __METHOD__), E_USER_DEPRECATED); + $mapping = $coll->getMapping(); if ($mapping['isInverseSide']) { return; // ignore inverse side @@ -139,9 +145,13 @@ public function delete(PersistentCollectionInterface $coll, array $options) * * @param PersistentCollectionInterface $coll * @param array $options + * + * @deprecated This method will be replaced with the updateAll method */ public function update(PersistentCollectionInterface $coll, array $options) { + @trigger_error(sprintf('The "%s" method is deprecated and will be changed to the signature of updateAll in 2.0.', __METHOD__), E_USER_DEPRECATED); + $mapping = $coll->getMapping(); if ($mapping['isInverseSide']) {