Issue #2293773 by Gábor Hojtsy, alexpott, effulgentsia, penyaskito,...

Issue #2293773 by Gábor Hojtsy, alexpott, effulgentsia, penyaskito, hussainweb: Fixed Field allowed values use dots in key names - not allowed in config.

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

@@ -110,7 +110,7 @@ public function get($key = '') {
    * {@inheritdoc}
    */
   public function setData(array $data) {
-    $this->data = $data;
+    parent::setData($data);
     $this->resetOverriddenData();
     return $this;
   }

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

@@ -160,8 +160,12 @@ public function get($key = '') {
    *
    * @return $this
    *   The configuration object.
+   *
+   * @throws \Drupal\Core\Config\ConfigValueException
+   *   If any key in $data in any depth contains a dot.
    */
   public function setData(array $data) {
+    $this->validateKeys($data);
     $this->data = $data;
     return $this;
   }
@@ -176,10 +180,16 @@ public function setData(array $data) {
    *
    * @return $this
    *   The configuration object.
+   *
+   * @throws \Drupal\Core\Config\ConfigValueException
+   *   If $value is an array and any of its keys in any depth contains a dot.
    */
   public function set($key, $value) {
     // The dot/period is a reserved character; it may appear between keys, but
     // not within keys.
+    if (is_array($value)) {
+      $this->validateKeys($value);
+    }
     $parts = explode('.', $key);
     if (count($parts) == 1) {
       $this->data[$key] = $value;
@@ -190,6 +200,28 @@ public function set($key, $value) {
     return $this;
   }
 
+  /**
+   * Validates all keys in a passed in config array structure.
+   *
+   * @param array $data
+   *   Configuration array structure.
+   *
+   * @return null
+   *
+   * @throws \Drupal\Core\Config\ConfigValueException
+   *   If any key in $data in any depth contains a dot.
+   */
+  protected function validateKeys(array $data) {
+    foreach ($data as $key => $value) {
+      if (strpos($key, '.') !== FALSE) {
+        throw new ConfigValueException(String::format('@key key contains a dot which is not supported.', array('@key' => $key)));
+      }
+      if (is_array($value)) {
+        $this->validateKeys($value);
+      }
+    }
+  }
+
   /**
    * Unsets a value in this configuration object.
    *

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

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Config\ConfigValueException.
+ */
+
+namespace Drupal\Core\Config;
+
+/**
+ * Exception thrown when config object values are invalid.
+ */
+class ConfigValueException extends ConfigException {}

core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php

@@ -237,7 +237,8 @@ protected function doSave($id, EntityInterface $entity) {
     }
 
     // Retrieve the desired properties and set them in config.
-    foreach ($entity->toArray() as $key => $value) {
+    $record = $this->mapToStorageRecord($entity);
+    foreach ($record as $key => $value) {
       $config->set($key, $value);
     }
     $config->save();
@@ -245,6 +246,19 @@ protected function doSave($id, EntityInterface $entity) {
     return $is_new ? SAVED_NEW : SAVED_UPDATED;
   }
 
+  /**
+   * Maps from an entity object to the storage record.
+   *
+   * @param \Drupal\Core\Entity\EntityInterface $entity
+   *   The entity object.
+   *
+   * @return array
+   *   The record to store.
+   */
+  protected function mapToStorageRecord(EntityInterface $entity) {
+    return $entity->toArray();
+  }
+
   /**
    * {@inheritdoc}
    */

core/lib/Drupal/Core/Field/FieldItemBase.php

@@ -255,4 +255,32 @@ public function instanceSettingsForm(array $form, array &$form_state) {
     return array();
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public static function settingsToConfigData(array $settings) {
+    return $settings;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function settingsFromConfigData(array $settings) {
+    return $settings;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function instanceSettingsToConfigData(array $settings) {
+    return $settings;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function instanceSettingsFromConfigData(array $settings) {
+    return $settings;
+  }
+
 }

core/lib/Drupal/Core/Field/FieldItemInterface.php

@@ -231,6 +231,94 @@ public static function defaultSettings();
    */
   public static function defaultInstanceSettings();
 
+  /**
+   * Returns a settings array that can be stored as a configuration value.
+   *
+   * For all use cases where field settings are stored and managed as
+   * configuration, this method is used to map from the field type's
+   * representation of its settings to a representation compatible with
+   * deployable configuration. This includes:
+   * - Array keys at any depth must not contain a ".".
+   * - Ideally, array keys at any depth are either numeric or can be enumerated
+   *   as a "mapping" within the configuration schema. While not strictly
+   *   required, this simplifies configuration translation UIs, configuration
+   *   migrations between Drupal versions, and other use cases.
+   * - To support configuration deployments, references to content entities
+   *   must use UUIDs rather than local IDs.
+   *
+   * An example of a conversion between representations might be an
+   * "allowed_values" setting that's structured by the field type as a
+   * \Drupal\Core\TypedData\AllowedValuesInterface::getPossibleOptions()
+   * result (i.e., values as keys and labels as values). For such a use case,
+   * in order to comply with the above, this method could convert that
+   * representation to a numerically indexed array whose values are sub-arrays
+   * with the schema definable keys of "value" and "label".
+   *
+   * @param array $settings
+   *   The field's settings in the field type's canonical representation.
+   *
+   * @return array
+   *   An array (either the unmodified $settings or a modified representation)
+   *   that is suitable for storing as a deployable configuration value.
+   *
+   * @see \Drupal\Core\Config\Config::set()
+   */
+  public static function settingsToConfigData(array $settings);
+
+  /**
+   * Returns a settings array in the field type's canonical representation.
+   *
+   * This function does the inverse of static::settingsToConfigData(). It's
+   * called when loading a field's settings from a configuration object.
+   *
+   * @param array $settings
+   *   The field's settings, as it is stored within a configuration object.
+   *
+   * @return array
+   *   The settings, in the representation expected by the field type and code
+   *   that interacts with it.
+   *
+   * @see \Drupal\Core\Field\FieldItemInterface::settingsToConfigData()
+   */
+  public static function settingsFromConfigData(array $settings);
+
+  /**
+   * Returns a settings array that can be stored as a configuration value.
+   *
+   * Same as static::settingsToConfigData(), but for the field's instance
+   * settings.
+   *
+   * @param array $settings
+   *   The field's instance settings in the field type's canonical
+   *   representation.
+   *
+   * @return array
+   *   An array (either the unmodified $settings or a modified representation)
+   *   that is suitable for storing as a deployable configuration value.
+   *
+   * @see \Drupal\Core\Field\FieldItemInterface::settingsToConfigData()
+   */
+  public static function instanceSettingsToConfigData(array $settings);
+
+  /**
+   * Returns a settings array in the field type's canonical representation.
+   *
+   * This function does the inverse of static::instanceSettingsToConfigData().
+   * It's called when loading a field's instance settings from a configuration
+   * object.
+   *
+   * @param array $settings
+   *   The field's instance settings, as it is stored within a configuration
+   *   object.
+   *
+   * @return array
+   *   The instance settings, in the representation expected by the field type
+   *   and code that interacts with it.
+   *
+   * @see \Drupal\Core\Field\FieldItemInterface::instanceSettingsToConfigData()
+   */
+  public static function instanceSettingsFromConfigData(array $settings);
+
   /**
    * Returns a form for the field-level settings.
    *

core/lib/Drupal/Core/Field/FieldTypePluginManager.php

@@ -80,4 +80,12 @@ public function getUiDefinitions() {
     });
   }
 
+  /**
+   * @inheritdoc
+   */
+  public function getPluginClass($type) {
+    $plugin_definition = $this->getDefinition($type, FALSE);
+    return $plugin_definition['class'];
+  }
+
 }

core/lib/Drupal/Core/Field/FieldTypePluginManagerInterface.php

@@ -48,4 +48,15 @@ public function getDefaultSettings($type);
    */
   public function getUiDefinitions();
 
+  /**
+   * Returns the PHP class that implements the field type plugin.
+   *
+   * @param string $type
+   *   A field type name.
+   *
+   * @return string
+   *   Field type plugin class name.
+   */
+  public function getPluginClass($type);
+
 }

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

@@ -9,6 +9,7 @@
 
 use Drupal\Component\Utility\String;
 use Drupal\Core\Config\ConfigNameException;
+use Drupal\Core\Config\ConfigValueException;
 use Drupal\Core\Config\InstallStorage;
 use Drupal\simpletest\DrupalUnitTestBase;
 use Drupal\Core\Config\FileStorage;
@@ -183,6 +184,31 @@ function testNameValidation() {
 
   }
 
+  /**
+   * Tests the validation of configuration object values.
+   */
+  function testValueValidation() {
+    // Verify that setData() will catch dotted keys.
+    $message = 'Expected ConfigValueException was thrown from setData() for value with dotted keys.';
+    try {
+      \Drupal::config('namespace.object')->setData(array('key.value' => 12))->save();
+      $this->fail($message);
+    }
+    catch (ConfigValueException $e) {
+      $this->pass($message);
+    }
+
+    // Verify that set() will catch dotted keys.
+    $message = 'Expected ConfigValueException was thrown from set() for value with dotted keys.';
+    try {
+      \Drupal::config('namespace.object')->set('foo', array('key.value' => 12))->save();
+      $this->fail($message);
+    }
+    catch (ConfigValueException $e) {
+      $this->pass($message);
+    }
+  }
+
   /**
    * Tests data type handling.
    */

core/modules/field/src/Entity/FieldInstanceConfig.php

@@ -254,7 +254,8 @@ public function __construct(array $values, $entity_type = 'field_instance_config
     }
 
     // Discard the 'field_type' entry that is added in config records to ease
-    // schema generation. See self::toArray().
+    // schema generation and mapping settings from storage.
+    // @see Drupal\field\Entity\FieldInstanceConfig::toArray().
     unset($values['field_type']);
 
     parent::__construct($values, $entity_type);
@@ -288,7 +289,9 @@ public function getType() {
   public function toArray() {
     $properties = parent::toArray();
     // Additionally, include the field type, that is needed to be able to
-    // generate the field-type-dependant parts of the config schema.
+    // generate the field-type-dependant parts of the config schema and to
+    // allow for mapping settings from storage by field type.
+    // @see \Drupal\field\FieldInstanceConfigStorage::mapFromStorageRecords().
     $properties['field_type'] = $this->getType();
 
     return $properties;

core/modules/field/src/FieldInstanceConfigStorage.php

@@ -9,8 +9,10 @@
 
 use Drupal\Core\Config\Config;
 use Drupal\Core\Config\Entity\ConfigEntityStorage;
+use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Entity\EntityManagerInterface;
 use Drupal\Core\Entity\EntityTypeInterface;
+use Drupal\Core\Field\FieldTypePluginManagerInterface;
 use Drupal\Core\Language\LanguageManagerInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Drupal\Core\Config\ConfigFactoryInterface;
@@ -42,6 +44,13 @@ class FieldInstanceConfigStorage extends ConfigEntityStorage {
    */
   protected $state;
 
+  /**
+   * The field type plugin manager.
+   *
+   * @var \Drupal\Core\Field\FieldTypePluginManagerInterface
+   */
+  protected $fieldTypeManager;
+
   /**
    * Constructs a FieldInstanceConfigStorage object.
    *
@@ -59,11 +68,14 @@ class FieldInstanceConfigStorage extends ConfigEntityStorage {
    *   The entity manager.
    * @param \Drupal\Core\State\StateInterface $state
    *   The state key value store.
+   * @param \Drupal\Component\Plugin\PluginManagerInterface\FieldTypePluginManagerInterface
+   *   The field type plugin manager.
    */
-  public function __construct(EntityTypeInterface $entity_type, ConfigFactoryInterface $config_factory, StorageInterface $config_storage, UuidInterface $uuid_service, LanguageManagerInterface $language_manager, EntityManagerInterface $entity_manager, StateInterface $state) {
+  public function __construct(EntityTypeInterface $entity_type, ConfigFactoryInterface $config_factory, StorageInterface $config_storage, UuidInterface $uuid_service, LanguageManagerInterface $language_manager, EntityManagerInterface $entity_manager, StateInterface $state, FieldTypePluginManagerInterface $field_type_manager) {
     parent::__construct($entity_type, $config_factory, $config_storage, $uuid_service, $language_manager);
     $this->entityManager = $entity_manager;
     $this->state = $state;
+    $this->fieldTypeManager = $field_type_manager;
   }
 
   /**
@@ -77,7 +89,8 @@ public static function createInstance(ContainerInterface $container, EntityTypeI
       $container->get('uuid'),
       $container->get('language_manager'),
       $container->get('entity.manager'),
-      $container->get('state')
+      $container->get('state'),
+      $container->get('plugin.manager.field.field_type')
     );
   }
 
@@ -175,4 +188,24 @@ public function loadByProperties(array $conditions = array()) {
     return $matching_instances;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function mapFromStorageRecords(array $records) {
+    foreach ($records as &$record) {
+      $class = $this->fieldTypeManager->getPluginClass($record['field_type']);
+      $record['settings'] = $class::instanceSettingsFromConfigData($record['settings']);
+    }
+    return parent::mapFromStorageRecords($records);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function mapToStorageRecord(EntityInterface $entity) {
+    $record = parent::mapToStorageRecord($entity);
+    $class = $this->fieldTypeManager->getPluginClass($record['field_type']);
+    $record['settings'] = $class::instanceSettingsToConfigData($record['settings']);
+    return $record;
+  }
 }

core/modules/field/src/FieldStorageConfigStorage.php

@@ -9,8 +9,10 @@
 
 use Drupal\Component\Uuid\UuidInterface;
 use Drupal\Core\Config\Entity\ConfigEntityStorage;
+use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Entity\EntityManagerInterface;
 use Drupal\Core\Entity\EntityTypeInterface;
+use Drupal\Core\Field\FieldTypePluginManagerInterface;
 use Drupal\Core\Language\LanguageManagerInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Drupal\Core\Config\ConfigFactoryInterface;
@@ -44,6 +46,13 @@ class FieldStorageConfigStorage extends ConfigEntityStorage {
    */
   protected $state;
 
+  /**
+   * The field type plugin manager.
+   *
+   * @var \Drupal\Core\Field\FieldTypePluginManagerInterface
+   */
+  protected $fieldTypeManager;
+
   /**
    * Constructs a FieldStorageConfigStorage object.
    *
@@ -63,12 +72,15 @@ class FieldStorageConfigStorage extends ConfigEntityStorage {
    *   The module handler.
    * @param \Drupal\Core\State\StateInterface $state
    *   The state key value store.
+   * @param \Drupal\Component\Plugin\PluginManagerInterface\FieldTypePluginManagerInterface
+   *   The field type plugin manager.
    */
-  public function __construct(EntityTypeInterface $entity_type, ConfigFactoryInterface $config_factory, StorageInterface $config_storage, UuidInterface $uuid_service, LanguageManagerInterface $language_manager, EntityManagerInterface $entity_manager, ModuleHandler $module_handler, StateInterface $state) {
+  public function __construct(EntityTypeInterface $entity_type, ConfigFactoryInterface $config_factory, StorageInterface $config_storage, UuidInterface $uuid_service, LanguageManagerInterface $language_manager, EntityManagerInterface $entity_manager, ModuleHandler $module_handler, StateInterface $state, FieldTypePluginManagerInterface $field_type_manager) {
     parent::__construct($entity_type, $config_factory, $config_storage, $uuid_service, $language_manager);
     $this->entityManager = $entity_manager;
     $this->moduleHandler = $module_handler;
     $this->state = $state;
+    $this->fieldTypeManager = $field_type_manager;
   }
 
   /**
@@ -83,7 +95,8 @@ public static function createInstance(ContainerInterface $container, EntityTypeI
       $container->get('language_manager'),
       $container->get('entity.manager'),
       $container->get('module_handler'),
-      $container->get('state')
+      $container->get('state'),
+      $container->get('plugin.manager.field.field_type')
     );
   }
 
@@ -154,4 +167,26 @@ public function loadByProperties(array $conditions = array()) {
     return $matches;
 
   }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function mapFromStorageRecords(array $records) {
+    foreach ($records as &$record) {
+      $class = $this->fieldTypeManager->getPluginClass($record['type']);
+      $record['settings'] = $class::settingsFromConfigData($record['settings']);
+    }
+    return parent::mapFromStorageRecords($records);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function mapToStorageRecord(EntityInterface $entity) {
+    $record = parent::mapToStorageRecord($entity);
+    $class = $this->fieldTypeManager->getPluginClass($record['type']);
+    $record['settings'] = $class::settingsToConfigData($record['settings']);
+    return $record;
+  }
+
 }

core/modules/options/config/schema/options.schema.yml

@@ -8,8 +8,15 @@ field.list_integer.settings:
       type: sequence
       label: 'Allowed values list'
       sequence:
-        - type: string
-          label: 'Value'
+        - type: mapping
+          label: 'Allowed value with label'
+          mapping:
+            value:
+              type: integer
+              label: 'Value'
+            label:
+              type: label
+              label: 'Label'
     allowed_values_function:
       type: string
       label: 'Allowed values function'
@@ -35,8 +42,18 @@ field.list_float.settings:
   label: 'List (float) settings'
   mapping:
     allowed_values:
-      type: ignore
+      type: sequence
       label: 'Allowed values list'
+      sequence:
+        - type: mapping
+          label: 'Allowed value with label'
+          mapping:
+            value:
+              type: float
+              label: 'Value'
+            label:
+              type: label
+              label: 'Label'
     allowed_values_function:
       type: string
       label: 'Allowed values function'
@@ -65,8 +82,15 @@ field.list_text.settings:
       type: sequence
       label: 'Allowed values list'
       sequence:
-        - type: string
-          label: 'Value'
+        - type: mapping
+          label: 'Allowed value with label'
+          mapping:
+            value:
+              type: string
+              label: 'Value'
+            label:
+              type: label
+              label: 'Label'
     allowed_values_function:
       type: string
       label: 'Allowed values function'

core/modules/options/src/Plugin/Field/FieldType/ListFloatItem.php

@@ -90,4 +90,23 @@ protected static function validateAllowedValue($option) {
     }
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public static function simplifyAllowedValues(array $structured_values) {
+    $values = array();
+    foreach ($structured_values as $item) {
+      // Nested elements are embedded in the label.
+      if (is_array($item['label'])) {
+        $item['label'] = static::simplifyAllowedValues($item['label']);
+      }
+      // Cast the value to a float first so that .5 and 0.5 are the same value
+      // and then cast to a string so that values like 0.5 can be used as array
+      // keys.
+      // @see http://php.net/manual/en/language.types.array.php
+      $values[(string) (float) $item['value']] = $item['label'];
+    }
+    return $values;
+  }
+
 }

core/modules/options/src/Plugin/Field/FieldType/ListItemBase.php

@@ -237,4 +237,76 @@ protected function allowedValuesString($values) {
     return implode("\n", $lines);
   }
 
+  /**
+   * @inheritdoc.
+   */
+  public static function settingsToConfigData(array $settings) {
+    if (isset($settings['allowed_values'])) {
+      $settings['allowed_values'] = static::structureAllowedValues($settings['allowed_values']);
+    }
+    return $settings;
+  }
+
+  /**
+   * @inheritdoc.
+   */
+  public static function settingsFromConfigData(array $settings) {
+    if (isset($settings['allowed_values'])) {
+      $settings['allowed_values'] = static::simplifyAllowedValues($settings['allowed_values']);
+    }
+    return $settings;
+  }
+
+  /**
+   * Simplifies allowed values to a key-value array from the structured array.
+   *
+   * @param array $structured_values
+   *   Array of items with a 'value' and 'label' key each for the allowed
+   *   values.
+   *
+   * @return array
+   *   Allowed values were the array key is the 'value' value, the value is
+   *   the 'label' value.
+   *
+   * @see Drupal\options\Plugin\Field\FieldType\ListItemBase::structureAllowedValues()
+   */
+  protected static function simplifyAllowedValues(array $structured_values) {
+    $values = array();
+    foreach ($structured_values as $item) {
+      if (is_array($item['label'])) {
+        // Nested elements are embedded in the label.
+        $item['label'] = static::simplifyAllowedValues($item['label']);
+      }
+      $values[$item['value']] = $item['label'];
+    }
+    return $values;
+  }
+
+  /**
+   * Creates a structured array of allowed values from a key-value array.
+   *
+   * @param array $values
+   *   Allowed values were the array key is the 'value' value, the value is
+   *   the 'label' value.
+   *
+   * @return array
+   *   Array of items with a 'value' and 'label' key each for the allowed
+   *   values.
+   *
+   * @see Drupal\options\Plugin\Field\FieldType\ListItemBase::simplifyAllowedValues()
+   */
+  protected static function structureAllowedValues(array $values) {
+    $structured_values = array();
+    foreach ($values as $value => $label) {
+      if (is_array($label)) {
+        $label = static::structureAllowedValues($label);
+      }
+      $structured_values[] = array(
+        'value' => $value,
+        'label' => $label,
+      );
+    }
+    return $structured_values;
+  }
+
 }

core/modules/options/src/Tests/OptionsFieldUITest.php

@@ -94,6 +94,11 @@ function testOptionsAllowedValuesInteger() {
     $string = "0|Zero";
     $array = array('0' => 'Zero');
     $this->assertAllowedValuesInput($string, $array, 'Values not in use can be removed.');
+
+    // Check that the same key can only be used once.
+    $string = "0|Zero\n0|One";
+    $array = array('0' => 'One');
+    $this->assertAllowedValuesInput($string, $array, 'Same value cannot be used multiple times.');
   }
 
   /**
@@ -144,6 +149,16 @@ function testOptionsAllowedValuesFloat() {
     $string = "0|Zero";
     $array = array('0' => 'Zero');
     $this->assertAllowedValuesInput($string, $array, 'Values not in use can be removed.');
+
+    // Check that the same key can only be used once.
+    $string = "0.5|Point five\n0.5|Half";
+    $array = array('0.5' => 'Half');
+    $this->assertAllowedValuesInput($string, $array, 'Same value cannot be used multiple times.');
+
+    // Check that different forms of the same float value cannot be used.
+    $string = "0|Zero\n.5|Point five\n0.5|Half";
+    $array = array('0' => 'Zero', '0.5' => 'Half');
+    $this->assertAllowedValuesInput($string, $array, 'Different forms of the same value cannot be used.');
   }
 
   /**
@@ -199,6 +214,16 @@ function testOptionsAllowedValuesText() {
     $string = "Zero";
     $array = array('Zero' => 'Zero');
     $this->assertAllowedValuesInput($string, $array, 'Values not in use can be removed.');
+
+    // Check that string values with dots can be used.
+    $string = "Zero\nexample.com|Example";
+    $array = array('Zero' => 'Zero', 'example.com' => 'Example');
+    $this->assertAllowedValuesInput($string, $array, 'String value with dot is supported.');
+
+    // Check that the same key can only be used once.
+    $string = "zero|Zero\nzero|One";
+    $array = array('zero' => 'One');
+    $this->assertAllowedValuesInput($string, $array, 'Same value cannot be used multiple times.');
   }
 
   /**