From 2998c43ba824492bbb888e6b99c1c0f349c7e5ad Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 1 Mar 2018 08:03:51 +0100 Subject: [PATCH] TASK: Refactor data processing Use own service to handle data processing. Classes like indexer should not know anything about the structure and how to process the data. Also rename record to data, as we can process just any data in form of an array. Relates: #116 --- Classes/DataProcessing/CopyToProcessor.php | 2 +- Classes/DataProcessing/GeoPointProcessor.php | 2 +- Classes/DataProcessing/ProcessorInterface.php | 7 +-- Classes/DataProcessing/RemoveProcessor.php | 2 +- Classes/DataProcessing/Service.php | 56 +++++++++++++++++++ Classes/Domain/Index/AbstractIndexer.php | 18 +++--- .../DataProcessing/CopyToProcessorTest.php | 16 +++--- .../DataProcessing/GeoPointProcessorTest.php | 20 +++---- .../DataProcessing/RemoveProcessorTest.php | 22 ++++---- 9 files changed, 98 insertions(+), 47 deletions(-) create mode 100644 Classes/DataProcessing/Service.php diff --git a/Classes/DataProcessing/CopyToProcessor.php b/Classes/DataProcessing/CopyToProcessor.php index eea555f..28c4294 100644 --- a/Classes/DataProcessing/CopyToProcessor.php +++ b/Classes/DataProcessing/CopyToProcessor.php @@ -25,7 +25,7 @@ namespace Codappix\SearchCore\DataProcessing; */ class CopyToProcessor implements ProcessorInterface { - public function processRecord(array $record, array $configuration) : array + public function processData(array $record, array $configuration) : array { $all = []; diff --git a/Classes/DataProcessing/GeoPointProcessor.php b/Classes/DataProcessing/GeoPointProcessor.php index f9256b3..c5b6d18 100644 --- a/Classes/DataProcessing/GeoPointProcessor.php +++ b/Classes/DataProcessing/GeoPointProcessor.php @@ -25,7 +25,7 @@ namespace Codappix\SearchCore\DataProcessing; */ class GeoPointProcessor implements ProcessorInterface { - public function processRecord(array $record, array $configuration) : array + public function processData(array $record, array $configuration) : array { if (! $this->canApply($record, $configuration)) { return $record; diff --git a/Classes/DataProcessing/ProcessorInterface.php b/Classes/DataProcessing/ProcessorInterface.php index 4dc0794..f7513f2 100644 --- a/Classes/DataProcessing/ProcessorInterface.php +++ b/Classes/DataProcessing/ProcessorInterface.php @@ -21,14 +21,13 @@ namespace Codappix\SearchCore\DataProcessing; */ /** - * All DataProcessing Processors should implement this interface, otherwise they - * will not be executed. + * All DataProcessing Processors should implement this interface. */ interface ProcessorInterface { /** - * Processes the given record. + * Processes the given data. * Also retrieves the configuration for this processor instance. */ - public function processRecord(array $record, array $configuration) : array; + public function processData(array $record, array $configuration) : array; } diff --git a/Classes/DataProcessing/RemoveProcessor.php b/Classes/DataProcessing/RemoveProcessor.php index 6e74237..b8d6283 100644 --- a/Classes/DataProcessing/RemoveProcessor.php +++ b/Classes/DataProcessing/RemoveProcessor.php @@ -27,7 +27,7 @@ use TYPO3\CMS\Core\Utility\GeneralUtility; */ class RemoveProcessor implements ProcessorInterface { - public function processRecord(array $record, array $configuration) : array + public function processData(array $record, array $configuration) : array { if (!isset($configuration['fields'])) { return $record; diff --git a/Classes/DataProcessing/Service.php b/Classes/DataProcessing/Service.php new file mode 100644 index 0000000..3e3b053 --- /dev/null +++ b/Classes/DataProcessing/Service.php @@ -0,0 +1,56 @@ + + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +use TYPO3\CMS\Extbase\Object\ObjectManagerInterface; + +/** + * Eases work with data processing. + */ +class Service +{ + /** + * @var ObjectManagerInterface + */ + protected $objectManager; + + public function __construct(ObjectManagerInterface $objectManager) + { + $this->objectManager = $objectManager; + } + + /** + * Executes the dataprocessor depending on configuration and returns the result. + * + * @param array|string $configuration Either the full configuration or only the class name. + */ + public function executeDataProcessor($configuration, array $data) : array + { + if (is_string($configuration)) { + $configuration = [ + '_typoScriptNodeValue' => $configuration, + ]; + } + + return $this->objectManager->get($configuration['_typoScriptNodeValue']) + ->processData($data, $configuration); + } +} diff --git a/Classes/Domain/Index/AbstractIndexer.php b/Classes/Domain/Index/AbstractIndexer.php index bc8674e..2de7dd5 100644 --- a/Classes/Domain/Index/AbstractIndexer.php +++ b/Classes/Domain/Index/AbstractIndexer.php @@ -43,6 +43,12 @@ abstract class AbstractIndexer implements IndexerInterface */ protected $identifier = ''; + /** + * @var \Codappix\SearchCore\DataProcessing\Service + * @inject + */ + protected $dataProcessorService; + /** * @var \TYPO3\CMS\Core\Log\Logger */ @@ -134,17 +140,7 @@ abstract class AbstractIndexer implements IndexerInterface { try { foreach ($this->configuration->get('indexing.' . $this->identifier . '.dataProcessing') as $configuration) { - $className = ''; - if (is_string($configuration)) { - $className = $configuration; - $configuration = []; - } else { - $className = $configuration['_typoScriptNodeValue']; - } - $dataProcessor = GeneralUtility::makeInstance($className); - if ($dataProcessor instanceof ProcessorInterface) { - $record = $dataProcessor->processRecord($record, $configuration); - } + $record = $this->dataProcessorService->executeDataProcessor($configuration, $record); } } catch (InvalidArgumentException $e) { // Nothing to do. diff --git a/Tests/Unit/DataProcessing/CopyToProcessorTest.php b/Tests/Unit/DataProcessing/CopyToProcessorTest.php index 2f9e498..28545a3 100644 --- a/Tests/Unit/DataProcessing/CopyToProcessorTest.php +++ b/Tests/Unit/DataProcessing/CopyToProcessorTest.php @@ -27,15 +27,15 @@ class CopyToProcessorTest extends AbstractUnitTestCase { /** * @test - * @dataProvider getPossibleRecordConfigurationCombinations + * @dataProvider getPossibleDataConfigurationCombinations */ - public function fieldsAreCopiedAsConfigured(array $record, array $configuration, array $expectedRecord) + public function fieldsAreCopiedAsConfigured(array $record, array $configuration, array $expectedData) { $subject = new CopyToProcessor(); - $processedRecord = $subject->processRecord($record, $configuration); + $processedData = $subject->processData($record, $configuration); $this->assertSame( - $expectedRecord, - $processedRecord, + $expectedData, + $processedData, 'The processor did not return the expected processed record.' ); } @@ -43,7 +43,7 @@ class CopyToProcessorTest extends AbstractUnitTestCase /** * @return array */ - public function getPossibleRecordConfigurationCombinations() + public function getPossibleDataConfigurationCombinations() { return [ 'Copy all fields to new field' => [ @@ -54,7 +54,7 @@ class CopyToProcessorTest extends AbstractUnitTestCase 'configuration' => [ 'to' => 'new_field', ], - 'expectedRecord' => [ + 'expectedData' => [ 'field 1' => 'Some content like lorem', 'field 2' => 'Some more content like ipsum', 'new_field' => 'Some content like lorem' . PHP_EOL . 'Some more content like ipsum', @@ -71,7 +71,7 @@ class CopyToProcessorTest extends AbstractUnitTestCase 'configuration' => [ 'to' => 'new_field', ], - 'expectedRecord' => [ + 'expectedData' => [ 'field 1' => 'Some content like lorem', 'field with sub2' => [ 'Tag 1', diff --git a/Tests/Unit/DataProcessing/GeoPointProcessorTest.php b/Tests/Unit/DataProcessing/GeoPointProcessorTest.php index 99b8eb3..02db659 100644 --- a/Tests/Unit/DataProcessing/GeoPointProcessorTest.php +++ b/Tests/Unit/DataProcessing/GeoPointProcessorTest.php @@ -27,15 +27,15 @@ class GeoPointProcessorTest extends AbstractUnitTestCase { /** * @test - * @dataProvider getPossibleRecordConfigurationCombinations + * @dataProvider getPossibleDataConfigurationCombinations */ - public function geoPointsAreAddedAsConfigured(array $record, array $configuration, array $expectedRecord) + public function geoPointsAreAddedAsConfigured(array $record, array $configuration, array $expectedData) { $subject = new GeoPointProcessor(); - $processedRecord = $subject->processRecord($record, $configuration); + $processedData = $subject->processData($record, $configuration); $this->assertSame( - $expectedRecord, - $processedRecord, + $expectedData, + $processedData, 'The processor did not return the expected processed record.' ); } @@ -43,7 +43,7 @@ class GeoPointProcessorTest extends AbstractUnitTestCase /** * @return array */ - public function getPossibleRecordConfigurationCombinations() + public function getPossibleDataConfigurationCombinations() { return [ 'Create new field with existing lat and lng' => [ @@ -56,7 +56,7 @@ class GeoPointProcessorTest extends AbstractUnitTestCase 'lat' => 'lat', 'lon' => 'lng', ], - 'expectedRecord' => [ + 'expectedData' => [ 'lat' => 23.232, 'lng' => 45.43, 'location' => [ @@ -73,7 +73,7 @@ class GeoPointProcessorTest extends AbstractUnitTestCase 'configuration' => [ 'to' => 'location', ], - 'expectedRecord' => [ + 'expectedData' => [ 'lat' => 23.232, 'lng' => 45.43, ], @@ -88,7 +88,7 @@ class GeoPointProcessorTest extends AbstractUnitTestCase 'lat' => 'lat', 'lon' => 'lng', ], - 'expectedRecord' => [ + 'expectedData' => [ 'lat' => '', 'lng' => '', ], @@ -103,7 +103,7 @@ class GeoPointProcessorTest extends AbstractUnitTestCase 'lat' => 'lat', 'lon' => 'lng', ], - 'expectedRecord' => [ + 'expectedData' => [ 'lat' => 'av', 'lng' => 'dsf', ], diff --git a/Tests/Unit/DataProcessing/RemoveProcessorTest.php b/Tests/Unit/DataProcessing/RemoveProcessorTest.php index dc55f73..cd23d16 100644 --- a/Tests/Unit/DataProcessing/RemoveProcessorTest.php +++ b/Tests/Unit/DataProcessing/RemoveProcessorTest.php @@ -27,15 +27,15 @@ class RemoveProcessorTest extends AbstractUnitTestCase { /** * @test - * @dataProvider getPossibleRecordConfigurationCombinations + * @dataProvider getPossibleDataConfigurationCombinations */ - public function fieldsAreCopiedAsConfigured(array $record, array $configuration, array $expectedRecord) + public function fieldsAreCopiedAsConfigured(array $record, array $configuration, array $expectedData) { $subject = new RemoveProcessor(); - $processedRecord = $subject->processRecord($record, $configuration); + $processedData = $subject->processData($record, $configuration); $this->assertSame( - $expectedRecord, - $processedRecord, + $expectedData, + $processedData, 'The processor did not return the expected processed record.' ); } @@ -43,7 +43,7 @@ class RemoveProcessorTest extends AbstractUnitTestCase /** * @return array */ - public function getPossibleRecordConfigurationCombinations() + public function getPossibleDataConfigurationCombinations() { return [ 'Nothing configured' => [ @@ -56,7 +56,7 @@ class RemoveProcessorTest extends AbstractUnitTestCase ], 'configuration' => [ ], - 'expectedRecord' => [ + 'expectedData' => [ 'field 1' => 'Some content like lorem', 'field with sub2' => [ 'Tag 1', @@ -76,7 +76,7 @@ class RemoveProcessorTest extends AbstractUnitTestCase 'fields' => 'field with sub2', '_typoScriptNodeValue' => 'Codappix\SearchCore\DataProcessing\RemoveProcessor', ], - 'expectedRecord' => [ + 'expectedData' => [ 'field 1' => 'Some content like lorem', ], ], @@ -92,7 +92,7 @@ class RemoveProcessorTest extends AbstractUnitTestCase 'fields' => 'non existing', '_typoScriptNodeValue' => 'Codappix\SearchCore\DataProcessing\RemoveProcessor', ], - 'expectedRecord' => [ + 'expectedData' => [ 'field 1' => 'Some content like lorem', 'field with sub2' => [ 'Tag 1', @@ -113,7 +113,7 @@ class RemoveProcessorTest extends AbstractUnitTestCase 'fields' => 'field 3, field with sub2', '_typoScriptNodeValue' => 'Codappix\SearchCore\DataProcessing\RemoveProcessor', ], - 'expectedRecord' => [ + 'expectedData' => [ 'field 1' => 'Some content like lorem', ], ], @@ -125,7 +125,7 @@ class RemoveProcessorTest extends AbstractUnitTestCase 'fields' => 'field 1', '_typoScriptNodeValue' => 'Codappix\SearchCore\DataProcessing\RemoveProcessor', ], - 'expectedRecord' => [ + 'expectedData' => [ ], ], ];