Issue #2414991 by alexpott: Prevent hook_config_schema_info_alter from adding...

Issue #2414991 by alexpott: Prevent hook_config_schema_info_alter from adding or removing definitions

core/lib/Drupal/Core/Config/Schema/ConfigSchemaAlterException.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Config\Schema\ConfigSchemaAlterException.
+ */
+
+namespace Drupal\Core\Config\Schema;
+
+/**
+ * Exception for when hook_config_schema_info_alter() adds or removes schema.
+ */
+class ConfigSchemaAlterException extends \RuntimeException {
+}

core/lib/Drupal/Core/Config/TypedConfigManager.php

@@ -8,7 +8,9 @@
 namespace Drupal\Core\Config;
 
 use Drupal\Component\Utility\NestedArray;
+use Drupal\Component\Utility\String;
 use Drupal\Core\Cache\CacheBackendInterface;
+use Drupal\Core\Config\Schema\ConfigSchemaAlterException;
 use Drupal\Core\Config\Schema\ConfigSchemaDiscovery;
 use Drupal\Core\Config\Schema\Element;
 use Drupal\Core\Extension\ModuleHandlerInterface;
@@ -296,6 +298,29 @@ public function hasConfigSchema($name) {
     return is_array($definition) && ($definition['class'] != '\Drupal\Core\Config\Schema\Undefined');
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function alterDefinitions(&$definitions) {
+    $discovered_schema = array_keys($definitions);
+    parent::alterDefinitions($definitions);
+    $altered_schema = array_keys($definitions);
+    if ($discovered_schema != $altered_schema) {
+      $added_keys = array_diff($altered_schema, $discovered_schema);
+      $removed_keys = array_diff($discovered_schema, $altered_schema);
+      if (!empty($added_keys) && !empty($removed_keys)) {
+        $message = 'Invoking hook_config_schema_info_alter() has added (@added) and removed (@removed) schema definitions';
+      }
+      elseif (!empty($added_keys)) {
+        $message = 'Invoking hook_config_schema_info_alter() has added (@added) schema definitions';
+      }
+      else {
+        $message = 'Invoking hook_config_schema_info_alter() has removed (@removed) schema definitions';
+      }
+      throw new ConfigSchemaAlterException(String::format($message, ['@added' => implode(',', $added_keys), '@removed' => implode(',', $removed_keys)]));
+    }
+  }
+
   /**
    * {@inheritdoc}
    */

core/lib/Drupal/Core/Plugin/DefaultPluginManager.php

@@ -224,9 +224,7 @@ protected function findDefinitions() {
     foreach ($definitions as $plugin_id => &$definition) {
       $this->processDefinition($definition, $plugin_id);
     }
-    if ($this->alterHook) {
-      $this->moduleHandler->alter($this->alterHook, $definitions);
-    }
+    $this->alterDefinitions($definitions);
     // If this plugin was provided by a module that does not exist, remove the
     // plugin definition.
     foreach ($definitions as $plugin_id => $plugin_definition) {
@@ -242,4 +240,16 @@ protected function findDefinitions() {
     return $definitions;
   }
 
+  /**
+   * Invokes the hook to alter the definitions if the alter hook is set.
+   *
+   * @param $definitions
+   *   The discovered plugin defintions.
+   */
+  protected function alterDefinitions(&$definitions) {
+    if ($this->alterHook) {
+      $this->moduleHandler->alter($this->alterHook, $definitions);
+    }
+  }
+
 }

core/modules/config/src/Tests/ConfigSchemaTest.php

@@ -9,6 +9,7 @@
 
 use Drupal\Core\Config\FileStorage;
 use Drupal\Core\Config\InstallStorage;
+use Drupal\Core\Config\Schema\ConfigSchemaAlterException;
 use Drupal\Core\TypedData\Type\IntegerInterface;
 use Drupal\Core\TypedData\Type\StringInterface;
 use Drupal\simpletest\KernelTestBase;
@@ -436,4 +437,54 @@ function testColonsInSchemaTypeDetermination() {
     $this->assertEqual($definition['type'], 'test_with_parents.plugin_types.*');
   }
 
+  /**
+   * Tests hook_config_schema_info_alter().
+   */
+  public function testConfigSchemaInfoAlter() {
+    /** @var \Drupal\Core\Config\TypedConfigManagerInterface $typed_config */
+    $typed_config = \Drupal::service('config.typed');
+    $typed_config->clearCachedDefinitions();
+
+    // Ensure that keys can not be added or removed by
+    // hook_config_schema_info_alter().
+    \Drupal::state()->set('config_schema_test_exception_remove', TRUE);
+    $message = 'Expected ConfigSchemaAlterException thrown.';
+    try {
+      $typed_config->getDefinitions();
+      $this->fail($message);
+    }
+    catch (ConfigSchemaAlterException $e) {
+      $this->pass($message);
+      $this->assertEqual($e->getMessage(), 'Invoking hook_config_schema_info_alter() has removed (config_schema_test.hook) schema definitions');
+    }
+
+    \Drupal::state()->set('config_schema_test_exception_add', TRUE);
+    $message = 'Expected ConfigSchemaAlterException thrown.';
+    try {
+      $typed_config->getDefinitions();
+      $this->fail($message);
+    }
+    catch (ConfigSchemaAlterException $e) {
+      $this->pass($message);
+      $this->assertEqual($e->getMessage(), 'Invoking hook_config_schema_info_alter() has added (config_schema_test.hook_added_defintion) and removed (config_schema_test.hook) schema definitions');
+    }
+
+    \Drupal::state()->set('config_schema_test_exception_remove', FALSE);
+    $message = 'Expected ConfigSchemaAlterException thrown.';
+    try {
+      $typed_config->getDefinitions();
+      $this->fail($message);
+    }
+    catch (ConfigSchemaAlterException $e) {
+      $this->pass($message);
+      $this->assertEqual($e->getMessage(), 'Invoking hook_config_schema_info_alter() has added (config_schema_test.hook_added_defintion) schema definitions');
+    }
+
+    // Tests that hook_config_schema_info_alter() can add additional metadata to
+    // existing configuration schema.
+    \Drupal::state()->set('config_schema_test_exception_add', FALSE);
+    $definitions = $typed_config->getDefinitions();
+    $this->assertEqual($definitions['config_schema_test.hook']['additional_metadata'], 'new schema info');
+  }
+
 }

core/modules/config/tests/config_schema_test/config/schema/config_schema_test.schema.yml

@@ -207,3 +207,5 @@ test_with_parents.plugin_types.*:
     value:
       type: string
 
+config_schema_test.hook:
+  type: string

core/modules/config/tests/config_schema_test/config_schema_test.module

+<?php
+
+/**
+ * @file
+ * Tests configuration schema functionality.
+ */
+
+/**
+ * Implements hook_config_schema_info_alter().
+ */
+function config_schema_test_config_schema_info_alter(&$definitions) {
+  if (\Drupal::state()->get('config_schema_test_exception_add')) {
+    $definitions['config_schema_test.hook_added_defintion'] = $definitions['config_schema_test.hook'];
+  }
+  if (\Drupal::state()->get('config_schema_test_exception_remove')) {
+    unset($definitions['config_schema_test.hook']);
+  }
+
+  // Since code can not be unloaded only alter the definition if it exists.
+  if (isset($definitions['config_schema_test.hook'])) {
+    $definitions['config_schema_test.hook']['additional_metadata'] = 'new schema info';
+  }
+}

core/modules/system/system.api.php

@@ -580,8 +580,8 @@ function hook_config_import_steps_alter(&$sync_steps, \Drupal\Core\Config\Config
  * configuration schema type to change default labels or form element renderers
  * used for configuration translation.
  *
- * It is strongly advised not to use this hook to add new data types or to
- * change the structure of existing ones. Keep in mind that there are tools
+ * If implementations of this hook add or remove configuration schema a
+ * ConfigSchemaAlterException will be thrown. Keep in mind that there are tools
  * that may use the configuration schema for static analysis of configuration
  * files, like the string extractor for the localization system. Such systems
  * won't work with dynamically defined configuration schemas.
@@ -591,6 +591,9 @@ function hook_config_import_steps_alter(&$sync_steps, \Drupal\Core\Config\Config
  * @param $definitions
  *   Associative array of configuration type definitions keyed by schema type
  *   names. The elements are themselves array with information about the type.
+ *
+ * @see \Drupal\Core\Config\TypedConfigManager
+ * @see \Drupal\Core\Config\Schema\ConfigSchemaAlterException
  */
 function hook_config_schema_info_alter(&$definitions) {
   // Enhance the text and date type definitions with classes to generate proper