[BUGFIX] EXT:form - fix array overrules within YAML preprocessing 65/55565/2
authorRalf Zimmermann <ralf.zimmermann@tritum.de>
Mon, 29 Jan 2018 13:56:45 +0000 (14:56 +0100)
committerChristian Kuhn <lolli@schwarzbu.ch>
Mon, 5 Feb 2018 18:27:19 +0000 (19:27 +0100)
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/55565
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: Peter Kraume <peter.kraume@gmx.de>
Tested-by: Peter Kraume <peter.kraume@gmx.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
typo3/sysext/form/Classes/Mvc/Configuration/YamlSource.php
typo3/sysext/form/Documentation/Concepts/Configuration/Index.rst
typo3/sysext/form/Tests/Unit/Mvc/Configuration/Fixtures/OverruleNonArrayValuesOverArrayValues1.yaml [new file with mode: 0644]
typo3/sysext/form/Tests/Unit/Mvc/Configuration/Fixtures/OverruleNonArrayValuesOverArrayValues2.yaml [new file with mode: 0644]
typo3/sysext/form/Tests/Unit/Mvc/Configuration/YamlSourceTest.php

index 2804fe6..ca83429 100644 (file)
@@ -105,7 +105,7 @@ class YamlSource
                 }
 
                 if (is_array($loadedConfiguration)) {
-                    ArrayUtility::mergeRecursiveWithOverrule($configuration, $loadedConfiguration);
+                    $this->mergeRecursiveWithOverrule($configuration, $loadedConfiguration);
                 }
             } catch (ParseException $exception) {
                 throw new ParseErrorException(
@@ -183,4 +183,28 @@ class YamlSource
         }
         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];
+            }
+        }
+    }
 }
index e620adf..e3d2ca3 100644 (file)
@@ -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.
diff --git a/typo3/sysext/form/Tests/Unit/Mvc/Configuration/Fixtures/OverruleNonArrayValuesOverArrayValues1.yaml b/typo3/sysext/form/Tests/Unit/Mvc/Configuration/Fixtures/OverruleNonArrayValuesOverArrayValues1.yaml
new file mode 100644 (file)
index 0000000..85635aa
--- /dev/null
@@ -0,0 +1,5 @@
+Form:
+  klaus01:
+    key01: value1
+    key02: value2
+  key03: value2
\ No newline at end of file
diff --git a/typo3/sysext/form/Tests/Unit/Mvc/Configuration/Fixtures/OverruleNonArrayValuesOverArrayValues2.yaml b/typo3/sysext/form/Tests/Unit/Mvc/Configuration/Fixtures/OverruleNonArrayValuesOverArrayValues2.yaml
new file mode 100644 (file)
index 0000000..25e2e41
--- /dev/null
@@ -0,0 +1,2 @@
+Form:
+  klaus01: null
\ No newline at end of file
index ccc114f..e51be26 100644 (file)
@@ -103,4 +103,135 @@ class YamlSourceTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
 
         $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);
+    }
 }