mirror of
https://github.com/doctrine/orm.git
synced 2026-03-23 22:42:18 +01:00
[PR #10455] Make Annotations/Attribute mapping drivers report fields for the classes where they are declared #12372
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?
Original Pull Request: https://github.com/doctrine/orm/pull/10455
State: closed
Merged: Yes
This PR will make the annotations and attribute mapping drivers report mapping configuration for the classes where it is declared, instead of omitting it and reporting it for subclasses only. This is necessary to be able to catch mis-configurations in
ClassMetadataFactory.Fixes #10417, closes #10450, closes #10454.
DoctrineBundle users: A new config setting to opt-in to the new mode will be implemented in https://github.com/doctrine/DoctrineBundle/pull/1661.
⚠️ Summary for users getting
MappingExceptionswith the new modeWhen you set the
$reportFieldsWhereDeclaredconstructor parameters totruefor the AnnotationDriver and/or AttributesDriver and getMappingExceptions, you may be doing one of the following:privatefields with the same name in different classes of an entity inheritance hierarchy (see #10450)As explained in these two PRs, the ORM cannot (or at least, was not designed to) support such configurations. Unfortunately, due to the old – now deprecated – driver behaviour, the misconfigurations could not be detected, and due to previously missing tests, this in turn was not noticed.
Current situation
The annotations mapping driver has the following condition to skip properties that are reported by the PHP reflection API:
69c7791ba2/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php (L345-L357)This code has been there basically unchanged since the initial 2.0 release. The same condition can be found in the attribute driver, probably it has been copied when attributes were added.
I think what the driver tries to do here is to deal with the fact that Reflection will also report
public/protectedproperties inherited from parent classes. This is supported by the observation (see #5744) that e. g. YAML and XML drivers do not contain this logic.The conditions are not precise enough for edge cases. They lead to some fields and configuration values not even being reported by the driver.
Only since the fields would be "discovered" again when reflecting on subclasses, they eventually end up in class metadata structures for the subclasses. In one case of inherited ID generator mappings, the
ClassMetadataFactorywould also rely on this behaviour.Two potential bugs that can result from this are demonstrated in #10450 and #10454.
Suggested solution
In order to find a more reliable way of separating properties that are merely reported again in subclasses from those that are actual re-declarations, use the information already available in
ClassMetadata. In particular,declaredtells us in which non-transient class a "field" was first seen.Make the mapping driver skip only those properties for which we already know that they have been declared in parent classes, and skip them only when the observed declaring class matches the expectation.
For all other properties, report them to
ClassMetadataFactoryand let that deal with consistency checking/error handling.#10450 and #10454 are merged into this PR to show that they pass now.
Soft deprecation strategy
To avoid throwing new/surprising exceptions (even for misconfigurations) during a minor version upgrade, the new driver mode is opt-in.
Users will have to set the
$reportFieldsWhereDeclaredconstructor parameters totruefor theAnnotationDriverand/orAttributesDriver. Unless they do so, a deprecation warning will be raised.In 3.0, the "new" mode will become the default. The constructor parameter can be deprecated (as of ORM 3.1, probably) and is a no-op.
We need to follow up in other places (DoctrineBundle, ... – what else?) to make this driver parameter an easy-to-change configuration setting.