Issue #2264179 by Gábor Hojtsy, vijaycs85, alexpott, benjy: Clarify use of...

Issue #2264179 by Gábor Hojtsy, vijaycs85, alexpott, benjy: Clarify use of property/undefined and add an ignore type in configuration schema.

core/config/schema/core.data_types.schema.yml

+# Base types provided by Drupal core.
+
+# Read https://drupal.org/node/1905070 for more details about configuration
+# schema, types and type resolution.
+
+# Undefined type used by the system to assign to elements at any level where
+# configuration schema is not defined. Using explicitly has the same effect as
+# not defining schema, so there is no point in doing that.
+undefined:
+  label: 'Undefined'
+  class: '\Drupal\Core\Config\Schema\Undefined'
+
+# Explicit type to use when no data typing is possible. Instead of using this
+# type, we strongly suggest you use configuration structures that can be
+# described with other structural elements of schema, and describe your schema
+# with those elements.
+ignore:
+  label: 'Ignore'
+  class: '\Drupal\Core\Config\Schema\Ignore'
+
 # Basic scalar data types from typed data.
 boolean:
   label: 'Boolean'
@@ -18,10 +38,7 @@ uri:
   label: 'Uri'
   class: '\Drupal\Core\TypedData\Plugin\DataType\Uri'
 
-# Basic data types for configuration.
-undefined:
-  label: 'Undefined'
-  class: '\Drupal\Core\Config\Schema\Property'
+# Container data types for lists with known and unknown keys.
 mapping:
   label: Mapping
   class: '\Drupal\Core\Config\Schema\Mapping'
@@ -29,11 +46,6 @@ sequence:
   label: Sequence
   class: '\Drupal\Core\Config\Schema\Sequence'
 
-# Default mapping for unknown types or types not found.
-default:
-  type: undefined
-  label: 'Unknown'
-
 # Simple extended data types:
 
 # Human readable string that must be plain text and editable with a text field.
@@ -59,6 +71,11 @@ date_format:
   label: 'PHP date format'
   translatable: true
 
+# HTML color value.
+color_hex:
+  type: string
+  label: 'Color'
+
 # Complex extended data types:
 
 # Mail text with subject and body parts.
@@ -194,11 +211,6 @@ route:
         - type: string
           label: 'Param'
 
-# HTML color value.
-color_hex:
-  type: string
-  label: 'Color'
-
 # Config dependencies.
 config_dependencies:
   type: mapping

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

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Config\Schema\Ignore.
+ */
+
+namespace Drupal\Core\Config\Schema;
+
+/**
+ * Configuration property to ignore.
+ */
+class Ignore extends Element {
+
+  /**
+   * {@inheritdoc}.
+   */
+  public function validate() {
+    return TRUE;
+  }
+}

core/lib/Drupal/Core/Config/Schema/Property.php → core/lib/Drupal/Core/Config/Schema/Undefined.php

@@ -2,21 +2,20 @@
 
 /**
  * @file
- * Contains \Drupal\Core\Config\Schema\Sequence.
+ * Contains \Drupal\Core\Config\Schema\Undefined.
  */
 
 namespace Drupal\Core\Config\Schema;
 
 /**
- * Generic configuration property.
+ * Undefined configuration element.
  */
-class Property extends Element {
+class Undefined extends Element {
 
   /**
-   * Implements TypedDataInterface::validate().
+   * {@inheritdoc}.
    */
   public function validate() {
     return isset($this->value);
   }
-
 }

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

@@ -8,10 +8,12 @@
 namespace Drupal\Core\Config;
 
 use Drupal\Component\Utility\String;
+use Drupal\Core\Config\Schema\Ignore;
 use Drupal\Core\Config\Schema\SchemaIncompleteException;
 use Drupal\Core\TypedData\PrimitiveInterface;
 use Drupal\Core\TypedData\Type\FloatInterface;
 use Drupal\Core\TypedData\Type\IntegerInterface;
+use Drupal\Core\Config\Schema\Undefined;
 
 /**
  * Provides a base class for configuration objects with storage support.
@@ -167,37 +169,36 @@ protected function validateValue($key, $value) {
    *   Exception on unsupported/undefined data type deducted.
    */
   protected function castValue($key, $value) {
-    if ($value === NULL) {
-      $value = NULL;
+    $element = FALSE;
+    try {
+      $element = $this->getSchemaWrapper()->get($key);
     }
-    elseif (is_scalar($value)) {
-      try {
-        $element = $this->getSchemaWrapper()->get($key);
-        if ($element instanceof PrimitiveInterface) {
-          // Special handling for integers and floats since the configuration
-          // system is primarily concerned with saving values from the Form API
-          // we have to special case the meaning of an empty string for numeric
-          // types. In PHP this would be casted to a 0 but for the purposes of
-          // configuration we need to treat this as a NULL.
-          if ($value === '' && ($element instanceof IntegerInterface || $element instanceof FloatInterface)) {
-            $value = NULL;
-          }
-          else {
-            $value = $element->getCastedValue();
-          }
+    catch (SchemaIncompleteException $e) {
+      // @todo Consider making schema handling more strict by throwing
+      // SchemaIncompleteException for all incomplete schema conditions *and*
+      // throwing it forward. See https://drupal.org/node/2183983.
+      // Until then, we need to handle the Undefined case below.
+    }
+    // Do not cast value if it is unknown or defined to be ignored.
+    if ($element && ($element instanceof Undefined || $element instanceof Ignore)) {
+      return $value;
+    }
+    if ((is_scalar($value) || $value === NULL)) {
+      if ($element && $element instanceof PrimitiveInterface) {
+        // Special handling for integers and floats since the configuration
+        // system is primarily concerned with saving values from the Form API
+        // we have to special case the meaning of an empty string for numeric
+        // types. In PHP this would be casted to a 0 but for the purposes of
+        // configuration we need to treat this as a NULL.
+        $empty_value =  $value === '' && ($element instanceof IntegerInterface || $element instanceof FloatInterface);
+
+        if ($value === NULL || $empty_value) {
+          $value = NULL;
         }
         else {
-          // Config only supports primitive data types. If the config schema
-          // does define a type $element will be an instance of
-          // \Drupal\Core\Config\Schema\Property. Convert it to string since it
-          // is the safest possible type.
-          $value = $element->getString();
+          $value = $element->getCastedValue();
         }
       }
-      catch (SchemaIncompleteException $e) {
-        // @todo throw an exception due to an incomplete schema.
-        // Fix as part of https://drupal.org/node/2183983.
-      }
     }
     else {
       // Throw exception on any non-scalar or non-array value.

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

@@ -152,9 +152,8 @@ public function getDefinition($base_plugin_id) {
       $type = $name;
     }
     else {
-      // If we don't have definition, return the 'default' element.
-      // This should map to 'undefined' type by default, unless overridden.
-      $type = 'default';
+      // If we don't have definition, return the 'undefined' element.
+      $type = 'undefined';
     }
     $definition = $definitions[$type];
     // Check whether this type is an extension of another one and compile it.
@@ -326,10 +325,9 @@ protected function replaceVariable($value, $data) {
    * {@inheritdoc}
    */
   public function hasConfigSchema($name) {
-    // The schema system falls back on the Property class for unknown types.
-    // See http://drupal.org/node/1905230
+    // The schema system falls back on the Undefined class for unknown types.
     $definition = $this->getDefinition($name);
-    return is_array($definition) && ($definition['class'] != '\Drupal\Core\Config\Schema\Property');
+    return is_array($definition) && ($definition['class'] != '\Drupal\Core\Config\Schema\Undefined');
   }
 
 }

core/modules/config/lib/Drupal/config/Tests/ConfigSchemaTest.php

@@ -7,6 +7,8 @@
 
 namespace Drupal\config\Tests;
 
+use Drupal\Core\Config\FileStorage;
+use Drupal\Core\Config\InstallStorage;
 use Drupal\Core\TypedData\Type\IntegerInterface;
 use Drupal\Core\TypedData\Type\StringInterface;
 use Drupal\simpletest\DrupalUnitTestBase;
@@ -40,15 +42,15 @@ public function setUp() {
    * Tests the basic metadata retrieval layer.
    */
   function testSchemaMapping() {
-    // Nonexistent configuration key will have Unknown as metadata.
+    // Nonexistent configuration key will have Undefined as metadata.
     $this->assertIdentical(FALSE, \Drupal::service('config.typed')->hasConfigSchema('config_schema_test.no_such_key'));
     $definition = \Drupal::service('config.typed')->getDefinition('config_schema_test.no_such_key');
     $expected = array();
-    $expected['label'] = 'Unknown';
-    $expected['class'] = '\Drupal\Core\Config\Schema\Property';
+    $expected['label'] = 'Undefined';
+    $expected['class'] = '\Drupal\Core\Config\Schema\Undefined';
     $this->assertEqual($definition, $expected, 'Retrieved the right metadata for nonexistent configuration.');
 
-    // Configuration file without schema will return Unknown as well.
+    // Configuration file without schema will return Undefined as well.
     $this->assertIdentical(FALSE, \Drupal::service('config.typed')->hasConfigSchema('config_schema_test.noschema'));
     $definition = \Drupal::service('config.typed')->getDefinition('config_schema_test.noschema');
     $this->assertEqual($definition, $expected, 'Retrieved the right metadata for configuration with no schema.');
@@ -68,13 +70,13 @@ function testSchemaMapping() {
     $definition = $config['testitem']->getDataDefinition();
     $expected = array();
     $expected['label'] = 'Test item';
-    $expected['class'] = '\Drupal\Core\Config\Schema\Property';
+    $expected['class'] = '\Drupal\Core\Config\Schema\Undefined';
     $expected['type'] = 'undefined';
     $this->assertEqual($definition, $expected, 'Automatic type detected for a scalar is undefined.');
     $definition = $config['testlist']->getDataDefinition();
     $expected = array();
     $expected['label'] = 'Test list';
-    $expected['class'] = '\Drupal\Core\Config\Schema\Property';
+    $expected['class'] = '\Drupal\Core\Config\Schema\Undefined';
     $expected['type'] = 'undefined';
     $this->assertEqual($definition, $expected, 'Automatic type detected for a list is undefined.');
 
@@ -93,6 +95,40 @@ function testSchemaMapping() {
     );
     $this->assertEqual($definition, $expected, 'Retrieved the right metadata for system.maintenance');
 
+    // Mixed schema with ignore elements.
+    $definition = \Drupal::service('config.typed')->getDefinition('config_schema_test.ignore');
+    $expected = array();
+    $expected['label'] = 'Ignore test';
+    $expected['class'] = '\Drupal\Core\Config\Schema\Mapping';
+    $expected['mapping']['label'] = array(
+      'label' =>  'Label',
+      'type' => 'label',
+    );
+    $expected['mapping']['irrelevant'] = array(
+      'label' => 'Irrelevant',
+      'type' => 'ignore',
+    );
+    $expected['mapping']['indescribable'] = array(
+      'label' => 'Indescribable',
+      'type' => 'ignore',
+    );
+    $expected['mapping']['weight'] = array(
+      'label' => 'Weight',
+      'type' => 'integer',
+    );
+    $this->assertEqual($definition, $expected);
+
+    // The ignore elements themselves.
+    $definition = \Drupal::service('config.typed')->get('config_schema_test.ignore')->get('irrelevant')->getDataDefinition();
+    $expected = array();
+    $expected['type'] = 'ignore';
+    $expected['label'] = 'Irrelevant';
+    $expected['class'] = '\Drupal\Core\Config\Schema\Ignore';
+    $this->assertEqual($definition, $expected);
+    $definition = \Drupal::service('config.typed')->get('config_schema_test.ignore')->get('indescribable')->getDataDefinition();
+    $expected['label'] = 'Indescribable';
+    $this->assertEqual($definition, $expected);
+
     // More complex case, generic type. Metadata for image style.
     $definition = \Drupal::service('config.typed')->getDefinition('image.style.large');
     $expected = array();
@@ -138,7 +174,7 @@ function testSchemaMapping() {
 
     $this->assertEqual($definition, $expected, 'Retrieved the right metadata for the first effect of image.style.medium');
 
-    // More complex, multiple filesystem marker test.
+    // More complex, several level deep test.
     $definition = \Drupal::service('config.typed')->getDefinition('config_schema_test.someschema.somemodule.section_one.subsection');
     // This should be the schema of config_schema_test.someschema.somemodule.*.*.
     $expected = array();
@@ -254,7 +290,7 @@ public function testConfigSaveWithSchema() {
       'integer' => '100',
       'null_integer' => '',
       'boolean' => 1,
-      // If the config schema doesn't have a type it should be casted to string.
+      // If the config schema doesn't have a type it shouldn't be casted.
       'no_type' => 1,
       'mapping' => array(
         'string' => 1
@@ -277,7 +313,7 @@ public function testConfigSaveWithSchema() {
       'integer' => 100,
       'null_integer' => NULL,
       'boolean' => TRUE,
-      'no_type' => '1',
+      'no_type' => 1,
       'mapping' => array(
         'string' => '1'
       ),
@@ -300,6 +336,14 @@ public function testConfigSaveWithSchema() {
       ->setData($untyped_values)
       ->save();
     $this->assertIdentical(\Drupal::config('config_schema_test.no_schema_data_types')->get(), $untyped_values);
+
+    // Ensure that configuration objects with keys marked as ignored are not
+    // changed when saved. The 'config_schema_test.ignore' will have been saved
+    // during the installation of configuration in the setUp method.
+    $extension_path = drupal_get_path('module', 'config_schema_test');
+    $install_storage = new FileStorage($extension_path . '/' . InstallStorage::CONFIG_INSTALL_DIRECTORY);
+    $original_data = $install_storage->read('config_schema_test.ignore');
+    $this->assertIdentical(\Drupal::config('config_schema_test.ignore')->get(), $original_data);
   }
 
   /**

core/modules/config/lib/Drupal/config/Tests/ConfigSchemaTestBase.php

@@ -8,7 +8,6 @@
 namespace Drupal\config\Tests;
 
 use Drupal\Core\Config\Schema\ArrayElement;
-use Drupal\Core\Config\Schema\Property;
 use Drupal\Core\Config\TypedConfigManagerInterface;
 use Drupal\Core\TypedData\Type\BooleanInterface;
 use Drupal\Core\TypedData\Type\StringInterface;
@@ -84,36 +83,34 @@ public function assertConfigSchema(TypedConfigManagerInterface $typed_config, $c
    *   Returns mixed value.
    */
   protected function checkValue($key, $value) {
-
+    $element = FALSE;
     try {
       $element = $this->schema->get($key);
     }
     catch (SchemaIncompleteException $e) {
-      $this->fail("{$this->configName}:$key has no schema.");
+      if (is_scalar($value) || $value === NULL) {
+        $this->fail("{$this->configName}:$key has no schema.");
+      }
     }
+    // Do not check value if it is defined to be ignored.
+    if ($element && $element instanceof Ignore) {
+      return $value;
+    }
+
     if (is_scalar($value) || $value === NULL) {
       $success = FALSE;
       $type = gettype($value);
       if ($element instanceof PrimitiveInterface) {
-        if ($type == 'integer' && $element instanceof IntegerInterface) {
-          $success = TRUE;
-        }
-        if ($type == 'double' && $element instanceof FloatInterface) {
-          $success = TRUE;
-        }
-        if ($type == 'boolean' && $element instanceof BooleanInterface) {
-          $success = TRUE;
-        }
-        if ($type == 'string' && ($element instanceof StringInterface || $element instanceof Property)) {
-          $success = TRUE;
-        }
-        // Null values are allowed for all scalar types.
-        if ($value === NULL) {
-          $success = TRUE;
-        }
+        $success =
+          ($type == 'integer' && $element instanceof IntegerInterface) ||
+          ($type == 'double' && $element instanceof FloatInterface) ||
+          ($type == 'boolean' && $element instanceof BooleanInterface) ||
+          ($type == 'string' && $element instanceof StringInterface) ||
+          // Null values are allowed for all types.
+          ($value === NULL);
       }
+      $class = get_class($element);
       if (!$success) {
-        $class = get_class($element);
         $this->fail("{$this->configName}:$key has the wrong schema. Variable type is $type and schema class is $class.");
       }
     }

core/modules/config/tests/config_schema_test/config/install/config_schema_test.ignore.yml

+label: 'Label string'
+irrelevant: 123
+indescribable:
+  abc: 789
+  def:
+   - 456
+   - 'abc'
+  xyz: 13.4
+weight: 27

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

@@ -132,3 +132,20 @@ config_schema_test.schema_in_install:
 config_schema_test_integer:
   type: integer
   label: 'Config test integer'
+
+config_schema_test.ignore:
+  type: mapping
+  label: 'Ignore test'
+  mapping:
+    label:
+      type: label
+      label: 'Label'
+    irrelevant:
+      type: ignore
+      label: 'Irrelevant'
+    indescribable:
+      type: ignore
+      label: 'Indescribable'
+    weight:
+      type: integer
+      label: 'Weight'