Issue #3238855 by bircher, pyrello, neclimdul, sker101, mparker17: Better... (c5bc2524) · Commits · project / config_split

config_split.services.yml

+1 −0

Original line number	Diff line number	Diff line
		@@ -36,6 +36,7 @@ services:
		class: Drupal\config_split\Config\ConfigPatchMerge
		arguments:
		- "@config_split.sorter"
		- "@config.typed"

		config_split.status_override:
		class: Drupal\config_split\Config\StatusOverride

src/Config/ConfigPatchMerge.php

+226 −79

Original line number	Diff line number	Diff line
		@@ -4,6 +4,11 @@

		namespace Drupal\config_split\Config;

		use Drupal\Core\Config\Schema\ArrayElement;
		use Drupal\Core\Config\Schema\Element;
		use Drupal\Core\Config\Schema\Sequence;
		use Drupal\Core\Config\TypedConfigManagerInterface;

		/**
		* The patch merging service.
		*
		@@ -18,14 +23,24 @@ class ConfigPatchMerge {
		*/
		protected $configSorter;

		/**
		* The typed config manager.
		*
		* @var \Drupal\Core\Config\TypedConfigManagerInterface
		*/
		protected $typedConfigManager;

		/**
		* The service constructor.
		*
		* @param \Drupal\config_split\Config\ConfigSorter $configSorter
		* The sorter.
		* @param \Drupal\Core\Config\TypedConfigManagerInterface $typedConfigManager
		* The typed config manager.
		*/
		public function __construct(ConfigSorter $configSorter) {
		public function __construct(ConfigSorter $configSorter, TypedConfigManagerInterface $typedConfigManager) {
		$this->configSorter = $configSorter;
		$this->typedConfigManager = $typedConfigManager;
		}

		/**
		@@ -35,14 +50,18 @@ public function __construct(ConfigSorter $configSorter) {
		* The original data.
		* @param array $new
		* The new data.
		* @param string $name
		* The name of the config.
		*
		* @return \Drupal\config_split\Config\ConfigPatch
		* The patch object.
		*/
		public function createPatch(array $original, array $new): ConfigPatch {
		public function createPatch(array $original, array $new, string $name): ConfigPatch {
		/** @var \Drupal\Core\Config\Schema\Element $element */
		$element = $this->typedConfigManager->createFromNameAndData($name, $original);
		return ConfigPatch::fromArray([
		'added' => self::diffArrayRecursive($new, $original),
		'removed' => self::diffArrayRecursive($original, $new),
		'added' => self::diffArray($new, $original, $element),
		'removed' => self::diffArray($original, $new, $element),
		]);
		}

		@@ -63,9 +82,12 @@ public function mergePatch(array $config, ConfigPatch $patch, string $name): arr
		if ($patch->isEmpty()) {
		return $config;
		}
		/** @var \Drupal\Core\Config\Schema\Element $element */
		$element = $this->typedConfigManager->createFromNameAndData($name, $config);

		$changed = self::diffArrayRecursive($config, $patch->getRemoved());
		$changed = self::mergeDeepArray([$changed, $patch->getAdded()]);
		$changed = self::diffArray($config, $patch->getRemoved(), $element);
		$changed = self::mergeArray($changed, $patch->getAdded(), $element);
		$changed = self::removeSequenceKeys($changed);

		// Make sure not to remove the dependencies key from config entities.
		if (isset($config['dependencies']) && !isset($changed['dependencies'])) {
		@@ -81,119 +103,147 @@ public function mergePatch(array $config, ConfigPatch $patch, string $name): arr
		/**
		* Recursively computes the difference of arrays.
		*
		* This is an alternate version of DiffArray::diffAssocRecursive that ignores
		* the position of numeric keys when constructing a diff between two arrays.
		* This method transforms the sequence keys and then calls the utility method.
		*
		* @param array $array1
		* The array to compare from.
		* @param array $array2
		* The array to compare to.
		* @param \Drupal\Core\Config\Schema\Element $element
		* The typed config element.
		* @param string $path
		* The path into the config.
		*
		* @return array
		* Returns an array containing all the values from array1 that are not
		* present in array2.
		*
		* @see DiffArray::diffAssocRecursive()
		* @see self::utilityDiffAssocRecursive()
		*/
		private static function diffArrayRecursive(array $array1, array $array2) {
		$difference = [];
		$is_sequence = self::isSequence($array1) && self::isSequence($array2);
		private static function diffArray(array $array1, array $array2, Element $element, string $path = ''): array {
		// This should not be necessary, but somehow objects have been part of it.
		self::handleStrayObjects($array1);
		self::handleStrayObjects($array2);

		// Make sure that all keys are strings addressing the elements.
		$keysMatter = TRUE;
		if (self::isSequence($array1, $array2, $element, $path)) {
		$keysMatter = FALSE;
		$array1 = self::transformSequenceKeys($array1, $keysMatter);
		$array2 = self::transformSequenceKeys($array2, $keysMatter);
		}

		$i = 0;
		$diff = [];
		foreach ($array1 as $key => $value) {
		if (is_array($value)) {
		// If this is a sequence, then we are searching for the value in $array2
		// and using the returned key instead.
		if ($is_sequence) {
		$key = array_search($value, $array2);
		}
		// If the key doesn't exist in $array2, add it to the diff and skip the
		// rest of the processing.
		if ($key === FALSE) {
		$difference[] = $value;
		continue;
		}
		if (!array_key_exists($key, $array2) \|\| !is_array($array2[$key])) {
		// Don't preserve the existing key if this is a sequence.
		if ($is_sequence) {
		$difference[] = $value;
		}
		else {
		$difference[$key] = $value;
		}
		$diff[$key] = $value;
		}
		else {
		$new_diff = self::diffArrayRecursive($value, $array2[$key]);
		// Fix the lookup key and recurse.
		$lookup_key = strpos((string) $key, 'config_split_sequence') === 0 ? $i : $key;
		$lookup_key = empty($path) ? $lookup_key : $path . '.' . $lookup_key;
		$new_diff = self::diffArray($value, $array2[$key], $element, $lookup_key);
		if (!empty($new_diff)) {
		$difference[$key] = $new_diff;
		$diff[$key] = $new_diff;
		}
		}
		}
		else {
		// If this is an object, cast it to an array. This should never happen.
		if (is_object($value)) {
		if (method_exists($value, 'toArray')) {
		$value = $value->toArray();
		}
		else {
		$value = (array) $value;
		}
		}
		// If this is a sequence, don't preserve the key.
		if ($is_sequence) {
		if (!in_array($value, $array2, TRUE)) {
		$difference[] = $value;
		}
		}
		elseif (!array_key_exists($key, $array2) \|\| $array2[$key] !== $value) {
		$difference[$key] = $value;
		$diff[$key] = $value;
		}
		$i++;
		}

		if (!$keysMatter) {
		// If there are no duplicates we can go back to integer keys for a more
		// natural patch export.
		$diff = array_values($diff);
		}

		return $difference;
		return $diff;
		}

		/**
		* Merges multiple arrays, recursively, and returns the merged array.
		* Merges two arrays recursively.
		*
		* This function is similar to NestedArray::mergeDeepArray(), with the
		* exception that it remove duplicate elements.
		*
		* @param array $arrays
		* An arrays of arrays to merge.
		* @param array $array1
		* The first array.
		* @param array $array2
		* The second array.
		* @param \Drupal\Core\Config\Schema\Element $element
		* The typed config element.
		* @param string $path
		* The path into the config.
		*
		* @return array
		* The merged array.
		*
		* @see NestedArray::mergeDeepArray()
		* @see self::utilityMergeDeepArray()
		*/
		private static function mergeDeepArray(array $arrays) {
		$result = [];
		private static function mergeArray(array $array1, array $array2, Element $element, string $path = ''): array {
		if (self::isSequence($array1, $array2, $element, $path)) {
		$array1 = self::transformSequenceKeys($array1);
		$array2 = self::transformSequenceKeys($array2);
		}

		foreach ($arrays as $array) {
		$is_sequence = self::isSequence($array);
		$result = [];
		foreach ([$array1, $array2] as $array) {
		$i = 0;
		foreach ($array as $key => $value) {
		// If this is a sequence, we are not preserving the key.
		if ($is_sequence) {
		// Don't add duplicate values.
		if (!in_array($value, $result)) {
		$result[] = $value;
		}
		}
		// Recurse when both values are arrays.
		elseif (isset($result[$key]) && is_array($result[$key]) && is_array($value)) {
		$result[$key] = self::mergeDeepArray([$result[$key], $value]);
		if (isset($result[$key]) && is_array($result[$key]) && is_array($value)) {
		// Fix the lookup key.
		$lookup_key = strpos((string) $key, 'config_split_sequence') === 0 ? $i : $key;
		$lookup_key = empty($path) ? $lookup_key : $path . '.' . $lookup_key;
		$result[$key] = self::mergeArray($result[$key], $value, $element, $lookup_key);
		}
		// Otherwise, use the latter value, overriding any previous value.
		else {
		$result[$key] = $value;
		}
		$i++;
		}
		}

		// The merged array is cleaned up after the merge is complete.
		return $result;
		}

		/**
		* Check if data is a sequence.
		*
		* @param array $array1
		* The data.
		* @param array $array2
		* The data.
		* @param \Drupal\Core\Config\Schema\Element $element
		* The typed config element.
		* @param string $path
		* The path into the config.
		*
		* @return bool
		* True if the input is considered a sequence.
		*/
		public static function isSequence(array $array1, array $array2, Element $element, string $path): bool {
		if ($element instanceof ArrayElement && !empty($path)) {
		try {
		$type = $element->get($path);
		if ($type instanceof Sequence) {
		return TRUE;
		}
		}
		catch (\InvalidArgumentException $exception) {
		// Fall back to the strict key comparison if the element can not be
		// accessed directly.
		}
		}

		// Both need to be indexed arrays or have escaped keys.
		return self::isList(self::removeSequenceKeys($array1)) && self::isList(self::removeSequenceKeys($array2));
		}

		/**
		* Check if given array is a normal indexed array or not.
		*
		@@ -203,14 +253,9 @@ private static function mergeDeepArray(array $arrays) {
		* @return bool
		* True if value is a sequence array, false otherwise.
		*/
		private static function isSequence($value): bool {
		// If the $value is empty, we'll err on the side of it not being a sequence.
		if (empty($value)) {
		return FALSE;
		}

		private static function isList($value): bool {
		// In Drupal 10 (php 8.1) this can be replaced with array_is_list.
		$expected_key = 0;

		foreach ($value as $key => $val) {
		if ($key !== $expected_key++) {
		return FALSE;
		@@ -220,4 +265,106 @@ private static function isSequence($value): bool {
		return TRUE;
		}

		/**
		* Transforms sequential keys into unique string keys.
		*
		* @param array $array
		* The array to transform.
		* @param bool $keysMatter
		* Will be set to true if there are duplicate entries or keys from uuids.
		*
		* @return array
		* The transformed array.
		*/
		private static function transformSequenceKeys(array $array, bool &$keysMatter = FALSE): array {
		$transformed = [];
		foreach ($array as $key => $value) {
		if (!is_numeric($key)) {
		// This check ensures we only transform sequences with numeric keys.
		$keysMatter = TRUE;
		return $array;
		}

		$candidate = $value;
		if (is_array($value)) {
		if (isset($value['uuid'])) {
		$keysMatter = TRUE;
		$candidate = $value['uuid'];
		}
		else {
		// For sequences that are arrays without uuids, there is not really a
		// good way to address them. So we use the key and accept that things
		// will inevitably not work when the order changes.
		$candidate = $key;
		}
		}

		// We don't know what the data is, so we make sure it can be used as a
		// yaml key by hashing it. We shorten the hash because it will be enough.
		$candidate = substr(sha1(serialize($candidate)), 0, 20);
		// Shorten the string by using more letters.
		$candidate = implode('', array_map(function ($piece) {
		// Five characters of base 16 fit into 4 characters of base 32.
		return base_convert($piece, 16, 32);
		}, str_split($candidate, 5)));

		$i = 1;
		$key = 'config_split_sequence_' . $candidate;
		while (array_key_exists($key, $transformed)) {
		$keysMatter = TRUE;
		$key = 'config_split_sequence_' . $i . '_' . $candidate;
		$i++;
		}

		$transformed[$key] = $value;
		}

		return $transformed;
		}

		/**
		* Recursively removes sequence keys added by self::transformSequenceKeys().
		*
		* @param array $array
		* The array to remove sequence keys from.
		*
		* @return array
		* The cleaned up array.
		*/
		private static function removeSequenceKeys(array $array): array {
		$sequence = FALSE;
		foreach ($array as $key => &$value) {
		if (strpos((string) $key, 'config_split_sequence_') === 0) {
		$sequence = TRUE;
		}
		if (is_array($value)) {
		$value = self::removeSequenceKeys($value);
		}
		}
		if ($sequence) {
		return array_values($array);
		}
		return $array;
		}

		/**
		* Make sure arrays do not contain objects.
		*
		* @param array $array
		* The array to massage.
		*/
		private static function handleStrayObjects(array &$array) {
		array_walk($array, function (&$value, $key) {
		// It is still unknown how this could happen.
		if (is_object($value)) {
		if (method_exists($value, 'toArray')) {
		$value = $value->toArray();
		}
		else {
		$value = (array) $value;
		}
		}
		});
		}

		}

src/ConfigSplitManager.php

+1 −1

Original line number	Diff line number	Diff line
		@@ -808,7 +808,7 @@ protected function createPatch($name, array $original, array $updated, StorageIn
		$existing = ConfigPatch::fromArray($storage->read(self::SPLIT_PARTIAL_PREFIX . $name));
		$updated = $this->patchMerge->mergePatch($updated, $existing, $name);
		}
		$patch = $this->patchMerge->createPatch($original, $updated);
		$patch = $this->patchMerge->createPatch($original, $updated, $name);
		if (!$patch->isEmpty()) {
		$storage->write(self::SPLIT_PARTIAL_PREFIX . $name, $patch->toArray());
		}

tests/src/Kernel/RolesSplittingTest.php

0 → 100644

+101 −0

Original line number	Diff line number	Diff line
		<?php

		declare(strict_types=1);

		namespace Drupal\Tests\config_split\Kernel;

		use Drupal\config_split\Config\ConfigPatch;
		use Drupal\Core\Config\StorageCopyTrait;
		use Drupal\KernelTests\KernelTestBase;
		use Drupal\Tests\config_filter\Kernel\ConfigStorageTestTrait;
		use Drupal\user\Entity\Role;

		/**
		* Test how roles are split as a proxy for other config with sequences.
		*
		* @group config_split
		*/
		class RolesSplittingTest extends KernelTestBase {

		use ConfigStorageTestTrait;
		use SplitTestTrait;
		use StorageCopyTrait;

		/**
		* {@inheritdoc}
		*/
		public static $modules = [
		'system',
		'user',
		'config_split',
		];

		/**
		* {@inheritdoc}
		*/
		protected function setUp(): void {
		parent::setUp();

		// We need the roles to play with.
		$this->installEntitySchema('user');
		}

		/**
		* Test splitting a module where a role has a permission from it.
		*/
		public function testRoleSplit() {
		// We use permissions from this module and split it.
		$this->enableModules(['shortcut']);

		// Create a role with permissions from different modules.
		$role = Role::create([
		'id' => 'test_role',
		'label' => $this->randomString(),
		'permissions' => [
		// Permissions from the user module.
		'administer account settings',
		'administer permissions',

		// Permissions from the shortcut module in the middle.
		'customize shortcut links',

		// More permissions from the user module.
		'select account cancellation method',
		'view user email addresses',
		],
		]);
		$role->save();

		// Create a split for the shourtcut module.
		$this->createSplitConfig('test_split', [
		// We use the collection storage so that we can read the patch directly.
		'storage' => 'collection',
		'module' => ['shortcut' => 0],
		]);

		// Run the export by accessing the export storage.
		$storage = $this->getExportStorage();

		// Permissions remain a sorted sequence aka list.
		$expectedPermissions = [
		'administer account settings',
		'administer permissions',
		'select account cancellation method',
		'view user email addresses',
		];
		$exported = $storage->read('user.role.test_role');
		self::assertEquals($expectedPermissions, $exported['permissions']);

		// The patch should just contain shortcut things.
		$expectedPatch = ConfigPatch::fromArray([
		'adding' => [
		'dependencies' => ['module' => ['shortcut']],
		'permissions' => ['customize shortcut links'],
		],
		'removing' => [],
		]);
		$patch = $storage->createCollection('split.test_split')->read('config_split.patch.user.role.test_role');
		self::assertEquals($expectedPatch->toArray(), $patch);
		}

		}

tests/src/Unit/ConfigPatchTest.php

+70 −6

Original line number	Diff line number	Diff line
		@@ -7,7 +7,10 @@
		use Drupal\config_split\Config\ConfigPatch;
		use Drupal\config_split\Config\ConfigPatchMerge;
		use Drupal\config_split\Config\ConfigSorter;
		use Drupal\Core\Config\Schema\Undefined;
		use Drupal\Core\Config\TypedConfigManagerInterface;
		use Drupal\Core\Template\Attribute;
		use Drupal\Core\TypedData\DataDefinition;
		use PHPUnit\Framework\TestCase;
		use Prophecy\Argument;

		@@ -39,7 +42,12 @@ public function setUp(): void {
		return $aggressiveSorter->sort(...$args);
		});

		$this->patchMerge = new ConfigPatchMerge($sorter->reveal());
		$typedConfigManager = $this->prophesize(TypedConfigManagerInterface::class);
		$typedConfigManager->createFromNameAndData(Argument::type('string'), Argument::type('array'))->will(function ($args) {
		return Undefined::createInstance(DataDefinition::create('any'));
		});

		$this->patchMerge = new ConfigPatchMerge($sorter->reveal(), $typedConfigManager->reveal());
		}

		/**
		@@ -55,8 +63,8 @@ public function setUp(): void {
		* @dataProvider standardPatchMergeProvider
		*/
		public function testStandardPatchMerge(array $configA, array $configB, ConfigPatch $expectedAB = NULL) {
		$patchAB = $this->patchMerge->createPatch($configA, $configB);
		$patchBA = $this->patchMerge->createPatch($configB, $configA);
		$patchAB = $this->patchMerge->createPatch($configA, $configB, 'test');
		$patchBA = $this->patchMerge->createPatch($configB, $configA, 'test');

		// Applying the patch gives the other config.
		self::assertEquals($configA, $this->patchMerge->mergePatch($configB, $patchBA, 'test'));
		@@ -185,6 +193,62 @@ public function standardPatchMergeProvider() {
		],
		],
		];

		yield 'test with duplicates' => [
		'configA' => [
		'list A' => [
		'A',
		'A',
		'A',
		'B',
		],
		],
		'configB' => [
		'list A' => [
		'A',
		'A',
		'A',
		'A',
		'A',
		'C',
		],
		],
		];

		yield 'test with nested lists' => [
		'configA' => [
		'list A' => [
		[
		'uuid' => '1111',
		'value' => 'A',
		],
		[
		'uuid' => '2222',
		'value' => 'A',
		],
		[
		'uuid' => '3333',
		'value' => 'A',
		],
		],
		],
		'configB' => [
		'list A' => [
		[
		'uuid' => '1111',
		'value' => 'A',
		],
		[
		'uuid' => '2222',
		'value' => 'B',
		],
		[
		'uuid' => '3333',
		'value' => 'A',
		],
		],
		],
		];
		}

		/**
		@@ -219,12 +283,12 @@ public function testUpdateMergeExample() {
		];

		// This is the patch which is already in the split storage.
		$patch1 = $this->patchMerge->createPatch($active, $updated);
		$patch1 = $this->patchMerge->createPatch($active, $updated, 'test');
		// This is the "fixed" sync storage so that we can create a merged patch.
		$fixed = $this->patchMerge->mergePatch($sync, $patch1, 'test');

		// This is the patch we want to export.
		$patch2 = $this->patchMerge->createPatch($active, $fixed);
		$patch2 = $this->patchMerge->createPatch($active, $fixed, 'test');
		// This is what we export.
		$export = $this->patchMerge->mergePatch($active, $patch2, 'test');
		self::assertEquals($expected, $export);
		@@ -309,7 +373,7 @@ public function testArrayWithObjectDiff(): void {
		'object' => (array) $object,
		];

		$patch1 = $this->patchMerge->createPatch($array1, $array2);
		$patch1 = $this->patchMerge->createPatch($array1, $array2, 'test');

		self::assertEquals($removed, $patch1->getRemoved());
		self::assertEquals($added, $patch1->getAdded());