diff --git a/core/config/schema/core.data_types.schema.yml b/core/config/schema/core.data_types.schema.yml index 4476e98d0c7efaecc72b038c75021f509f1947ba..d866c01b1ed14a11d22505d1475cde1844016d77 100644 --- a/core/config/schema/core.data_types.schema.yml +++ b/core/config/schema/core.data_types.schema.yml @@ -94,6 +94,16 @@ color_hex: type: string label: 'Color' +# Machine-readable identifier that can only contain certain characters. +machine_name: + type: string + label: 'Machine name' + constraints: + Regex: '/^[a-z0-9_]+$/' + Length: + # @see \Drupal\Core\Config\Entity\ConfigEntityStorage::MAX_ID_LENGTH + max: 166 + # Complex extended data types: # Root of a configuration object. diff --git a/core/modules/block/config/schema/block.schema.yml b/core/modules/block/config/schema/block.schema.yml index d768a9619503220ad3f28260bfb04466023823e8..64dda31dbbb77f5e54b184e97d1e603165b7960a 100644 --- a/core/modules/block/config/schema/block.schema.yml +++ b/core/modules/block/config/schema/block.schema.yml @@ -5,8 +5,13 @@ block.block.*: label: 'Block' mapping: id: - type: string + type: machine_name label: 'ID' + # Blocks have atypical machine names: they allow periods for historical reasons. + # @see https://www.drupal.org/project/drupal/issues/2685917 + # @see https://www.drupal.org/project/drupal/issues/2043527 + constraints: + Regex: '/^[a-z0-9_.]+$/' theme: type: string label: 'Theme' diff --git a/core/modules/block/tests/src/Kernel/BlockValidationTest.php b/core/modules/block/tests/src/Kernel/BlockValidationTest.php index c3c6142e3520a5662acdfcf52484787a629439af..db6f502eab1c88f6dd8daf40104a40ed55b9ef17 100644 --- a/core/modules/block/tests/src/Kernel/BlockValidationTest.php +++ b/core/modules/block/tests/src/Kernel/BlockValidationTest.php @@ -39,4 +39,18 @@ public function testInvalidPluginId(): void { $this->assertValidationErrors(['plugin' => "The 'non_existent' plugin does not exist."]); } + /** + * Block names are atypical in that they allow periods in the machine name. + */ + public function providerInvalidMachineNameCharacters(): array { + $cases = parent::providerInvalidMachineNameCharacters(); + // Remove the existing test case that verifies a machine name containing + // periods is invalid. + $this->assertSame(['period.separated', FALSE], $cases['INVALID: period separated']); + unset($cases['INVALID: period separated']); + // And instead add a test case that verifies it is allowed for blocks. + $cases['VALID: period separated'] = ['period.separated', TRUE]; + return $cases; + } + } diff --git a/core/modules/block_content/config/schema/block_content.schema.yml b/core/modules/block_content/config/schema/block_content.schema.yml index 77a898f5c05556031327596a1d883f7dcb10b4fd..59cb4744a673a0aa2bc09539005dafb2ae2e6a4c 100644 --- a/core/modules/block_content/config/schema/block_content.schema.yml +++ b/core/modules/block_content/config/schema/block_content.schema.yml @@ -5,7 +5,7 @@ block_content.type.*: label: 'Block type settings' mapping: id: - type: string + type: machine_name label: 'ID' label: type: label diff --git a/core/modules/comment/config/schema/comment.schema.yml b/core/modules/comment/config/schema/comment.schema.yml index 7c31ccbba9bb87dbdf6db9b00187392a90ef2e6c..f53d0730e05841132e39d4106cbe8bbde42f7119 100644 --- a/core/modules/comment/config/schema/comment.schema.yml +++ b/core/modules/comment/config/schema/comment.schema.yml @@ -38,7 +38,7 @@ comment.type.*: label: 'Comment type settings' mapping: id: - type: string + type: machine_name label: 'ID' label: type: label diff --git a/core/modules/contact/config/schema/contact.schema.yml b/core/modules/contact/config/schema/contact.schema.yml index 1f0585db856b255e111dcf0d9c3190cb01581387..5fb69d63393afc583166e99e06c3037b3da64e2e 100644 --- a/core/modules/contact/config/schema/contact.schema.yml +++ b/core/modules/contact/config/schema/contact.schema.yml @@ -5,8 +5,13 @@ contact.form.*: label: 'Contact form' mapping: id: - type: string + type: machine_name label: 'ID' + constraints: + Length: + # Contact form IDs are specifically limited to 32 characters. + # @see \Drupal\contact\ContactFormEditForm::form() + max: 32 label: type: label label: 'Label' diff --git a/core/modules/filter/config/schema/filter.schema.yml b/core/modules/filter/config/schema/filter.schema.yml index b45a4755958c2dd4bddc2360835f76f46a00664d..04c61d0f814199dac47762d83e290218693565f7 100644 --- a/core/modules/filter/config/schema/filter.schema.yml +++ b/core/modules/filter/config/schema/filter.schema.yml @@ -19,8 +19,13 @@ filter.format.*: type: label label: 'Name' format: - type: string + type: machine_name label: 'Machine name' + # Filter format machine names can be up to 255 characters. + # @see \Drupal\filter\FilterFormatFormBase::form() + constraints: + Length: + max: 255 weight: type: integer label: 'Weight' diff --git a/core/modules/image/config/schema/image.schema.yml b/core/modules/image/config/schema/image.schema.yml index 5368f85f4cdc2ebdfaf26c66bd358c249fbdcda8..8cbd56b6f6b5be60085e46794c0a34b7ff5dfa0f 100644 --- a/core/modules/image/config/schema/image.schema.yml +++ b/core/modules/image/config/schema/image.schema.yml @@ -5,7 +5,7 @@ image.style.*: label: 'Image style' mapping: name: - type: string + type: machine_name label: type: label label: 'Label' diff --git a/core/modules/media/config/schema/media.schema.yml b/core/modules/media/config/schema/media.schema.yml index b5322c9ccfeec9439047ef87cf316651a38d45f9..29c56d37502e28e554c3162ac57eb0a19c8ad0a2 100644 --- a/core/modules/media/config/schema/media.schema.yml +++ b/core/modules/media/config/schema/media.schema.yml @@ -20,8 +20,13 @@ media.type.*: label: 'Media type' mapping: id: - type: string + type: machine_name label: 'Machine name' + constraints: + Length: + # Media type IDs are specifically limited to 32 characters. + # @see \Drupal\media\MediaTypeForm::form() + max: 32 label: type: label label: 'Name' diff --git a/core/modules/media/tests/src/Kernel/MediaTypeValidationTest.php b/core/modules/media/tests/src/Kernel/MediaTypeValidationTest.php index c5f2774b2c896a7473a8ce570b0cbcd43491c100..8315284280a75d1896f74faae4cca1731a668ada 100644 --- a/core/modules/media/tests/src/Kernel/MediaTypeValidationTest.php +++ b/core/modules/media/tests/src/Kernel/MediaTypeValidationTest.php @@ -24,7 +24,7 @@ class MediaTypeValidationTest extends ConfigEntityValidationTestBase { */ protected function setUp(): void { parent::setUp(); - $this->entity = $this->createMediaType('test'); + $this->entity = $this->createMediaType('test', ['id' => 'test_media']); } } diff --git a/core/modules/node/config/schema/node.schema.yml b/core/modules/node/config/schema/node.schema.yml index 1caa3576e7660935fe4856a5a445fd877c6e36e6..c33ab3ee2331be006e3005103c0038d530590584 100644 --- a/core/modules/node/config/schema/node.schema.yml +++ b/core/modules/node/config/schema/node.schema.yml @@ -16,8 +16,13 @@ node.type.*: type: label label: 'Name' type: - type: string + type: machine_name label: 'Machine-readable name' + constraints: + # Node type machine names are specifically limited to 32 characters. + # @see \Drupal\node\NodeTypeForm::form() + Length: + max: 32 description: type: text label: 'Description' diff --git a/core/modules/responsive_image/config/schema/responsive_image.schema.yml b/core/modules/responsive_image/config/schema/responsive_image.schema.yml index 8fae6175938bdf77fe47a9f54ef001fed5f9aa1d..32c5d2dac8f464d47db11b15864bc97d1b7930bf 100644 --- a/core/modules/responsive_image/config/schema/responsive_image.schema.yml +++ b/core/modules/responsive_image/config/schema/responsive_image.schema.yml @@ -5,7 +5,7 @@ responsive_image.styles.*: label: 'Responsive image style' mapping: id: - type: string + type: machine_name label: 'Machine-readable name' label: type: label diff --git a/core/modules/search/config/schema/search.schema.yml b/core/modules/search/config/schema/search.schema.yml index 64d75b8db9a982603d331ae4b02857c5577fa006..1f47b02c0afc4d3eec4f24a247518e6ea8e8cf95 100644 --- a/core/modules/search/config/schema/search.schema.yml +++ b/core/modules/search/config/schema/search.schema.yml @@ -72,7 +72,7 @@ search.page.*: label: 'Search page' mapping: id: - type: string + type: machine_name label: 'ID' label: type: label diff --git a/core/modules/shortcut/config/schema/shortcut.schema.yml b/core/modules/shortcut/config/schema/shortcut.schema.yml index 7d9420ba12127336ecd1415759ed2a21838f8dca..6fceb5741e27c6292f94456405fcbd715717d0f3 100644 --- a/core/modules/shortcut/config/schema/shortcut.schema.yml +++ b/core/modules/shortcut/config/schema/shortcut.schema.yml @@ -5,8 +5,15 @@ shortcut.set.*: label: 'Shortcut settings' mapping: id: - type: string + type: machine_name label: 'ID' + # Shortcut set IDs are specifically limited to 23 characters, and allow + # dashes but not underscores. + # @see \Drupal\shortcut\ShortcutSetForm::form() + constraints: + Regex: '/^[a-z0-9-]+$/' + Length: + max: 23 label: type: label label: 'Label' diff --git a/core/modules/shortcut/tests/src/Kernel/ShortcutSetValidationTest.php b/core/modules/shortcut/tests/src/Kernel/ShortcutSetValidationTest.php index a27f3980d18658214687c2c138c8ace96d034e5d..dd9dbc1bc0d15900b51b3b55abfe93b4f1ee0097 100644 --- a/core/modules/shortcut/tests/src/Kernel/ShortcutSetValidationTest.php +++ b/core/modules/shortcut/tests/src/Kernel/ShortcutSetValidationTest.php @@ -26,10 +26,35 @@ protected function setUp(): void { $this->installEntitySchema('shortcut'); $this->entity = ShortcutSet::create([ - 'id' => 'test', + 'id' => 'test-shortcut-set', 'label' => 'Test', ]); $this->entity->save(); } + /** + * Shortcut set IDs are atypical: they allow dashes and disallow underscores. + */ + public function providerInvalidMachineNameCharacters(): array { + $cases = parent::providerInvalidMachineNameCharacters(); + + // Remove the existing test case that verifies a machine name containing + // dashes is invalid. + $this->assertSame(['dash-separated', FALSE], $cases['INVALID: dash separated']); + unset($cases['INVALID: dash separated']); + // And instead add a test case that verifies it is allowed for shortcut + // sets. + $cases['VALID: dash separated'] = ['dash-separated', TRUE]; + + // Remove the existing test case that verifies a machine name containing + // underscores is valid. + $this->assertSame(['underscore_separated', TRUE], $cases['VALID: underscore separated']); + unset($cases['VALID: underscore separated']); + // And instead add a test case that verifies it is disallowed for shortcut + // sets. + $cases['INVALID: underscore separated'] = ['underscore_separated', FALSE]; + + return $cases; + } + } diff --git a/core/modules/system/config/schema/system.schema.yml b/core/modules/system/config/schema/system.schema.yml index 5d2a5c3fc7cc92650c4a02d8b7d4ca3916e0ad7e..3de5d43481243a669e30d69d5f3dad533dac77a0 100644 --- a/core/modules/system/config/schema/system.schema.yml +++ b/core/modules/system/config/schema/system.schema.yml @@ -212,8 +212,15 @@ system.menu.*: label: 'Menu' mapping: id: - type: string + type: machine_name label: 'ID' + # Menu IDs are specifically limited to 32 characters, and allow dashes but not + # underscores. + # @see \Drupal\menu_ui\MenuForm::form() + constraints: + Regex: '/^[a-z0-9-]+$/' + Length: + max: 32 label: type: label label: 'Label' @@ -229,7 +236,7 @@ system.action.*: label: 'System action' mapping: id: - type: string + type: machine_name label: 'ID' label: type: label diff --git a/core/modules/system/tests/src/Kernel/Entity/MenuValidationTest.php b/core/modules/system/tests/src/Kernel/Entity/MenuValidationTest.php index 1c1ebd752b680e1708cc1fa3ccb01e9c148b92c5..7cb041759b2d64f109f9bc2546b2bf9e8ca8558d 100644 --- a/core/modules/system/tests/src/Kernel/Entity/MenuValidationTest.php +++ b/core/modules/system/tests/src/Kernel/Entity/MenuValidationTest.php @@ -19,10 +19,33 @@ protected function setUp(): void { parent::setUp(); $this->entity = Menu::create([ - 'id' => 'test', + 'id' => 'test-menu', 'label' => 'Test', ]); $this->entity->save(); } + /** + * Menu IDs are atypical: they allow dashes and disallow underscores. + */ + public function providerInvalidMachineNameCharacters(): array { + $cases = parent::providerInvalidMachineNameCharacters(); + + // Remove the existing test case that verifies a machine name containing + // dashes is invalid. + $this->assertSame(['dash-separated', FALSE], $cases['INVALID: dash separated']); + unset($cases['INVALID: dash separated']); + // And instead add a test case that verifies it is allowed for menus. + $cases['VALID: dash separated'] = ['dash-separated', TRUE]; + + // Remove the existing test case that verifies a machine name containing + // underscores is valid. + $this->assertSame(['underscore_separated', TRUE], $cases['VALID: underscore separated']); + unset($cases['VALID: underscore separated']); + // And instead add a test case that verifies it is disallowed for menus. + $cases['INVALID: underscore separated'] = ['underscore_separated', FALSE]; + + return $cases; + } + } diff --git a/core/modules/taxonomy/config/schema/taxonomy.schema.yml b/core/modules/taxonomy/config/schema/taxonomy.schema.yml index 1b472752ab120524bdbaa557cbc1ed9e1a2d9742..3e24333784b2d1808cf8bb38da3cf47e12a48d48 100644 --- a/core/modules/taxonomy/config/schema/taxonomy.schema.yml +++ b/core/modules/taxonomy/config/schema/taxonomy.schema.yml @@ -22,8 +22,13 @@ taxonomy.vocabulary.*: type: label label: 'Name' vid: - type: string + type: machine_name label: 'Machine name' + # Vocabulary machine names are specifically limited to 32 characters. + # @see \Drupal\taxonomy\VocabularyForm::form() + constraints: + Length: + max: 32 description: type: label label: 'Description' diff --git a/core/modules/tour/config/schema/tour.schema.yml b/core/modules/tour/config/schema/tour.schema.yml index d8ffed213379ec5289b62137e70908ddc54ed41a..05fcb5b0581ed26bacaecdce3110694998a38ca7 100644 --- a/core/modules/tour/config/schema/tour.schema.yml +++ b/core/modules/tour/config/schema/tour.schema.yml @@ -5,7 +5,7 @@ tour.tour.*: label: 'Tour settings' mapping: id: - type: string + type: machine_name label: 'ID' label: type: label diff --git a/core/modules/tour/tests/src/Kernel/TourValidationTest.php b/core/modules/tour/tests/src/Kernel/TourValidationTest.php new file mode 100644 index 0000000000000000000000000000000000000000..42b0d28640d158cd1cda54c50e5b633cd35d3a07 --- /dev/null +++ b/core/modules/tour/tests/src/Kernel/TourValidationTest.php @@ -0,0 +1,34 @@ +<?php + +namespace Drupal\Tests\tour\Kernel; + +use Drupal\KernelTests\Core\Config\ConfigEntityValidationTestBase; +use Drupal\tour\Entity\Tour; + +/** + * Tests validation of tour entities. + * + * @group tour + */ +class TourValidationTest extends ConfigEntityValidationTestBase { + + /** + * {@inheritdoc} + */ + protected static $modules = ['tour']; + + /** + * {@inheritdoc} + */ + protected function setUp(): void { + parent::setUp(); + + $this->entity = Tour::create([ + 'id' => 'test', + 'label' => 'Test', + 'module' => 'system', + ]); + $this->entity->save(); + } + +} diff --git a/core/modules/user/config/schema/user.schema.yml b/core/modules/user/config/schema/user.schema.yml index b778163bf5c0a85a6dd3351f1db4987d506679a3..2e61a61599ad907df5082c07cd6430216453218b 100644 --- a/core/modules/user/config/schema/user.schema.yml +++ b/core/modules/user/config/schema/user.schema.yml @@ -108,7 +108,7 @@ user.role.*: label: 'User role settings' mapping: id: - type: string + type: machine_name label: 'ID' label: type: label diff --git a/core/modules/views/config/schema/views.schema.yml b/core/modules/views/config/schema/views.schema.yml index 0d38b89d1bf4b8cf1bfba82578b53a72f0b5d155..1939f88fb8acbca755d4465dd8811cf95e210c7f 100644 --- a/core/modules/views/config/schema/views.schema.yml +++ b/core/modules/views/config/schema/views.schema.yml @@ -70,8 +70,13 @@ views.view.*: label: 'View' mapping: id: - type: string + type: machine_name label: 'ID' + constraints: + Length: + # View IDs are specifically limited to 128 characters. + # @see \Drupal\views_ui\ViewAddForm::form() + max: 128 label: type: label label: 'Label' diff --git a/core/modules/workflows/config/schema/workflows.schema.yml b/core/modules/workflows/config/schema/workflows.schema.yml index 587a2d5f5b530cd966be5d36bf33e36dd20bc9c9..72a47a353e2b5c1668ebc2f2d6b896f5ac40f981 100644 --- a/core/modules/workflows/config/schema/workflows.schema.yml +++ b/core/modules/workflows/config/schema/workflows.schema.yml @@ -3,7 +3,7 @@ workflows.workflow.*: label: 'Workflow' mapping: id: - type: string + type: machine_name label: 'ID' label: type: label diff --git a/core/tests/Drupal/KernelTests/Core/Config/ConfigEntityValidationTestBase.php b/core/tests/Drupal/KernelTests/Core/Config/ConfigEntityValidationTestBase.php index 3512517f2554bfb5a092d6417a1ffc52df8c09ef..2aa8cedb87442c7868cfd37a7ca2045852b25aad 100644 --- a/core/tests/Drupal/KernelTests/Core/Config/ConfigEntityValidationTestBase.php +++ b/core/tests/Drupal/KernelTests/Core/Config/ConfigEntityValidationTestBase.php @@ -39,6 +39,106 @@ protected function setUp(): void { $this->container = $this->container->get('kernel')->getContainer(); } + /** + * Ensures that the entity created in ::setUp() has no validation errors. + */ + public function testEntityIsValid(): void { + $this->assertInstanceOf(ConfigEntityInterface::class, $this->entity); + $this->assertValidationErrors([]); + } + + /** + * Returns the validation constraints applied to the entity's ID. + * + * If the entity type does not define an ID key, the test will fail. If an ID + * key is defined but is not using the `machine_name` data type, the test will + * be skipped. + * + * @return array[] + * The validation constraint configuration applied to the entity's ID. + */ + protected function getMachineNameConstraints(): array { + $id_key = $this->entity->getEntityType()->getKey('id'); + $this->assertNotEmpty($id_key, "The entity under test does not define an ID key."); + + $data_definition = $this->entity->getTypedData() + ->get($id_key) + ->getDataDefinition(); + if ($data_definition->getDataType() === 'machine_name') { + return $data_definition->getConstraints(); + } + else { + $this->markTestSkipped("The entity's ID key does not use the machine_name data type."); + } + } + + /** + * Data provider for ::testInvalidMachineNameCharacters(). + * + * @return array[] + * The test cases. + */ + public function providerInvalidMachineNameCharacters(): array { + return [ + 'INVALID: space separated' => ['space separated', FALSE], + 'INVALID: dash separated' => ['dash-separated', FALSE], + 'INVALID: uppercase letters' => ['Uppercase_Letters', FALSE], + 'INVALID: period separated' => ['period.separated', FALSE], + 'VALID: underscore separated' => ['underscore_separated', TRUE], + ]; + } + + /** + * Tests that the entity's ID is tested for invalid characters. + * + * @param string $machine_name + * A machine name to test. + * @param bool $is_expected_to_be_valid + * Whether this machine name is expected to be considered valid. + * + * @dataProvider providerInvalidMachineNameCharacters + */ + public function testInvalidMachineNameCharacters(string $machine_name, bool $is_expected_to_be_valid): void { + $constraints = $this->getMachineNameConstraints(); + + $this->assertNotEmpty($constraints['Regex']); + $this->assertIsString($constraints['Regex']); + + $id_key = $this->entity->getEntityType()->getKey('id'); + if ($is_expected_to_be_valid) { + $expected_errors = []; + } + else { + $expected_errors = [$id_key => 'This value is not valid.']; + } + + $this->entity->set( + $id_key, + $machine_name + ); + $this->assertValidationErrors($expected_errors); + } + + /** + * Tests that the entity ID's length is validated if it is a machine name. + */ + public function testMachineNameLength(): void { + $constraints = $this->getMachineNameConstraints(); + + $max_length = $constraints['Length']['max']; + $this->assertIsInt($max_length); + $this->assertGreaterThan(0, $max_length); + + $id_key = $this->entity->getEntityType()->getKey('id'); + $this->entity->set( + $id_key, + mb_strtolower($this->randomMachineName($max_length + 2)) + ); + $this->assertValidationErrors([ + $id_key => 'This value is too long. It should have <em class="placeholder">' . $max_length . '</em> characters or less.', + ]); + } + /** * Data provider for ::testConfigDependenciesValidation(). * @@ -159,11 +259,6 @@ public function providerConfigDependenciesValidation(): array { * @dataProvider providerConfigDependenciesValidation */ public function testConfigDependenciesValidation(array $dependencies, array $expected_messages): void { - $this->assertInstanceOf(ConfigEntityInterface::class, $this->entity); - - // The entity should have valid data to begin with. - $this->assertValidationErrors([]); - // Add the dependencies we were given to the dependencies that may already // exist in the entity. $dependencies = NestedArray::mergeDeep($dependencies, $this->entity->getDependencies()); diff --git a/core/tests/Drupal/KernelTests/Core/Config/ConfigSchemaTest.php b/core/tests/Drupal/KernelTests/Core/Config/ConfigSchemaTest.php index 602ca3624e3f1ff7a48914614e604cbaaaf5fad7..05e7b54c79a02bda77797d743cd9ac27aad8226d 100644 --- a/core/tests/Drupal/KernelTests/Core/Config/ConfigSchemaTest.php +++ b/core/tests/Drupal/KernelTests/Core/Config/ConfigSchemaTest.php @@ -176,7 +176,7 @@ public function testSchemaMapping() { $expected['class'] = Mapping::class; $expected['definition_class'] = '\Drupal\Core\TypedData\MapDataDefinition'; $expected['unwrap_for_canonical_representation'] = TRUE; - $expected['mapping']['name']['type'] = 'string'; + $expected['mapping']['name']['type'] = 'machine_name'; $expected['mapping']['uuid']['type'] = 'uuid'; $expected['mapping']['uuid']['label'] = 'UUID'; $expected['mapping']['langcode']['type'] = 'string'; @@ -185,7 +185,6 @@ public function testSchemaMapping() { $expected['mapping']['status']['label'] = 'Status'; $expected['mapping']['dependencies']['type'] = 'config_dependencies'; $expected['mapping']['dependencies']['label'] = 'Dependencies'; - $expected['mapping']['name']['type'] = 'string'; $expected['mapping']['label']['type'] = 'label'; $expected['mapping']['label']['label'] = 'Label'; $expected['mapping']['effects']['type'] = 'sequence';