doctrine / mongodb-odm

The Official PHP MongoDB ORM/ODM
https://www.doctrine-project.org/projects/doctrine-mongodb-odm/en/latest/
MIT License
1.09k stars 504 forks source link

Mongo query contains duplicate part of property path when targeting an embed-many class with discriminatorMap-based mapping #2697

Open melkamar opened 2 weeks ago

melkamar commented 2 weeks ago

Bug Report

Q A
BC Break no
Version 2.4.3

Summary

For the following query I am getting a duplicated part of the query string:

$builder
    ->field('dataModel.dynamicDataModel.properties.meetingInfo.properties.date.value')
    ->lte(new \DateTime('2021-01-22'));

var_dump($builder->getQuery()->debug());
array(1) {
  ["dataModel.dynamicDataModel.properties.meetingInfo.properties.date.value.date.value"] ...
                                                                            ^^^^^^^^^^ this is duplicated for some reason

This seems to happen when there is a mapping along the path of the query with this metadata:

How to reproduce

I have pinned this down to this location in DocumentPersister. In my case, when that code is being executed, I have this context:

$fieldName is properties.meetingInfo.properties.date.value

$mapping has these values:

{
  "type": "many",
  "embedded": true,
  "targetDocument": null,
  "collectionClass": "RH\\CoreBundle\\Components\\DoctrineBridge\\Collections\\RHArrayCollection",
  "name": "properties",
  "strategy": "set",
  "nullable": false,
  "fieldName": "properties",
  "discriminatorField": "type",
  "discriminatorMap": {
    "null": "RH\\DynamicDataBundle\\Document\\Data\\NullData",
    "array": "RH\\DynamicDataBundle\\Document\\Data\\ArrayData",
    ...
  }
}

The problem is that when the code linked above is executed:

        // No further processing for fields without a targetDocument mapping
        if (! isset($mapping['targetDocument'])) {
            if ($nextObjectProperty) {
                $fieldName .= '.' . $nextObjectProperty; // ---> This is where the suffix gets duplicated and causes problems
            }

            return [[$fieldName, $value]];
        }

At the point before $fieldName .= '.' . $nextObjectProperty; is executed, the values are:

$fieldName == 'properties.meetingInfo.properties.date.value'
$nextObjectProperty == 'date.value'

Which causes the returned field name to contain the .date.value part twice.

The values are set by the condition on line 1276 evaluating to true:

if (
    $mapping['type'] === ClassMetadata::MANY && CollectionHelper::isHash($mapping['strategy'])
        && isset($e[2])
) {
    $objectProperty       = $e[2];
    $objectPropertyPrefix = $e[1] . '.';
    $nextObjectProperty   = implode('.', array_slice($e, 3));
}

This if-branch does not change the value of $fieldName like the other branches do, and thus when a part of the string is then appended to it, it is wrong. If I add the same line as is in the other if-branch below:

    $fieldName            = $e[0] . '.' . $e[1] . '.' . $e[2];

Then my case starts working as expected.


I don't understand the logic of the code well enough to know what is the correct fix here. It's either

malarzm commented 2 weeks ago

Thanks for really thorough description and investigation! I've put the report into next bugfix milestone, we'll see if we manage to fix it soon

I don't understand the logic of the code well enough

Yeah, this is a total can of worms :) You never know (until you run tests) if the applied fix does not break something else ;)