mirror of
https://github.com/doctrine/orm.git
synced 2026-03-23 22:42:18 +01:00
findBy broken for inherited entity property lookups #6124
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @themizzi on GitHub (Dec 7, 2018).
Originally assigned to: @themizzi on GitHub.
Bug Report
Summary
findBy does not work correctly for inherited entities. proof here: https://github.com/themizzi/doctrine-find-by-metadata-bug
Current behavior
If EntityB has a OneToMany relationship to EntityA, and EntityC inherits (via join) from EntityA. If there is no cache, calling
$entityBRepo->findBy('entityA' => $newEntityCInstance)works fine. On a subsequent request after cache has already been generated and metadata is lazy loaded, the request breaks. on sqlite this just fails, on MySQL this potentially loads up an incorrect entity.How to reproduce
Please see link above for an example and reproduction steps.
Expected behavior
Find by can find classes by properties with an inheritance structure at all times.
@Ocramius commented on GitHub (Dec 7, 2018):
@themizzi please reproduce this bug in isolation within an integration test, like in https://github.com/doctrine/doctrine2/tree/master/tests/Doctrine/Tests/ORM/Functional/Ticket.
@themizzi commented on GitHub (Dec 7, 2018):
will try to do this weekend. other priorities call for now.
@themizzi commented on GitHub (Dec 8, 2018):
Created regression test. Might have some time to looking into an appropriate fix this weekend.
@themizzi commented on GitHub (Dec 8, 2018):
Added a fix for this to check if the class is transient before loading metadata during value retrieval.
This fix keeps existing functionality working. However, while looking over this, I see a potential issue with the existing implementation. Matching criteria in findBy does not care if an entity is related or not via an association mapping. It only cares if the metadata for a class is loaded. So if we are performing a findBy on
propertyAwhich has a target entity ofEntityA, one can pass in any class that already has metadata loaded, related or not toEntityA, and doctrine will convert the value based on the identifier mapping for that class. I believe performing a check on whether the value class is part of the association mapping should be done here, but that could potentially break existing implementations that have abused the current logic. I will open another issue for that logic if appropriate.