Problem/Motivation
The #2212069: Non-English Drupal sites get default configuration in English, edited in English, originals not actually used if translated patch introduced a way to add langcode automatically to top level configuration names. @alexpott expressed concern that that adds an implicit assumption about configuration schema that is not explicitly in the type, and we should introduce explicit configuration language codes instead.
Proposed resolution
1. Add a config_object base type. This means that for a configuration name, developers would either define it using config_object or config_entity. This makes it REALLY easy to spot definitions for high level config in schemas.
2. Use the config_object base type for top level configuration (which are not config entities). Remove explicit langcodes from the ones now extending config_object.
3. Fix language.mappings which used a top level sequence type (and therefore incompatible with adding a top level langcode).
4. Remove breakpoint schemas which are not even for configuration, they are for plugin data (not stored within configuration).
5. Fix tests.
Remaining tasks
Review. Commit.
User interface changes
None.
API changes
Configuration objects should now extend from config_object
.
Beta phase evaluation
Issue category | Bug because config files cannot be parsed by their schema alone, unless the langcode key is explicitly defined on them. |
---|---|
Issue priority | Major because the extent of changes required. |
Disruption | Disruption in how configuration schemas need to be defined. |
Comment | File | Size | Author |
---|---|---|---|
#30 | interdiff.txt | 2.23 KB | Gábor Hojtsy |
#30 | 2459971-30.patch | 40.69 KB | Gábor Hojtsy |
#27 | interdiff.txt | 18 KB | Gábor Hojtsy |
#27 | 2459971-27.patch | 38.72 KB | Gábor Hojtsy |
#26 | 2459971-26.patch | 21.21 KB | Gábor Hojtsy |
Comments
Comment #1
Gábor HojtsyComment #2
Gábor HojtsyComment #3
Gábor HojtsyNot sure about the best way to figure out the top level types. This will definitely fail hard and show us some.
Comment #4
Gábor HojtsyComment #6
Gábor HojtsyComment #7
Gábor HojtsyWith surprisingly little changes, we can eliminate all explicit
langcode:
keys except the newly addedconfig_object
type and one in migrate which does not seem to be a top level type. Usingconfig_object
where langcode was present before and in top level definitions of system and user. Will likely go on to produce more fails from other areas.Comment #9
Gábor HojtsyComment #11
Gábor HojtsyFix ConfigImportUITest and ConfigSchemaTest by updating the expectations (label and key order).
Fix the missing config object base type for menu overrides.
Comment #12
Gábor HojtsyConfigImporterTest needed the same fix of item ordering, so resaving based on the schema will not cause changes.
Comment #15
Gábor HojtsyConfigImportUITest still needed some keys reordered.
The field.settings is a config object, fixed schema.
Comment #18
Gábor HojtsyComment #19
Gábor HojtsyIn the CMI meeting review, @alexpott asked to not move the position of the langcode of the config entities, so the change is not that dramatic. We can keep the override in config_entity for that in the place we expect it.
Comment #21
Gábor HojtsyContinue with the file fixes in file.schema.
Comment #23
Gábor HojtsySo from the results in #19, reordering on merge would not take it, so we need to define config_entity as a simple mapping with the langcode explicitly if we want to keep the ordering.
Comment #26
Gábor HojtsyNext up is locale settings.
Comment #27
Gábor HojtsyWent through all the schema files and compared entries to config objects. Found a whole bunch more.
Also found breakpoint schemas defined. But the breakpoint YAML files are not in config, they are in the theme/module. So they have no use in the config schema. (Removing here because their root type was sequence which would not be playing nice with the mapping assumed in config anyway).
Comment #28
Gábor HojtsyUpdating issue summary.
Comment #30
Gábor HojtsyAdd the missing langcode keys in the config schema test.
Comment #31
Gábor HojtsyPatch is done and complete as far as I am concerned. Please review.
Comment #32
vijaycs85Changes looks good to me.
just a thought: I see the new type config_object is only applied for non-config-entity configs. Why don't we add to config entities as well?
Comment #33
Gábor Hojtsy@vijaycs85: Thanks for the review. I had an encounter with trying to use config_object as a base type for config_entity from #7 to #19. @alexpott asked to keep the order of config entity keys as uuid and THEN langcode, which is not possible with this base typing due to how PHP merges arrays. (Even if we include the langcode definition in the config entity type at a different position, see #23).
Comment #34
Gábor HojtsyAlso added a pretty detailed change notice draft at https://www.drupal.org/node/2460751
Comment #35
webchickThis feels alexpott-ish.
Comment #36
alexpottI think deprecating the param is the right way to go - we're giving d8 contrib a chance to upgrade by leaving the code in - we should file a follow up to remove this before 8.0.0 because I think there are many advantages to have all top level config objects extend from the same type. It makes adding any other global keys super simple. I can't think of another other than langcode but you never know. Committed 5b19144 and pushed to 8.0.x. Thanks!
Comment #38
Gábor HojtsyThanks! Opened #2462889: Remove TypedConfigManagerInterface::getDefinition()'s $is_config_name before Drupal 8.0.0.