Problem/Motivation
#3364109: Configuration schema & required values: add test coverage for `nullable: true` validation support introduced the FullyValidatable constraint, which is a no-op validation constraint used only to signal that a particular config schema type is opting in to stricter validation (to avoid BC breaks/ecosystem disruption, and allow for gradual adoption). See the change record.
Quoting @effulgentsia from #3364109-60: Configuration schema & required values: add test coverage for `nullable: true` validation support:
$root_type_has_opted_in = FALSE; + foreach ($parent->getRoot()->getConstraints() as $constraint) { + if ($constraint instanceof FullyValidatableConstraint) { + $root_type_has_opted_in = TRUE;Since this MR only adds
FullyValidatableConstraintto menus and shortcut sets, neither of which has dynamic types (plugins or third party settings), I think this is fine for now. However, before addingFullyValidatableConstraintto config roots that include any descendants with dynamic types, I think we need to work out how to handle those boundaries. It's impossible for a config root to know that all of the plugins within it are fully validatable, so I think the semantics ofFullyValidatableConstraintmust be limited to only its descendants with static types. For an element that has an ancestor with a dynamic type, we need to use that ancestor's presence or absence ofFullyValidatableConstraintinstead of checking->getRoot(). But again, we can punt that to a follow-up since that's not surfaced for menus and shortcuts.
Examples of where this occurs:
- Text Editor config entity type
-
editor.editor.*: type: config_entity label: 'Text editor' mapping: … settings: type: editor.settings.[%parent.editor] image_upload: type: editor.image_upload_settings.[status] - Block config entity type
-
block.block.*: type: config_entity label: 'Block' mapping: … settings: type: block.settings.[%parent.plugin] … - Action config entity type
-
system.action.*: type: config_entity label: 'System action' mapping: … configuration: type: action.configuration.[%parent.plugin] - Recursively: in a Text Editor's editor-plugin-specific settings:
-
editor.settings.ckeditor5: type: mapping label: 'CKEditor 5 settings' mapping: … plugins: type: sequence label: 'Plugins' orderby: key sequence: type: ckeditor5.plugin.[%key] - In simple config — no examples in core, but for example the CDN contrib module:
-
cdn.conditions: type: sequence label: 'Conditions' sequence: type: cdn.condition.[%key]
If the consequences of
config_test.types.fully_validatable:
type: config_test.types
constraints:
FullyValidatable: ~
would apply not just to that config schema type, but in to each of the different subbranches for each dynamic type (see \Drupal\Core\Config\TypedConfigManager::replaceVariable())…
… then that would mean that for example the developer of a block plugin could start getting bug reports, because validation errors start appearing for his block plugin's settings:
- even though the Block config entity type may declare itself fully validatable, that does not mean that every block plugin's own settings are fully validatable
- even though the Text Editor config entity type may declare itself fully validatable, that does not mean that every text editor plugin's own settings are fully validatable
- (recursive case: branches in branches) even though the CKEditor 5 text editor plugin may declare itself fully validatable, that does not mean that every CKEditor 5 plugin's own settings are fully validatable
Steps to reproduce
See test coverage.
Proposed resolution
- Limit the effects of
FullyValidatableto the boundaries of its authority.In other words: if
block.block.*declares itself fully validatable, that means that all its property paths are fully validatable, up until the point a dynamic type is used, because that is the only way to allow for contributed/custom modules to provide additional config property paths of their own. - Allow a dynamic type to also opt in to being fully validatable.
In other words:
- Block plugins (
block.settings.local_tasks_block,block.settings.system_branding_block,block.settings.system_menu_block:*, etc.) - Text Editor plugins (
editor.settings.ckeditor5,editor.settings.unicorn, etc.) - CKEditor 5 plugins (
ckeditor5.plugin.ckeditor5_language,ckeditor5.plugin.ckeditor5_heading, etc.)
… all could opt in to stricter validation if each of them also specified:
constraints: FullyValidatable: ~ - Block plugins (
Remaining tasks
Write tests as soon as #3364109: Configuration schema & required values: add test coverage for `nullable: true` validation support has landed.
User interface changes
None.
API changes
None.
Data model changes
None.
Release notes snippet
N/A
Comments
Comment #2
wim leersFix HTML.
Comment #3
wim leersComment #4
wim leersOver at #3364108-91: Configuration schema & required keys, I'm fixing this as part of that issue. I think that will be easier. Let's find if others agree.
Comment #5
wim leersOthers agreed! This landed: #3364108-99: Configuration schema & required keys. 🥳