[TASK] Move property merge behavior 45/57045/2
authorRalf Zimmermann <ralf.zimmermann@tritum.de>
Thu, 24 May 2018 07:13:44 +0000 (09:13 +0200)
committerMathias Brodala <mbrodala@pagemachine.de>
Fri, 8 Jun 2018 07:24:08 +0000 (09:24 +0200)
Move the property merge behavior from setOptions() into the related
methods (setRenderingOption(), setProperty(), setDefaultValue()).

Releases: master
Resolves: #85072
Change-Id: I1c002aeb8d889af68d6147d5c588709cd89ebf3c
Reviewed-on: https://review.typo3.org/57045
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Reviewed-by: Björn Jacob <bjoern.jacob@tritum.de>
Tested-by: Björn Jacob <bjoern.jacob@tritum.de>
Reviewed-by: Mathias Brodala <mbrodala@pagemachine.de>
Tested-by: Mathias Brodala <mbrodala@pagemachine.de>
typo3/sysext/form/Classes/Domain/Model/FormDefinition.php
typo3/sysext/form/Classes/Domain/Model/FormElements/AbstractFormElement.php
typo3/sysext/form/Classes/Domain/Model/FormElements/Section.php
typo3/sysext/form/Classes/Domain/Model/Renderable/AbstractRenderable.php
typo3/sysext/form/Tests/Unit/Domain/FormElements/AbstractFormElementTest.php
typo3/sysext/form/Tests/Unit/Domain/FormElements/SectionTest.php
typo3/sysext/form/Tests/Unit/Domain/Renderable/AbstractRenderableTest.php [new file with mode: 0644]

index ce1a34a..974476d 100644 (file)
@@ -354,13 +354,7 @@ class FormDefinition extends AbstractCompositeRenderable
         }
         if (isset($options['renderingOptions'])) {
             foreach ($options['renderingOptions'] as $key => $value) {
-                if (is_array($value)) {
-                    $currentValue = $this->getRenderingOptions()[$key] ?? [];
-                    ArrayUtility::mergeRecursiveWithOverrule($currentValue, $value);
-                    $this->setRenderingOption($key, $currentValue);
-                } else {
-                    $this->setRenderingOption($key, $value);
-                }
+                $this->setRenderingOption($key, $value);
             }
         }
         if (isset($options['finishers'])) {
index c68db7d..7b475b9 100644 (file)
@@ -119,6 +119,11 @@ abstract class AbstractFormElement extends AbstractRenderable implements FormEle
     public function setDefaultValue($defaultValue)
     {
         $formDefinition = $this->getRootForm();
+        $currentDefaultValue = $formDefinition->getElementDefaultValueByIdentifier($this->identifier);
+        if (is_array($currentDefaultValue) && is_array($defaultValue)) {
+            ArrayUtility::mergeRecursiveWithOverrule($currentDefaultValue, $defaultValue);
+            $defaultValue = ArrayUtility::removeNullValuesRecursive($currentDefaultValue);
+        }
         $formDefinition->addElementDefaultValue($this->identifier, $defaultValue);
     }
 
@@ -149,6 +154,9 @@ abstract class AbstractFormElement extends AbstractRenderable implements FormEle
     {
         if (is_array($value) && isset($this->properties[$key]) && is_array($this->properties[$key])) {
             ArrayUtility::mergeRecursiveWithOverrule($this->properties[$key], $value);
+            $this->properties[$key] = ArrayUtility::removeNullValuesRecursive($this->properties[$key]);
+        } elseif ($value === null) {
+            unset($this->properties[$key]);
         } else {
             $this->properties[$key] = $value;
         }
index cb2484d..fdb2a2a 100644 (file)
@@ -119,6 +119,9 @@ class Section extends AbstractSection implements FormElementInterface
     {
         if (is_array($value) && isset($this->properties[$key]) && is_array($this->properties[$key])) {
             ArrayUtility::mergeRecursiveWithOverrule($this->properties[$key], $value);
+            $this->properties[$key] = ArrayUtility::removeNullValuesRecursive($this->properties[$key]);
+        } elseif ($value === null) {
+            unset($this->properties[$key]);
         } else {
             $this->properties[$key] = $value;
         }
@@ -134,7 +137,14 @@ class Section extends AbstractSection implements FormElementInterface
      */
     public function setRenderingOption(string $key, $value)
     {
-        $this->renderingOptions[$key] = $value;
+        if (is_array($value) && isset($this->renderingOptions[$key]) && is_array($this->renderingOptions[$key])) {
+            ArrayUtility::mergeRecursiveWithOverrule($this->renderingOptions[$key], $value);
+            $this->renderingOptions[$key] = ArrayUtility::removeNullValuesRecursive($this->renderingOptions[$key]);
+        } elseif ($value === null) {
+            unset($this->renderingOptions[$key]);
+        } else {
+            $this->renderingOptions[$key] = $value;
+        }
     }
 
     /**
index 624268b..c590931 100644 (file)
@@ -146,13 +146,7 @@ abstract class AbstractRenderable implements RenderableInterface
 
         if (isset($options['renderingOptions'])) {
             foreach ($options['renderingOptions'] as $key => $value) {
-                if (is_array($value)) {
-                    $currentValue = $this->getRenderingOptions()[$key] ?? [];
-                    ArrayUtility::mergeRecursiveWithOverrule($currentValue, $value);
-                    $this->setRenderingOption($key, $currentValue);
-                } else {
-                    $this->setRenderingOption($key, $value);
-                }
+                $this->setRenderingOption($key, $value);
             }
         }
 
@@ -268,7 +262,14 @@ abstract class AbstractRenderable implements RenderableInterface
      */
     public function setRenderingOption(string $key, $value)
     {
-        $this->renderingOptions[$key] = $value;
+        if (is_array($value) && isset($this->renderingOptions[$key]) && is_array($this->renderingOptions[$key])) {
+            ArrayUtility::mergeRecursiveWithOverrule($this->renderingOptions[$key], $value);
+            $this->renderingOptions[$key] = ArrayUtility::removeNullValuesRecursive($this->renderingOptions[$key]);
+        } elseif ($value === null) {
+            unset($this->renderingOptions[$key]);
+        } else {
+            $this->renderingOptions[$key] = $value;
+        }
     }
 
     /**
index 9170bdd..54d961f 100644 (file)
@@ -2,6 +2,19 @@
 declare(strict_types = 1);
 namespace TYPO3\CMS\Form\Tests\Unit\Domain\FormElements;
 
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Form\Domain\Exception\IdentifierNotValidException;
 use TYPO3\CMS\Form\Domain\Model\FormDefinition;
@@ -84,6 +97,43 @@ class AbstractFormElementTest extends UnitTestCase
     /**
      * @test
      */
+    public function setPropertyUnsetIfValueIsNull(): void
+    {
+        $subject = $this->getMockForAbstractClass(AbstractFormElement::class, ['an_id', 'a_type']);
+
+        $expected = ['foo-1' => ['bar-1' => 'foo-2']];
+        $subject->setProperty('foo-1', ['bar-1' => 'foo-2']);
+        $subject->setProperty('foo-2', ['bar-2' => 'foo-3']);
+        $subject->setProperty('foo-2', null);
+
+        $this->assertSame($expected, $subject->getProperties());
+    }
+
+    /**
+     * @test
+     */
+    public function setPropertyUnsetIfValueIsArrayWithSomeNullVales(): void
+    {
+        $subject = $this->getMockForAbstractClass(AbstractFormElement::class, ['an_id', 'a_type']);
+
+        $expected = [
+            'foo-1' => [
+                'bar-1' => 'foo-2'
+            ],
+            'foo-2' => [
+                'bar-2' => 'foo-3'
+            ]
+        ];
+        $subject->setProperty('foo-1', ['bar-1' => 'foo-2']);
+        $subject->setProperty('foo-2', ['bar-2' => 'foo-3', 'bar-3' => 'foo-4']);
+        $subject->setProperty('foo-2', ['bar-3' => null]);
+
+        $this->assertSame($expected, $subject->getProperties());
+    }
+
+    /**
+     * @test
+     */
     public function constructThrowsExceptionWhenIdentifierIsEmpty(): void
     {
         $this->expectException(IdentifierNotValidException::class);
@@ -213,4 +263,174 @@ class AbstractFormElementTest extends UnitTestCase
             $abstractFormElementMock2->getUniqueIdentifier()
         );
     }
+
+    /**
+     * @test
+     */
+    public function setDefaultValueSetStringValueIfKeyDoesNotExists(): void
+    {
+        $formDefinitionMock = $this->getAccessibleMock(FormDefinition::class, [
+            'dummy'
+        ], [], '', false);
+
+        $abstractFormElementMock = $this->getAccessibleMockForAbstractClass(
+            AbstractFormElement::class,
+            ['is_in', 'a_type'],
+            '',
+            true,
+            true,
+            true,
+            ['getRootForm']
+        );
+
+        $abstractFormElementMock
+            ->method('getRootForm')
+            ->willReturn($formDefinitionMock);
+
+        $input = 'foo';
+        $expected = 'foo';
+
+        $abstractFormElementMock->setDefaultValue($input);
+
+        $this->assertSame($expected, $abstractFormElementMock->getDefaultValue());
+    }
+
+    /**
+     * @test
+     */
+    public function setDefaultValueSetArrayValueIfKeyDoesNotExists(): void
+    {
+        $formDefinitionMock = $this->getAccessibleMock(FormDefinition::class, [
+            'dummy'
+        ], [], '', false);
+
+        $abstractFormElementMock = $this->getAccessibleMockForAbstractClass(
+            AbstractFormElement::class,
+            ['is_in', 'a_type'],
+            '',
+            true,
+            true,
+            true,
+            ['getRootForm']
+        );
+
+        $abstractFormElementMock
+            ->method('getRootForm')
+            ->willReturn($formDefinitionMock);
+
+        $input = ['foo' => 'bar'];
+        $expected = ['foo' => 'bar'];
+
+        $abstractFormElementMock->setDefaultValue($input);
+
+        $this->assertSame($expected, $abstractFormElementMock->getDefaultValue());
+    }
+
+    /**
+     * @test
+     */
+    public function setDefaultValueUnsetIfValueIsArrayWithSomeNullVales(): void
+    {
+        $formDefinitionMock = $this->getAccessibleMock(FormDefinition::class, [
+            'dummy'
+        ], [], '', false);
+
+        $abstractFormElementMock = $this->getAccessibleMockForAbstractClass(
+            AbstractFormElement::class,
+            ['is_in', 'a_type'],
+            '',
+            true,
+            true,
+            true,
+            ['getRootForm']
+        );
+
+        $abstractFormElementMock
+            ->method('getRootForm')
+            ->willReturn($formDefinitionMock);
+
+        $input1 = [
+            'foo-1' => [
+                'bar-1' => 'foo-2'
+            ],
+            'foo-2' => [
+                'bar-2' => 'foo-3',
+                'bar-3' => 'foo-4'
+            ]
+        ];
+
+        $input2 = [
+            'foo-2' => [
+                'bar-3' => null
+            ]
+        ];
+
+        $expected = [
+            'foo-1' => [
+                'bar-1' => 'foo-2'
+            ],
+            'foo-2' => [
+                'bar-2' => 'foo-3'
+            ]
+        ];
+
+        $abstractFormElementMock->setDefaultValue($input1);
+        $abstractFormElementMock->setDefaultValue($input2);
+
+        $this->assertSame($expected, $abstractFormElementMock->getDefaultValue());
+    }
+
+    /**
+     * @test
+     */
+    public function setDefaultValueAddValueIfValueIsArray(): void
+    {
+        $formDefinitionMock = $this->getAccessibleMock(FormDefinition::class, [
+            'dummy'
+        ], [], '', false);
+
+        $abstractFormElementMock = $this->getAccessibleMockForAbstractClass(
+            AbstractFormElement::class,
+            ['is_in', 'a_type'],
+            '',
+            true,
+            true,
+            true,
+            ['getRootForm']
+        );
+
+        $abstractFormElementMock
+            ->method('getRootForm')
+            ->willReturn($formDefinitionMock);
+
+        $input1 = [
+            'foo-1' => [
+                'bar-1' => 'foo-2'
+            ],
+            'foo-2' => [
+                'bar-2' => 'foo-3',
+            ]
+        ];
+
+        $input2 = [
+            'foo-2' => [
+                'bar-3' => 'foo-4'
+            ]
+        ];
+
+        $expected = [
+            'foo-1' => [
+                'bar-1' => 'foo-2'
+            ],
+            'foo-2' => [
+                'bar-2' => 'foo-3',
+                'bar-3' => 'foo-4'
+            ]
+        ];
+
+        $abstractFormElementMock->setDefaultValue($input1);
+        $abstractFormElementMock->setDefaultValue($input2);
+
+        $this->assertSame($expected, $abstractFormElementMock->getDefaultValue());
+    }
 }
index fd48763..6ed7b7a 100644 (file)
@@ -2,6 +2,19 @@
 declare(strict_types = 1);
 namespace TYPO3\CMS\Form\Tests\Unit\Domain\FormElements;
 
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
 use TYPO3\CMS\Form\Domain\Model\FormElements\Section;
 use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
 
@@ -86,4 +99,113 @@ class SectionTest extends UnitTestCase
         $this->assertTrue(array_key_exists('bar', $properties['foo']));
         $this->assertEquals('baz', $properties['foo']['bar']);
     }
+
+    /**
+     * @test
+     */
+    public function setPropertyUnsetIfValueIsNull(): void
+    {
+        $expected = ['foo-1' => ['bar-1' => 'foo-2']];
+        $this->sectionInstance->setProperty('foo-1', ['bar-1' => 'foo-2']);
+        $this->sectionInstance->setProperty('foo-2', ['bar-2' => 'foo-3']);
+        $this->sectionInstance->setProperty('foo-2', null);
+
+        $this->assertSame($expected, $this->sectionInstance->getProperties());
+    }
+
+    /**
+     * @test
+     */
+    public function setPropertyUnsetIfValueIsArrayWithSomeNullVales(): void
+    {
+        $expected = [
+            'foo-1' => [
+                'bar-1' => 'foo-2'
+            ],
+            'foo-2' => [
+                'bar-2' => 'foo-3'
+            ]
+        ];
+        $this->sectionInstance->setProperty('foo-1', ['bar-1' => 'foo-2']);
+        $this->sectionInstance->setProperty('foo-2', ['bar-2' => 'foo-3', 'bar-3' => 'foo-4']);
+        $this->sectionInstance->setProperty('foo-2', ['bar-3' => null]);
+
+        $this->assertSame($expected, $this->sectionInstance->getProperties());
+    }
+
+    /**
+     * @test
+     */
+    public function setRenderingOptionSetStringValueIfKeyDoesNotExists(): void
+    {
+        $expected = ['foo' => 'bar'];
+        $this->sectionInstance->setRenderingOption('foo', 'bar');
+
+        $this->assertSame($expected, $this->sectionInstance->getRenderingOptions());
+    }
+
+    /**
+     * @test
+     */
+    public function setRenderingOptionSetArrayValueIfKeyDoesNotExists(): void
+    {
+        $expected = ['foo-1' => ['bar' => 'foo-2']];
+        $this->sectionInstance->setRenderingOption('foo-1', ['bar' => 'foo-2']);
+
+        $this->assertSame($expected, $this->sectionInstance->getRenderingOptions());
+    }
+
+    /**
+     * @test
+     */
+    public function setRenderingOptionUnsetIfValueIsNull(): void
+    {
+        $expected = ['foo-1' => ['bar-1' => 'foo-2']];
+        $this->sectionInstance->setRenderingOption('foo-1', ['bar-1' => 'foo-2']);
+        $this->sectionInstance->setRenderingOption('foo-2', ['bar-2' => 'foo-3']);
+        $this->sectionInstance->setRenderingOption('foo-2', null);
+
+        $this->assertSame($expected, $this->sectionInstance->getRenderingOptions());
+    }
+
+    /**
+     * @test
+     */
+    public function setRenderingOptionUnsetIfValueIsArrayWithSomeNullVales(): void
+    {
+        $expected = [
+            'foo-1' => [
+                'bar-1' => 'foo-2'
+            ],
+            'foo-2' => [
+                'bar-2' => 'foo-3'
+            ]
+        ];
+        $this->sectionInstance->setRenderingOption('foo-1', ['bar-1' => 'foo-2']);
+        $this->sectionInstance->setRenderingOption('foo-2', ['bar-2' => 'foo-3', 'bar-3' => 'foo-4']);
+        $this->sectionInstance->setRenderingOption('foo-2', ['bar-3' => null]);
+
+        $this->assertSame($expected, $this->sectionInstance->getRenderingOptions());
+    }
+
+    /**
+     * @test
+     */
+    public function setRenderingOptionAddValueIfValueIsArray(): void
+    {
+        $expected = [
+            'foo-1' => [
+                'bar-1' => 'foo-2'
+            ],
+            'foo-2' => [
+                'bar-2' => 'foo-3',
+                'bar-3' => 'foo-4'
+            ]
+        ];
+        $this->sectionInstance->setRenderingOption('foo-1', ['bar-1' => 'foo-2']);
+        $this->sectionInstance->setRenderingOption('foo-2', ['bar-2' => 'foo-3']);
+        $this->sectionInstance->setRenderingOption('foo-2', ['bar-3' => 'foo-4']);
+
+        $this->assertSame($expected, $this->sectionInstance->getRenderingOptions());
+    }
 }
diff --git a/typo3/sysext/form/Tests/Unit/Domain/Renderable/AbstractRenderableTest.php b/typo3/sysext/form/Tests/Unit/Domain/Renderable/AbstractRenderableTest.php
new file mode 100644 (file)
index 0000000..0c20ebe
--- /dev/null
@@ -0,0 +1,110 @@
+<?php
+namespace TYPO3\CMS\Form\Tests\Unit\Domain\Renderable;
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+use TYPO3\CMS\Form\Domain\Model\Renderable\AbstractRenderable;
+
+/**
+ * Test case
+ */
+class AbstractRenderableTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
+{
+
+    /**
+     * @test
+     */
+    public function setRenderingOptionSetStringValueIfKeyDoesNotExists()
+    {
+        $abstractRenderableMock = $this->getMockForAbstractClass(AbstractRenderable::class);
+
+        $expected = ['foo' => 'bar'];
+        $abstractRenderableMock->setRenderingOption('foo', 'bar');
+
+        $this->assertSame($expected, $abstractRenderableMock->getRenderingOptions());
+    }
+
+    /**
+     * @test
+     */
+    public function setRenderingOptionSetArrayValueIfKeyDoesNotExists()
+    {
+        $abstractRenderableMock = $this->getMockForAbstractClass(AbstractRenderable::class);
+
+        $expected = ['foo-1' => ['bar' => 'foo-2']];
+        $abstractRenderableMock->setRenderingOption('foo-1', ['bar' => 'foo-2']);
+
+        $this->assertSame($expected, $abstractRenderableMock->getRenderingOptions());
+    }
+
+    /**
+     * @test
+     */
+    public function setRenderingOptionUnsetIfValueIsNull()
+    {
+        $abstractRenderableMock = $this->getMockForAbstractClass(AbstractRenderable::class);
+
+        $expected = ['foo-1' => ['bar-1' => 'foo-2']];
+        $abstractRenderableMock->setRenderingOption('foo-1', ['bar-1' => 'foo-2']);
+        $abstractRenderableMock->setRenderingOption('foo-2', ['bar-2' => 'foo-3']);
+        $abstractRenderableMock->setRenderingOption('foo-2', null);
+
+        $this->assertSame($expected, $abstractRenderableMock->getRenderingOptions());
+    }
+
+    /**
+     * @test
+     */
+    public function setRenderingOptionUnsetIfValueIsArrayWithSomeNullVales()
+    {
+        $abstractRenderableMock = $this->getMockForAbstractClass(AbstractRenderable::class);
+
+        $expected = [
+            'foo-1' => [
+                'bar-1' => 'foo-2'
+            ],
+            'foo-2' => [
+                'bar-2' => 'foo-3'
+            ]
+        ];
+        $abstractRenderableMock->setRenderingOption('foo-1', ['bar-1' => 'foo-2']);
+        $abstractRenderableMock->setRenderingOption('foo-2', ['bar-2' => 'foo-3', 'bar-3' => 'foo-4']);
+        $abstractRenderableMock->setRenderingOption('foo-2', ['bar-3' => null]);
+
+        $this->assertSame($expected, $abstractRenderableMock->getRenderingOptions());
+    }
+
+    /**
+     * @test
+     */
+    public function setRenderingOptionAddValueIfValueIsArray()
+    {
+        $abstractRenderableMock = $this->getMockForAbstractClass(AbstractRenderable::class);
+
+        $expected = [
+            'foo-1' => [
+                'bar-1' => 'foo-2'
+            ],
+            'foo-2' => [
+                'bar-2' => 'foo-3',
+                'bar-3' => 'foo-4'
+            ]
+        ];
+        $abstractRenderableMock->setRenderingOption('foo-1', ['bar-1' => 'foo-2']);
+        $abstractRenderableMock->setRenderingOption('foo-2', ['bar-2' => 'foo-3']);
+        $abstractRenderableMock->setRenderingOption('foo-2', ['bar-3' => 'foo-4']);
+
+        $this->assertSame($expected, $abstractRenderableMock->getRenderingOptions());
+    }
+}