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

Impossible to query nested documents with STORE_AS_REF #1723

Closed TiMESPLiNTER closed 6 years ago

TiMESPLiNTER commented 6 years ago

doctrine/mongodb-odm 1.2.1

I like to find all persons having an entry for company with id foo-123. I use the following query for it:

$persons = $this->personRepository->createQueryBuilder()
            ->field('companies.company.id')->equals('foo-123')
            ->getQuery()->getQuery();

This leads to the following query:

Array
(
    [type] => 1
    [query] => Array
        (
            [companies.company._id] => "foo-123"
        )

    [newObj] => Array
        (
        )

)

The problem is that it somehow converts the .id part to ._id although the ODM STORE_AS_REF object has only a field named id (without underscore).

This seems to me like a bug.

This is an example of a person document:

{
    "_id" : "bar-456",
    "gender" : "MALE",
    "firstName" : "John",
    "lastName" : "Doe",
    "companies" : [ 
        {
            "company" : {
                "id" : "foo-123"
            },
            "role" : "CEO"
        }
    ]
}

companies holds mapManyEmbeddeds of ReferenceCompany and this mapOneReferences a STORE_AS_REF on the Company document itself.

Person ODM mapping

$metadata->mapManyEmbedded([
    'fieldName' => 'companies',
    'targetDocument' => ReferenceCompany::class,
]);

ReferenceCompany ODM mapping

$metadata->mapOneReference([
    'fieldName' => 'company',
    'targetDocument' => Company::class,
    'storeAs' => ClassMetadataInfo::REFERENCE_STORE_AS_REF
]);
malarzm commented 6 years ago

If I'm seeing correctly this is happening because preparing for queries does not check whether a field is a reference, just whether it has a targetDocument here - probably it wasn't expected that a ref can get there. @TiMESPLiNTER are you also willing to work on a fix?

Offhand you should be ok using references (given you have an object that is referenced).

TiMESPLiNTER commented 6 years ago

@malarzm I'm willing to contribute yes.

I added a test case to reproduce the behavior: #1728

When I try to use references I end in a PHP error:

 $persons = $queryBuilder
            ->field('companies.company')->references($company)
            ->getQuery()->getQuery();

leads to a Doctrine\ODM\MongoDB\Mapping\MappingException:

No mapping found for field 'companies.company' in class 'Documents\Person'. ClassMetadataInfo.php(1879): Doctrine\ODM\MongoDB\Mapping\MappingException::mappingNotFound('...', 'companies.compa...')

malarzm commented 6 years ago

Ah my bad, references can't handle embedded mapping. Offhand it could for one level deep, that could be implemented in the future :)

TiMESPLiNTER commented 6 years ago

@malarzm I just debugged the whole thing a bit using my test case and from what I can tell the problem is already here: https://github.com/doctrine/mongodb-odm/blob/adf0b7ed469113e93829fb777619754e7d709a7d/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php#L1092

On this line the $fieldName gets rewritten from id to _id cause the if exp evaluates to true.

Just to test I added $fieldName !== 'id' to the condition which then leaves the fieldName unattended. This leads to right query but still doesn't return the matching documents.

malarzm commented 6 years ago

This leads to right query but still doesn't return the matching documents.

Hmmm maybe try with an $elemMatch?

TiMESPLiNTER commented 6 years ago

Hmmm maybe try with an $elemMatch?

@malarzm sorry but I'm not that experienced with mongodb at all so I can't really follow your suggestion. Maybe you can give me more detailed hints/instructions?

malarzm commented 6 years ago

Nevermind the suggestion, the query seems to be working fine for me locally:

maciek(mongod-3.0.12) test> db.coll.find({"companies.company.id": "foo-123"});
{
  "_id": "bar-456",
  "gender": "MALE",
  "firstName": "John",
  "lastName": "Doe",
  "companies": [
    {
      "company": {
        "id": "foo-123"
      },
      "role": "CEO"
    }
  ]
}
Fetched 1 record(s) in 0ms

I'll try to see what happens in your test case

malarzm commented 6 years ago

@TiMESPLiNTER in the meantime this seems to be working:

$persons = $this->personRepository->createQueryBuilder()
            ->field('companies.company')->equals(['id' => 'foo-123'])
            ->getQuery()->getQuery();
malarzm commented 6 years ago

Given my previous idea did not work I think we should allow references/includesReference to either work with nested properties or allow user to pass mapping to the function (latter being way easier):

    public function references($document, ?array $mapping = null)
    {
        $this->requiresCurrentField();
        $mapping = $mapping ?: $this->getReferenceMapping();

@alcaeus what do you think?

alcaeus commented 6 years ago

At the moment, no operator works with nested properties. This is something we should address in 2.0 (and I believe we've previously had a similar case).

oodrive-media commented 6 years ago

We try to avoid to split ->field('companies.company.id')->equals('foo-123') in ->field('companies.company')->equals(['id' => 'foo-123'])

Did you find a workaround that could avoid us to split the field as "Malarzm" suggested?

malarzm commented 6 years ago

Closing as #1793 was just merged