diff --git a/core/config/schema/core.data_types.schema.yml b/core/config/schema/core.data_types.schema.yml index 73db361..e69873e 100644 --- a/core/config/schema/core.data_types.schema.yml +++ b/core/config/schema/core.data_types.schema.yml @@ -344,6 +344,16 @@ condition.plugin: sequence: type: string +condition.plugin.*: + type: condition.plugin + +condition_plugins: + type: sequence + label: 'Conditions' + sequence: + type: condition.plugin.[%key] + label: ' Condition' + display_variant.plugin: type: mapping label: 'Display variant' diff --git a/core/lib/Drupal/Component/Plugin/LazyPluginCollection.php b/core/lib/Drupal/Component/Plugin/LazyPluginCollection.php index 1d90af7..2dbf291 100644 --- a/core/lib/Drupal/Component/Plugin/LazyPluginCollection.php +++ b/core/lib/Drupal/Component/Plugin/LazyPluginCollection.php @@ -157,4 +157,12 @@ public function count() { return count($this->instanceIDs); } + /** + * Validates the configuration for this plugin collection. + * + * This should be called before the object storing the configuration is saved. + */ + public function validateConfiguration() { + } + } diff --git a/core/lib/Drupal/Core/Condition/ConditionPluginCollection.php b/core/lib/Drupal/Core/Condition/ConditionPluginCollection.php index 5801ac1..04becf2 100644 --- a/core/lib/Drupal/Core/Condition/ConditionPluginCollection.php +++ b/core/lib/Drupal/Core/Condition/ConditionPluginCollection.php @@ -3,6 +3,7 @@ namespace Drupal\Core\Condition; use Drupal\Component\Plugin\Context\ContextInterface; +use Drupal\Core\Config\ConfigValueCaster; use Drupal\Core\Plugin\DefaultLazyPluginCollection; /** @@ -10,6 +11,15 @@ */ class ConditionPluginCollection extends DefaultLazyPluginCollection { + use ConfigValueCaster; + + /** + * The typed config manager. + * + * @var \Drupal\Core\Config\TypedConfigManagerInterface + */ + protected $typedConfigManager; + /** * An array of collected contexts for conditions. * @@ -74,4 +84,36 @@ public function getConditionContexts() { return $this->conditionContexts; } + /** + * {@inheritdoc} + */ + public function validateConfiguration() { + $data = $configuration = $this->getConfiguration(); + // Ensure that the schema wrapper has the latest data. + $this->initSchemaWrapper($data); + foreach ($data as $key => $value) { + $data[$key] = $this->castValue($key, $value); + } + if ($data !== $configuration) { + $this->setConfiguration($data); + } + } + + /** + * {@inheritdoc} + */ + protected function typedConfigManager() { + if (!$this->typedConfigManager) { + $this->typedConfigManager = \Drupal::service('config.typed'); + } + return $this->typedConfigManager; + } + + /** + * {@inheritdoc} + */ + protected function getName() { + return 'condition_plugins'; + } + } diff --git a/core/lib/Drupal/Core/Config/Config.php b/core/lib/Drupal/Core/Config/Config.php index 0128c66..5482198 100644 --- a/core/lib/Drupal/Core/Config/Config.php +++ b/core/lib/Drupal/Core/Config/Config.php @@ -207,7 +207,7 @@ public function save($has_trusted_data = FALSE) { if (!$has_trusted_data) { if ($this->typedConfigManager->hasConfigSchema($this->name)) { // Ensure that the schema wrapper has the latest data. - $this->schemaWrapper = NULL; + $this->initSchemaWrapper($this->data); foreach ($this->data as $key => $value) { $this->data[$key] = $this->castValue($key, $value); } diff --git a/core/lib/Drupal/Core/Config/StorableConfigBase.php b/core/lib/Drupal/Core/Config/ConfigValueCaster.php similarity index 55% copy from core/lib/Drupal/Core/Config/StorableConfigBase.php copy to core/lib/Drupal/Core/Config/ConfigValueCaster.php index 40a25ef..41c4ac0 100644 --- a/core/lib/Drupal/Core/Config/StorableConfigBase.php +++ b/core/lib/Drupal/Core/Config/ConfigValueCaster.php @@ -9,117 +9,47 @@ use Drupal\Core\Config\Schema\Undefined; /** - * Provides a base class for configuration objects with storage support. - * - * Encapsulates all capabilities needed for configuration handling for a - * specific configuration object, including storage and data type casting. - * - * The default implementation in \Drupal\Core\Config\Config adds support for - * runtime overrides. Extend from StorableConfigBase directly to manage - * configuration with a storage backend that does not support overrides. - * - * @see \Drupal\Core\Config\Config + * Provides a trait for casting configuration data based on its config schema. */ -abstract class StorableConfigBase extends ConfigBase { +trait ConfigValueCaster { /** - * The storage used to load and save this configuration object. - * - * @var \Drupal\Core\Config\StorageInterface - */ - protected $storage; - - /** - * The config schema wrapper object for this configuration object. + * The config schema wrapper object for this typed configuration. * * @var \Drupal\Core\Config\Schema\Element */ protected $schemaWrapper; /** - * The typed config manager. - * - * @var \Drupal\Core\Config\TypedConfigManagerInterface - */ - protected $typedConfigManager; - - /** - * Whether the configuration object is new or has been saved to the storage. + * Returns the name of this typed configuration definition. * - * @var bool + * @return string + * The name of the typed configuration definition. */ - protected $isNew = TRUE; + abstract protected function getName(); /** - * The data of the configuration object. + * Wraps the typed config manager. * - * @var array + * @return \Drupal\Core\Config\TypedConfigManagerInterface */ - protected $originalData = array(); + abstract protected function typedConfigManager(); /** - * Saves the configuration object. - * - * Must invalidate the cache tags associated with the configuration object. - * - * @param bool $has_trusted_data - * Set to TRUE if the configuration data has already been checked to ensure - * it conforms to schema. Generally this is only used during module and - * theme installation. - * - * @return $this - * - * @see \Drupal\Core\Config\ConfigInstaller::createConfiguration() - */ - abstract public function save($has_trusted_data = FALSE); - - /** - * Deletes the configuration object. - * - * Must invalidate the cache tags associated with the configuration object. - * - * @return $this - */ - abstract public function delete(); - - /** - * Initializes a configuration object with pre-loaded data. + * Initializes the schema wrapper with the current configuration data. * * @param array $data - * Array of loaded data for this configuration object. - * - * @return $this - * The configuration object. - */ - public function initWithData(array $data) { - $this->isNew = FALSE; - $this->data = $data; - $this->originalData = $this->data; - return $this; - } - - /** - * Returns whether this configuration object is new. - * - * @return bool - * TRUE if this configuration object does not exist in storage. - */ - public function isNew() { - return $this->isNew; - } - - /** - * Retrieves the storage used to load and save this configuration object. - * - * @return \Drupal\Core\Config\StorageInterface - * The configuration storage object. + * Array of loaded data for this typed configuration. */ - public function getStorage() { - return $this->storage; + protected function initSchemaWrapper($data) { + $typed_config_manager = $this->typedConfigManager(); + $definition = $typed_config_manager->getDefinition($this->getName()); + $data_definition = $typed_config_manager->buildDataDefinition($definition, $data); + $this->schemaWrapper = $typed_config_manager->create($data_definition, $data); } /** - * Gets the schema wrapper for the whole configuration object. + * Gets the schema wrapper for the typed configuration. * * The schema wrapper is dependent on the configuration name and the whole * data structure, so if the name or the data changes in any way, the wrapper @@ -129,15 +59,13 @@ public function getStorage() { */ protected function getSchemaWrapper() { if (!isset($this->schemaWrapper)) { - $definition = $this->typedConfigManager->getDefinition($this->name); - $data_definition = $this->typedConfigManager->buildDataDefinition($definition, $this->data); - $this->schemaWrapper = $this->typedConfigManager->create($data_definition, $this->data); + throw new \RuntimeException('The schema wrapper must be instantiated with data'); } return $this->schemaWrapper; } /** - * Validate the values are allowed data types. + * Validates that the values are allowed data types. * * @param string $key * A string that maps to a key within the configuration data. diff --git a/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php index 08e7e19..067c6ee 100644 --- a/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php @@ -317,6 +317,7 @@ public function preSave(EntityStorageInterface $storage) { // Any changes to the plugin configuration must be saved to the entity's // copy as well. foreach ($this->getPluginCollections() as $plugin_config_key => $plugin_collection) { + $plugin_collection->validateConfiguration(); $this->set($plugin_config_key, $plugin_collection->getConfiguration()); } } diff --git a/core/lib/Drupal/Core/Config/StorableConfigBase.php b/core/lib/Drupal/Core/Config/StorableConfigBase.php index 40a25ef..4a95ada 100644 --- a/core/lib/Drupal/Core/Config/StorableConfigBase.php +++ b/core/lib/Drupal/Core/Config/StorableConfigBase.php @@ -22,6 +22,8 @@ */ abstract class StorableConfigBase extends ConfigBase { + use ConfigValueCaster; + /** * The storage used to load and save this configuration object. * @@ -30,13 +32,6 @@ protected $storage; /** - * The config schema wrapper object for this configuration object. - * - * @var \Drupal\Core\Config\Schema\Element - */ - protected $schemaWrapper; - - /** * The typed config manager. * * @var \Drupal\Core\Config\TypedConfigManagerInterface @@ -109,6 +104,13 @@ public function isNew() { } /** + * {@inheritdoc} + */ + protected function typedConfigManager() { + return $this->typedConfigManager; + } + + /** * Retrieves the storage used to load and save this configuration object. * * @return \Drupal\Core\Config\StorageInterface @@ -118,100 +120,4 @@ public function getStorage() { return $this->storage; } - /** - * Gets the schema wrapper for the whole configuration object. - * - * The schema wrapper is dependent on the configuration name and the whole - * data structure, so if the name or the data changes in any way, the wrapper - * should be reset. - * - * @return \Drupal\Core\Config\Schema\Element - */ - protected function getSchemaWrapper() { - if (!isset($this->schemaWrapper)) { - $definition = $this->typedConfigManager->getDefinition($this->name); - $data_definition = $this->typedConfigManager->buildDataDefinition($definition, $this->data); - $this->schemaWrapper = $this->typedConfigManager->create($data_definition, $this->data); - } - return $this->schemaWrapper; - } - - /** - * Validate the values are allowed data types. - * - * @param string $key - * A string that maps to a key within the configuration data. - * @param string $value - * Value to associate with the key. - * - * @return null - * - * @throws \Drupal\Core\Config\UnsupportedDataTypeConfigException - * If the value is unsupported in configuration. - */ - protected function validateValue($key, $value) { - // Minimal validation. Should not try to serialize resources or non-arrays. - if (is_array($value)) { - foreach ($value as $nested_value_key => $nested_value) { - $this->validateValue($key . '.' . $nested_value_key, $nested_value); - } - } - elseif ($value !== NULL && !is_scalar($value)) { - throw new UnsupportedDataTypeConfigException("Invalid data type for config element {$this->getName()}:$key"); - } - } - - /** - * Casts the value to correct data type using the configuration schema. - * - * @param string $key - * A string that maps to a key within the configuration data. - * @param string $value - * Value to associate with the key. - * - * @return mixed - * The value cast to the type indicated in the schema. - * - * @throws \Drupal\Core\Config\UnsupportedDataTypeConfigException - * If the value is unsupported in configuration. - */ - protected function castValue($key, $value) { - $element = $this->getSchemaWrapper()->get($key); - // Do not cast value if it is unknown or defined to be ignored. - if ($element && ($element instanceof Undefined || $element instanceof Ignore)) { - // Do validate the value (may throw UnsupportedDataTypeConfigException) - // to ensure unsupported types are not supported in this case either. - $this->validateValue($key, $value); - 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 { - $value = $element->getCastedValue(); - } - } - } - else { - // Throw exception on any non-scalar or non-array value. - if (!is_array($value)) { - throw new UnsupportedDataTypeConfigException("Invalid data type for config element {$this->getName()}:$key"); - } - // Recurse into any nested keys. - foreach ($value as $nested_value_key => $nested_value) { - $value[$nested_value_key] = $this->castValue($key . '.' . $nested_value_key, $nested_value); - } - } - return $value; - } - } diff --git a/core/modules/block/config/schema/block.schema.yml b/core/modules/block/config/schema/block.schema.yml index 16d79bc..1412d96 100644 --- a/core/modules/block/config/schema/block.schema.yml +++ b/core/modules/block/config/schema/block.schema.yml @@ -25,11 +25,8 @@ block.block.*: settings: type: block.settings.[%parent.plugin] visibility: - type: sequence + type: condition_plugins label: 'Visibility Conditions' - sequence: - type: condition.plugin.[id] - label: 'Visibility Condition' block.settings.*: type: block_settings diff --git a/core/modules/block/src/BlockForm.php b/core/modules/block/src/BlockForm.php index 35258b8..2b1ee51 100644 --- a/core/modules/block/src/BlockForm.php +++ b/core/modules/block/src/BlockForm.php @@ -319,13 +319,6 @@ public function validateForm(array &$form, FormStateInterface $form_state) { protected function validateVisibility(array $form, FormStateInterface $form_state) { // Validate visibility condition settings. foreach ($form_state->getValue('visibility') as $condition_id => $values) { - // All condition plugins use 'negate' as a Boolean in their schema. - // However, certain form elements may return it as 0/1. Cast here to - // ensure the data is in the expected type. - if (array_key_exists('negate', $values)) { - $values['negate'] = (bool) $values['negate']; - } - // Allow the condition to validate the form. $condition = $form_state->get(['conditions', $condition_id]); $condition->validateConfigurationForm($form['visibility'][$condition_id], SubformState::createForSubform($form['visibility'][$condition_id], $form, $form_state)); @@ -353,9 +346,6 @@ public function submitForm(array &$form, FormStateInterface $form_state) { $this->submitVisibility($form, $form_state); - // Save the settings of the plugin. - $entity->save(); - drupal_set_message($this->t('The block configuration has been saved.')); $form_state->setRedirect( 'block.admin_display_theme', diff --git a/core/tests/Drupal/KernelTests/Core/Condition/ConditionPluginCollectionTest.php b/core/tests/Drupal/KernelTests/Core/Condition/ConditionPluginCollectionTest.php new file mode 100644 index 0000000..0e4d5d5 --- /dev/null +++ b/core/tests/Drupal/KernelTests/Core/Condition/ConditionPluginCollectionTest.php @@ -0,0 +1,57 @@ + 'request_path', + 'pages' => '/user/*', + 'negate' => 0, + 'context_mapping' => [], + ]; + // Include a condition that matches default values but with a type mismatch + // on 'negate' by using 0 instead of FALSE. + $configuration['user_role'] = [ + 'id' => 'user_role', + 'roles' => [], + 'negate' => 0, + 'context_mapping' => [], + ]; + $collection = new ConditionPluginCollection(\Drupal::service('plugin.manager.condition'), $configuration); + + // Before validation, the configuration has not changed. + $this->assertEquals($configuration, $collection->getConfiguration()); + + // Validate the configuration. + $collection->validateConfiguration(); + + $expected['request_path'] = [ + 'id' => 'request_path', + 'pages' => '/user/*', + 'negate' => FALSE, + 'context_mapping' => [], + ]; + $this->assertEquals($expected, $collection->getConfiguration()); + } + +}