From 242211e054a46b3b3c6ea8dc79d67fa2e075c0e8 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Wed, 26 Jul 2017 15:42:29 +0200 Subject: [PATCH 1/6] Add generic reference as replacement for DBRef --- docs/en/reference/annotations-reference.rst | 13 +- docs/en/reference/reference-mapping.rst | 1 + doctrine-mongo-mapping.xsd | 1 + .../ODM/MongoDB/Aggregation/Stage/Lookup.php | 30 ++++- lib/Doctrine/ODM/MongoDB/DocumentManager.php | 36 +++--- .../ODM/MongoDB/Hydrator/HydratorFactory.php | 4 +- .../ODM/MongoDB/Mapping/ClassMetadataInfo.php | 24 ++++ .../MongoDB/Persisters/DocumentPersister.php | 36 ++++-- lib/Doctrine/ODM/MongoDB/Query/Expr.php | 36 +++--- .../ODM/MongoDB/Query/ReferencePrimer.php | 3 +- lib/Doctrine/ODM/MongoDB/SchemaManager.php | 2 +- .../Tests/Aggregation/Stage/LookupTest.php | 112 ++++++++++++++++++ .../ODM/MongoDB/Tests/DocumentManagerTest.php | 7 ++ .../Functional/DocumentPersisterTest.php | 83 +++++++++++++ tests/Documents/ReferenceUser.php | 46 +++++++ tests/Documents/User.php | 6 + 16 files changed, 383 insertions(+), 57 deletions(-) diff --git a/docs/en/reference/annotations-reference.rst b/docs/en/reference/annotations-reference.rst index 1fcf4b3f69..c13789955a 100644 --- a/docs/en/reference/annotations-reference.rst +++ b/docs/en/reference/annotations-reference.rst @@ -1222,10 +1222,11 @@ Optional attributes: - simple - deprecated (use ``storeAs: id``) - - storeAs - Indicates how to store the reference. ``id`` uses ``MongoId``, - ``dbRef`` uses a `DBRef`_ without ``$db`` value and ``dbRefWithDb`` stores - a full `DBRef`_ (``$ref``, ``$id``, and ``$db``). Note that ``id`` - references are not compatible with the discriminators. + storeAs - Indicates how to store the reference. ``id`` stores the identifier, + ``ref`` an embedded object containing the ``id`` field and (optionally) a + discriminator. ``dbRef`` and ``dbRefWithDb`` store a ``DBRef`` object. They + are deprecated in favor of ``ref``. Note that ``id`` references are not + compatible with the discriminators. - cascade - Cascade Option - @@ -1297,8 +1298,8 @@ Optional attributes: simple - deprecated (use ``storeAs: id``) - storeAs - Indicates how to store the reference. ``id`` uses ``MongoId``, - ``dbRef`` uses a `DBRef`_ without ``$db`` value and ``dbRefWithDb`` stores - a full `DBRef`_ (``$ref``, ``$id``, and ``$db``). Note that ``id`` + ``ref`` stores fields ``id`` and ``ref`` (similar to DBRef without $ prefix) value and ``refWithDb`` stores + a additionally db as parameter. ``dbRef`` and ``dbRefWithDb`` use ``DBRef``, they are deprecated in favor of ref and refWithDb. Note that ``id`` references are not compatible with the discriminators. - cascade - Cascade Option diff --git a/docs/en/reference/reference-mapping.rst b/docs/en/reference/reference-mapping.rst index dadfb93cd0..c3846ae04c 100644 --- a/docs/en/reference/reference-mapping.rst +++ b/docs/en/reference/reference-mapping.rst @@ -360,6 +360,7 @@ The ``storeAs`` option has three possible values: - **dbRefWithDb**: Uses a `DBRef`_ with ``$ref``, ``$id``, and ``$db`` fields (this is the default) - **dbRef**: Uses a `DBRef`_ with ``$ref`` and ``$id`` +- **ref**: Uses a custom embedded object with an ``id`` field - **id**: Uses a ``MongoId`` .. note:: diff --git a/doctrine-mongo-mapping.xsd b/doctrine-mongo-mapping.xsd index 1abd97dffc..a4b109c764 100644 --- a/doctrine-mongo-mapping.xsd +++ b/doctrine-mongo-mapping.xsd @@ -124,6 +124,7 @@ + diff --git a/lib/Doctrine/ODM/MongoDB/Aggregation/Stage/Lookup.php b/lib/Doctrine/ODM/MongoDB/Aggregation/Stage/Lookup.php index d603315933..49bc7d76d1 100644 --- a/lib/Doctrine/ODM/MongoDB/Aggregation/Stage/Lookup.php +++ b/lib/Doctrine/ODM/MongoDB/Aggregation/Stage/Lookup.php @@ -96,26 +96,44 @@ private function fromReference($fieldName) parent::from($targetMapping->getCollection()); if ($referenceMapping['isOwningSide']) { - if ($referenceMapping['storeAs'] !== ClassMetadataInfo::REFERENCE_STORE_AS_ID) { - throw MappingException::cannotLookupNonIdReference($this->class->name, $fieldName); + switch ($referenceMapping['storeAs']) { + case ClassMetadataInfo::REFERENCE_STORE_AS_ID: + $referencedFieldName = $referenceMapping['name']; + break; + + case ClassMetadataInfo::REFERENCE_STORE_AS_REF: + $referencedFieldName = $referenceMapping['name'] . '.id'; + break; + + default: + throw MappingException::cannotLookupNonIdReference($this->class->name, $fieldName); } $this ->foreignField('_id') - ->localField($referenceMapping['name']); + ->localField($referencedFieldName); } else { if (isset($referenceMapping['repositoryMethod'])) { throw MappingException::repositoryMethodLookupNotAllowed($this->class->name, $fieldName); } $mappedByMapping = $targetMapping->getFieldMapping($referenceMapping['mappedBy']); - if ($mappedByMapping['storeAs'] !== ClassMetadataInfo::REFERENCE_STORE_AS_ID) { - throw MappingException::cannotLookupNonIdReference($this->class->name, $fieldName); + switch ($mappedByMapping['storeAs']) { + case ClassMetadataInfo::REFERENCE_STORE_AS_ID: + $referencedFieldName = $mappedByMapping['name']; + break; + + case ClassMetadataInfo::REFERENCE_STORE_AS_REF: + $referencedFieldName = $mappedByMapping['name'] . '.id'; + break; + + default: + throw MappingException::cannotLookupNonIdReference($this->class->name, $fieldName); } $this ->localField('_id') - ->foreignField($mappedByMapping['name']); + ->foreignField($referencedFieldName); } return $this; diff --git a/lib/Doctrine/ODM/MongoDB/DocumentManager.php b/lib/Doctrine/ODM/MongoDB/DocumentManager.php index b1f5f417cb..bbdfd95bde 100644 --- a/lib/Doctrine/ODM/MongoDB/DocumentManager.php +++ b/lib/Doctrine/ODM/MongoDB/DocumentManager.php @@ -691,20 +691,28 @@ public function createDBRef($document, array $referenceMapping = null) ); } - if ($referenceMapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { - if ($class->inheritanceType === ClassMetadataInfo::INHERITANCE_TYPE_SINGLE_COLLECTION) { - throw MappingException::simpleReferenceMustNotTargetDiscriminatedDocument($referenceMapping['targetDocument']); - } - return $class->getDatabaseIdentifierValue($id); - } - - $dbRef = array( - '$ref' => $class->getCollection(), - '$id' => $class->getDatabaseIdentifierValue($id), - ); - - if ($referenceMapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB) { - $dbRef['$db'] = $this->getDocumentDatabase($class->name)->getName(); + switch ($referenceMapping['storeAs']) { + case ClassMetadataInfo::REFERENCE_STORE_AS_ID: + if ($class->inheritanceType === ClassMetadataInfo::INHERITANCE_TYPE_SINGLE_COLLECTION) { + throw MappingException::simpleReferenceMustNotTargetDiscriminatedDocument($referenceMapping['targetDocument']); + } + + return $class->getDatabaseIdentifierValue($id); + break; + + + case ClassMetadataInfo::REFERENCE_STORE_AS_REF: + $dbRef = ['id' => $class->getDatabaseIdentifierValue($id)]; + break; + + default: + $dbRef = [ + '$ref' => $class->getCollection(), + '$id' => $class->getDatabaseIdentifierValue($id), + ]; + if ($referenceMapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB) { + $dbRef['$db'] = $this->getDocumentDatabase($class->name)->getName(); + } } /* If the class has a discriminator (field and value), use it. A child diff --git a/lib/Doctrine/ODM/MongoDB/Hydrator/HydratorFactory.php b/lib/Doctrine/ODM/MongoDB/Hydrator/HydratorFactory.php index cfc4842ee2..1c0e0b96b1 100644 --- a/lib/Doctrine/ODM/MongoDB/Hydrator/HydratorFactory.php +++ b/lib/Doctrine/ODM/MongoDB/Hydrator/HydratorFactory.php @@ -260,7 +260,7 @@ private function generateHydratorClass(ClassMetadata $class, $hydratorClassName, \$mongoId = \$reference; } else { \$className = \$this->unitOfWork->getClassNameForAssociation(\$this->class->fieldMappings['%2\$s'], \$reference); - \$mongoId = \$reference['\$id']; + \$mongoId = \$reference[ClassMetadataInfo::getReferencePrefix(\$this->class->fieldMappings['%2\$s']['storeAs']) . 'id']; } \$targetMetadata = \$this->dm->getClassMetadata(\$className); \$id = \$targetMetadata->getPHPIdentifierValue(\$mongoId); @@ -296,7 +296,7 @@ private function generateHydratorClass(ClassMetadata $class, $hydratorClassName, \$className = \$mapping['targetDocument']; \$targetClass = \$this->dm->getClassMetadata(\$mapping['targetDocument']); \$mappedByMapping = \$targetClass->fieldMappings[\$mapping['mappedBy']]; - \$mappedByFieldName = isset(\$mappedByMapping['storeAs']) && \$mappedByMapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID ? \$mapping['mappedBy'] : \$mapping['mappedBy'].'.\$id'; + \$mappedByFieldName = isset(\$mappedByMapping['storeAs']) && \$mappedByMapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID ? \$mapping['mappedBy'] : \$mapping['mappedBy'].'.'.ClassMetadataInfo::getReferencePrefix(\$mappedByMapping['storeAs']).'id'; \$criteria = array_merge( array(\$mappedByFieldName => \$data['_id']), isset(\$this->class->fieldMappings['%2\$s']['criteria']) ? \$this->class->fieldMappings['%2\$s']['criteria'] : array() diff --git a/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadataInfo.php index f8f43b85c5..8da57aa163 100644 --- a/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadataInfo.php @@ -103,6 +103,7 @@ class ClassMetadataInfo implements \Doctrine\Common\Persistence\Mapping\ClassMet const REFERENCE_STORE_AS_ID = 'id'; const REFERENCE_STORE_AS_DB_REF = 'dbRef'; const REFERENCE_STORE_AS_DB_REF_WITH_DB = 'dbRefWithDb'; + const REFERENCE_STORE_AS_REF = 'ref'; /* The inheritance mapping types */ /** @@ -474,6 +475,29 @@ public function __construct($documentName) $this->rootDocumentName = $documentName; } + /** + * Helper method to get reference id of ref* type references + * @param array $reference + * @param string $storeAs + * @return mixed + * @internal + */ + public static function getReferenceId($reference, $storeAs) + { + return $reference[ClassMetadataInfo::getReferencePrefix($storeAs) . 'id']; + } + + /** + * Returns the reference prefix used for a referene + * @param string $storeAs + * @return string + * @internal + */ + public static function getReferencePrefix($storeAs) + { + return $storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_REF ? '' : '$'; + } + /** * {@inheritDoc} */ diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php index bcb39d839e..1a0c6d6cd4 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php @@ -723,7 +723,7 @@ private function loadReferenceManyCollectionOwningSide(PersistentCollectionInter $mongoId = $reference; } else { $className = $this->uow->getClassNameForAssociation($mapping, $reference); - $mongoId = $reference['$id']; + $mongoId = ClassMetadataInfo::getReferenceId($reference, $mapping['storeAs']); } $id = $this->dm->getClassMetadata($className)->getPHPIdentifierValue($mongoId); @@ -806,7 +806,12 @@ public function createReferenceManyInverseSideQuery(PersistentCollectionInterfac $ownerClass = $this->dm->getClassMetadata(get_class($owner)); $targetClass = $this->dm->getClassMetadata($mapping['targetDocument']); $mappedByMapping = isset($targetClass->fieldMappings[$mapping['mappedBy']]) ? $targetClass->fieldMappings[$mapping['mappedBy']] : array(); - $mappedByFieldName = isset($mappedByMapping['storeAs']) && $mappedByMapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID ? $mapping['mappedBy'] : $mapping['mappedBy'] . '.$id'; + if (isset($mappedByMapping['storeAs']) && $mappedByMapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { + $mappedByFieldName = $mapping['mappedBy']; + } else { + $mappedByFieldName = $mapping['mappedBy'] . '.' . ClassMetadataInfo::getReferencePrefix(isset($mappedByMapping['storeAs']) ? $mappedByMapping['storeAs'] : null) . 'id'; + } + $criteria = $this->cm->merge( array($mappedByFieldName => $ownerClass->getIdentifierObject($owner)), $this->dm->getFilterCollection()->getFilterCriteria($targetClass), @@ -1178,7 +1183,7 @@ private function prepareQueryElement($fieldName, $value = null, $class = null, $ // Prepare DBRef identifiers or the mapped field's property path $fieldName = ($objectPropertyIsId && ! empty($mapping['reference']) && $mapping['storeAs'] !== ClassMetadataInfo::REFERENCE_STORE_AS_ID) - ? $e[0] . '.$id' + ? $e[0] . '.' . ClassMetadataInfo::getReferencePrefix($mapping['storeAs']) . 'id' : $e[0] . '.' . $objectPropertyPrefix . $targetMapping['name']; // Process targetDocument identifier fields @@ -1425,20 +1430,25 @@ private function getWriteOptions(array $options = array()) private function prepareDbRefElement($fieldName, $value, array $mapping, $inNewObj) { $dbRef = $this->dm->createDBRef($value, $mapping); - if ($inNewObj) { + if ($inNewObj || $mapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { return [[$fieldName, $dbRef]]; } - $keys = ['$ref' => true, '$id' => true, '$db' => true]; - if ($mapping['storeAs'] !== ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB) { - unset($keys['$db']); - } - if (isset($mapping['targetDocument'])) { - unset($keys['$ref'], $keys['$db']); + + if ($mapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_REF) { + $keys = ['id' => true]; + } else { + $keys = ['$ref' => true, '$id' => true, '$db' => true]; + + if ($mapping['storeAs'] !== ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB) { + unset($keys['$db']); + } + + if (isset($mapping['targetDocument'])) { + unset($keys['$ref'], $keys['$db']); + } } - if ($mapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { - return [[$fieldName, $dbRef]]; - } elseif ($mapping['type'] === 'many') { + if ($mapping['type'] === 'many') { return [[$fieldName, ['$elemMatch' => array_intersect_key($dbRef, $keys)]]]; } else { return array_map( diff --git a/lib/Doctrine/ODM/MongoDB/Query/Expr.php b/lib/Doctrine/ODM/MongoDB/Query/Expr.php index 7c4830973e..c966a7b9f8 100644 --- a/lib/Doctrine/ODM/MongoDB/Query/Expr.php +++ b/lib/Doctrine/ODM/MongoDB/Query/Expr.php @@ -79,18 +79,22 @@ public function references($document) if ($storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { $this->query[$mapping['name']] = $dbRef; } else { - $keys = array('ref' => true, 'id' => true, 'db' => true); + if ($storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_REF) { + $keys = ['id' => true]; + } else { + $keys = ['$ref' => true, '$id' => true, '$db' => true]; - if ($storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF) { - unset($keys['db']); - } + if ($storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF) { + unset($keys['$db']); + } - if (isset($mapping['targetDocument'])) { - unset($keys['ref'], $keys['db']); + if (isset($mapping['targetDocument'])) { + unset($keys['$ref'], $keys['$db']); + } } foreach ($keys as $key => $value) { - $this->query[$mapping['name'] . '.$' . $key] = $dbRef['$' . $key]; + $this->query[$mapping['name'] . '.' . $key] = $dbRef[$key]; } } } else { @@ -117,18 +121,22 @@ public function includesReferenceTo($document) if ($storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { $this->query[$mapping['name']] = $dbRef; } else { - $keys = array('ref' => true, 'id' => true, 'db' => true); + if ($storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_REF) { + $keys = ['id' => true]; + } else { + $keys = ['$ref' => true, '$id' => true, '$db' => true]; - if ($storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF) { - unset($keys['db']); - } + if ($storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF) { + unset($keys['$db']); + } - if (isset($mapping['targetDocument'])) { - unset($keys['ref'], $keys['db']); + if (isset($mapping['targetDocument'])) { + unset($keys['$ref'], $keys['$db']); + } } foreach ($keys as $key => $value) { - $this->query[$mapping['name']]['$elemMatch']['$' . $key] = $dbRef['$' . $key]; + $this->query[$mapping['name']]['$elemMatch'][$key] = $dbRef[$key]; } } } else { diff --git a/lib/Doctrine/ODM/MongoDB/Query/ReferencePrimer.php b/lib/Doctrine/ODM/MongoDB/Query/ReferencePrimer.php index 2d5529f479..3f0c099343 100644 --- a/lib/Doctrine/ODM/MongoDB/Query/ReferencePrimer.php +++ b/lib/Doctrine/ODM/MongoDB/Query/ReferencePrimer.php @@ -255,6 +255,7 @@ private function addManyReferences(PersistentCollectionInterface $persistentColl { $mapping = $persistentCollection->getMapping(); + if ($mapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { $className = $mapping['targetDocument']; $class = $this->dm->getClassMetadata($className); @@ -264,7 +265,7 @@ private function addManyReferences(PersistentCollectionInterface $persistentColl if ($mapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { $id = $reference; } else { - $id = $reference['$id']; + $id = $reference[ClassMetadataInfo::getReferencePrefix($mapping['storeAs']) . 'id']; $className = $this->uow->getClassNameForAssociation($mapping, $reference); $class = $this->dm->getClassMetadata($className); } diff --git a/lib/Doctrine/ODM/MongoDB/SchemaManager.php b/lib/Doctrine/ODM/MongoDB/SchemaManager.php index dbf4d35301..dfab94205b 100644 --- a/lib/Doctrine/ODM/MongoDB/SchemaManager.php +++ b/lib/Doctrine/ODM/MongoDB/SchemaManager.php @@ -195,7 +195,7 @@ private function doGetDocumentIndexes($documentName, array &$visited) if ($key == $fieldMapping['name']) { $key = $fieldMapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID ? $key - : $key . '.$id'; + : $key . '.' . ClassMetadataInfo::getReferencePrefix($fieldMapping['storeAs']) . 'id'; } $newKeys[$key] = $v; } diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Aggregation/Stage/LookupTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Aggregation/Stage/LookupTest.php index ade3200cce..a90ca2b1ac 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Aggregation/Stage/LookupTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Aggregation/Stage/LookupTest.php @@ -133,6 +133,42 @@ public function testLookupStageReferenceMany() $this->assertSame('malarzm', $result[1]['users'][0]['username']); } + public function testLookupStageReferenceManyStoreAsRef() + { + $this->requireMongoDB32('$lookup tests require at least MongoDB 3.2.0'); + $this->insertTestData(); + + $builder = $this->dm->createAggregationBuilder(\Documents\ReferenceUser::class); + $builder + ->unwind('$referencedUsers') + ->lookup('referencedUsers') + ->alias('users'); + + $expectedPipeline = [ + [ + '$unwind' => '$referencedUsers', + ], + [ + '$lookup' => [ + 'from' => 'users', + 'localField' => 'referencedUsers.id', + 'foreignField' => '_id', + 'as' => 'users', + ] + ] + ]; + + $this->assertEquals($expectedPipeline, $builder->getPipeline()); + + $result = $builder->execute()->toArray(); + + $this->assertCount(2, $result); + $this->assertCount(1, $result[0]['users']); + $this->assertSame('alcaeus', $result[0]['users'][0]['username']); + $this->assertCount(1, $result[1]['users']); + $this->assertSame('malarzm', $result[1]['users'][0]['username']); + } + public function testLookupStageReferenceManyWithoutUnwindMongoDB32() { $this->requireMongoDB32('$lookup tests require at least MongoDB 3.2.0'); @@ -264,6 +300,76 @@ public function testLookupStageReferenceManyInverse() $this->assertCount(1, $result[0]['simpleReferenceManyInverse']); } + public function testLookupStageReferenceOneInverseStoreAsRef() + { + $this->requireMongoDB32('$lookup tests require at least MongoDB 3.2.0'); + $this->insertTestData(); + + $builder = $this->dm->createAggregationBuilder(\Documents\User::class); + $builder + ->match() + ->field('username') + ->equals('alcaeus') + ->lookup('embeddedReferenceOneInverse') + ->alias('embeddedReferenceOneInverse'); + + $expectedPipeline = [ + [ + '$match' => ['username' => 'alcaeus'] + ], + [ + '$lookup' => [ + 'from' => 'ReferenceUser', + 'localField' => '_id', + 'foreignField' => 'referencedUser.id', + 'as' => 'embeddedReferenceOneInverse', + ] + ] + ]; + + $this->assertEquals($expectedPipeline, $builder->getPipeline()); + + $result = $builder->execute()->toArray(); + + $this->assertCount(1, $result); + $this->assertCount(1, $result[0]['embeddedReferenceOneInverse']); + } + + public function testLookupStageReferenceManyInverseStoreAsRef() + { + $this->requireMongoDB32('$lookup tests require at least MongoDB 3.2.0'); + $this->insertTestData(); + + $builder = $this->dm->createAggregationBuilder(\Documents\User::class); + $builder + ->match() + ->field('username') + ->equals('alcaeus') + ->lookup('embeddedReferenceManyInverse') + ->alias('embeddedReferenceManyInverse'); + + $expectedPipeline = [ + [ + '$match' => ['username' => 'alcaeus'] + ], + [ + '$lookup' => [ + 'from' => 'ReferenceUser', + 'localField' => '_id', + 'foreignField' => 'referencedUsers.id', + 'as' => 'embeddedReferenceManyInverse', + ] + ] + ]; + + $this->assertEquals($expectedPipeline, $builder->getPipeline()); + + $result = $builder->execute()->toArray(); + + $this->assertCount(1, $result); + $this->assertCount(1, $result[0]['embeddedReferenceManyInverse']); + } + private function insertTestData() { $user1 = new \Documents\User(); @@ -281,7 +387,13 @@ private function insertTestData() $simpleReferenceUser->addUser($user1); $simpleReferenceUser->addUser($user2); + $referenceUser = new \Documents\ReferenceUser(); + $referenceUser->setReferencedUser($user1); + $referenceUser->addReferencedUser($user1); + $referenceUser->addReferencedUser($user2); + $this->dm->persist($simpleReferenceUser); + $this->dm->persist($referenceUser); $this->dm->flush(); } } diff --git a/tests/Doctrine/ODM/MongoDB/Tests/DocumentManagerTest.php b/tests/Doctrine/ODM/MongoDB/Tests/DocumentManagerTest.php index 0d9b7d0d2e..820765b4d8 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/DocumentManagerTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/DocumentManagerTest.php @@ -242,6 +242,10 @@ public function testDifferentStoreAsDbReferences() $this->assertArrayHasKey('$ref', $dbRef); $this->assertArrayHasKey('$id', $dbRef); $this->assertArrayHasKey('$db', $dbRef); + + $dbRef = $this->dm->createDBRef($r, $class->associationMappings['ref4']); + $this->assertCount(1, $dbRef); + $this->assertArrayHasKey('id', $dbRef); } private function getMockClassMetadataFactory() @@ -279,4 +283,7 @@ class ReferenceStoreAsDocument /** @ODM\ReferenceOne(targetDocument="Documents\User", storeAs="dbRefWithDb") */ public $ref3; + + /** @ODM\ReferenceOne(targetDocument="Documents\User", storeAs="ref") */ + public $ref4; } diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/DocumentPersisterTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/DocumentPersisterTest.php index bf9321b6e3..c7a130db4c 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Functional/DocumentPersisterTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Functional/DocumentPersisterTest.php @@ -345,6 +345,83 @@ public function testPrepareQueryOrNewObjWithDBRefReferenceToTargetDocumentWithHa $this->assertEquals($expected, $documentPersister->prepareQueryOrNewObj($value)); } + public function testPrepareQueryOrNewObjWithEmbeddedReferenceToTargetDocumentWithNormalIdType() + { + $class = DocumentPersisterTestHashIdDocument::class; + $documentPersister = $this->uow->getDocumentPersister($class); + + $id = new \MongoId(); + + $value = array('embeddedRef.id' => (string) $id); + $expected = array('embeddedRef.id' => $id); + + $this->assertEquals($expected, $documentPersister->prepareQueryOrNewObj($value)); + + $value = array('embeddedRef.id' => array('$exists' => true)); + $expected = array('embeddedRef.id' => array('$exists' => true)); + + $this->assertEquals($expected, $documentPersister->prepareQueryOrNewObj($value)); + + $value = array('embeddedRef.id' => array('$elemMatch' => (string) $id)); + $expected = array('embeddedRef.id' => array('$elemMatch' => $id)); + + $this->assertEquals($expected, $documentPersister->prepareQueryOrNewObj($value)); + + $value = array('embeddedRef.id' => array('$in' => array((string) $id))); + $expected = array('embeddedRef.id' => array('$in' => array($id))); + + $this->assertEquals($expected, $documentPersister->prepareQueryOrNewObj($value)); + + $value = array('embeddedRef.id' => array('$not' => array('$elemMatch' => (string) $id))); + $expected = array('embeddedRef.id' => array('$not' => array('$elemMatch' => $id))); + + $this->assertEquals($expected, $documentPersister->prepareQueryOrNewObj($value)); + + $value = array('embeddedRef.id' => array('$not' => array('$in' => array((string) $id)))); + $expected = array('embeddedRef.id' => array('$not' => array('$in' => array($id)))); + + $this->assertEquals($expected, $documentPersister->prepareQueryOrNewObj($value)); + } + + /** + * @dataProvider provideHashIdentifiers + */ + public function testPrepareQueryOrNewObjWithEmbeddedReferenceToTargetDocumentWithHashIdType($hashId) + { + $class = DocumentPersisterTestDocument::class; + $documentPersister = $this->uow->getDocumentPersister($class); + + $value = array('embeddedRef.id' => $hashId); + $expected = array('embeddedRef.id' => (object) $hashId); + + $this->assertEquals($expected, $documentPersister->prepareQueryOrNewObj($value)); + + $value = array('embeddedRef.id' => array('$exists' => true)); + $expected = array('embeddedRef.id' => array('$exists' => true)); + + $this->assertEquals($expected, $documentPersister->prepareQueryOrNewObj($value)); + + $value = array('embeddedRef.id' => array('$elemMatch' => $hashId)); + $expected = array('embeddedRef.id' => array('$elemMatch' => (object) $hashId)); + + $this->assertEquals($expected, $documentPersister->prepareQueryOrNewObj($value)); + + $value = array('embeddedRef.id' => array('$in' => array($hashId))); + $expected = array('embeddedRef.id' => array('$in' => array((object) $hashId))); + + $this->assertEquals($expected, $documentPersister->prepareQueryOrNewObj($value)); + + $value = array('embeddedRef.id' => array('$not' => array('$elemMatch' => $hashId))); + $expected = array('embeddedRef.id' => array('$not' => array('$elemMatch' => (object) $hashId))); + + $this->assertEquals($expected, $documentPersister->prepareQueryOrNewObj($value)); + + $value = array('embeddedRef.id' => array('$not' => array('$in' => array($hashId)))); + $expected = array('embeddedRef.id' => array('$not' => array('$in' => array((object) $hashId)))); + + $this->assertEquals($expected, $documentPersister->prepareQueryOrNewObj($value)); + } + /** * @return array */ @@ -547,6 +624,9 @@ class DocumentPersisterTestDocument /** @ODM\ReferenceOne(targetDocument="DocumentPersisterTestHashIdDocument", storeAs="dbRefWithDb") */ public $complexRef; + + /** @ODM\ReferenceOne(targetDocument="DocumentPersisterTestHashIdDocument", storeAs="ref") */ + public $embeddedRef; } /** @ODM\Document */ @@ -628,6 +708,9 @@ class DocumentPersisterTestHashIdDocument /** @ODM\ReferenceOne(targetDocument="DocumentPersisterTestDocument", storeAs="dbRefWithDb") */ public $complexRef; + + /** @ODM\ReferenceOne(targetDocument="DocumentPersisterTestDocument", storeAs="ref") */ + public $embeddedRef; } /** @ODM\Document(writeConcern="majority") */ diff --git a/tests/Documents/ReferenceUser.php b/tests/Documents/ReferenceUser.php index 51542bd6bd..088bd96eeb 100644 --- a/tests/Documents/ReferenceUser.php +++ b/tests/Documents/ReferenceUser.php @@ -58,6 +58,20 @@ class ReferenceUser */ public $otherUsers = array(); + /** + * @ODM\ReferenceOne(targetDocument="Documents\User", storeAs="ref") + * + * @var User + */ + public $referencedUser; + + /** + * @ODM\ReferenceMany(targetDocument="Documents\User", storeAs="ref") + * + * @var User[] + */ + public $referencedUsers = array(); + /** * @ODM\Field(type="string") * @@ -161,6 +175,38 @@ public function getOtherUsers() return $this->otherUsers; } + /** + * @param User $referencedUser + */ + public function setReferencedUser(User $referencedUser) + { + $this->referencedUser = $referencedUser; + } + + /** + * @return User + */ + public function getreferencedUser() + { + return $this->referencedUser; + } + + /** + * @param User $referencedUser + */ + public function addReferencedUser(User $referencedUser) + { + $this->referencedUsers[] = $referencedUser; + } + + /** + * @return User[] + */ + public function getReferencedUsers() + { + return $this->referencedUsers; + } + /** * @param string $name */ diff --git a/tests/Documents/User.php b/tests/Documents/User.php index 1e7d0c4972..b421fecbb2 100644 --- a/tests/Documents/User.php +++ b/tests/Documents/User.php @@ -80,6 +80,12 @@ class User extends BaseDocument /** @ODM\ReferenceMany(targetDocument="Documents\SimpleReferenceUser", mappedBy="users") */ protected $simpleReferenceManyInverse; + /** @ODM\ReferenceOne(targetDocument="Documents\ReferenceUser", mappedBy="referencedUser") */ + protected $embeddedReferenceOneInverse; + + /** @ODM\ReferenceMany(targetDocument="Documents\ReferenceUser", mappedBy="referencedUsers") */ + protected $embeddedReferenceManyInverse; + /** @ODM\Field(type="collection") */ private $logs = array(); From 23b2c93b35867f4ec2d90c7381b094f52c27f5f0 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Sun, 24 Sep 2017 13:54:54 +0200 Subject: [PATCH 2/6] Update documentation --- docs/en/reference/annotations-reference.rst | 25 ++++++++++++--------- docs/en/reference/reference-mapping.rst | 4 ++-- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/docs/en/reference/annotations-reference.rst b/docs/en/reference/annotations-reference.rst index c13789955a..bbaefc7235 100644 --- a/docs/en/reference/annotations-reference.rst +++ b/docs/en/reference/annotations-reference.rst @@ -1218,25 +1218,26 @@ documents. Optional attributes: - - targetDocument - A |FQCN| of the target document. + targetDocument - A |FQCN| of the target document. A ``targetDocument`` is + required when using ``storeAs: id``. - simple - deprecated (use ``storeAs: id``) - storeAs - Indicates how to store the reference. ``id`` stores the identifier, ``ref`` an embedded object containing the ``id`` field and (optionally) a - discriminator. ``dbRef`` and ``dbRefWithDb`` store a ``DBRef`` object. They + discriminator. ``dbRef`` and ``dbRefWithDb`` store a `DBRef`_ object. They are deprecated in favor of ``ref``. Note that ``id`` references are not compatible with the discriminators. - cascade - Cascade Option - discriminatorField - The field name to store the discriminator value within - the `DBRef`_ object. + the reference object. - discriminatorMap - Map of discriminator values to class names. - defaultDiscriminatorValue - A default value for discriminatorField if no value - has been set in the embedded document. + has been set in the referenced document. - inversedBy - The field name of the inverse side. Only allowed on owning side. - @@ -1293,24 +1294,26 @@ Defines an instance variable holds a related document instance. Optional attributes: - - targetDocument - A |FQCN| of the target document. + targetDocument - A |FQCN| of the target document. A ``targetDocument`` is + required when using ``storeAs: id``. - simple - deprecated (use ``storeAs: id``) - - storeAs - Indicates how to store the reference. ``id`` uses ``MongoId``, - ``ref`` stores fields ``id`` and ``ref`` (similar to DBRef without $ prefix) value and ``refWithDb`` stores - a additionally db as parameter. ``dbRef`` and ``dbRefWithDb`` use ``DBRef``, they are deprecated in favor of ref and refWithDb. Note that ``id`` - references are not compatible with the discriminators. + storeAs - Indicates how to store the reference. ``id`` stores the identifier, + ``ref`` an embedded object containing the ``id`` field and (optionally) a + discriminator. ``dbRef`` and ``dbRefWithDb`` store a `DBRef`_ object. They + are deprecated in favor of ``ref``. Note that ``id`` references are not + compatible with the discriminators. - cascade - Cascade Option - discriminatorField - The field name to store the discriminator value within - the `DBRef`_ object. + the reference object. - discriminatorMap - Map of discriminator values to class names. - defaultDiscriminatorValue - A default value for discriminatorField if no value - has been set in the embedded document. + has been set in the referenced document. - inversedBy - The field name of the inverse side. Only allowed on owning side. - diff --git a/docs/en/reference/reference-mapping.rst b/docs/en/reference/reference-mapping.rst index c3846ae04c..285309aca2 100644 --- a/docs/en/reference/reference-mapping.rst +++ b/docs/en/reference/reference-mapping.rst @@ -356,12 +356,12 @@ fields and as ``MongoId``, it is possible to save references as `DBRef`_ without the ``$db`` field. This solves problems when the database name changes (and also reduces the amount of storage used). -The ``storeAs`` option has three possible values: +The ``storeAs`` option has the following possible values: - **dbRefWithDb**: Uses a `DBRef`_ with ``$ref``, ``$id``, and ``$db`` fields (this is the default) - **dbRef**: Uses a `DBRef`_ with ``$ref`` and ``$id`` - **ref**: Uses a custom embedded object with an ``id`` field -- **id**: Uses a ``MongoId`` +- **id**: Uses the identifier of the referenced object .. note:: From 5a8b85c8a3d601f4bd451a1e386a2ad970162a87 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Sun, 24 Sep 2017 13:54:59 +0200 Subject: [PATCH 3/6] Introduce createReference method in DocumentManager --- lib/Doctrine/ODM/MongoDB/DocumentManager.php | 36 +++++++++++++++---- .../MongoDB/Persisters/DocumentPersister.php | 2 +- .../MongoDB/Persisters/PersistenceBuilder.php | 2 +- lib/Doctrine/ODM/MongoDB/Query/Expr.php | 12 ++++--- .../ODM/MongoDB/Tests/DocumentManagerTest.php | 17 ++++----- .../Tests/Functional/ReferencePrimerTest.php | 3 +- .../ODM/MongoDB/Tests/Query/BuilderTest.php | 4 +-- .../ODM/MongoDB/Tests/Query/ExprTest.php | 6 ++-- 8 files changed, 55 insertions(+), 27 deletions(-) diff --git a/lib/Doctrine/ODM/MongoDB/DocumentManager.php b/lib/Doctrine/ODM/MongoDB/DocumentManager.php index bbdfd95bde..9a8a55caaf 100644 --- a/lib/Doctrine/ODM/MongoDB/DocumentManager.php +++ b/lib/Doctrine/ODM/MongoDB/DocumentManager.php @@ -668,15 +668,15 @@ public function getConfiguration() } /** - * Returns a DBRef array for the supplied document. + * Returns a reference to the supplied document. * - * @param mixed $document A document object + * @param object $document A document object * @param array $referenceMapping Mapping for the field that references the document * * @throws \InvalidArgumentException - * @return array A DBRef array + * @return mixed The reference for the document in question, according to the desired mapping */ - public function createDBRef($document, array $referenceMapping = null) + public function createReference($document, array $referenceMapping) { if ( ! is_object($document)) { throw new \InvalidArgumentException('Cannot create a DBRef, the document is not an object'); @@ -691,7 +691,8 @@ public function createDBRef($document, array $referenceMapping = null) ); } - switch ($referenceMapping['storeAs']) { + $storeAs = isset($referenceMapping['storeAs']) ? $referenceMapping['storeAs'] : null; + switch ($storeAs) { case ClassMetadataInfo::REFERENCE_STORE_AS_ID: if ($class->inheritanceType === ClassMetadataInfo::INHERITANCE_TYPE_SINGLE_COLLECTION) { throw MappingException::simpleReferenceMustNotTargetDiscriminatedDocument($referenceMapping['targetDocument']); @@ -710,7 +711,7 @@ public function createDBRef($document, array $referenceMapping = null) '$ref' => $class->getCollection(), '$id' => $class->getDatabaseIdentifierValue($id), ]; - if ($referenceMapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB) { + if ($storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB) { $dbRef['$db'] = $this->getDocumentDatabase($class->name)->getName(); } } @@ -728,7 +729,7 @@ public function createDBRef($document, array $referenceMapping = null) /* Add a discriminator value if the referenced document is not mapped * explicitly to a targetDocument class. */ - if ($referenceMapping !== null && ! isset($referenceMapping['targetDocument'])) { + if (! isset($referenceMapping['targetDocument'])) { $discriminatorField = $referenceMapping['discriminatorField']; $discriminatorValue = isset($referenceMapping['discriminatorMap']) ? array_search($class->name, $referenceMapping['discriminatorMap']) @@ -750,6 +751,27 @@ public function createDBRef($document, array $referenceMapping = null) return $dbRef; } + /** + * Returns a DBRef array for the supplied document. + * + * @param mixed $document A document object + * @param array $referenceMapping Mapping for the field that references the document + * + * @throws \InvalidArgumentException + * @return array A DBRef array + * @deprecated Deprecated in favor of createReference; will be removed in 2.0 + */ + public function createDBRef($document, array $referenceMapping = null) + { + @trigger_error('The ' . __METHOD__ . ' method has been deprecated and will be removed in ODM 2.0. Use createReference() instead.', E_USER_DEPRECATED); + + if (!isset($referenceMapping['storeAs'])) { + $referenceMapping['storeAs'] = ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF; + } + + return $this->createReference($document, $referenceMapping); + } + /** * Throws an exception if the DocumentManager is closed or currently not active. * diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php index 1a0c6d6cd4..0e810e54ff 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php @@ -1429,7 +1429,7 @@ private function getWriteOptions(array $options = array()) */ private function prepareDbRefElement($fieldName, $value, array $mapping, $inNewObj) { - $dbRef = $this->dm->createDBRef($value, $mapping); + $dbRef = $this->dm->createReference($value, $mapping); if ($inNewObj || $mapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { return [[$fieldName, $dbRef]]; } diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php b/lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php index 44659b2c1a..caa90be3e9 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php @@ -308,7 +308,7 @@ public function prepareUpsertData($document) */ public function prepareReferencedDocumentValue(array $referenceMapping, $document) { - return $this->dm->createDBRef($document, $referenceMapping); + return $this->dm->createReference($document, $referenceMapping); } /** diff --git a/lib/Doctrine/ODM/MongoDB/Query/Expr.php b/lib/Doctrine/ODM/MongoDB/Query/Expr.php index c966a7b9f8..353b594309 100644 --- a/lib/Doctrine/ODM/MongoDB/Query/Expr.php +++ b/lib/Doctrine/ODM/MongoDB/Query/Expr.php @@ -73,7 +73,7 @@ public function references($document) { if ($this->currentField) { $mapping = $this->getReferenceMapping(); - $dbRef = $this->dm->createDBRef($document, $mapping); + $dbRef = $this->dm->createReference($document, $mapping); $storeAs = array_key_exists('storeAs', $mapping) ? $mapping['storeAs'] : null; if ($storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { @@ -98,7 +98,9 @@ public function references($document) } } } else { - $dbRef = $this->dm->createDBRef($document); + @trigger_error('Calling ' . __METHOD__ . ' without a current field set will no longer be possible in ODM 2.0.', E_USER_DEPRECATED); + + $dbRef = $this->dm->createReference($document); $this->query = $dbRef; } @@ -115,7 +117,7 @@ public function includesReferenceTo($document) { if ($this->currentField) { $mapping = $this->getReferenceMapping(); - $dbRef = $this->dm->createDBRef($document, $mapping); + $dbRef = $this->dm->createReference($document, $mapping); $storeAs = array_key_exists('storeAs', $mapping) ? $mapping['storeAs'] : null; if ($storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { @@ -140,7 +142,9 @@ public function includesReferenceTo($document) } } } else { - $dbRef = $this->dm->createDBRef($document); + @trigger_error('Calling ' . __METHOD__ . ' without a current field set will no longer be possible in ODM 2.0.', E_USER_DEPRECATED); + + $dbRef = $this->dm->createReference($document); $this->query['$elemMatch'] = $dbRef; } diff --git a/tests/Doctrine/ODM/MongoDB/Tests/DocumentManagerTest.php b/tests/Doctrine/ODM/MongoDB/Tests/DocumentManagerTest.php index 820765b4d8..5b3a3934fa 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/DocumentManagerTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/DocumentManagerTest.php @@ -5,6 +5,7 @@ use Doctrine\ODM\MongoDB\Mapping\ClassMetadataInfo; use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM; use Doctrine\ODM\MongoDB\Tests\Mocks\DocumentManagerMock; +use Documents\CmsPhonenumber; class DocumentManagerTest extends \Doctrine\ODM\MongoDB\Tests\BaseTest { @@ -195,16 +196,16 @@ public function testGetDocumentCollectionAppliesClassMetadataSlaveOkay() public function testCannotCreateDbRefWithoutId() { $d = new \Documents\User(); - $this->dm->createDBRef($d); + $this->dm->createReference($d, ['storeAs' => ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF]); } public function testCreateDbRefWithNonNullEmptyId() { - $phonenumber = new \Documents\CmsPhonenumber(); + $phonenumber = new CmsPhonenumber(); $phonenumber->phonenumber = 0; $this->dm->persist($phonenumber); - $dbRef = $this->dm->createDBRef($phonenumber); + $dbRef = $this->dm->createReference($phonenumber, ['storeAs' => ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF, 'targetDocument' => CmsPhonenumber::class]); $this->assertSame(array('$ref' => 'CmsPhonenumber', '$id' => 0), $dbRef); } @@ -219,7 +220,7 @@ public function testDisriminatedSimpleReferenceFails() $r = new \Documents\Tournament\ParticipantSolo('Maciej'); $this->dm->persist($r); $class = $this->dm->getClassMetadata(get_class($d)); - $this->dm->createDBRef($r, $class->associationMappings['ref']); + $this->dm->createReference($r, $class->associationMappings['ref']); } public function testDifferentStoreAsDbReferences() @@ -229,21 +230,21 @@ public function testDifferentStoreAsDbReferences() $d = new ReferenceStoreAsDocument(); $class = $this->dm->getClassMetadata(get_class($d)); - $dbRef = $this->dm->createDBRef($r, $class->associationMappings['ref1']); + $dbRef = $this->dm->createReference($r, $class->associationMappings['ref1']); $this->assertInstanceOf('MongoId', $dbRef); - $dbRef = $this->dm->createDBRef($r, $class->associationMappings['ref2']); + $dbRef = $this->dm->createReference($r, $class->associationMappings['ref2']); $this->assertCount(2, $dbRef); $this->assertArrayHasKey('$ref', $dbRef); $this->assertArrayHasKey('$id', $dbRef); - $dbRef = $this->dm->createDBRef($r, $class->associationMappings['ref3']); + $dbRef = $this->dm->createReference($r, $class->associationMappings['ref3']); $this->assertCount(3, $dbRef); $this->assertArrayHasKey('$ref', $dbRef); $this->assertArrayHasKey('$id', $dbRef); $this->assertArrayHasKey('$db', $dbRef); - $dbRef = $this->dm->createDBRef($r, $class->associationMappings['ref4']); + $dbRef = $this->dm->createReference($r, $class->associationMappings['ref4']); $this->assertCount(1, $dbRef); $this->assertArrayHasKey('id', $dbRef); } diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/ReferencePrimerTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/ReferencePrimerTest.php index fc3c736dfd..1161501889 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Functional/ReferencePrimerTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Functional/ReferencePrimerTest.php @@ -4,6 +4,7 @@ use Doctrine\ODM\MongoDB\DocumentManager; use Doctrine\ODM\MongoDB\Mapping\ClassMetadata; +use Doctrine\ODM\MongoDB\Mapping\ClassMetadataInfo; use Doctrine\ODM\MongoDB\Proxy\Proxy; use Doctrine\ODM\MongoDB\Query\Query; use Documents\Account; @@ -446,7 +447,7 @@ public function testPrimeReferencesInFindAndModifyResult() $this->dm->persist($group); $this->dm->flush(); - $groupDBRef = $this->dm->createDBRef($group); + $groupDBRef = $this->dm->createReference($group, ['storeAs' => ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF, 'targetDocument' => Group::class]); $this->dm->clear(); diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Query/BuilderTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Query/BuilderTest.php index 6b4495ea09..61e6d88f87 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Query/BuilderTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Query/BuilderTest.php @@ -149,7 +149,7 @@ public function testArrayUpdateOperatorsOnReferenceMany($class, $field) ->field($field)->addToSet($f) ->getQuery()->debug(); - $expected = $this->dm->createDBRef($f, $this->dm->getClassMetadata($class)->fieldMappings[$field]); + $expected = $this->dm->createReference($f, $this->dm->getClassMetadata($class)->fieldMappings[$field]); $this->assertEquals($expected, $q1['newObj']['$addToSet'][$field]); } @@ -173,7 +173,7 @@ public function testArrayUpdateOperatorsOnReferenceOne($class, $field) ->field($field)->set($f) ->getQuery()->debug(); - $expected = $this->dm->createDBRef($f, $this->dm->getClassMetadata($class)->fieldMappings[$field]); + $expected = $this->dm->createReference($f, $this->dm->getClassMetadata($class)->fieldMappings[$field]); $this->assertEquals($expected, $q1['newObj']['$set'][$field]); } diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Query/ExprTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Query/ExprTest.php index 355ee81a4a..cc6672dfa2 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Query/ExprTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Query/ExprTest.php @@ -192,7 +192,7 @@ public function testReferencesUsesMinimalKeys() $expected = array('foo.$id' => '1234'); $dm->expects($this->once()) - ->method('createDBRef') + ->method('createReference') ->will($this->returnValue(array('$ref' => 'coll', '$id' => '1234', '$db' => 'db'))); $dm->expects($this->once()) ->method('getUnitOfWork') @@ -225,7 +225,7 @@ public function testReferencesUsesAllKeys() $expected = array('foo.$ref' => 'coll', 'foo.$id' => '1234', 'foo.$db' => 'db'); $dm->expects($this->once()) - ->method('createDBRef') + ->method('createReference') ->will($this->returnValue(array('$ref' => 'coll', '$id' => '1234', '$db' => 'db'))); $dm->expects($this->once()) ->method('getUnitOfWork') @@ -258,7 +258,7 @@ public function testReferencesUsesSomeKeys() $expected = array('foo.$ref' => 'coll', 'foo.$id' => '1234'); $dm->expects($this->once()) - ->method('createDBRef') + ->method('createReference') ->will($this->returnValue(array('$ref' => 'coll', '$id' => '1234'))); $dm->expects($this->once()) ->method('getUnitOfWork') From 20845529fc020a42a221c9e0c29f59982ff31517 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Sun, 24 Sep 2017 14:25:35 +0200 Subject: [PATCH 4/6] Add method to generate the reference field name This avoids repeatedly checking for the type of reference and duplicating logic. It also allows us to make getReferencePrefix private. --- .../ODM/MongoDB/Aggregation/Stage/Lookup.php | 10 ++----- .../ODM/MongoDB/Hydrator/HydratorFactory.php | 11 +++----- .../ODM/MongoDB/Mapping/ClassMetadataInfo.php | 27 ++++++++++++++++--- .../MongoDB/Persisters/DocumentPersister.php | 21 +++++---------- .../ODM/MongoDB/Query/ReferencePrimer.php | 8 +++--- lib/Doctrine/ODM/MongoDB/SchemaManager.php | 4 +-- lib/Doctrine/ODM/MongoDB/UnitOfWork.php | 2 +- 7 files changed, 39 insertions(+), 44 deletions(-) diff --git a/lib/Doctrine/ODM/MongoDB/Aggregation/Stage/Lookup.php b/lib/Doctrine/ODM/MongoDB/Aggregation/Stage/Lookup.php index 49bc7d76d1..552a3cf406 100644 --- a/lib/Doctrine/ODM/MongoDB/Aggregation/Stage/Lookup.php +++ b/lib/Doctrine/ODM/MongoDB/Aggregation/Stage/Lookup.php @@ -98,11 +98,8 @@ private function fromReference($fieldName) if ($referenceMapping['isOwningSide']) { switch ($referenceMapping['storeAs']) { case ClassMetadataInfo::REFERENCE_STORE_AS_ID: - $referencedFieldName = $referenceMapping['name']; - break; - case ClassMetadataInfo::REFERENCE_STORE_AS_REF: - $referencedFieldName = $referenceMapping['name'] . '.id'; + $referencedFieldName = ClassMetadataInfo::getReferenceFieldName($referenceMapping['storeAs'], $referenceMapping['name']); break; default: @@ -120,11 +117,8 @@ private function fromReference($fieldName) $mappedByMapping = $targetMapping->getFieldMapping($referenceMapping['mappedBy']); switch ($mappedByMapping['storeAs']) { case ClassMetadataInfo::REFERENCE_STORE_AS_ID: - $referencedFieldName = $mappedByMapping['name']; - break; - case ClassMetadataInfo::REFERENCE_STORE_AS_REF: - $referencedFieldName = $mappedByMapping['name'] . '.id'; + $referencedFieldName = ClassMetadataInfo::getReferenceFieldName($mappedByMapping['storeAs'], $mappedByMapping['name']); break; default: diff --git a/lib/Doctrine/ODM/MongoDB/Hydrator/HydratorFactory.php b/lib/Doctrine/ODM/MongoDB/Hydrator/HydratorFactory.php index 1c0e0b96b1..c8f9e6a1ea 100644 --- a/lib/Doctrine/ODM/MongoDB/Hydrator/HydratorFactory.php +++ b/lib/Doctrine/ODM/MongoDB/Hydrator/HydratorFactory.php @@ -255,13 +255,8 @@ private function generateHydratorClass(ClassMetadata $class, $hydratorClassName, /** @ReferenceOne */ if (isset(\$data['%1\$s'])) { \$reference = \$data['%1\$s']; - if (isset(\$this->class->fieldMappings['%2\$s']['storeAs']) && \$this->class->fieldMappings['%2\$s']['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { - \$className = \$this->class->fieldMappings['%2\$s']['targetDocument']; - \$mongoId = \$reference; - } else { - \$className = \$this->unitOfWork->getClassNameForAssociation(\$this->class->fieldMappings['%2\$s'], \$reference); - \$mongoId = \$reference[ClassMetadataInfo::getReferencePrefix(\$this->class->fieldMappings['%2\$s']['storeAs']) . 'id']; - } + \$className = \$this->unitOfWork->getClassNameForAssociation(\$this->class->fieldMappings['%2\$s'], \$reference); + \$mongoId = ClassMetadataInfo::getReferenceId(\$reference, \$this->class->fieldMappings['%2\$s']['storeAs']); \$targetMetadata = \$this->dm->getClassMetadata(\$className); \$id = \$targetMetadata->getPHPIdentifierValue(\$mongoId); \$return = \$this->dm->getReference(\$className, \$id); @@ -296,7 +291,7 @@ private function generateHydratorClass(ClassMetadata $class, $hydratorClassName, \$className = \$mapping['targetDocument']; \$targetClass = \$this->dm->getClassMetadata(\$mapping['targetDocument']); \$mappedByMapping = \$targetClass->fieldMappings[\$mapping['mappedBy']]; - \$mappedByFieldName = isset(\$mappedByMapping['storeAs']) && \$mappedByMapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID ? \$mapping['mappedBy'] : \$mapping['mappedBy'].'.'.ClassMetadataInfo::getReferencePrefix(\$mappedByMapping['storeAs']).'id'; + \$mappedByFieldName = ClassMetadataInfo::getReferenceFieldName(\$mappedByMapping['storeAs'], \$mapping['mappedBy']); \$criteria = array_merge( array(\$mappedByFieldName => \$data['_id']), isset(\$this->class->fieldMappings['%2\$s']['criteria']) ? \$this->class->fieldMappings['%2\$s']['criteria'] : array() diff --git a/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadataInfo.php index 8da57aa163..751704fa55 100644 --- a/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadataInfo.php @@ -484,20 +484,39 @@ public function __construct($documentName) */ public static function getReferenceId($reference, $storeAs) { - return $reference[ClassMetadataInfo::getReferencePrefix($storeAs) . 'id']; + return $storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_ID ? $reference : $reference[ClassMetadataInfo::getReferencePrefix($storeAs) . 'id']; } /** - * Returns the reference prefix used for a referene + * Returns the reference prefix used for a reference * @param string $storeAs * @return string - * @internal */ - public static function getReferencePrefix($storeAs) + private static function getReferencePrefix($storeAs) { + if (!in_array($storeAs, [ClassMetadataInfo::REFERENCE_STORE_AS_REF, ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF, ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB])) { + throw new \LogicException('Can only get a reference prefix for DBRef and reference arrays'); + } + return $storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_REF ? '' : '$'; } + /** + * Returns a fully qualified field name for a given reference + * @param string $storeAs + * @param string $pathPrefix The field path prefix + * @return string + * @internal + */ + public static function getReferenceFieldName($storeAs, $pathPrefix = '') + { + if ($storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { + return $pathPrefix; + } + + return ($pathPrefix ? $pathPrefix . '.' : '') . static::getReferencePrefix($storeAs) . 'id'; + } + /** * {@inheritDoc} */ diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php index 0e810e54ff..7708ce58e9 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php @@ -718,13 +718,8 @@ private function loadReferenceManyCollectionOwningSide(PersistentCollectionInter $sorted = isset($mapping['sort']) && $mapping['sort']; foreach ($collection->getMongoData() as $key => $reference) { - if (isset($mapping['storeAs']) && $mapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { - $className = $mapping['targetDocument']; - $mongoId = $reference; - } else { - $className = $this->uow->getClassNameForAssociation($mapping, $reference); - $mongoId = ClassMetadataInfo::getReferenceId($reference, $mapping['storeAs']); - } + $className = $this->uow->getClassNameForAssociation($mapping, $reference); + $mongoId = ClassMetadataInfo::getReferenceId($reference, $mapping['storeAs']); $id = $this->dm->getClassMetadata($className)->getPHPIdentifierValue($mongoId); // create a reference to the class and id @@ -806,11 +801,7 @@ public function createReferenceManyInverseSideQuery(PersistentCollectionInterfac $ownerClass = $this->dm->getClassMetadata(get_class($owner)); $targetClass = $this->dm->getClassMetadata($mapping['targetDocument']); $mappedByMapping = isset($targetClass->fieldMappings[$mapping['mappedBy']]) ? $targetClass->fieldMappings[$mapping['mappedBy']] : array(); - if (isset($mappedByMapping['storeAs']) && $mappedByMapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { - $mappedByFieldName = $mapping['mappedBy']; - } else { - $mappedByFieldName = $mapping['mappedBy'] . '.' . ClassMetadataInfo::getReferencePrefix(isset($mappedByMapping['storeAs']) ? $mappedByMapping['storeAs'] : null) . 'id'; - } + $mappedByFieldName = ClassMetadataInfo::getReferenceFieldName(isset($mappedByMapping['storeAs']) ? $mappedByMapping['storeAs'] : ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF, $mapping['mappedBy']); $criteria = $this->cm->merge( array($mappedByFieldName => $ownerClass->getIdentifierObject($owner)), @@ -1061,7 +1052,7 @@ private function prepareQueryElement($fieldName, $value = null, $class = null, $ if (! empty($mapping['reference']) && is_object($value) && ! ($value instanceof \MongoId)) { try { - return $this->prepareDbRefElement($fieldName, $value, $mapping, $inNewObj); + return $this->prepareReference($fieldName, $value, $mapping, $inNewObj); } catch (MappingException $e) { // do nothing in case passed object is not mapped document } @@ -1183,7 +1174,7 @@ private function prepareQueryElement($fieldName, $value = null, $class = null, $ // Prepare DBRef identifiers or the mapped field's property path $fieldName = ($objectPropertyIsId && ! empty($mapping['reference']) && $mapping['storeAs'] !== ClassMetadataInfo::REFERENCE_STORE_AS_ID) - ? $e[0] . '.' . ClassMetadataInfo::getReferencePrefix($mapping['storeAs']) . 'id' + ? ClassMetadataInfo::getReferenceFieldName($mapping['storeAs'], $e[0]) : $e[0] . '.' . $objectPropertyPrefix . $targetMapping['name']; // Process targetDocument identifier fields @@ -1427,7 +1418,7 @@ private function getWriteOptions(array $options = array()) * @param bool $inNewObj * @return array */ - private function prepareDbRefElement($fieldName, $value, array $mapping, $inNewObj) + private function prepareReference($fieldName, $value, array $mapping, $inNewObj) { $dbRef = $this->dm->createReference($value, $mapping); if ($inNewObj || $mapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { diff --git a/lib/Doctrine/ODM/MongoDB/Query/ReferencePrimer.php b/lib/Doctrine/ODM/MongoDB/Query/ReferencePrimer.php index 3f0c099343..4db943d925 100644 --- a/lib/Doctrine/ODM/MongoDB/Query/ReferencePrimer.php +++ b/lib/Doctrine/ODM/MongoDB/Query/ReferencePrimer.php @@ -255,17 +255,15 @@ private function addManyReferences(PersistentCollectionInterface $persistentColl { $mapping = $persistentCollection->getMapping(); - if ($mapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { $className = $mapping['targetDocument']; $class = $this->dm->getClassMetadata($className); } foreach ($persistentCollection->getMongoData() as $reference) { - if ($mapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { - $id = $reference; - } else { - $id = $reference[ClassMetadataInfo::getReferencePrefix($mapping['storeAs']) . 'id']; + $id = ClassMetadataInfo::getReferenceId($reference, $mapping['storeAs']); + + if ($mapping['storeAs'] !== ClassMetadataInfo::REFERENCE_STORE_AS_ID) { $className = $this->uow->getClassNameForAssociation($mapping, $reference); $class = $this->dm->getClassMetadata($className); } diff --git a/lib/Doctrine/ODM/MongoDB/SchemaManager.php b/lib/Doctrine/ODM/MongoDB/SchemaManager.php index dfab94205b..b2b12965b2 100644 --- a/lib/Doctrine/ODM/MongoDB/SchemaManager.php +++ b/lib/Doctrine/ODM/MongoDB/SchemaManager.php @@ -193,9 +193,7 @@ private function doGetDocumentIndexes($documentName, array &$visited) $newKeys = array(); foreach ($index['keys'] as $key => $v) { if ($key == $fieldMapping['name']) { - $key = $fieldMapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID - ? $key - : $key . '.' . ClassMetadataInfo::getReferencePrefix($fieldMapping['storeAs']) . 'id'; + $key = ClassMetadataInfo::getReferenceFieldName($fieldMapping['storeAs'], $key); } $newKeys[$key] = $v; } diff --git a/lib/Doctrine/ODM/MongoDB/UnitOfWork.php b/lib/Doctrine/ODM/MongoDB/UnitOfWork.php index 8669822170..6e1625faff 100644 --- a/lib/Doctrine/ODM/MongoDB/UnitOfWork.php +++ b/lib/Doctrine/ODM/MongoDB/UnitOfWork.php @@ -2625,7 +2625,7 @@ public function getClassNameForAssociation(array $mapping, $data) : $discriminatorValue; } - $class = $this->dm->getClassMetadata($mapping['targetDocument']); + $class = $this->dm->getClassMetadata($mapping['targetDocument']); if (isset($class->discriminatorField, $data[$class->discriminatorField])) { $discriminatorValue = $data[$class->discriminatorField]; From 802d6abe6152c5d63fcd748528d680a7528f8c89 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Sun, 24 Sep 2017 14:30:15 +0200 Subject: [PATCH 5/6] Move server version check to setup method --- .../Tests/Aggregation/Stage/LookupTest.php | 35 ++++--------------- 1 file changed, 7 insertions(+), 28 deletions(-) diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Aggregation/Stage/LookupTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Aggregation/Stage/LookupTest.php index a90ca2b1ac..66ed68f968 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Aggregation/Stage/LookupTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Aggregation/Stage/LookupTest.php @@ -4,11 +4,17 @@ class LookupTest extends \Doctrine\ODM\MongoDB\Tests\BaseTest { - public function testLookupStage() + /** + * @before + */ + public function prepareTest() { $this->requireMongoDB32('$lookup tests require at least MongoDB 3.2.0'); $this->insertTestData(); + } + public function testLookupStage() + { $builder = $this->dm->createAggregationBuilder(\Documents\SimpleReferenceUser::class); $builder ->lookup('user') @@ -36,9 +42,6 @@ public function testLookupStage() public function testLookupStageWithClassName() { - $this->requireMongoDB32('$lookup tests require at least MongoDB 3.2.0'); - $this->insertTestData(); - $builder = $this->dm->createAggregationBuilder(\Documents\SimpleReferenceUser::class); $builder ->lookup(\Documents\User::class) @@ -68,9 +71,6 @@ public function testLookupStageWithClassName() public function testLookupStageWithCollectionName() { - $this->requireMongoDB32('$lookup tests require at least MongoDB 3.2.0'); - $this->insertTestData(); - $builder = $this->dm->createAggregationBuilder(\Documents\SimpleReferenceUser::class); $builder ->lookup('randomCollectionName') @@ -99,9 +99,6 @@ public function testLookupStageWithCollectionName() public function testLookupStageReferenceMany() { - $this->requireMongoDB32('$lookup tests require at least MongoDB 3.2.0'); - $this->insertTestData(); - $builder = $this->dm->createAggregationBuilder(\Documents\SimpleReferenceUser::class); $builder ->unwind('$users') @@ -135,9 +132,6 @@ public function testLookupStageReferenceMany() public function testLookupStageReferenceManyStoreAsRef() { - $this->requireMongoDB32('$lookup tests require at least MongoDB 3.2.0'); - $this->insertTestData(); - $builder = $this->dm->createAggregationBuilder(\Documents\ReferenceUser::class); $builder ->unwind('$referencedUsers') @@ -171,9 +165,7 @@ public function testLookupStageReferenceManyStoreAsRef() public function testLookupStageReferenceManyWithoutUnwindMongoDB32() { - $this->requireMongoDB32('$lookup tests require at least MongoDB 3.2.0'); $this->skipOnMongoDB34('$lookup tests without unwind will not work on MongoDB 3.4.0'); - $this->insertTestData(); $builder = $this->dm->createAggregationBuilder(\Documents\SimpleReferenceUser::class); $builder @@ -202,7 +194,6 @@ public function testLookupStageReferenceManyWithoutUnwindMongoDB32() public function testLookupStageReferenceManyWithoutUnwindMongoDB34() { $this->requireMongoDB34('$lookup tests with unwind require at least MongoDB 3.4.0'); - $this->insertTestData(); $builder = $this->dm->createAggregationBuilder(\Documents\SimpleReferenceUser::class); $builder @@ -232,9 +223,6 @@ public function testLookupStageReferenceManyWithoutUnwindMongoDB34() public function testLookupStageReferenceOneInverse() { - $this->requireMongoDB32('$lookup tests require at least MongoDB 3.2.0'); - $this->insertTestData(); - $builder = $this->dm->createAggregationBuilder(\Documents\User::class); $builder ->match() @@ -267,9 +255,6 @@ public function testLookupStageReferenceOneInverse() public function testLookupStageReferenceManyInverse() { - $this->requireMongoDB32('$lookup tests require at least MongoDB 3.2.0'); - $this->insertTestData(); - $builder = $this->dm->createAggregationBuilder(\Documents\User::class); $builder ->match() @@ -302,9 +287,6 @@ public function testLookupStageReferenceManyInverse() public function testLookupStageReferenceOneInverseStoreAsRef() { - $this->requireMongoDB32('$lookup tests require at least MongoDB 3.2.0'); - $this->insertTestData(); - $builder = $this->dm->createAggregationBuilder(\Documents\User::class); $builder ->match() @@ -337,9 +319,6 @@ public function testLookupStageReferenceOneInverseStoreAsRef() public function testLookupStageReferenceManyInverseStoreAsRef() { - $this->requireMongoDB32('$lookup tests require at least MongoDB 3.2.0'); - $this->insertTestData(); - $builder = $this->dm->createAggregationBuilder(\Documents\User::class); $builder ->match() From 0ac42f6c033e422a97d0e28c0f6fb2a5fc9460fd Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Wed, 27 Sep 2017 07:02:29 +0200 Subject: [PATCH 6/6] Apply review feedback --- docs/en/reference/annotations-reference.rst | 4 +- lib/Doctrine/ODM/MongoDB/DocumentManager.php | 27 +++++--- .../ODM/MongoDB/Mapping/ClassMetadataInfo.php | 2 +- .../MongoDB/Persisters/DocumentPersister.php | 38 ++++++----- lib/Doctrine/ODM/MongoDB/Query/Expr.php | 64 ++++++++++++------- .../ODM/MongoDB/Tests/Query/ExprTest.php | 4 +- 6 files changed, 86 insertions(+), 53 deletions(-) diff --git a/docs/en/reference/annotations-reference.rst b/docs/en/reference/annotations-reference.rst index bbaefc7235..9db3efae6d 100644 --- a/docs/en/reference/annotations-reference.rst +++ b/docs/en/reference/annotations-reference.rst @@ -1225,7 +1225,7 @@ Optional attributes: - storeAs - Indicates how to store the reference. ``id`` stores the identifier, ``ref`` an embedded object containing the ``id`` field and (optionally) a - discriminator. ``dbRef`` and ``dbRefWithDb`` store a `DBRef`_ object. They + discriminator. ``dbRef`` and ``dbRefWithDb`` store a `DBRef`_ object and are deprecated in favor of ``ref``. Note that ``id`` references are not compatible with the discriminators. - @@ -1301,7 +1301,7 @@ Optional attributes: - storeAs - Indicates how to store the reference. ``id`` stores the identifier, ``ref`` an embedded object containing the ``id`` field and (optionally) a - discriminator. ``dbRef`` and ``dbRefWithDb`` store a `DBRef`_ object. They + discriminator. ``dbRef`` and ``dbRefWithDb`` store a `DBRef`_ object and are deprecated in favor of ``ref``. Note that ``id`` references are not compatible with the discriminators. - diff --git a/lib/Doctrine/ODM/MongoDB/DocumentManager.php b/lib/Doctrine/ODM/MongoDB/DocumentManager.php index 9a8a55caaf..da07e1a64b 100644 --- a/lib/Doctrine/ODM/MongoDB/DocumentManager.php +++ b/lib/Doctrine/ODM/MongoDB/DocumentManager.php @@ -703,17 +703,26 @@ public function createReference($document, array $referenceMapping) case ClassMetadataInfo::REFERENCE_STORE_AS_REF: - $dbRef = ['id' => $class->getDatabaseIdentifierValue($id)]; + $reference = ['id' => $class->getDatabaseIdentifierValue($id)]; break; - default: - $dbRef = [ + case ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF: + $reference = [ '$ref' => $class->getCollection(), '$id' => $class->getDatabaseIdentifierValue($id), ]; - if ($storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB) { - $dbRef['$db'] = $this->getDocumentDatabase($class->name)->getName(); - } + break; + + case ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB: + $reference = [ + '$ref' => $class->getCollection(), + '$id' => $class->getDatabaseIdentifierValue($id), + '$db' => $this->getDocumentDatabase($class->name)->getName(), + ]; + break; + + default: + throw new \InvalidArgumentException("Reference type {$storeAs} is invalid."); } /* If the class has a discriminator (field and value), use it. A child @@ -721,7 +730,7 @@ public function createReference($document, array $referenceMapping) * discriminator field and no value, so default to the full class name. */ if (isset($class->discriminatorField)) { - $dbRef[$class->discriminatorField] = isset($class->discriminatorValue) + $reference[$class->discriminatorField] = isset($class->discriminatorValue) ? $class->discriminatorValue : $class->name; } @@ -745,10 +754,10 @@ public function createReference($document, array $referenceMapping) $discriminatorValue = $class->name; } - $dbRef[$discriminatorField] = $discriminatorValue; + $reference[$discriminatorField] = $discriminatorValue; } - return $dbRef; + return $reference; } /** diff --git a/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadataInfo.php index 751704fa55..060c2cb08b 100644 --- a/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadataInfo.php @@ -477,7 +477,7 @@ public function __construct($documentName) /** * Helper method to get reference id of ref* type references - * @param array $reference + * @param mixed $reference * @param string $storeAs * @return mixed * @internal diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php index 7708ce58e9..752274db71 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php @@ -1420,31 +1420,39 @@ private function getWriteOptions(array $options = array()) */ private function prepareReference($fieldName, $value, array $mapping, $inNewObj) { - $dbRef = $this->dm->createReference($value, $mapping); + $reference = $this->dm->createReference($value, $mapping); if ($inNewObj || $mapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { - return [[$fieldName, $dbRef]]; + return [[$fieldName, $reference]]; } - if ($mapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_REF) { - $keys = ['id' => true]; - } else { - $keys = ['$ref' => true, '$id' => true, '$db' => true]; + switch ($mapping['storeAs']) { + case ClassMetadataInfo::REFERENCE_STORE_AS_REF: + $keys = ['id' => true]; + break; - if ($mapping['storeAs'] !== ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB) { - unset($keys['$db']); - } + case ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF: + case ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB: + $keys = ['$ref' => true, '$id' => true, '$db' => true]; - if (isset($mapping['targetDocument'])) { - unset($keys['$ref'], $keys['$db']); - } + if ($mapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF) { + unset($keys['$db']); + } + + if (isset($mapping['targetDocument'])) { + unset($keys['$ref'], $keys['$db']); + } + break; + + default: + throw new \InvalidArgumentException("Reference type {$mapping['storeAs']} is invalid."); } if ($mapping['type'] === 'many') { - return [[$fieldName, ['$elemMatch' => array_intersect_key($dbRef, $keys)]]]; + return [[$fieldName, ['$elemMatch' => array_intersect_key($reference, $keys)]]]; } else { return array_map( - function ($key) use ($dbRef, $fieldName) { - return [$fieldName . '.' . $key, $dbRef[$key]]; + function ($key) use ($reference, $fieldName) { + return [$fieldName . '.' . $key, $reference[$key]]; }, array_keys($keys) ); diff --git a/lib/Doctrine/ODM/MongoDB/Query/Expr.php b/lib/Doctrine/ODM/MongoDB/Query/Expr.php index 353b594309..1bed27c6db 100644 --- a/lib/Doctrine/ODM/MongoDB/Query/Expr.php +++ b/lib/Doctrine/ODM/MongoDB/Query/Expr.php @@ -73,15 +73,21 @@ public function references($document) { if ($this->currentField) { $mapping = $this->getReferenceMapping(); - $dbRef = $this->dm->createReference($document, $mapping); + $reference = $this->dm->createReference($document, $mapping); $storeAs = array_key_exists('storeAs', $mapping) ? $mapping['storeAs'] : null; - if ($storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { - $this->query[$mapping['name']] = $dbRef; - } else { - if ($storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_REF) { + switch ($storeAs) { + case ClassMetadataInfo::REFERENCE_STORE_AS_ID: + $this->query[$mapping['name']] = $reference; + return $this; + break; + + case ClassMetadataInfo::REFERENCE_STORE_AS_REF: $keys = ['id' => true]; - } else { + break; + + case ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF: + case ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB: $keys = ['$ref' => true, '$id' => true, '$db' => true]; if ($storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF) { @@ -91,17 +97,19 @@ public function references($document) if (isset($mapping['targetDocument'])) { unset($keys['$ref'], $keys['$db']); } - } + break; - foreach ($keys as $key => $value) { - $this->query[$mapping['name'] . '.' . $key] = $dbRef[$key]; - } + default: + throw new \InvalidArgumentException("Reference type {$storeAs} is invalid."); + } + + foreach ($keys as $key => $value) { + $this->query[$mapping['name'] . '.' . $key] = $reference[$key]; } } else { @trigger_error('Calling ' . __METHOD__ . ' without a current field set will no longer be possible in ODM 2.0.', E_USER_DEPRECATED); - $dbRef = $this->dm->createReference($document); - $this->query = $dbRef; + $this->query = $this->dm->createDBRef($document); } return $this; @@ -117,15 +125,21 @@ public function includesReferenceTo($document) { if ($this->currentField) { $mapping = $this->getReferenceMapping(); - $dbRef = $this->dm->createReference($document, $mapping); + $reference = $this->dm->createReference($document, $mapping); $storeAs = array_key_exists('storeAs', $mapping) ? $mapping['storeAs'] : null; - if ($storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { - $this->query[$mapping['name']] = $dbRef; - } else { - if ($storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_REF) { + switch ($storeAs) { + case ClassMetadataInfo::REFERENCE_STORE_AS_ID: + $this->query[$mapping['name']] = $reference; + return $this; + break; + + case ClassMetadataInfo::REFERENCE_STORE_AS_REF: $keys = ['id' => true]; - } else { + break; + + case ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF: + case ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB: $keys = ['$ref' => true, '$id' => true, '$db' => true]; if ($storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF) { @@ -135,17 +149,19 @@ public function includesReferenceTo($document) if (isset($mapping['targetDocument'])) { unset($keys['$ref'], $keys['$db']); } - } + break; - foreach ($keys as $key => $value) { - $this->query[$mapping['name']]['$elemMatch'][$key] = $dbRef[$key]; - } + default: + throw new \InvalidArgumentException("Reference type {$storeAs} is invalid."); + } + + foreach ($keys as $key => $value) { + $this->query[$mapping['name']]['$elemMatch'][$key] = $reference[$key]; } } else { @trigger_error('Calling ' . __METHOD__ . ' without a current field set will no longer be possible in ODM 2.0.', E_USER_DEPRECATED); - $dbRef = $this->dm->createReference($document); - $this->query['$elemMatch'] = $dbRef; + $this->query['$elemMatch'] = $this->dm->createDBRef($document); } return $this; diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Query/ExprTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Query/ExprTest.php index cc6672dfa2..348c230b32 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Query/ExprTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Query/ExprTest.php @@ -206,7 +206,7 @@ public function testReferencesUsesMinimalKeys() ->will($this->returnValue($expected)); $class->expects($this->once()) ->method('getFieldMapping') - ->will($this->returnValue(array('targetDocument' => 'Foo', 'name' => 'foo'))); + ->will($this->returnValue(array('targetDocument' => 'Foo', 'name' => 'foo', 'storeAs' => ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB))); $expr = new Expr($dm); $expr->setClassMetadata($class); @@ -239,7 +239,7 @@ public function testReferencesUsesAllKeys() ->will($this->returnValue($expected)); $class->expects($this->once()) ->method('getFieldMapping') - ->will($this->returnValue(array('name' => 'foo'))); + ->will($this->returnValue(array('name' => 'foo', 'storeAs' => ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB))); $expr = new Expr($dm); $expr->setClassMetadata($class);