Skip to content

Commit

Permalink
[BUGFIX] EXT:form - fix array overrules within YAML preprocessing
Browse files Browse the repository at this point in the history 
If you use the "__inheritance" operator within an EXT:form configuration
file, configuration keys of the parent element can be deleted in the
child element by giving the configuration key in the child element
the value NULL.
See https://docs.typo3.org/typo3cms/extensions/form/latest/Concepts/
Configuration/Index.html#inheritances for further information.

Before the "__inheritance" operators are executed, all configuration
files are merged using
TYPO3\CMS\Core\Utility\ArrayUtility::mergeRecursiveWithOverrule().

However, this does not work if you are using several configuration files.

Let's assume the configuration key in the previous configuration file is
an array. mergeRecursiveWithOverrule() does not delete this
configuration key, if the configuration key in the overriding
configuration file is not an array (for example: NULL). This is simply
ignored by mergeRecursiveWithOverrule().

This patch fixes this issue by adding a variation of
array_merge_recursive().

Resolves: #82051
Releases: master, 8.7
Change-Id: Id9d256226a3eb82f6bc3fd03904f944719e525e7
Reviewed-on: https://review.typo3.org/55487
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Tobi Kretschmann <tobi@tobishome.de>
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Susanne Moog <susanne.moog@typo3.org>
Tested-by: Susanne Moog <susanne.moog@typo3.org>
Reviewed-by: Frank Naegler <frank.naegler@typo3.org>
Tested-by: Frank Naegler <frank.naegler@typo3.org>
Reviewed-by: Peter Kraume <peter.kraume@gmx.de>
Tested-by: Peter Kraume <peter.kraume@gmx.de>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
  • Loading branch information
@waldhacker1 @ohader
waldhacker1 authored and ohader committed Feb 5, 2018
1 parent 308242e commit 7789987
Show file tree
Hide file tree
Showing 5 changed files with 164 additions and 3 deletions.
26 changes: 25 additions & 1 deletion typo3/sysext/form/Classes/Mvc/Configuration/YamlSource.php
View file
Expand Up @@ -105,7 +105,7 @@ public function load(array $filesToLoad): array
}

if (is_array($loadedConfiguration)) {
ArrayUtility::mergeRecursiveWithOverrule($configuration, $loadedConfiguration);
$this->mergeRecursiveWithOverrule($configuration, $loadedConfiguration);
}
} catch (ParseException $exception) {
throw new ParseErrorException(
Expand Down Expand Up @@ -182,4 +182,28 @@ protected function getHeaderFromFile($file): string
}
return $header;
}

/**
* The differences to the existing PHP function array_merge_recursive() are:
* * If the original value is an array and the overrule value is something else
* (like null) the overrule value is used.
* (TYPO3\CMS\Core\Utility\ArrayUtility::mergeRecursiveWithOverrule does not do this)
*
* @param array $original Original array. It will be *modified* by this method and contains the result afterwards!
* @param array $overrule Overrule array, overruling the original array
*/
protected function mergeRecursiveWithOverrule(array &$original, array $overrule)
{
foreach ($overrule as $key => $_) {
if (isset($original[$key]) && is_array($original[$key])) {
if (is_array($overrule[$key])) {
$this->mergeRecursiveWithOverrule($original[$key], $overrule[$key]);
} else {
$original[$key] = $overrule[$key];
}
} else {
$original[$key] = $overrule[$key];
}
}
}
}
3 changes: 1 addition & 2 deletions typo3/sysext/form/Documentation/Concepts/Configuration/Index.rst
View file
Expand Up @@ -244,8 +244,7 @@ The final YAML configuration is not based on one huge file. Instead, it is
a compilation of a sequential process:

- First of all, all registered configuration files are parsed as YAML and
are overlain according to their order. ``TYPO3\CMS\Core\Utility\ArrayUtility::mergeRecursiveWithOverrule()``
is involved in this first step.
are overlaid according to their order.
- After that, the ``__inheritances`` operator is applied. It is a unique
operator introduced by the form framework.
- Finally, all configuration entries with a value of ``null`` are deleted.
Expand Down
5 changes: 5 additions & 0 deletions ...xt/form/Tests/Unit/Mvc/Configuration/Fixtures/OverruleNonArrayValuesOverArrayValues1.yaml
View file
@@ -0,0 +1,5 @@
Form:
klaus01:
key01: value1
key02: value2
key03: value2
2 changes: 2 additions & 0 deletions ...xt/form/Tests/Unit/Mvc/Configuration/Fixtures/OverruleNonArrayValuesOverArrayValues2.yaml
View file
@@ -0,0 +1,2 @@
Form:
klaus01: null
131 changes: 131 additions & 0 deletions typo3/sysext/form/Tests/Unit/Mvc/Configuration/YamlSourceTest.php
View file
Expand Up @@ -103,4 +103,135 @@ public function getHeaderFromFileReturnsHeaderPart()

$this->assertSame($expected, $mockYamlSource->_call('getHeaderFromFile', $input));
}

/**
* @test
*/
public function loadOverruleNonArrayValuesOverArrayValues()
{
$mockYamlSource = $this->getAccessibleMock(YamlSource::class, ['dummy'], [], '', false);

$input = [
'EXT:form/Tests/Unit/Mvc/Configuration/Fixtures/OverruleNonArrayValuesOverArrayValues1.yaml',
'EXT:form/Tests/Unit/Mvc/Configuration/Fixtures/OverruleNonArrayValuesOverArrayValues2.yaml'
];

$expected = [
'Form' => [
'klaus01' => null,
'key03' => 'value2',
],
];

$this->assertSame($expected, $mockYamlSource->_call('load', $input));
}

/**
* @return array
*/
public function mergeRecursiveWithOverruleCalculatesExpectedResultDataProvider()
{
return [
'Override array can reset string to array' => [
[
'first' => [
'second' => 'foo',
],
],
[
'first' => [
'second' => ['third' => 'bar'],
],
],
[
'first' => [
'second' => ['third' => 'bar'],
],
],
],
'Override array does reset array to string' => [
[
'first' => [],
],
[
'first' => 'foo',
],
[
'first' => 'foo', // Note that ArrayUtility::mergeRecursiveWithOverrule returns [] here
],
],
'Override array does override null with string' => [
[
'first' => null,
],
[
'first' => 'foo',
],
[
'first' => 'foo',
],
],
'Override array does override null with empty string' => [
[
'first' => null,
],
[
'first' => '',
],
[
'first' => '',
],
],
'Override array does override string with null' => [
[
'first' => 'foo',
],
[
'first' => null,
],
[
'first' => null, // Note that ArrayUtility::mergeRecursiveWithOverrule returns 'foo' here
],
],
'Override array does override null with null' => [
[
'first' => null,
],
[
'first' => null,
],
[
'first' => null, // Note that ArrayUtility::mergeRecursiveWithOverrule returns '' here
],
],
'Override can add keys' => [
[
'first' => 'foo',
],
[
'second' => 'bar',
],
[
'first' => 'foo',
'second' => 'bar',
],
],
];
}

/**
* Note the data provider is similar to the data provider for ArrayUtility::mergeRecursiveWithOverrule()
*
* @test
* @dataProvider mergeRecursiveWithOverruleCalculatesExpectedResultDataProvider
* @param array $input1 Input 1
* @param array $input2 Input 2
* @param array $expected expected array
*/
public function mergeRecursiveWithOverruleCalculatesExpectedResult($input1, $input2, $expected)
{
$mockYamlSource = $this->getAccessibleMock(YamlSource::class, ['dummy'], [], '', false);
$mockYamlSource->_callRef('mergeRecursiveWithOverrule', $input1, $input2);
$this->assertSame($expected, $input1);
}
}

0 comments on commit 7789987

Please sign in to comment.