Skip to content

Commit

Permalink
Add validation for storage strategies
Browse files Browse the repository at this point in the history 
This also forces all EmbedOne and ReferenceOne mappings to use the set strategy. All other strategies will throw an error.
  • Loading branch information
@alcaeus
alcaeus committed Feb 6, 2016
1 parent 3177284 commit 8398477
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 29 deletions.
71 changes: 50 additions & 21 deletions lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadataInfo.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1165,23 +1165,6 @@ public function mapField(array $mapping)
}
}
unset($this->generatorOptions['type']);
} elseif (!isset($mapping['embedded']) && !isset($mapping['reference']) && isset($mapping['type'])) {
switch ($mapping['type']) {
case 'int':
case 'float':
$allowedStrategies = [self::STORAGE_STRATEGY_SET, self::STORAGE_STRATEGY_INCREMENT];
break;
default:
$allowedStrategies = [self::STORAGE_STRATEGY_SET];
}

if (isset($mapping['strategy'])) {
if (! in_array($mapping['strategy'], $allowedStrategies)) {
throw MappingException::invalidStorageStrategy($this->name, $mapping['fieldName'], $mapping['type'], $mapping['strategy']);
}
} else {
$mapping['strategy'] = self::STORAGE_STRATEGY_SET;
}
}

if ( ! isset($mapping['nullable'])) {
Expand Down Expand Up @@ -1251,10 +1234,7 @@ public function mapField(array $mapping)
}
}

if (isset($mapping['reference']) && $mapping['type'] === 'many' && $mapping['isOwningSide']
&& ! empty($mapping['sort']) && ! CollectionHelper::usesSet($mapping['strategy'])) {
throw MappingException::referenceManySortMustNotBeUsedWithNonSetCollectionStrategy($this->name, $mapping['fieldName'], $mapping['strategy']);
}
$this->applyStorageStrategy($mapping);

$this->fieldMappings[$mapping['fieldName']] = $mapping;
if (isset($mapping['association'])) {
Expand All @@ -1264,6 +1244,55 @@ public function mapField(array $mapping)
return $mapping;
}

/**
* Validates the storage strategy of a mapping for consistency
* @param array $mapping
* @throws \Doctrine\ODM\MongoDB\Mapping\MappingException
*/
private function applyStorageStrategy(array &$mapping)
{
if (! isset($mapping['type']) || isset($mapping['id'])) {
return;
}

switch (true) {
case $mapping['type'] == 'int':
case $mapping['type'] == 'float':
$defaultStrategy = self::STORAGE_STRATEGY_SET;
$allowedStrategies = [self::STORAGE_STRATEGY_SET, self::STORAGE_STRATEGY_INCREMENT];
break;

case $mapping['type'] == 'many':
$defaultStrategy = CollectionHelper::DEFAULT_STRATEGY;
$allowedStrategies = [
self::STORAGE_STRATEGY_PUSH_ALL,
self::STORAGE_STRATEGY_ADD_TO_SET,
self::STORAGE_STRATEGY_SET,
self::STORAGE_STRATEGY_SET_ARRAY,
self::STORAGE_STRATEGY_ATOMIC_SET,
self::STORAGE_STRATEGY_ATOMIC_SET_ARRAY,
];
break;

default:
$defaultStrategy = self::STORAGE_STRATEGY_SET;
$allowedStrategies = [self::STORAGE_STRATEGY_SET];
}

if (! isset($mapping['strategy'])) {
$mapping['strategy'] = $defaultStrategy;
}

if (! in_array($mapping['strategy'], $allowedStrategies)) {
throw MappingException::invalidStorageStrategy($this->name, $mapping['fieldName'], $mapping['type'], $mapping['strategy']);
}

if (isset($mapping['reference']) && $mapping['type'] === 'many' && $mapping['isOwningSide']
&& ! empty($mapping['sort']) && ! CollectionHelper::usesSet($mapping['strategy'])) {
throw MappingException::referenceManySortMustNotBeUsedWithNonSetCollectionStrategy($this->name, $mapping['fieldName'], $mapping['strategy']);
}
}

/**
* Map a MongoGridFSFile.
*
Expand Down
6 changes: 4 additions & 2 deletions lib/Doctrine/ODM/MongoDB/Mapping/Driver/XmlDriver.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,12 +236,13 @@ private function addFieldMapping(ClassMetadataInfo $class, $mapping)
private function addEmbedMapping(ClassMetadataInfo $class, $embed, $type)
{
$attributes = $embed->attributes();
$defaultStrategy = $type == 'one' ? ClassMetadataInfo::STORAGE_STRATEGY_SET : CollectionHelper::DEFAULT_STRATEGY;
$mapping = array(
'type' => $type,
'embedded' => true,
'targetDocument' => isset($attributes['target-document']) ? (string) $attributes['target-document'] : null,
'name' => (string) $attributes['field'],
'strategy' => isset($attributes['strategy']) ? (string) $attributes['strategy'] : CollectionHelper::DEFAULT_STRATEGY,
'strategy' => isset($attributes['strategy']) ? (string) $attributes['strategy'] : $defaultStrategy,
);
if (isset($attributes['fieldName'])) {
$mapping['fieldName'] = (string) $attributes['fieldName'];
Expand Down Expand Up @@ -275,6 +276,7 @@ private function addReferenceMapping(ClassMetadataInfo $class, $reference, $type
$cascade = current($cascade) ?: next($cascade);
}
$attributes = $reference->attributes();
$defaultStrategy = $type == 'one' ? ClassMetadataInfo::STORAGE_STRATEGY_SET : CollectionHelper::DEFAULT_STRATEGY;
$mapping = array(
'cascade' => $cascade,
'orphanRemoval' => isset($attributes['orphan-removal']) ? ('true' === (string) $attributes['orphan-removal']) : false,
Expand All @@ -283,7 +285,7 @@ private function addReferenceMapping(ClassMetadataInfo $class, $reference, $type
'simple' => isset($attributes['simple']) ? ('true' === (string) $attributes['simple']) : false,
'targetDocument' => isset($attributes['target-document']) ? (string) $attributes['target-document'] : null,
'name' => (string) $attributes['field'],
'strategy' => isset($attributes['strategy']) ? (string) $attributes['strategy'] : CollectionHelper::DEFAULT_STRATEGY,
'strategy' => isset($attributes['strategy']) ? (string) $attributes['strategy'] : $defaultStrategy,
'inversedBy' => isset($attributes['inversed-by']) ? (string) $attributes['inversed-by'] : null,
'mappedBy' => isset($attributes['mapped-by']) ? (string) $attributes['mapped-by'] : null,
'repositoryMethod' => isset($attributes['repository-method']) ? (string) $attributes['repository-method'] : null,
Expand Down
6 changes: 4 additions & 2 deletions lib/Doctrine/ODM/MongoDB/Mapping/Driver/YamlDriver.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,12 +233,13 @@ private function addFieldMapping(ClassMetadataInfo $class, $mapping)

private function addMappingFromEmbed(ClassMetadataInfo $class, $fieldName, $embed, $type)
{
$defaultStrategy = $type == 'one' ? ClassMetadataInfo::STORAGE_STRATEGY_SET : CollectionHelper::DEFAULT_STRATEGY;
$mapping = array(
'type' => $type,
'embedded' => true,
'targetDocument' => isset($embed['targetDocument']) ? $embed['targetDocument'] : null,
'fieldName' => $fieldName,
'strategy' => isset($embed['strategy']) ? (string) $embed['strategy'] : CollectionHelper::DEFAULT_STRATEGY,
'strategy' => isset($embed['strategy']) ? (string) $embed['strategy'] : $defaultStrategy,
);
if (isset($embed['name'])) {
$mapping['name'] = $embed['name'];
Expand All @@ -257,6 +258,7 @@ private function addMappingFromEmbed(ClassMetadataInfo $class, $fieldName, $embe

private function addMappingFromReference(ClassMetadataInfo $class, $fieldName, $reference, $type)
{
$defaultStrategy = $type == 'one' ? ClassMetadataInfo::STORAGE_STRATEGY_SET : CollectionHelper::DEFAULT_STRATEGY;
$mapping = array(
'cascade' => isset($reference['cascade']) ? $reference['cascade'] : null,
'orphanRemoval' => isset($reference['orphanRemoval']) ? $reference['orphanRemoval'] : false,
Expand All @@ -265,7 +267,7 @@ private function addMappingFromReference(ClassMetadataInfo $class, $fieldName, $
'simple' => isset($reference['simple']) ? (boolean) $reference['simple'] : false,
'targetDocument' => isset($reference['targetDocument']) ? $reference['targetDocument'] : null,
'fieldName' => $fieldName,
'strategy' => isset($reference['strategy']) ? (string) $reference['strategy'] : CollectionHelper::DEFAULT_STRATEGY,
'strategy' => isset($reference['strategy']) ? (string) $reference['strategy'] : $defaultStrategy,
'inversedBy' => isset($reference['inversedBy']) ? (string) $reference['inversedBy'] : null,
'mappedBy' => isset($reference['mappedBy']) ? (string) $reference['mappedBy'] : null,
'repositoryMethod' => isset($reference['repositoryMethod']) ? (string) $reference['repositoryMethod'] : null,
Expand Down
2 changes: 1 addition & 1 deletion lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1043,7 +1043,7 @@ private function prepareQueryElement($fieldName, $value = null, $class = null, $
return array($fieldName, $value);
}

if (isset($mapping['strategy']) && CollectionHelper::isHash($mapping['strategy'])
if ($mapping['type'] == 'many' && isset($mapping['strategy']) && CollectionHelper::isHash($mapping['strategy'])
&& isset($e[2])) {
$objectProperty = $e[2];
$objectPropertyPrefix = $e[1] . '.';
Expand Down
6 changes: 3 additions & 3 deletions tests/Doctrine/ODM/MongoDB/Tests/Mapping/Driver/AbstractDriverTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public function testDriver()
'isInverseSide' => false,
'isOwningSide' => true,
'nullable' => false,
'strategy' => ClassMetadataInfo::STORAGE_STRATEGY_PUSH_ALL,
'strategy' => ClassMetadataInfo::STORAGE_STRATEGY_SET,
), $classMetadata->fieldMappings['address']);

$this->assertEquals(array(
Expand Down Expand Up @@ -152,7 +152,7 @@ public function testDriver()
'isInverseSide' => false,
'isOwningSide' => true,
'nullable' => false,
'strategy' => ClassMetadataInfo::STORAGE_STRATEGY_PUSH_ALL,
'strategy' => ClassMetadataInfo::STORAGE_STRATEGY_SET,
'inversedBy' => null,
'mappedBy' => null,
'repositoryMethod' => null,
Expand All @@ -178,7 +178,7 @@ public function testDriver()
'isInverseSide' => false,
'isOwningSide' => true,
'nullable' => false,
'strategy' => ClassMetadataInfo::STORAGE_STRATEGY_PUSH_ALL,
'strategy' => ClassMetadataInfo::STORAGE_STRATEGY_SET,
'inversedBy' => null,
'mappedBy' => null,
'repositoryMethod' => null,
Expand Down

0 comments on commit 8398477

Please sign in to comment.