From 9769ee1cb68b465c5a845d79bb3a185185aa9aef Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Mon, 3 Jul 2017 22:59:18 +0200 Subject: [PATCH 01/34] TASK: Update composer Require TYPO3 CMS 8.2 and PHP 7.1 as minimum. Use conventionally lower case web folder. --- composer.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/composer.json b/composer.json index debb71f..2aba101 100644 --- a/composer.json +++ b/composer.json @@ -16,8 +16,8 @@ } }, "require" : { - "php": ">=5.6.0", - "typo3/cms": "~7.6", + "php": ">=7.1.0", + "typo3/cms": "~8.2", "ruflin/elastica": "~3.2" }, "require-dev": { @@ -30,14 +30,14 @@ }, "scripts": { "post-autoload-dump": [ - "mkdir -p .Build/Web/typo3conf/ext/", - "[ -L .Build/Web/typo3conf/ext/search_core ] || ln -snvf ../../../../. .Build/Web/typo3conf/ext/search_core" + "mkdir -p .Build/web/typo3conf/ext/", + "[ -L .Build/web/typo3conf/ext/search_core ] || ln -snvf ../../../../. .Build/web/typo3conf/ext/search_core" ] }, "extra": { "typo3/cms": { "cms-package-dir": "{$vendor-dir}/typo3/cms", - "web-dir": ".Build/Web" + "web-dir": ".Build/web" } }, "authors": [ From 9d20524706053368ff9c0e82fc839cbbbb4cd16a Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 4 Jul 2017 10:12:47 +0200 Subject: [PATCH 02/34] WIP|TASK: Use new TYPO3 Use testing framework configuration. Use new db. Begin with replacement of old TYPO3_DB. --- Classes/Domain/Index/TcaIndexer.php | 10 ++++++++-- Makefile | 8 +++++--- Tests/Functional/FunctionalTests.xml | 2 +- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/Classes/Domain/Index/TcaIndexer.php b/Classes/Domain/Index/TcaIndexer.php index b50c6fa..3abdcb3 100644 --- a/Classes/Domain/Index/TcaIndexer.php +++ b/Classes/Domain/Index/TcaIndexer.php @@ -112,7 +112,7 @@ class TcaIndexer implements IndexerInterface */ protected function getRecords($offset, $limit) { - $records = $GLOBALS['TYPO3_DB']->exec_SELECTgetRows( + $records = $this->getDatabaseConnection()->exec_SELECTgetRows( $this->tcaTableService->getFields(), $this->tcaTableService->getTableClause(), $this->tcaTableService->getWhereClause(), @@ -139,7 +139,7 @@ class TcaIndexer implements IndexerInterface */ protected function getRecord($identifier) { - $record = $GLOBALS['TYPO3_DB']->exec_SELECTgetSingleRow( + $record = $this->getDatabaseConnection()->exec_SELECTgetSingleRow( $this->tcaTableService->getFields(), $this->tcaTableService->getTableClause(), $this->tcaTableService->getWhereClause() @@ -156,4 +156,10 @@ class TcaIndexer implements IndexerInterface return $record; } + + protected function getDatabaseConnection() + { + return GeneralUtility::makeInstance(ConnectionPool::class) + ->getConnectionByName('Default'); + } } diff --git a/Makefile b/Makefile index 589dd7d..566709a 100644 --- a/Makefile +++ b/Makefile @@ -1,10 +1,11 @@ mkfile_path := $(abspath $(lastword $(MAKEFILE_LIST))) current_dir := $(dir $(mkfile_path)) -TYPO3_WEB_DIR := $(current_dir).Build/Web +TYPO3_WEB_DIR := $(current_dir).Build/web +TYPO3_PATH_ROOT := $(current_dir).Build/web # Allow different versions on travis -TYPO3_VERSION ?= ~7.6 -typo3DatabaseName ?= "searchcore_test" +TYPO3_VERSION ?= ~8.2 +typo3DatabaseName ?= "searchcore_test2" typo3DatabaseUsername ?= "dev" typo3DatabasePassword ?= "dev" typo3DatabaseHost ?= "127.0.0.1" @@ -21,6 +22,7 @@ functionalTests: typo3DatabaseHost=$(typo3DatabaseHost) \ TYPO3_PATH_WEB=$(TYPO3_WEB_DIR) \ .Build/bin/phpunit --colors --debug -v \ + --stop-on-error \ -c Tests/Functional/FunctionalTests.xml uploadCodeCoverage: uploadCodeCoverageToScrutinizer uploadCodeCoverageToCodacy diff --git a/Tests/Functional/FunctionalTests.xml b/Tests/Functional/FunctionalTests.xml index d42ef21..a318f65 100644 --- a/Tests/Functional/FunctionalTests.xml +++ b/Tests/Functional/FunctionalTests.xml @@ -1,7 +1,7 @@ Date: Fri, 7 Jul 2017 12:03:06 +0200 Subject: [PATCH 03/34] TASK: Migrate dev dependencies As testing framework is used, we can prefer dist again to speed up composer installation. --- Makefile | 4 ++-- Tests/Unit/UnitTests.xml | 2 +- composer.json | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index e8177b9..87bcdfc 100644 --- a/Makefile +++ b/Makefile @@ -4,7 +4,7 @@ current_dir := $(dir $(mkfile_path)) TYPO3_WEB_DIR := $(current_dir).Build/web TYPO3_PATH_ROOT := $(current_dir).Build/web # Allow different versions on travis -TYPO3_VERSION ?= ~8.2 +TYPO3_VERSION ?= ~8.7 typo3DatabaseName ?= "searchcore_test2" typo3DatabaseUsername ?= "dev" typo3DatabasePassword ?= "dev" @@ -12,7 +12,7 @@ typo3DatabaseHost ?= "127.0.0.1" .PHONY: install install: clean - COMPOSER_PROCESS_TIMEOUT=1000 composer require -vv --dev --prefer-source --ignore-platform-reqs typo3/cms="$(TYPO3_VERSION)" + COMPOSER_PROCESS_TIMEOUT=1000 composer require -vv --dev --prefer-dist --ignore-platform-reqs typo3/cms="$(TYPO3_VERSION)" git checkout composer.json functionalTests: diff --git a/Tests/Unit/UnitTests.xml b/Tests/Unit/UnitTests.xml index 6456405..6486594 100644 --- a/Tests/Unit/UnitTests.xml +++ b/Tests/Unit/UnitTests.xml @@ -1,7 +1,7 @@ Date: Fri, 7 Jul 2017 12:19:35 +0200 Subject: [PATCH 04/34] BUGFIX: Allow tests to run without database connection Ad TYPO3 Core now makes use of Doctrine, a connection is required to build system where. Therefore we move it to an own method to exchange the execution inside of tests. --- .../Index/TcaIndexer/TcaTableService.php | 26 ++++++++++++------- .../Index/TcaIndexer/TcaTableServiceTest.php | 6 +++++ 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/Classes/Domain/Index/TcaIndexer/TcaTableService.php b/Classes/Domain/Index/TcaIndexer/TcaTableService.php index 21e6374..65adf70 100644 --- a/Classes/Domain/Index/TcaIndexer/TcaTableService.php +++ b/Classes/Domain/Index/TcaIndexer/TcaTableService.php @@ -142,15 +142,7 @@ class TcaTableService */ public function getWhereClause() { - $whereClause = '1=1' - . BackendUtility::BEenableFields($this->tableName) - . BackendUtility::deleteClause($this->tableName) - - . BackendUtility::BEenableFields('pages') - . BackendUtility::deleteClause('pages') - . ' AND pages.no_search = 0' - ; - + $whereClause = $this->getSystemWhereClause(); $userDefinedWhere = $this->configuration->getIfExists('indexing.' . $this->getTableName() . '.additionalWhereClause'); if (is_string($userDefinedWhere)) { $whereClause .= ' AND ' . $userDefinedWhere; @@ -169,6 +161,22 @@ class TcaTableService return $whereClause; } + /** + * Generate SQL for TYPO3 as a system, to make sure only available records + * are fetched. + */ + public function getSystemWhereClause() : string + { + return '1=1' + . BackendUtility::BEenableFields($this->tableName) + . BackendUtility::deleteClause($this->tableName) + + . BackendUtility::BEenableFields('pages') + . BackendUtility::deleteClause('pages') + . ' AND pages.no_search = 0' + ; + } + /** * @return string */ diff --git a/Tests/Unit/Domain/Index/TcaIndexer/TcaTableServiceTest.php b/Tests/Unit/Domain/Index/TcaIndexer/TcaTableServiceTest.php index 9e88d4b..330c16e 100644 --- a/Tests/Unit/Domain/Index/TcaIndexer/TcaTableServiceTest.php +++ b/Tests/Unit/Domain/Index/TcaIndexer/TcaTableServiceTest.php @@ -60,6 +60,9 @@ class TcaTableServiceTest extends AbstractUnitTestCase ->method('getIfExists') ->withConsecutive(['indexing.table.additionalWhereClause'], ['indexing.table.rootLineBlacklist']) ->will($this->onConsecutiveCalls(null, false)); + $this->subject->expects($this->once()) + ->method('getSystemWhereClause') + ->will($this->returnValue('1=1 AND pages.no_search = 0')); $this->assertSame( '1=1 AND pages.no_search = 0', @@ -76,6 +79,9 @@ class TcaTableServiceTest extends AbstractUnitTestCase ->method('getIfExists') ->withConsecutive(['indexing.table.additionalWhereClause'], ['indexing.table.rootLineBlacklist']) ->will($this->onConsecutiveCalls('table.field = "someValue"', false)); + $this->subject->expects($this->once()) + ->method('getSystemWhereClause') + ->will($this->returnValue('1=1 AND pages.no_search = 0')); $this->assertSame( '1=1 AND pages.no_search = 0 AND table.field = "someValue"', From cf902dde83f64aac4a3326810012d38f69f5f622 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Fri, 7 Jul 2017 14:44:32 +0200 Subject: [PATCH 05/34] TASK: Make extension more compatible Migrate sql to doctrine. Migrate relation resolver to use new API. --- Classes/Database/Doctrine/Join.php | 50 ++++++++++ Classes/Database/Doctrine/Where.php | 53 +++++++++++ Classes/Domain/Index/TcaIndexer.php | 29 ++++-- .../Index/TcaIndexer/RelationResolver.php | 94 ++++--------------- .../Index/TcaIndexer/TcaTableService.php | 90 +++++++++--------- Makefile | 1 - .../Connection/Elasticsearch/FilterTest.php | 2 +- .../Elasticsearch/IndexTcaTableTest.php | 2 +- .../Index/TcaIndexer/TcaTableServiceTest.php | 14 ++- 9 files changed, 202 insertions(+), 133 deletions(-) create mode 100644 Classes/Database/Doctrine/Join.php create mode 100644 Classes/Database/Doctrine/Where.php diff --git a/Classes/Database/Doctrine/Join.php b/Classes/Database/Doctrine/Join.php new file mode 100644 index 0000000..df1a8c6 --- /dev/null +++ b/Classes/Database/Doctrine/Join.php @@ -0,0 +1,50 @@ + + * + * 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. + */ + +class Join +{ + /** + * @var string + */ + protected $table = ''; + + /** + * @var string + */ + protected $condition = ''; + + public function __construct(string $table, string $condition) + { + $this->table = $table; + $this->condition = $condition; + } + + public function getTable() : string + { + return $this->table; + } + + public function getCondition() : string + { + return $this->condition; + } +} diff --git a/Classes/Database/Doctrine/Where.php b/Classes/Database/Doctrine/Where.php new file mode 100644 index 0000000..3398991 --- /dev/null +++ b/Classes/Database/Doctrine/Where.php @@ -0,0 +1,53 @@ + + * + * 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. + */ + +/** + * + */ +class Where +{ + /** + * @var string + */ + protected $statement = ''; + + /** + * @var array + */ + protected $parameters = []; + + public function __construct(string $statement, array $parameters) + { + $this->statement = $statement; + $this->parameters = $parameters; + } + + public function getStatement() : string + { + return $this->statement; + } + + public function getParameters() : array + { + return $this->parameters; + } +} diff --git a/Classes/Domain/Index/TcaIndexer.php b/Classes/Domain/Index/TcaIndexer.php index c0a77a1..18444ce 100644 --- a/Classes/Domain/Index/TcaIndexer.php +++ b/Classes/Domain/Index/TcaIndexer.php @@ -21,6 +21,8 @@ namespace Codappix\SearchCore\Domain\Index; */ use Codappix\SearchCore\Connection\ConnectionInterface; +use TYPO3\CMS\Core\Database\ConnectionPool; +use TYPO3\CMS\Core\Utility\GeneralUtility; /** * Will index the given table using configuration from TCA. @@ -51,14 +53,22 @@ class TcaIndexer extends AbstractIndexer */ protected function getRecords($offset, $limit) { - $records = $this->getDatabaseConnection()->exec_SELECTgetRows( - $this->tcaTableService->getFields(), - $this->tcaTableService->getTableClause(), - $this->tcaTableService->getWhereClause(), - '', - '', - (int) $offset . ',' . (int) $limit - ); + $queryBuilder = $this->getDatabaseConnection()->getQueryBuilderForTable($this->tcaTableService->getTableName()); + $where = $this->tcaTableService->getWhereClause(); + $query = $queryBuilder->select(... $this->tcaTableService->getFields()) + ->from($this->tcaTableService->getTableClause()) + ->where($where->getStatement()) + ->setParameters($where->getParameters()) + ->setFirstResult($offset) + ->setMaxResults($limit); + + foreach ($this->tcaTableService->getJoins() as $join) { + $query->from($join->getTable()); + $query->andWhere($join->getCondition()); + } + + $records = $query->execute()->fetchAll(); + if ($records === null) { return null; } @@ -106,7 +116,6 @@ class TcaIndexer extends AbstractIndexer protected function getDatabaseConnection() { - return GeneralUtility::makeInstance(ConnectionPool::class) - ->getConnectionByName('Default'); + return GeneralUtility::makeInstance(ConnectionPool::class); } } diff --git a/Classes/Domain/Index/TcaIndexer/RelationResolver.php b/Classes/Domain/Index/TcaIndexer/RelationResolver.php index b09e483..93cfffb 100644 --- a/Classes/Domain/Index/TcaIndexer/RelationResolver.php +++ b/Classes/Domain/Index/TcaIndexer/RelationResolver.php @@ -42,38 +42,31 @@ class RelationResolver implements Singleton * @param TcaTableService $service * @param array $record */ - public function resolveRelationsForRecord(TcaTableService $service, array &$record) + public function resolveRelationsForRecord(TcaTableService $service, array &$record) : void { - $formData = GeneralUtility::makeInstance( - FormDataCompiler::class, - GeneralUtility::makeInstance(TcaDatabaseRecord::class) - )->compile([ - 'tableName' => $service->getTableName(), - 'vanillaUid' => (int)$record['uid'], - 'command' => 'edit', - ]); - $record = $formData['databaseRow']; - foreach (array_keys($record) as $column) { + // TODO: Define / configure fields to exclude?! + if ($column === 'pid') { + continue; + } + $record[$column] = BackendUtility::getProcessedValueExtra($service->getTableName(), $column, $record[$column], 0, $record['uid']); + try { $config = $service->getColumnConfig($column); + + if ($this->isRelation($config)) { + $record[$column] = $this->resolveValue($record[$column], $config); + } } catch (InvalidArgumentException $e) { // Column is not configured. - continue; } - - if (! $this->isRelation($config) || !is_array($formData['processedTca']['columns'][$column])) { - continue; - } - - $record[$column] = $this->resolveValue($record[$column], $formData['processedTca']['columns'][$column]); } } /** * Resolve the given value from TYPO3 API response. * - * @param string $value The value from FormEngine to resolve. + * @param mixed $value The value from FormEngine to resolve. * @param array $tcaColumn The tca config of the relation. * * @return array|string @@ -83,15 +76,10 @@ class RelationResolver implements Singleton if ($value === '' || $value === '0') { return ''; } - if ($tcaColumn['config']['type'] === 'select') { - return $this->resolveSelectValue($value, $tcaColumn); - } - if ($tcaColumn['config']['type'] === 'group' && strpos($value, '|') !== false) { + + if ($tcaColumn['type'] === 'select' || $tcaColumn['type'] === 'group') { return $this->resolveForeignDbValue($value); } - if ($tcaColumn['config']['type'] === 'inline') { - return $this->resolveInlineValue($tcaColumn); - } return ''; } @@ -100,66 +88,24 @@ class RelationResolver implements Singleton * @param array Column config. * @return bool */ - protected function isRelation(array &$config) + protected function isRelation(array &$config) : bool { return isset($config['foreign_table']) - || (isset($config['items']) && is_array($config['items'])) + || (isset($config['renderType']) && $config['renderType'] !== 'selectSingle') || (isset($config['internal_type']) && strtolower($config['internal_type']) === 'db') ; } - /** - * Resolves internal representation of select to array of labels. - * - * @param array $value - * @param array $tcaColumn - * @return array - */ - protected function resolveSelectValue(array $values, array $tcaColumn) - { - $resolvedValues = []; - - foreach ($tcaColumn['config']['items'] as $item) { - if (in_array($item[1], $values)) { - $resolvedValues[] = $item[0]; - } - } - - if ($tcaColumn['config']['renderType'] === 'selectSingle' || $tcaColumn['config']['maxitems'] === 1) { - return current($resolvedValues); - } - - return $resolvedValues; - } - /** * @param string $value * * @return array */ - protected function resolveForeignDbValue($value) + protected function resolveForeignDbValue(string $value) : array { - $titles = []; - - foreach (explode(',', urldecode($value)) as $title) { - $titles[] = explode('|', $title)[1]; + if ($value === 'N/A') { + return []; } - - return $titles; - } - - /** - * @param array $tcaColumn - * @return array - */ - protected function resolveInlineValue(array $tcaColumn) - { - $titles = []; - - foreach ($tcaColumn['children'] as $selected) { - $titles[] = $selected['recordTitle']; - } - - return $titles; + return array_map('trim', explode(';', $value)); } } diff --git a/Classes/Domain/Index/TcaIndexer/TcaTableService.php b/Classes/Domain/Index/TcaIndexer/TcaTableService.php index 65adf70..77b7578 100644 --- a/Classes/Domain/Index/TcaIndexer/TcaTableService.php +++ b/Classes/Domain/Index/TcaIndexer/TcaTableService.php @@ -21,6 +21,8 @@ namespace Codappix\SearchCore\Domain\Index\TcaIndexer; */ use Codappix\SearchCore\Configuration\ConfigurationContainerInterface; +use Codappix\SearchCore\Database\Doctrine\Join; +use Codappix\SearchCore\Database\Doctrine\Where; use Codappix\SearchCore\Domain\Index\IndexingException; use TYPO3\CMS\Backend\Utility\BackendUtility; use TYPO3\CMS\Core\Utility\GeneralUtility; @@ -92,7 +94,7 @@ class TcaTableService /** * @return string */ - public function getTableName() + public function getTableName() : string { return $this->tableName; } @@ -100,9 +102,9 @@ class TcaTableService /** * @return string */ - public function getTableClause() + public function getTableClause() : string { - return $this->tableName . ' LEFT JOIN pages on ' . $this->tableName . '.pid = pages.uid'; + return $this->tableName; } /** @@ -111,7 +113,7 @@ class TcaTableService * @param array &$records * @return void */ - public function filterRecordsByRootLineBlacklist(array &$records) + public function filterRecordsByRootLineBlacklist(array &$records) : void { $records = array_filter( $records, @@ -125,7 +127,7 @@ class TcaTableService * Adjust record accordingly to configuration. * @param array &$record */ - public function prepareRecord(array &$record) + public function prepareRecord(array &$record) : void { $this->relationResolver->resolveRelationsForRecord($this, $record); @@ -137,11 +139,9 @@ class TcaTableService } } - /** - * @return string - */ - public function getWhereClause() + public function getWhereClause() : Where { + $parameters = []; $whereClause = $this->getSystemWhereClause(); $userDefinedWhere = $this->configuration->getIfExists('indexing.' . $this->getTableName() . '.additionalWhereClause'); if (is_string($userDefinedWhere)) { @@ -149,16 +149,41 @@ class TcaTableService } if ($this->isBlacklistedRootLineConfigured()) { - $whereClause .= ' AND pages.uid NOT IN (' - . implode(',', $this->getBlacklistedRootLine()) - . ')' - . ' AND pages.pid NOT IN (' - . implode(',', $this->getBlacklistedRootLine()) - . ')'; + $parameters[':blacklistedRootLine'] = $this->getBlacklistedRootLine(); + $whereClause .= ' AND pages.uid NOT IN (:blacklistedRootLine)' + . ' AND pages.pid NOT IN (:blacklistedRootLine)'; } $this->logger->debug('Generated where clause.', [$this->tableName, $whereClause]); - return $whereClause; + return new Where($whereClause, $parameters); + } + + public function getFields() : array + { + $fields = array_merge( + ['uid','pid'], + array_filter( + array_keys($this->tca['columns']), + function ($columnName) { + return !$this->isSystemField($columnName); + } + ) + ); + + foreach ($fields as $key => $field) { + $fields[$key] = $this->tableName . '.' . $field; + } + + $this->logger->debug('Generated fields.', [$this->tableName, $fields]); + return $fields; + return implode(', ', $fields); + } + + public function getJoins() : array + { + return [ + new Join('pages', 'pages.uid = ' . $this->tableName . '.pid'), + ]; } /** @@ -177,34 +202,11 @@ class TcaTableService ; } - /** - * @return string - */ - public function getFields() - { - $fields = array_merge( - ['uid','pid'], - array_filter( - array_keys($this->tca['columns']), - function ($columnName) { - return !$this->isSystemField($columnName); - } - ) - ); - - foreach ($fields as $key => $field) { - $fields[$key] = $this->tableName . '.' . $field; - } - - $this->logger->debug('Generated fields.', [$this->tableName, $fields]); - return implode(',', $fields); - } - /** * @param string * @return bool */ - protected function isSystemField($columnName) + protected function isSystemField($columnName) : bool { $systemFields = [ // Versioning fields, @@ -228,7 +230,7 @@ class TcaTableService * @return array * @throws InvalidArgumentException */ - public function getColumnConfig($columnName) + public function getColumnConfig($columnName) : array { if (!isset($this->tca['columns'][$columnName])) { throw new InvalidArgumentException( @@ -250,7 +252,7 @@ class TcaTableService * @param array &$record * @return bool */ - protected function isRecordBlacklistedByRootline(array &$record) + protected function isRecordBlacklistedByRootline(array &$record) : bool { // If no rootline exists, the record is on a unreachable page and therefore blacklisted. $rootline = BackendUtility::BEgetRootLine($record['pid']); @@ -275,7 +277,7 @@ class TcaTableService * * @return bool */ - protected function isBlackListedRootLineConfigured() + protected function isBlackListedRootLineConfigured() : bool { return (bool) $this->configuration->getIfExists('indexing.' . $this->getTableName() . '.rootLineBlacklist'); } @@ -285,7 +287,7 @@ class TcaTableService * * @return array */ - protected function getBlackListedRootLine() + protected function getBlackListedRootLine() : array { return GeneralUtility::intExplode(',', $this->configuration->getIfExists('indexing.' . $this->getTableName() . '.rootLineBlacklist')); } diff --git a/Makefile b/Makefile index 87bcdfc..b6bc72b 100644 --- a/Makefile +++ b/Makefile @@ -22,7 +22,6 @@ functionalTests: typo3DatabaseHost=$(typo3DatabaseHost) \ TYPO3_PATH_WEB=$(TYPO3_WEB_DIR) \ .Build/bin/phpunit --colors --debug -v \ - --stop-on-error \ -c Tests/Functional/FunctionalTests.xml unitTests: diff --git a/Tests/Functional/Connection/Elasticsearch/FilterTest.php b/Tests/Functional/Connection/Elasticsearch/FilterTest.php index aa5a5dc..f0b8404 100644 --- a/Tests/Functional/Connection/Elasticsearch/FilterTest.php +++ b/Tests/Functional/Connection/Elasticsearch/FilterTest.php @@ -55,7 +55,7 @@ class FilterTest extends AbstractFunctionalTestCase $searchRequest->setFilter(['CType' => 'HTML']); $result = $searchService->search($searchRequest); - $this->assertSame('5', $result->getResults()[0]['uid'], 'Did not get the expected result entry.'); + $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.'); } diff --git a/Tests/Functional/Connection/Elasticsearch/IndexTcaTableTest.php b/Tests/Functional/Connection/Elasticsearch/IndexTcaTableTest.php index 9678d83..8a76c77 100644 --- a/Tests/Functional/Connection/Elasticsearch/IndexTcaTableTest.php +++ b/Tests/Functional/Connection/Elasticsearch/IndexTcaTableTest.php @@ -150,7 +150,7 @@ class IndexTcaTableTest extends AbstractFunctionalTestCase ['_source' => [ 'uid' => '9', 'CType' => 'Header', // Testing items - 'categories' => ['Category 1', 'Category 2'], // Testing mm (with sorting) + 'categories' => ['Category 2', 'Category 1'], // Testing mm ]], $response->getData()['hits']['hits'][0], false, diff --git a/Tests/Unit/Domain/Index/TcaIndexer/TcaTableServiceTest.php b/Tests/Unit/Domain/Index/TcaIndexer/TcaTableServiceTest.php index 330c16e..58c03b4 100644 --- a/Tests/Unit/Domain/Index/TcaIndexer/TcaTableServiceTest.php +++ b/Tests/Unit/Domain/Index/TcaIndexer/TcaTableServiceTest.php @@ -64,9 +64,14 @@ class TcaTableServiceTest extends AbstractUnitTestCase ->method('getSystemWhereClause') ->will($this->returnValue('1=1 AND pages.no_search = 0')); + $whereClause =$this->subject->getWhereClause(); $this->assertSame( '1=1 AND pages.no_search = 0', - $this->subject->getWhereClause() + $whereClause->getStatement() + ); + $this->assertSame( + [], + $whereClause->getParameters() ); } @@ -83,9 +88,14 @@ class TcaTableServiceTest extends AbstractUnitTestCase ->method('getSystemWhereClause') ->will($this->returnValue('1=1 AND pages.no_search = 0')); + $whereClause = $this->subject->getWhereClause(); $this->assertSame( '1=1 AND pages.no_search = 0 AND table.field = "someValue"', - $this->subject->getWhereClause() + $whereClause->getStatement() + ); + $this->assertSame( + [], + $whereClause->getParameters() ); } } From d61a86f8fefd6f0eb36d9c3262d6f5fc534f7cab Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Fri, 7 Jul 2017 16:14:41 +0200 Subject: [PATCH 06/34] TASK: Finish TYPO3 CMS 8 update --- Classes/Database/Doctrine/Where.php | 3 --- Classes/Domain/Index/TcaIndexer/RelationResolver.php | 12 ++++++++++-- .../Hooks/DataHandler/AbstractDataHandlerTest.php | 3 +-- .../Hooks/DataHandler/ProcessesAllowedTablesTest.php | 11 ++++++----- .../Indexing/TcaIndexer/RelationResolverTest.php | 6 +----- ext_localconf.php | 4 ++-- 6 files changed, 20 insertions(+), 19 deletions(-) diff --git a/Classes/Database/Doctrine/Where.php b/Classes/Database/Doctrine/Where.php index 3398991..6586b8a 100644 --- a/Classes/Database/Doctrine/Where.php +++ b/Classes/Database/Doctrine/Where.php @@ -20,9 +20,6 @@ namespace Codappix\SearchCore\Database\Doctrine; * 02110-1301, USA. */ -/** - * - */ class Where { /** diff --git a/Classes/Domain/Index/TcaIndexer/RelationResolver.php b/Classes/Domain/Index/TcaIndexer/RelationResolver.php index 93cfffb..bfc0cc4 100644 --- a/Classes/Domain/Index/TcaIndexer/RelationResolver.php +++ b/Classes/Domain/Index/TcaIndexer/RelationResolver.php @@ -73,13 +73,16 @@ class RelationResolver implements Singleton */ protected function resolveValue($value, array $tcaColumn) { - if ($value === '' || $value === '0') { + if ($value === '') { return ''; } - if ($tcaColumn['type'] === 'select' || $tcaColumn['type'] === 'group') { + if ($tcaColumn['type'] === 'select') { return $this->resolveForeignDbValue($value); } + if ($tcaColumn['type'] === 'inline' || $tcaColumn['type'] === 'group') { + return $this->resolveInlineValue($value); + } return ''; } @@ -108,4 +111,9 @@ class RelationResolver implements Singleton } return array_map('trim', explode(';', $value)); } + + protected function resolveInlineValue(string $value) : array + { + return array_map('trim', explode(',', $value)); + } } diff --git a/Tests/Functional/Hooks/DataHandler/AbstractDataHandlerTest.php b/Tests/Functional/Hooks/DataHandler/AbstractDataHandlerTest.php index 40b64ea..0b0f128 100644 --- a/Tests/Functional/Hooks/DataHandler/AbstractDataHandlerTest.php +++ b/Tests/Functional/Hooks/DataHandler/AbstractDataHandlerTest.php @@ -50,7 +50,6 @@ abstract class AbstractDataHandlerTest extends AbstractFunctionalTestCase ->setMethods(['add', 'update', 'delete']) ->getMock(); - // This way TYPO3 will use our mock instead of a new instance. - $GLOBALS['T3_VAR']['getUserObj']['&' . DataHandlerHook::class] = new DataHandlerHook($this->subject); + GeneralUtility::setSingletonInstance(\Codappix\SearchCore\Hook\DataHandler::class, new DataHandlerHook($this->subject)); } } diff --git a/Tests/Functional/Hooks/DataHandler/ProcessesAllowedTablesTest.php b/Tests/Functional/Hooks/DataHandler/ProcessesAllowedTablesTest.php index 715fe29..7fa1cc5 100644 --- a/Tests/Functional/Hooks/DataHandler/ProcessesAllowedTablesTest.php +++ b/Tests/Functional/Hooks/DataHandler/ProcessesAllowedTablesTest.php @@ -47,7 +47,8 @@ class ProcessesAllowedTablesTest extends AbstractDataHandlerTest */ public function deletionWillBeTriggeredForTtContent() { - $this->subject->expects($this->exactly(1))->method('delete') + $this->subject->expects($this->exactly(1)) + ->method('delete') ->with($this->equalTo('tt_content'), $this->equalTo('1')); $tce = GeneralUtility::makeInstance(Typo3DataHandler::class); @@ -71,9 +72,9 @@ class ProcessesAllowedTablesTest extends AbstractDataHandlerTest ->with( $this->equalTo('tt_content'), $this->callback(function ($record) { - return isset($record['uid']) && $record['uid'] === '1' - && isset($record['pid']) && $record['pid'] === '1' - && isset($record['colPos']) && $record['colPos'] === '1' + return isset($record['uid']) && $record['uid'] === 1 + && isset($record['pid']) && $record['pid'] === 1 + && isset($record['colPos']) && $record['colPos'] === 1 ; }) ); @@ -99,7 +100,7 @@ class ProcessesAllowedTablesTest extends AbstractDataHandlerTest ->with( $this->equalTo('tt_content'), $this->callback(function ($record) { - return isset($record['uid']) && $record['uid'] === 2 + return isset($record['uid']) && $record['uid'] === '2' && isset($record['pid']) && $record['pid'] === 1 && isset($record['header']) && $record['header'] === 'a new record' ; diff --git a/Tests/Functional/Indexing/TcaIndexer/RelationResolverTest.php b/Tests/Functional/Indexing/TcaIndexer/RelationResolverTest.php index 5577ac0..a491677 100644 --- a/Tests/Functional/Indexing/TcaIndexer/RelationResolverTest.php +++ b/Tests/Functional/Indexing/TcaIndexer/RelationResolverTest.php @@ -37,10 +37,6 @@ class RelationResolverTest extends AbstractFunctionalTestCase $objectManager = GeneralUtility::makeInstance(ObjectManager::class); $table = 'sys_file'; - // Only by adding the field to showitem, it will be processed by FormEngine. - // We use this field to test inline relations, as there is only one alternative. - $GLOBALS['TCA']['sys_file']['types'][1]['showitem'] .= ',metadata'; - $subject = $objectManager->get(TcaTableService::class, $table); $record = BackendUtility::getRecord($table, 1); $subject->prepareRecord($record); @@ -113,8 +109,8 @@ class RelationResolverTest extends AbstractFunctionalTestCase $this->assertEquals( [ - 'Category 1', 'Category 2', + 'Category 1', ], $record['categories'], 'Foreign mm select relation was not resolved as expected.' diff --git a/ext_localconf.php b/ext_localconf.php index 1c1f9cd..54bf43b 100644 --- a/ext_localconf.php +++ b/ext_localconf.php @@ -16,10 +16,10 @@ call_user_func( ], 't3lib/class.t3lib_tcemain.php' => [ 'processCmdmapClass' => [ - $extensionKey => '&' . \Codappix\SearchCore\Hook\DataHandler::class, + $extensionKey => \Codappix\SearchCore\Hook\DataHandler::class, ], 'processDatamapClass' => [ - $extensionKey => '&' . \Codappix\SearchCore\Hook\DataHandler::class, + $extensionKey => \Codappix\SearchCore\Hook\DataHandler::class, ], ], ], From a8fdb8a44d79beea2ba3f6af7d5dc4db982f1f6f Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Fri, 7 Jul 2017 16:24:56 +0200 Subject: [PATCH 07/34] TASK: Update dependencies for CMS 8 --- ext_emconf.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext_emconf.php b/ext_emconf.php index 118e2df..4d342a5 100644 --- a/ext_emconf.php +++ b/ext_emconf.php @@ -7,8 +7,8 @@ $EM_CONF[$_EXTKEY] = [ 'clearCacheOnLoad' => 1, 'constraints' => [ 'depends' => [ - 'typo3' => '7.6.0-7.6.99', - 'php' => '5.6.0-7.99.99' + 'typo3' => '8.7.0-8.7.99', + 'php' => '7.1.0-7.99.99' ], 'conflicts' => [], ], From 2cd5debf977c8e1e3893cf645da9624ed73782ae Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Fri, 7 Jul 2017 16:44:57 +0200 Subject: [PATCH 08/34] BUGFIX: Fix broken getRecord method Also add test covering method. --- Classes/Domain/Index/TcaIndexer.php | 43 ++++++++++--------- .../Elasticsearch/IndexTcaTableTest.php | 23 ++++++++++ 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/Classes/Domain/Index/TcaIndexer.php b/Classes/Domain/Index/TcaIndexer.php index 18444ce..7923565 100644 --- a/Classes/Domain/Index/TcaIndexer.php +++ b/Classes/Domain/Index/TcaIndexer.php @@ -22,6 +22,7 @@ namespace Codappix\SearchCore\Domain\Index; use Codappix\SearchCore\Connection\ConnectionInterface; use TYPO3\CMS\Core\Database\ConnectionPool; +use TYPO3\CMS\Core\Database\Query\QueryBuilder; use TYPO3\CMS\Core\Utility\GeneralUtility; /** @@ -53,21 +54,11 @@ class TcaIndexer extends AbstractIndexer */ protected function getRecords($offset, $limit) { - $queryBuilder = $this->getDatabaseConnection()->getQueryBuilderForTable($this->tcaTableService->getTableName()); - $where = $this->tcaTableService->getWhereClause(); - $query = $queryBuilder->select(... $this->tcaTableService->getFields()) - ->from($this->tcaTableService->getTableClause()) - ->where($where->getStatement()) - ->setParameters($where->getParameters()) + $records = $this->getQuery() ->setFirstResult($offset) - ->setMaxResults($limit); - - foreach ($this->tcaTableService->getJoins() as $join) { - $query->from($join->getTable()); - $query->andWhere($join->getCondition()); - } - - $records = $query->execute()->fetchAll(); + ->setMaxResults($limit) + ->execute() + ->fetchAll(); if ($records === null) { return null; @@ -88,12 +79,7 @@ class TcaIndexer extends AbstractIndexer */ protected function getRecord($identifier) { - $record = $this->getDatabaseConnection()->exec_SELECTgetSingleRow( - $this->tcaTableService->getFields(), - $this->tcaTableService->getTableClause(), - $this->tcaTableService->getWhereClause() - . ' AND ' . $this->tcaTableService->getTableName() . '.uid = ' . (int) $identifier - ); + $record = $this->getQuery()->execute()->fetch(); if ($record === false || $record === null) { throw new NoRecordFoundException( @@ -114,6 +100,23 @@ class TcaIndexer extends AbstractIndexer return $this->tcaTableService->getTableName(); } + protected function getQuery() : QueryBuilder + { + $queryBuilder = $this->getDatabaseConnection()->getQueryBuilderForTable($this->tcaTableService->getTableName()); + $where = $this->tcaTableService->getWhereClause(); + $query = $queryBuilder->select(... $this->tcaTableService->getFields()) + ->from($this->tcaTableService->getTableClause()) + ->where($where->getStatement()) + ->setParameters($where->getParameters()); + + foreach ($this->tcaTableService->getJoins() as $join) { + $query->from($join->getTable()); + $query->andWhere($join->getCondition()); + } + + return $query; + } + protected function getDatabaseConnection() { return GeneralUtility::makeInstance(ConnectionPool::class); diff --git a/Tests/Functional/Connection/Elasticsearch/IndexTcaTableTest.php b/Tests/Functional/Connection/Elasticsearch/IndexTcaTableTest.php index 8a76c77..7210e01 100644 --- a/Tests/Functional/Connection/Elasticsearch/IndexTcaTableTest.php +++ b/Tests/Functional/Connection/Elasticsearch/IndexTcaTableTest.php @@ -59,6 +59,29 @@ class IndexTcaTableTest extends AbstractFunctionalTestCase ); } + /** + * @test + */ + public function indexSingleBasicTtContent() + { + \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(ObjectManager::class) + ->get(IndexerFactory::class) + ->getIndexer('tt_content') + ->indexDocument(6) + ; + + $response = $this->client->request('typo3content/_search?q=*:*'); + + $this->assertTrue($response->isOK(), 'Elastica did not answer with ok code.'); + $this->assertSame($response->getData()['hits']['total'], 1, 'Not exactly 1 document was indexed.'); + $this->assertArraySubset( + ['_source' => ['header' => 'indexed content element']], + $response->getData()['hits']['hits'][0], + false, + 'Record was not indexed.' + ); + } + /** * @test * @expectedException \Codappix\SearchCore\Domain\Index\IndexingException From 9fff750e852343091fec047139b9fa2c2663ba5e Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Fri, 7 Jul 2017 16:29:16 +0200 Subject: [PATCH 09/34] BUGFIX: Fix TypoScript warning Constants can't use multi line strings. --- Configuration/TypoScript/constants.txt | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Configuration/TypoScript/constants.txt b/Configuration/TypoScript/constants.txt index f97c039..149d610 100644 --- a/Configuration/TypoScript/constants.txt +++ b/Configuration/TypoScript/constants.txt @@ -14,10 +14,7 @@ plugin { # should also be added, together with additionalWhereClause # based on doktypes tt_content { - additionalWhereClause ( - pages.doktype NOT IN (3, 199) - AND tt_content.CType NOT IN ('gridelements_pi1', 'list', 'div', 'menu', 'shortcut', 'search', 'login') - ) + additionalWhereClause = pages.doktype NOT IN (3, 199) AND tt_content.CType NOT IN ('gridelements_pi1', 'list', 'div', 'menu', 'shortcut', 'search', 'login') } } } From 31937d2d6f671bbe5ac5bc6eeb2792a5afeb84a9 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Fri, 28 Jul 2017 11:58:24 +0200 Subject: [PATCH 10/34] BUGFIX: Fix broken test --- Tests/Unit/Domain/Search/QueryFactoryTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Tests/Unit/Domain/Search/QueryFactoryTest.php b/Tests/Unit/Domain/Search/QueryFactoryTest.php index 33ef852..9ff690a 100644 --- a/Tests/Unit/Domain/Search/QueryFactoryTest.php +++ b/Tests/Unit/Domain/Search/QueryFactoryTest.php @@ -153,6 +153,9 @@ class QueryFactoryTest extends AbstractUnitTestCase */ public function sizeIsAddedToQuery() { + $this->configuration->expects($this->any()) + ->method('get') + ->will($this->throwException(new InvalidArgumentException)); $searchRequest = new SearchRequest('SearchWord'); $searchRequest->setSize(45); From 56ce88b0055edf1fba62a827bb73346c5a66a912 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Fri, 4 Aug 2017 13:39:18 +0200 Subject: [PATCH 11/34] TASK: Resolve relations always to array Do not resolve to empty string but array to allow same handling for all relations. --- Classes/Domain/Index/TcaIndexer/RelationResolver.php | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/Classes/Domain/Index/TcaIndexer/RelationResolver.php b/Classes/Domain/Index/TcaIndexer/RelationResolver.php index b09e483..483fa4b 100644 --- a/Classes/Domain/Index/TcaIndexer/RelationResolver.php +++ b/Classes/Domain/Index/TcaIndexer/RelationResolver.php @@ -20,13 +20,10 @@ namespace Codappix\SearchCore\Domain\Index\TcaIndexer; * 02110-1301, USA. */ -use TYPO3\CMS\Backend\Utility\BackendUtility; -use TYPO3\CMS\Core\SingletonInterface as Singleton; -use TYPO3\CMS\Core\Utility\GeneralUtility; -use TYPO3\CMS\Extbase\Utility\LocalizationUtility; - use TYPO3\CMS\Backend\Form\FormDataCompiler; use TYPO3\CMS\Backend\Form\FormDataGroup\TcaDatabaseRecord; +use TYPO3\CMS\Core\SingletonInterface as Singleton; +use TYPO3\CMS\Core\Utility\GeneralUtility; /** * Resolves relations from TCA using TCA. @@ -81,7 +78,7 @@ class RelationResolver implements Singleton protected function resolveValue($value, array $tcaColumn) { if ($value === '' || $value === '0') { - return ''; + return []; } if ($tcaColumn['config']['type'] === 'select') { return $this->resolveSelectValue($value, $tcaColumn); @@ -93,7 +90,7 @@ class RelationResolver implements Singleton return $this->resolveInlineValue($tcaColumn); } - return ''; + return []; } /** From ac78464c030ab66a94a2b49c1fb186ef6e8e0de4 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Fri, 4 Aug 2017 13:39:48 +0200 Subject: [PATCH 12/34] TASK: Provide more helpful logging --- Classes/Connection/Elasticsearch/DocumentFactory.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Classes/Connection/Elasticsearch/DocumentFactory.php b/Classes/Connection/Elasticsearch/DocumentFactory.php index 99d29e5..390c592 100644 --- a/Classes/Connection/Elasticsearch/DocumentFactory.php +++ b/Classes/Connection/Elasticsearch/DocumentFactory.php @@ -61,7 +61,10 @@ class DocumentFactory implements Singleton $identifier = $document['search_identifier']; unset($document['search_identifier']); - $this->logger->debug('Convert document to document', [$identifier, $document]); + $this->logger->debug( + sprintf('Convert %s %u to document.', $documentType, $identifier), + [$identifier, $document] + ); return new \Elastica\Document($identifier, $document); } From 6595d7129abebb8e810d525bf5db27f1152fd4f5 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Mon, 7 Aug 2017 18:04:07 +0200 Subject: [PATCH 13/34] TASK: Move additionalWhereClause to constants To keep things the same, move additionalWhereClause of tt_content also to constants. --- Configuration/TypoScript/constants.txt | 4 ++++ Configuration/TypoScript/setup.txt | 5 +---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Configuration/TypoScript/constants.txt b/Configuration/TypoScript/constants.txt index bcf191e..1a39851 100644 --- a/Configuration/TypoScript/constants.txt +++ b/Configuration/TypoScript/constants.txt @@ -9,6 +9,10 @@ plugin { } indexing { + tt_content { + additionalWhereClause = tt_content.CType NOT IN ('gridelements_pi1', 'list', 'div', 'menu', 'shortcut', 'search', 'login') AND tt_content.bodytext != '' + } + pages { additionalWhereClause = pages.doktype NOT IN (3, 199, 6, 254, 255) abstractFields = abstract, description, bodytext diff --git a/Configuration/TypoScript/setup.txt b/Configuration/TypoScript/setup.txt index d77c42e..67612e1 100644 --- a/Configuration/TypoScript/setup.txt +++ b/Configuration/TypoScript/setup.txt @@ -12,10 +12,7 @@ plugin { # Not for direct indexing therefore no indexer. # Used to configure tt_content fetching while indexing pages tt_content { - additionalWhereClause ( - tt_content.CType NOT IN ('gridelements_pi1', 'list', 'div', 'menu', 'shortcut', 'search', 'login') - AND tt_content.bodytext != '' - ) + additionalWhereClause = {$plugin.tx_searchcore.settings.indexing.tt_content.additionalWhereClause} } pages { From baca4824d5f6b38ef572840072cc00265d7d1fb5 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 8 Aug 2017 09:47:12 +0200 Subject: [PATCH 14/34] BUGFIX: Do not test php 5.6 As TYPO3 7.6.x introduced breaking change to not support PHP 5.6 any longer. --- .travis.yml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.travis.yml b/.travis.yml index 3253c3c..28c45aa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,7 +10,6 @@ before_install: language: php php: - - 5.6 - 7.0 - 7.1 @@ -31,12 +30,6 @@ env: matrix: fast_finish: true - exclude: - # TYPO3 no longer supports 5.6 - - env: TYPO3_VERSION="~8" - php: 5.6 - - env: TYPO3_VERSION="dev-master" - php: 5.6 allow_failures: - env: TYPO3_VERSION="~8" php: 7.0 From 67d1180ee18b9d4dcc57d2fad7b10f976da12527 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 8 Aug 2017 10:36:50 +0200 Subject: [PATCH 15/34] TASK: Remove php 7.0 testing As we do not support PHP 7.0. --- .travis.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index ea6d947..21d058f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,7 +11,6 @@ before_install: language: php php: - - 7.0 - 7.1 env: @@ -31,8 +30,6 @@ env: matrix: fast_finish: true allow_failures: - - env: TYPO3_VERSION="dev-master" - php: 7.0 - env: TYPO3_VERSION="dev-master" php: 7.1 From 49a56496c6cd301d01451ab86772b573d5c6b2e1 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 8 Aug 2017 11:54:32 +0200 Subject: [PATCH 16/34] TASK: Fix codacy issues Break line to not exceed maximum line length. Use imported namespace to shorten line. --- Classes/Domain/Index/TcaIndexer/RelationResolver.php | 8 +++++++- .../Hooks/DataHandler/AbstractDataHandlerTest.php | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/Classes/Domain/Index/TcaIndexer/RelationResolver.php b/Classes/Domain/Index/TcaIndexer/RelationResolver.php index 5d5a256..88aa982 100644 --- a/Classes/Domain/Index/TcaIndexer/RelationResolver.php +++ b/Classes/Domain/Index/TcaIndexer/RelationResolver.php @@ -39,7 +39,13 @@ class RelationResolver implements Singleton if ($column === 'pid') { continue; } - $record[$column] = BackendUtility::getProcessedValueExtra($service->getTableName(), $column, $record[$column], 0, $record['uid']); + $record[$column] = BackendUtility::getProcessedValueExtra( + $service->getTableName(), + $column, + $record[$column], + 0, + $record['uid'] + ); try { $config = $service->getColumnConfig($column); diff --git a/Tests/Functional/Hooks/DataHandler/AbstractDataHandlerTest.php b/Tests/Functional/Hooks/DataHandler/AbstractDataHandlerTest.php index 0b0f128..84ac0bd 100644 --- a/Tests/Functional/Hooks/DataHandler/AbstractDataHandlerTest.php +++ b/Tests/Functional/Hooks/DataHandler/AbstractDataHandlerTest.php @@ -50,6 +50,6 @@ abstract class AbstractDataHandlerTest extends AbstractFunctionalTestCase ->setMethods(['add', 'update', 'delete']) ->getMock(); - GeneralUtility::setSingletonInstance(\Codappix\SearchCore\Hook\DataHandler::class, new DataHandlerHook($this->subject)); + GeneralUtility::setSingletonInstance(DataHandlerHook::class, new DataHandlerHook($this->subject)); } } From 17eb35a92bf152a5d571f2e97b44b22bb791cf2f Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 8 Aug 2017 12:58:01 +0200 Subject: [PATCH 17/34] FEATURE: Respect inherited start- and endtime for pages Do not index records below tables that extend their start- or endtime to their subpages are not accessible due to timing now. --- .../Index/TcaIndexer/TcaTableService.php | 44 ++++++++++++++----- .../Indexing/PagesIndexer/InheritedTiming.xml | 34 ++++++++++++++ .../Functional/Indexing/PagesIndexerTest.php | 29 ++++++++++++ 3 files changed, 95 insertions(+), 12 deletions(-) create mode 100644 Tests/Functional/Fixtures/Indexing/PagesIndexer/InheritedTiming.xml diff --git a/Classes/Domain/Index/TcaIndexer/TcaTableService.php b/Classes/Domain/Index/TcaIndexer/TcaTableService.php index b5f48ab..488f949 100644 --- a/Classes/Domain/Index/TcaIndexer/TcaTableService.php +++ b/Classes/Domain/Index/TcaIndexer/TcaTableService.php @@ -24,6 +24,8 @@ use Codappix\SearchCore\Configuration\ConfigurationContainerInterface; use Codappix\SearchCore\Domain\Index\IndexingException; use TYPO3\CMS\Backend\Utility\BackendUtility; use TYPO3\CMS\Core\Utility\GeneralUtility; +use TYPO3\CMS\Core\Utility\RootlineUtility; +use TYPO3\CMS\Extbase\Object\ObjectManagerInterface; /** * Encapsulate logik related to TCA configuration. @@ -47,15 +49,20 @@ class TcaTableService */ protected $configuration; + /** + * @var RelationResolver + */ + protected $relationResolver; + /** * @var \TYPO3\CMS\Core\Log\Logger */ protected $logger; /** - * @var RelationResolver + * @var ObjectManagerInterface */ - protected $relationResolver; + protected $objectManager; /** * Inject log manager to get concrete logger from it. @@ -67,6 +74,14 @@ class TcaTableService $this->logger = $logManager->getLogger(__CLASS__); } + /** + * @param ObjectManagerInterface $objectManager + */ + public function injectObjectManager(ObjectManagerInterface $objectManager) + { + $this->objectManager = $objectManager; + } + /** * @param string $tableName * @param ConfigurationContainerInterface $configuration @@ -243,26 +258,31 @@ class TcaTableService * Checks whether the given record was blacklisted by root line. * This can be configured by typoscript as whole root lines can be black listed. * - * NOTE: Does not support pages yet. We have to add a switch once we - * support them to use uid instead. - * * @param array &$record * @return bool */ protected function isRecordBlacklistedByRootline(array &$record) { // If no rootline exists, the record is on a unreachable page and therefore blacklisted. - $rootline = BackendUtility::BEgetRootLine($record['pid']); + if ($record['pid'] == 0) { + return false; + } + + $rootline = $this->objectManager->get(RootlineUtility::class, $record['pid'])->get(); if (!isset($rootline[0])) { return true; } - // Check configured black list if present. - if ($this->isBlackListedRootLineConfigured()) { - foreach ($rootline as $pageInRootLine) { - if (in_array($pageInRootLine['uid'], $this->getBlackListedRootLine())) { - return true; - } + foreach ($rootline as $pageInRootLine) { + // Check configured black list if present. + if ($this->isBlackListedRootLineConfigured() && in_array($pageInRootLine['uid'], $this->getBlackListedRootLine())) { + return true; + } + if ($pageInRootLine['extendToSubpages'] && ( + ($pageInRootLine['endtime'] > 0 && $pageInRootLine['endtime'] <= time()) + || ($pageInRootLine['starttime'] > 0 && $pageInRootLine['starttime'] >= time()) + )) { + return true; } } diff --git a/Tests/Functional/Fixtures/Indexing/PagesIndexer/InheritedTiming.xml b/Tests/Functional/Fixtures/Indexing/PagesIndexer/InheritedTiming.xml new file mode 100644 index 0000000..552ba74 --- /dev/null +++ b/Tests/Functional/Fixtures/Indexing/PagesIndexer/InheritedTiming.xml @@ -0,0 +1,34 @@ + + + + + 2 + 1 + Some disabled page due to timing + 1502186635 + 1 + + + 3 + 2 + Some disabled page due to inherited timing + + + 4 + 1 + Some disabled page due to timing + 2147483647 + 1 + + + 5 + 4 + Some disabled page due to inherited timing + + + + 6 + 1 + Some enabled page due to no be below inherited disabled timing + + diff --git a/Tests/Functional/Indexing/PagesIndexerTest.php b/Tests/Functional/Indexing/PagesIndexerTest.php index d0440ba..2f00d2f 100644 --- a/Tests/Functional/Indexing/PagesIndexerTest.php +++ b/Tests/Functional/Indexing/PagesIndexerTest.php @@ -61,4 +61,33 @@ class PagesIndexerTest extends AbstractFunctionalTestCase $this->inject($indexer, 'connection', $connection); $indexer->indexAllDocuments(); } + + /** + * @test + */ + public function inheritedTimingIsRespectedDuringIndexing() + { + $this->importDataSet('Tests/Functional/Fixtures/Indexing/PagesIndexer/InheritedTiming.xml'); + + $objectManager = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(ObjectManager::class); + $tableName = 'pages'; + + $connection = $this->getMockBuilder(Elasticsearch::class) + ->setMethods(['addDocuments']) + ->disableOriginalConstructor() + ->getMock(); + + $connection->expects($this->once()) + ->method('addDocuments') + ->with( + $this->stringContains($tableName), + $this->callback(function ($documents) { + return count($documents) === 2; + }) + ); + + $indexer = $objectManager->get(IndexerFactory::class)->getIndexer($tableName); + $this->inject($indexer, 'connection', $connection); + $indexer->indexAllDocuments(); + } } From f7e1bd1cdfa8f42e4610596185270a520cad6eb1 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 8 Aug 2017 17:07:23 +0200 Subject: [PATCH 18/34] FEATURE: Implement necessary logic to support PaginateViewHelper --- Classes/Connection/Elasticsearch.php | 2 +- .../Connection/Elasticsearch/SearchResult.php | 98 +++++++--- Classes/Connection/SearchRequestInterface.php | 14 +- Classes/Connection/SearchResultInterface.php | 17 +- Classes/Domain/Model/SearchRequest.php | 183 ++++++++++++++++-- Classes/Domain/Search/QueryFactory.php | 10 +- Classes/Domain/Search/SearchService.php | 3 +- Tests/Unit/Domain/Search/QueryFactoryTest.php | 10 +- .../Unit/Domain/Search/SearchServiceTest.php | 4 +- 9 files changed, 264 insertions(+), 77 deletions(-) diff --git a/Classes/Connection/Elasticsearch.php b/Classes/Connection/Elasticsearch.php index f6321bc..4c66b6a 100644 --- a/Classes/Connection/Elasticsearch.php +++ b/Classes/Connection/Elasticsearch.php @@ -189,7 +189,7 @@ class Elasticsearch implements Singleton, ConnectionInterface $search->addIndex('typo3content'); $search->setQuery($this->queryFactory->create($searchRequest)); - return $this->objectManager->get(SearchResult::class, $search->search()); + return $this->objectManager->get(SearchResult::class, $searchRequest, $search->search()); } /** diff --git a/Classes/Connection/Elasticsearch/SearchResult.php b/Classes/Connection/Elasticsearch/SearchResult.php index 486b6eb..46ffb14 100644 --- a/Classes/Connection/Elasticsearch/SearchResult.php +++ b/Classes/Connection/Elasticsearch/SearchResult.php @@ -22,11 +22,17 @@ namespace Codappix\SearchCore\Connection\Elasticsearch; use Codappix\SearchCore\Connection\FacetInterface; use Codappix\SearchCore\Connection\ResultItemInterface; +use Codappix\SearchCore\Connection\SearchRequestInterface; use Codappix\SearchCore\Connection\SearchResultInterface; use TYPO3\CMS\Extbase\Object\ObjectManagerInterface; class SearchResult implements SearchResultInterface { + /** + * @var SearchRequestInterface + */ + protected $searchRequest; + /** * @var \Elastica\ResultSet */ @@ -47,8 +53,12 @@ class SearchResult implements SearchResultInterface */ protected $objectManager; - public function __construct(\Elastica\ResultSet $result, ObjectManagerInterface $objectManager) - { + public function __construct( + SearchRequestInterface $searchRequest, + \Elastica\ResultSet $result, + ObjectManagerInterface $objectManager + ) { + $this->searchRequest = $searchRequest; $this->result = $result; $this->objectManager = $objectManager; } @@ -75,25 +85,37 @@ class SearchResult implements SearchResultInterface return $this->facets; } - /** - * Returns the total sum of matching results. - * - * @return int - */ - public function getTotalCount() + public function getCurrentCount() { - return $this->result->getTotalHits(); + return $this->result->count(); + } + + protected function initResults() + { + if ($this->results !== []) { + return; + } + + foreach ($this->result->getResults() as $result) { + $this->results[] = new ResultItem($result); + } + } + + protected function initFacets() + { + if ($this->facets !== [] || !$this->result->hasAggregations()) { + return; + } + + foreach ($this->result->getAggregations() as $aggregationName => $aggregation) { + $this->facets[$aggregationName] = $this->objectManager->get(Facet::class, $aggregationName, $aggregation); + } } // Countable - Interface - /** - * Returns the total sum of results contained in this result. - * - * @return int - */ public function count() { - return $this->result->count(); + return $this->result->getTotalHits(); } // Iterator - Interface @@ -122,25 +144,41 @@ class SearchResult implements SearchResultInterface $this->result->rewind(); } - protected function initResults() - { - if ($this->results !== []) { - return; - } + // Extbase QueryResultInterface - Implemented to support Pagination of Fluid. - foreach ($this->result->getResults() as $result) { - $this->results[] = new ResultItem($result); - } + public function getQuery() + { + return $this->searchRequest; } - protected function initFacets() + public function getFirst() { - if ($this->facets !== [] || !$this->result->hasAggregations()) { - return; - } + throw new \BadMethodCallException('Method is not implemented yet.', 1502195121); + } - foreach ($this->result->getAggregations() as $aggregationName => $aggregation) { - $this->facets[$aggregationName] = $this->objectManager->get(Facet::class, $aggregationName, $aggregation); - } + 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/SearchRequestInterface.php b/Classes/Connection/SearchRequestInterface.php index 7ec34ff..7c7956e 100644 --- a/Classes/Connection/SearchRequestInterface.php +++ b/Classes/Connection/SearchRequestInterface.php @@ -20,10 +20,9 @@ namespace Codappix\SearchCore\Connection; * 02110-1301, USA. */ -/** - * - */ -interface SearchRequestInterface +use TYPO3\CMS\Extbase\Persistence\QueryInterface; + +interface SearchRequestInterface extends QueryInterface { /** * Returns the actual string the user searched for. @@ -41,11 +40,4 @@ interface SearchRequestInterface * @return array */ public function getFilter(); - - /** - * Defines how many results should be fetched. - * - * @return int - */ - public function getSize(); } diff --git a/Classes/Connection/SearchResultInterface.php b/Classes/Connection/SearchResultInterface.php index 5c45bcd..d52a8ba 100644 --- a/Classes/Connection/SearchResultInterface.php +++ b/Classes/Connection/SearchResultInterface.php @@ -20,10 +20,12 @@ namespace Codappix\SearchCore\Connection; * 02110-1301, USA. */ +use TYPO3\CMS\Extbase\Persistence\QueryResultInterface; + /** * A search result. */ -interface SearchResultInterface extends \Iterator, \Countable +interface SearchResultInterface extends \Iterator, \Countable, QueryResultInterface { /** * @return array @@ -38,18 +40,9 @@ interface SearchResultInterface extends \Iterator, \Countable public function getFacets(); /** - * Returns the total sum of matching results. + * Returns the number of results in current result * * @return int */ - public function getTotalCount(); - - // Countable - Interface - - /** - * Returns the total sum of results contained in this result. - * - * @return int - */ - public function count(); + public function getCurrentCount(); } diff --git a/Classes/Domain/Model/SearchRequest.php b/Classes/Domain/Model/SearchRequest.php index 7850b98..7d6436c 100644 --- a/Classes/Domain/Model/SearchRequest.php +++ b/Classes/Domain/Model/SearchRequest.php @@ -20,6 +20,7 @@ namespace Codappix\SearchCore\Domain\Model; * 02110-1301, USA. */ +use Codappix\SearchCore\Connection\ConnectionInterface; use Codappix\SearchCore\Connection\FacetRequestInterface; use Codappix\SearchCore\Connection\SearchRequestInterface; @@ -35,11 +36,6 @@ class SearchRequest implements SearchRequestInterface */ protected $query = ''; - /** - * @var int - */ - protected $size = 10; - /** * @var array */ @@ -50,6 +46,23 @@ class SearchRequest implements SearchRequestInterface */ protected $facets = []; + /** + * @var int + */ + protected $offset = 0; + + /** + * @var int + */ + protected $limit = 10; + + /** + * Used for QueryInterface implementation to allow execute method to work. + * + * @var ConnectionInterface + */ + protected $connection = null; + /** * @param string $query */ @@ -119,18 +132,162 @@ class SearchRequest implements SearchRequestInterface } /** - * @return int + * Define connection to use for this request. + * Necessary to allow implementation of execute for interface. + * + * @param ConnectionInterface $connection */ - public function getSize() + public function setConnection(ConnectionInterface $connection) { - return $this->size; + $this->connection = $connection; } - /** - * @param int $size - */ - public function setSize($size) + // Extbase QueryInterface + // Current implementation covers only paginate widget support. + public function execute($returnRawQueryResult = false) { - $this->size = (int) $size; + if ($this->connection instanceof ConnectionInterface) { + return $this->connection->search($this); + } + + throw new \InvalidArgumentException( + 'Connection was not set before, therefore execute can not work. Use `setConnection` before.', + 1502197732 + ); + } + + public function setLimit($limit) + { + $this->limit = (int) $limit; + } + + public function setOffset($offset) + { + $this->offset = (int) $offset; + } + + public function getLimit() + { + return $this->limit; + } + + public function getOffset() + { + return $this->offset; + } + + public function getSource() + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196146); + } + + public function setOrderings(array $orderings) + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196163); + } + + public function matching($constraint) + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196197); + } + + public function logicalAnd($constraint1) + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196166); + } + + public function logicalOr($constraint1) + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196198); + } + + public function logicalNot(\TYPO3\CMS\Extbase\Persistence\Generic\Qom\ConstraintInterface $constraint) + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196166); + } + + public function equals($propertyName, $operand, $caseSensitive = true) + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196199); + } + + public function like($propertyName, $operand, $caseSensitive = true) + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196167); + } + + public function contains($propertyName, $operand) + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196200); + } + + public function in($propertyName, $operand) + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196167); + } + + public function lessThan($propertyName, $operand) + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196201); + } + + public function lessThanOrEqual($propertyName, $operand) + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196168); + } + + public function greaterThan($propertyName, $operand) + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196202); + } + + public function greaterThanOrEqual($propertyName, $operand) + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196168); + } + + public function getType() + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196203); + } + + public function setQuerySettings(\TYPO3\CMS\Extbase\Persistence\Generic\QuerySettingsInterface $querySettings) + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196168); + } + + public function getQuerySettings() + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196205); + } + + public function count() + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196169); + } + + public function getOrderings() + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196206); + } + + public function getConstraint() + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196171); + } + + public function isEmpty($propertyName) + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196207); + } + + public function setSource(\TYPO3\CMS\Extbase\Persistence\Generic\Qom\SourceInterface $source) + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196172); + } + + public function getStatement() + { + throw new \BadMethodCallException('Method is not implemented yet.', 1502196208); } } diff --git a/Classes/Domain/Search/QueryFactory.php b/Classes/Domain/Search/QueryFactory.php index ce702f9..4b3c775 100644 --- a/Classes/Domain/Search/QueryFactory.php +++ b/Classes/Domain/Search/QueryFactory.php @@ -97,8 +97,8 @@ class QueryFactory protected function addSize(SearchRequestInterface $searchRequest) { $this->query = ArrayUtility::arrayMergeRecursiveOverrule($this->query, [ - 'from' => 0, - 'size' => $searchRequest->getSize(), + 'from' => $searchRequest->getOffset(), + 'size' => $searchRequest->getLimit(), ]); } @@ -151,8 +151,8 @@ class QueryFactory 'query' => [ 'bool' => [ 'should' => $boostQueryParts, - ], - ], + ], + ], ]); } @@ -163,7 +163,7 @@ class QueryFactory 'function_score' => [ 'query' => $this->query['query'], 'field_value_factor' => $this->configuration->get('searching.fieldValueFactor'), - ], + ], ]; } catch (InvalidArgumentException $e) { return; diff --git a/Classes/Domain/Search/SearchService.php b/Classes/Domain/Search/SearchService.php index bb8b138..114ebfe 100644 --- a/Classes/Domain/Search/SearchService.php +++ b/Classes/Domain/Search/SearchService.php @@ -69,6 +69,7 @@ class SearchService */ public function search(SearchRequestInterface $searchRequest) { + $searchRequest->setConnection($this->connection); $this->addSize($searchRequest); $this->addConfiguredFacets($searchRequest); @@ -82,7 +83,7 @@ class SearchService */ protected function addSize(SearchRequestInterface $searchRequest) { - $searchRequest->setSize( + $searchRequest->setLimit( $this->configuration->getIfExists('searching.size') ?: 10 ); } diff --git a/Tests/Unit/Domain/Search/QueryFactoryTest.php b/Tests/Unit/Domain/Search/QueryFactoryTest.php index 9ff690a..de82ebf 100644 --- a/Tests/Unit/Domain/Search/QueryFactoryTest.php +++ b/Tests/Unit/Domain/Search/QueryFactoryTest.php @@ -157,13 +157,19 @@ class QueryFactoryTest extends AbstractUnitTestCase ->method('get') ->will($this->throwException(new InvalidArgumentException)); $searchRequest = new SearchRequest('SearchWord'); - $searchRequest->setSize(45); + $searchRequest->setLimit(45); + $searchRequest->setOffset(35); $query = $this->subject->create($searchRequest); $this->assertSame( 45, $query->toArray()['size'], - 'Size was not added to query.' + 'Limit was not added to query.' + ); + $this->assertSame( + 35, + $query->toArray()['from'], + 'From was not added to query.' ); } diff --git a/Tests/Unit/Domain/Search/SearchServiceTest.php b/Tests/Unit/Domain/Search/SearchServiceTest.php index 6a90a3c..046f751 100644 --- a/Tests/Unit/Domain/Search/SearchServiceTest.php +++ b/Tests/Unit/Domain/Search/SearchServiceTest.php @@ -67,7 +67,7 @@ class SearchServiceTest extends AbstractUnitTestCase $this->connection->expects($this->once()) ->method('search') ->with($this->callback(function ($searchRequest) { - return $searchRequest->getSize() === 45; + return $searchRequest->getLimit() === 45; })); $searchRequest = new SearchRequest('SearchWord'); @@ -86,7 +86,7 @@ class SearchServiceTest extends AbstractUnitTestCase $this->connection->expects($this->once()) ->method('search') ->with($this->callback(function ($searchRequest) { - return $searchRequest->getSize() === 10; + return $searchRequest->getLimit() === 10; })); $searchRequest = new SearchRequest('SearchWord'); From c5766f5b1269c77140cb8d5fa117546381837706 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 10 Aug 2017 08:54:36 +0200 Subject: [PATCH 19/34] BUGFIX: Use fresh query for each creation This prevents issues with modifying an build query. --- Classes/Domain/Search/QueryFactory.php | 62 ++++++++++++++------------ 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/Classes/Domain/Search/QueryFactory.php b/Classes/Domain/Search/QueryFactory.php index ce702f9..4edc7b0 100644 --- a/Classes/Domain/Search/QueryFactory.php +++ b/Classes/Domain/Search/QueryFactory.php @@ -39,11 +39,6 @@ class QueryFactory */ protected $configuration; - /** - * @var array - */ - protected $query = []; - /** * @param \TYPO3\CMS\Core\Log\LogManager $logManager * @param ConfigurationContainerInterface $configuration @@ -77,26 +72,28 @@ class QueryFactory */ protected function createElasticaQuery(SearchRequestInterface $searchRequest) { - $this->addSize($searchRequest); - $this->addSearch($searchRequest); - $this->addBoosts($searchRequest); - $this->addFilter($searchRequest); - $this->addFacets($searchRequest); + $query = []; + $this->addSize($searchRequest, $query); + $this->addSearch($searchRequest, $query); + $this->addBoosts($searchRequest, $query); + $this->addFilter($searchRequest, $query); + $this->addFacets($searchRequest, $query); // Use last, as it might change structure of query. // Better approach would be something like DQL to generate query and build result in the end. - $this->addFactorBoost(); + $this->addFactorBoost($query); - $this->logger->debug('Generated elasticsearch query.', [$this->query]); - return new \Elastica\Query($this->query); + $this->logger->debug('Generated elasticsearch query.', [$query]); + return new \Elastica\Query($query); } /** * @param SearchRequestInterface $searchRequest + * @param array &$query */ - protected function addSize(SearchRequestInterface $searchRequest) + protected function addSize(SearchRequestInterface $searchRequest, array &$query) { - $this->query = ArrayUtility::arrayMergeRecursiveOverrule($this->query, [ + $query = ArrayUtility::arrayMergeRecursiveOverrule($query, [ 'from' => 0, 'size' => $searchRequest->getSize(), ]); @@ -104,19 +101,20 @@ class QueryFactory /** * @param SearchRequestInterface $searchRequest + * @param array &$query */ - protected function addSearch(SearchRequestInterface $searchRequest) + protected function addSearch(SearchRequestInterface $searchRequest, array &$query) { - $this->query = ArrayUtility::setValueByPath( - $this->query, + $query = ArrayUtility::setValueByPath( + $query, 'query.bool.must.0.match._all.query', $searchRequest->getSearchTerm() ); $minimumShouldMatch = $this->configuration->getIfExists('searching.minimumShouldMatch'); if ($minimumShouldMatch) { - $this->query = ArrayUtility::setValueByPath( - $this->query, + $query = ArrayUtility::setValueByPath( + $query, 'query.bool.must.0.match._all.minimum_should_match', $minimumShouldMatch ); @@ -125,8 +123,9 @@ class QueryFactory /** * @param SearchRequestInterface $searchRequest + * @param array &$query */ - protected function addBoosts(SearchRequestInterface $searchRequest) + protected function addBoosts(SearchRequestInterface $searchRequest, array &$query) { try { $fields = $this->configuration->get('searching.boost'); @@ -147,7 +146,7 @@ class QueryFactory ]; } - $this->query = ArrayUtility::arrayMergeRecursiveOverrule($this->query, [ + $query = ArrayUtility::arrayMergeRecursiveOverrule($query, [ 'query' => [ 'bool' => [ 'should' => $boostQueryParts, @@ -156,12 +155,15 @@ class QueryFactory ]); } - protected function addFactorBoost() + /** + * @param array &$query + */ + protected function addFactorBoost(array &$query) { try { - $this->query['query'] = [ + $query['query'] = [ 'function_score' => [ - 'query' => $this->query['query'], + 'query' => $query['query'], 'field_value_factor' => $this->configuration->get('searching.fieldValueFactor'), ], ]; @@ -172,8 +174,9 @@ class QueryFactory /** * @param SearchRequestInterface $searchRequest + * @param array &$query */ - protected function addFilter(SearchRequestInterface $searchRequest) + protected function addFilter(SearchRequestInterface $searchRequest, array &$query) { if (! $searchRequest->hasFilter()) { return; @@ -188,7 +191,7 @@ class QueryFactory ]; } - $this->query = ArrayUtility::arrayMergeRecursiveOverrule($this->query, [ + $query = ArrayUtility::arrayMergeRecursiveOverrule($query, [ 'query' => [ 'bool' => [ 'filter' => $terms, @@ -199,11 +202,12 @@ class QueryFactory /** * @param SearchRequestInterface $searchRequest + * @param array &$query */ - protected function addFacets(SearchRequestInterface $searchRequest) + protected function addFacets(SearchRequestInterface $searchRequest, array &$query) { foreach ($searchRequest->getFacets() as $facet) { - $this->query = ArrayUtility::arrayMergeRecursiveOverrule($this->query, [ + $query = ArrayUtility::arrayMergeRecursiveOverrule($query, [ 'aggs' => [ $facet->getIdentifier() => [ 'terms' => [ From 51863c9e5daacfe662bbc7676a9383f9ae11eca2 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 10 Aug 2017 08:59:48 +0200 Subject: [PATCH 20/34] TASK: Cleanup PR issues --- Tests/Unit/Domain/Index/TcaIndexer/TcaTableServiceTest.php | 2 +- composer.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/Unit/Domain/Index/TcaIndexer/TcaTableServiceTest.php b/Tests/Unit/Domain/Index/TcaIndexer/TcaTableServiceTest.php index 58c03b4..7fb0280 100644 --- a/Tests/Unit/Domain/Index/TcaIndexer/TcaTableServiceTest.php +++ b/Tests/Unit/Domain/Index/TcaIndexer/TcaTableServiceTest.php @@ -64,7 +64,7 @@ class TcaTableServiceTest extends AbstractUnitTestCase ->method('getSystemWhereClause') ->will($this->returnValue('1=1 AND pages.no_search = 0')); - $whereClause =$this->subject->getWhereClause(); + $whereClause = $this->subject->getWhereClause(); $this->assertSame( '1=1 AND pages.no_search = 0', $whereClause->getStatement() diff --git a/composer.json b/composer.json index f56b60a..0061751 100644 --- a/composer.json +++ b/composer.json @@ -17,7 +17,7 @@ }, "require" : { "php": ">=7.1.0", - "typo3/cms": "~8.2", + "typo3/cms": "~8.7", "ruflin/elastica": "~3.2" }, "require-dev": { From 416e49026ea2500fa32848183528c4e066f73f51 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 10 Aug 2017 09:05:20 +0200 Subject: [PATCH 21/34] TASK: Break line exceeding max line length --- Classes/Domain/Index/TcaIndexer/TcaTableService.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Classes/Domain/Index/TcaIndexer/TcaTableService.php b/Classes/Domain/Index/TcaIndexer/TcaTableService.php index 488f949..dd3df5f 100644 --- a/Classes/Domain/Index/TcaIndexer/TcaTableService.php +++ b/Classes/Domain/Index/TcaIndexer/TcaTableService.php @@ -275,7 +275,9 @@ class TcaTableService foreach ($rootline as $pageInRootLine) { // Check configured black list if present. - if ($this->isBlackListedRootLineConfigured() && in_array($pageInRootLine['uid'], $this->getBlackListedRootLine())) { + if ($this->isBlackListedRootLineConfigured() + && in_array($pageInRootLine['uid'], $this->getBlackListedRootLine()) + ) { return true; } if ($pageInRootLine['extendToSubpages'] && ( From f311357d0eea0bc8eb8c8e89d58dc07a45a006ad Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 15 Aug 2017 08:30:23 +0200 Subject: [PATCH 22/34] TASK: Fix indentation --- Classes/Domain/Search/QueryFactory.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Classes/Domain/Search/QueryFactory.php b/Classes/Domain/Search/QueryFactory.php index ba4bc40..bdcd6f6 100644 --- a/Classes/Domain/Search/QueryFactory.php +++ b/Classes/Domain/Search/QueryFactory.php @@ -150,8 +150,8 @@ class QueryFactory 'query' => [ 'bool' => [ 'should' => $boostQueryParts, - ], - ], + ], + ], ]); } @@ -165,7 +165,7 @@ class QueryFactory 'function_score' => [ 'query' => $query['query'], 'field_value_factor' => $this->configuration->get('searching.fieldValueFactor'), - ], + ], ]; } catch (InvalidArgumentException $e) { return; From 040206c95d90e97dbd23334dc48f85de1a6bd16d Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 15 Aug 2017 09:21:04 +0200 Subject: [PATCH 23/34] FEATURE: Respect further root line cases Respect the following situations during indexing: - Page is not reachable due to broken root line. - Page is not reachable due to being below a recycler. --- .../Index/TcaIndexer/TcaTableService.php | 36 ++++++++++++++++--- .../Indexing/PagesIndexer/BrokenRootLine.xml | 20 +++++++++++ .../Indexing/PagesIndexer/Recycler.xml | 21 +++++++++++ .../Functional/Indexing/PagesIndexerTest.php | 15 ++++++-- 4 files changed, 85 insertions(+), 7 deletions(-) create mode 100644 Tests/Functional/Fixtures/Indexing/PagesIndexer/BrokenRootLine.xml create mode 100644 Tests/Functional/Fixtures/Indexing/PagesIndexer/Recycler.xml diff --git a/Classes/Domain/Index/TcaIndexer/TcaTableService.php b/Classes/Domain/Index/TcaIndexer/TcaTableService.php index dd3df5f..70bb52d 100644 --- a/Classes/Domain/Index/TcaIndexer/TcaTableService.php +++ b/Classes/Domain/Index/TcaIndexer/TcaTableService.php @@ -258,18 +258,27 @@ class TcaTableService * Checks whether the given record was blacklisted by root line. * This can be configured by typoscript as whole root lines can be black listed. * + * Also further TYPO3 mechanics are taken into account. Does a valid root + * line exist, is page inside a recycler, is inherited start- endtime + * excluded, etc. + * * @param array &$record * @return bool */ protected function isRecordBlacklistedByRootline(array &$record) { - // If no rootline exists, the record is on a unreachable page and therefore blacklisted. - if ($record['pid'] == 0) { - return false; + $pageUid = $record['pid']; + if ($this->tableName === 'pages') { + $pageUid = $record['uid']; } - $rootline = $this->objectManager->get(RootlineUtility::class, $record['pid'])->get(); - if (!isset($rootline[0])) { + try { + $rootline = $this->objectManager->get(RootlineUtility::class, $pageUid)->get(); + } catch (\RuntimeException $e) { + $this->logger->notice( + sprintf('Could not fetch rootline for page %u, because: %s', $pageUid, $e->getMessage()), + [$record, $e] + ); return true; } @@ -278,12 +287,29 @@ class TcaTableService if ($this->isBlackListedRootLineConfigured() && in_array($pageInRootLine['uid'], $this->getBlackListedRootLine()) ) { + $this->logger->info( + sprintf( + 'Record %u is black listed due to configured root line configuration of page %u.', + $record['uid'], + $pageInRootLine['uid'] + ), + [$record, $pageInRootLine] + ); return true; } + if ($pageInRootLine['extendToSubpages'] && ( ($pageInRootLine['endtime'] > 0 && $pageInRootLine['endtime'] <= time()) || ($pageInRootLine['starttime'] > 0 && $pageInRootLine['starttime'] >= time()) )) { + $this->logger->info( + sprintf( + 'Record %u is black listed due to configured timing of parent page %u.', + $record['uid'], + $pageInRootLine['uid'] + ), + [$record, $pageInRootLine] + ); return true; } } diff --git a/Tests/Functional/Fixtures/Indexing/PagesIndexer/BrokenRootLine.xml b/Tests/Functional/Fixtures/Indexing/PagesIndexer/BrokenRootLine.xml new file mode 100644 index 0000000..81f2316 --- /dev/null +++ b/Tests/Functional/Fixtures/Indexing/PagesIndexer/BrokenRootLine.xml @@ -0,0 +1,20 @@ + + + + + 3 + 2 + Some disabled page due broken root line + + + 4 + 3 + Some disabled page due to parent pages root line being broken + + + + 6 + 1 + Some enabled page due valid root line + + diff --git a/Tests/Functional/Fixtures/Indexing/PagesIndexer/Recycler.xml b/Tests/Functional/Fixtures/Indexing/PagesIndexer/Recycler.xml new file mode 100644 index 0000000..1421ad5 --- /dev/null +++ b/Tests/Functional/Fixtures/Indexing/PagesIndexer/Recycler.xml @@ -0,0 +1,21 @@ + + + + + 2 + 1 + Some disabled page due being recycler + 255 + + + 3 + 2 + Some disabled page due to parent page being recycler + + + + 6 + 1 + Some enabled page due to no be below recycler + + diff --git a/Tests/Functional/Indexing/PagesIndexerTest.php b/Tests/Functional/Indexing/PagesIndexerTest.php index 2f00d2f..244413d 100644 --- a/Tests/Functional/Indexing/PagesIndexerTest.php +++ b/Tests/Functional/Indexing/PagesIndexerTest.php @@ -64,10 +64,12 @@ class PagesIndexerTest extends AbstractFunctionalTestCase /** * @test + * @dataProvider rootLineDataSets + * @param string $dataSetPath */ - public function inheritedTimingIsRespectedDuringIndexing() + public function rootLineIsRespectedDuringIndexing($dataSetPath) { - $this->importDataSet('Tests/Functional/Fixtures/Indexing/PagesIndexer/InheritedTiming.xml'); + $this->importDataSet($dataSetPath); $objectManager = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(ObjectManager::class); $tableName = 'pages'; @@ -90,4 +92,13 @@ class PagesIndexerTest extends AbstractFunctionalTestCase $this->inject($indexer, 'connection', $connection); $indexer->indexAllDocuments(); } + + public function rootLineDataSets() + { + return [ + 'Broken root line' => ['Tests/Functional/Fixtures/Indexing/PagesIndexer/BrokenRootLine.xml'], + 'Recycler doktype' => ['Tests/Functional/Fixtures/Indexing/PagesIndexer/Recycler.xml'], + 'Extended timing to sub pages' => ['Tests/Functional/Fixtures/Indexing/PagesIndexer/InheritedTiming.xml'], + ]; + } } From 9617733826155086d6cb8694ab9f5fe317efc023 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 15 Aug 2017 09:27:27 +0200 Subject: [PATCH 24/34] BUGFIX: Fix accessing non existing property --- Classes/Domain/Search/QueryFactory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Classes/Domain/Search/QueryFactory.php b/Classes/Domain/Search/QueryFactory.php index bdcd6f6..60f22e2 100644 --- a/Classes/Domain/Search/QueryFactory.php +++ b/Classes/Domain/Search/QueryFactory.php @@ -93,7 +93,7 @@ class QueryFactory */ protected function addSize(SearchRequestInterface $searchRequest, array &$query) { - $query = ArrayUtility::arrayMergeRecursiveOverrule($this->query, [ + $query = ArrayUtility::arrayMergeRecursiveOverrule($query, [ 'from' => $searchRequest->getOffset(), 'size' => $searchRequest->getLimit(), ]); From 4b26799e7f86e7fcf0fdf91eb347eeb5f42afc70 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 15 Aug 2017 09:42:18 +0200 Subject: [PATCH 25/34] BUGFIX: Do not test with master of TYPO3 This version only supports V8 of TYPO3 CMS. --- .travis.yml | 9 --------- 1 file changed, 9 deletions(-) diff --git a/.travis.yml b/.travis.yml index 9d60175..4490198 100644 --- a/.travis.yml +++ b/.travis.yml @@ -23,15 +23,6 @@ env: - typo3DatabaseHost="127.0.0.1" - typo3DatabaseUsername="travis" - typo3DatabasePassword="" - matrix: - - TYPO3_VERSION="~8" - - TYPO3_VERSION="dev-master" - -matrix: - fast_finish: true - allow_failures: - - env: TYPO3_VERSION="dev-master" - php: 7.1 matrix: fast_finish: true From fe754964fe0319d9f3d9a51022882b4108a98d28 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Mon, 21 Aug 2017 12:10:34 +0200 Subject: [PATCH 26/34] BUGFIX: Fetch record to update --- Classes/Domain/Index/TcaIndexer.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Classes/Domain/Index/TcaIndexer.php b/Classes/Domain/Index/TcaIndexer.php index 25bef53..b5cb766 100644 --- a/Classes/Domain/Index/TcaIndexer.php +++ b/Classes/Domain/Index/TcaIndexer.php @@ -83,7 +83,10 @@ class TcaIndexer extends AbstractIndexer */ protected function getRecord($identifier) { - $record = $this->getQuery()->execute()->fetch(); + $query = $this->getQuery(); + $query = $query->andWhere($this->tcaTableService->getTableName() . '.uid = ' . (int) $identifier); + $record = $query->execute()->fetch(); + if ($record === false || $record === null) { throw new NoRecordFoundException( From efc2fb7da60448aa760671fab71d4887db84bb10 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Fri, 25 Aug 2017 11:46:46 +0200 Subject: [PATCH 27/34] BUGFIX: Remove pr issue --- Classes/Domain/Index/TcaIndexer.php | 1 - 1 file changed, 1 deletion(-) diff --git a/Classes/Domain/Index/TcaIndexer.php b/Classes/Domain/Index/TcaIndexer.php index b5cb766..c35b9ea 100644 --- a/Classes/Domain/Index/TcaIndexer.php +++ b/Classes/Domain/Index/TcaIndexer.php @@ -87,7 +87,6 @@ class TcaIndexer extends AbstractIndexer $query = $query->andWhere($this->tcaTableService->getTableName() . '.uid = ' . (int) $identifier); $record = $query->execute()->fetch(); - if ($record === false || $record === null) { throw new NoRecordFoundException( 'Record could not be fetched from database: "' . $identifier . '". Perhaps record is not active.', From b31f315ec46176ca5d95dd0300f3bdf344c98d8f Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Wed, 6 Sep 2017 22:38:46 +0200 Subject: [PATCH 28/34] BUGFIX: Allow iteration / pagination of result items Implement necessary logic based on mapped result items, not elastica result items. --- .../Connection/Elasticsearch/SearchResult.php | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/Classes/Connection/Elasticsearch/SearchResult.php b/Classes/Connection/Elasticsearch/SearchResult.php index 46ffb14..f45f02f 100644 --- a/Classes/Connection/Elasticsearch/SearchResult.php +++ b/Classes/Connection/Elasticsearch/SearchResult.php @@ -48,6 +48,13 @@ class SearchResult implements SearchResultInterface */ protected $results = []; + /** + * For Iterator interface. + * + * @var int + */ + protected $position = 0; + /** * @var ObjectManagerInterface */ @@ -121,27 +128,29 @@ class SearchResult implements SearchResultInterface // Iterator - Interface public function current() { - return $this->result->current(); + return $this->getResults()[$this->position]; } public function next() { - return $this->result->next(); + ++$this->position; + + return $this->current(); } public function key() { - return $this->result->key(); + return $this->position; } public function valid() { - return $this->result->valid(); + return isset($this->getResults()[$this->position]); } public function rewind() { - $this->result->rewind(); + $this->position = 0; } // Extbase QueryResultInterface - Implemented to support Pagination of Fluid. From be752485176938ac302dd9d88367b6c6fe026fce Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Fri, 15 Sep 2017 21:35:52 +0200 Subject: [PATCH 29/34] FEATURE: Allow configured filters Add new feature to allow pre configured filters. The filters will be applied to all requests. --- Classes/Domain/Search/SearchService.php | 18 ++++ Documentation/source/configuration.rst | 17 ++++ .../Unit/Domain/Search/SearchServiceTest.php | 86 +++++++++++++++++++ 3 files changed, 121 insertions(+) diff --git a/Classes/Domain/Search/SearchService.php b/Classes/Domain/Search/SearchService.php index 114ebfe..384f6aa 100644 --- a/Classes/Domain/Search/SearchService.php +++ b/Classes/Domain/Search/SearchService.php @@ -72,6 +72,7 @@ class SearchService $searchRequest->setConnection($this->connection); $this->addSize($searchRequest); $this->addConfiguredFacets($searchRequest); + $this->addConfiguredFilters($searchRequest); return $this->connection->search($searchRequest); } @@ -113,4 +114,21 @@ class SearchService )); } } + + /** + * Add filters from configuration, e.g. flexform or TypoScript. + * + * @param SearchRequestInterface $searchRequest + */ + protected function addConfiguredFilters(SearchRequestInterface $searchRequest) + { + try { + $searchRequest->setFilter(array_merge( + $searchRequest->getFilter(), + $this->configuration->get('searching.filter') + )); + } catch (InvalidArgumentException $e) { + // Nothing todo, no filter configured. + } + } } diff --git a/Documentation/source/configuration.rst b/Documentation/source/configuration.rst index 8ea8e29..c4677e1 100644 --- a/Documentation/source/configuration.rst +++ b/Documentation/source/configuration.rst @@ -279,6 +279,23 @@ Searching The above example will provide a facet with options for all found ``CType`` results together with a count. +.. _filter: + +``filter`` +""""""""""" + + Used by: While building search request. + + Define filter that should be set for all requests. + + Example:: + + plugin.tx_searchcore.settings.searching.filter { + property = value + } + + For Elasticsearch the fields have to be filterable, e.g. need a mapping as ``keyword``. + .. _minimumShouldMatch: ``minimumShouldMatch`` diff --git a/Tests/Unit/Domain/Search/SearchServiceTest.php b/Tests/Unit/Domain/Search/SearchServiceTest.php index 046f751..9149e51 100644 --- a/Tests/Unit/Domain/Search/SearchServiceTest.php +++ b/Tests/Unit/Domain/Search/SearchServiceTest.php @@ -21,6 +21,7 @@ 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\Domain\Model\SearchRequest; use Codappix\SearchCore\Domain\Search\SearchService; @@ -64,6 +65,10 @@ class SearchServiceTest extends AbstractUnitTestCase ->method('getIfExists') ->withConsecutive(['searching.size'], ['searching.facets']) ->will($this->onConsecutiveCalls(45, null)); + $this->configuration->expects($this->exactly(1)) + ->method('get') + ->with('searching.filter') + ->will($this->throwException(new InvalidArgumentException)); $this->connection->expects($this->once()) ->method('search') ->with($this->callback(function ($searchRequest) { @@ -83,6 +88,10 @@ class SearchServiceTest extends AbstractUnitTestCase ->method('getIfExists') ->withConsecutive(['searching.size'], ['searching.facets']) ->will($this->onConsecutiveCalls(null, null)); + $this->configuration->expects($this->exactly(1)) + ->method('get') + ->with('searching.filter') + ->will($this->throwException(new InvalidArgumentException)); $this->connection->expects($this->once()) ->method('search') ->with($this->callback(function ($searchRequest) { @@ -92,4 +101,81 @@ class SearchServiceTest extends AbstractUnitTestCase $searchRequest = new SearchRequest('SearchWord'); $this->subject->search($searchRequest); } + + /** + * @test + */ + public function configuredFilterAreAddedToRequestWithoutAnyFilter() + { + $this->configuration->expects($this->exactly(2)) + ->method('getIfExists') + ->withConsecutive(['searching.size'], ['searching.facets']) + ->will($this->onConsecutiveCalls(null, null)); + $this->configuration->expects($this->exactly(1)) + ->method('get') + ->with('searching.filter') + ->willReturn(['property' => 'something']); + + $this->connection->expects($this->once()) + ->method('search') + ->with($this->callback(function ($searchRequest) { + return $searchRequest->getFilter() === ['property' => 'something']; + })); + + $searchRequest = new SearchRequest('SearchWord'); + $this->subject->search($searchRequest); + } + + /** + * @test + */ + public function configuredFilterAreAddedToRequestWithExistingFilter() + { + $this->configuration->expects($this->exactly(2)) + ->method('getIfExists') + ->withConsecutive(['searching.size'], ['searching.facets']) + ->will($this->onConsecutiveCalls(null, null)); + $this->configuration->expects($this->exactly(1)) + ->method('get') + ->with('searching.filter') + ->willReturn(['property' => 'something']); + + $this->connection->expects($this->once()) + ->method('search') + ->with($this->callback(function ($searchRequest) { + return $searchRequest->getFilter() === [ + 'anotherProperty' => 'anything', + 'property' => 'something', + ]; + })); + + $searchRequest = new SearchRequest('SearchWord'); + $searchRequest->setFilter(['anotherProperty' => 'anything']); + $this->subject->search($searchRequest); + } + + /** + * @test + */ + public function nonConfiguredFilterIsNotChangingRequestWithExistingFilter() + { + $this->configuration->expects($this->exactly(2)) + ->method('getIfExists') + ->withConsecutive(['searching.size'], ['searching.facets']) + ->will($this->onConsecutiveCalls(null, null)); + $this->configuration->expects($this->exactly(1)) + ->method('get') + ->with('searching.filter') + ->will($this->throwException(new InvalidArgumentException)); + + $this->connection->expects($this->once()) + ->method('search') + ->with($this->callback(function ($searchRequest) { + return $searchRequest->getFilter() === ['anotherProperty' => 'anything']; + })); + + $searchRequest = new SearchRequest('SearchWord'); + $searchRequest->setFilter(['anotherProperty' => 'anything']); + $this->subject->search($searchRequest); + } } From 4de18289052beca9bf1ac9acfb77d464df1b6208 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Fri, 15 Sep 2017 21:36:52 +0200 Subject: [PATCH 30/34] FIX: Fix phpcs issues with annotations --- Classes/Domain/Search/QueryFactory.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Classes/Domain/Search/QueryFactory.php b/Classes/Domain/Search/QueryFactory.php index 60f22e2..f3efd68 100644 --- a/Classes/Domain/Search/QueryFactory.php +++ b/Classes/Domain/Search/QueryFactory.php @@ -89,7 +89,7 @@ class QueryFactory /** * @param SearchRequestInterface $searchRequest - * @param array &$query + * @param array $query */ protected function addSize(SearchRequestInterface $searchRequest, array &$query) { @@ -101,7 +101,7 @@ class QueryFactory /** * @param SearchRequestInterface $searchRequest - * @param array &$query + * @param array $query */ protected function addSearch(SearchRequestInterface $searchRequest, array &$query) { @@ -123,7 +123,7 @@ class QueryFactory /** * @param SearchRequestInterface $searchRequest - * @param array &$query + * @param array $query */ protected function addBoosts(SearchRequestInterface $searchRequest, array &$query) { @@ -156,7 +156,7 @@ class QueryFactory } /** - * @param array &$query + * @param array $query */ protected function addFactorBoost(array &$query) { @@ -174,7 +174,7 @@ class QueryFactory /** * @param SearchRequestInterface $searchRequest - * @param array &$query + * @param array $query */ protected function addFilter(SearchRequestInterface $searchRequest, array &$query) { @@ -202,7 +202,7 @@ class QueryFactory /** * @param SearchRequestInterface $searchRequest - * @param array &$query + * @param array $query */ protected function addFacets(SearchRequestInterface $searchRequest, array &$query) { From 13004e86f2694aa576cb99c08bbe3f9c3279d6a3 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Fri, 15 Sep 2017 21:54:47 +0200 Subject: [PATCH 31/34] FEATURE: Allow filter mode by not forcing a search term --- Classes/Domain/Model/SearchRequest.php | 2 +- Classes/Domain/Search/QueryFactory.php | 4 ++++ Tests/Unit/Domain/Search/QueryFactoryTest.php | 19 +++++++++++++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/Classes/Domain/Model/SearchRequest.php b/Classes/Domain/Model/SearchRequest.php index 7d6436c..39e477c 100644 --- a/Classes/Domain/Model/SearchRequest.php +++ b/Classes/Domain/Model/SearchRequest.php @@ -66,7 +66,7 @@ class SearchRequest implements SearchRequestInterface /** * @param string $query */ - public function __construct($query) + public function __construct($query = '') { $this->query = (string) $query; } diff --git a/Classes/Domain/Search/QueryFactory.php b/Classes/Domain/Search/QueryFactory.php index f3efd68..9455f80 100644 --- a/Classes/Domain/Search/QueryFactory.php +++ b/Classes/Domain/Search/QueryFactory.php @@ -105,6 +105,10 @@ class QueryFactory */ protected function addSearch(SearchRequestInterface $searchRequest, array &$query) { + if (trim($searchRequest->getSearchTerm()) === '') { + return; + } + $query = ArrayUtility::setValueByPath( $query, 'query.bool.must.0.match._all.query', diff --git a/Tests/Unit/Domain/Search/QueryFactoryTest.php b/Tests/Unit/Domain/Search/QueryFactoryTest.php index de82ebf..54fdc2a 100644 --- a/Tests/Unit/Domain/Search/QueryFactoryTest.php +++ b/Tests/Unit/Domain/Search/QueryFactoryTest.php @@ -324,4 +324,23 @@ class QueryFactoryTest extends AbstractUnitTestCase 'Boosts were not added to query.' ); } + + /** + * @test + */ + public function emptySearchStringWillNotAddSearchToQuery() + { + $searchRequest = new SearchRequest(); + + $this->configuration->expects($this->any()) + ->method('get') + ->will($this->throwException(new InvalidArgumentException)); + + $query = $this->subject->create($searchRequest); + $this->assertInstanceOf( + stdClass, + $query->toArray()['query']['match_all'], + 'Empty search request does not create expected query.' + ); + } } From 9a0d73f1c784ea09dd57ac3a9a0bcacce10a31ac Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Fri, 15 Sep 2017 22:26:52 +0200 Subject: [PATCH 32/34] FEATURE: Allow to switch from search to filter mode --- Classes/Controller/SearchController.php | 11 +++ Documentation/source/configuration.rst | 17 ++++ .../Unit/Controller/SearchControllerTest.php | 99 +++++++++++++++++++ 3 files changed, 127 insertions(+) create mode 100644 Tests/Unit/Controller/SearchControllerTest.php diff --git a/Classes/Controller/SearchController.php b/Classes/Controller/SearchController.php index 068f9a1..a02cd80 100644 --- a/Classes/Controller/SearchController.php +++ b/Classes/Controller/SearchController.php @@ -44,6 +44,17 @@ class SearchController extends ActionController parent::__construct(); } + public function initializeSearchAction() + { + if (isset($this->settings['searching']['mode']) && $this->settings['searching']['mode'] === 'filter' + && $this->request->hasArgument('searchRequest') === false + ) { + $this->request->setArguments([ + 'searchRequest' => $this->objectManager->get(SearchRequest::class), + ]); + } + } + /** * Process a search and deliver original request and result to view. * diff --git a/Documentation/source/configuration.rst b/Documentation/source/configuration.rst index c4677e1..549ca68 100644 --- a/Documentation/source/configuration.rst +++ b/Documentation/source/configuration.rst @@ -346,3 +346,20 @@ Searching factor = 2 missing = 1 } + +.. _mode: + +``mode`` +"""""""" + + Used by: Controller while preparing action. + + Define to switch from search to filter mode. + + Example:: + + plugin.tx_searchcore.settings.searching { + mode = filter + } + + Only ``filter`` is allowed as value. Will submit an empty query to switch to filter mode. diff --git a/Tests/Unit/Controller/SearchControllerTest.php b/Tests/Unit/Controller/SearchControllerTest.php new file mode 100644 index 0000000..fa38665 --- /dev/null +++ b/Tests/Unit/Controller/SearchControllerTest.php @@ -0,0 +1,99 @@ + + * + * 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\Controller\SearchController; +use Codappix\SearchCore\Domain\Model\SearchRequest; +use Codappix\SearchCore\Domain\Search\SearchService; +use Codappix\SearchCore\Tests\Unit\AbstractUnitTestCase; +use TYPO3\CMS\Extbase\Mvc\Web\Request; +use TYPO3\CMS\Extbase\Object\ObjectManager; + +class SearchControllerTest extends AbstractUnitTestCase +{ + /** + * @var SearchController + */ + protected $subject; + + /** + * @var Request + */ + protected $request; + + public function setUp() + { + \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance( + \TYPO3\CMS\Core\Cache\CacheManager::class + )->setCacheConfigurations([ + 'extbase_object' => [ + 'backend' => \TYPO3\CMS\Core\Cache\Backend\NullBackend::class, + ], + 'extbase_datamapfactory_datamap' => [ + 'backend' => \TYPO3\CMS\Core\Cache\Backend\NullBackend::class, + ], + ]); + + parent::setUp(); + + $searchService = $this->getMockBuilder(SearchService::class) + ->disableOriginalConstructor() + ->getMock(); + $this->request = new Request(); + + $this->subject = new SearchController($searchService); + $this->inject($this->subject, 'request', $this->request); + $this->inject($this->subject, 'objectManager', new ObjectManager()); + } + + /** + * @test + */ + public function searchRequestArgumentIsAddedIfModeIsFilterAndArgumentDoesNotExist() + { + $this->inject($this->subject, 'settings', [ + 'searching' => [ + 'mode' => 'filter', + ] + ]); + + $this->subject->initializeSearchAction(); + $this->assertInstanceOf( + SearchRequest::class, + $this->request->getArgument('searchRequest'), + 'Search request was not created.' + ); + } + + /** + * @test + */ + public function searchRequestArgumentIsNotAddedIfModeIsNotFilter() + { + $this->inject($this->subject, 'settings', ['searching' => []]); + + $this->subject->initializeSearchAction(); + $this->assertFalse( + $this->request->hasArgument('searchRequest'), + 'Search request should not exist.' + ); + } +} From a47b1c3a97751b477c0319867f89911e2985620f Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Fri, 15 Sep 2017 22:29:20 +0200 Subject: [PATCH 33/34] TASK: Remove unused fields for plugin content element As we do not make use of recursion or pages, we hide the inputs. --- Configuration/TCA/Overrides/tt_content.php | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Configuration/TCA/Overrides/tt_content.php diff --git a/Configuration/TCA/Overrides/tt_content.php b/Configuration/TCA/Overrides/tt_content.php new file mode 100644 index 0000000..6a976c6 --- /dev/null +++ b/Configuration/TCA/Overrides/tt_content.php @@ -0,0 +1,3 @@ + Date: Fri, 15 Sep 2017 23:47:34 +0200 Subject: [PATCH 34/34] BUGFIX: Keep existing arguments in filter mode E.g. to support paginate widget arguments. --- Classes/Controller/SearchController.php | 9 ++++-- .../Unit/Controller/SearchControllerTest.php | 29 +++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/Classes/Controller/SearchController.php b/Classes/Controller/SearchController.php index a02cd80..0fa7f73 100644 --- a/Classes/Controller/SearchController.php +++ b/Classes/Controller/SearchController.php @@ -49,9 +49,12 @@ class SearchController extends ActionController if (isset($this->settings['searching']['mode']) && $this->settings['searching']['mode'] === 'filter' && $this->request->hasArgument('searchRequest') === false ) { - $this->request->setArguments([ - 'searchRequest' => $this->objectManager->get(SearchRequest::class), - ]); + $this->request->setArguments(array_merge( + $this->request->getArguments(), + [ + 'searchRequest' => $this->objectManager->get(SearchRequest::class), + ] + )); } } diff --git a/Tests/Unit/Controller/SearchControllerTest.php b/Tests/Unit/Controller/SearchControllerTest.php index fa38665..67c6d98 100644 --- a/Tests/Unit/Controller/SearchControllerTest.php +++ b/Tests/Unit/Controller/SearchControllerTest.php @@ -83,6 +83,35 @@ class SearchControllerTest extends AbstractUnitTestCase ); } + /** + * @test + */ + public function searchRequestArgumentIsAddedToExistingArguments() + { + $this->request->setArguments([ + '@widget_0' => [ + 'currentPage' => '7', + ] + ]); + $this->inject($this->subject, 'settings', [ + 'searching' => [ + 'mode' => 'filter', + ] + ]); + + $this->subject->initializeSearchAction(); + $this->assertInstanceOf( + SearchRequest::class, + $this->request->getArgument('searchRequest'), + 'Search request was not created.' + ); + $this->assertSame( + ['currentPage' => '7'], + $this->request->getArgument('@widget_0'), + 'Existing arguments were not kept.' + ); + } + /** * @test */