Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
#1763974: Convert entity type info into plugins will provide the first plugin type on behalf of an include.
Each plugin type has an $owner and a $type.
For modules, $owner is always the module name.
So in that issue, I chose "Core" as the $owner, and "Entity" as the $type.
But sun suggested "Entity" as the $owner and "Type as the $type, which also makes sense.
Should we standardize on the name of the subsystem as the $owner? If so, we'll need to rename that one.
Comments
Comment #1
tim.plunkettAlso, it occurs to me that when contrib wants to provide an entity type, they'd still need to use the Core namespace, which is wonky.
Comment #2
EclipseGc CreditAttribution: EclipseGc commentedSo, the annotation namespaces have the benefit of telling future devs where they can find the management classes for plugins of this type. a Core owner should immediately imply core\lib\Drupal\Core, and in the case of Core Entity it should imply the directory core\lib\Drupal\Core\Plugin\Core\Entity. As a developer, when I come across node module's lib\Drupal\node\Plugin\Core\Entity, I should be able to deduce the above directory structure immediately. This is why the plugin folks are kind of hard headed about this because whatever you pass there needs to be informative to future developers so that they can dig through the scaffolding for the system if they needed to get a better understanding of it.
I'm not opposed to Entity\Type as the owner\type here, as long as it resides in either the entity module, or if the entity subsystem ends up with its own PSR-0 namespace... and while were on the topic, that's exactly what the difference between Entity\Type (psr-0 namespace) and entity\Type (module namespace) should imply to a developer looking at this later.
In short, Yes, these constructor variables for the AnnotatedClassDiscovery are for discovery of plugins, and they COULD be arbitrary... but they REALLY REALLY shouldn't be because there's a whole set of "win" we could get in what I'll term "reverse discovery" with regard to DX.
Hopefully this rambling made sense.
Eclipse
Comment #3
sunThanks for the rambling, but unfortunately that does not answer the concrete question at hand. What is the answer?
Comment #4
sunRe-reading #2 once more, the answer and solution that strictly follows those ramblings seems pretty clear, but it completely wrecks the architectural design of the plugin system:
Which obviously leads to:
Drupal\node\Plugin\Core\Entity\Type\Node
Not to mention, yet another subdirectory level of doom.
In general, it looks and feels extremely wonky to have "Core" in there. Symfony and other frameworks also have "core services" for which bundles/extensions supply dependent implementations, but you never see a "Symfony" directory in them.
Also note that "Core" is really just our second trashbin for Components, which just happen to not be decoupled yet. The actual topic is:
What's the $owner for components?
PHP namespaces are case-insensitive, but PSR-0 is case-sensitive. So technically
Drupal\node\Plugin\Entity\Type
andDrupal\node\Plugin\entity\ViewMode
could co-exist. However, only as long as $type does not overlap and the same extension (here: node) does not try to implement plugins for both owners — that is, because there are case-insensitive filesystems (e.g., on Windows), which are not able to differ between the subdirectory names.If we want to avoid that situation at all costs, then we would have to go with
Core\Entity\Type
.However, in reality, that only leads to the next best problem: "Core" is not really an owner, and if there is "Core", then there is also "Component" at some point. And alas, the plugin system was designed to be a generic component that can be adopted by other systems and components, so there is a chance that there might be a "Component" at some point that is not a Drupal component.
Thus, the ultimate bullet-proof answer can only be this:
Comment #5
fagoThe bullet-proof suggestion above is really verbose.. :-/
Howsoever, we also have TypedData plugin implementions below the TypedData and Entity component which right now do not use annotations, system.module register this classes. But we want to convert this to annotations and they probably won't stay the only plugins provided by components. Thus, we should make sure that core components are treated like modules for the plugin system, i.e. plugin managers and implementations below a core component should follow the same directory structure as used for a module.
Comment #6
sdboyer CreditAttribution: sdboyer commented#2 - i get what you're going for. but 'reverse discovery' is only an acceptable argument if it is readily discoverable for humans as well as machines, as far as i'm concerned. and human reverse discovery stops applying when we have umpteen patterns governing namespaces and directory locations. no one is going to be able to keep track of why anything is where it is. this is just magic string prefixing all over again, and that is one of the chief things that confused the living shit out of people with ctools plugins for years. and namespaces are *way* more confusing that string prefixing.
namespaces really aren't a system that scale well into multiple masters determining their structure. we need to get this madness under control. this is about the least fortunate way in which we could emulate Java.
Comment #7
attiks CreditAttribution: attiks commentedFYI: Maybe not really related, but have a look at #1831154: Allow the AnnotatedClassDiscovery to use custom locations
Comment #8
sunRelated: #1836008: Remove drupal_classloader() use in Drupal\Core\AnnotatedClassDiscovery
Comment #9
effulgentsia CreditAttribution: effulgentsia commentedI have a slight modification to #2: plugin manager classes should ideally always be named
Drupal\[$owner]\Plugin\[$CamelizedType]Manager
If the entity type manager class is
Drupal\Core\Entity\Plugin\EntityManager
, that means that $owner should beCore\Entity
and $type should beentity
. However, I would suggest renaming the manager class toDrupal\Core\Entity\Plugin\EntityTypeManager
orDrupal\Core\Entity\Plugin\TypeManager
, which would leave $owner asCore\Entity
, and make $type intoentity_type
ortype
.#4 is correct that this convention does not allow for plugin types / managers to be provided by non-Drupal projects. Frankly, I think that's ok. Those projects can pick their own convention. At some point, we may want Drupal plugin types and non-Drupal plugin types to more cleanly co-exist, but let's cross that bridge when we have more concrete examples of non-Drupal plugin types in the wild.
Comment #10
sunI think this issue mainly targets plugins though, not plugin managers. Although that might not matter that much.
What I rather want to stress on is that we should forget about "Core". Ideally, yesterday.
"Core" has to be replaced with "Component". The only reason "Core" exists is that the components within are not properly decoupled yet. As soon as they are, there is no "Core" anymore.
"Core" shares the same problem with "Component" though: It has literally no meaning at all. All that both are denoting is a trashbin for components. They are taxonomy terms. They do not represent an owner. Like, at all. Not in the way the plugin system understands owners. The components within are owners.
We not only need to forget about "Core", we also need to forget about "Component".
That leaves us with "Drupal" and "Entity". If I could make any sense of this, then the $owner would have to be
Drupal\Entity
. So as to uniquely differentiate it fromDoctrine\Entity
.Once we arrive at that point, we only have to decide whether the plugin system is truly generic or not. If it is, then the $owner must be
Drupal\Entity
. If it is not, then the $owner could just beEntity
.This is crystal clear from my perspective. All we need is a decision on whether To Be Generic, Or Not To Be. That is the question.
In the case of just
Entity
, due to the way the plugin system works, that would inherently prevent co-existence of components and extensions that share the same, case-insensitive name. (e.g., a contributedentity
module would not be able to participate in the plugin system)All in all,
Drupal\Entity
as $owner appears to be the correct answer for me. That would leave us with:Comment #12
effulgentsia CreditAttribution: effulgentsia commentedSo we'd have $owner =
Drupal\Entity
,Drupal\Mail
, etc. for Drupal's various non-module subsystems, andentity
,mail
, etc. for correspondingly named modules? I can accept that.Comment #13
effulgentsia CreditAttribution: effulgentsia commentedThough, if we're breaking the mapping of plugin manager lives in
Drupal\[$owner]\Plugin\[$CamelizedType]Manager
, then why not $owner =Drupal
, $type =entity_type
?Comment #14
tstoecklerWhy, instead of
Drupal\Mail
,Drupal\Entity
, not simplyMail
,Entity
?Comment #15
sun#13:
I don't consider "Drupal" to be a formal subject/owner. "Drupal" only complements the actual component name, with the sole purpose to uniquely differentiate it from a non-Drupal Entity component.
That, on its own, circles back into the ultimate question of whether the plugin system should be super-generic or not. If we decide against, then there's little point in a "Drupal" prefix in the first place.
#12:
Having plugin namespaces of components in core "extra-vendorized", but not those of extensions, is actually weird from my perspective — but mayhaps acceptable. True consistency without any weirdness would circle back into the full-blown bullet-proof plugin namespaces of #4.
Comment #16
effulgentsia CreditAttribution: effulgentsia commentedThe problem with owner of
Entity
is that if you also have an entity.module creating an owner ofentity
, then you cannot have modules that implement plugins for both owners, because on a case-insensitive file system, you can't simultaneously havemodules/node/Plugin/Entity/
andmodules/node/Plugin/entity/
.If the reason for a
Drupal
prefix is to avoid collision with non-Drupal code, then we need it for modules too: i.e.,Drupal\aggregator
, which means we can't useDrupal\Entity
as that collides withDrupal\entity
.Unlike sun, I do consider "Drupal" to be a perfectly ok formal owner of everything that's not in a module, so #13 remains my current preference.
But, if we must allow for non-collision of Drupal module owners with non-Drupal owners (i.e., we want to allow owner of Assetic (the non-Drupal project) and assetic (a hypothetical Drupal module)), then we can do owner = full namespace (i.e.,
Drupal\Core\Entity
andDrupal\aggregator
), but I'm not convinced that we'll have enough use cases of needing this collision protection to justify the DX loss.Comment #17
effulgentsia CreditAttribution: effulgentsia commentedIf PSR-0 allows for namespace parts to start with a leading _, then we can use that for subsystems: e.g., allow the $owner of plugin types defined by Drupal\Core\Entity to be
_entity
.But, I'm still wondering what the objection to
Drupal
,Core
, orComponent
as $owner values is. All $owner needs to be is a space within which plugin type names can be unique. Each contrib module needs to be its own $owner, because there's no way to realistically coordinate unique plugin type names among thousands of contrib maintainers. But, we can certainly ensure unique plugin type names across the entirety of Drupal subsystems.Comment #18
tstoecklerI don't think not being able to have a generic entity.module should be an argument against this approach. If you imagine a fully decoupled and component-ized Drupal, the differences between modules and components are not that large. Modules can have a bunch of things (bundle, schema, config, ...) that components can't, but those are really implementation details (i.e. we could easily support a Schema.php or whatever in each component if we wanted to). Conceptually, though, the two are not far apart. So having both an Entity component and an entity.module could be considered a WTF, actually. Regardless of what we do here, I would always suggest to name the module something more specific (i.e. entity_ui.module, entity_remote.module, or whatever the heck a theoretical entity.module might do) in order for it not to be confused with the component.
Comment #19
fagoYes, having component names and modules names in prefixes is a general namespacing problem - it's not special to plugin types - as pointed ou by #18 also. Therefore, I'd suggest we generally solve the namespacing problem and use Entity as owner. We could disallow modules being named like components? But then, is it possible to add non-core components without having a module?
Comment #20
effulgentsia CreditAttribution: effulgentsia commentedWhile it's okay to tell people, "don't name your 8.x modules the same as any Drupal core component names that exist when 8.0 is shipped", what if we want to add a core component in 8.1-8.n? Not only would we need to check whether a d.o. contrib project already exists by that name (annoying, but possible), but additionally, we'd have no way of knowing if some sites out there are running a custom module with that name.
With function names in include files, yes, we've always had this problem, and would therefore not introduce new function name prefixes in minor releases. But a benefit of namespaced classes is that we no longer have this problem, except for plugin discovery.
https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-0.md, confirms that "_ has no special meaning in a namespace." Given this, is there any objection to making
$owner = '_' . strtolower($third_part_of_namespace)
(i.e., "_entity")? It would also have the benefit of, in a folder listing of $moduledir/lib/Drupal/$modulename/Plugin, putting all core subsystem plugin type owners at the top.Comment #21
EclipseGc CreditAttribution: EclipseGc commentedI think the tweaks we're trying to make to abstracting annotated class discovery up to a component should in effect remove this complaint as arbitrary directories would be able to be injected as namespaces, so search Drupal\Core\Entity for plugins local to it would become totally kosher. Is this a fair evaluation of what the actual complaint here is?
Eclipse
Comment #22
effulgentsia CreditAttribution: effulgentsia commented#1849752: Abstract non-Drupal-specific parts of AnnotatedClassDiscovery into a Drupal\Component base class will help allow us to do whatever arbitrary thing we want, including pulling entity type classes out of
Plugin
entirely, so potentially we could decide that in the case of entity types, we want to abandon thePlugin/$owner/$type
convention entirely and renameDrupal\node\Plugin\Core\Entity\Node
to something more likeDrupal\node\Entity\Node
. I guess one option is we decide to do that not just for entity types, but for all other plugin types defined by core subsystems. However, if we decide we want to keep thePlugin/$owner/$type
convention for at least some core subsystem plugin types, then we'll still need to set a convention for what $owner should be. Contrib can always break whatever conventions it wants, but we still need conventions for core.Comment #23
EclipseGc CreditAttribution: EclipseGc commentedSo the reason the convention is Plugin\$owner\$type is because I wanted plugin directories to be obviously plugins. Drupal\node\Entity\Node, while nice and clean, doesn't tell anyone it's a plugin (which is fine considering the fact that it's actually not right now, but my point stands). If we want to occasionally break that convention, I'm ok with providing methodologies for doing that as #1849752: Abstract non-Drupal-specific parts of AnnotatedClassDiscovery into a Drupal\Component base class does, but another thing that I thought this was complaining about was the fact that Drupal\Core\Entity can't contain plugins, which that issue also fixes. I'm just questioning if that removes the need for this issue to be anything but meta. And agreed upon convention is great, exceptions to that convention are acceptable, but if the referenced issue takes care of the technical sides of these concerns, are we down to just a convention discussion?
Eclipse
Comment #24
andypostSo issue about menus as config entities needs suggestion to place a required Menu entity, glad to hear any suggestions #1814916: Convert menus into entities
Chx recomends
lib/Drupa/Core/Menu
Comment #25
effulgentsia CreditAttribution: effulgentsia commentedFYI: #1847002-26: Move entity type classes from Drupal\$provider\Plugin\Core\Entity to Drupal\$provider\Entity and comments that follow it are related to this.
Comment #26
tim.plunkettThis is essentially obsolete after #1987298: Shorten directory structure and PSR-0 namespacing for plugins.