On the eve of POST/PATCH/DELETE config entities support validation

vaplas created an issue. See original summary.

This patch is based on #208 patch.

Review:

1. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php
   @@ -17,12 +19,14 @@
   -abstract class ConfigEntityBase extends Entity implements ConfigEntityInterface {
   +abstract class ConfigEntityBase extends Entity implements ConfigEntityInterface, ValidatableInterface {
   ...
   +  use ConfigValidatableTrait;
   

   To test the all config entities (I hope).

2. +++ b/core/modules/rest/tests/modules/rest_test_access_helper/rest_test_access_helper.module
   @@ -0,0 +1,18 @@
   +function rest_test_access_helper_entity_type_alter(&$entity_info) {
   +  $entity_info['base_field_override']->setHandlerClass("access", BaseFieldOverrideAccessControlHandlerHelper::class);
   +  $entity_info['editor']->setHandlerClass("access", EditorAccessControlHandlerHelper::class);
   +  $entity_info['field_config']->setHandlerClass("access", FieldConfigAccessControlHandlerHelper::class);
   +  $entity_info['field_storage_config']->setHandlerClass("access", FieldStorageConfigAccessControlHandlerHelper::class);
   +  $entity_info['filter_format']->setHandlerClass("access", FilterFormatAccessControlHandlerHelper::class);
   +}
   

   Most entities are porting like a charm, but these have access problems. In patch, all access controllers are changed via this helper module.

3. +++ b/core/modules/rest/tests/modules/rest_test_access_helper/src/BaseFieldOverrideAccessControlHandlerHelper.php
   @@ -0,0 +1,20 @@
   +    return AccessResult::allowedIfHasPermission($account, 'administer node fields');
   
   +++ b/core/modules/rest/tests/modules/rest_test_access_helper/src/FieldConfigAccessControlHandlerHelper.php
   @@ -0,0 +1,21 @@
   +    return AccessResult::allowedIfHasPermissions($account, ['administer node fields']);
   
   +++ b/core/modules/rest/tests/modules/rest_test_access_helper/src/FieldStorageConfigAccessControlHandlerHelper.php
   @@ -0,0 +1,20 @@
   +    return AccessResult::allowedIfHasPermission($account, 'administer node fields');
   

   Of course, this is stupid permissions. We should not check only specific 'node' type. But just to demonstrate the passing of the tests :)

4. +++ b/core/modules/rest/tests/modules/rest_test_base_field_override_helper/rest_test_base_field_override_helper.module
   @@ -0,0 +1,22 @@
   +function rest_test_base_field_override_helper_entity_base_field_info(\Drupal\Core\Entity\EntityTypeInterface $entity_type) {
   

   Additional module for stamping the definitions, for the BaseFieldOverride test.

5. +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php
   @@ -798,12 +798,13 @@ public function testPost() {
   -      $label_field = $this->entity->getEntityType()->hasKey('label') ? $this->entity->getEntityType()->getKey('label') : static::$labelFieldName;
   +      $label_field = static::$labelFieldName ?: $this->entity->getEntityType()->getKey('label');
   ...
   @@ -1042,7 +1043,7 @@ public function testPatch() {
   -    $label_field = $this->entity->getEntityType()->hasKey('label') ? $this->entity->getEntityType()->getKey('label') : static::$labelFieldName;
   +    $label_field = static::$labelFieldName ?: $this->entity->getEntityType()->getKey('label');
   ...
   @@ -1327,7 +1328,7 @@ protected function getEntityResourcePostUrl() {
   -    $label_field = $this->entity->getEntityType()->hasKey('label') ? $this->entity->getEntityType()->getKey('label') : static::$labelFieldName;
   +    $label_field = static::$labelFieldName ?: $this->entity->getEntityType()->getKey('label');
   

   In some cases, we need the ability to redefine the label, even if the entity has it defined. Example FieldStorageConfig use id for label:

    * @ConfigEntityType(
   ...
    *   entity_keys = {
    *     "id" = "id",
    *     "label" = "id"
    *   },

6. +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php
   @@ -798,12 +798,13 @@ public function testPost() {
   -      $this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nlabel: This value should be of the correct primitive type.\n", $response);
   
   +      $this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\n$label: This value should be of the correct primitive type.\n", $response);
   

   use $label instead of 'label', because now it can be different.

7. +++ b/core/modules/rest/tests/src/Functional/EntityResource/Vocabulary/VocabularyJsonAnonTest.php
   @@ -21,12 +21,4 @@ class VocabularyJsonAnonTest extends VocabularyResourceTestBase {
   -  /**
   -   * Disable the GET test coverage due to bug in taxonomy module.
   -   * @todo Fix in https://www.drupal.org/node/2805281: remove this override.
   -   */
   -  public function testGet() {
   -    $this->markTestSkipped();
   -  }
   -
   

   A pleasant surprise + few other nit typos.

I also forgot about assertNormalizationEdgeCases part of porting.

Well, fair question) Maybe I was wrong, and I'm ready close this issue without problems. I just thought that if we add basic support for POST/PATCH/DELETE testing to our tests after:

+++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php
@@ -392,9 +401,8 @@ protected function getBaseRoute($canonical_path, $method) {
-    if ($this->isConfigEntityResource()) {
-      // Currently only GET is supported for Config Entities.
-      // @todo Remove when supported https://www.drupal.org/node/2300677
+    // Without validation, it's impossible to support creation or modification.
+    if (!$this->entityType->entityClassImplements(ValidatableInterface::class)) {
       $unsupported_methods = ['POST', 'PUT', 'DELETE', 'PATCH'];

it will allow us to introduce ValidatableInterface on a more global level, like:

+++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php
@@ -17,12 +19,14 @@
-abstract class ConfigEntityBase extends Entity implements ConfigEntityInterface {
+abstract class ConfigEntityBase extends Entity implements ConfigEntityInterface, ValidatableInterface {

In any case, I was interested to check our config entities on base operations.