Provide a default config_prefix based on entity type ID and provider

Makes sense, two questions...

a) You kept all explicit config prefixes where the current one doesn't match what it would generate by default. Is that just to make the patch easier or is there a more specific reason? Because some of them are pretty weird, especially now with the removed module prefix ("config_prefix = entity" ?!)

b) In light of #2031717: Make entity module not required, can we also think about entities provided by components here? Right now, plugins provided by a component have "Core"/"Component" as the provider, simply because this was never thought about in AnnotatedClassDiscovery::getProviderFromNamespace() and the regex there produces that? Should we have an issue to discuss that and define it if should be core, core.$component (or something else) and another issue to special case that in the config prefix/dependency system (always exists)

1) Yeah, language is weird, also the interaction between entities and language objects, saving them and so on. Agree that it's not for this issue to improve that. Avoiding the repetition by explicitly specifying it is fine with me, I wasn't sure if it's just temporary to keep the patch small or if it's still OK to define a config_prefix that is != entity type ID in the long term, sounds like it is.

2) I guess I agree. I think it is a bit weird because "Core"/"Component" isn't really a thing in that context, just a collection of components (there really shouldn't be any classes without a sub-namespace directly in Core IMHO). This is more obvious for "Component" but not relevant for us anyway as there can't be any component-provided entity types ;)

Per @alexpott, this is blocking #2080823: Create API to discover config entities' soft dependencies and use this to present a confirm form on module uninstall.

Yay! This is exactly how I imagined all of our config/plugin/menu_link/route/whatnot IDs to work since 1-2 years.

It's pointless to have to manually include the extension/provider in declarations, because an extension/provider simply MUST NOT be allowed to create IDs outside of its own namespace.

That unilaterally applies to all declarations we have and the approach of this patch should be the leading example for everything else, IMHO.

My extension system revamp plans already foresee to make "core" an actual "extension"/provider itself, so that should not present a blocker.
(FWIW, "component" is irrelevant, since those are just random PHP libraries.)

Needs a reroll.

I think this at least needs to be updated/mentioned in existing change records and possibly a new one...

Closed #1888218: Default configuration entities provided by a module should include the module name in the machine name as a duplicate of this issue.

I have several questions about the patch that went in here:

1. @alexpott said this was a hard blocker for #2080823: Create API to discover config entities' soft dependencies and use this to present a confirm form on module uninstall. However, there were no failing tests on that issue without this patch, nor any tests added nor changed in this issue. Where is the test coverage for whatever breaks horribly when this patch isn't in? My test in #1888218: Default configuration entities provided by a module should include the module name in the machine name was a different specific bug caused by this general problem that was since fixed in a different issue, so it won't work for test coverage.
2. The names are extremely confusing. Either config_prefix is provider_module.config_entity_type_label_thing, or it's just config_entity_type_label_thing. In this patch, it's used to mean both.
3. The patch changes expectations about the format of the config object name -- that it should be of the format provider_module.config_entity_type_label_thing instead of something.something.whatever.something -- but the new expectation is not codified anywhere; it just happens magically inside a getter. At the minimum, if it's so important, shouldn't it have a unit test for possible inputs?
4. Shouldn't this have a change record?

Yes, that.

#2100203: Make config entities use dots in machine names consistently also needs to be re-evaluated in light of this issue and is possibly another duplicate.

I don't think this needs a change notice - it's a small API addition to a new API. If config_prefix is completely removed we might want one then, but patch here does not break any APIs really.

It does break existing config entities because they need to update their EntityType annotation.

We need to at a minimum update the documentation for ConfigEntityType::$config_prefix and especially ConfigEntityType::getConfigPrefix(), as they currently say exactly the same thing even though they're now completely different things. That should have been included in this patch TBH. Edit: the method docs are actually on EntityType::getConfigPrefix(), oh dear. EntityType has a config-specific method?...

My question is, what's the usecase for the config prefix being different from the configuration entity type ID in the first place? That seems like what would tell us what the property should be called. When would I ever want to call my node_type configuration objects balloon_hat?

Or is that why you're referencing #1862600: Entity type names/IDs are not properly namespaced by owner (e.g., taxonomy.term vs. taxonomy_term), and so config_prefix here is now just a hack to let us create the slightly more aesthetic node.type.page.yml instead of node.node_type.page.yml? Is there no other usecase for it?

It does break existing config entities because they need to update their EntityType annotation.

Yes, in the sense that they'll suddenly be saving views.views.view.viewname.yml and that their views.view.viewname.yml views will no longer work until they fix the config_prefix. This is part of what I was concerned about -- we are changing an expectation that will fail silently when someone updates HEAD. Edit: Another reason it would have been preferable to rename the property or method, so it would actually fail informatively for un-updated code.

@alexpott, change records serve two purposes: documenting before-and-after for upgrading your code, and notifying core and contrib developers of API changes in real time. Coupling these ideas seems great until you have a 3+ year dev cycle with 15 pages of API changes, and I've raised the issue before of how to make them manageable, but I haven't been given time to work on it this quarter. We only use 8.x to 8.x change records for things that are important to notify core and contrib developers of, not every little change. There are "introduced in branch" and "introduced in version" fields for change records, and hypothetically this should cover it, but people don't actually use the "introduced in version" field that way or consistently for alphas.

I could see a value in changing it to be something explicit like "Incompatible with" where we could have < 8.x, < 8.0-alpha9, < 8.1, or whatever, so that people can easily find the change record. File a feature request perhaps?

We could plan to get rid of the change record for this issue if and when we remove config_prefix entirely, and create the change record so that it references both issues.

Yes, in the sense that they'll suddenly be saving views.views.view.viewname.yml and that their views.view.viewname.yml views will no longer work until they fix the config_prefix.

Who is "they" here? We changed the annotation in the View entity class. The only people that will be affected are those providing new config entity *types* outside core, not default config entities themselves. You'll note that not a single YAML file was changed here.