Contrib.social

Fully validatable config schema types: support dynamic types

Open on Drupal.org →
Created on 12 December 2023, about 1 year ago
Updated 5 January 2024, 12 months ago

Problem/Motivation

📌 Configuration schema & required values: add test coverage for `nullable: true` validation support Fixed 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 FullyValidatableConstraint to menus and shortcut sets, neither of which has dynamic types (plugins or third party settings), I think this is fine for now. However, before adding FullyValidatableConstraint to 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 of FullyValidatableConstraint must 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 of FullyValidatableConstraint instead 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:

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

  1. Limit the effects of FullyValidatable to 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.

  2. 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: ~

Remaining tasks

Write tests as soon as 📌 Configuration schema & required values: add test coverage for `nullable: true` validation support Fixed has landed.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

N/A

📌 Task
Status

Closed: duplicate

Version

11.0 🔥

Component
Configuration  →

Last updated 2 days ago

Created by

🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

Live updates comments and jobs are added and updated live.
  • Contributed project blocker

    It denotes an issue that prevents porting of a contributed project to the stable version of Drupal due to missing APIs, regressions, and so on.

  • Needs subsystem maintainer review

    It is used to alert the maintainer(s) of a particular core subsystem that an issue significantly impacts their subsystem, and their signoff is needed (see the governance policy draft for more information). Also, if you use this tag, make sure the issue component is set to the correct subsystem. If an issue significantly impacts more than one subsystem, use needs framework manager review instead.

  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Comments & Activities

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024