From 4a6b7355b20b23b7815ee77595cbd9260b4d09b3 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Thu, 18 Sep 2025 22:19:07 +0200 Subject: [PATCH 1/7] Support dot syntax when preparing nested query values --- .../MongoDB/Persisters/DocumentPersister.php | 40 ++++++++------- .../ODM/MongoDB/Tests/Query/BuilderTest.php | 49 +++++++++++++++++++ 2 files changed, 70 insertions(+), 19 deletions(-) diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php index 2d81620021..14d75284cd 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php @@ -1068,7 +1068,6 @@ public function prepareQueryOrNewObj(array $query, bool $isNewObj = false): arra $preparedQueryElements = $this->prepareQueryElement($key, $value, null, true, $isNewObj); foreach ($preparedQueryElements as [$preparedKey, $preparedValue]) { - $preparedValue = $this->convertToDatabaseValue($key, $preparedValue); $preparedQuery[$preparedKey] = $preparedValue; } } @@ -1083,21 +1082,23 @@ public function prepareQueryOrNewObj(array $query, bool $isNewObj = false): arra * * @return mixed */ - private function convertToDatabaseValue(string $fieldName, $value) + private function convertToDatabaseValue(string $fieldName, $value, ?ClassMetadata $class = null) { + $class ??= $this->class; + if (is_array($value)) { foreach ($value as $k => $v) { if ($k === '$exists' || $k === '$type' || $k === '$currentDate') { continue; } - $value[$k] = $this->convertToDatabaseValue($fieldName, $v); + $value[$k] = $this->convertToDatabaseValue($fieldName, $v, $class); } return $value; } - if (! $this->class->hasField($fieldName)) { + if (! $class->hasField($fieldName)) { if ($value instanceof BackedEnum) { $value = $value->value; } @@ -1105,7 +1106,7 @@ private function convertToDatabaseValue(string $fieldName, $value) return Type::convertPHPToDatabaseValue($value); } - $mapping = $this->class->fieldMappings[$fieldName]; + $mapping = $class->fieldMappings[$fieldName]; $typeName = $mapping['type']; if (! empty($mapping['reference']) || ! empty($mapping['embedded'])) { @@ -1143,15 +1144,15 @@ private function convertToDatabaseValue(string $fieldName, $value) * * @return array */ - private function prepareQueryElement(string $fieldName, $value = null, ?ClassMetadata $class = null, bool $prepareValue = true, bool $inNewObj = false): array + private function prepareQueryElement(string $originalFieldName, $value = null, ?ClassMetadata $class = null, bool $prepareValue = true, bool $inNewObj = false): array { $class ??= $this->class; // @todo Consider inlining calls to ClassMetadata methods // Process all non-identifier fields by translating field names - if ($class->hasField($fieldName) && ! $class->isIdentifier($fieldName)) { - $mapping = $class->fieldMappings[$fieldName]; + if ($class->hasField($originalFieldName) && ! $class->isIdentifier($originalFieldName)) { + $mapping = $class->fieldMappings[$originalFieldName]; $fieldName = $mapping['name']; if (! $prepareValue) { @@ -1176,7 +1177,7 @@ private function prepareQueryElement(string $fieldName, $value = null, ?ClassMet // No further preparation unless we're dealing with a simple reference if (empty($mapping['reference']) || $mapping['storeAs'] !== ClassMetadata::REFERENCE_STORE_AS_ID || empty((array) $value)) { - return [[$fieldName, $value]]; + return [[$fieldName, $this->convertToDatabaseValue($originalFieldName, $value, $class)]]; } // Additional preparation for one or more simple reference values @@ -1195,7 +1196,7 @@ private function prepareQueryElement(string $fieldName, $value = null, ?ClassMet } // Process identifier fields - if (($class->hasField($fieldName) && $class->isIdentifier($fieldName)) || $fieldName === '_id') { + if (($class->hasField($originalFieldName) && $class->isIdentifier($originalFieldName)) || $originalFieldName === '_id') { $fieldName = '_id'; if (! $prepareValue) { @@ -1215,8 +1216,8 @@ private function prepareQueryElement(string $fieldName, $value = null, ?ClassMet } // No processing for unmapped, non-identifier, non-dotted field names - if (strpos($fieldName, '.') === false) { - return [[$fieldName, $value]]; + if (strpos($originalFieldName, '.') === false) { + return [[$originalFieldName, $prepareValue ? $this->convertToDatabaseValue($originalFieldName, $value, $class) : $value]]; } /* Process "fieldName.objectProperty" queries (on arrays or objects). @@ -1225,11 +1226,11 @@ private function prepareQueryElement(string $fieldName, $value = null, ?ClassMet * significant: "fieldName.objectProperty" with an optional index or key * for collections stored as either BSON arrays or objects. */ - $e = explode('.', $fieldName, 4); + $e = explode('.', $originalFieldName, 4); // No further processing for unmapped fields if (! isset($class->fieldMappings[$e[0]])) { - return [[$fieldName, $value]]; + return [[$originalFieldName, $prepareValue ? $this->convertToDatabaseValue($e[0], $value, $class) : $value]]; } $mapping = $class->fieldMappings[$e[0]]; @@ -1246,6 +1247,7 @@ private function prepareQueryElement(string $fieldName, $value = null, ?ClassMet $mapping['type'] === ClassMetadata::MANY && CollectionHelper::isHash($mapping['strategy']) && isset($e[2]) ) { + $fieldName = $originalFieldName; $objectProperty = $e[2]; $objectPropertyPrefix = $e[1] . '.'; $nextObjectProperty = implode('.', array_slice($e, 3)); @@ -1262,7 +1264,7 @@ private function prepareQueryElement(string $fieldName, $value = null, ?ClassMet } else { $fieldName = $e[0] . '.' . $e[1]; - return [[$fieldName, $value]]; + return [[$fieldName, $prepareValue ? $this->convertToDatabaseValue($e[0], $value, $class) : $value]]; } // No further processing for fields without a targetDocument mapping @@ -1271,7 +1273,7 @@ private function prepareQueryElement(string $fieldName, $value = null, ?ClassMet $fieldName .= '.' . $nextObjectProperty; } - return [[$fieldName, $value]]; + return [[$fieldName, $prepareValue ? $this->convertToDatabaseValue($e[0], $value, $class) : $value]]; } $targetClass = $this->dm->getClassMetadata($mapping['targetDocument']); @@ -1282,7 +1284,7 @@ private function prepareQueryElement(string $fieldName, $value = null, ?ClassMet $fieldName .= '.' . $nextObjectProperty; } - return [[$fieldName, $value]]; + return [[$fieldName, $prepareValue ? $this->convertToDatabaseValue($objectProperty, $value, $targetClass) : $value]]; } $targetMapping = $targetClass->getFieldMapping($objectProperty); @@ -1329,7 +1331,7 @@ private function prepareQueryElement(string $fieldName, $value = null, ?ClassMet $nextObjectProperty = '$' . $nextObjectProperty; } - $fieldNames = [[$nextObjectProperty, $value]]; + $fieldNames = [[$nextObjectProperty, $prepareValue ? $this->convertToDatabaseValue($nextObjectProperty, $value, $nextTargetClass) : $value]]; } return array_map(static function ($preparedTuple) use ($fieldName) { @@ -1339,7 +1341,7 @@ private function prepareQueryElement(string $fieldName, $value = null, ?ClassMet }, $fieldNames); } - return [[$fieldName, $value]]; + return [[$fieldName, $this->convertToDatabaseValue($objectProperty, $value, $targetClass)]]; } /** diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Query/BuilderTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Query/BuilderTest.php index f3915f7993..5efcdbbd33 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Query/BuilderTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Query/BuilderTest.php @@ -837,6 +837,35 @@ public function testNonRewindable(): void self::assertInstanceOf(UnrewindableIterator::class, $query->execute()); } + public function testQueryPreparesNestedValues(): void + { + $objectId = new ObjectId(); + $oidString = (string) $objectId; + + $embedded = new EmbeddedForNestedFieldPreparation(); + $embedded->objectId = $oidString; + + $builder = new Builder($this->dm, DocumentForNestedFieldPreparation::class); + $builder + ->updateOne() + ->field('embedded.embedded.objectId') + ->equals($oidString) + ->field('embedded.embedded.embedded') + ->set($embedded); + + $query = $builder->getQuery()->getQuery(); + $filter = $query['query']; + $newObj = $query['newObj']; + + self::assertArrayHasKey('embedded.embedded.test', $filter); + self::assertInstanceOf(ObjectId::class, $filter['embedded.embedded.test']); + + self::assertArrayHasKey('embedded.embedded.embedded', $newObj['$set']); + self::assertIsArray($newObj['$set']['embedded.embedded.embedded']); + self::assertArrayHasKey('test', $newObj['$set']['embedded.embedded.embedded']); + self::assertInstanceOf(ObjectId::class, $newObj['$set']['embedded.embedded.embedded']['test']); + } + private function getTestQueryBuilder(): Builder { return new Builder($this->dm, User::class); @@ -922,3 +951,23 @@ class ChildC extends ParentClass #[ODM\ReferenceMany(storeAs: 'dbRef')] public $featurePartialMany; } + +#[ODM\Document] +class DocumentForNestedFieldPreparation +{ + #[ODM\Id] + public string $id; + + #[ODM\EmbedOne(targetDocument: EmbeddedForNestedFieldPreparation::class)] + public EmbeddedForNestedFieldPreparation $embedded; +} + +#[ODM\EmbeddedDocument] +class EmbeddedForNestedFieldPreparation +{ + #[ODM\Field(name: 'test', type: Type::OBJECTID)] + public string $objectId; + + #[ODM\EmbedOne(targetDocument: self::class)] + public ?EmbeddedForNestedFieldPreparation $embedded = null; +} From d04139059e972197bd51bb0f41ea306dbc13d489 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Tue, 23 Sep 2025 14:41:21 +0200 Subject: [PATCH 2/7] Refactor query element preparation for more consistency --- .../MongoDB/Persisters/DocumentPersister.php | 242 ++++++++---------- .../Functional/DocumentPersisterTest.php | 8 +- .../Tests/Functional/Ticket/GH2825Test.php | 127 +++++++++ 3 files changed, 242 insertions(+), 135 deletions(-) create mode 100644 tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH2825Test.php diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php index 14d75284cd..fe78d852d2 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php @@ -62,6 +62,7 @@ use function is_string; use function spl_object_id; use function sprintf; +use function str_contains; use function strpos; use function strtolower; use function trigger_deprecation; @@ -1041,7 +1042,7 @@ public function addFilterToPreparedQuery(array $preparedQuery): array * * PHP field names and types will be converted to those used by MongoDB. * - * @param array $query + * @param array $query * * @return array */ @@ -1049,24 +1050,28 @@ public function prepareQueryOrNewObj(array $query, bool $isNewObj = false): arra { $preparedQuery = []; - foreach ($query as $key => $value) { - $key = (string) $key; + foreach ($query as $field => $value) { + $field = (string) $field; - // Recursively prepare logical query clauses - if (in_array($key, ['$and', '$or', '$nor'], true) && is_array($value)) { - foreach ($value as $k2 => $v2) { - $preparedQuery[$key][$k2] = $this->prepareQueryOrNewObj($v2, $isNewObj); - } + // Recursively prepare logical query clauses, treating each value as a separate query element + if (in_array($field, ['$and', '$or', '$nor'], true) && is_array($value)) { + $preparedQuery[$field] = array_map( + fn ($v) => $this->prepareQueryOrNewObj($v, $isNewObj), + $value, + ); continue; } - if (isset($key[0]) && $key[0] === '$' && is_array($value)) { - $preparedQuery[$key] = $this->prepareQueryOrNewObj($value, $isNewObj); + // Recursively prepare nested operators, treating the value as a single query element + if (isset($field[0]) && $field[0] === '$' && is_array($value)) { + $preparedQuery[$field] = $this->prepareQueryOrNewObj($value, $isNewObj); + continue; } - $preparedQueryElements = $this->prepareQueryElement($key, $value, null, true, $isNewObj); + // Prepare a single query element. This may produce multiple queries (e.g. for references) + $preparedQueryElements = $this->prepareQueryElement($field, $value, null, true, $isNewObj); foreach ($preparedQueryElements as [$preparedKey, $preparedValue]) { $preparedQuery[$preparedKey] = $preparedValue; } @@ -1084,8 +1089,6 @@ public function prepareQueryOrNewObj(array $query, bool $isNewObj = false): arra */ private function convertToDatabaseValue(string $fieldName, $value, ?ClassMetadata $class = null) { - $class ??= $this->class; - if (is_array($value)) { foreach ($value as $k => $v) { if ($k === '$exists' || $k === '$type' || $k === '$currentDate') { @@ -1098,7 +1101,7 @@ private function convertToDatabaseValue(string $fieldName, $value, ?ClassMetadat return $value; } - if (! $class->hasField($fieldName)) { + if (! $class || ! $class->hasField($fieldName)) { if ($value instanceof BackedEnum) { $value = $value->value; } @@ -1133,6 +1136,20 @@ private function convertToDatabaseValue(string $fieldName, $value, ?ClassMetadat return $value; } + private function prepareQueryReference(mixed $value, ClassMetadata $class): mixed + { + if (! is_array($value)) { + return $class->getDatabaseIdentifierValue($value); + } + + // Objects without operators or with DBRef fields can be converted immediately + if (! $this->hasQueryOperators($value) || $this->hasDBRefFields($value)) { + return $class->getDatabaseIdentifierValue($value); + } + + return $this->prepareQueryExpression($value, $class); + } + /** * Prepares a query value and converts the PHP value to the database value * if it is an identifier. @@ -1144,16 +1161,22 @@ private function convertToDatabaseValue(string $fieldName, $value, ?ClassMetadat * * @return array */ - private function prepareQueryElement(string $originalFieldName, $value = null, ?ClassMetadata $class = null, bool $prepareValue = true, bool $inNewObj = false): array + private function prepareQueryElement(string $originalFieldName, $value = null, ?ClassMetadata $class = null, bool $prepareValue = true, bool $inNewObj = false, string $fieldNamePrefix = ''): array { - $class ??= $this->class; + $class ??= $this->class; + $fieldName = $fieldNamePrefix . $originalFieldName; + + // Process identifier fields + if (($class->hasField($originalFieldName) && $class->isIdentifier($originalFieldName)) || $originalFieldName === '_id') { + $fieldName = $fieldNamePrefix . '_id'; - // @todo Consider inlining calls to ClassMetadata methods + return [[$fieldName, $prepareValue ? $this->prepareQueryReference($value, $class) : $value]]; + } // Process all non-identifier fields by translating field names - if ($class->hasField($originalFieldName) && ! $class->isIdentifier($originalFieldName)) { + if ($class->hasField($originalFieldName)) { $mapping = $class->fieldMappings[$originalFieldName]; - $fieldName = $mapping['name']; + $fieldName = $fieldNamePrefix . $mapping['name']; if (! $prepareValue) { return [[$fieldName, $value]]; @@ -1195,29 +1218,9 @@ private function prepareQueryElement(string $originalFieldName, $value = null, ? return [[$fieldName, $this->prepareQueryExpression($value, $targetClass)]]; } - // Process identifier fields - if (($class->hasField($originalFieldName) && $class->isIdentifier($originalFieldName)) || $originalFieldName === '_id') { - $fieldName = '_id'; - - if (! $prepareValue) { - return [[$fieldName, $value]]; - } - - if (! is_array($value)) { - return [[$fieldName, $class->getDatabaseIdentifierValue($value)]]; - } - - // Objects without operators or with DBRef fields can be converted immediately - if (! $this->hasQueryOperators($value) || $this->hasDBRefFields($value)) { - return [[$fieldName, $class->getDatabaseIdentifierValue($value)]]; - } - - return [[$fieldName, $this->prepareQueryExpression($value, $class)]]; - } - // No processing for unmapped, non-identifier, non-dotted field names - if (strpos($originalFieldName, '.') === false) { - return [[$originalFieldName, $prepareValue ? $this->convertToDatabaseValue($originalFieldName, $value, $class) : $value]]; + if (! str_contains($originalFieldName, '.')) { + return [[$fieldName, $prepareValue ? $this->convertToDatabaseValue($originalFieldName, $value, $class) : $value]]; } /* Process "fieldName.objectProperty" queries (on arrays or objects). @@ -1226,122 +1229,99 @@ private function prepareQueryElement(string $originalFieldName, $value = null, ? * significant: "fieldName.objectProperty" with an optional index or key * for collections stored as either BSON arrays or objects. */ - $e = explode('.', $originalFieldName, 4); + $fieldNameParts = explode('.', $originalFieldName, 4); + $partCount = count($fieldNameParts); + assert($partCount >= 2); // No further processing for unmapped fields - if (! isset($class->fieldMappings[$e[0]])) { - return [[$originalFieldName, $prepareValue ? $this->convertToDatabaseValue($e[0], $value, $class) : $value]]; + if (! $class->hasField($fieldNameParts[0])) { + return [[$fieldName, $prepareValue ? $this->convertToDatabaseValue($fieldNameParts[0], $value, $class) : $value]]; } - $mapping = $class->fieldMappings[$e[0]]; - $e[0] = $mapping['name']; + $mapping = $class->fieldMappings[$fieldNameParts[0]]; + $fieldName = $fieldNamePrefix . $mapping['name'] . '.' . implode('.', array_slice($fieldNameParts, 1)); // Hash and raw fields will not be prepared beyond the field name if ($mapping['type'] === Type::HASH || $mapping['type'] === Type::RAW) { - $fieldName = implode('.', $e); - return [[$fieldName, $value]]; } - if ( - $mapping['type'] === ClassMetadata::MANY && CollectionHelper::isHash($mapping['strategy']) - && isset($e[2]) - ) { - $fieldName = $originalFieldName; - $objectProperty = $e[2]; - $objectPropertyPrefix = $e[1] . '.'; - $nextObjectProperty = implode('.', array_slice($e, 3)); - } elseif ($e[1] !== '$') { - $fieldName = $e[0] . '.' . $e[1]; - $objectProperty = $e[1]; - $objectPropertyPrefix = ''; - $nextObjectProperty = implode('.', array_slice($e, 2)); - } elseif (isset($e[2])) { - $fieldName = $e[0] . '.' . $e[1] . '.' . $e[2]; - $objectProperty = $e[2]; - $objectPropertyPrefix = $e[1] . '.'; - $nextObjectProperty = implode('.', array_slice($e, 3)); + if (isset($mapping['targetDocument'])) { + // For associations with a targetDocument (i.e. embedded or reference), get the class metadata for the target document + $targetClass = $this->dm->getClassMetadata($mapping['targetDocument']); + } elseif (is_object($value) && ! $this->dm->getMetadataFactory()->isTransient($value::class)) { + // For associations without a targetDocument, try to infer the class metadata from the object + $targetClass = $this->dm->getClassMetadata($value::class); } else { - $fieldName = $e[0] . '.' . $e[1]; - - return [[$fieldName, $prepareValue ? $this->convertToDatabaseValue($e[0], $value, $class) : $value]]; - } - - // No further processing for fields without a targetDocument mapping - if (! isset($mapping['targetDocument'])) { - if ($nextObjectProperty) { - $fieldName .= '.' . $nextObjectProperty; + // Without a target document, no further processing is possible + return [[$fieldName, $prepareValue ? $this->convertToDatabaseValue($fieldNameParts[0], $value) : $value]]; + } + + // Don't recurse for references. Instead, prepare the reference directly + if (! empty($mapping['reference'])) { + // First part is the name of the reference + // Second part is either a positional operator, index/key, or the name of a field + // Third part (if any) is the name of a field + // That means, we can implode all field parts except the first as the next field name + if ($fieldNameParts[1] === '$') { + $objectProperty = $fieldNameParts[2]; + $referencePrefix = $fieldNamePrefix . $mapping['name'] . '.$'; + } else { + $objectProperty = $fieldNameParts[1]; + $referencePrefix = $fieldNamePrefix . $mapping['name']; } - return [[$fieldName, $prepareValue ? $this->convertToDatabaseValue($e[0], $value, $class) : $value]]; - } - - $targetClass = $this->dm->getClassMetadata($mapping['targetDocument']); + if ($targetClass->hasField($objectProperty) && $targetClass->isIdentifier($objectProperty)) { + $fieldName = ClassMetadata::getReferenceFieldName($mapping['storeAs'], $referencePrefix); - // No further processing for unmapped targetDocument fields - if (! $targetClass->hasField($objectProperty)) { - if ($nextObjectProperty) { - $fieldName .= '.' . $nextObjectProperty; + return [[$fieldName, $prepareValue ? $this->prepareQueryReference($value, $targetClass) : $value]]; } return [[$fieldName, $prepareValue ? $this->convertToDatabaseValue($objectProperty, $value, $targetClass) : $value]]; } - $targetMapping = $targetClass->getFieldMapping($objectProperty); - $objectPropertyIsId = $targetClass->isIdentifier($objectProperty); - - // Prepare DBRef identifiers or the mapped field's property path - $fieldName = $objectPropertyIsId && ! empty($mapping['reference']) && $mapping['storeAs'] !== ClassMetadata::REFERENCE_STORE_AS_ID - ? ClassMetadata::getReferenceFieldName($mapping['storeAs'], $e[0]) - : $e[0] . '.' . $objectPropertyPrefix . $targetMapping['name']; - - // Process targetDocument identifier fields - if ($objectPropertyIsId) { - if (! $prepareValue) { - return [[$fieldName, $value]]; - } - - if (! is_array($value)) { - return [[$fieldName, $targetClass->getDatabaseIdentifierValue($value)]]; - } - - // Objects without operators or with DBRef fields can be converted immediately - if (! $this->hasQueryOperators($value) || $this->hasDBRefFields($value)) { - return [[$fieldName, $targetClass->getDatabaseIdentifierValue($value)]]; - } - - return [[$fieldName, $this->prepareQueryExpression($value, $targetClass)]]; - } - - /* The property path may include a third field segment, excluding the - * collection item pointer. If present, this next object property must - * be processed recursively. + /* + * 1 element: impossible (because of the dot) + * 2 elements: fieldName.objectProperty, fieldName., or fieldName.$. For EmbedMany and ReferenceMany, treat the second element as index if $inNewObj is true and convert the value. Otherwise, recurse. + * 3+ elements: fieldname.foo.bar, fieldName..foo, or fieldName.$foo. For EmbedMany and ReferenceMany, treat the second element as index, and recurse into the third element. Otherwise, recurse with the second element as field name. */ - if ($nextObjectProperty) { - // Respect the targetDocument's class metadata when recursing - $nextTargetClass = isset($targetMapping['targetDocument']) - ? $this->dm->getClassMetadata($targetMapping['targetDocument']) - : null; - - if (empty($targetMapping['reference'])) { - $fieldNames = $this->prepareQueryElement($nextObjectProperty, $value, $nextTargetClass, $prepareValue); - } else { - // No recursive processing for references as most probably somebody is querying DBRef or alike - if ($nextObjectProperty[0] !== '$' && in_array($targetMapping['storeAs'], [ClassMetadata::REFERENCE_STORE_AS_DB_REF_WITH_DB, ClassMetadata::REFERENCE_STORE_AS_DB_REF])) { - $nextObjectProperty = '$' . $nextObjectProperty; + if ($mapping['type'] === ClassMetadata::MANY) { + if ($inNewObj || CollectionHelper::isHash($mapping['strategy'])) { + // When there are only two segments in a hash or when serialising a new object, we seem to be replacing an entire element. Don't recurse, just convert the value. + if ($partCount === 2) { + $fieldName = $fieldNamePrefix . $mapping['name'] . '.' . $fieldNameParts[1]; + + return [[$fieldName, $prepareValue ? $this->convertToDatabaseValue($fieldNameParts[0], $value, $targetClass) : $value]]; } - $fieldNames = [[$nextObjectProperty, $prepareValue ? $this->convertToDatabaseValue($nextObjectProperty, $value, $nextTargetClass) : $value]]; + // When there are more than two segments, treat the second segment (index/key/positional operator) as part of the field name and recurse into the rest + $newPrefix = $fieldNamePrefix . $mapping['name'] . '.' . $fieldNameParts[1] . '.'; + $newFieldName = implode('.', array_slice($fieldNameParts, 2)); + } else { + // When serialising a query, the second segment is a positional operator ($), a numeric index for collections, or anything else for a hash. + $newPrefix = $fieldNamePrefix . $mapping['name'] . '.'; + $newFieldName = implode('.', array_slice($fieldNameParts, 1)); } - return array_map(static function ($preparedTuple) use ($fieldName) { - [$key, $value] = $preparedTuple; - - return [$fieldName . '.' . $key, $value]; - }, $fieldNames); + return $this->prepareQueryElement( + $newFieldName, + $value, + $targetClass, + $prepareValue, + $inNewObj, + $newPrefix, + ); } - return [[$fieldName, $this->convertToDatabaseValue($objectProperty, $value, $targetClass)]]; + // For everything else, recurse with the first segment as field name and the target document class + return $this->prepareQueryElement( + implode('.', array_slice($fieldNameParts, 1)), + $value, + $targetClass, + $prepareValue, + $inNewObj, + $fieldNamePrefix . $mapping['name'] . '.', + ); } /** diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/DocumentPersisterTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/DocumentPersisterTest.php index 69592e719b..885d63f7d4 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Functional/DocumentPersisterTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Functional/DocumentPersisterTest.php @@ -134,10 +134,10 @@ public static function getTestPrepareFieldNameData(): array ['association.nested', 'associationName.nestedName'], ['association.nested.$id', 'associationName.nestedName.$id'], ['association.nested._id', 'associationName.nestedName._id'], - ['association.nested.id', 'associationName.nestedName._id'], - ['association.nested.association.nested.$id', 'associationName.nestedName.associationName.nestedName.$id'], - ['association.nested.association.nested.id', 'associationName.nestedName.associationName.nestedName._id'], - ['association.nested.association.nested.firstName', 'associationName.nestedName.associationName.nestedName.firstName'], + ['association.nested.id', 'associationName.nestedName.id'], + ['association.nested.association.nested.$id', 'associationName.nestedName.association.nested.$id'], + ['association.nested.association.nested.id', 'associationName.nestedName.association.nested.id'], + ['association.nested.association.nested.firstName', 'associationName.nestedName.association.nested.firstName'], ]; } diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH2825Test.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH2825Test.php new file mode 100644 index 0000000000..9ba85c6954 --- /dev/null +++ b/tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH2825Test.php @@ -0,0 +1,127 @@ +embedded = new GH2825Embedded('level 1'); + + $this->dm->persist($document); + $this->dm->flush(); + + $embedded = new GH2825Embedded('level 2'); + + $this->dm->persist($embedded); + + $this->dm->createQueryBuilder(GH2825Document::class) + ->updateOne() + ->field('id')->equals($document->id) + ->field('embedded.embedded')->set($embedded) + ->getQuery() + ->execute(); + + $result = $this->dm->getDocumentCollection(GH2825Document::class) + ->findOne(['_id' => new ObjectId($document->id)]); + + self::assertSame('level 1', $result['embedded']['renamed']); + self::assertSame('level 2', $result['embedded']['embedded']['renamed']); + } + + public function testQueryBuilderUpdatesReferencesCorrectly(): void + { + $document = new GH2825Document('document'); + $document->embedded = new GH2825Embedded('embedded'); + $reference = new GH2825Document('referenced'); + + $this->dm->persist($document); + $this->dm->persist($reference); + + $this->dm->flush(); + + $this->dm->createQueryBuilder(GH2825Document::class) + ->updateOne() + ->field('id')->equals($document->id) + ->field('referenceStoreAsId')->set($reference) + ->field('referenceStoreAsRef')->set($reference) + ->field('referenceStoreAsDbRef')->set($reference) + ->field('embedded.referenceStoreAsId')->set($reference) + ->field('embedded.referenceStoreAsRef')->set($reference) + ->field('embedded.referenceStoreAsDbRef')->set($reference) + ->getQuery() + ->execute(); + + $result = $this->dm->getDocumentCollection(GH2825Document::class) + ->findOne(['_id' => new ObjectId($document->id)], ['typeMap' => ['root' => 'array', 'document' => 'array']]); + + $referenceId = new ObjectId($reference->id); + + self::assertEquals($referenceId, $result['referenceStoreAsId']); + self::assertEquals(['id' => $referenceId], $result['referenceStoreAsRef']); + self::assertEquals(['$ref' => 'GH2825Document', '$id' => $referenceId], $result['referenceStoreAsDbRef']); + + self::assertEquals($referenceId, $result['embedded']['referenceStoreAsId']); + self::assertEquals(['id' => $referenceId], $result['embedded']['referenceStoreAsRef']); + self::assertEquals(['$ref' => 'GH2825Document', '$id' => $referenceId], $result['embedded']['referenceStoreAsDbRef']); + } +} + +#[ODM\Document] +class GH2825Document +{ + #[ODM\Id] + public string|null $id; + + #[ODM\Field] + public string $name; + + #[ODM\EmbedOne(targetDocument: GH2825Embedded::class)] + public GH2825Embedded|null $embedded = null; + + #[ODM\ReferenceOne(targetDocument: self::class, storeAs: ClassMetadata::REFERENCE_STORE_AS_ID)] + public GH2825Document|null $referenceStoreAsId = null; + + #[ODM\ReferenceOne(targetDocument: self::class, storeAs: ClassMetadata::REFERENCE_STORE_AS_REF)] + public GH2825Document|null $referenceStoreAsRef = null; + + #[ODM\ReferenceOne(targetDocument: self::class, storeAs: ClassMetadata::REFERENCE_STORE_AS_DB_REF)] + public GH2825Document|null $referenceStoreAsDbRef = null; + + public function __construct(string $name) + { + $this->name = $name; + } +} + +#[ODM\EmbeddedDocument] +class GH2825Embedded +{ + #[ODM\Field(name: 'renamed')] + public string $property; + + #[ODM\EmbedOne(targetDocument: self::class)] + public GH2825Embedded $embedded; + + #[ODM\ReferenceOne(targetDocument: GH2825Document::class, storeAs: ClassMetadata::REFERENCE_STORE_AS_ID)] + public GH2825Document|null $referenceStoreAsId = null; + + #[ODM\ReferenceOne(targetDocument: GH2825Document::class, storeAs: ClassMetadata::REFERENCE_STORE_AS_REF)] + public GH2825Document|null $referenceStoreAsRef = null; + + #[ODM\ReferenceOne(targetDocument: GH2825Document::class, storeAs: ClassMetadata::REFERENCE_STORE_AS_DB_REF)] + public GH2825Document|null $referenceStoreAsDbRef = null; + + public function __construct(string $property) + { + $this->property = $property; + } +} From a13bd4c3e03350d6342fa2cd502653595c687aff Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Tue, 30 Sep 2025 09:17:24 +0200 Subject: [PATCH 3/7] Address code review feedback --- .../MongoDB/Persisters/DocumentPersister.php | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php index fe78d852d2..b2a27dc0dd 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php @@ -1138,12 +1138,14 @@ private function convertToDatabaseValue(string $fieldName, $value, ?ClassMetadat private function prepareQueryReference(mixed $value, ClassMetadata $class): mixed { - if (! is_array($value)) { - return $class->getDatabaseIdentifierValue($value); - } - - // Objects without operators or with DBRef fields can be converted immediately - if (! $this->hasQueryOperators($value) || $this->hasDBRefFields($value)) { + if ( + // Scalar values are prepared immediately + ! is_array($value) + // Objects without operators can be prepared immediately + || ! $this->hasQueryOperators($value) + // Objects with DBRef fields can be prepared immediately + || $this->hasDBRefFields($value) + ) { return $class->getDatabaseIdentifierValue($value); } @@ -1159,7 +1161,7 @@ private function prepareQueryReference(mixed $value, ClassMetadata $class): mixe * * @param mixed $value * - * @return array + * @return array Returns an array of tuples containing the prepared field name and value */ private function prepareQueryElement(string $originalFieldName, $value = null, ?ClassMetadata $class = null, bool $prepareValue = true, bool $inNewObj = false, string $fieldNamePrefix = ''): array { @@ -1264,6 +1266,7 @@ private function prepareQueryElement(string $originalFieldName, $value = null, ? // Third part (if any) is the name of a field // That means, we can implode all field parts except the first as the next field name if ($fieldNameParts[1] === '$') { + assert($partCount >= 3); $objectProperty = $fieldNameParts[2]; $referencePrefix = $fieldNamePrefix . $mapping['name'] . '.$'; } else { @@ -1283,7 +1286,7 @@ private function prepareQueryElement(string $originalFieldName, $value = null, ? /* * 1 element: impossible (because of the dot) * 2 elements: fieldName.objectProperty, fieldName., or fieldName.$. For EmbedMany and ReferenceMany, treat the second element as index if $inNewObj is true and convert the value. Otherwise, recurse. - * 3+ elements: fieldname.foo.bar, fieldName..foo, or fieldName.$foo. For EmbedMany and ReferenceMany, treat the second element as index, and recurse into the third element. Otherwise, recurse with the second element as field name. + * 3+ elements: fieldname.foo.bar, fieldName..foo, or fieldName.$.foo. For EmbedMany and ReferenceMany, treat the second element as index, and recurse into the third element. Otherwise, recurse with the second element as field name. */ if ($mapping['type'] === ClassMetadata::MANY) { if ($inNewObj || CollectionHelper::isHash($mapping['strategy'])) { @@ -1298,7 +1301,7 @@ private function prepareQueryElement(string $originalFieldName, $value = null, ? $newPrefix = $fieldNamePrefix . $mapping['name'] . '.' . $fieldNameParts[1] . '.'; $newFieldName = implode('.', array_slice($fieldNameParts, 2)); } else { - // When serialising a query, the second segment is a positional operator ($), a numeric index for collections, or anything else for a hash. + // When serializing a query, the second segment is a positional operator ($), a numeric index for collections, or anything else for a hash. $newPrefix = $fieldNamePrefix . $mapping['name'] . '.'; $newFieldName = implode('.', array_slice($fieldNameParts, 1)); } From 1859767dd54251528f6cde78f67e169f4bcf2777 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Tue, 30 Sep 2025 14:24:23 +0200 Subject: [PATCH 4/7] Add tests and fix preparation of positional operators --- .../MongoDB/Persisters/DocumentPersister.php | 41 ++-- .../MongoDB/Persisters/PersistenceBuilder.php | 28 ++- .../Tests/Functional/Ticket/GH2825Test.php | 176 +++++++++++++++++- 3 files changed, 213 insertions(+), 32 deletions(-) diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php index b2a27dc0dd..954999323d 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php @@ -1259,30 +1259,6 @@ private function prepareQueryElement(string $originalFieldName, $value = null, ? return [[$fieldName, $prepareValue ? $this->convertToDatabaseValue($fieldNameParts[0], $value) : $value]]; } - // Don't recurse for references. Instead, prepare the reference directly - if (! empty($mapping['reference'])) { - // First part is the name of the reference - // Second part is either a positional operator, index/key, or the name of a field - // Third part (if any) is the name of a field - // That means, we can implode all field parts except the first as the next field name - if ($fieldNameParts[1] === '$') { - assert($partCount >= 3); - $objectProperty = $fieldNameParts[2]; - $referencePrefix = $fieldNamePrefix . $mapping['name'] . '.$'; - } else { - $objectProperty = $fieldNameParts[1]; - $referencePrefix = $fieldNamePrefix . $mapping['name']; - } - - if ($targetClass->hasField($objectProperty) && $targetClass->isIdentifier($objectProperty)) { - $fieldName = ClassMetadata::getReferenceFieldName($mapping['storeAs'], $referencePrefix); - - return [[$fieldName, $prepareValue ? $this->prepareQueryReference($value, $targetClass) : $value]]; - } - - return [[$fieldName, $prepareValue ? $this->convertToDatabaseValue($objectProperty, $value, $targetClass) : $value]]; - } - /* * 1 element: impossible (because of the dot) * 2 elements: fieldName.objectProperty, fieldName., or fieldName.$. For EmbedMany and ReferenceMany, treat the second element as index if $inNewObj is true and convert the value. Otherwise, recurse. @@ -1292,9 +1268,20 @@ private function prepareQueryElement(string $originalFieldName, $value = null, ? if ($inNewObj || CollectionHelper::isHash($mapping['strategy'])) { // When there are only two segments in a hash or when serialising a new object, we seem to be replacing an entire element. Don't recurse, just convert the value. if ($partCount === 2) { - $fieldName = $fieldNamePrefix . $mapping['name'] . '.' . $fieldNameParts[1]; - - return [[$fieldName, $prepareValue ? $this->convertToDatabaseValue($fieldNameParts[0], $value, $targetClass) : $value]]; + // In order to prepare the embedded document value, we need to recurse with the original field name, then append the second segment + $prepared = $this->prepareQueryElement( + $mapping['name'], + $value, + $targetClass, + $prepareValue, + $inNewObj, + $fieldNamePrefix, + ); + + $preparedFieldName = $prepared[0][0]; + $preparedValue = $prepared[0][1]; + + return [[$preparedFieldName . '.' . $fieldNameParts[1], $preparedValue]]; } // When there are more than two segments, treat the second segment (index/key/positional operator) as part of the field name and recurse into the rest diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php b/lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php index 80c257d66c..059a0b6eed 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php @@ -4,6 +4,8 @@ namespace Doctrine\ODM\MongoDB\Persisters; +use Doctrine\Common\Collections\ArrayCollection; +use Doctrine\Common\Collections\Collection; use Doctrine\ODM\MongoDB\DocumentManager; use Doctrine\ODM\MongoDB\Mapping\ClassMetadata; use Doctrine\ODM\MongoDB\Mapping\MappingException; @@ -392,7 +394,12 @@ public function prepareEmbeddedDocumentValue(array $embeddedMapping, $embeddedDo break; } - $value = $this->prepareAssociatedCollectionValue($rawValue, $includeNestedCollections); + // Prepare persistent collection if it's not already one + $collection = $rawValue instanceof PersistentCollectionInterface + ? $rawValue + : $this->preparePersistentCollection($mapping, $embeddedDocument, $rawValue); + + $value = $this->prepareAssociatedCollectionValue($collection, $includeNestedCollections); break; default: @@ -507,4 +514,23 @@ public function prepareAssociatedCollectionValue(PersistentCollectionInterface $ return $setData; } + + private function preparePersistentCollection(array $mapping, object $owner, mixed $rawValue): PersistentCollectionInterface + { + if ($rawValue instanceof PersistentCollectionInterface) { + return $rawValue; + } + + // If $actualData[$name] is not a Collection then use an ArrayCollection. + if (! $rawValue instanceof Collection) { + $rawValue = new ArrayCollection($rawValue); + } + + // Inject PersistentCollection + $coll = $this->dm->getConfiguration()->getPersistentCollectionFactory()->create($this->dm, $mapping, $rawValue); + $coll->setOwner($owner, $mapping); + $coll->setDirty(! $rawValue->isEmpty()); + + return $coll; + } } diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH2825Test.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH2825Test.php index 9ba85c6954..1405a47b04 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH2825Test.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH2825Test.php @@ -4,6 +4,8 @@ namespace Doctrine\ODM\MongoDB\Tests\Functional\Ticket; +use Doctrine\Common\Collections\ArrayCollection; +use Doctrine\Common\Collections\Collection; use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM; use Doctrine\ODM\MongoDB\Mapping\ClassMetadata; use Doctrine\ODM\MongoDB\Tests\BaseTestCase; @@ -11,7 +13,7 @@ class GH2825Test extends BaseTestCase { - public function testQueryBuilderUpdatesEmbeddedDocumentCorrectly(): void + public function testQueryBuilderUpdatesEmbedOneCorrectly(): void { $document = new GH2825Document('foo'); $document->embedded = new GH2825Embedded('level 1'); @@ -37,7 +39,62 @@ public function testQueryBuilderUpdatesEmbeddedDocumentCorrectly(): void self::assertSame('level 2', $result['embedded']['embedded']['renamed']); } - public function testQueryBuilderUpdatesReferencesCorrectly(): void + public function testQueryBuilderUpdatesEmbedManyCorrectly(): void + { + $document = new GH2825Document('foo'); + $document->embeddedDocuments[] = new GH2825Embedded('level 1'); + + $this->dm->persist($document); + $this->dm->flush(); + + $embedded = new GH2825Embedded('level 2'); + + $this->dm->persist($embedded); + + $this->dm->createQueryBuilder(GH2825Document::class) + ->updateOne() + ->field('id')->equals($document->id) + ->field('embeddedDocuments.property')->equals('level 1') + ->field('embeddedDocuments.$.embedded')->set($embedded) + ->getQuery() + ->execute(); + + $result = $this->dm->getDocumentCollection(GH2825Document::class) + ->findOne(['_id' => new ObjectId($document->id)]); + + self::assertIsArray($result['embeddedDocuments']); + self::assertSame('level 1', $result['embeddedDocuments'][0]['renamed']); + self::assertSame('level 2', $result['embeddedDocuments'][0]['embedded']['renamed']); + } + + public function testQueryBuilderReplacesEmbedManyCorrectly(): void + { + $document = new GH2825Document('foo'); + $document->embeddedDocuments[] = new GH2825Embedded('original'); + + $this->dm->persist($document); + $this->dm->flush(); + + $embedded = new GH2825Embedded('replaced'); + + $this->dm->persist($embedded); + + $this->dm->createQueryBuilder(GH2825Document::class) + ->updateOne() + ->field('id')->equals($document->id) + ->field('embeddedDocuments.property')->equals('original') + ->field('embeddedDocuments.$')->set($embedded) + ->getQuery() + ->execute(); + + $result = $this->dm->getDocumentCollection(GH2825Document::class) + ->findOne(['_id' => new ObjectId($document->id)]); + + self::assertIsArray($result['embeddedDocuments']); + self::assertSame('replaced', $result['embeddedDocuments'][0]['renamed']); + } + + public function testQueryBuilderUpdatesReferenceOneCorrectly(): void { $document = new GH2825Document('document'); $document->embedded = new GH2825Embedded('embedded'); @@ -73,6 +130,77 @@ public function testQueryBuilderUpdatesReferencesCorrectly(): void self::assertEquals(['id' => $referenceId], $result['embedded']['referenceStoreAsRef']); self::assertEquals(['$ref' => 'GH2825Document', '$id' => $referenceId], $result['embedded']['referenceStoreAsDbRef']); } + + public function testQueryBuilderUpdatesReferenceManyCorrectly(): void + { + $document = new GH2825Document('document'); + $document->embeddedDocuments[] = new GH2825Embedded('embedded'); + $reference = new GH2825Document('referenced'); + + $this->dm->persist($document); + $this->dm->persist($reference); + + $this->dm->flush(); + + $this->dm->createQueryBuilder(GH2825Document::class) + ->updateOne() + ->field('id')->equals($document->id) + ->field('embeddedDocuments.property')->equals('embedded') + ->field('embeddedDocuments.$.referenceStoreAsId')->set($reference) + ->field('embeddedDocuments.$.referenceStoreAsRef')->set($reference) + ->field('embeddedDocuments.$.referenceStoreAsDbRef')->set($reference) + ->getQuery() + ->execute(); + + $result = $this->dm->getDocumentCollection(GH2825Document::class) + ->findOne(['_id' => new ObjectId($document->id)], ['typeMap' => ['root' => 'array', 'document' => 'array']]); + + $referenceId = new ObjectId($reference->id); + + self::assertIsArray($result['embeddedDocuments']); + + self::assertEquals($referenceId, $result['embeddedDocuments'][0]['referenceStoreAsId']); + self::assertEquals(['id' => $referenceId], $result['embeddedDocuments'][0]['referenceStoreAsRef']); + self::assertEquals(['$ref' => 'GH2825Document', '$id' => $referenceId], $result['embeddedDocuments'][0]['referenceStoreAsDbRef']); + } + + public function testQueryBuilderReplacesReferenceManyCorrectly(): void + { + $document = new GH2825Document('document'); + $reference = new GH2825Document('original'); + $otherReference = new GH2825Document('original'); + $document->referencedDocumentsStoreAsId[] = $reference; + $document->referencedDocumentsStoreAsRef[] = $reference; + $document->referencedDocumentsStoreAsDbRef[] = $reference; + + $this->dm->persist($document); + $this->dm->persist($reference); + $this->dm->persist($otherReference); + + $this->dm->flush(); + + $this->dm->createQueryBuilder(GH2825Document::class) + ->updateOne() + ->field('id')->equals($document->id) + ->field('referencedDocumentsStoreAsId.0')->set($otherReference) + ->field('referencedDocumentsStoreAsRef.0')->set($otherReference) + ->field('referencedDocumentsStoreAsDbRef.0')->set($otherReference) + ->getQuery() + ->execute(); + + $result = $this->dm->getDocumentCollection(GH2825Document::class) + ->findOne(['_id' => new ObjectId($document->id)], ['typeMap' => ['root' => 'array', 'document' => 'array']]); + + $referenceId = new ObjectId($otherReference->id); + + self::assertIsArray($result['referencedDocumentsStoreAsId']); + self::assertIsArray($result['referencedDocumentsStoreAsRef']); + self::assertIsArray($result['referencedDocumentsStoreAsDbRef']); + + self::assertEquals($referenceId, $result['referencedDocumentsStoreAsId'][0]); + self::assertEquals(['id' => $referenceId], $result['referencedDocumentsStoreAsRef'][0]); + self::assertEquals(['$ref' => 'GH2825Document', '$id' => $referenceId], $result['referencedDocumentsStoreAsDbRef'][0]); + } } #[ODM\Document] @@ -96,9 +224,29 @@ class GH2825Document #[ODM\ReferenceOne(targetDocument: self::class, storeAs: ClassMetadata::REFERENCE_STORE_AS_DB_REF)] public GH2825Document|null $referenceStoreAsDbRef = null; + /** @var Collection */ + #[ODM\EmbedMany(targetDocument: GH2825Embedded::class)] + public Collection $embeddedDocuments; + + /** @var Collection */ + #[ODM\ReferenceMany(targetDocument: self::class, storeAs: ClassMetadata::REFERENCE_STORE_AS_ID)] + public Collection $referencedDocumentsStoreAsId; + + /** @var Collection */ + #[ODM\ReferenceMany(targetDocument: self::class, storeAs: ClassMetadata::REFERENCE_STORE_AS_REF)] + public Collection $referencedDocumentsStoreAsRef; + + /** @var Collection */ + #[ODM\ReferenceMany(targetDocument: self::class, storeAs: ClassMetadata::REFERENCE_STORE_AS_DB_REF)] + public Collection $referencedDocumentsStoreAsDbRef; + public function __construct(string $name) { - $this->name = $name; + $this->name = $name; + $this->embeddedDocuments = new ArrayCollection(); + $this->referencedDocumentsStoreAsId = new ArrayCollection(); + $this->referencedDocumentsStoreAsRef = new ArrayCollection(); + $this->referencedDocumentsStoreAsDbRef = new ArrayCollection(); } } @@ -120,8 +268,28 @@ class GH2825Embedded #[ODM\ReferenceOne(targetDocument: GH2825Document::class, storeAs: ClassMetadata::REFERENCE_STORE_AS_DB_REF)] public GH2825Document|null $referenceStoreAsDbRef = null; + /** @var Collection */ + #[ODM\EmbedMany(targetDocument: self::class)] + public Collection $embeddedDocuments; + + /** @var Collection */ + #[ODM\ReferenceMany(targetDocument: GH2825Document::class, storeAs: ClassMetadata::REFERENCE_STORE_AS_ID)] + public Collection $referencedDocumentsStoreAsId; + + /** @var Collection */ + #[ODM\ReferenceMany(targetDocument: GH2825Document::class, storeAs: ClassMetadata::REFERENCE_STORE_AS_REF)] + public Collection $referencedDocumentsStoreAsRef; + + /** @var Collection */ + #[ODM\ReferenceMany(targetDocument: GH2825Document::class, storeAs: ClassMetadata::REFERENCE_STORE_AS_DB_REF)] + public Collection $referencedDocumentsStoreAsDbRef; + public function __construct(string $property) { - $this->property = $property; + $this->property = $property; + $this->embeddedDocuments = new ArrayCollection(); + $this->referencedDocumentsStoreAsId = new ArrayCollection(); + $this->referencedDocumentsStoreAsRef = new ArrayCollection(); + $this->referencedDocumentsStoreAsDbRef = new ArrayCollection(); } } From 7620b561dc53a2653dfd4ee66a85946f1501cf7c Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Wed, 1 Oct 2025 10:06:41 +0200 Subject: [PATCH 5/7] Fix preparation of references in queries --- .../MongoDB/Persisters/DocumentPersister.php | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php index 954999323d..1bf370fc02 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php @@ -1259,6 +1259,30 @@ private function prepareQueryElement(string $originalFieldName, $value = null, ? return [[$fieldName, $prepareValue ? $this->convertToDatabaseValue($fieldNameParts[0], $value) : $value]]; } + // Don't recurse for references in queries. Instead, prepare them directly + if (! $inNewObj && ! empty($mapping['reference'])) { + // First part is the name of the reference + // Second part is either a positional operator, index/key, or the name of a field + // Third part (if any) is the name of a field + // That means, we can implode all field parts except the first as the next field name + if ($fieldNameParts[1] === '$') { + assert($partCount >= 3); + $objectProperty = $fieldNameParts[2]; + $referencePrefix = $fieldNamePrefix . $mapping['name'] . '.$'; + } else { + $objectProperty = $fieldNameParts[1]; + $referencePrefix = $fieldNamePrefix . $mapping['name']; + } + + if ($targetClass->hasField($objectProperty) && $targetClass->isIdentifier($objectProperty)) { + $fieldName = ClassMetadata::getReferenceFieldName($mapping['storeAs'], $referencePrefix); + + return [[$fieldName, $prepareValue ? $this->prepareQueryReference($value, $targetClass) : $value]]; + } + + return [[$fieldName, $prepareValue ? $this->convertToDatabaseValue($objectProperty, $value, $targetClass) : $value]]; + } + /* * 1 element: impossible (because of the dot) * 2 elements: fieldName.objectProperty, fieldName., or fieldName.$. For EmbedMany and ReferenceMany, treat the second element as index if $inNewObj is true and convert the value. Otherwise, recurse. From af06a28be48ef7b24f581e6e8aaf9b4184ecd402 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Wed, 1 Oct 2025 10:17:56 +0200 Subject: [PATCH 6/7] Update PHPStan baseline --- phpstan-baseline.neon | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 3e870535f8..e3833254b8 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -738,6 +738,18 @@ parameters: count: 1 path: lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php + - + message: '#^Method Doctrine\\ODM\\MongoDB\\Persisters\\PersistenceBuilder\:\:preparePersistentCollection\(\) has parameter \$mapping with no value type specified in iterable type array\.$#' + identifier: missingType.iterableValue + count: 1 + path: lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php + + - + message: '#^Method Doctrine\\ODM\\MongoDB\\Persisters\\PersistenceBuilder\:\:preparePersistentCollection\(\) return type with generic interface Doctrine\\ODM\\MongoDB\\PersistentCollection\\PersistentCollectionInterface does not specify its types\: TKey, T$#' + identifier: missingType.generics + count: 1 + path: lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php + - message: '#^Call to an undefined static method Doctrine\\ODM\\MongoDB\\Proxy\\Factory\\LazyGhostProxyFactory\:\:createLazyGhost\(\)\.$#' identifier: staticMethod.notFound From e53b8eeaa425c918b935d09bf437d2d7c8d8b913 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 6 Oct 2025 15:13:44 +0200 Subject: [PATCH 7/7] Narrow type for value when preparing persistent collection --- lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php b/lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php index 059a0b6eed..beb53ee47f 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php @@ -515,7 +515,8 @@ public function prepareAssociatedCollectionValue(PersistentCollectionInterface $ return $setData; } - private function preparePersistentCollection(array $mapping, object $owner, mixed $rawValue): PersistentCollectionInterface + /** @param array|Collection $rawValue */ + private function preparePersistentCollection(array $mapping, object $owner, array|Collection $rawValue): PersistentCollectionInterface { if ($rawValue instanceof PersistentCollectionInterface) { return $rawValue;