From fa39f06433eebd7f2b52bc48a33a7fba05d6f150 Mon Sep 17 00:00:00 2001
From: xjm <xjm@65776.no-reply.drupal.org>
Date: Sat, 4 Mar 2017 12:42:17 -0600
Subject: [PATCH] Issue #2361539 by alexpott, webflo, AjitS, himanshu-dixit,
tameeshb, xjm: Config export key order is not predictable for sequences, add
orderby property to config schema
---
core/config/schema/core.data_types.schema.yml | 2 +-
.../Config/Schema/SequenceDataDefinition.php | 28 ++++++++
.../Drupal/Core/Config/StorableConfigBase.php | 25 +++++++
.../schema/config_schema_test.schema.yml | 38 ++++++++++
.../Core/Config/ConfigSchemaTest.php | 70 +++++++++++++++++++
5 files changed, 162 insertions(+), 1 deletion(-)
create mode 100644 core/lib/Drupal/Core/Config/Schema/SequenceDataDefinition.php
diff --git a/core/config/schema/core.data_types.schema.yml b/core/config/schema/core.data_types.schema.yml
index 0fbeeedbe039..a0bd01aa2435 100644
--- a/core/config/schema/core.data_types.schema.yml
+++ b/core/config/schema/core.data_types.schema.yml
@@ -46,7 +46,7 @@ mapping:
sequence:
label: Sequence
class: '\Drupal\Core\Config\Schema\Sequence'
- definition_class: '\Drupal\Core\TypedData\ListDataDefinition'
+ definition_class: '\Drupal\Core\Config\Schema\SequenceDataDefinition'
# Simple extended data types:
diff --git a/core/lib/Drupal/Core/Config/Schema/SequenceDataDefinition.php b/core/lib/Drupal/Core/Config/Schema/SequenceDataDefinition.php
new file mode 100644
index 000000000000..c317e29e02cf
--- /dev/null
+++ b/core/lib/Drupal/Core/Config/Schema/SequenceDataDefinition.php
@@ -0,0 +1,28 @@
+<?php
+
+namespace Drupal\Core\Config\Schema;
+
+use Drupal\Core\TypedData\ListDataDefinition;
+
+/**
+ * A typed data definition class for defining sequences in configuration.
+ */
+class SequenceDataDefinition extends ListDataDefinition {
+
+ /**
+ * Gets the description of how the sequence should be sorted.
+ *
+ * Only the top level of the array should be sorted. Top-level keys should be
+ * discarded when using 'value' sorting. If the sequence is an associative
+ * array 'key' sorting is recommended, if not 'value' sorting is recommended.
+ *
+ * @return string|null
+ * May be 'key' (to sort by key), 'value' (to sort by value, discarding
+ * keys), or NULL (if the schema does not describe how the sequence should
+ * be sorted).
+ */
+ public function getOrderBy() {
+ return isset($this->definition['orderby']) ? $this->definition['orderby'] : NULL;
+ }
+
+}
diff --git a/core/lib/Drupal/Core/Config/StorableConfigBase.php b/core/lib/Drupal/Core/Config/StorableConfigBase.php
index 4b227c9efbdc..0751e9f59f2a 100644
--- a/core/lib/Drupal/Core/Config/StorableConfigBase.php
+++ b/core/lib/Drupal/Core/Config/StorableConfigBase.php
@@ -3,6 +3,8 @@
namespace Drupal\Core\Config;
use Drupal\Core\Config\Schema\Ignore;
+use Drupal\Core\Config\Schema\Sequence;
+use Drupal\Core\Config\Schema\SequenceDataDefinition;
use Drupal\Core\TypedData\PrimitiveInterface;
use Drupal\Core\TypedData\Type\FloatInterface;
use Drupal\Core\TypedData\Type\IntegerInterface;
@@ -210,6 +212,29 @@ protected function castValue($key, $value) {
foreach ($value as $nested_value_key => $nested_value) {
$value[$nested_value_key] = $this->castValue($key . '.' . $nested_value_key, $nested_value);
}
+
+ if ($element instanceof Sequence) {
+ $data_definition = $element->getDataDefinition();
+ if ($data_definition instanceof SequenceDataDefinition) {
+ // Apply any sorting defined on the schema.
+ switch ($data_definition->getOrderBy()) {
+ case 'key':
+ ksort($value);
+ break;
+
+ case 'value':
+ // The PHP documentation notes that "Be careful when sorting
+ // arrays with mixed types values because sort() can produce
+ // unpredictable results". There is no risk here because
+ // \Drupal\Core\Config\StorableConfigBase::castValue() has
+ // already cast all values to the same type using the
+ // configuration schema.
+ sort($value);
+ break;
+
+ }
+ }
+ }
}
return $value;
}
diff --git a/core/modules/config/tests/config_schema_test/config/schema/config_schema_test.schema.yml b/core/modules/config/tests/config_schema_test/config/schema/config_schema_test.schema.yml
index c10e2686bd8d..ce894c243959 100644
--- a/core/modules/config/tests/config_schema_test/config/schema/config_schema_test.schema.yml
+++ b/core/modules/config/tests/config_schema_test/config/schema/config_schema_test.schema.yml
@@ -297,3 +297,41 @@ test.double_brackets.breed:
mapping:
breed:
type: string
+
+config_schema_test.schema_sequence_sort:
+ type: config_object
+ mapping:
+ keyed_sort:
+ type: sequence
+ orderby: key
+ sequence:
+ - type: string
+ value_sort:
+ type: sequence
+ orderby: value
+ sequence:
+ - type: string
+ no_sort:
+ type: sequence
+ sequence:
+ - type: string
+ complex_sort_value:
+ type: sequence
+ orderby: value
+ sequence:
+ - type: mapping
+ mapping:
+ foo:
+ type: string
+ bar:
+ type: string
+ complex_sort_key:
+ type: sequence
+ orderby: key
+ sequence:
+ - type: mapping
+ mapping:
+ foo:
+ type: string
+ bar:
+ type: string
diff --git a/core/tests/Drupal/KernelTests/Core/Config/ConfigSchemaTest.php b/core/tests/Drupal/KernelTests/Core/Config/ConfigSchemaTest.php
index 84aa27132672..95737d0d87d1 100644
--- a/core/tests/Drupal/KernelTests/Core/Config/ConfigSchemaTest.php
+++ b/core/tests/Drupal/KernelTests/Core/Config/ConfigSchemaTest.php
@@ -395,6 +395,76 @@ public function testConfigSaveWithSchema() {
$this->assertIdentical($installed_data, $original_data);
}
+ /**
+ * Tests configuration sequence sorting using schemas.
+ */
+ public function testConfigSaveWithSequenceSorting() {
+ $data = [
+ 'keyed_sort' => [
+ 'b' => '1',
+ 'a' => '2',
+ ],
+ 'no_sort' => [
+ 'b' => '2',
+ 'a' => '1',
+ ],
+ ];
+ // Save config which has a schema that enforces sorting.
+ $this->config('config_schema_test.schema_sequence_sort')
+ ->setData($data)
+ ->save();
+ $this->assertSame(['a' => '2', 'b' => '1'], $this->config('config_schema_test.schema_sequence_sort')->get('keyed_sort'));
+ $this->assertSame(['b' => '2', 'a' => '1'], $this->config('config_schema_test.schema_sequence_sort')->get('no_sort'));
+
+ $data = [
+ 'value_sort' => ['b', 'a'],
+ 'no_sort' => ['b', 'a'],
+ ];
+ // Save config which has a schema that enforces sorting.
+ $this->config('config_schema_test.schema_sequence_sort')
+ ->setData($data)
+ ->save();
+
+ $this->assertSame(['a', 'b'], $this->config('config_schema_test.schema_sequence_sort')->get('value_sort'));
+ $this->assertSame(['b', 'a'], $this->config('config_schema_test.schema_sequence_sort')->get('no_sort'));
+
+ // Value sort does not preserve keys - this is intentional.
+ $data = [
+ 'value_sort' => [1 => 'b', 2 => 'a'],
+ 'no_sort' => [1 => 'b', 2 => 'a'],
+ ];
+ // Save config which has a schema that enforces sorting.
+ $this->config('config_schema_test.schema_sequence_sort')
+ ->setData($data)
+ ->save();
+
+ $this->assertSame(['a', 'b'], $this->config('config_schema_test.schema_sequence_sort')->get('value_sort'));
+ $this->assertSame([1 => 'b', 2 => 'a'], $this->config('config_schema_test.schema_sequence_sort')->get('no_sort'));
+
+ // Test sorts do not destroy complex values.
+ $data = [
+ 'complex_sort_value' => [['foo' => 'b', 'bar' => 'b'] , ['foo' => 'a', 'bar' => 'a']],
+ 'complex_sort_key' => ['b' => ['foo' => '1', 'bar' => '1'] , 'a' => ['foo' => '2', 'bar' => '2']],
+ ];
+ $this->config('config_schema_test.schema_sequence_sort')
+ ->setData($data)
+ ->save();
+ $this->assertSame([['foo' => 'a', 'bar' => 'a'], ['foo' => 'b', 'bar' => 'b']], $this->config('config_schema_test.schema_sequence_sort')->get('complex_sort_value'));
+ $this->assertSame(['a' => ['foo' => '2', 'bar' => '2'], 'b' => ['foo' => '1', 'bar' => '1']], $this->config('config_schema_test.schema_sequence_sort')->get('complex_sort_key'));
+
+ // Swap the previous test scenario around.
+ $data = [
+ 'complex_sort_value' => ['b' => ['foo' => '1', 'bar' => '1'] , 'a' => ['foo' => '2', 'bar' => '2']],
+ 'complex_sort_key' => [['foo' => 'b', 'bar' => 'b'] , ['foo' => 'a', 'bar' => 'a']],
+ ];
+ $this->config('config_schema_test.schema_sequence_sort')
+ ->setData($data)
+ ->save();
+ $this->assertSame([['foo' => '1', 'bar' => '1'], ['foo' => '2', 'bar' => '2']], $this->config('config_schema_test.schema_sequence_sort')->get('complex_sort_value'));
+ $this->assertSame([['foo' => 'b', 'bar' => 'b'], ['foo' => 'a', 'bar' => 'a']], $this->config('config_schema_test.schema_sequence_sort')->get('complex_sort_key'));
+
+ }
+
/**
* Tests fallback to a greedy wildcard.
*/
--
GitLab