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

[GH-8265] Prototype for Attribute Metadata Driver #8266

Merged
merged 43 commits into from
Mar 23, 2021

Conversation

beberlei
Copy link
Member

@beberlei beberlei commented Sep 13, 2020

Support for Attribute Metadata Driver with PHP 8+

Note: For now the AttributeDriver does not support attribute overrides, association overrides, named queries, named native queries and sql resultset mapping, because of missing nested attributes support. This is not a big problem, because these are quite edge case features some of which we have thought about removing anyways. As for the overwrites, we should think of a solution in the near future.

A few other deviations from the Annotation based Driver:

  • Instead of @JoinTable holding join columns and inverse join columns as a property, you use #[ORM\JoinColumn] and #[ORM\InverseJoinColumn] on the top level.
  • Instead of @Table holding indexes and unique constraints, these are also attributes on the top level of a declaration.

Todos:

Fixes #8265

@TomasVotruba
Copy link
Contributor

There is related PR in Symfony annotations class reader. I share it here, because it might bring some inspiration or path to find united way with parsing and working with attributes for both Symfony and Dotrine.

I have modified the existing AnnotationClassLoader so that it merges attributes and annotations. This way, an application could transparently switch from annotations to attributes. Moreover, the AnnotationClassLoader does not require an annotation reader anymore.

The file https://github.com/symfony/symfony/pull/37474/files#diff-90cbecfd4505e6e8af5842db11add3b5

@beberlei
Copy link
Member Author

beberlei commented Oct 4, 2020

@TomasVotruba i don't believe it makes sense to integrate AnnotationClassLoader with attributes for more than a few simple cases, because attributes will not be 100% compatible with annotations. Tieing them together makes everything complicated instead.

@TomasVotruba
Copy link
Contributor

TomasVotruba commented Oct 4, 2020

@beberlei Thanks for sharing. I haven't really digged into it, as the migration path is my only focus here. It would be better to comment to the author there, if you find it useful.

@beberlei beberlei added this to the 2.9.0 milestone Dec 6, 2020
@walva
Copy link

walva commented Mar 10, 2021

Good luck guys ! You rock !

@albe
Copy link
Contributor

albe commented May 28, 2021

Great work on getting native attribute support into doctrine! @beberlei we (neos/flow framework) however now suffer from a slight change in expectations for some annotation classes - namely the ManyToOne, ManyToMany and Embedded now have a required first constructor argument. While those arguments were always considered "required" in a pure doctrine environment, we partially enhanced the DX by extracting some information for the annotations from other reflection data (like the targetEntity/class the annotation targets). Hence we have usage of doctrine annotations like:

@var SomeEntity
@ORM\ManyToMany

which now breaks, because the ManyToMany is attempted to be constructed with null as targetEntity in the DocParser. Until now we solved the "missing" targetEntity (and other things) via a custom AnnotationDriver and that worked out quite nicely. With this change we are at a loss though, because we'd need to hook into the DocParser now, which happens way too early.

See neos/flow-development-collection#2487 (comment)

While we can "solve" this by restricting doctrine/orm to <2.9, we would also like to move towards PHP8 support and attributes. For us the best solution would be if those three constructors would allow null for their first argument, but I understand that would mean you'd need to check for that null at runtime which is a step backward. Optimally the Annotation would not be instanciated until it is used for mapping, but that would be too great a change.
Are there any other possible ways to "enhance" the annotations and hook into the instanciation process?

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

Successfully merging this pull request may close these issues.

Add New Attribute Metadata Driver
8 participants