From 45f969cf59b2b1987fcfb35a88936970e3a59fb4 Mon Sep 17 00:00:00 2001 From: Christian Benthake <typo3@cben.co> Date: Fri, 2 Aug 2019 20:27:32 +0000 Subject: [PATCH] [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> --- .../Property/TypeConverter/ArrayConverter.php | 33 +++++-- .../TypeConverter/ArrayConverterTest.php | 87 +++++++++++++++++-- 2 files changed, 105 insertions(+), 15 deletions(-) diff --git a/typo3/sysext/extbase/Classes/Property/TypeConverter/ArrayConverter.php b/typo3/sysext/extbase/Classes/Property/TypeConverter/ArrayConverter.php index 9a09a50b5bed..7b96ca7a4261 100644 --- a/typo3/sysext/extbase/Classes/Property/TypeConverter/ArrayConverter.php +++ b/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; } diff --git a/typo3/sysext/extbase/Tests/Unit/Property/TypeConverter/ArrayConverterTest.php b/typo3/sysext/extbase/Tests/Unit/Property/TypeConverter/ArrayConverterTest.php index 64e75e92440f..93d30dc8cbaa 100644 --- a/typo3/sysext/extbase/Tests/Unit/Property/TypeConverter/ArrayConverterTest.php +++ b/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); + } } -- GitLab