Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
melkamar opened this issue Nov 15, 2024 · 1 comment
Labels
Milestone

Comments

@melkamar
Copy link
Contributor

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:

  • embedded: true
  • type: many
  • targetDocument: null
  • strategy: set

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

  • Adding the $fieldName = $e[0] . '.' . $e[1] . '.' . $e[2]; to the first if-branch, or
  • Changing the condition so that it evaluates to false for the $mapping context I provided above (in which case if-branch on line 1288 gets executed)
@malarzm malarzm added the Bug label Nov 16, 2024
@malarzm malarzm added this to the 2.9.1 milestone Nov 16, 2024
@malarzm
Copy link
Member

malarzm commented Nov 16, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants