[TASK] EXT:form - Optimize validation in InArrayValidator 84/47484/2
authorFlorian Mast <flo.mast@web.de>
Sat, 5 Mar 2016 15:21:59 +0000 (16:21 +0100)
committerMorton Jonuschat <m.jonuschat@mojocode.de>
Fri, 1 Apr 2016 10:34:28 +0000 (12:34 +0200)
The InArrayValidator is currently case sensitive. This
patchset adds a new TypoScript option "ignorecase".

Furthermore, unit tests are added to test the new option
"ignorecase". The patchset also solves an unreported
bug which occurs when using the InArrayValidator in the
form wizard.

Resolves: #69355
Releases: master, 7.6
Change-Id: I9f93b533947bbaef2259c2a53265af452a78924f
Reviewed-on: https://review.typo3.org/47105
Reviewed-by: Susanne Moog <typo3@susannemoog.de>
Tested-by: Susanne Moog <typo3@susannemoog.de>
Reviewed-by: Morton Jonuschat <m.jonuschat@mojocode.de>
Tested-by: Morton Jonuschat <m.jonuschat@mojocode.de>
(cherry picked from commit e8d3853c54fca4198d16f5b1de76a48171740655)
Reviewed-on: https://review.typo3.org/47484

typo3/sysext/form/Classes/Domain/Validator/AbstractValidator.php
typo3/sysext/form/Classes/Domain/Validator/InArrayValidator.php
typo3/sysext/form/Tests/Unit/Validator/AbstractValidatorTest.php
typo3/sysext/form/Tests/Unit/Validator/InArrayValidatorTest.php
typo3/sysext/form/Tests/Unit/Validator/LengthValidatorTest.php

index 0fac6ab..bc9400a 100755 (executable)
@@ -14,6 +14,7 @@ namespace TYPO3\CMS\Form\Domain\Validator;
  * The TYPO3 project - inspiring people to share!
  */
 
+use TYPO3\CMS\Extbase\Utility\LocalizationUtility;
 use TYPO3\CMS\Form\Utility\FormUtility;
 
 abstract class AbstractValidator extends \TYPO3\CMS\Extbase\Validation\Validator\AbstractValidator
@@ -89,7 +90,7 @@ abstract class AbstractValidator extends \TYPO3\CMS\Extbase\Validation\Validator
     public function getLocalLanguageLabel($type = '')
     {
         $label = static::LOCALISATION_OBJECT_NAME . '.' . $type;
-        $message = \TYPO3\CMS\Extbase\Utility\LocalizationUtility::translate($label, 'form');
+        $message = LocalizationUtility::translate($label, 'form');
         return $message;
     }
 
index 1b82605..92c7fc4 100755 (executable)
@@ -14,6 +14,9 @@ namespace TYPO3\CMS\Form\Domain\Validator;
  * The TYPO3 project - inspiring people to share!
  */
 
+use TYPO3\CMS\Core\Charset\CharsetConverter;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+
 class InArrayValidator extends AbstractValidator
 {
     /**
@@ -22,11 +25,33 @@ class InArrayValidator extends AbstractValidator
     protected $supportedOptions = array(
         'element' => array('', 'The name of the element', 'string', true),
         'errorMessage' => array('', 'The error message', 'array', true),
-        'array.' => array('', 'The array value', 'array', true),
+        'array' => array('', 'The array value', 'array', true),
         'strict' => array('', 'Compare types', 'boolean', false),
+        'ignorecase' => array('', 'Ignore cases', 'boolean', false)
     );
 
     /**
+     * @var CharsetConverter
+     */
+    protected $charsetConverter;
+
+    /**
+     * constructor
+     *
+     * creates charsetConverter object if option ignorecase is set
+     *
+     * @param array $options
+     * @throws \TYPO3\CMS\Extbase\Validation\Exception\InvalidValidationOptionsException
+     */
+    public function __construct(array $options)
+    {
+        parent::__construct($options);
+        if (!empty($this->options['ignorecase'])) {
+            $this->charsetConverter = GeneralUtility::makeInstance(CharsetConverter::class);
+        }
+    }
+
+    /**
      * Constant for localisation
      *
      * @var string
@@ -42,7 +67,18 @@ class InArrayValidator extends AbstractValidator
      */
     public function isValid($value)
     {
-        if (!in_array($value, (array)$this->options['array.'], !empty($this->options['strict']))) {
+        if (empty($value) || !is_string($value)) {
+            return;
+        }
+        $allowedOptionsArray = GeneralUtility::trimExplode(',', $this->options['array'], true);
+        if (!empty($this->options['ignorecase'])) {
+            $value = $this->charsetConverter->conv_case('utf-8', $value, 'toLower');
+            foreach ($allowedOptionsArray as &$option) {
+                $option = $this->charsetConverter->conv_case('utf-8', $option, 'toLower');
+            }
+        }
+
+        if (!in_array($value, $allowedOptionsArray, !empty($this->options['strict']))) {
             $this->addError(
                 $this->renderMessage(
                     $this->options['errorMessage'][0],
index 213ff74..57f5491 100644 (file)
@@ -15,6 +15,7 @@ namespace TYPO3\CMS\Form\Tests\Unit\Validator;
  */
 
 use TYPO3\CMS\Form\Domain\Validator\AbstractValidator;
+use TYPO3\CMS\Form\Utility\FormUtility;
 
 /**
  * Test case
@@ -35,8 +36,12 @@ abstract class AbstractValidatorTest extends \TYPO3\CMS\Core\Tests\UnitTestCase
      */
     protected function createSubject(array $options)
     {
-        $subject = $this->getAccessibleMock($this->subjectClassName, array('renderMessage'), array($options));
-        $subject->method('renderMessage')->will($this->returnValue('error'));
+        /** @var AbstractValidator $subject */
+        $subject = $this->getMock($this->subjectClassName, ['getLocalLanguageLabel'], ['options' => $options]);
+
+        /** @var FormUtility $formUtilityMock */
+        $formUtilityMock = $this->getMock(FormUtility::class, array(), array(), '', false);
+        $subject->setFormUtility($formUtilityMock);
         return $subject;
     }
 }
index ebccb3a..5e41f9b 100644 (file)
@@ -13,96 +13,318 @@ namespace TYPO3\CMS\Form\Tests\Unit\Validator;
  *
  * The TYPO3 project - inspiring people to share!
  */
+use TYPO3\CMS\Form\Domain\Validator\InArrayValidator;
 
 /**
  * Test case
  */
 class InArrayValidatorTest extends AbstractValidatorTest
 {
+
     /**
      * @var string
      */
-    protected $subjectClassName = \TYPO3\CMS\Form\Domain\Validator\InArrayValidator::class;
+    protected $subjectClassName = InArrayValidator::class;
 
     /**
+     * used for tests with valid input
+     * will result in no errors returned
+     *
      * @return array
      */
     public function validArrayProvider()
     {
-        return array(
-            '12 in (12, 13)' => array(array(array(12, 13), 12))
-        );
+        return [
+            '12 in (12, 13)' => [
+                '12',
+                '12,13',
+            ],
+            '1 in (5, 3234, oOIUoi8, 3434, 343, 34, 3, 1, 333434, 1234, ssd, ysdfsa)' => [
+                '1',
+                '5,3234,oOIUoi8,3434,343,34,3,1,333434,1234,ssd,ysdfsa',
+            ],
+            'Rißtissen in (Rißtissen, Überligen, Karlsruhe)' => [
+                'Rißtissen',
+                'Rißtissen,Überligen,Karlsruhe',
+            ],
+            'Pizza in (Pizza, Lasange, Strogonvo)' => [
+                'Pizza',
+                'Pizza,Lasange,Strogonvo',
+            ],
+        ];
     }
 
     /**
+     * used for test with invalid input
+     * will result in errors returned
+     *
      * @return array
      */
     public function invalidArrayProvider()
     {
-        return array(
-            '12 in (11, 13)' => array(array(array(11, 13), 12)),
-        );
+        return [
+            '12 in (11, 13)' => [
+                '12',
+                '11,13',
+            ],
+            '1 in (5, 3234, oOIUoi8, 3434, 343, 34, 3, 333434, 1234, ssd, ysdfsa)' => [
+                '1',
+                '5,3234,oOIUoi8,3434,343,34,3,333434,1234,ssd,ysdfsa',
+            ],
+            'Eimeldingen in (Rißtissen, Überligen, Karlsruhe)' => [
+                'Eimeldingen',
+                'Rißtissen,Überligen,Karlsruhe',
+            ],
+            'pizza in (Pizza, Lasange, Strogonvo)' => [
+                'pizza',
+                'Pizza,Lasange,Strogonvo',
+            ],
+        ];
+    }
+
+    /**
+     * used for tests with valid input
+     * ignorecase is set to true
+     * results in no errors returned
+     *
+     * @return array
+     */
+    public function validArrayIgnoreCaseProvider()
+    {
+        return [
+            '12 in (12, 13)' => [
+                '12',
+                '12,13',
+            ],
+            '1 in (5, 3234, oOIUoi8, 3434, 343, 34, 3, 1, 333434, 1234, ssd, ysdfsa)' => [
+                '1',
+                '5,3234,oOIUoi8,3434,343,34,3,1,333434,1234,ssd,ysdfsa',
+            ],
+            'Rißtissen in (Rißtissen, Überligen, Karlsruhe)' => [
+                'Rißtissen',
+                'Rißtissen,Überligen,Karlsruhe',
+            ],
+            'überlingen in (Rißtissen, Überligen, Karlsruhe)' => [
+                'überlingen',
+                'Rißtissen,Überlingen,Karlsruhe',
+            ],
+            'Österreich in (österreich, deutschland, schweiz)' => [
+                'Österreich',
+                'österreich,deutschland,schweiz',
+            ],
+            'pizza in (Pizza, Lasange, Strogonvo)' => [
+                'pizza',
+                'Pizza,Lasange,Strogonvo',
+            ],
+            'lasange in (Pizza, Lasange, Strogonvo)' => [
+                'lasange',
+                'Pizza,Lasange,Strogonvo',
+            ],
+        ];
+    }
+
+    /**
+     * used for tests with invalid input
+     * ignorecase is set to true
+     * results in errors returned
+     *
+     * @return array
+     */
+    public function invalidArrayIgnoreCaseProvider()
+    {
+        return [
+            'zwölf in (12, 13)' => [
+                'zwölf',
+                '12,13',
+            ],
+            '7 in (5, 3234, oOIUoi8, 3434, 343, 34, 3, 1, 333434, 1234, ssd, ysdfsa)' => [
+                '7',
+                '5,3234,oOIUoi8,3434,343,34,3,1,333434,1234,ssd,ysdfsa',
+            ],
+            'riss in (Rißtissen, Überligen, Karlsruhe)' => [
+                'riss',
+                'Rißtissen,Überligen,Karlsruhe',
+            ],
+            'pizzas in (Pizza, Lasange, Strogonvo)' => [
+                'pizzas',
+                'Pizza,Lasange,Strogonvo',
+            ],
+            'lusange in (Pizza, Lasange, Strogonvo)' => [
+                'lusange',
+                'Pizza,Lasange,Strogonvo',
+            ],
+        ];
     }
 
     /**
      * @test
      * @dataProvider validArrayProvider
+     *
+     * @param string $value
+     * @param string $allowedOptionsString
      */
-    public function validateForValidInputHasEmptyErrorResult($input)
+    public function validateForValidInputReturnsNoErrors($value, $allowedOptionsString)
     {
-        $options = array('element' => uniqid('test'), 'errorMessage' => uniqid('error'));
-        $options['array.'] = $input[0];
+        $options = [
+            'element' => uniqid('test'),
+            'errorMessage' => uniqid('error'),
+        ];
+        $options['array'] = $allowedOptionsString;
         $subject = $this->createSubject($options);
 
-        $this->assertEmpty(
-            $subject->validate($input[1])->getErrors()
-        );
+        $this->assertFalse($subject->validate($value)->hasErrors());
     }
 
     /**
      * @test
      * @dataProvider invalidArrayProvider
+     *
+     * @param string $value
+     * @param string $allowedOptionsString
      */
-    public function validateForInvalidInputHasNotEmptyErrorResult($input)
+    public function validateForInvalidInputReturnsErrors($value, $allowedOptionsString)
     {
-        $options = array('element' => uniqid('test'), 'errorMessage' => uniqid('error'));
-        $options['array.'] = $input[0];
+        $options = [
+            'element' => uniqid('test'),
+            'errorMessage' => uniqid('error'),
+        ];
+        $options['array'] = $allowedOptionsString;
         $subject = $this->createSubject($options);
 
-        $this->assertNotEmpty(
-            $subject->validate($input[1])->getErrors()
-        );
+        $this->assertTrue($subject->validate($value)->hasErrors());
     }
 
     /**
      * @test
      * @dataProvider validArrayProvider
+     *
+     * @param string $value
+     * @param string $allowedOptionsString
      */
-    public function validateForValidInputWithStrictComparisonHasEmptyErrorResult($input)
-    {
-        $options = array('element' => uniqid('test'), 'errorMessage' => uniqid('error'));
-        $options['array.'] = $input[0];
+    public function validateForValidInputWithStrictComparisonReturnsNoErrors(
+        $value,
+        $allowedOptionsString
+    ) {
+        $options = [
+            'element' => uniqid('test'),
+            'errorMessage' => uniqid('error'),
+        ];
+        $options['array'] = $allowedOptionsString;
         $options['strict'] = true;
         $subject = $this->createSubject($options);
 
-        $this->assertEmpty(
-            $subject->validate($input[1])->getErrors()
-        );
+        $this->assertFalse($subject->validate($value)->hasErrors());
     }
 
     /**
      * @test
      * @dataProvider invalidArrayProvider
+     *
+     * @param string $value
+     * @param string $allowedOptionsString
      */
-    public function validateForInvalidInputWithStrictComparisonHasNotEmptyErrorResult($input)
-    {
-        $options = array('element' => uniqid('test'), 'errorMessage' => uniqid('error'));
-        $options['array.'] = $input[0];
+    public function validateForInvalidInputWithStrictComparisonReturnsErrors(
+        $value,
+        $allowedOptionsString
+    ) {
+        $options = [
+            'element' => uniqid('test'),
+            'errorMessage' => uniqid('error'),
+        ];
+        $options['array'] = $allowedOptionsString;
+        $options['strict'] = true;
+        $subject = $this->createSubject($options);
+
+        $this->assertTrue($subject->validate($value)->hasErrors());
+    }
+
+    /**
+     * @test
+     * @dataProvider validArrayIgnoreCaseProvider
+     *
+     * @param string $value
+     * @param string $allowedOptionsString
+     */
+    public function validateForValidInputWithIgnoreCaseReturnsNoErrors(
+        $value,
+        $allowedOptionsString
+    ) {
+        $options = [
+            'element' => uniqid('test'),
+            'errorMessage' => uniqid('error'),
+        ];
+        $options['array'] = $allowedOptionsString;
+        $options['ignorecase'] = true;
+        $subject = $this->createSubject($options);
+
+        $this->assertFalse($subject->validate($value)->hasErrors());
+    }
+
+    /**
+     * @test
+     * @dataProvider invalidArrayIgnoreCaseProvider
+     *
+     * @param string $value
+     * @param string $allowedOptionsString
+     */
+    public function validateForInvalidInputWithIgnoreCaseReturnsErrors(
+        $value,
+        $allowedOptionsString
+    ) {
+        $options = [
+            'element' => uniqid('test'),
+            'errorMessage' => uniqid('error'),
+        ];
+        $options['array'] = $allowedOptionsString;
+        $options['ignorecase'] = true;
+        $subject = $this->createSubject($options);
+
+        $this->assertTrue($subject->validate($value)->hasErrors());
+    }
+
+    /**
+     * @test
+     * @dataProvider validArrayIgnoreCaseProvider
+     *
+     * @param string $value
+     * @param string $allowedOptionsString
+     */
+    public function validateForValidInputWithIgnoreCaseAndStrictReturnsNoErrors(
+        $value,
+        $allowedOptionsString
+    ) {
+        $options = [
+            'element' => uniqid('test'),
+            'errorMessage' => uniqid('error'),
+        ];
+        $options['array'] = $allowedOptionsString;
+        $options['ignorecase'] = true;
+        $options['strict'] = true;
+        $subject = $this->createSubject($options);
+
+        $this->assertFalse($subject->validate($value)->hasErrors());
+    }
+
+    /**
+     * @test
+     * @dataProvider invalidArrayIgnoreCaseProvider
+     *
+     * @param string $value
+     * @param string $allowedOptionsString
+     */
+    public function validateForInvalidInputWithIgnoreCaseAndStrictReturnsErrors(
+        $value,
+        $allowedOptionsString
+    ) {
+        $options = [
+            'element' => uniqid('test'),
+            'errorMessage' => uniqid('error'),
+        ];
+        $options['array'] = $allowedOptionsString;
+        $options['ignorecase'] = true;
         $options['strict'] = true;
         $subject = $this->createSubject($options);
 
-        $this->assertNotEmpty(
-            $subject->validate($input[1])->getErrors()
-        );
+        $this->assertTrue($subject->validate($value)->hasErrors());
     }
 }
index 4725b16..35013a4 100644 (file)
@@ -16,6 +16,7 @@ namespace TYPO3\CMS\Form\Tests\Unit\Validator;
 
 use Prophecy\Argument;
 use TYPO3\CMS\Core\Charset\CharsetConverter;
+use TYPO3\CMS\Form\Domain\Validator\LengthValidator;
 
 /**
  * Test case
@@ -82,8 +83,9 @@ class LengthValidatorTest extends AbstractValidatorTest
         $options = array('element' => uniqid('test'), 'errorMessage' => uniqid('error'));
         $options['minimum'] = $input[0];
         $options['maximum'] = $input[1];
+        /** @var LengthValidator $subject */
         $subject = $this->createSubject($options);
-        $subject->_set('charsetConverter', $this->charsetConverterProphecy->reveal());
+        $subject->injectCharsetConverter($this->charsetConverterProphecy->reveal());
 
         $this->assertEmpty(
             $subject->validate($input[2])->getErrors()
@@ -114,8 +116,9 @@ class LengthValidatorTest extends AbstractValidatorTest
         $options = array('element' => uniqid('test'), 'errorMessage' => uniqid('error'));
         $options['minimum'] = $input[0];
         $options['maximum'] = $input[1];
+        /** @var LengthValidator $subject */
         $subject = $this->createSubject($options);
-        $subject->_set('charsetConverter', $this->charsetConverterProphecy->reveal());
+        $subject->injectCharsetConverter($this->charsetConverterProphecy->reveal());
 
         $this->assertNotEmpty(
             $subject->validate($input[2])->getErrors()