From b7b783a7fed644ac6c302cc0c8103f719b2721c3 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Sat, 11 Nov 2017 16:46:03 +0100 Subject: [PATCH 01/22] TASK: Use Code Sniffer at travis This way we need no external service. Each developer can fully run all tests and cgl on local environment. Also this integrated better into IDEs and editors. --- .travis.yml | 1 + Classes/Connection/Elasticsearch.php | 5 +++- .../Index/TcaIndexer/TcaTableService.php | 9 ++++++-- Classes/Domain/Search/QueryFactory.php | 11 +++++++-- Classes/Hook/DataHandler.php | 9 ++++++-- Makefile | 5 +++- .../ConfigurationUtilityTest.php | 7 ++++-- Tests/Unit/Domain/Search/QueryFactoryTest.php | 1 - composer.json | 11 ++++----- phpcs.xml.dist | 23 +++++++++++++++++++ 10 files changed, 64 insertions(+), 18 deletions(-) create mode 100644 phpcs.xml.dist diff --git a/.travis.yml b/.travis.yml index 4490198..0adb894 100644 --- a/.travis.yml +++ b/.travis.yml @@ -33,6 +33,7 @@ services: install: make install script: + - make cgl - make unitTests - make functionalTests diff --git a/Classes/Connection/Elasticsearch.php b/Classes/Connection/Elasticsearch.php index 8a3cb2b..b86bebf 100644 --- a/Classes/Connection/Elasticsearch.php +++ b/Classes/Connection/Elasticsearch.php @@ -132,7 +132,10 @@ class Elasticsearch implements Singleton, ConnectionInterface } ); } catch (\Elastica\Exception\NotFoundException $exception) { - $this->logger->debug('Tried to delete document in index, which does not exist.', [$documentType, $identifier]); + $this->logger->debug( + 'Tried to delete document in index, which does not exist.', + [$documentType, $identifier] + ); } } diff --git a/Classes/Domain/Index/TcaIndexer/TcaTableService.php b/Classes/Domain/Index/TcaIndexer/TcaTableService.php index 7a322a0..bf631b1 100644 --- a/Classes/Domain/Index/TcaIndexer/TcaTableService.php +++ b/Classes/Domain/Index/TcaIndexer/TcaTableService.php @@ -159,7 +159,9 @@ class TcaTableService $parameters = []; $whereClause = $this->getSystemWhereClause(); - $userDefinedWhere = $this->configuration->getIfExists('indexing.' . $this->getTableName() . '.additionalWhereClause'); + $userDefinedWhere = $this->configuration->getIfExists( + 'indexing.' . $this->getTableName() . '.additionalWhereClause' + ); if (is_string($userDefinedWhere)) { $whereClause .= ' AND ' . $userDefinedWhere; } @@ -361,6 +363,9 @@ class TcaTableService */ protected function getBlackListedRootLine() : array { - return GeneralUtility::intExplode(',', $this->configuration->getIfExists('indexing.' . $this->getTableName() . '.rootLineBlacklist')); + return GeneralUtility::intExplode( + ',', + $this->configuration->getIfExists('indexing.' . $this->getTableName() . '.rootLineBlacklist') + ); } } diff --git a/Classes/Domain/Search/QueryFactory.php b/Classes/Domain/Search/QueryFactory.php index f73372d..5879b9f 100644 --- a/Classes/Domain/Search/QueryFactory.php +++ b/Classes/Domain/Search/QueryFactory.php @@ -163,7 +163,11 @@ class QueryFactory { try { $query = ArrayUtility::arrayMergeRecursiveOverrule($query, [ - 'stored_fields' => GeneralUtility::trimExplode(',', $this->configuration->get('searching.fields.stored_fields'), true), + 'stored_fields' => GeneralUtility::trimExplode( + ',', + $this->configuration->get('searching.fields.stored_fields'), + true + ), ]); } catch (InvalidArgumentException $e) { // Nothing configured @@ -171,7 +175,10 @@ class QueryFactory try { $scriptFields = $this->configuration->get('searching.fields.script_fields'); - $scriptFields = $this->configurationUtility->replaceArrayValuesWithRequestContent($searchRequest, $scriptFields); + $scriptFields = $this->configurationUtility->replaceArrayValuesWithRequestContent( + $searchRequest, + $scriptFields + ); $scriptFields = $this->configurationUtility->filterByCondition($scriptFields); if ($scriptFields !== []) { $query = ArrayUtility::arrayMergeRecursiveOverrule($query, ['script_fields' => $scriptFields]); diff --git a/Classes/Hook/DataHandler.php b/Classes/Hook/DataHandler.php index d0eb1ba..0caac7c 100644 --- a/Classes/Hook/DataHandler.php +++ b/Classes/Hook/DataHandler.php @@ -103,8 +103,13 @@ class DataHandler implements Singleton * * @return bool False if hook was not processed. */ - public function processDatamap_afterDatabaseOperations($status, $table, $uid, array $fieldArray, CoreDataHandler $dataHandler) - { + public function processDatamap_afterDatabaseOperations( + $status, + $table, + $uid, + array $fieldArray, + CoreDataHandler $dataHandler + ) { if (! $this->shouldProcessHookForTable($table)) { $this->logger->debug('Database update not processed.', [$table, $uid]); return false; diff --git a/Makefile b/Makefile index b6bc72b..b8f4ac5 100644 --- a/Makefile +++ b/Makefile @@ -12,9 +12,12 @@ typo3DatabaseHost ?= "127.0.0.1" .PHONY: install install: clean - COMPOSER_PROCESS_TIMEOUT=1000 composer require -vv --dev --prefer-dist --ignore-platform-reqs typo3/cms="$(TYPO3_VERSION)" + COMPOSER_PROCESS_TIMEOUT=1000 composer require -vv --dev --prefer-dist typo3/cms="$(TYPO3_VERSION)" git checkout composer.json +cgl: + ./.Build/bin/phpcs + functionalTests: typo3DatabaseName=$(typo3DatabaseName) \ typo3DatabaseUsername=$(typo3DatabaseUsername) \ diff --git a/Tests/Unit/Configuration/ConfigurationUtilityTest.php b/Tests/Unit/Configuration/ConfigurationUtilityTest.php index b2dc5f8..4db367c 100644 --- a/Tests/Unit/Configuration/ConfigurationUtilityTest.php +++ b/Tests/Unit/Configuration/ConfigurationUtilityTest.php @@ -31,8 +31,11 @@ class ConfigurationUtilityTest extends AbstractUnitTestCase * @test * @dataProvider possibleRequestAndConfigurationForFluidtemplate */ - public function recursiveEntriesAreProcessedAsFluidtemplate(SearchRequestInterface $searchRequest, array $array, array $expected) - { + public function recursiveEntriesAreProcessedAsFluidtemplate( + SearchRequestInterface $searchRequest, + array $array, + array $expected + ) { $subject = new ConfigurationUtility(); $this->assertSame( diff --git a/Tests/Unit/Domain/Search/QueryFactoryTest.php b/Tests/Unit/Domain/Search/QueryFactoryTest.php index 6301cdd..218f8e5 100644 --- a/Tests/Unit/Domain/Search/QueryFactoryTest.php +++ b/Tests/Unit/Domain/Search/QueryFactoryTest.php @@ -465,7 +465,6 @@ class QueryFactoryTest extends AbstractUnitTestCase $query->toArray()['script_fields'], 'Script fields were not added to query as expected.' ); - } /** diff --git a/composer.json b/composer.json index 985be01..efa5146 100644 --- a/composer.json +++ b/composer.json @@ -16,13 +16,13 @@ } }, "require" : { - "php": ">=7.1.0", - "typo3/cms": "~8.7", + "php": ">=5.6.0", "ruflin/elastica": "~3.2" }, "require-dev": { - "typo3/testing-framework": "~1.1.0", - "phpunit/phpunit": "~6.2.0" + "phpunit/phpunit": "~5.7.0", + "squizlabs/php_codesniffer": "~3.1.1", + "typo3/cms": "~7.6" }, "config": { "optimize-autoloader": true, @@ -36,9 +36,6 @@ ] }, "extra": { - "branch-alias": { - "dev-develop": "1.0.x-dev" - }, "typo3/cms": { "cms-package-dir": "{$vendor-dir}/typo3/cms", "web-dir": ".Build/web" diff --git a/phpcs.xml.dist b/phpcs.xml.dist new file mode 100644 index 0000000..e722968 --- /dev/null +++ b/phpcs.xml.dist @@ -0,0 +1,23 @@ + + + The coding standard for search_core. + + Classes/ + Tests/ + + + + + + + + + + + + + + + Classes/Hook/DataHandler.php + + From 0bac2df6a4bb8636e15db523486818c03343dabb Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Sat, 11 Nov 2017 17:01:30 +0100 Subject: [PATCH 02/22] BUGFIX: Fix broken dependencies --- composer.json | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/composer.json b/composer.json index efa5146..2761473 100644 --- a/composer.json +++ b/composer.json @@ -16,13 +16,14 @@ } }, "require" : { - "php": ">=5.6.0", + "php": ">=7.1.0", + "typo3/cms": "~8.7", "ruflin/elastica": "~3.2" }, "require-dev": { - "phpunit/phpunit": "~5.7.0", - "squizlabs/php_codesniffer": "~3.1.1", - "typo3/cms": "~7.6" + "phpunit/phpunit": "~6.4.4", + "typo3/testing-framework": "~1.1.5", + "squizlabs/php_codesniffer": "~3.1.1" }, "config": { "optimize-autoloader": true, From 33d4116fe6a024e05fadc7cad8eae5600c11a847 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Mon, 29 Jan 2018 22:45:53 +0100 Subject: [PATCH 03/22] TASK: Fix license for packagist --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 2761473..f3bd4c3 100644 --- a/composer.json +++ b/composer.json @@ -3,7 +3,7 @@ "type": "typo3-cms-extension", "description": "Codappix Search Core.", "homepage": "https://github.com/Codappix/search_core", - "license": ["GPL-2.0+"], + "license": ["GPL-2.0-or-later"], "autoload": { "psr-4": { "Codappix\\SearchCore\\": "Classes" From 92af364b8dbde0f9e633ef1e4ebcec72639c6360 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 22 Feb 2018 20:49:25 +0100 Subject: [PATCH 04/22] FEATURE: Allow to disable elasticsearch integration This extension currently ships with Elasticsearch integration which is enabled by default. This behaviour is kept for backwards compatibility. Still you now have the possibility to disable this integration in extension manager, just check the "disable" box for elasticsearch. In the future elasticsearch will become another extension and no default is shipped with search_core. But for now, as we are still in alpha / beta phase we keep things together to keep development fast. Resolves: #111 --- Documentation/source/configuration.rst | 5 ++++- Documentation/source/installation.rst | 4 ++++ ext_conf_template.txt | 4 ++++ ext_localconf.php | 17 ++++++++++++----- 4 files changed, 24 insertions(+), 6 deletions(-) create mode 100644 ext_conf_template.txt diff --git a/Documentation/source/configuration.rst b/Documentation/source/configuration.rst index eca5ba4..09289ac 100644 --- a/Documentation/source/configuration.rst +++ b/Documentation/source/configuration.rst @@ -5,9 +5,12 @@ Configuration ============= +Installation wide configuration is handled inside of the extension manager. Just check out the +options there, they all have labels. + The extension offers the following configuration options through TypoScript. If you overwrite them through `setup` make sure to keep them in the `module` area as they will be accessed from backend -mode of TYPO3. Do so by placing the following line at the end:: +mode of TYPO3 for indexing. Do so by placing the following line at the end:: module.tx_searchcore < plugin.tx_searchcore diff --git a/Documentation/source/installation.rst b/Documentation/source/installation.rst index ddfb065..5bb0c56 100644 --- a/Documentation/source/installation.rst +++ b/Documentation/source/installation.rst @@ -19,4 +19,8 @@ In that case you need to install all dependencies yourself. Dependencies are: Afterwards you need to enable the extension through the extension manager and include the static TypoScript setup. +If you **don't** want to use the included elasticsearch integration, you have to disable it in the +extension manager configuration of the extension by checking the checkbox. +It's currently enabled by default but will be moved into its own extension in the future. + .. _downloading: https://github.com/DanielSiepmann/search_core/archive/master.zip diff --git a/ext_conf_template.txt b/ext_conf_template.txt new file mode 100644 index 0000000..5ddadb4 --- /dev/null +++ b/ext_conf_template.txt @@ -0,0 +1,4 @@ +disable { + # cat=basic/enable; type=boolean; label=Disable Elasticsearch, which is enabled by default + elasticsearch = true +} diff --git a/ext_localconf.php b/ext_localconf.php index c52d05d..a713d0b 100644 --- a/ext_localconf.php +++ b/ext_localconf.php @@ -37,11 +37,18 @@ call_user_func( ] ); - \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance('TYPO3\CMS\Extbase\Object\Container\Container') - ->registerImplementation( - 'Codappix\SearchCore\Connection\ConnectionInterface', - 'Codappix\SearchCore\Connection\Elasticsearch' - ); + // API does make use of object manager, therefore use GLOBALS + $extensionConfiguration = unserialize($GLOBALS['TYPO3_CONF_VARS']['EXT']['extConf'][$extensionKey]); + if ($extensionConfiguration === false + || !isset($extensionConfiguration['disable.']['elasticsearch']) + || $extensionConfiguration['disable.']['elasticsearch'] !== '1' + ) { + \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(\TYPO3\CMS\Extbase\Object\Container\Container::class) + ->registerImplementation( + \Codappix\SearchCore\Connection\ConnectionInterface::class, + \Codappix\SearchCore\Connection\Elasticsearch::class + ); + } }, $_EXTKEY ); From ebaeaf4c92e224618211723d80f3acd6ba73fb68 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 22 Feb 2018 21:59:13 +0100 Subject: [PATCH 05/22] TASK: Support PHP 7.0 As some (e.g. debian) do not provide PHP 7.1 and we did not use so much features which were introduced in PHP 7.1, we add support for PHP 7.0. --- .travis.yml | 1 + Classes/Domain/Index/TcaIndexer/RelationResolver.php | 2 +- Classes/Domain/Index/TcaIndexer/TcaTableService.php | 4 ++-- Makefile | 2 +- composer.json | 4 ++-- 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index 4490198..7414a0e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,6 +11,7 @@ before_install: language: php php: + - 7.0 - 7.1 env: diff --git a/Classes/Domain/Index/TcaIndexer/RelationResolver.php b/Classes/Domain/Index/TcaIndexer/RelationResolver.php index a2302ae..fb19a4f 100644 --- a/Classes/Domain/Index/TcaIndexer/RelationResolver.php +++ b/Classes/Domain/Index/TcaIndexer/RelationResolver.php @@ -33,7 +33,7 @@ use TYPO3\CMS\Core\Utility\GeneralUtility; */ class RelationResolver implements Singleton { - public function resolveRelationsForRecord(TcaTableService $service, array &$record) : void + public function resolveRelationsForRecord(TcaTableService $service, array &$record) { foreach (array_keys($record) as $column) { // TODO: Define / configure fields to exclude?! diff --git a/Classes/Domain/Index/TcaIndexer/TcaTableService.php b/Classes/Domain/Index/TcaIndexer/TcaTableService.php index 7a322a0..f059248 100644 --- a/Classes/Domain/Index/TcaIndexer/TcaTableService.php +++ b/Classes/Domain/Index/TcaIndexer/TcaTableService.php @@ -129,7 +129,7 @@ class TcaTableService * @param array &$records * @return void */ - public function filterRecordsByRootLineBlacklist(array &$records) : void + public function filterRecordsByRootLineBlacklist(array &$records) { $records = array_filter( $records, @@ -142,7 +142,7 @@ class TcaTableService /** * @param array &$record */ - public function prepareRecord(array &$record) : void + public function prepareRecord(array &$record) { $this->relationResolver->resolveRelationsForRecord($this, $record); diff --git a/Makefile b/Makefile index b6bc72b..4fb0ce4 100644 --- a/Makefile +++ b/Makefile @@ -12,7 +12,7 @@ typo3DatabaseHost ?= "127.0.0.1" .PHONY: install install: clean - COMPOSER_PROCESS_TIMEOUT=1000 composer require -vv --dev --prefer-dist --ignore-platform-reqs typo3/cms="$(TYPO3_VERSION)" + COMPOSER_PROCESS_TIMEOUT=1000 composer require -vv --dev --prefer-dist typo3/cms="$(TYPO3_VERSION)" git checkout composer.json functionalTests: diff --git a/composer.json b/composer.json index b704e77..c260226 100644 --- a/composer.json +++ b/composer.json @@ -15,8 +15,8 @@ "TYPO3\\CMS\\Core\\Tests\\": ".Build/vendor/typo3/cms/typo3/sysext/core/Tests/" } }, - "require" : { - "php": ">=7.1.0", + "require": { + "php": ">=7.0.0", "typo3/cms": "~8.7", "ruflin/elastica": "~3.2" }, From 47b32820346282fe7969e904ebf18583d0b8c1ae Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 22 Feb 2018 21:55:52 +0100 Subject: [PATCH 06/22] BUGFIX: Allow indexing of new records with their relations Relations were inserted by TYPO3's DataHandler after indexing and were therefore not indexed. We now use a later hook after DataHandler has finished everything, so we know that we can index. As it's not relevant, we do not differentiate between add and update anymore, as both trigger "indexDocument" internal. Resolves: #112 --- Classes/Domain/Service/DataHandler.php | 10 ---- Classes/Hook/DataHandler.php | 50 ++++++++--------- .../Form/Finisher/DataHandlerFinisher.php | 4 +- .../IgnoresUnkownOperationTest.php | 54 ------------------- .../DataHandler/NonAllowedTablesTest.php | 6 +-- .../ProcessesAllowedTablesTest.php | 6 +-- .../Form/Finisher/DataHandlerFinisherTest.php | 11 ++-- 7 files changed, 32 insertions(+), 109 deletions(-) delete mode 100644 Tests/Functional/Hooks/DataHandler/IgnoresUnkownOperationTest.php diff --git a/Classes/Domain/Service/DataHandler.php b/Classes/Domain/Service/DataHandler.php index 6ac8069..10a01fe 100644 --- a/Classes/Domain/Service/DataHandler.php +++ b/Classes/Domain/Service/DataHandler.php @@ -83,16 +83,6 @@ class DataHandler implements Singleton $this->indexerFactory = $indexerFactory; } - /** - * @param string $table - * @param array $record - */ - public function add($table, array $record) - { - $this->logger->debug('Record received for add.', [$table, $record]); - $this->getIndexer($table)->indexDocument($record['uid']); - } - /** * @param string $table */ diff --git a/Classes/Hook/DataHandler.php b/Classes/Hook/DataHandler.php index d0eb1ba..fa4b09f 100644 --- a/Classes/Hook/DataHandler.php +++ b/Classes/Hook/DataHandler.php @@ -92,42 +92,36 @@ class DataHandler implements Singleton return true; } - /** - * Called by CoreDataHandler on database operations, e.g. if new records were created or records were updated. - * - * @param string $status - * @param string $table - * @param string|int $uid - * @param array $fieldArray - * @param CoreDataHandler $dataHandler - * - * @return bool False if hook was not processed. - */ - public function processDatamap_afterDatabaseOperations($status, $table, $uid, array $fieldArray, CoreDataHandler $dataHandler) + public function processDatamap_afterAllOperations(CoreDataHandler $dataHandler) + { + foreach ($dataHandler->datamap as $table => $record) { + $uid = key($record); + $fieldData = current($record); + + if (isset($fieldArray['uid'])) { + $uid = $fieldArray['uid']; + } elseif (isset($dataHandler->substNEWwithIDs[$uid])) { + $uid = $dataHandler->substNEWwithIDs[$uid]; + } + + $this->processRecord($table, $uid); + } + } + + protected function processRecord(string $table, int $uid) : bool { if (! $this->shouldProcessHookForTable($table)) { - $this->logger->debug('Database update not processed.', [$table, $uid]); + $this->logger->debug('Indexing of record not processed.', [$table, $uid]); return false; } - if ($status === 'new') { - $fieldArray['uid'] = $dataHandler->substNEWwithIDs[$uid]; - $this->dataHandler->add($table, $fieldArray); + $record = $this->getRecord($table, $uid); + if ($record !== null) { + $this->dataHandler->update($table, $record); return true; } - if ($status === 'update') { - $record = $this->getRecord($table, $uid); - if ($record !== null) { - $this->dataHandler->update($table, $record); - } - return true; - } - - $this->logger->debug( - 'Database update not processed, cause status is unhandled.', - [$status, $table, $uid, $fieldArray] - ); + $this->logger->debug('Indexing of record not processed, as he was not found in Database.', [$table, $uid]); return false; } diff --git a/Classes/Integration/Form/Finisher/DataHandlerFinisher.php b/Classes/Integration/Form/Finisher/DataHandlerFinisher.php index 02d6bde..696d8ad 100644 --- a/Classes/Integration/Form/Finisher/DataHandlerFinisher.php +++ b/Classes/Integration/Form/Finisher/DataHandlerFinisher.php @@ -58,10 +58,8 @@ class DataHandlerFinisher extends AbstractFinisher switch ($action) { case 'update': - $this->dataHandler->update($tableName, $record); - break; case 'add': - $this->dataHandler->add($tableName, $record); + $this->dataHandler->update($tableName, $record); break; case 'delete': $this->dataHandler->delete($tableName, $record['uid']); diff --git a/Tests/Functional/Hooks/DataHandler/IgnoresUnkownOperationTest.php b/Tests/Functional/Hooks/DataHandler/IgnoresUnkownOperationTest.php deleted file mode 100644 index b1676e3..0000000 --- a/Tests/Functional/Hooks/DataHandler/IgnoresUnkownOperationTest.php +++ /dev/null @@ -1,54 +0,0 @@ - - * - * 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 Codappix\SearchCore\Configuration\ConfigurationContainerInterface; -use Codappix\SearchCore\Domain\Service\DataHandler as DataHandlerService; -use Codappix\SearchCore\Hook\DataHandler as DataHandlerHook; -use TYPO3\CMS\Core\DataHandling\DataHandler as Typo3DataHandler; -use TYPO3\CMS\Core\Utility\GeneralUtility; -use TYPO3\CMS\Extbase\Object\ObjectManager; - -class IgnoresUnkownOperationTest extends AbstractDataHandlerTest -{ - /** - * @var DataHandlerService|\PHPUnit_Framework_MockObject_MockObject|AccessibleObjectInterface - */ - protected $subject; - - /** - * @test - */ - public function dataHandlerCommandSomethingIsIgnored() - { - $subject = new DataHandlerHook($this->subject); - $this->assertFalse( - $subject->processDatamap_afterDatabaseOperations( - 'something', - 'tt_content', - 1, - [], - new Typo3DataHandler - ), - 'Hook processed status "something".' - ); - } -} diff --git a/Tests/Functional/Hooks/DataHandler/NonAllowedTablesTest.php b/Tests/Functional/Hooks/DataHandler/NonAllowedTablesTest.php index c33701d..509f053 100644 --- a/Tests/Functional/Hooks/DataHandler/NonAllowedTablesTest.php +++ b/Tests/Functional/Hooks/DataHandler/NonAllowedTablesTest.php @@ -64,7 +64,7 @@ class NonAllowedTablesTest extends AbstractDataHandlerTest /** * @test */ - public function updateWillNotBeTriggeredForSysCategory() + public function updateWillNotBeTriggeredForExistingSysCategory() { $this->subject->expects($this->exactly(0))->method('update'); @@ -83,9 +83,9 @@ class NonAllowedTablesTest extends AbstractDataHandlerTest /** * @test */ - public function addWillNotBeTriggeredForSysCategoy() + public function updateWillNotBeTriggeredForNewSysCategoy() { - $this->subject->expects($this->exactly(0))->method('add'); + $this->subject->expects($this->exactly(0))->method('update'); $tce = GeneralUtility::makeInstance(Typo3DataHandler::class); $tce->stripslashes_values = 0; diff --git a/Tests/Functional/Hooks/DataHandler/ProcessesAllowedTablesTest.php b/Tests/Functional/Hooks/DataHandler/ProcessesAllowedTablesTest.php index 6a027cf..29ee14c 100644 --- a/Tests/Functional/Hooks/DataHandler/ProcessesAllowedTablesTest.php +++ b/Tests/Functional/Hooks/DataHandler/ProcessesAllowedTablesTest.php @@ -66,7 +66,7 @@ class ProcessesAllowedTablesTest extends AbstractDataHandlerTest /** * @test */ - public function updateWillBeTriggeredForTtContent() + public function updateWillBeTriggeredForExistingTtContent() { $this->subject->expects($this->exactly(1))->method('update') ->with( @@ -94,9 +94,9 @@ class ProcessesAllowedTablesTest extends AbstractDataHandlerTest /** * @test */ - public function addWillBeTriggeredForTtContent() + public function updateWillBeTriggeredForNewTtContent() { - $this->subject->expects($this->exactly(1))->method('add') + $this->subject->expects($this->exactly(1))->method('update') ->with( $this->equalTo('tt_content'), $this->callback(function ($record) { diff --git a/Tests/Unit/Integration/Form/Finisher/DataHandlerFinisherTest.php b/Tests/Unit/Integration/Form/Finisher/DataHandlerFinisherTest.php index 2211cc2..24c5059 100644 --- a/Tests/Unit/Integration/Form/Finisher/DataHandlerFinisherTest.php +++ b/Tests/Unit/Integration/Form/Finisher/DataHandlerFinisherTest.php @@ -83,19 +83,14 @@ class DataHandlerFinisherTest extends AbstractUnitTestCase public function possibleFinisherSetup() : array { return [ - 'valid add configuration' => [ - 'action' => 'add', - 'nonCalledActions' => ['delete', 'update'], - 'expectedSecondArgument' => ['uid' => 23], - ], 'valid update configuration' => [ 'action' => 'update', - 'nonCalledActions' => ['delete', 'add'], + 'nonCalledActions' => ['delete'], 'expectedSecondArgument' => ['uid' => 23], ], 'valid delete configuration' => [ 'action' => 'delete', - 'nonCalledActions' => ['update', 'add'], + 'nonCalledActions' => ['update'], 'expectedSecondArgument' => 23, ], ]; @@ -109,7 +104,7 @@ class DataHandlerFinisherTest extends AbstractUnitTestCase { $this->subject->setOptions($options); - foreach (['add', 'update', 'delete'] as $nonCalledAction) { + foreach (['update', 'delete'] as $nonCalledAction) { $this->dataHandlerMock->expects($this->never())->method($nonCalledAction); } From 350f8a52b621500433f3a776fb6bf03c34c1a96b Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 20 Feb 2018 12:01:28 +0100 Subject: [PATCH 07/22] FEATURE: Use extbase for processor instantiation This way injects will be resolved inside of processors, enabling developers to inject dependencies. We use inject instead of constructor as indexers mostly will change the constructor and should not need to add the objectmanager. Resolves: #117 --- Classes/Domain/Index/AbstractIndexer.php | 9 ++++++++- Documentation/source/configuration/indexing.rst | 3 +++ Tests/Unit/Domain/Index/AbstractIndexerTest.php | 14 ++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/Classes/Domain/Index/AbstractIndexer.php b/Classes/Domain/Index/AbstractIndexer.php index bc8674e..c48ecb6 100644 --- a/Classes/Domain/Index/AbstractIndexer.php +++ b/Classes/Domain/Index/AbstractIndexer.php @@ -48,6 +48,12 @@ abstract class AbstractIndexer implements IndexerInterface */ protected $logger; + /** + * @var \TYPO3\CMS\Extbase\Object\ObjectManagerInterface + * @inject + */ + protected $objectManager; + /** * Inject log manager to get concrete logger from it. * @@ -141,7 +147,8 @@ abstract class AbstractIndexer implements IndexerInterface } else { $className = $configuration['_typoScriptNodeValue']; } - $dataProcessor = GeneralUtility::makeInstance($className); + + $dataProcessor = $this->objectManager->get($className); if ($dataProcessor instanceof ProcessorInterface) { $record = $dataProcessor->processRecord($record, $configuration); } diff --git a/Documentation/source/configuration/indexing.rst b/Documentation/source/configuration/indexing.rst index b605008..619e56a 100644 --- a/Documentation/source/configuration/indexing.rst +++ b/Documentation/source/configuration/indexing.rst @@ -205,3 +205,6 @@ class name) as done in the examples above. By implementing also the same interface as necessary for TYPO3 :ref:`t3tsref:cobj-fluidtemplate-properties-dataprocessing`, you are able to reuse the same code also for Fluid to prepare the same record fetched from DB for your fluid. + +Dependency injection is possible inside of processors, as we instantiate through extbase +``ObjectManager``. diff --git a/Tests/Unit/Domain/Index/AbstractIndexerTest.php b/Tests/Unit/Domain/Index/AbstractIndexerTest.php index 3bbc97d..8c1e527 100644 --- a/Tests/Unit/Domain/Index/AbstractIndexerTest.php +++ b/Tests/Unit/Domain/Index/AbstractIndexerTest.php @@ -26,6 +26,7 @@ use Codappix\SearchCore\Connection\ConnectionInterface; use Codappix\SearchCore\DataProcessing\CopyToProcessor; use Codappix\SearchCore\Domain\Index\AbstractIndexer; use Codappix\SearchCore\Tests\Unit\AbstractUnitTestCase; +use TYPO3\CMS\Extbase\Object\ObjectManagerInterface; class AbstractIndexerTest extends AbstractUnitTestCase { @@ -44,17 +45,25 @@ class AbstractIndexerTest extends AbstractUnitTestCase */ protected $connection; + /** + * @var ObjectManagerInterface + */ + protected $objectManager; + public function setUp() { parent::setUp(); $this->configuration = $this->getMockBuilder(ConfigurationContainerInterface::class)->getMock(); $this->connection = $this->getMockBuilder(ConnectionInterface::class)->getMock(); + $this->objectManager = $this->getMockBuilder(ObjectManagerInterface::class)->getMock(); + $this->subject = $this->getMockForAbstractClass(AbstractIndexer::class, [ $this->connection, $this->configuration ]); + $this->inject($this->subject, 'objectManager', $this->objectManager); $this->subject->injectLogger($this->getMockedLogger()); $this->subject->setIdentifier('testTable'); $this->subject->expects($this->any()) @@ -73,6 +82,11 @@ class AbstractIndexerTest extends AbstractUnitTestCase $expectedRecord['new_test_field2'] = 'test' . PHP_EOL . 'test'; $expectedRecord['search_abstract'] = ''; + $this->objectManager->expects($this->any()) + ->method('get') + ->with(CopyToProcessor::class) + ->willReturn(new CopyToProcessor()); + $this->configuration->expects($this->exactly(2)) ->method('get') ->withConsecutive(['indexing.testTable.dataProcessing'], ['indexing.testTable.abstractFields']) From 2998c43ba824492bbb888e6b99c1c0f349c7e5ad Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 1 Mar 2018 08:03:51 +0100 Subject: [PATCH 08/22] 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' => [ ], ], ]; From 769fb8237b14671f8e50717c100b39a7f06dc97c Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 6 Mar 2018 09:04:47 +0100 Subject: [PATCH 09/22] TASK: Add data processing to search result Search results are now processed through data processing by search service. The result will be a SearchResult model from our domain. Also SearchResult will execute new queries, e.g. from paginate widget, through SearchService to apply data processing again. Remove duplicate stub code to trait, to keep own logic and code clean. --- .../Connection/Elasticsearch/SearchResult.php | 39 +----- Classes/Connection/ResultItemInterface.php | 9 +- Classes/Connection/SearchRequestInterface.php | 7 + .../Domain/Model/QueryResultInterfaceStub.php | 61 +++++++++ .../Model}/ResultItem.php | 11 +- Classes/Domain/Model/SearchRequest.php | 13 +- Classes/Domain/Model/SearchResult.php | 129 ++++++++++++++++++ Classes/Domain/Search/SearchService.php | 46 ++++++- .../Unit/Domain/Index/AbstractIndexerTest.php | 35 ++++- .../Unit/Domain/Search/SearchServiceTest.php | 84 +++++++++--- 10 files changed, 369 insertions(+), 65 deletions(-) create mode 100644 Classes/Domain/Model/QueryResultInterfaceStub.php rename Classes/{Connection/Elasticsearch => Domain/Model}/ResultItem.php (88%) create mode 100644 Classes/Domain/Model/SearchResult.php diff --git a/Classes/Connection/Elasticsearch/SearchResult.php b/Classes/Connection/Elasticsearch/SearchResult.php index 3919722..867d823 100644 --- a/Classes/Connection/Elasticsearch/SearchResult.php +++ b/Classes/Connection/Elasticsearch/SearchResult.php @@ -24,10 +24,14 @@ use Codappix\SearchCore\Connection\FacetInterface; use Codappix\SearchCore\Connection\ResultItemInterface; use Codappix\SearchCore\Connection\SearchRequestInterface; use Codappix\SearchCore\Connection\SearchResultInterface; +use Codappix\SearchCore\Domain\Model\QueryResultInterfaceStub; +use Codappix\SearchCore\Domain\Model\ResultItem; use TYPO3\CMS\Extbase\Object\ObjectManagerInterface; class SearchResult implements SearchResultInterface { + use QueryResultInterfaceStub; + /** * @var SearchRequestInterface */ @@ -104,7 +108,7 @@ class SearchResult implements SearchResultInterface } foreach ($this->result->getResults() as $result) { - $this->results[] = new ResultItem($result); + $this->results[] = new ResultItem($result->getData()); } } @@ -153,41 +157,8 @@ class SearchResult implements SearchResultInterface $this->position = 0; } - // Extbase QueryResultInterface - Implemented to support Pagination of Fluid. - public function getQuery() { return $this->searchRequest; } - - public function getFirst() - { - throw new \BadMethodCallException('Method is not implemented yet.', 1502195121); - } - - public function toArray() - { - throw new \BadMethodCallException('Method is not implemented yet.', 1502195135); - } - - public function offsetExists($offset) - { - // Return false to allow Fluid to use appropriate getter methods. - return false; - } - - public function offsetGet($offset) - { - throw new \BadMethodCallException('Use getter to fetch properties.', 1502196933); - } - - public function offsetSet($offset, $value) - { - throw new \BadMethodCallException('You are not allowed to modify the result.', 1502196934); - } - - public function offsetUnset($offset) - { - throw new \BadMethodCallException('You are not allowed to modify the result.', 1502196936); - } } diff --git a/Classes/Connection/ResultItemInterface.php b/Classes/Connection/ResultItemInterface.php index 56add2a..d7fe1a9 100644 --- a/Classes/Connection/ResultItemInterface.php +++ b/Classes/Connection/ResultItemInterface.php @@ -25,5 +25,12 @@ namespace Codappix\SearchCore\Connection; */ interface ResultItemInterface extends \ArrayAccess { - + /** + * Returns every information as array. + * + * Provide key/column/field => data. + * + * Used e.g. for dataprocessing. + */ + public function getPlainData() : array; } diff --git a/Classes/Connection/SearchRequestInterface.php b/Classes/Connection/SearchRequestInterface.php index 7c7956e..e24adc3 100644 --- a/Classes/Connection/SearchRequestInterface.php +++ b/Classes/Connection/SearchRequestInterface.php @@ -20,6 +20,7 @@ namespace Codappix\SearchCore\Connection; * 02110-1301, USA. */ +use Codappix\SearchCore\Domain\Search\SearchService; use TYPO3\CMS\Extbase\Persistence\QueryInterface; interface SearchRequestInterface extends QueryInterface @@ -40,4 +41,10 @@ interface SearchRequestInterface extends QueryInterface * @return array */ public function getFilter(); + + /** + * Workaround for paginate widget support which will + * use the request to build another search. + */ + public function setSearchService(SearchService $searchService); } diff --git a/Classes/Domain/Model/QueryResultInterfaceStub.php b/Classes/Domain/Model/QueryResultInterfaceStub.php new file mode 100644 index 0000000..960fd40 --- /dev/null +++ b/Classes/Domain/Model/QueryResultInterfaceStub.php @@ -0,0 +1,61 @@ + + * + * 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. + */ + +/** + * As we have to stay compatible with QueryResultInterface + * of extbase but can and need not to provide all methods, + * this stub will provde the non implemented methods to + * keep real implementations clean. + */ +trait QueryResultInterfaceStub +{ + public function getFirst() + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502195121); + } + + public function toArray() + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502195135); + } + + public function offsetExists($offset) + { + // Return false to allow Fluid to use appropriate getter methods. + return false; + } + + public function offsetGet($offset) + { + throw new \BadMethodCallException('Use getter to fetch properties.', 1502196933); + } + + public function offsetSet($offset, $value) + { + throw new \BadMethodCallException('You are not allowed to modify the result.', 1502196934); + } + + public function offsetUnset($offset) + { + throw new \BadMethodCallException('You are not allowed to modify the result.', 1502196936); + } +} diff --git a/Classes/Connection/Elasticsearch/ResultItem.php b/Classes/Domain/Model/ResultItem.php similarity index 88% rename from Classes/Connection/Elasticsearch/ResultItem.php rename to Classes/Domain/Model/ResultItem.php index e56783c..f63b408 100644 --- a/Classes/Connection/Elasticsearch/ResultItem.php +++ b/Classes/Domain/Model/ResultItem.php @@ -1,5 +1,5 @@ @@ -29,9 +29,14 @@ class ResultItem implements ResultItemInterface */ protected $data = []; - public function __construct(\Elastica\Result $result) + public function __construct(array $result) { - $this->data = $result->getData(); + $this->data = $data; + } + + public function getPlainData() : array + { + return $this->data; } public function offsetExists($offset) diff --git a/Classes/Domain/Model/SearchRequest.php b/Classes/Domain/Model/SearchRequest.php index d751ce8..d835e7b 100644 --- a/Classes/Domain/Model/SearchRequest.php +++ b/Classes/Domain/Model/SearchRequest.php @@ -23,6 +23,7 @@ namespace Codappix\SearchCore\Domain\Model; use Codappix\SearchCore\Connection\ConnectionInterface; use Codappix\SearchCore\Connection\FacetRequestInterface; use Codappix\SearchCore\Connection\SearchRequestInterface; +use Codappix\SearchCore\Domain\Search\SearchService; /** * Represents a search request used to process an actual search. @@ -63,6 +64,11 @@ class SearchRequest implements SearchRequestInterface */ protected $connection = null; + /** + * @var SearchService + */ + protected $searchService = null; + /** * @param string $query */ @@ -143,12 +149,17 @@ class SearchRequest implements SearchRequestInterface $this->connection = $connection; } + public function setSearchService(SearchService $searchService) + { + $this->searchService = $searchService; + } + // Extbase QueryInterface // Current implementation covers only paginate widget support. public function execute($returnRawQueryResult = false) { if ($this->connection instanceof ConnectionInterface) { - return $this->connection->search($this); + return $this->searchService->processResult($this->connection->search($this)); } throw new \InvalidArgumentException( diff --git a/Classes/Domain/Model/SearchResult.php b/Classes/Domain/Model/SearchResult.php new file mode 100644 index 0000000..d91820d --- /dev/null +++ b/Classes/Domain/Model/SearchResult.php @@ -0,0 +1,129 @@ + + * + * 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 Codappix\SearchCore\Connection\ResultItemInterface; +use Codappix\SearchCore\Connection\SearchResultInterface; +use Codappix\SearchCore\Domain\Model\QueryResultInterfaceStub; + +/** + * Generic model for mapping a concrete search result from a connection. + */ +class SearchResult implements SearchResultInterface +{ + use QueryResultInterfaceStub; + + /** + * @var SearchResultInterface + */ + protected $originalSearchResult; + + /** + * @var array + */ + protected $resultItems = []; + + /** + * @var array + */ + protected $results = []; + + /** + * For Iterator interface. + * + * @var int + */ + protected $position = 0; + + public function __construct(SearchResultInterface $originalSearchResult, array $resultItems) + { + $this->originalSearchResult = $originalSearchResult; + $this->resultItems = $resultItems; + } + + /** + * @return array + */ + public function getResults() + { + $this->initResults(); + + return $this->results; + } + + protected function initResults() + { + if ($this->results !== []) { + return; + } + + foreach ($this->resultItems as $item) { + $this->results[] = new ResultItem($item); + } + } + + public function getFacets() + { + return $this->originalSearchResult->getFacets(); + } + + public function getCurrentCount() + { + return $this->originalSearchResult->getCurrentCount(); + } + + public function count() + { + return $this->originalSearchResult->count(); + } + + public function current() + { + return $this->getResults()[$this->position]; + } + + public function next() + { + ++$this->position; + + return $this->current(); + } + + public function key() + { + return $this->position; + } + + public function valid() + { + return isset($this->getResults()[$this->position]); + } + + public function rewind() + { + $this->position = 0; + } + + public function getQuery() + { + return $this->originalSearchResult->getQuery(); + } +} diff --git a/Classes/Domain/Search/SearchService.php b/Classes/Domain/Search/SearchService.php index 120ab3b..48be32c 100644 --- a/Classes/Domain/Search/SearchService.php +++ b/Classes/Domain/Search/SearchService.php @@ -25,7 +25,9 @@ use Codappix\SearchCore\Configuration\InvalidArgumentException; use Codappix\SearchCore\Connection\ConnectionInterface; use Codappix\SearchCore\Connection\SearchRequestInterface; use Codappix\SearchCore\Connection\SearchResultInterface; +use Codappix\SearchCore\DataProcessing\Service as DataProcessorService; use Codappix\SearchCore\Domain\Model\FacetRequest; +use Codappix\SearchCore\Domain\Model\SearchResult; use TYPO3\CMS\Core\Utility\ArrayUtility; use TYPO3\CMS\Extbase\Object\ObjectManagerInterface; @@ -49,19 +51,27 @@ class SearchService */ protected $objectManager; + /** + * @var DataProcessorService + */ + protected $dataProcessorService; + /** * @param ConnectionInterface $connection * @param ConfigurationContainerInterface $configuration * @param ObjectManagerInterface $objectManager + * @param DataProcessorService $dataProcessorService */ public function __construct( ConnectionInterface $connection, ConfigurationContainerInterface $configuration, - ObjectManagerInterface $objectManager + ObjectManagerInterface $objectManager, + DataProcessorService $dataProcessorService ) { $this->connection = $connection; $this->configuration = $configuration; $this->objectManager = $objectManager; + $this->dataProcessorService = $dataProcessorService; } /** @@ -70,12 +80,15 @@ class SearchService */ public function search(SearchRequestInterface $searchRequest) { - $searchRequest->setConnection($this->connection); $this->addSize($searchRequest); $this->addConfiguredFacets($searchRequest); $this->addConfiguredFilters($searchRequest); - return $this->connection->search($searchRequest); + // Add connection to request to enable paginate widget support + $searchRequest->setConnection($this->connection); + $searchRequest->setSearchService($this); + + return $this->processResult($this->connection->search($searchRequest)); } /** @@ -138,4 +151,31 @@ class SearchService // Nothing todo, no filter configured. } } + + /** + * Processes the result, e.g. applies configured data processing to result. + */ + public function processResult(SearchResultInterface $searchResult) : SearchResultInterface + { + try { + $newSearchResultItems = []; + foreach ($this->configuration->get('searching.dataProcessing') as $configuration) { + foreach ($searchResult as $resultItem) { + $newSearchResultItems[] = $this->dataProcessorService->executeDataProcessor( + $configuration, + $resultItem->getPlainData() + ); + } + } + + return $this->objectManager->get( + SearchResult::class, + $searchResult, + $newSearchResultItems + ); + } + catch (InvalidArgumentException $e) { + return $searchResult; + } + } } diff --git a/Tests/Unit/Domain/Index/AbstractIndexerTest.php b/Tests/Unit/Domain/Index/AbstractIndexerTest.php index 3bbc97d..411cc24 100644 --- a/Tests/Unit/Domain/Index/AbstractIndexerTest.php +++ b/Tests/Unit/Domain/Index/AbstractIndexerTest.php @@ -24,6 +24,7 @@ use Codappix\SearchCore\Configuration\ConfigurationContainerInterface; use Codappix\SearchCore\Configuration\InvalidArgumentException; use Codappix\SearchCore\Connection\ConnectionInterface; use Codappix\SearchCore\DataProcessing\CopyToProcessor; +use Codappix\SearchCore\DataProcessing\Service as DataProcessorService; use Codappix\SearchCore\Domain\Index\AbstractIndexer; use Codappix\SearchCore\Tests\Unit\AbstractUnitTestCase; @@ -44,17 +45,26 @@ class AbstractIndexerTest extends AbstractUnitTestCase */ protected $connection; + /** + * @var DataProcessorService + */ + protected $dataProcessorService; + public function setUp() { parent::setUp(); $this->configuration = $this->getMockBuilder(ConfigurationContainerInterface::class)->getMock(); $this->connection = $this->getMockBuilder(ConnectionInterface::class)->getMock(); + $this->dataProcessorService = $this->getMockBuilder(DataProcessorService::class) + ->disableOriginalConstructor() + ->getMock(); $this->subject = $this->getMockForAbstractClass(AbstractIndexer::class, [ $this->connection, $this->configuration ]); + $this->inject($this->subject, 'dataProcessorService', $this->dataProcessorService); $this->subject->injectLogger($this->getMockedLogger()); $this->subject->setIdentifier('testTable'); $this->subject->expects($this->any()) @@ -73,7 +83,30 @@ class AbstractIndexerTest extends AbstractUnitTestCase $expectedRecord['new_test_field2'] = 'test' . PHP_EOL . 'test'; $expectedRecord['search_abstract'] = ''; - $this->configuration->expects($this->exactly(2)) + $this->dataProcessorService->expects($this->any()) + ->method('executeDataProcessor') + ->withConsecutive( + [ + [ + '_typoScriptNodeValue' => CopyToProcessor::class, + 'to' => 'new_test_field', + ], + $record, + ], + [ + [ + '_typoScriptNodeValue' => CopyToProcessor::class, + 'to' => 'new_test_field2', + ], + array_merge($record, ['new_test_field' => 'test']), + ] + ) + ->will($this->onConsecutiveCalls( + array_merge($record, ['new_test_field' => 'test']), + $expectedRecord + )); + + $this->configuration->expects($this->any()) ->method('get') ->withConsecutive(['indexing.testTable.dataProcessing'], ['indexing.testTable.abstractFields']) ->will($this->onConsecutiveCalls([ diff --git a/Tests/Unit/Domain/Search/SearchServiceTest.php b/Tests/Unit/Domain/Search/SearchServiceTest.php index c63e29e..aaf2e5c 100644 --- a/Tests/Unit/Domain/Search/SearchServiceTest.php +++ b/Tests/Unit/Domain/Search/SearchServiceTest.php @@ -23,6 +23,8 @@ namespace Copyright\SearchCore\Tests\Unit\Domain\Search; use Codappix\SearchCore\Configuration\ConfigurationContainerInterface; use Codappix\SearchCore\Configuration\InvalidArgumentException; use Codappix\SearchCore\Connection\ConnectionInterface; +use Codappix\SearchCore\Connection\SearchResultInterface; +use Codappix\SearchCore\DataProcessing\Service as DataProcessorService; use Codappix\SearchCore\Domain\Model\SearchRequest; use Codappix\SearchCore\Domain\Search\SearchService; use Codappix\SearchCore\Tests\Unit\AbstractUnitTestCase; @@ -35,24 +37,59 @@ class SearchServiceTest extends AbstractUnitTestCase */ protected $subject; + /** + * @var SearchResultInterface + */ + protected $result; + + /** + * @var ConnectionInterface + */ + protected $connection; + + /** + * @var ConfigurationContainerInterface + */ + protected $configuration; + + /** + * @var ObjectManagerInterface + */ + protected $objectManager; + + /** + * @var DataProcessorService + */ + protected $dataProcessorService; + public function setUp() { parent::setUp(); + $this->result = $this->getMockBuilder(SearchResultInterface::class) + ->disableOriginalConstructor() + ->getMock(); $this->connection = $this->getMockBuilder(ConnectionInterface::class) ->disableOriginalConstructor() ->getMock(); + $this->connection->expects($this->any()) + ->method('search') + ->willReturn($this->result); $this->configuration = $this->getMockBuilder(ConfigurationContainerInterface::class) ->disableOriginalConstructor() ->getMock(); $this->objectManager = $this->getMockBuilder(ObjectManagerInterface::class) ->disableOriginalConstructor() ->getMock(); + $this->dataProcessorService = $this->getMockBuilder(DataProcessorService::class) + ->setConstructorArgs([$this->objectManager]) + ->getMock(); $this->subject = new SearchService( $this->connection, $this->configuration, - $this->objectManager + $this->objectManager, + $this->dataProcessorService ); } @@ -61,13 +98,12 @@ class SearchServiceTest extends AbstractUnitTestCase */ public function sizeIsAddedFromConfiguration() { - $this->configuration->expects($this->exactly(2)) + $this->configuration->expects($this->any()) ->method('getIfExists') ->withConsecutive(['searching.size'], ['searching.facets']) ->will($this->onConsecutiveCalls(45, null)); - $this->configuration->expects($this->exactly(1)) + $this->configuration->expects($this->any()) ->method('get') - ->with('searching.filter') ->will($this->throwException(new InvalidArgumentException)); $this->connection->expects($this->once()) ->method('search') @@ -84,13 +120,12 @@ class SearchServiceTest extends AbstractUnitTestCase */ public function defaultSizeIsAddedIfNothingIsConfigured() { - $this->configuration->expects($this->exactly(2)) + $this->configuration->expects($this->any()) ->method('getIfExists') ->withConsecutive(['searching.size'], ['searching.facets']) ->will($this->onConsecutiveCalls(null, null)); - $this->configuration->expects($this->exactly(1)) + $this->configuration->expects($this->any()) ->method('get') - ->with('searching.filter') ->will($this->throwException(new InvalidArgumentException)); $this->connection->expects($this->once()) ->method('search') @@ -107,14 +142,16 @@ class SearchServiceTest extends AbstractUnitTestCase */ public function configuredFilterAreAddedToRequestWithoutAnyFilter() { - $this->configuration->expects($this->exactly(2)) + $this->configuration->expects($this->any()) ->method('getIfExists') ->withConsecutive(['searching.size'], ['searching.facets']) ->will($this->onConsecutiveCalls(null, null)); - $this->configuration->expects($this->exactly(1)) + $this->configuration->expects($this->any()) ->method('get') - ->with('searching.filter') - ->willReturn(['property' => 'something']); + ->will($this->onConsecutiveCalls( + ['property' => 'something'], + $this->throwException(new InvalidArgumentException) + )); $this->connection->expects($this->once()) ->method('search') @@ -131,14 +168,16 @@ class SearchServiceTest extends AbstractUnitTestCase */ public function configuredFilterAreAddedToRequestWithExistingFilter() { - $this->configuration->expects($this->exactly(2)) + $this->configuration->expects($this->any()) ->method('getIfExists') ->withConsecutive(['searching.size'], ['searching.facets']) ->will($this->onConsecutiveCalls(null, null)); - $this->configuration->expects($this->exactly(1)) + $this->configuration->expects($this->any()) ->method('get') - ->with('searching.filter') - ->willReturn(['property' => 'something']); + ->will($this->onConsecutiveCalls( + ['property' => 'something'], + $this->throwException(new InvalidArgumentException) + )); $this->connection->expects($this->once()) ->method('search') @@ -159,13 +198,12 @@ class SearchServiceTest extends AbstractUnitTestCase */ public function nonConfiguredFilterIsNotChangingRequestWithExistingFilter() { - $this->configuration->expects($this->exactly(2)) + $this->configuration->expects($this->any()) ->method('getIfExists') ->withConsecutive(['searching.size'], ['searching.facets']) ->will($this->onConsecutiveCalls(null, null)); - $this->configuration->expects($this->exactly(1)) + $this->configuration->expects($this->any()) ->method('get') - ->with('searching.filter') ->will($this->throwException(new InvalidArgumentException)); $this->connection->expects($this->once()) @@ -184,14 +222,16 @@ class SearchServiceTest extends AbstractUnitTestCase */ public function emptyConfiguredFilterIsNotChangingRequestWithExistingFilter() { - $this->configuration->expects($this->exactly(2)) + $this->configuration->expects($this->any()) ->method('getIfExists') ->withConsecutive(['searching.size'], ['searching.facets']) ->will($this->onConsecutiveCalls(null, null)); - $this->configuration->expects($this->exactly(1)) + $this->configuration->expects($this->any()) ->method('get') - ->with('searching.filter') - ->willReturn(['anotherProperty' => '']); + ->will($this->onConsecutiveCalls( + ['anotherProperty' => ''], + $this->throwException(new InvalidArgumentException) + )); $this->connection->expects($this->once()) ->method('search') From 79aba3c544003afc6e2628f864aeb9edd1da5cf5 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 6 Mar 2018 09:19:09 +0100 Subject: [PATCH 10/22] TASK: Add test cases for ResultItem Model Check whether all methods work as expected. E.g. we can retrieve data in all ways, but not change anything. --- Classes/Domain/Model/ResultItem.php | 2 +- Tests/Unit/Domain/Model/ResultItemTest.php | 113 +++++++++++++++++++++ 2 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 Tests/Unit/Domain/Model/ResultItemTest.php diff --git a/Classes/Domain/Model/ResultItem.php b/Classes/Domain/Model/ResultItem.php index f63b408..d0e370a 100644 --- a/Classes/Domain/Model/ResultItem.php +++ b/Classes/Domain/Model/ResultItem.php @@ -31,7 +31,7 @@ class ResultItem implements ResultItemInterface public function __construct(array $result) { - $this->data = $data; + $this->data = $result; } public function getPlainData() : array diff --git a/Tests/Unit/Domain/Model/ResultItemTest.php b/Tests/Unit/Domain/Model/ResultItemTest.php new file mode 100644 index 0000000..df11a5d --- /dev/null +++ b/Tests/Unit/Domain/Model/ResultItemTest.php @@ -0,0 +1,113 @@ + + * + * 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 Codappix\SearchCore\Domain\Model\ResultItem; +use Codappix\SearchCore\Tests\Unit\AbstractUnitTestCase; + +class ResultItemTest extends AbstractUnitTestCase +{ + /** + * @test + */ + public function plainDataCanBeRetrieved() + { + $originalData = [ + 'uid' => 10, + 'title' => 'Some title', + ]; + $expectedData = $originalData; + + $subject = new ResultItem($originalData); + $this->assertSame( + $expectedData, + $subject->getPlainData(), + 'Could not retrieve plain data from result item.' + ); + } + + /** + * @test + */ + public function dataCanBeRetrievedInArrayNotation() + { + $originalData = [ + 'uid' => 10, + 'title' => 'Some title', + ]; + $expectedData = $originalData; + + $subject = new ResultItem($originalData); + $this->assertSame( + 'Some title', + $subject['title'], + 'Could not retrieve title in array notation.' + ); + } + + /** + * @test + */ + public function existenceOfDataCanBeChecked() + { + $originalData = [ + 'uid' => 10, + 'title' => 'Some title', + ]; + $expectedData = $originalData; + + $subject = new ResultItem($originalData); + $this->assertTrue(isset($subject['title']), 'Could not determine that title exists.'); + $this->assertFalse(isset($subject['title2']), 'Could not determine that title2 does not exists.'); + } + + /** + * @test + */ + public function dataCanNotBeChanged() + { + $originalData = [ + 'uid' => 10, + 'title' => 'Some title', + ]; + $expectedData = $originalData; + + $subject = new ResultItem($originalData); + $this->expectException(\BadMethodCallException::class); + $subject['title'] = 'New Title'; + } + + /** + * @test + */ + public function dataCanNotBeRemoved() + { + $originalData = [ + 'uid' => 10, + 'title' => 'Some title', + ]; + $expectedData = $originalData; + + $subject = new ResultItem($originalData); + $this->expectException(\BadMethodCallException::class); + unset($subject['title']); + } +} From cf91251be36e08cd749457adbbf1e1ce5404f15f Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 6 Mar 2018 09:36:39 +0100 Subject: [PATCH 11/22] TASK :Add tests for SearchRequest Make sure exceptions with helpful messages are thrown if one object is missing when execute is called. Also make sure the expected methods are called. --- Classes/Domain/Model/SearchRequest.php | 18 +++-- Tests/Unit/Domain/Model/SearchRequestTest.php | 69 +++++++++++++++++-- 2 files changed, 75 insertions(+), 12 deletions(-) diff --git a/Classes/Domain/Model/SearchRequest.php b/Classes/Domain/Model/SearchRequest.php index d835e7b..69237bb 100644 --- a/Classes/Domain/Model/SearchRequest.php +++ b/Classes/Domain/Model/SearchRequest.php @@ -158,14 +158,20 @@ class SearchRequest implements SearchRequestInterface // Current implementation covers only paginate widget support. public function execute($returnRawQueryResult = false) { - if ($this->connection instanceof ConnectionInterface) { - return $this->searchService->processResult($this->connection->search($this)); + if (! ($this->connection instanceof ConnectionInterface)) { + throw new \InvalidArgumentException( + 'Connection was not set before, therefore execute can not work. Use `setConnection` before.', + 1502197732 + ); + } + if (! ($this->searchService instanceof SearchService)) { + throw new \InvalidArgumentException( + 'SearchService was not set before, therefore execute can not work. Use `setSearchService` before.', + 1520325175 + ); } - throw new \InvalidArgumentException( - 'Connection was not set before, therefore execute can not work. Use `setConnection` before.', - 1502197732 - ); + return $this->searchService->processResult($this->connection->search($this)); } public function setLimit($limit) diff --git a/Tests/Unit/Domain/Model/SearchRequestTest.php b/Tests/Unit/Domain/Model/SearchRequestTest.php index 127baf2..28f90e3 100644 --- a/Tests/Unit/Domain/Model/SearchRequestTest.php +++ b/Tests/Unit/Domain/Model/SearchRequestTest.php @@ -20,7 +20,10 @@ namespace Codappix\SearchCore\Tests\Unit\Domain\Model; * 02110-1301, USA. */ +use Codappix\SearchCore\Connection\ConnectionInterface; +use Codappix\SearchCore\Connection\SearchResultInterface; use Codappix\SearchCore\Domain\Model\SearchRequest; +use Codappix\SearchCore\Domain\Search\SearchService; use Codappix\SearchCore\Tests\Unit\AbstractUnitTestCase; class SearchRequestTest extends AbstractUnitTestCase @@ -31,12 +34,12 @@ class SearchRequestTest extends AbstractUnitTestCase */ public function emptyFilterWillNotBeSet(array $filter) { - $searchRequest = new SearchRequest(); - $searchRequest->setFilter($filter); + $subject = new SearchRequest(); + $subject->setFilter($filter); $this->assertSame( [], - $searchRequest->getFilter(), + $subject->getFilter(), 'Empty filter were set, even if they should not.' ); } @@ -68,13 +71,67 @@ class SearchRequestTest extends AbstractUnitTestCase public function filterIsSet() { $filter = ['someField' => 'someValue']; - $searchRequest = new SearchRequest(); - $searchRequest->setFilter($filter); + $subject = new SearchRequest(); + $subject->setFilter($filter); $this->assertSame( $filter, - $searchRequest->getFilter(), + $subject->getFilter(), 'Filter was not set.' ); } + + /** + * @test + */ + public function exceptionIsThrownIfSearchServiceWasNotSet() + { + $subject = new SearchRequest(); + $subject->setConnection($this->getMockBuilder(ConnectionInterface::class)->getMock()); + $this->expectException(\InvalidArgumentException::class); + $subject->execute(); + } + + /** + * @test + */ + public function exceptionIsThrownIfConnectionWasNotSet() + { + $subject = new SearchRequest(); + $subject->setSearchService( + $this->getMockBuilder(SearchService::class) + ->disableOriginalConstructor() + ->getMock() + ); + $this->expectException(\InvalidArgumentException::class); + $subject->execute(); + } + + /** + * @test + */ + public function executionMakesUseOfProvidedConnectionAndSearchService() + { + $searchServiceMock = $this->getMockBuilder(SearchService::class) + ->disableOriginalConstructor() + ->getMock(); + $connectionMock = $this->getMockBuilder(ConnectionInterface::class) + ->getMock(); + $searchResultMock = $this->getMockBuilder(SearchResultInterface::class) + ->getMock(); + + $subject = new SearchRequest(); + $subject->setSearchService($searchServiceMock); + $subject->setConnection($connectionMock); + + $connectionMock->expects($this->once()) + ->method('search') + ->with($subject) + ->willReturn($searchResultMock); + $searchServiceMock->expects($this->once()) + ->method('processResult') + ->with($searchResultMock); + + $subject->execute(); + } } From 45bb12cf519bc51f69a15fff38e56fd1bdba86cd Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 6 Mar 2018 09:44:26 +0100 Subject: [PATCH 12/22] TASK: Add tests for search result model --- Tests/Unit/Domain/Model/SearchResultTest.php | 100 +++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 Tests/Unit/Domain/Model/SearchResultTest.php diff --git a/Tests/Unit/Domain/Model/SearchResultTest.php b/Tests/Unit/Domain/Model/SearchResultTest.php new file mode 100644 index 0000000..910e06e --- /dev/null +++ b/Tests/Unit/Domain/Model/SearchResultTest.php @@ -0,0 +1,100 @@ + + * + * 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 Codappix\SearchCore\Connection\ResultItemInterface; +use Codappix\SearchCore\Connection\SearchResultInterface; +use Codappix\SearchCore\Domain\Model\SearchResult; +use Codappix\SearchCore\Tests\Unit\AbstractUnitTestCase; + +class SearchResultTest extends AbstractUnitTestCase +{ + /** + * @test + */ + public function countIsRetrievedFromOriginalResult() + { + $originalSearchResultMock = $this->getMockBuilder(SearchResultInterface::class)->getMock(); + $originalSearchResultMock->expects($this->once())->method('count'); + + $subject = new SearchResult($originalSearchResultMock, []); + $subject->count(); + } + + /** + * @test + */ + public function currentCountIsRetrievedFromOriginalResult() + { + $originalSearchResultMock = $this->getMockBuilder(SearchResultInterface::class)->getMock(); + $originalSearchResultMock->expects($this->once())->method('getCurrentCount'); + + $subject = new SearchResult($originalSearchResultMock, []); + $subject->getCurrentCount(); + } + + /** + * @test + */ + public function facetsAreRetrievedFromOriginalResult() + { + $originalSearchResultMock = $this->getMockBuilder(SearchResultInterface::class)->getMock(); + $originalSearchResultMock->expects($this->once())->method('getFacets'); + + $subject = new SearchResult($originalSearchResultMock, []); + $subject->getFacets(); + } + + /** + * @test + */ + public function resultItemsCanBeRetrieved() + { + $originalSearchResultMock = $this->getMockBuilder(SearchResultInterface::class)->getMock(); + $data = [ + [ + 'uid' => 10, + 'title' => 'Some Title', + ], + [ + 'uid' => 11, + 'title' => 'Some Title 2', + ], + [ + 'uid' => 12, + 'title' => 'Some Title 3', + ], + ]; + + $subject = new SearchResult($originalSearchResultMock, $data); + $resultItems = $subject->getResults(); + + $this->assertCount(3, $resultItems); + + $this->assertSame(10, $resultItems[0]['uid']); + $this->assertSame(11, $resultItems[1]['uid']); + $this->assertSame(12, $resultItems[2]['uid']); + + $this->assertInstanceOf(ResultItemInterface::class, $resultItems[0]); + $this->assertInstanceOf(ResultItemInterface::class, $resultItems[1]); + $this->assertInstanceOf(ResultItemInterface::class, $resultItems[2]); + } +} From 0210110ccfcdee1e22316ddbfc94dd26c499bf86 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 6 Mar 2018 10:26:39 +0100 Subject: [PATCH 13/22] TASK: Add test for applied data processing on search result --- .../Unit/Domain/Search/SearchServiceTest.php | 91 +++++++++++++++++-- 1 file changed, 81 insertions(+), 10 deletions(-) diff --git a/Tests/Unit/Domain/Search/SearchServiceTest.php b/Tests/Unit/Domain/Search/SearchServiceTest.php index aaf2e5c..6263f23 100644 --- a/Tests/Unit/Domain/Search/SearchServiceTest.php +++ b/Tests/Unit/Domain/Search/SearchServiceTest.php @@ -1,5 +1,5 @@ @@ -26,6 +26,7 @@ use Codappix\SearchCore\Connection\ConnectionInterface; use Codappix\SearchCore\Connection\SearchResultInterface; use Codappix\SearchCore\DataProcessing\Service as DataProcessorService; use Codappix\SearchCore\Domain\Model\SearchRequest; +use Codappix\SearchCore\Domain\Model\SearchResult; use Codappix\SearchCore\Domain\Search\SearchService; use Codappix\SearchCore\Tests\Unit\AbstractUnitTestCase; use TYPO3\CMS\Extbase\Object\ObjectManagerInterface; @@ -72,9 +73,6 @@ class SearchServiceTest extends AbstractUnitTestCase $this->connection = $this->getMockBuilder(ConnectionInterface::class) ->disableOriginalConstructor() ->getMock(); - $this->connection->expects($this->any()) - ->method('search') - ->willReturn($this->result); $this->configuration = $this->getMockBuilder(ConfigurationContainerInterface::class) ->disableOriginalConstructor() ->getMock(); @@ -109,7 +107,8 @@ class SearchServiceTest extends AbstractUnitTestCase ->method('search') ->with($this->callback(function ($searchRequest) { return $searchRequest->getLimit() === 45; - })); + })) + ->willReturn($this->getMockBuilder(SearchResultInterface::class)->getMock()); $searchRequest = new SearchRequest('SearchWord'); $this->subject->search($searchRequest); @@ -131,7 +130,8 @@ class SearchServiceTest extends AbstractUnitTestCase ->method('search') ->with($this->callback(function ($searchRequest) { return $searchRequest->getLimit() === 10; - })); + })) + ->willReturn($this->getMockBuilder(SearchResultInterface::class)->getMock()); $searchRequest = new SearchRequest('SearchWord'); $this->subject->search($searchRequest); @@ -157,7 +157,8 @@ class SearchServiceTest extends AbstractUnitTestCase ->method('search') ->with($this->callback(function ($searchRequest) { return $searchRequest->getFilter() === ['property' => 'something']; - })); + })) + ->willReturn($this->getMockBuilder(SearchResultInterface::class)->getMock()); $searchRequest = new SearchRequest('SearchWord'); $this->subject->search($searchRequest); @@ -186,7 +187,8 @@ class SearchServiceTest extends AbstractUnitTestCase 'anotherProperty' => 'anything', 'property' => 'something', ]; - })); + })) + ->willReturn($this->getMockBuilder(SearchResultInterface::class)->getMock()); $searchRequest = new SearchRequest('SearchWord'); $searchRequest->setFilter(['anotherProperty' => 'anything']); @@ -210,7 +212,8 @@ class SearchServiceTest extends AbstractUnitTestCase ->method('search') ->with($this->callback(function ($searchRequest) { return $searchRequest->getFilter() === ['anotherProperty' => 'anything']; - })); + })) + ->willReturn($this->getMockBuilder(SearchResultInterface::class)->getMock()); $searchRequest = new SearchRequest('SearchWord'); $searchRequest->setFilter(['anotherProperty' => 'anything']); @@ -237,10 +240,78 @@ class SearchServiceTest extends AbstractUnitTestCase ->method('search') ->with($this->callback(function ($searchRequest) { return $searchRequest->getFilter() === ['anotherProperty' => 'anything']; - })); + })) + ->willReturn($this->getMockBuilder(SearchResultInterface::class)->getMock()); $searchRequest = new SearchRequest('SearchWord'); $searchRequest->setFilter(['anotherProperty' => 'anything']); $this->subject->search($searchRequest); } + + /** + * @test + */ + public function originalSearchResultIsReturnedIfNoDataProcessorIsConfigured() + { + $this->configuration->expects($this->any()) + ->method('getIfExists') + ->withConsecutive(['searching.size'], ['searching.facets']) + ->will($this->onConsecutiveCalls(null, null)); + $this->configuration->expects($this->any()) + ->method('get') + ->will($this->throwException(new InvalidArgumentException)); + + $searchResultMock = $this->getMockBuilder(SearchResultInterface::class)->getMock(); + + $this->connection->expects($this->once()) + ->method('search') + ->willReturn($searchResultMock); + + $this->dataProcessorService->expects($this->never())->method('executeDataProcessor'); + + $searchRequest = new SearchRequest(''); + $this->assertSame($searchResultMock, $this->subject->search($searchRequest), 'Did not get created result without applied data processing'); + } + + /** + * @test + */ + public function configuredDataProcessorsAreExecutedOnSearchResult() + { + $this->configuration->expects($this->any()) + ->method('getIfExists') + ->withConsecutive(['searching.size'], ['searching.facets']) + ->will($this->onConsecutiveCalls(null, null)); + $this->configuration->expects($this->any()) + ->method('get') + ->will($this->onConsecutiveCalls( + $this->throwException(new InvalidArgumentException), + ['SomeProcessorClass'] + )); + + $searchResultMock = $this->getMockBuilder(SearchResultInterface::class)->getMock(); + $searchResult = new SearchResult($searchResultMock, [['field 1' => 'value 1']]); + + $this->connection->expects($this->once()) + ->method('search') + ->willReturn($searchResult); + + $this->dataProcessorService->expects($this->once()) + ->method('executeDataProcessor') + ->with('SomeProcessorClass', ['field 1' => 'value 1']) + ->willReturn([ + 'field 1' => 'value 1', + 'field 2' => 'value 2', + ]); + + $this->objectManager->expects($this->once()) + ->method('get') + ->with(SearchResult::class, $searchResult, [ + ['field 1' => 'value 1', 'field 2' => 'value 2'] + ]) + ->willReturn($searchResultMock); + + $searchRequest = new SearchRequest(''); + $this->assertSame($searchResultMock, $this->subject->search($searchRequest), 'Did not get created result with applied data processing'); + } } From 176c466d7e49f072ed11ca23c03b5c84608b33ed Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 6 Mar 2018 10:47:14 +0100 Subject: [PATCH 14/22] TASK: Update documentation for data processing --- Documentation/source/concepts.rst | 2 + .../dataProcessing/availableAndPlanned.rst | 32 ++++++++++++++++ .../source/configuration/indexing.rst | 37 ++----------------- .../source/configuration/searching.rst | 34 +++++++++++++++++ Documentation/source/features.rst | 10 +++++ 5 files changed, 81 insertions(+), 34 deletions(-) create mode 100644 Documentation/source/configuration/dataProcessing/availableAndPlanned.rst diff --git a/Documentation/source/concepts.rst b/Documentation/source/concepts.rst index 0fe6c5d..f4c5bbd 100644 --- a/Documentation/source/concepts.rst +++ b/Documentation/source/concepts.rst @@ -36,5 +36,7 @@ DataProcessing Before data is transfered to search service, it can be processed by "DataProcessors" like already known by :ref:`t3tsref:cobj-fluidtemplate-properties-dataprocessing` of :ref:`t3tsref:cobj-fluidtemplate`. +The same is true for retrieved search results. They can be processed again by "DataProcessors" to +prepare data for display in Templates or further usage. Configuration is done through TypoScript, see :ref:`dataProcessing`. diff --git a/Documentation/source/configuration/dataProcessing/availableAndPlanned.rst b/Documentation/source/configuration/dataProcessing/availableAndPlanned.rst new file mode 100644 index 0000000..8284820 --- /dev/null +++ b/Documentation/source/configuration/dataProcessing/availableAndPlanned.rst @@ -0,0 +1,32 @@ +The following Processor are available: + +.. toctree:: + :maxdepth: 1 + :glob: + + /configuration/dataProcessing/CopyToProcessor + /configuration/dataProcessing/RemoveProcessor + /configuration/dataProcessing/GeoPointProcessor + +The following Processor are planned: + + ``Codappix\SearchCore\DataProcessing\ReplaceProcessor`` + Will execute a search and replace on configured fields. + + ``Codappix\SearchCore\DataProcessing\RootLevelProcessor`` + Will attach the root level to the record. + + ``Codappix\SearchCore\DataProcessing\ChannelProcessor`` + Will add a configurable channel to the record, e.g. if you have different areas in your + website like "products" and "infos". + + ``Codappix\SearchCore\DataProcessing\RelationResolverProcessor`` + Resolves all relations using the TCA. + +Of course you are able to provide further processors. Just implement +``Codappix\SearchCore\DataProcessing\ProcessorInterface`` and use the FQCN (=Fully qualified +class name) as done in the examples above. + +By implementing also the same interface as necessary for TYPO3 +:ref:`t3tsref:cobj-fluidtemplate-properties-dataprocessing`, you are able to reuse the same code +also for Fluid to prepare the same record fetched from DB for your fluid. diff --git a/Documentation/source/configuration/indexing.rst b/Documentation/source/configuration/indexing.rst index b605008..7d6c3d4 100644 --- a/Documentation/source/configuration/indexing.rst +++ b/Documentation/source/configuration/indexing.rst @@ -146,7 +146,7 @@ Example:: dataProcessing -------------- -Used by: All connections while indexing. +Used by: All connections while indexing, due to implementation inside ``AbstractIndexer``. Configure modifications on each document before sending it to the configured connection. Same as provided by TYPO3 for :ref:`t3tsref:cobj-fluidtemplate` through @@ -170,38 +170,7 @@ Example:: The above example will copy all existing fields to the field ``search_spellcheck``. Afterwards all fields, including ``search_spellcheck`` will be copied to ``search_all``. -E.g. used to index all information into a field for :ref:`spellchecking` or searching with -different :ref:`mapping`. -The following Processor are available: +.. include:: /configuration/dataProcessing/availableAndPlanned.rst -.. toctree:: - :maxdepth: 1 - :glob: - - dataProcessing/CopyToProcessor - dataProcessing/RemoveProcessor - dataProcessing/GeoPointProcessor - -The following Processor are planned: - - ``Codappix\SearchCore\DataProcessing\ReplaceProcessor`` - Will execute a search and replace on configured fields. - - ``Codappix\SearchCore\DataProcessing\RootLevelProcessor`` - Will attach the root level to the record. - - ``Codappix\SearchCore\DataProcessing\ChannelProcessor`` - Will add a configurable channel to the record, e.g. if you have different areas in your - website like "products" and "infos". - - ``Codappix\SearchCore\DataProcessing\RelationResolverProcessor`` - Resolves all relations using the TCA. - -Of course you are able to provide further processors. Just implement -``Codappix\SearchCore\DataProcessing\ProcessorInterface`` and use the FQCN (=Fully qualified -class name) as done in the examples above. - -By implementing also the same interface as necessary for TYPO3 -:ref:`t3tsref:cobj-fluidtemplate-properties-dataprocessing`, you are able to reuse the same code -also for Fluid to prepare the same record fetched from DB for your fluid. +Also data processors are available for search results too, see :ref:`searching_dataProcessing`. diff --git a/Documentation/source/configuration/searching.rst b/Documentation/source/configuration/searching.rst index ae73fad..bfed51e 100644 --- a/Documentation/source/configuration/searching.rst +++ b/Documentation/source/configuration/searching.rst @@ -216,3 +216,37 @@ Example:: } Only ``filter`` is allowed as value. Will submit an empty query to switch to filter mode. + +.. _searching_dataProcessing: + +dataProcessing +-------------- + +Used by: All connections while indexing, due to implementation inside ``SearchService``. + +Configure modifications on each document before returning search result. Same as provided by TYPO3 +for :ref:`t3tsref:cobj-fluidtemplate` through +:ref:`t3tsref:cobj-fluidtemplate-properties-dataprocessing`. + +All processors are applied in configured order. Allowing to work with already processed data. + +Example:: + + plugin.tx_searchcore.settings.searching.dataProcessing { + 1 = Codappix\SearchCore\DataProcessing\CopyToProcessor + 1 { + to = search_spellcheck + } + + 2 = Codappix\SearchCore\DataProcessing\CopyToProcessor + 2 { + to = search_all + } + } + +The above example will copy all existing fields to the field ``search_spellcheck``. Afterwards +all fields, including ``search_spellcheck`` will be copied to ``search_all``. + +.. include:: /configuration/dataProcessing/availableAndPlanned.rst + +Also data processors are available while indexing too, see :ref:`dataProcessing`. diff --git a/Documentation/source/features.rst b/Documentation/source/features.rst index 8baf453..1a4abd6 100644 --- a/Documentation/source/features.rst +++ b/Documentation/source/features.rst @@ -29,6 +29,16 @@ Also multiple filter are supported. Filtering results by fields for string conte Facets / aggregates are also possible. Therefore a mapping has to be defined in TypoScript for indexing, and the facets itself while searching. +.. _features_dataProcessing: + +DataProcessing +============== + +DataProcessing, as known from ``FLUIDTEMPLATE`` is available while indexing and for search results. +Each item can be processed by multiple processor to prepare data for indexing and output. + +See :ref:`concepts_indexing_dataprocessing` in :ref:`concepts` section. + .. _features_planned: Planned From 04aaad12fe611b994f0c9e7f704e0c73fcfc7079 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 6 Mar 2018 11:16:00 +0100 Subject: [PATCH 15/22] FEATURE: Provide ContentObjectDataProcessorAdapterProcessor Allow integrator to execute any existing data processor for content objects. Resolves: #118 --- ...entObjectDataProcessorAdapterProcessor.php | 59 +++++++++++++++++++ ...entObjectDataProcessorAdapterProcessor.rst | 32 ++++++++++ .../dataProcessing/availableAndPlanned.rst | 3 +- ...bjectDataProcessorAdapterProcessorTest.php | 55 +++++++++++++++++ 4 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 Classes/DataProcessing/ContentObjectDataProcessorAdapterProcessor.php create mode 100644 Documentation/source/configuration/dataProcessing/ContentObjectDataProcessorAdapterProcessor.rst create mode 100644 Tests/Functional/DataProcessing/ContentObjectDataProcessorAdapterProcessorTest.php diff --git a/Classes/DataProcessing/ContentObjectDataProcessorAdapterProcessor.php b/Classes/DataProcessing/ContentObjectDataProcessorAdapterProcessor.php new file mode 100644 index 0000000..322486b --- /dev/null +++ b/Classes/DataProcessing/ContentObjectDataProcessorAdapterProcessor.php @@ -0,0 +1,59 @@ + + * + * 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\Core\Utility\GeneralUtility; +use TYPO3\CMS\Extbase\Service\TypoScriptService; +use TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer; + +/** + * Executes an existing TYPO3 DataProcessor on the given data. + */ +class ContentObjectDataProcessorAdapterProcessor implements ProcessorInterface +{ + /** + * @var TypoScriptService + */ + protected $typoScriptService; + + public function __construct(TypoScriptService $typoScriptService) + { + $this->typoScriptService = $typoScriptService; + } + + public function processData(array $data, array $configuration) : array + { + $dataProcessor = GeneralUtility::makeInstance($configuration['_dataProcessor']); + $contentObjectRenderer = GeneralUtility::makeInstance(ContentObjectRenderer::class); + + $contentObjectRenderer->data = $data; + if (isset($configuration['_table'])) { + $contentObjectRenderer->start($data, $configuration['_table']); + } + + return $dataProcessor->process( + $contentObjectRenderer, + [], + $this->typoScriptService->convertPlainArrayToTypoScriptArray($configuration), + $data + ); + } +} diff --git a/Documentation/source/configuration/dataProcessing/ContentObjectDataProcessorAdapterProcessor.rst b/Documentation/source/configuration/dataProcessing/ContentObjectDataProcessorAdapterProcessor.rst new file mode 100644 index 0000000..7d8f0b2 --- /dev/null +++ b/Documentation/source/configuration/dataProcessing/ContentObjectDataProcessorAdapterProcessor.rst @@ -0,0 +1,32 @@ +``Codappix\SearchCore\DataProcessing\ContentObjectDataProcessorAdapterProcessor`` +================================================================================= + +Will execute an existing TYPO3 data processor. + +Possible Options: + +``_dataProcessor`` + Necessary, defined which data processor to apply. Provide the same as you would to call the + processor. +``_table`` + Defines the "current" table as used by some processors, e.g. + ``TYPO3\CMS\Frontend\DataProcessing\FilesProcessor``. + +All further options are passed to the configured data processor. Therefore they are documented at +each data processor. + +Example:: + + plugin.tx_searchcore.settings.searching.dataProcessing { + 1 = Codappix\SearchCore\DataProcessing\ContentObjectDataProcessorAdapterProcessor + 1 { + _table = pages + _dataProcessor = TYPO3\CMS\Frontend\DataProcessing\FilesProcessor + + references.fieldName = media + as = images + } + } + +The above example will create a new field ``images`` with resolved FAL relations from ``media`` +field. diff --git a/Documentation/source/configuration/dataProcessing/availableAndPlanned.rst b/Documentation/source/configuration/dataProcessing/availableAndPlanned.rst index 0d9385b..9f31736 100644 --- a/Documentation/source/configuration/dataProcessing/availableAndPlanned.rst +++ b/Documentation/source/configuration/dataProcessing/availableAndPlanned.rst @@ -4,9 +4,10 @@ The following Processor are available: :maxdepth: 1 :glob: + /configuration/dataProcessing/ContentObjectDataProcessorAdapterProcessor /configuration/dataProcessing/CopyToProcessor - /configuration/dataProcessing/RemoveProcessor /configuration/dataProcessing/GeoPointProcessor + /configuration/dataProcessing/RemoveProcessor The following Processor are planned: diff --git a/Tests/Functional/DataProcessing/ContentObjectDataProcessorAdapterProcessorTest.php b/Tests/Functional/DataProcessing/ContentObjectDataProcessorAdapterProcessorTest.php new file mode 100644 index 0000000..871b205 --- /dev/null +++ b/Tests/Functional/DataProcessing/ContentObjectDataProcessorAdapterProcessorTest.php @@ -0,0 +1,55 @@ + + * + * 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 Codappix\SearchCore\DataProcessing\ContentObjectDataProcessorAdapterProcessor; +use Codappix\SearchCore\Tests\Functional\AbstractFunctionalTestCase; +use TYPO3\CMS\Extbase\Service\TypoScriptService; +use TYPO3\CMS\Frontend\DataProcessing\SplitProcessor; + +class ContentObjectDataProcessorAdapterProcessorTest extends AbstractFunctionalTestCase +{ + /** + * @test + */ + public function contentObjectDataProcessorIsExecuted() + { + $record = ['content' => 'value1, value2']; + $configuration = [ + '_dataProcessor' => SplitProcessor::class, + 'delimiter' => ',', + 'fieldName' => 'content', + 'as' => 'new_content', + ]; + $expectedData = [ + 'content' => 'value1, value2', + 'new_content' => ['value1', 'value2'], + ]; + + $subject = new ContentObjectDataProcessorAdapterProcessor(new TypoScriptService); + $processedData = $subject->processData($record, $configuration); + $this->assertSame( + $expectedData, + $processedData, + 'The processor did not return the expected processed record.' + ); + } +} From 5d1e7c41bc6bfdddea1a2818c6df3e8b6e373a7a Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 6 Mar 2018 11:36:05 +0100 Subject: [PATCH 16/22] !!!|FEATURE: Pass facet configuration to search service Do not limit integrator in possibilities to configure. Therefore previously configure facets for a field need to be adjusted to contain full configuration for elasticsearch. See changelog. Resolves: #120 --- Classes/Connection/Elasticsearch/Facet.php | 23 +++--- Classes/Connection/FacetRequestInterface.php | 10 +-- Classes/Domain/Model/FacetRequest.php | 24 ++---- Classes/Domain/Search/QueryFactory.php | 6 +- Classes/Domain/Search/SearchService.php | 7 +- Documentation/source/changelog.rst | 8 ++ .../20180406-120-facet-configuration.rst | 40 ++++++++++ Documentation/source/index.rst | 1 + .../Connection/Elasticsearch/FacetTest.php | 75 +++++++++++++++++++ .../Connection/Elasticsearch/FilterTest.php | 33 -------- Tests/Functional/Fixtures/BasicSetup.ts | 6 -- Tests/Functional/Fixtures/Searching/Facet.ts | 17 +++++ Tests/Unit/Domain/Search/QueryFactoryTest.php | 4 +- 13 files changed, 167 insertions(+), 87 deletions(-) create mode 100644 Documentation/source/changelog.rst create mode 100644 Documentation/source/changelog/20180406-120-facet-configuration.rst create mode 100644 Tests/Functional/Connection/Elasticsearch/FacetTest.php create mode 100644 Tests/Functional/Fixtures/Searching/Facet.ts diff --git a/Classes/Connection/Elasticsearch/Facet.php b/Classes/Connection/Elasticsearch/Facet.php index e24a659..1142d88 100644 --- a/Classes/Connection/Elasticsearch/Facet.php +++ b/Classes/Connection/Elasticsearch/Facet.php @@ -45,25 +45,26 @@ class Facet implements FacetInterface */ protected $options = []; - public function __construct($name, array $aggregation, ConfigurationContainerInterface $configuration) + public function __construct(string $name, array $aggregation, ConfigurationContainerInterface $configuration) { $this->name = $name; $this->buckets = $aggregation['buckets']; - $this->field = $configuration->getIfExists('searching.facets.' . $this->name . '.field') ?: ''; + + $config = $configuration->getIfExists('searching.facets.' . $this->name) ?: []; + foreach ($config as $configEntry) { + if (isset($configEntry['field'])) { + $this->field = $configEntry['field']; + break; + } + } } - /** - * @return string - */ - public function getName() + public function getName() : string { return $this->name; } - /** - * @return string - */ - public function getField() + public function getField() : string { return $this->field; } @@ -73,7 +74,7 @@ class Facet implements FacetInterface * * @return array */ - public function getOptions() + public function getOptions() : array { $this->initOptions(); diff --git a/Classes/Connection/FacetRequestInterface.php b/Classes/Connection/FacetRequestInterface.php index 9352f96..cdc16b1 100644 --- a/Classes/Connection/FacetRequestInterface.php +++ b/Classes/Connection/FacetRequestInterface.php @@ -28,15 +28,11 @@ interface FacetRequestInterface /** * The identifier of the facet, used as key in arrays and to get the facet * from search request, etc. - * - * @return string */ - public function getIdentifier(); + public function getIdentifier() : string; /** - * The field to use for facet building. - * - * @return string + * The config to use for facet building. */ - public function getField(); + public function getConfig() : array; } diff --git a/Classes/Domain/Model/FacetRequest.php b/Classes/Domain/Model/FacetRequest.php index b1dbc4c..0d82ad3 100644 --- a/Classes/Domain/Model/FacetRequest.php +++ b/Classes/Domain/Model/FacetRequest.php @@ -30,37 +30,27 @@ class FacetRequest implements FacetRequestInterface protected $identifier = ''; /** - * @var string + * @var array */ - protected $field = ''; + protected $config = []; /** - * TODO: Add validation / exception? * As the facets come from configuration this might be a good idea to help * integrators find issues. - * - * @param string $identifier - * @param string $field */ - public function __construct($identifier, $field) + public function __construct(string $identifier, array $config) { $this->identifier = $identifier; - $this->field = $field; + $this->config = $config; } - /** - * @return string - */ - public function getIdentifier() + public function getIdentifier() : string { return $this->identifier; } - /** - * @return string - */ - public function getField() + public function getConfig() : array { - return $this->field; + return $this->config; } } diff --git a/Classes/Domain/Search/QueryFactory.php b/Classes/Domain/Search/QueryFactory.php index 980b763..8e122ab 100644 --- a/Classes/Domain/Search/QueryFactory.php +++ b/Classes/Domain/Search/QueryFactory.php @@ -239,11 +239,7 @@ class QueryFactory foreach ($searchRequest->getFacets() as $facet) { $query = ArrayUtility::arrayMergeRecursiveOverrule($query, [ 'aggs' => [ - $facet->getIdentifier() => [ - 'terms' => [ - 'field' => $facet->getField(), - ], - ], + $facet->getIdentifier() => $facet->getConfig(), ], ]); } diff --git a/Classes/Domain/Search/SearchService.php b/Classes/Domain/Search/SearchService.php index 120ab3b..40c99bd 100644 --- a/Classes/Domain/Search/SearchService.php +++ b/Classes/Domain/Search/SearchService.php @@ -103,15 +103,10 @@ class SearchService } foreach ($facetsConfig as $identifier => $facetConfig) { - if (!isset($facetConfig['field']) || trim($facetConfig['field']) === '') { - // TODO: Finish throw - throw new \Exception('message', 1499171142); - } - $searchRequest->addFacet($this->objectManager->get( FacetRequest::class, $identifier, - $facetConfig['field'] + $facetConfig )); } } diff --git a/Documentation/source/changelog.rst b/Documentation/source/changelog.rst new file mode 100644 index 0000000..0d7f020 --- /dev/null +++ b/Documentation/source/changelog.rst @@ -0,0 +1,8 @@ +Changelog +========= + +.. toctree:: + :maxdepth: 1 + :glob: + + changelog/* diff --git a/Documentation/source/changelog/20180406-120-facet-configuration.rst b/Documentation/source/changelog/20180406-120-facet-configuration.rst new file mode 100644 index 0000000..53021d9 --- /dev/null +++ b/Documentation/source/changelog/20180406-120-facet-configuration.rst @@ -0,0 +1,40 @@ +Breacking Change 120 "Pass facets configuration to elasticsearch" +================================================================= + +In order to allow arbitrary facet configuration, we do not process the facet configuration anymore. +Instead integrators are able to configure facets for search service "as is". We just pipe the +configuration through. + +Therefore the following, which worked before, does not work anymore: + +.. code-block:: typoscript + :linenos: + :emphasize-lines: 4 + + plugin.tx_searchcore.settings.search { + facets { + category { + field = categories + } + } + } + +Instead you have to provide the full configuration yourself: + +.. code-block:: typoscript + :linenos: + :emphasize-lines: 4,6 + + plugin.tx_searchcore.settings.search { + facets { + category { + terms { + field = categories + } + } + } + } + +You need to add line 4 and 6, the additional level ``terms`` for elasticsearch. + +See :issue:`120`. diff --git a/Documentation/source/index.rst b/Documentation/source/index.rst index 5427fc9..2edfae3 100644 --- a/Documentation/source/index.rst +++ b/Documentation/source/index.rst @@ -15,3 +15,4 @@ Table of Contents connections indexer development + changelog diff --git a/Tests/Functional/Connection/Elasticsearch/FacetTest.php b/Tests/Functional/Connection/Elasticsearch/FacetTest.php new file mode 100644 index 0000000..c89bc10 --- /dev/null +++ b/Tests/Functional/Connection/Elasticsearch/FacetTest.php @@ -0,0 +1,75 @@ + + * + * 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 Codappix\SearchCore\Domain\Index\IndexerFactory; +use Codappix\SearchCore\Domain\Model\SearchRequest; +use Codappix\SearchCore\Domain\Search\SearchService; +use TYPO3\CMS\Extbase\Object\ObjectManager; + +class FacetTest extends AbstractFunctionalTestCase +{ + protected function getTypoScriptFilesForFrontendRootPage() + { + return array_merge(parent::getTypoScriptFilesForFrontendRootPage(), ['EXT:search_core/Tests/Functional/Fixtures/Searching/Facet.ts']); + } + + protected function getDataSets() + { + return array_merge( + parent::getDataSets(), + ['Tests/Functional/Fixtures/Searching/Filter.xml'] + ); + } + + /** + * @test + */ + public function itsPossibleToFetchFacetsForField() + { + \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(ObjectManager::class) + ->get(IndexerFactory::class) + ->getIndexer('tt_content') + ->indexAllDocuments() + ; + + $searchService = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(ObjectManager::class) + ->get(SearchService::class); + + $searchRequest = new SearchRequest(); + $result = $searchService->search($searchRequest); + + $this->assertSame(1, count($result->getFacets()), 'Did not receive the single defined facet.'); + + $facet = current($result->getFacets()); + $this->assertSame('contentTypes', $facet->getName(), 'Name of facet was not as expected.'); + $this->assertSame('CType', $facet->getField(), 'Field of facet was not expected.'); + + $options = $facet->getOptions(); + $this->assertSame(2, count($options), 'Did not receive the expected number of possible options for facet.'); + $option = $options['HTML']; + $this->assertSame('HTML', $option->getName(), 'Option did not have expected Name.'); + $this->assertSame(1, $option->getCount(), 'Option did not have expected count.'); + $option = $options['Header']; + $this->assertSame('Header', $option->getName(), 'Option did not have expected Name.'); + $this->assertSame(1, $option->getCount(), 'Option did not have expected count.'); + } +} diff --git a/Tests/Functional/Connection/Elasticsearch/FilterTest.php b/Tests/Functional/Connection/Elasticsearch/FilterTest.php index e364a4a..8bbe0f0 100644 --- a/Tests/Functional/Connection/Elasticsearch/FilterTest.php +++ b/Tests/Functional/Connection/Elasticsearch/FilterTest.php @@ -58,37 +58,4 @@ class FilterTest extends AbstractFunctionalTestCase $this->assertSame(5, $result->getResults()[0]['uid'], 'Did not get the expected result entry.'); $this->assertSame(1, count($result), 'Did not receive the single filtered element.'); } - - /** - * @test - */ - public function itsPossibleToFetchFacetsForField() - { - \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(ObjectManager::class) - ->get(IndexerFactory::class) - ->getIndexer('tt_content') - ->indexAllDocuments() - ; - - $searchService = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(ObjectManager::class) - ->get(SearchService::class); - - $searchRequest = new SearchRequest('Search Word'); - $result = $searchService->search($searchRequest); - - $this->assertSame(1, count($result->getFacets()), 'Did not receive the single defined facet.'); - - $facet = current($result->getFacets()); - $this->assertSame('contentTypes', $facet->getName(), 'Name of facet was not as expected.'); - $this->assertSame('CType', $facet->getField(), 'Field of facet was not expected.'); - - $options = $facet->getOptions(); - $this->assertSame(2, count($options), 'Did not receive the expected number of possible options for facet.'); - $option = $options['HTML']; - $this->assertSame('HTML', $option->getName(), 'Option did not have expected Name.'); - $this->assertSame(1, $option->getCount(), 'Option did not have expected count.'); - $option = $options['Header']; - $this->assertSame('Header', $option->getName(), 'Option did not have expected Name.'); - $this->assertSame(1, $option->getCount(), 'Option did not have expected count.'); - } } diff --git a/Tests/Functional/Fixtures/BasicSetup.ts b/Tests/Functional/Fixtures/BasicSetup.ts index b7b0c6a..4129362 100644 --- a/Tests/Functional/Fixtures/BasicSetup.ts +++ b/Tests/Functional/Fixtures/BasicSetup.ts @@ -37,12 +37,6 @@ plugin { } searching { - facets { - contentTypes { - field = CType - } - } - fields { query = _all } diff --git a/Tests/Functional/Fixtures/Searching/Facet.ts b/Tests/Functional/Fixtures/Searching/Facet.ts new file mode 100644 index 0000000..10eeae6 --- /dev/null +++ b/Tests/Functional/Fixtures/Searching/Facet.ts @@ -0,0 +1,17 @@ +plugin { + tx_searchcore { + settings { + searching { + facets { + contentTypes { + terms { + field = CType + } + } + } + } + } + } +} + +module.tx_searchcore < plugin.tx_searchcore diff --git a/Tests/Unit/Domain/Search/QueryFactoryTest.php b/Tests/Unit/Domain/Search/QueryFactoryTest.php index e0fc86d..881cd8b 100644 --- a/Tests/Unit/Domain/Search/QueryFactoryTest.php +++ b/Tests/Unit/Domain/Search/QueryFactoryTest.php @@ -118,8 +118,8 @@ class QueryFactoryTest extends AbstractUnitTestCase { $this->configureConfigurationMockWithDefault(); $searchRequest = new SearchRequest('SearchWord'); - $searchRequest->addFacet(new FacetRequest('Identifier', 'FieldName')); - $searchRequest->addFacet(new FacetRequest('Identifier 2', 'FieldName 2')); + $searchRequest->addFacet(new FacetRequest('Identifier', ['terms' => ['field' => 'FieldName']])); + $searchRequest->addFacet(new FacetRequest('Identifier 2', ['terms' => ['field' => 'FieldName 2']])); $query = $this->subject->create($searchRequest); $this->assertSame( From a893303939178090c03f60ca6aed61673962e7e8 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 6 Mar 2018 13:04:42 +0100 Subject: [PATCH 17/22] TASK: Improve ResultItemTest Do not repeat content, use variable which is also better to read. And do not add unnecessary, unused, variables. --- Tests/Unit/Domain/Model/ResultItemTest.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Tests/Unit/Domain/Model/ResultItemTest.php b/Tests/Unit/Domain/Model/ResultItemTest.php index df11a5d..53477c7 100644 --- a/Tests/Unit/Domain/Model/ResultItemTest.php +++ b/Tests/Unit/Domain/Model/ResultItemTest.php @@ -57,7 +57,7 @@ class ResultItemTest extends AbstractUnitTestCase $subject = new ResultItem($originalData); $this->assertSame( - 'Some title', + $originalData['title'], $subject['title'], 'Could not retrieve title in array notation.' ); @@ -72,7 +72,6 @@ class ResultItemTest extends AbstractUnitTestCase 'uid' => 10, 'title' => 'Some title', ]; - $expectedData = $originalData; $subject = new ResultItem($originalData); $this->assertTrue(isset($subject['title']), 'Could not determine that title exists.'); @@ -88,7 +87,6 @@ class ResultItemTest extends AbstractUnitTestCase 'uid' => 10, 'title' => 'Some title', ]; - $expectedData = $originalData; $subject = new ResultItem($originalData); $this->expectException(\BadMethodCallException::class); @@ -104,7 +102,6 @@ class ResultItemTest extends AbstractUnitTestCase 'uid' => 10, 'title' => 'Some title', ]; - $expectedData = $originalData; $subject = new ResultItem($originalData); $this->expectException(\BadMethodCallException::class); From 6456f315030fd6a60f007816ac0155328d92db4a Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 6 Mar 2018 13:23:08 +0100 Subject: [PATCH 18/22] TASK: Make test more readable Make sure everyone knows what we compare, do not add hardcoded information. --- Tests/Unit/Domain/Model/SearchResultTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tests/Unit/Domain/Model/SearchResultTest.php b/Tests/Unit/Domain/Model/SearchResultTest.php index 910e06e..eebb3c1 100644 --- a/Tests/Unit/Domain/Model/SearchResultTest.php +++ b/Tests/Unit/Domain/Model/SearchResultTest.php @@ -89,9 +89,9 @@ class SearchResultTest extends AbstractUnitTestCase $this->assertCount(3, $resultItems); - $this->assertSame(10, $resultItems[0]['uid']); - $this->assertSame(11, $resultItems[1]['uid']); - $this->assertSame(12, $resultItems[2]['uid']); + $this->assertSame($data[0]['uid'], $resultItems[0]['uid']); + $this->assertSame($data[1]['uid'], $resultItems[1]['uid']); + $this->assertSame($data[2]['uid'], $resultItems[2]['uid']); $this->assertInstanceOf(ResultItemInterface::class, $resultItems[0]); $this->assertInstanceOf(ResultItemInterface::class, $resultItems[1]); From 3731bcf4743821a6906078a9f546848294f56b00 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 6 Mar 2018 16:39:07 +0100 Subject: [PATCH 19/22] TASK: Fix CGL --- Classes/Domain/Search/SearchService.php | 3 +-- Tests/Unit/Domain/Search/SearchServiceTest.php | 12 ++++++++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/Classes/Domain/Search/SearchService.php b/Classes/Domain/Search/SearchService.php index a331ab4..03fc11b 100644 --- a/Classes/Domain/Search/SearchService.php +++ b/Classes/Domain/Search/SearchService.php @@ -168,8 +168,7 @@ class SearchService $searchResult, $newSearchResultItems ); - } - catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException $e) { return $searchResult; } } diff --git a/Tests/Unit/Domain/Search/SearchServiceTest.php b/Tests/Unit/Domain/Search/SearchServiceTest.php index 6263f23..3d6791e 100644 --- a/Tests/Unit/Domain/Search/SearchServiceTest.php +++ b/Tests/Unit/Domain/Search/SearchServiceTest.php @@ -270,7 +270,11 @@ class SearchServiceTest extends AbstractUnitTestCase $this->dataProcessorService->expects($this->never())->method('executeDataProcessor'); $searchRequest = new SearchRequest(''); - $this->assertSame($searchResultMock, $this->subject->search($searchRequest), 'Did not get created result without applied data processing'); + $this->assertSame( + $searchResultMock, + $this->subject->search($searchRequest), + 'Did not get created result without applied data processing' + ); } /** @@ -312,6 +316,10 @@ class SearchServiceTest extends AbstractUnitTestCase ->willReturn($searchResultMock); $searchRequest = new SearchRequest(''); - $this->assertSame($searchResultMock, $this->subject->search($searchRequest), 'Did not get created result with applied data processing'); + $this->assertSame( + $searchResultMock, + $this->subject->search($searchRequest), + 'Did not get created result with applied data processing' + ); } } From 6544ec07d315a6d3b1624d9a5add948d078e14cb Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 6 Mar 2018 15:56:14 +0100 Subject: [PATCH 20/22] FEATURE: Support display name for facet option As some search services, like elasticsearch, allow generation of a string that should be displayed in frontend, we provide a new getter for that. The old existing name can be a fallback in custom implementations. --- .../Connection/Elasticsearch/FacetOption.php | 21 +++--- Classes/Connection/FacetOptionInterface.php | 14 ++-- .../Elasticsearch/FacetOptionTest.php | 64 +++++++++++++++++++ 3 files changed, 85 insertions(+), 14 deletions(-) create mode 100644 Tests/Unit/Connection/Elasticsearch/FacetOptionTest.php diff --git a/Classes/Connection/Elasticsearch/FacetOption.php b/Classes/Connection/Elasticsearch/FacetOption.php index 7359434..6b544bb 100644 --- a/Classes/Connection/Elasticsearch/FacetOption.php +++ b/Classes/Connection/Elasticsearch/FacetOption.php @@ -29,6 +29,11 @@ class FacetOption implements FacetOptionInterface */ protected $name = ''; + /** + * @var string + */ + protected $displayName = ''; + /** * @var int */ @@ -40,21 +45,21 @@ class FacetOption implements FacetOptionInterface public function __construct(array $bucket) { $this->name = $bucket['key']; + $this->displayName = isset($bucket['key_as_string']) ? $bucket['key_as_string'] : $this->getName(); $this->count = $bucket['doc_count']; } - /** - * @return string - */ - public function getName() + public function getName() : string { return $this->name; } - /** - * @return int - */ - public function getCount() + public function getDisplayName() : string + { + return $this->displayName; + } + + public function getCount() : int { return $this->count; } diff --git a/Classes/Connection/FacetOptionInterface.php b/Classes/Connection/FacetOptionInterface.php index 51a1efd..bf998db 100644 --- a/Classes/Connection/FacetOptionInterface.php +++ b/Classes/Connection/FacetOptionInterface.php @@ -28,15 +28,17 @@ interface FacetOptionInterface /** * Returns the name of this option. Equivalent * to value used for filtering. - * - * @return string */ - public function getName(); + public function getName() : string; + + /** + * If a pre-rendered name is provided, this will be returned. + * Otherwise it's the same as getName(). + */ + public function getDisplayName() : string; /** * Returns the number of found results for this option. - * - * @return int */ - public function getCount(); + public function getCount() : int; } diff --git a/Tests/Unit/Connection/Elasticsearch/FacetOptionTest.php b/Tests/Unit/Connection/Elasticsearch/FacetOptionTest.php new file mode 100644 index 0000000..696762c --- /dev/null +++ b/Tests/Unit/Connection/Elasticsearch/FacetOptionTest.php @@ -0,0 +1,64 @@ + + * + * 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 Codappix\SearchCore\Connection\Elasticsearch\FacetOption; +use Codappix\SearchCore\Tests\Unit\AbstractUnitTestCase; + +class FacetOptionTest extends AbstractUnitTestCase +{ + /** + * @test + */ + public function displayNameIsReturnedAsExpected() + { + $bucket = [ + 'key' => 'Name', + 'key_as_string' => 'DisplayName', + 'doc_count' => 10, + ]; + $subject = new FacetOption($bucket); + + $this->assertSame( + $bucket['key_as_string'], + $subject->getDisplayName(), + 'Display name was not returned as expected.' + ); + } + + /** + * @test + */ + public function displayNameIsReturnedAsExpectedIfNotProvided() + { + $bucket = [ + 'key' => 'Name', + 'doc_count' => 10, + ]; + $subject = new FacetOption($bucket); + + $this->assertSame( + $bucket['key'], + $subject->getDisplayName(), + 'Display name was not returned as expected.' + ); + } +} From 88f301f22888f0b3ce411dcdc1b2af7252005296 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 6 Mar 2018 16:34:59 +0100 Subject: [PATCH 21/22] FEATURE: Allow range queries for elasticsearch Allow "raw" configuration and support "range" type. Also prevent adding boosts if no search term was submitted which can be boosted. Resolves: #119 --- Classes/Domain/Search/QueryFactory.php | 16 ++++++ .../source/configuration/searching.rst | 47 ++++++++++++++--- Tests/Unit/Domain/Search/QueryFactoryTest.php | 52 +++++++++++++++++++ 3 files changed, 107 insertions(+), 8 deletions(-) diff --git a/Classes/Domain/Search/QueryFactory.php b/Classes/Domain/Search/QueryFactory.php index 978d88a..dcf6ce5 100644 --- a/Classes/Domain/Search/QueryFactory.php +++ b/Classes/Domain/Search/QueryFactory.php @@ -121,6 +121,10 @@ class QueryFactory return; } + if (trim($searchRequest->getSearchTerm()) === '') { + return; + } + $boostQueryParts = []; foreach ($fields as $fieldName => $boostValue) { @@ -238,6 +242,18 @@ class QueryFactory } } + if (isset($config['raw'])) { + $filter = array_merge($config['raw'], $filter); + } + + if ($config['type'] === 'range') { + return [ + 'range' => [ + $config['field'] => $filter, + ], + ]; + } + return [$config['field'] => $filter]; } diff --git a/Documentation/source/configuration/searching.rst b/Documentation/source/configuration/searching.rst index bfed51e..857aa74 100644 --- a/Documentation/source/configuration/searching.rst +++ b/Documentation/source/configuration/searching.rst @@ -119,20 +119,48 @@ E.g. you submit a filter in form of: .. code-block:: html - - - + + Due to TYPO3 7.x fluid limitations, we build this input ourself. + No longer necessary in 8 and above + + + This will create a ``distance`` filter with subproperties. To make this filter actually work, you can add the following TypoScript, which will be added to the filter:: mapping { filter { - distance { - field = geo_distance + month { + type = range + field = released + raw { + format = yyyy-MM + } + fields { - distance = distance - location = location + gte = from + lte = to } } } @@ -143,9 +171,12 @@ in elasticsearch. In above example they do match, but you can also use different On the left hand side is the elasticsearch field name, on the right side the one submitted as a filter. -The ``field``, in above example ``geo_distance``, will be used as the elasticsearch field for +The ``field``, in above example ``released``, will be used as the elasticsearch field for filtering. This way you can use arbitrary filter names and map them to existing elasticsearch fields. +Everything that is configured inside ``raw`` is passed, as is, to search service, e.g. +elasticsearch. + .. _fields: fields diff --git a/Tests/Unit/Domain/Search/QueryFactoryTest.php b/Tests/Unit/Domain/Search/QueryFactoryTest.php index 881cd8b..dc824c5 100644 --- a/Tests/Unit/Domain/Search/QueryFactoryTest.php +++ b/Tests/Unit/Domain/Search/QueryFactoryTest.php @@ -86,6 +86,58 @@ class QueryFactoryTest extends AbstractUnitTestCase ); } + /** + * @test + */ + public function rangeFilterIsAddedToQuery() + { + $this->configureConfigurationMockWithDefault(); + $this->configuration->expects($this->any()) + ->method('getIfExists') + ->will($this->returnCallback(function ($configName) { + if ($configName === 'searching.mapping.filter.month') { + return [ + 'type' => 'range', + 'field' => 'released', + 'raw' => [ + 'format' => 'yyyy-MM', + ], + 'fields' => [ + 'gte' => 'from', + 'lte' => 'to', + ], + ]; + } + + return []; + })); + + $searchRequest = new SearchRequest('SearchWord'); + $searchRequest->setFilter([ + 'month' => [ + 'from' => '2016-03', + 'to' => '2017-11', + ], + ]); + + $query = $this->subject->create($searchRequest); + $this->assertSame( + [ + [ + 'range' => [ + 'released' => [ + 'format' => 'yyyy-MM', + 'gte' => '2016-03', + 'lte' => '2017-11', + ], + ], + ] + ], + $query->toArray()['query']['bool']['filter'], + 'Filter was not added to query.' + ); + } + /** * @test */ From 30a833ce68c5af08a97667a6e0b35b06ccb3aaa0 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 8 Mar 2018 08:23:37 +0100 Subject: [PATCH 22/22] TASK: Add branch alias to provide version number Allow to require extension with 1.0.0, if dev is allowed. --- composer.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/composer.json b/composer.json index a630385..f58a062 100644 --- a/composer.json +++ b/composer.json @@ -37,6 +37,9 @@ ] }, "extra": { + "branch-alias": { + "dev-develop": "1.0.x-dev" + }, "typo3/cms": { "cms-package-dir": "{$vendor-dir}/typo3/cms", "web-dir": ".Build/web"