Skip to content

Commit

Permalink
Simplified methods for working with nested collections, remove redund…
Browse files Browse the repository at this point in the history
…ant checks
  • Loading branch information
watari committed Nov 27, 2018
1 parent 7ec5b67 commit 50eef5e
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 132 deletions.
223 changes: 95 additions & 128 deletions lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
use function get_class;
use function implode;
use function sort;
use function spl_object_hash;
use function sprintf;
use function strpos;

Expand All @@ -42,19 +41,7 @@
*/
class CollectionPersister
{
/**
* Validation map that is used for strategy validation in insertCollections method.
*/
public 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;

/** @var PersistenceBuilder */
Expand All @@ -71,14 +58,13 @@ 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 PersistentCollectionInterface[] $collections
* @param array $options
*/
public function deleteAll(array $collections, array $options) : void
public function deleteAll(object $parent, array $collections, array $options) : void
{
$parents = [];
$unsetPathsMap = [];

foreach ($collections as $collection) {
Expand All @@ -89,17 +75,17 @@ public function deleteAll(array $collections, array $options) : void
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');
}
[$propertyPath, $parent] = $this->getPathAndParent($collection);
$oid = spl_object_hash($parent);
$parents[$oid] = $parent;
$unsetPathsMap[$oid][$propertyPath] = true;
[$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);
}

/**
Expand Down Expand Up @@ -159,7 +145,7 @@ public function update(PersistentCollectionInterface $coll, array $options) : vo
* @param PersistentCollectionInterface[] $collections
* @param array $options
*/
public function updateAll(array $collections, array $options) : void
public function updateAll(object $parent, array $collections, array $options) : void
{
$setStrategyColls = [];
$addPushStrategyColls = [];
Expand Down Expand Up @@ -191,14 +177,14 @@ public function updateAll(array $collections, array $options) : void
}

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);
}

/**
Expand Down Expand Up @@ -230,40 +216,35 @@ private function setCollection(PersistentCollectionInterface $coll, array $optio
* @param PersistentCollectionInterface[] $collections
* @param array $options
*/
private function setCollections(array $collections, array $options) : void
private function setCollections(object $parent, array $collections, array $options) : void
{
$parents = [];
$pathCollMap = [];
$pathsMap = [];
$paths = [];
foreach ($collections as $coll) {
[$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;
}
[$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);
}

/**
Expand Down Expand Up @@ -307,11 +288,10 @@ private function deleteElements(PersistentCollectionInterface $coll, array $opti
* @param PersistentCollectionInterface[] $collections
* @param array $options
*/
private function deleteCollections(array $collections, array $options) : void
private function deleteCollections(object $parent, array $collections, array $options) : void
{
$parents = [];
$pathCollMap = [];
$pathsMap = [];
$paths = [];
$deleteDiffMap = [];

foreach ($collections as $coll) {
Expand All @@ -324,43 +304,39 @@ private function deleteCollections(array $collections, array $options) : void
if (empty($deleteDiff)) {
continue;
}
[$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;
}
[$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);
}

/**
Expand Down Expand Up @@ -417,13 +393,12 @@ private function insertElements(PersistentCollectionInterface $coll, array $opti
* @param PersistentCollectionInterface[] $collections
* @param array $options
*/
private function insertCollections(array $collections, array $options) : void
private function insertCollections(object $parent, array $collections, array $options) : void
{
$parents = [];
$pushAllPathCollMap = [];
$addToSetPathCollMap = [];
$pushAllPathsMap = [];
$addToSetPathsMap = [];
$pushAllPaths = [];
$addToSetPaths = [];
$diffsMap = [];

foreach ($collections as $coll) {
Expand All @@ -440,53 +415,45 @@ private function insertCollections(array $collections, array $options) : void
$mapping = $coll->getMapping();
$strategy = $mapping['strategy'];

if (empty(self::INSERT_STRATEGIES_MAP[$strategy])) {
throw new LogicException('Invalid strategy ' . $strategy . ' given for insertCollections');
}

[$propertyPath, $parent] = $this->getPathAndParent($coll);
$oid = spl_object_hash($parent);
$parents[$oid] = $parent;
$diffsMap[$oid][$propertyPath] = $insertDiff;
[$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:
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(
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
);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php
Original file line number Diff line number Diff line change
Expand Up @@ -1279,7 +1279,7 @@ private function handleCollections(object $document, array $options) : void
$collections[] = $coll;
}
if (! empty($collections)) {
$this->cp->deleteAll($collections, $options);
$this->cp->deleteAll($document, $collections, $options);
}
// Collection updates (deleteRows, updateRows, insertRows)
$collections = [];
Expand All @@ -1291,7 +1291,7 @@ private function handleCollections(object $document, array $options) : void
$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) {
Expand Down
Loading

0 comments on commit 50eef5e

Please sign in to comment.