Drupal
/
drupal
mirror of https://git.drupalcode.org/project/drupal.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Issue #3364108 by Wim Leers, borisson_, phenaproxima, bircher: Configuration schema & required keys

merge-requests/6028/head
effulgentsia 2024-01-04 10:14:30 -08:00
parent b7f45944ad
commit 7b49d8700c
18 changed files with 653 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
core/config/schema/core.data_types.schema.yml
View File

@ -51,7 +51,8 @@ mapping:
definition_class: '\Drupal\Core\TypedData\MapDataDefinition'
mapping: {}
constraints:
# By default, only allow the explicitly listed mapping keys.
# By default, allow the explicitly listed mapping keys, and require their
# presence unless `requiredKey: false` is specified.
ValidKeys: '<infer>'
sequence:
label: Sequence

6
core/config/schema/core.entity.schema.yml
View File

@ -83,6 +83,9 @@ field_formatter:
type: field.formatter.settings.[%parent.type]
label: 'Settings'
third_party_settings:
# Third party settings are always optional: they're an optional extension
# point.
requiredKey: false
type: sequence
label: 'Third party settings'
sequence:
@ -138,6 +141,9 @@ core.entity_form_display.*.*.*:
type: field.widget.settings.[%parent.type]
label: 'Settings'
third_party_settings:
# Third party settings are always optional: they're an optional extension
# point.
requiredKey: false
type: sequence
label: 'Third party settings'
sequence:

10
core/lib/Drupal/Core/Config/Schema/SequenceDataDefinition.php
View File

@ -29,7 +29,15 @@ class SequenceDataDefinition extends ListDataDefinition {
* {@inheritdoc}
*/
public function getDataType() {
return 'sequence';
// TRICKY: this class extends ListDataDefinition, which always returns a
// hardcoded "list". But this is a typed data type used in config schemas,
// and hence many subtypes of it exists. The actual concrete subtype must
// always be returned.
// This effectively means skipping the parent implementation and matching
// the grandparent implementation.
// @see \Drupal\Core\TypedData\ListDataDefinition::setDataType()
// @see \Drupal\Core\TypedData\ListDataDefinition::getDataType()
return $this->definition['type'];
}
}

81
core/lib/Drupal/Core/Config/TypedConfigManager.php
View File

@ -7,9 +7,12 @@ use Drupal\Component\Utility\NestedArray;
use Drupal\Core\Cache\CacheBackendInterface;
use Drupal\Core\Config\Schema\ConfigSchemaAlterException;
use Drupal\Core\Config\Schema\ConfigSchemaDiscovery;
use Drupal\Core\Config\Schema\SequenceDataDefinition;
use Drupal\Core\DependencyInjection\ClassResolverInterface;
use Drupal\Core\Config\Schema\Undefined;
use Drupal\Core\Extension\ModuleHandlerInterface;
use Drupal\Core\TypedData\MapDataDefinition;
use Drupal\Core\TypedData\TraversableTypedDataInterface;
use Drupal\Core\TypedData\TypedDataManager;
use Drupal\Core\Validation\Plugin\Validation\Constraint\FullyValidatableConstraint;
@ -138,6 +141,7 @@ class TypedConfigManager extends TypedDataManager implements TypedConfigManagerI
// @see https://www.drupal.org/node/3364109
// @see \Drupal\Core\TypedData\TypedDataManager::getDefaultConstraints()
if ($parent) {
$root_type = $parent->getRoot()->getDataDefinition()->getDataType();
$root_type_has_opted_in = FALSE;
foreach ($parent->getRoot()->getConstraints() as $constraint) {
if ($constraint instanceof FullyValidatableConstraint) {
@ -145,6 +149,37 @@ class TypedConfigManager extends TypedDataManager implements TypedConfigManagerI
break;
}
}
// If this is a dynamically typed property path, then not only must the
// (absolute) root type be considered, but also the (relative) static root
// type: the resolved type.
// For example, `block.block.*:settings` has a dynamic type defined:
// `block.settings.[%parent.plugin]`, but `block.block.*:plugin` does not.
// Consequently, the value at the `plugin` property path depends only on
// the `block.block.*` config schema type and hence only that config
// schema type must have the `FullyValidatable` constraint, because it
// defines which value are required.
// In contrast, the `block.block.*:settings` property path depends on
// whichever dynamic type `block.settings.[%parent.plugin]` resolved to,
// to be able to know which values are required. Therefore that resolved
// type determines which values are required and whether it is fully
// validatable.
// So for example the `block.settings.system_branding_block` config schema
// type would also need to have the `FullyValidatable` constraint to
// consider its schema-defined keys to require values:
// - use_site_logo
// - use_site_name
// - use_site_slogan
$static_type_root = TypedConfigManager::getStaticTypeRoot($parent);
$static_type_root_type = $static_type_root->getDataDefinition()->getDataType();
if ($root_type !== $static_type_root_type) {
$root_type_has_opted_in = FALSE;
foreach ($static_type_root->getConstraints() as $c) {
if ($c instanceof FullyValidatableConstraint) {
$root_type_has_opted_in = TRUE;
break;
}
}
}
if ($root_type_has_opted_in) {
$data_definition->setRequired(!isset($data_definition['nullable']) || $data_definition['nullable'] === FALSE);
}
@ -153,6 +188,52 @@ class TypedConfigManager extends TypedDataManager implements TypedConfigManagerI
return $data_definition;
}
/**
* Gets the static type root for a config schema object.
*
* @param \Drupal\Core\TypedData\TraversableTypedDataInterface $object
* A config schema object to get the static type root for.
*
* @return \Drupal\Core\TypedData\TraversableTypedDataInterface
* The ancestral config schema object at which the static type root lies:
* either the first ancestor with a dynamic type (for example:
* `block.block.*:settings`, which has the `block.settings.[%parent.plugin]`
* type) or the (absolute) root of the config object (in this example:
* `block.block.*`).
*/
public static function getStaticTypeRoot(TraversableTypedDataInterface $object): TraversableTypedDataInterface {
$root = $object->getRoot();
$static_type_root = NULL;
while ($static_type_root === NULL && $object !== $root) {
// Use the parent data definition to determine the type of this mapping
// (including the dynamic placeholders). For example:
// - `editor.settings.[%parent.editor]`
// - `editor.image_upload_settings.[status]`.
$parent_data_def = $object->getParent()->getDataDefinition();
$original_mapping_type = match (TRUE) {
$parent_data_def instanceof MapDataDefinition => $parent_data_def->toArray()['mapping'][$object->getName()]['type'],
$parent_data_def instanceof SequenceDataDefinition => $parent_data_def->toArray()['sequence']['type'],
default => throw new \LogicException('Invalid config schema detected.'),
};
// If this mapping's type was dynamically defined, then this is the static
// type root inside which all types are statically defined.
if (str_contains($original_mapping_type, ']')) {
$static_type_root = $object;
break;
}
$object = $object->getParent();
}
// Either the discovered static type root is not the actual root, or no
// static type root was found and it is the root config object.
assert(($static_type_root !== NULL && $static_type_root !== $root) || ($static_type_root === NULL && $object->getParent() === NULL));
return $static_type_root ?? $root;
}
/**
* Determines the typed config type for a plugin ID.
*

14
core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/ValidKeysConstraint.php
View File

@ -34,6 +34,20 @@ class ValidKeysConstraint extends Constraint {
*/
public string $dynamicInvalidKeyMessage = "'@key' is an unknown key because @dynamic_type_property_path is @dynamic_type_property_value (see config schema type @resolved_dynamic_type).";
/**
* The error message if a key is missing.
*
* @var string
*/
public string $missingRequiredKeyMessage = "'@key' is a required key.";
/**
* The error message if a dynamically required key is missing.
*
* @var string
*/
public string $dynamicMissingRequiredKeyMessage = "'@key' is a required key because @dynamic_type_property_path is @dynamic_type_property_value (see config schema type @resolved_dynamic_type).";
/**
* The error message if the array being validated is a list.
*

80
core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/ValidKeysConstraintValidator.php
View File

@ -6,6 +6,7 @@ namespace Drupal\Core\Validation\Plugin\Validation\Constraint;
use Drupal\Core\Config\Schema\Mapping;
use Drupal\Core\Config\Schema\SequenceDataDefinition;
use Drupal\Core\Config\TypedConfigManager;
use Drupal\Core\TypedData\MapDataDefinition;
use Symfony\Component\Validator\Constraint;
use Symfony\Component\Validator\ConstraintValidator;
@ -89,6 +90,85 @@ class ValidKeysConstraintValidator extends ConstraintValidator {
$this->context->addViolation($constraint->dynamicInvalidKeyMessage, ['@key' => $key] + self::getDynamicMessageParameters($mapping));
}
}
// All keys are optional by default (meaning they can be omitted). This is
// unintuitive and contradicts Drupal core's documentation.
// @see https://www.drupal.org/node/2264179
// To gradually evolve configuration schemas in the Drupal ecosystem to be
// validatable, this needs to be clarified in a non-disruptive way. Any
// config schema type definition — that is, a top-level entry in a
// *.schema.yml file — can opt into stricter behavior, whereby a key is
// required unless it specifies `requiredKey: false`, by adding
// `FullyValidatable` as a top-level validation constraint.
// @see https://www.drupal.org/node/3364108
// @see https://www.drupal.org/node/3364109
$root_type = $this->context->getObject()->getRoot()->getDataDefinition()->getDataType();
$root_type_has_opted_in = FALSE;
foreach ($this->context->getObject()->getRoot()->getConstraints() as $c) {
if ($c instanceof FullyValidatableConstraint) {
$root_type_has_opted_in = TRUE;
break;
}
}
// Return early: do not generate validation errors for keys that are
// required.
if (!$root_type_has_opted_in) {
return;
}
// If this is a dynamically typed property path, then not only must the
// (absolute) root type be considered, but also the (relative) static root
// type: the resolved type.
// For example, `block.block.*:settings` has a dynamic type defined:
// `block.settings.[%parent.plugin]`, but `block.block.*:plugin` does not.
// Consequently, the value at the `plugin` property path depends only on the
// `block.block.*` config schema type and hence only that config schema type
// must have the `FullyValidatable` constraint, because it defines which
// keys are required.
// In contrast, the `block.block.*:settings` property path depends on
// whichever dynamic type `block.settings.[%parent.plugin]` resolved to, to
// be able to know which keys are required. Therefore that resolved type
// determines which keys are required and whether it is fully validatable.
// So for example the `block.settings.system_branding_block` config schema
// type would also need to have the `FullyValidatable` constraint to
// consider its schema-defined keys to be required:
// - use_site_logo
// - use_site_name
// - use_site_slogan
$static_type_root = TypedConfigManager::getStaticTypeRoot($this->context->getObject());
$static_type_root_type = $static_type_root->getDataDefinition()->getDataType();
if ($root_type !== $static_type_root_type) {
$root_type_has_opted_in = FALSE;
foreach ($static_type_root->getConstraints() as $c) {
if ($c instanceof FullyValidatableConstraint) {
$root_type_has_opted_in = TRUE;
break;
}
}
// Return early: do not generate validation errors for keys that are
// required.
if (!$root_type_has_opted_in) {
return;
}
}
$required_keys = array_intersect($mapping->getRequiredKeys(), $constraint->getAllowedKeys($this->context));
// Statically required: same principle as for "statically valid" above, but
// this time restricted to the subset of statically valid keys that do not
// have `requiredKey: false`.
$statically_required_keys = array_diff($required_keys, $all_dynamically_valid_keys);
$missing_keys = array_diff($statically_required_keys, array_keys($value));
foreach ($missing_keys as $key) {
$this->context->addViolation($constraint->missingRequiredKeyMessage, ['@key' => $key]);
}
// Dynamically required: same principle as for "dynamically valid" above,
// but this time restricted to the subset of dynamically valid keys that do
// not have `requiredKey: false`.
$dynamically_required_keys = array_intersect($required_keys, $all_dynamically_valid_keys);
$missing_dynamically_required_keys = array_diff($dynamically_required_keys, array_keys($value));
foreach ($missing_dynamically_required_keys as $key) {
$this->context->addViolation($constraint->dynamicMissingRequiredKeyMessage, ['@key' => $key] + self::getDynamicMessageParameters($mapping));
}
}
/**

16
core/modules/config/tests/config_schema_test/config_schema_test.module
View File

@ -20,4 +20,20 @@ function config_schema_test_config_schema_info_alter(&$definitions) {
if (isset($definitions['config_schema_test.hook'])) {
$definitions['config_schema_test.hook']['additional_metadata'] = 'new schema info';
}
// @see \Drupal\KernelTests\Core\TypedData\ValidKeysConstraintValidatorTest
if (\Drupal::state()->get('config_schema_test_block_fully_validatable')) {
$definitions['block.block.*']['constraints']['FullyValidatable'] = NULL;
}
else {
unset($definitions['block.block.*']['constraints']);
}
// @see \Drupal\Tests\node\Kernel\NodeTypeValidationTest::testThirdPartySettingsMenuUi()
if (\Drupal::state()->get('config_schema_test_menu_ui_third_party_settings_fully_validatable')) {
$definitions['node.type.*.third_party.menu_ui']['constraints']['FullyValidatable'] = NULL;
}
else {
unset($definitions['node.type.*.third_party.menu_ui']['constraints']);
}
}

4
core/modules/config/tests/config_test/config/install/config_test.types.yml
View File

@ -23,3 +23,7 @@ nullable_int: null
nullable_octal: ~
nullable_string: null
nullable_string_int: ~
# To test required vs optional keys.
mapping_with_only_required_keys: {}
mapping_with_some_required_keys: {}
mapping_with_only_optional_keys: {}

33
core/modules/config/tests/config_test/config/schema/config_test.schema.yml
View File

@ -90,6 +90,13 @@ config_test.types:
type: integer
label: 'Integer'
octal:
# Symfony 5.1's YAML parser issues a deprecation when reading octal with a
# leading zero, to comply with YAML 1.2. However PECL YAML is still YAML 1.1
# compliant.
# @see core/modules/config/tests/config_test/config/install/config_test.types.yml
# @see \Drupal\KernelTests\Core\Config\ConfigCRUDTest::testDataTypes()
# @todo Mark as required again in https://www.drupal.org/project/drupal/issues/3205480
requiredKey: false
type: integer
label: 'Octal'
string:
@ -142,8 +149,32 @@ config_test.types:
type: string
label: 'Nullable string integer'
nullable: true
# To test required vs optional keys.
mapping_with_only_required_keys:
type: mapping
label: 'Mapping with only required keys'
mapping:
north: { type: string }
east: { type: string }
south: { type: string }
west: { type: string }
mapping_with_some_required_keys:
type: mapping
label: 'Mapping with only required keys'
mapping:
north: { type: string }
east: { type: string, requiredKey: false }
south: { type: string }
west: { type: string, requiredKey: false }
mapping_with_only_optional_keys:
type: mapping
label: 'Mapping with only optional keys'
mapping:
north: { type: string, requiredKey: false }
east: { type: string, requiredKey: false }
south: { type: string, requiredKey: false }
west: { type: string, requiredKey: false }
# cspell:ignore validatable
config_test.types.fully_validatable:
type: config_test.types
constraints:

13
core/modules/editor/tests/src/Kernel/EditorValidationTest.php
View File

@ -92,4 +92,17 @@ class EditorValidationTest extends ConfigEntityValidationTestBase {
]);
}
/**
* {@inheritdoc}
*/
public function testRequiredPropertyKeysMissing(?array $additional_expected_validation_errors_when_missing = NULL): void {
parent::testRequiredPropertyKeysMissing([
'dependencies' => [
// @see ::testInvalidDependencies()
// @see \Drupal\Core\Config\Plugin\Validation\Constraint\RequiredConfigDependenciesConstraintValidator
'' => 'This text editor requires a text format.',
],
]);
}
}

16
core/modules/field/tests/src/Kernel/Entity/FieldConfigValidationTest.php
View File

@ -98,13 +98,27 @@ class FieldConfigValidationTest extends FieldStorageConfigValidationTest {
parent::testImmutableProperties($valid_values);
}
/**
* {@inheritdoc}
*/
public function testRequiredPropertyKeysMissing(?array $additional_expected_validation_errors_when_missing = NULL): void {
parent::testRequiredPropertyKeysMissing([
'dependencies' => [
// @see ::testInvalidDependencies()
// @see \Drupal\Core\Config\Plugin\Validation\Constraint\RequiredConfigDependenciesConstraintValidator
'' => 'This field requires a field storage.',
],
]);
}
/**
* {@inheritdoc}
*/
public function testRequiredPropertyValuesMissing(?array $additional_expected_validation_errors_when_missing = NULL): void {
parent::testRequiredPropertyValuesMissing([
'dependencies' => [
// @see testInvalidDependencies()
// @see ::testInvalidDependencies()
// @see \Drupal\Core\Config\Plugin\Validation\Constraint\RequiredConfigDependenciesConstraintValidator
'' => 'This field requires a field storage.',
],
]);

21
core/modules/node/tests/src/Kernel/NodeTypeValidationTest.php
View File

@ -76,4 +76,25 @@ class NodeTypeValidationTest extends ConfigEntityValidationTestBase {
]);
}
/**
* @testWith [true, {"third_party_settings.menu_ui": "'parent' is a required key."}]
* [false, {}]
*/
public function testThirdPartySettingsMenuUi(bool $third_party_settings_menu_ui_fully_validatable, array $expected_validation_errors): void {
$this->enableModules(['menu_ui']);
// Set or unset the `FullyValidatable` constraint on
// `node.type.*.third_party.menu_ui`.
$this->enableModules(['config_schema_test']);
\Drupal::state()->set('config_schema_test_menu_ui_third_party_settings_fully_validatable', $third_party_settings_menu_ui_fully_validatable);
$this->container->get('kernel')->rebuildContainer();
$this->entity = $this->createContentType();
// @see system.menu.main.yml
$this->installConfig(['system']);
$this->entity->setThirdPartySetting('menu_ui', 'available_menus', ['main']);
$this->assertValidationErrors($expected_validation_errors);
}
}

4
core/modules/system/config/schema/system.schema.yml
View File

@ -373,6 +373,8 @@ system.advisories:
block.settings.system_branding_block:
type: block_settings
label: 'Branding block'
constraints:
FullyValidatable: ~
mapping:
use_site_logo:
type: boolean
@ -401,6 +403,8 @@ block.settings.system_menu_block:*:
block.settings.local_tasks_block:
type: block_settings
label: 'Tabs block'
constraints:
FullyValidatable: ~
mapping:
primary:
type: boolean

8
core/tests/Drupal/KernelTests/Config/Schema/MappingTest.php
View File

@ -133,7 +133,13 @@ class MappingTest extends KernelTestBase {
case 'editor.editor.funky':
$this->enableModules(['filter', 'editor', 'ckeditor5']);
FilterFormat::create(['format' => 'funky', 'name' => 'Funky'])->save();
Editor::create(['format' => 'funky', 'editor' => 'ckeditor5'])->save();
Editor::create([
'format' => 'funky',
'editor' => 'ckeditor5',
'image_upload' => [
'status' => FALSE,
],
])->save();
break;
case 'field.field.node.forum.comment_forum':

3
core/tests/Drupal/KernelTests/Core/Config/ConfigCRUDTest.php
View File

@ -294,6 +294,9 @@ class ConfigCRUDTest extends KernelTestBase {
'nullable_octal' => NULL,
'nullable_string' => NULL,
'nullable_string_int' => NULL,
'mapping_with_only_required_keys' => [],
'mapping_with_some_required_keys' => [],
'mapping_with_only_optional_keys' => [],
];
$data = ['_core' => ['default_config_hash' => Crypt::hashBase64(serialize($data))]] + $data;
$this->assertSame($data, $config->get());

112
core/tests/Drupal/KernelTests/Core/Config/ConfigEntityValidationTestBase.php
View File

@ -4,11 +4,14 @@ namespace Drupal\KernelTests\Core\Config;
use Drupal\Component\Utility\NestedArray;
use Drupal\Core\Config\Entity\ConfigEntityInterface;
use Drupal\Core\Config\Schema\Mapping;
use Drupal\Core\Config\TypedConfigManagerInterface;
use Drupal\Core\Entity\EntityWithPluginCollectionInterface;
use Drupal\Core\Entity\Plugin\DataType\ConfigEntityAdapter;
use Drupal\Core\Language\LanguageInterface;
use Drupal\Core\Language\LanguageManager;
use Drupal\Core\TypedData\Plugin\DataType\LanguageReference;
use Drupal\Core\TypedData\TypedDataInterface;
use Drupal\Core\Validation\Plugin\Validation\Constraint\FullyValidatableConstraint;
use Drupal\KernelTests\KernelTestBase;
use Drupal\language\Entity\ConfigurableLanguage;
@ -47,6 +50,22 @@ abstract class ConfigEntityValidationTestBase extends KernelTestBase {
*/
protected bool $hasLabel = TRUE;
/**
* The config entity mapping properties with >=1 required keys.
*
* All top-level properties of a config entity are guaranteed to be defined
* (since they are defined as properties on the corresponding PHP class). That
* is why they can never trigger "required key" validation errors. Only for
* non-top-level properties can such validation errors be triggered, and hence
* that is only possible on top-level properties of `type: mapping`.
*
* @var string[]
* @see \Drupal\Core\Config\Entity\ConfigEntityType::getPropertiesToExport()
* @see ::testRequiredPropertyKeysMissing()
* @see \Drupal\Core\Validation\Plugin\Validation\Constraint\ValidKeysConstraintValidator
*/
protected static array $propertiesWithRequiredKeys = [];
/**
* The config entity properties whose values are optional (set to NULL).
*
@ -461,6 +480,51 @@ abstract class ConfigEntityValidationTestBase extends KernelTestBase {
}
}
/**
* A property that is required must have a value (i.e. not NULL).
*
* @param string[]|null $additional_expected_validation_errors_when_missing
* Some required config entity properties have additional validation
* constraints that cause additional messages to appear. Keys must be
* config entity properties, values must be arrays as expected by
* ::assertValidationErrors().
*
* @todo Remove this optional parameter in https://www.drupal.org/project/drupal/issues/2820364#comment-15333069
*
* @return void
*/
public function testRequiredPropertyKeysMissing(?array $additional_expected_validation_errors_when_missing = NULL): void {
$config_entity_properties = array_keys($this->entity->getEntityType()->getPropertiesToExport());
if (!empty(array_diff(array_keys($additional_expected_validation_errors_when_missing ?? []), $config_entity_properties))) {
throw new \LogicException(sprintf('The test %s lists `%s` in $additional_expected_validation_errors_when_missing but it is not a property of the `%s` config entity type.',
get_called_class(),
implode(', ', array_diff(array_keys($additional_expected_validation_errors_when_missing), $config_entity_properties)),
$this->entity->getEntityTypeId(),
));
}
$mapping_properties = array_keys(array_filter(
ConfigEntityAdapter::createFromEntity($this->entity)->getProperties(FALSE),
fn (TypedDataInterface $v) => $v instanceof Mapping
));
$required_property_keys = $this->getRequiredPropertyKeys();
if (!$this->isFullyValidatable()) {
$this->assertEmpty($required_property_keys, 'No keys can be required when a config entity type is not fully validatable.');
}
$original_entity = clone $this->entity;
foreach ($mapping_properties as $property) {
$this->entity = clone $original_entity;
$this->entity->set($property, []);
$expected_validation_errors = array_key_exists($property, $required_property_keys)
? [$property => $required_property_keys[$property]]
: [];
$this->assertValidationErrors(($additional_expected_validation_errors_when_missing[$property] ?? []) + $expected_validation_errors);
}
}
/**
* A property that is required must have a value (i.e. not NULL).
*
@ -560,6 +624,54 @@ abstract class ConfigEntityValidationTestBase extends KernelTestBase {
return FALSE;
}
/**
* Determines the config entity mapping properties with required keys.
*
* This refers only to the top-level properties of the config entity which are expected to be mappings, and of those mappings, only the ones which have required keys.
*
* @return string[]
* An array of key-value pairs, with:
* - keys: names of the config entity properties which are mappings that
* contain required keys.
* - values: the corresponding expected validation error message.
*/
protected function getRequiredPropertyKeys(): array {
// If a config entity type is not fully validatable, no mapping property
// keys are required.
if (!$this->isFullyValidatable()) {
return [];
}
$config_entity_properties = array_keys($this->entity->getEntityType()
->getPropertiesToExport());
// Otherwise, all mapping property keys are required except for those marked
// optional. Rather than inspecting config schema, require authors of tests
// to explicitly list optional properties in a `propertiesWithRequiredKeys`
// property on this class.
// @see \Drupal\KernelTests\Config\Schema\MappingTest::testMappingInterpretation()
$class = static::class;
$properties_with_required_keys = [];
while ($class) {
if (property_exists($class, 'propertiesWithRequiredKeys')) {
$properties_with_required_keys += $class::$propertiesWithRequiredKeys;
}
$class = get_parent_class($class);
}
// Guide developers when $propertiesWithRequiredKeys does not contain
// sensible values.
if (!empty(array_diff(array_keys($properties_with_required_keys), $config_entity_properties))) {
throw new \LogicException(sprintf('The %s test class lists %s in $propertiesWithRequiredKeys but it is not a property of the %s config entity type.',
get_called_class(),
implode(', ', array_diff(array_keys($properties_with_required_keys), $config_entity_properties)),
$this->entity->getEntityTypeId()
));
}
return $properties_with_required_keys;
}
/**
* Determines the config entity properties with optional values.
*

62
core/tests/Drupal/KernelTests/Core/Config/SchemaCheckTraitTest.php
View File

@ -42,7 +42,7 @@ class SchemaCheckTraitTest extends KernelTestBase {
*
* @dataProvider providerCheckConfigSchema
*/
public function testCheckConfigSchema(string $type_to_validate_against, bool $validate_constraints, array|bool $nulled_expectations, array $expectations) {
public function testCheckConfigSchema(string $type_to_validate_against, bool $validate_constraints, array|bool $nulled_expectations, array|bool $no_data_expectations, array $expectations) {
// Test a non existing schema.
$ret = $this->checkConfigSchema($this->typedConfig, 'config_schema_test.no_schema', $this->config('config_schema_test.no_schema')->get());
$this->assertFalse($ret);
@ -64,6 +64,12 @@ class SchemaCheckTraitTest extends KernelTestBase {
$ret = $this->checkConfigSchema($this->typedConfig, $type_to_validate_against, $config_data, $validate_constraints);
$this->assertEquals($expectations, $ret);
// Omit all data, this should trigger validation errors for required keys
// missing.
$config_data = [];
$ret = $this->checkConfigSchema($this->typedConfig, $type_to_validate_against, $config_data, $validate_constraints);
$this->assertEquals($no_data_expectations, $ret);
}
public function providerCheckConfigSchema(): array {
@ -75,6 +81,9 @@ class SchemaCheckTraitTest extends KernelTestBase {
// @see \Drupal\Core\Config\ConfigInstaller::createConfiguration()
'config_test.types:_core' => 'variable type is NULL but applied schema class is Drupal\Core\Config\Schema\Mapping',
'config_test.types:array' => 'variable type is NULL but applied schema class is Drupal\Core\Config\Schema\Sequence',
'config_test.types:mapping_with_only_required_keys' => 'variable type is NULL but applied schema class is Drupal\Core\Config\Schema\Mapping',
'config_test.types:mapping_with_some_required_keys' => 'variable type is NULL but applied schema class is Drupal\Core\Config\Schema\Mapping',
'config_test.types:mapping_with_only_optional_keys' => 'variable type is NULL but applied schema class is Drupal\Core\Config\Schema\Mapping',
];
$expected_storage_type_check_errors = [
'config_test.types:new_key' => 'missing schema',
@ -93,19 +102,24 @@ class SchemaCheckTraitTest extends KernelTestBase {
'config_test.types',
FALSE,
$expected_storage_null_check_errors,
TRUE,
$expected_storage_type_check_errors,
],
'config_test.types, with validation' => [
'config_test.types',
TRUE,
$expected_storage_null_check_errors,
TRUE,
$expected_storage_type_check_errors + $expected_validation_errors,
],
];
// Test that if the exact same schema is reused but now has the constraint
// "FullyValidatable" specified at the top level, that `NULL` values are now
// trigger validation errors, except when `nullable: true` is set.
// "FullyValidatable" specified at the top level, that:
// 1. `NULL` values now trigger validation errors, except when
// `nullable: true` is set.
// 2. missing required keys now trigger validation errors, except when
// `requiredKey: false` is set.
// @see `type: config_test.types.fully_validatable`
// @see core/modules/config/tests/config_test/config/schema/config_test.schema.yml
$expected_storage_null_check_errors = [
@ -114,6 +128,9 @@ class SchemaCheckTraitTest extends KernelTestBase {
// @see \Drupal\Core\Config\ConfigInstaller::createConfiguration()
'config_test.types.fully_validatable:_core' => 'variable type is NULL but applied schema class is Drupal\Core\Config\Schema\Mapping',
'config_test.types.fully_validatable:array' => 'variable type is NULL but applied schema class is Drupal\Core\Config\Schema\Sequence',
'config_test.types.fully_validatable:mapping_with_only_required_keys' => 'variable type is NULL but applied schema class is Drupal\Core\Config\Schema\Mapping',
'config_test.types.fully_validatable:mapping_with_some_required_keys' => 'variable type is NULL but applied schema class is Drupal\Core\Config\Schema\Mapping',
'config_test.types.fully_validatable:mapping_with_only_optional_keys' => 'variable type is NULL but applied schema class is Drupal\Core\Config\Schema\Mapping',
];
$expected_storage_type_check_errors = [
'config_test.types.fully_validatable:new_key' => 'missing schema',
@ -125,6 +142,7 @@ class SchemaCheckTraitTest extends KernelTestBase {
'config_test.types.fully_validatable',
FALSE,
$expected_storage_null_check_errors,
TRUE,
$expected_storage_type_check_errors,
],
'config_test.types.fully_validatable, with validation' => [
@ -141,8 +159,44 @@ class SchemaCheckTraitTest extends KernelTestBase {
'[int] This value should not be null.',
'[string] This value should not be null.',
'[string_int] This value should not be null.',
'[mapping_with_only_required_keys] This value should not be null.',
'[mapping_with_some_required_keys] This value should not be null.',
'[mapping_with_only_optional_keys] This value should not be null.',
],
[
"[] 'array' is a required key.",
"[] 'boolean' is a required key.",
"[] 'exp' is a required key.",
"[] 'float' is a required key.",
"[] 'float_as_integer' is a required key.",
"[] 'hex' is a required key.",
"[] 'int' is a required key.",
"[] 'string' is a required key.",
"[] 'string_int' is a required key.",
"[] 'nullable_array' is a required key.",
"[] 'nullable_boolean' is a required key.",
"[] 'nullable_exp' is a required key.",
"[] 'nullable_float' is a required key.",
"[] 'nullable_float_as_integer' is a required key.",
"[] 'nullable_hex' is a required key.",
"[] 'nullable_int' is a required key.",
"[] 'nullable_octal' is a required key.",
"[] 'nullable_string' is a required key.",
"[] 'nullable_string_int' is a required key.",
"[] 'mapping_with_only_required_keys' is a required key.",
"[] 'mapping_with_some_required_keys' is a required key.",
"[] 'mapping_with_only_optional_keys' is a required key.",
],
$expected_storage_type_check_errors + $expected_validation_errors + [
// For `mapping_with_only_required_keys`: errors for all 4 keys.
3 => "[mapping_with_only_required_keys] 'north' is a required key.",
4 => "[mapping_with_only_required_keys] 'east' is a required key.",
5 => "[mapping_with_only_required_keys] 'south' is a required key.",
6 => "[mapping_with_only_required_keys] 'west' is a required key.",
// For `mapping_with_some_required_keys`: errors for 2 required keys.
7 => "[mapping_with_some_required_keys] 'north' is a required key.",
8 => "[mapping_with_some_required_keys] 'south' is a required key.",
],
$expected_storage_type_check_errors + $expected_validation_errors,
],
];

177
core/tests/Drupal/KernelTests/Core/TypedData/ValidKeysConstraintValidatorTest.php
View File

@ -33,8 +33,13 @@ class ValidKeysConstraintValidatorTest extends KernelTestBase {
protected function setUp(): void {
parent::setUp();
// Install the Block module and create a Block config entity, so that we can
// test that the validator infers keys from a defined schema.
// test that the validator infers the required keys from a defined schema.
$this->enableModules(['system', 'block']);
// Also install the config_schema_test module, to enable testing with
// config entities as the example in the test cases below, simulating both
// possible schema states: fully validatable and not fully validatable.
// @see \Drupal\KernelTests\Config\Schema\MappingTest::testMappingInterpretations()
$this->enableModules(['config_schema_test']);
/** @var \Drupal\Core\Extension\ThemeInstallerInterface $theme_installer */
$theme_installer = $this->container->get('theme_installer');
$theme_installer->install(['stark']);
@ -43,6 +48,20 @@ class ValidKeysConstraintValidatorTest extends KernelTestBase {
'id' => 'branding',
'plugin' => 'system_branding_block',
'theme' => 'stark',
'status' => TRUE,
'weight' => 0,
'provider' => 'system',
'settings' => [
'use_site_logo' => TRUE,
'use_site_name' => TRUE,
'use_site_slogan' => TRUE,
'label_display' => FALSE,
// TRICKY: these 4 are inherited from `type: block_settings`.
'status' => TRUE,
'info' => '',
'view_mode' => 'full',
'context_mapping' => [],
],
]);
$block->save();
@ -100,6 +119,111 @@ class ValidKeysConstraintValidatorTest extends KernelTestBase {
);
}
/**
* Tests detecting missing required keys.
*
* @testWith [true, {"settings": "'label_display' is a required key."}]
* [false, {}]
*
* @see \Drupal\Core\Validation\Plugin\Validation\Constraint\ValidKeysConstraint::$missingRequiredKeyMessage
*/
public function testRequiredKeys(bool $block_is_fully_validatable, array $expected_validation_errors): void {
// Set or unset the `FullyValidatable` constraint on `block.block.*`.
\Drupal::state()->set('config_schema_test_block_fully_validatable', $block_is_fully_validatable);
$this->container->get('kernel')->rebuildContainer();
$this->config = $this->container->get('config.typed')
->get('block.block.branding');
// Start from the valid config.
$this->assertEmpty($this->config->validate());
// Then modify only one thing: remove the `label_display` setting.
$data = $this->config->toArray();
unset($data['settings']['label_display']);
$this->config = $this->container->get('config.typed')
->createFromNameAndData('block.block.branding', $data);
// Now 1 validation error should be triggered: one for the missing
// (statically) required key. It is only required because all block plugins
// are required to set it: see `type: block_settings`.
// @see \Drupal\system\Plugin\Block\SystemBrandingBlock::defaultConfiguration()
// @see \Drupal\system\Plugin\Block\SystemPoweredByBlock::defaultConfiguration()
$this->assertValidationErrors('block.block.branding', $data, $expected_validation_errors);
}
/**
* Tests detecting missing dynamically required keys.
*
* @testWith [true, {"settings": "'use_site_name' is a required key because plugin is system_branding_block (see config schema type block.settings.system_branding_block)."}]
* [false, {}]
*
* @see \Drupal\Core\Validation\Plugin\Validation\Constraint\ValidKeysConstraint::$dynamicMissingRequiredKeyMessage
*/
public function testDynamicallyRequiredKeys(bool $block_is_fully_validatable, array $expected_validation_errors): void {
// Set or unset the `FullyValidatable` constraint on `block.block.*`.
\Drupal::state()->set('config_schema_test_block_fully_validatable', $block_is_fully_validatable);
$this->container->get('kernel')->rebuildContainer();
$this->config = $this->container->get('config.typed')
->get('block.block.branding');
// Start from the valid config.
$this->assertEmpty($this->config->validate());
// Then modify only one thing: remove the `use_site_name` setting.
$data = $this->config->toArray();
unset($data['settings']['use_site_name']);
$this->config = $this->container->get('config.typed')
->createFromNameAndData('block.block.branding', $data);
// Now 1 validation error should be triggered: one for the missing
// required key. It is only dynamically required because not
// all block plugins support this key in their configuration.
// @see \Drupal\system\Plugin\Block\SystemBrandingBlock::defaultConfiguration()
// @see \Drupal\system\Plugin\Block\SystemPoweredByBlock::defaultConfiguration()
$this->assertValidationErrors('block.block.branding', $data, $expected_validation_errors);
}
/**
* Tests detecting both unknown and required keys.
*
* @testWith [true, ["'primary' is a required key because plugin is local_tasks_block (see config schema type block.settings.local_tasks_block).", "'secondary' is a required key because plugin is local_tasks_block (see config schema type block.settings.local_tasks_block)."]]
* [false, []]
*
* @see \Drupal\Core\Validation\Plugin\Validation\Constraint\ValidKeysConstraint::$dynamicInvalidKeyMessage
* @see \Drupal\Core\Validation\Plugin\Validation\Constraint\ValidKeysConstraint::$dynamicMissingRequiredKeyMessage
*/
public function testBothUnknownAndDynamicallyRequiredKeys(bool $block_is_fully_validatable, array $additional_expected_validation_errors): void {
// Set or unset the `FullyValidatable` constraint on `block.block.*`.
\Drupal::state()->set('config_schema_test_block_fully_validatable', $block_is_fully_validatable);
$this->container->get('kernel')->rebuildContainer();
$this->config = $this->container->get('config.typed')
->get('block.block.branding');
// Start from the valid config.
$this->assertEmpty($this->config->validate());
// Then modify only one thing: the block plugin that is being used.
$data = $this->config->toArray();
$data['plugin'] = 'local_tasks_block';
$this->config = $this->container->get('config.typed')
->createFromNameAndData('block.block.branding', $data);
// Now 3 validation errors should be triggered: one for each of the settings
// that exist in the "branding" block but not the "powered by" block.
// @see \Drupal\system\Plugin\Block\SystemBrandingBlock::defaultConfiguration()
// @see \Drupal\system\Plugin\Block\SystemPoweredByBlock::defaultConfiguration()
$this->assertValidationErrors('block.block.branding', $data,
[
'settings' => [
"'use_site_logo' is an unknown key because plugin is local_tasks_block (see config schema type block.settings.local_tasks_block).",
"'use_site_name' is an unknown key because plugin is local_tasks_block (see config schema type block.settings.local_tasks_block).",
"'use_site_slogan' is an unknown key because plugin is local_tasks_block (see config schema type block.settings.local_tasks_block).",
...$additional_expected_validation_errors,
],
],
);
}
/**
* Tests the ValidKeys constraint validator.
*/
@ -172,6 +296,24 @@ class ValidKeysConstraintValidatorTest extends KernelTestBase {
unset($definition['mapping']['east']['requiredKey']);
$violations = $typed_config->create(clone $definition, $value)->validate();
$this->assertCount(0, $violations);
// If in the mapping definition some keys that do NOT have
// `requiredKey: false` set, then they MUST be set.
// First test without changing the value: no error should occur because all
// keys passed to the ValidKeys constraint have a value.
unset($definition['mapping']['south']['requiredKey']);
unset($definition['mapping']['east']['requiredKey']);
$violations = $typed_config->create(clone $definition, $value)->validate();
$this->assertCount(0, $violations);
// Then remove the required key-value pair: this must trigger an error, but
// only if the root type has opted in.
unset($value['south']);
$violations = $typed_config->create(clone $definition, $value)->validate();
$this->assertCount(0, $violations);
$definition->addConstraint('FullyValidatable', NULL);
$violations = $typed_config->create(clone $definition, $value)->validate();
$this->assertCount(1, $violations);
$this->assertSame("'south' is a required key.", (string) $violations->get(0)->getMessage());
}
/**
@ -203,6 +345,39 @@ class ValidKeysConstraintValidatorTest extends KernelTestBase {
$config->validate();
}
/**
* Tests ValidKeys constraint validator detecting optional keys.
*/
public function testMarkedAsOptional(): void {
\Drupal::state()->set('config_schema_test_block_fully_validatable', TRUE);
$this->container->get('kernel')->rebuildContainer();
$this->config = $this->container->get('config.typed')
->get('block.block.branding');
$violations = $this->config->validate();
$this->assertCount(0, $violations);
// Reference to the mapping in the schema, to allow adjusting it for testing
// purposes.
assert($this->config->getDataDefinition() instanceof MapDataDefinition);
$mapping = $this->config->getDataDefinition()['mapping'];
// Removing a key-value pair should trigger a validation error.
$data = $this->config->getValue();
unset($data['status']);
$this->config->setValue($data);
$violations = $this->config->validate();
$this->assertCount(1, $violations);
$this->assertSame("'status' is a required key.", (string) $violations->get(0)
->getMessage());
// Unless a key is explicitly marked as optional.
$mapping['status']['requiredKey'] = FALSE;
$this->config->getDataDefinition()['mapping'] = $mapping;
$violations = $this->config->validate();
$this->assertCount(0, $violations);
}
/**
* Asserts a set of validation errors is raised when the config is validated.
*