[BUGFIX] ArrayConverter can now convert all types of strings

The ArrayConverter has been extended to be able to handle more
string formats. Previously, it was only able to handle empty strings.

Resolves: #88389
Releases: master
Change-Id: I418072cdb32118cfe4e81ff01930cabbc86fee16
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/61410


Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Daniel Haupt <mail@danielhaupt.de>
Tested-by: Alexander Schnitzler <git@alexanderschnitzler.de>
Reviewed-by: Daniel Haupt <mail@danielhaupt.de>
Reviewed-by: Alexander Schnitzler <git@alexanderschnitzler.de>

typo3/sysext/extbase/Classes/Property/TypeConverter/ArrayConverter.php

@@ -16,11 +16,18 @@ namespace TYPO3\CMS\Extbase\Property\TypeConverter;
  * The TYPO3 project - inspiring people to share!
  */
 
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Extbase\Property\Exception\TypeConverterException;
+
 /**
- * Converter which transforms arrays to arrays.
+ * Converter which transforms strings/arrays to arrays.
  */
 class ArrayConverter extends \TYPO3\CMS\Extbase\Property\TypeConverter\AbstractTypeConverter
 {
+    public const CONFIGURATION_DELIMITER = 'delimiter';
+    public const CONFIGURATION_REMOVE_EMPTY_VALUES = 'removeEmptyValues';
+    public const CONFIGURATION_LIMIT = 'limit';
+
     /**
      * @var string[]
      */
@@ -46,26 +53,38 @@ class ArrayConverter extends \TYPO3\CMS\Extbase\Property\TypeConverter\AbstractT
      */
     public function canConvertFrom($source, string $targetType): bool
     {
-        return (is_string($source) && $source === '') || is_array($source);
+        return is_string($source) || is_array($source);
     }
 
     /**
      * Convert from $source to $targetType, a noop if the source is an array.
      * If it is an empty string it will be converted to an empty array.
+     * If the type converter has a configuration, it can convert non-empty strings, too
      *
      * @param string|array $source
      * @param string $targetType
      * @param array $convertedChildProperties
      * @param \TYPO3\CMS\Extbase\Property\PropertyMappingConfigurationInterface $configuration
-     * @return array
+     * @return array|string
      */
     public function convertFrom($source, string $targetType, array $convertedChildProperties = [], \TYPO3\CMS\Extbase\Property\PropertyMappingConfigurationInterface $configuration = null)
     {
-        if (is_string($source)) {
-            if ($source === '') {
-                $source = [];
-            }
+        if (!is_string($source)) {
+            return $source;
+        }
+        if ($source === '') {
+            return [];
+        }
+        if ($configuration === null) {
+            return $source;
+        }
+        $delimiter = $configuration->getConfigurationValue(self::class, self::CONFIGURATION_DELIMITER);
+        $removeEmptyValues = $configuration->getConfigurationValue(self::class, self::CONFIGURATION_REMOVE_EMPTY_VALUES) ?? false;
+        $limit = $configuration->getConfigurationValue(self::class, self::CONFIGURATION_LIMIT) ?? 0;
+        if (!is_string($delimiter)) {
+            throw new TypeConverterException('No delimiter configured for ' . self::class . ' and non-empty value given.', 1582877555);
         }
+        $source = GeneralUtility::trimExplode($delimiter, $source, $removeEmptyValues, $limit);
 
         return $source;
     }

typo3/sysext/extbase/Tests/Unit/Property/TypeConverter/ArrayConverterTest.php

@@ -14,6 +14,10 @@ namespace TYPO3\CMS\Extbase\Tests\Unit\Property\TypeConverter;
  * The TYPO3 project - inspiring people to share!
  */
 
+use TYPO3\CMS\Extbase\Property\Exception\TypeConverterException;
+use TYPO3\CMS\Extbase\Property\PropertyMappingConfiguration;
+use TYPO3\CMS\Extbase\Property\PropertyMappingConfigurationInterface;
+use TYPO3\CMS\Extbase\Property\TypeConverter\ArrayConverter;
 use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
 
 /**
@@ -29,13 +33,13 @@ class ArrayConverterTest extends UnitTestCase
     protected function setUp(): void
     {
         parent::setUp();
-        $this->converter = new \TYPO3\CMS\Extbase\Property\TypeConverter\ArrayConverter();
+        $this->converter = new ArrayConverter();
     }
 
     /**
      * @test
      */
-    public function checkMetadata()
+    public function checkMetadata(): void
     {
         self::assertEquals(['array', 'string'], $this->converter->getSupportedSourceTypes(), 'Source types do not match');
         self::assertEquals('array', $this->converter->getSupportedTargetType(), 'Target type does not match');
@@ -45,7 +49,7 @@ class ArrayConverterTest extends UnitTestCase
     /**
      * @test
      */
-    public function convertFromDoesNotModifyTheSourceArray()
+    public function convertFromDoesNotModifyTheSourceArray(): void
     {
         $sourceArray = ['Foo' => 'Bar', 'Baz'];
         self::assertEquals($sourceArray, $this->converter->convertFrom($sourceArray, 'array'));
@@ -54,7 +58,7 @@ class ArrayConverterTest extends UnitTestCase
     /**
      * @return array
      */
-    public function stringToArrayDataProvider()
+    public function stringToArrayDataProvider(): array
     {
         return [
             'Empty string to empty array' => ['', []],
@@ -68,7 +72,7 @@ class ArrayConverterTest extends UnitTestCase
      * @param string $source
      * @param array $expectedResult
      */
-    public function canConvertFromEmptyString($source, $expectedResult)
+    public function canConvertFromEmptyString($source, $expectedResult): void
     {
         self::assertEquals($expectedResult, $this->converter->convertFrom($source, 'array'));
     }
@@ -76,11 +80,59 @@ class ArrayConverterTest extends UnitTestCase
     /**
      * @return array
      */
-    public function canConvertFromDataProvider()
+    public function stringToArrayWithConfigurationDataProvider(): array
+    {
+        return [
+            'Sentence with two spaces string converts to array, delimiter is space' => ['foo  bar', $this->getStringConf(' ', true), ['foo', 'bar']],
+            'String List with comma converts to array, delimiter: ,' => ['foo,bar', $this->getStringConf(','), ['foo', 'bar']],
+            'String List with comma converts to array; no empty: true, delimiter: ,' => ['foo,,bar', $this->getStringConf(',', true), ['foo', 'bar']],
+            'Long sentence splits into two parts; delimiter: space' => ['Foo bar is a long sentence', $this->getStringConf(' ', false, 2), ['Foo', 'bar is a long sentence']],
+            'All available configuration options' => ['This. Is. Awesome... Right?', $this->getStringConf('.', true, 4), ['This', 'Is', 'Awesome', 'Right?']]
+        ];
+    }
+
+    private function getStringConf(?string $delimiter = null, ?bool $removeEmptyValues = null, ?int $limit = null): PropertyMappingConfigurationInterface
+    {
+        $configuration = new PropertyMappingConfiguration();
+        if ($delimiter) {
+            $configuration->setTypeConverterOption(ArrayConverter::class, 'delimiter', $delimiter);
+        }
+        if ($removeEmptyValues) {
+            $configuration->setTypeConverterOption(ArrayConverter::class, 'removeEmptyValues', $removeEmptyValues);
+        }
+        if ($limit) {
+            $configuration->setTypeConverterOption(ArrayConverter::class, 'limit', $limit);
+        }
+        $configuration->setTypeConverterOptions(ArrayConverter::class, [
+            'delimiter' => $delimiter,
+            'removeEmptyValues' => $removeEmptyValues,
+            'limit' => $limit,
+        ]);
+        return $configuration;
+    }
+
+    /**
+     * @test
+     * @dataProvider stringToArrayWithConfigurationDataProvider
+     *
+     * @param string $source
+     * @param PropertyMappingConfigurationInterface $configuration
+     * @param array $expectedResult
+     */
+    public function canConvertWithConfigurationFromString($source, PropertyMappingConfigurationInterface $configuration, $expectedResult): void
+    {
+        self::assertEquals($expectedResult, $this->converter->convertFrom($source, 'array', [], $configuration));
+    }
+
+    /**
+     * @return array
+     */
+    public function canConvertFromDataProvider(): array
     {
         return [
             'Can convert empty string' => ['', true],
-            'Can not convert not empty string' => ['foo', false],
+            'Can convert not empty string' => ['foo', true],
+            'Can not convert number' => [12, false],
             'Can convert array' => [['foo'], true],
         ];
     }
@@ -92,8 +144,27 @@ class ArrayConverterTest extends UnitTestCase
      * @param mixed $source
      * @param bool $expectedResult
      */
-    public function canConvertFromReturnsCorrectBooleans($source, $expectedResult)
+    public function canConvertFromReturnsCorrectBooleans($source, $expectedResult): void
     {
         self::assertSame($expectedResult, $this->converter->canConvertFrom($source, 'array'));
     }
+
+    /**
+     * @test
+     */
+    public function throwsTypeConverterExceptionIfDelimiterIsNotGiven(): void
+    {
+        $this->expectException(TypeConverterException::class);
+        $this->expectExceptionCode(1582877555);
+        $this->converter->convertFrom('foo', 'array', [], new PropertyMappingConfiguration());
+    }
+
+    /**
+     * @test
+     */
+    public function returnsSourceUnchangedIfNonEmptyValueWithNoConfigurationIsGiven(): void
+    {
+        $result = $this->converter->convertFrom('foo', 'array', [], null);
+        self::assertSame('foo', $result);
+    }
 }