Updated: Comment #0
Problem/Motivation
In #2005716: Promote EntityType to a domain object, we are adding an AnnotationBase class. This is due to the newly expanded AnnotationInterface.
The Plugin class, which is the one extended by 95% of core, returns an array.
This is fast and easy, but may turn out to be a mistake in the long run.
The current base class is abstract and does not implement the get() method.
We could make it concrete, and implement get() with a best-practices approach.
Proposed resolution
The new EntityType annotation class delegates to another class.
The annotation class could also just return itself.
Remaining tasks
Decide what is best practice, implement it.
User interface changes
N/A
API changes
API addition only.
Comments
Comment #1
EclipseGc CreditAttribution: EclipseGc commentedSo, having discussed this a little already, what I'd really like to see is something along these lines:
AnnotationBase is basically for classes, and the Plugin annotation class is basically for arrays. Both implement AnnotationInterface, but there's no inheritance from each other. That being said, I'd suggest a rename is likely in order, something like ClassAnnotationBase and ArrayAnnotationBase. Also, like AnnotationBase, Plugin (whatever it is ultimately named) should also be abstract. This has been on my mental todo list for a LONG time.
One of the benefits arrays have over classes is how dead simple they are to extend with custom keys. We don't need new getters/setters/whatever, and with that in mind, I'd very much like to put into place a solution with the Class returning annotations that allows for some other proxy class to be returned instead. The current patch in #2005716 has an optional 'entity_type_class' key in the plugin values that allows the plugin definition values to be proxied through another class. I'd suggest we formalize this so that it's baked into the assumptions of the AnnotationBase class for all plugin types who use this approach to leverage. It's not AS potentially robust as the array solution, but it mitigates a lot of problems and provides some really sane solutions to other potential use cases. I think TypedData could benefit from a similar approach by returning DataDefinitionInterface objects since it seems to be expecting those for most of the other methods on the manager class.
Suffice it to say, I like where this is generally heading, and I think some rather minor refactoring and formalizing could make this super viable for a couple other odd situations in core w/o needing to affect the rest of the plugin system at large.
Eclipse
Comment #2
tim.plunkettThis is blocked on #2169853: EntityType Annotation does not extend AnnotationBase
Comment #3
tim.plunkettNo need to let this sit on me