From 5aa9704ce3a5669c55f8114f4d508dd1c3babb62 Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Fri, 15 Dec 2023 00:55:46 +0000
Subject: [PATCH] Issue #3364109 by Wim Leers, effulgentsia, lauriii,
phenaproxima, borisson_, bircher, alexpott: Configuration schema & required
values: add test coverage for `nullable: true` validation support
---
.../Drupal/Core/Config/TypedConfigManager.php | 30 ++++
.../Constraint/FullyValidatableConstraint.php | 15 ++
.../FullyValidatableConstraintValidator.php | 24 +++
.../Constraint/NotNullConstraintValidator.php | 7 +-
.../ValidKeysConstraintValidator.php | 6 +
core/misc/cspell/drupal-dictionary.txt | 1 +
.../config/install/config_test.types.yml | 11 ++
.../config/schema/config_test.schema.yml | 50 ++++++
.../tests/src/Kernel/EditorValidationTest.php | 13 ++
.../Entity/FieldConfigValidationTest.php | 12 ++
.../src/Entity/ContentLanguageSettings.php | 5 +-
.../MigrateValidatableEntityInterface.php | 2 -
.../config/schema/shortcut.schema.yml | 2 +
.../system/config/schema/system.schema.yml | 9 +-
.../src/Kernel/Entity/MenuValidationTest.php | 27 +++
.../Core/Config/ConfigCRUDTest.php | 10 ++
.../Config/ConfigEntityValidationTestBase.php | 167 ++++++++++++++++++
.../Core/Config/SchemaCheckTraitTest.php | 71 +++++++-
18 files changed, 452 insertions(+), 10 deletions(-)
create mode 100644 core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/FullyValidatableConstraint.php
create mode 100644 core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/FullyValidatableConstraintValidator.php
diff --git a/core/lib/Drupal/Core/Config/TypedConfigManager.php b/core/lib/Drupal/Core/Config/TypedConfigManager.php
index 7d1981c76d40..dcd9d2e961ad 100644
--- a/core/lib/Drupal/Core/Config/TypedConfigManager.php
+++ b/core/lib/Drupal/Core/Config/TypedConfigManager.php
@@ -11,6 +11,7 @@
use Drupal\Core\Config\Schema\Undefined;
use Drupal\Core\Extension\ModuleHandlerInterface;
use Drupal\Core\TypedData\TypedDataManager;
+use Drupal\Core\Validation\Plugin\Validation\Constraint\FullyValidatableConstraint;
/**
* Manages config schema type plugins.
@@ -120,6 +121,35 @@ public function buildDataDefinition(array $definition, $value, $name = NULL, $pa
$data_definition[$key] = $value;
}
}
+
+ // All values are optional by default (meaning they can be NULL), except for
+ // mappings and sequences. A sequence can only be NULL when `nullable: true`
+ // is set on the config schema type definition. This is unintuitive and
+ // contradicts Drupal core's documentation.
+ // @see https://www.drupal.org/node/2264179
+ // @see https://www.drupal.org/node/1978714
+ // 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 property
+ // cannot be NULL unless it specifies `nullable: true`, by adding
+ // `FullyValidatable` as a top-level validation constraint.
+ // @see https://www.drupal.org/node/3364108
+ // @see https://www.drupal.org/node/3364109
+ // @see \Drupal\Core\TypedData\TypedDataManager::getDefaultConstraints()
+ if ($parent) {
+ $root_type_has_opted_in = FALSE;
+ foreach ($parent->getRoot()->getConstraints() as $constraint) {
+ if ($constraint 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);
+ }
+ }
+
return $data_definition;
}
diff --git a/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/FullyValidatableConstraint.php b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/FullyValidatableConstraint.php
new file mode 100644
index 000000000000..70db1c6b98ed
--- /dev/null
+++ b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/FullyValidatableConstraint.php
@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types = 1);
+
+namespace Drupal\Core\Validation\Plugin\Validation\Constraint;
+
+use Symfony\Component\Validator\Constraint;
+
+/**
+ * @Constraint(
+ * id = "FullyValidatable",
+ * label = @Translation("Whether this config schema type is fully validatable", context = "Validation"),
+ * )
+ */
+final class FullyValidatableConstraint extends Constraint {}
diff --git a/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/FullyValidatableConstraintValidator.php b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/FullyValidatableConstraintValidator.php
new file mode 100644
index 000000000000..16c0a7cb81ba
--- /dev/null
+++ b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/FullyValidatableConstraintValidator.php
@@ -0,0 +1,24 @@
+<?php
+
+declare(strict_types = 1);
+
+namespace Drupal\Core\Validation\Plugin\Validation\Constraint;
+
+use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\ConstraintValidator;
+
+/**
+ * FullyValidatable constraint.
+ *
+ * @internal
+ */
+final class FullyValidatableConstraintValidator extends ConstraintValidator {
+
+ /**
+ * {@inheritdoc}
+ */
+ public function validate(mixed $value, Constraint $constraint) {
+ // No-op.
+ }
+
+}
diff --git a/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/NotNullConstraintValidator.php b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/NotNullConstraintValidator.php
index 1cbf5d9600b8..dfb1e15ad2ce 100644
--- a/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/NotNullConstraintValidator.php
+++ b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/NotNullConstraintValidator.php
@@ -2,6 +2,7 @@
namespace Drupal\Core\Validation\Plugin\Validation\Constraint;
+use Drupal\Core\Config\Schema\ArrayElement;
use Drupal\Core\TypedData\ComplexDataInterface;
use Drupal\Core\TypedData\ListInterface;
use Drupal\Core\TypedData\Validation\TypedDataAwareValidatorTrait;
@@ -25,7 +26,11 @@ class NotNullConstraintValidator extends NotNullValidator {
*/
public function validate($value, Constraint $constraint) {
$typed_data = $this->getTypedData();
- if (($typed_data instanceof ListInterface || $typed_data instanceof ComplexDataInterface) && $typed_data->isEmpty()) {
+ // TRICKY: the Mapping and Sequence data types both extend ArrayElement
+ // (which implements ComplexDataInterface), but configuration schema sees a
+ // substantial difference between an empty sequence/mapping and NULL. So we
+ // want to make sure we don't treat an empty array as NULL.
+ if (($typed_data instanceof ListInterface || $typed_data instanceof ComplexDataInterface) && !$typed_data instanceof ArrayElement && $typed_data->isEmpty()) {
$value = NULL;
}
parent::validate($value, $constraint);
diff --git a/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/ValidKeysConstraintValidator.php b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/ValidKeysConstraintValidator.php
index 5b52e7c59676..90c2ff76a946 100644
--- a/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/ValidKeysConstraintValidator.php
+++ b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/ValidKeysConstraintValidator.php
@@ -20,6 +20,12 @@ public function validate(mixed $value, Constraint $constraint) {
assert($constraint instanceof ValidKeysConstraint);
if (!is_array($value)) {
+ // If the value is NULL, then the `NotNull` constraint validator will
+ // set the appropriate validation error message.
+ // @see \Drupal\Core\Validation\Plugin\Validation\Constraint\NotNullConstraintValidator
+ if ($value === NULL) {
+ return;
+ }
throw new UnexpectedTypeException($value, 'array');
}
diff --git a/core/misc/cspell/drupal-dictionary.txt b/core/misc/cspell/drupal-dictionary.txt
index 6186bc3494ac..a660e99cb2c5 100644
--- a/core/misc/cspell/drupal-dictionary.txt
+++ b/core/misc/cspell/drupal-dictionary.txt
@@ -10,3 +10,4 @@ olivero's
simpletest
tempstore
umami
+validatable
diff --git a/core/modules/config/tests/config_test/config/install/config_test.types.yml b/core/modules/config/tests/config_test/config/install/config_test.types.yml
index 452d16a382da..5075bb435c8f 100644
--- a/core/modules/config/tests/config_test/config/install/config_test.types.yml
+++ b/core/modules/config/tests/config_test/config/install/config_test.types.yml
@@ -12,3 +12,14 @@ int: 99
# octal: 0775
string: string
string_int: '1'
+# To validate all types can be nullable.
+nullable_array: null
+nullable_boolean: ~
+nullable_exp: null
+nullable_float: ~
+nullable_float_as_integer: null
+nullable_hex: ~
+nullable_int: null
+nullable_octal: ~
+nullable_string: null
+nullable_string_int: ~
diff --git a/core/modules/config/tests/config_test/config/schema/config_test.schema.yml b/core/modules/config/tests/config_test/config/schema/config_test.schema.yml
index 723738bebff8..ad8f455a4636 100644
--- a/core/modules/config/tests/config_test/config/schema/config_test.schema.yml
+++ b/core/modules/config/tests/config_test/config/schema/config_test.schema.yml
@@ -98,6 +98,56 @@ config_test.types:
string_int:
type: string
label: 'String integer'
+ # All the above types, but now nullable.
+ nullable_array:
+ type: sequence
+ label: 'Nullable array'
+ nullable: true
+ sequence:
+ type: string
+ label: 'Item'
+ nullable_boolean:
+ type: boolean
+ label: 'Nullable boolean'
+ nullable: true
+ nullable_exp:
+ type: float
+ label: 'Nullable exponential'
+ nullable: true
+ nullable_float:
+ type: float
+ label: 'Nullable float'
+ nullable: true
+ nullable_float_as_integer:
+ type: float
+ label: 'Float'
+ nullable: true
+ nullable_hex:
+ type: integer
+ label: 'Nullable hexadecimal'
+ nullable: true
+ nullable_int:
+ type: integer
+ label: 'Nullable integer'
+ nullable: true
+ nullable_octal:
+ type: integer
+ label: 'Nullable octal'
+ nullable: true
+ nullable_string:
+ type: string
+ label: 'Nullable string'
+ nullable: true
+ nullable_string_int:
+ type: string
+ label: 'Nullable string integer'
+ nullable: true
+
+# cspell:ignore validatable
+config_test.types.fully_validatable:
+ type: config_test.types
+ constraints:
+ FullyValidatable: ~
config_test.no_status.default:
type: config_object
diff --git a/core/modules/editor/tests/src/Kernel/EditorValidationTest.php b/core/modules/editor/tests/src/Kernel/EditorValidationTest.php
index 5c6b1b87e3a8..2ae9184de5a3 100644
--- a/core/modules/editor/tests/src/Kernel/EditorValidationTest.php
+++ b/core/modules/editor/tests/src/Kernel/EditorValidationTest.php
@@ -79,4 +79,17 @@ public function testLabelValidation(): void {
$this->markTestSkipped();
}
+ /**
+ * {@inheritdoc}
+ */
+ public function testRequiredPropertyValuesMissing(?array $additional_expected_validation_errors_when_missing = NULL): void {
+ parent::testRequiredPropertyValuesMissing([
+ 'dependencies' => [
+ // @see ::testInvalidDependencies()
+ // @see \Drupal\Core\Config\Plugin\Validation\Constraint\RequiredConfigDependenciesConstraintValidator
+ '' => 'This text editor requires a text format.',
+ ],
+ ]);
+ }
+
}
diff --git a/core/modules/field/tests/src/Kernel/Entity/FieldConfigValidationTest.php b/core/modules/field/tests/src/Kernel/Entity/FieldConfigValidationTest.php
index 0d6b9bc06723..384f2d4040f2 100644
--- a/core/modules/field/tests/src/Kernel/Entity/FieldConfigValidationTest.php
+++ b/core/modules/field/tests/src/Kernel/Entity/FieldConfigValidationTest.php
@@ -98,4 +98,16 @@ public function testImmutableProperties(array $valid_values = []): void {
parent::testImmutableProperties($valid_values);
}
+ /**
+ * {@inheritdoc}
+ */
+ public function testRequiredPropertyValuesMissing(?array $additional_expected_validation_errors_when_missing = NULL): void {
+ parent::testRequiredPropertyValuesMissing([
+ 'dependencies' => [
+ // @see testInvalidDependencies()
+ '' => 'This field requires a field storage.',
+ ],
+ ]);
+ }
+
}
diff --git a/core/modules/language/src/Entity/ContentLanguageSettings.php b/core/modules/language/src/Entity/ContentLanguageSettings.php
index 00e8d1fa69cb..72f554a0a053 100644
--- a/core/modules/language/src/Entity/ContentLanguageSettings.php
+++ b/core/modules/language/src/Entity/ContentLanguageSettings.php
@@ -33,7 +33,10 @@
* "default_langcode",
* "language_alterable",
* },
- * list_cache_tags = { "rendered" }
+ * list_cache_tags = { "rendered" },
+ * constraints = {
+ * "ImmutableProperties" = {"id", "target_entity_type_id", "target_bundle"},
+ * },
* )
*/
class ContentLanguageSettings extends ConfigEntityBase implements ContentLanguageSettingsInterface {
diff --git a/core/modules/migrate/src/Plugin/MigrateValidatableEntityInterface.php b/core/modules/migrate/src/Plugin/MigrateValidatableEntityInterface.php
index e1a6684e8843..baf0cdfcffda 100644
--- a/core/modules/migrate/src/Plugin/MigrateValidatableEntityInterface.php
+++ b/core/modules/migrate/src/Plugin/MigrateValidatableEntityInterface.php
@@ -4,8 +4,6 @@
use Drupal\Core\Entity\FieldableEntityInterface;
-// cspell:ignore validatable
-
/**
* To implement by a destination plugin that should provide entity validation.
*
diff --git a/core/modules/shortcut/config/schema/shortcut.schema.yml b/core/modules/shortcut/config/schema/shortcut.schema.yml
index 11bcf6353bdc..14692ca8d7ff 100644
--- a/core/modules/shortcut/config/schema/shortcut.schema.yml
+++ b/core/modules/shortcut/config/schema/shortcut.schema.yml
@@ -19,6 +19,8 @@ shortcut.set.*:
label:
type: required_label
label: 'Label'
+ constraints:
+ FullyValidatable: ~
# Schema for theme settings.
theme_settings.third_party.shortcut:
diff --git a/core/modules/system/config/schema/system.schema.yml b/core/modules/system/config/schema/system.schema.yml
index a495c9cf7af8..2c12de19a884 100644
--- a/core/modules/system/config/schema/system.schema.yml
+++ b/core/modules/system/config/schema/system.schema.yml
@@ -226,9 +226,16 @@ system.menu.*:
description:
type: label
label: 'Menu description'
+ # @see \Drupal\menu_ui\MenuForm::form()
+ nullable: true
+ constraints:
+ Length:
+ max: 512
locked:
type: boolean
- label: ''
+ label: 'Locked'
+ constraints:
+ FullyValidatable: ~
system.action.*:
type: config_entity
diff --git a/core/modules/system/tests/src/Kernel/Entity/MenuValidationTest.php b/core/modules/system/tests/src/Kernel/Entity/MenuValidationTest.php
index 7cb041759b2d..c05a78f096da 100644
--- a/core/modules/system/tests/src/Kernel/Entity/MenuValidationTest.php
+++ b/core/modules/system/tests/src/Kernel/Entity/MenuValidationTest.php
@@ -12,6 +12,13 @@
*/
class MenuValidationTest extends ConfigEntityValidationTestBase {
+ /**
+ * {@inheritdoc}
+ */
+ protected static array $propertiesWithOptionalValues = [
+ 'description',
+ ];
+
/**
* {@inheritdoc}
*/
@@ -48,4 +55,24 @@ public function providerInvalidMachineNameCharacters(): array {
return $cases;
}
+ /**
+ * Tests that description is optional, and limited to 512 characters.
+ *
+ * phpcs:disable Drupal.Commenting
+ * cspell:disable
+ *
+ * @testWith [null, {}]
+ * ["", {}]
+ * ["This is an ASCII description.", {}]
+ * ["This is an emoji in a description: 🕺.", []]
+ * ["Iste et sunt ut cum. Suscipit officia molestias amet provident et sunt sit. Tenetur doloribus odit sapiente doloremque sequi id dignissimos. In rerum nihil voluptatibus architecto laborum. Repellendus eligendi laborum id nesciunt alias incidunt non. Tenetur deserunt facere voluptas nisi id. Aut ab eaque eligendi. Nihil quasi illum sit provident voluptatem repellat temporibus autem. Mollitia quisquam error facilis quasi voluptate. Dignissimos quis culpa nobis veritatis ut vel laudantium cumque. Rerum mollitia deleniti possimus placeat rerum. Reiciendis distinctio soluta voluptatem.", {"description": "This value is too long. It should have <em class=\"placeholder\">512</em> characters or less."}]
+ *
+ * cspell:enable
+ * phpcs:enable Drupal.Commenting
+ */
+ public function testDescription(?string $description, array $expected_errors): void {
+ $this->entity->set('description', $description);
+ $this->assertValidationErrors($expected_errors);
+ }
+
}
diff --git a/core/tests/Drupal/KernelTests/Core/Config/ConfigCRUDTest.php b/core/tests/Drupal/KernelTests/Core/Config/ConfigCRUDTest.php
index 2200f4673a2e..ef1fa03197b1 100644
--- a/core/tests/Drupal/KernelTests/Core/Config/ConfigCRUDTest.php
+++ b/core/tests/Drupal/KernelTests/Core/Config/ConfigCRUDTest.php
@@ -284,6 +284,16 @@ public function testDataTypes() {
// 'octal' => 0775,
'string' => 'string',
'string_int' => '1',
+ 'nullable_array' => NULL,
+ 'nullable_boolean' => NULL,
+ 'nullable_exp' => NULL,
+ 'nullable_float' => NULL,
+ 'nullable_float_as_integer' => NULL,
+ 'nullable_hex' => NULL,
+ 'nullable_int' => NULL,
+ 'nullable_octal' => NULL,
+ 'nullable_string' => NULL,
+ 'nullable_string_int' => NULL,
];
$data = ['_core' => ['default_config_hash' => Crypt::hashBase64(serialize($data))]] + $data;
$this->assertSame($data, $config->get());
diff --git a/core/tests/Drupal/KernelTests/Core/Config/ConfigEntityValidationTestBase.php b/core/tests/Drupal/KernelTests/Core/Config/ConfigEntityValidationTestBase.php
index 04ba751ab92c..c7889c169e7c 100644
--- a/core/tests/Drupal/KernelTests/Core/Config/ConfigEntityValidationTestBase.php
+++ b/core/tests/Drupal/KernelTests/Core/Config/ConfigEntityValidationTestBase.php
@@ -4,9 +4,12 @@
use Drupal\Component\Utility\NestedArray;
use Drupal\Core\Config\Entity\ConfigEntityInterface;
+use Drupal\Core\Config\TypedConfigManagerInterface;
+use Drupal\Core\Entity\EntityWithPluginCollectionInterface;
use Drupal\Core\Language\LanguageInterface;
use Drupal\Core\Language\LanguageManager;
use Drupal\Core\TypedData\Plugin\DataType\LanguageReference;
+use Drupal\Core\Validation\Plugin\Validation\Constraint\FullyValidatableConstraint;
use Drupal\KernelTests\KernelTestBase;
use Drupal\language\Entity\ConfigurableLanguage;
@@ -44,6 +47,18 @@ abstract class ConfigEntityValidationTestBase extends KernelTestBase {
*/
protected bool $hasLabel = TRUE;
+ /**
+ * The config entity properties whose values are optional (set to NULL).
+ *
+ * @var string[]
+ * @see \Drupal\Core\Config\Entity\ConfigEntityTypeInterface::getPropertiesToExport()
+ * @see ::testRequiredPropertyValuesMissing()
+ */
+ protected static array $propertiesWithOptionalValues = [
+ '_core',
+ 'third_party_settings',
+ ];
+
/**
* {@inheritdoc}
*/
@@ -446,4 +461,156 @@ public function testImmutableProperties(array $valid_values = []): void {
}
}
+ /**
+ * 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 testRequiredPropertyValuesMissing(?array $additional_expected_validation_errors_when_missing = NULL): void {
+ $config_entity_properties = array_keys($this->entity->getEntityType()->getPropertiesToExport());
+
+ // Guide developers when $additional_expected_validation_errors_when_missing
+ // does not contain sensible values.
+ $non_existing_properties = array_diff(array_keys($additional_expected_validation_errors_when_missing ?? []), $config_entity_properties);
+ if ($non_existing_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.',
+ __METHOD__,
+ implode(', ', $non_existing_properties),
+ $this->entity->getEntityTypeId(),
+ ));
+ }
+ $properties_with_optional_values = $this->getPropertiesWithOptionalValues();
+
+ // Get the config entity properties that are immutable.
+ // @see ::testImmutableProperties()
+ $immutable_properties = $this->entity->getEntityType()->getConstraints()['ImmutableProperties'];
+
+ // Config entity properties containing plugin collections are special cases:
+ // setting them to NULL would cause them to get out of sync with the plugin
+ // collection.
+ // @see \Drupal\Core\Config\Entity\ConfigEntityBase::set()
+ // @see \Drupal\Core\Config\Entity\ConfigEntityBase::preSave()
+ $plugin_collection_properties = $this->entity instanceof EntityWithPluginCollectionInterface
+ ? array_keys($this->entity->getPluginCollections())
+ : [];
+
+ // To test properties with missing required values, $this->entity must be
+ // modified to be able to use ::assertValidationErrors(). To allow restoring
+ // $this->entity to its original value for each tested property, a clone of
+ // the original entity is needed.
+ $original_entity = clone $this->entity;
+ foreach ($config_entity_properties as $property) {
+ // Do not try to set immutable properties to NULL: their immutability is
+ // already tested.
+ // @see ::testImmutableProperties()
+ if (in_array($property, $immutable_properties, TRUE)) {
+ continue;
+ }
+
+ // Do not try to set plugin collection properties to NULL.
+ if (in_array($property, $plugin_collection_properties, TRUE)) {
+ continue;
+ }
+
+ $this->entity = clone $original_entity;
+ $this->entity->set($property, NULL);
+ $expected_validation_errors = in_array($property, $properties_with_optional_values, TRUE)
+ ? []
+ : [$property => 'This value should not be null.'];
+
+ // @see `type: required_label`
+ // @see \Symfony\Component\Validator\Constraints\NotBlank
+ if (!$this->isFullyValidatable() && $this->entity->getEntityType()->getKey('label') == $property) {
+ $expected_validation_errors = [$property => 'This value should not be blank.'];
+ }
+
+ $this->assertValidationErrors(($additional_expected_validation_errors_when_missing[$property] ?? []) + $expected_validation_errors);
+ }
+ }
+
+ /**
+ * Whether the tested config entity type is fully validatable.
+ *
+ * @return bool
+ * Whether the tested config entity type is fully validatable.
+ */
+ protected function isFullyValidatable(): bool {
+ $typed_config = $this->container->get('config.typed');
+ assert($typed_config instanceof TypedConfigManagerInterface);
+ // @see \Drupal\Core\Entity\Plugin\DataType\ConfigEntityAdapter::getConfigTypedData()
+ $config_entity_type_schema_constraints = $typed_config
+ ->createFromNameAndData(
+ $this->entity->getConfigDependencyName(),
+ $this->entity->toArray()
+ )->getConstraints();
+
+ foreach ($config_entity_type_schema_constraints as $constraint) {
+ if ($constraint instanceof FullyValidatableConstraint) {
+ return TRUE;
+ }
+ }
+ return FALSE;
+ }
+
+ /**
+ * Determines the config entity properties with optional values.
+ *
+ * @return string[]
+ * The config entity properties whose values are optional.
+ */
+ protected function getPropertiesWithOptionalValues(): array {
+ $config_entity_properties = array_keys($this->entity->getEntityType()
+ ->getPropertiesToExport());
+
+ // If a config entity type is not fully validatable, all properties are
+ // optional, with the exception of `type: langcode` and
+ // `type: required_label`.
+ if (!$this->isFullyValidatable()) {
+ return array_diff($config_entity_properties, [
+ // @see `type: langcode`
+ // @see \Symfony\Component\Validator\Constraints\NotNull
+ 'langcode',
+ 'default_langcode',
+ // @see `type: required_label`
+ // @see \Symfony\Component\Validator\Constraints\NotBlank
+ $this->entity->getEntityType()->getKey('label'),
+ ]);
+ }
+
+ // Otherwise, all properties are required except for those marked
+ // optional. Rather than inspecting config schema, require authors of tests
+ // to explicitly list optional properties in a
+ // `propertiesWithOptionalValues` property on this class.
+ $class = static::class;
+ $optional_properties = [];
+ while ($class) {
+ if (property_exists($class, 'propertiesWithOptionalValues')) {
+ $optional_properties = array_merge($optional_properties, $class::$propertiesWithOptionalValues);
+ }
+ $class = get_parent_class($class);
+ }
+ $optional_properties = array_unique($optional_properties);
+
+ // Guide developers when $optionalProperties does not contain sensible
+ // values.
+ $non_existing_properties = array_diff($optional_properties, $config_entity_properties);
+ if ($non_existing_properties) {
+ throw new \LogicException(sprintf('The %s test class lists %s in $optionalProperties but it is not a property of the %s config entity type.',
+ static::class,
+ implode(', ', $non_existing_properties),
+ $this->entity->getEntityTypeId()
+ ));
+ }
+
+ return $optional_properties;
+ }
+
}
diff --git a/core/tests/Drupal/KernelTests/Core/Config/SchemaCheckTraitTest.php b/core/tests/Drupal/KernelTests/Core/Config/SchemaCheckTraitTest.php
index e0a6340797f5..95b95c693192 100644
--- a/core/tests/Drupal/KernelTests/Core/Config/SchemaCheckTraitTest.php
+++ b/core/tests/Drupal/KernelTests/Core/Config/SchemaCheckTraitTest.php
@@ -42,7 +42,7 @@ protected function setUp(): void {
*
* @dataProvider providerCheckConfigSchema
*/
- public function testCheckConfigSchema(bool $validate_constraints, array $expectations) {
+ public function testCheckConfigSchema(string $type_to_validate_against, bool $validate_constraints, array|bool $nulled_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);
@@ -52,18 +52,30 @@ public function testCheckConfigSchema(bool $validate_constraints, array $expecta
$ret = $this->checkConfigSchema($this->typedConfig, 'config_test.types', $config_data);
$this->assertTrue($ret);
+ // Test it is possible to mark any schema type as required (not nullable).
+ $nulled_config_data = array_fill_keys(array_keys($config_data), NULL);
+ $ret = $this->checkConfigSchema($this->typedConfig, $type_to_validate_against, $nulled_config_data, $validate_constraints);
+ $this->assertSame($nulled_expectations, $ret);
+
// Add a new key, a new array and overwrite boolean with array to test the
// error messages.
$config_data = ['new_key' => 'new_value', 'new_array' => []] + $config_data;
$config_data['boolean'] = [];
- $ret = $this->checkConfigSchema($this->typedConfig, 'config_test.types', $config_data, $validate_constraints);
+ $ret = $this->checkConfigSchema($this->typedConfig, $type_to_validate_against, $config_data, $validate_constraints);
$this->assertEquals($expectations, $ret);
}
public function providerCheckConfigSchema(): array {
// Storage type check errors.
// @see \Drupal\Core\Config\Schema\SchemaCheckTrait::checkValue()
+ $expected_storage_null_check_errors = [
+ // TRICKY: `_core` is added during installation even if it is absent from
+ // core/modules/config/tests/config_test/config/install/config_test.dynamic.dotted.default.yml.
+ // @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',
+ ];
$expected_storage_type_check_errors = [
'config_test.types:new_key' => 'missing schema',
'config_test.types:new_array' => 'missing schema',
@@ -76,16 +88,65 @@ public function providerCheckConfigSchema(): array {
'1' => "[new_array] 'new_array' is not a supported key.",
'2' => '[boolean] This value should be of the correct primitive type.',
];
- return [
- 'without validation' => [
+ $basic_cases = [
+ 'config_test.types, without validation' => [
+ 'config_test.types',
+ FALSE,
+ $expected_storage_null_check_errors,
+ $expected_storage_type_check_errors,
+ ],
+ 'config_test.types, with validation' => [
+ 'config_test.types',
+ TRUE,
+ $expected_storage_null_check_errors,
+ $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.
+ // @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 = [
+ // TRICKY: `_core` is added during installation even if it is absent from
+ // core/modules/config/tests/config_test/config/install/config_test.dynamic.dotted.default.yml.
+ // @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',
+ ];
+ $expected_storage_type_check_errors = [
+ 'config_test.types.fully_validatable:new_key' => 'missing schema',
+ 'config_test.types.fully_validatable:new_array' => 'missing schema',
+ 'config_test.types.fully_validatable:boolean' => 'non-scalar value but not defined as an array (such as mapping or sequence)',
+ ];
+ $opt_in_cases = [
+ 'config_test.types.fully_validatable, without validation' => [
+ 'config_test.types.fully_validatable',
FALSE,
+ $expected_storage_null_check_errors,
$expected_storage_type_check_errors,
],
- 'with validation' => [
+ 'config_test.types.fully_validatable, with validation' => [
+ 'config_test.types.fully_validatable',
TRUE,
+ $expected_storage_null_check_errors + [
+ '[_core] This value should not be null.',
+ '[array] This value should not be null.',
+ '[boolean] This value should not be null.',
+ '[exp] This value should not be null.',
+ '[float] This value should not be null.',
+ '[float_as_integer] This value should not be null.',
+ '[hex] This value should not be null.',
+ '[int] This value should not be null.',
+ '[string] This value should not be null.',
+ '[string_int] This value should not be null.',
+ ],
$expected_storage_type_check_errors + $expected_validation_errors,
],
];
+
+ return array_merge($basic_cases, $opt_in_cases);
}
}
--
GitLab