From eafed7fb11322d9ba1201db4290938224d1686ae Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 18 Jul 2017 09:27:46 +0200 Subject: [PATCH 1/8] TASK: Add keys for better access to Facets --- Classes/Connection/Elasticsearch/Facet.php | 2 +- Classes/Connection/Elasticsearch/SearchResult.php | 2 +- Tests/Functional/Connection/Elasticsearch/FilterTest.php | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Classes/Connection/Elasticsearch/Facet.php b/Classes/Connection/Elasticsearch/Facet.php index 7893f5f..e24a659 100644 --- a/Classes/Connection/Elasticsearch/Facet.php +++ b/Classes/Connection/Elasticsearch/Facet.php @@ -87,7 +87,7 @@ class Facet implements FacetInterface } foreach ($this->buckets as $bucket) { - $this->options[] = new FacetOption($bucket); + $this->options[$bucket['key']] = new FacetOption($bucket); } } } diff --git a/Classes/Connection/Elasticsearch/SearchResult.php b/Classes/Connection/Elasticsearch/SearchResult.php index f8ffdd8..486b6eb 100644 --- a/Classes/Connection/Elasticsearch/SearchResult.php +++ b/Classes/Connection/Elasticsearch/SearchResult.php @@ -140,7 +140,7 @@ class SearchResult implements SearchResultInterface } foreach ($this->result->getAggregations() as $aggregationName => $aggregation) { - $this->facets[] = $this->objectManager->get(Facet::class, $aggregationName, $aggregation); + $this->facets[$aggregationName] = $this->objectManager->get(Facet::class, $aggregationName, $aggregation); } } } diff --git a/Tests/Functional/Connection/Elasticsearch/FilterTest.php b/Tests/Functional/Connection/Elasticsearch/FilterTest.php index aa5a5dc..b5771f4 100644 --- a/Tests/Functional/Connection/Elasticsearch/FilterTest.php +++ b/Tests/Functional/Connection/Elasticsearch/FilterTest.php @@ -78,16 +78,16 @@ class FilterTest extends AbstractFunctionalTestCase $this->assertSame(1, count($result->getFacets()), 'Did not receive the single defined facet.'); - $facet = $result->getFacets()[0]; + $facet = current($result->getFacets()); $this->assertSame('contentTypes', $facet->getName(), 'Name of facet was not as expected.'); $this->assertSame('CType', $facet->getField(), 'Field of facet was not expected.'); $options = $facet->getOptions(); $this->assertSame(2, count($options), 'Did not receive the expected number of possible options for facet.'); - $option = $options[0]; + $option = $options['HTML']; $this->assertSame('HTML', $option->getName(), 'Option did not have expected Name.'); $this->assertSame(1, $option->getCount(), 'Option did not have expected count.'); - $option = $options[1]; + $option = $options['Header']; $this->assertSame('Header', $option->getName(), 'Option did not have expected Name.'); $this->assertSame(1, $option->getCount(), 'Option did not have expected count.'); } From 1030e8d5cfb7af8b30cc8d902f84059104df716f Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 18 Jul 2017 10:44:39 +0200 Subject: [PATCH 2/8] FEATURE: Make number of search results to fetch configurable --- Classes/Connection/SearchRequestInterface.php | 7 ++ Classes/Domain/Model/SearchRequest.php | 21 ++++ Classes/Domain/Search/QueryFactory.php | 12 +++ Classes/Domain/Search/SearchService.php | 12 +++ Documentation/source/configuration.rst | 15 +++ Tests/Unit/Domain/Search/QueryFactoryTest.php | 16 ++++ .../Unit/Domain/Search/SearchServiceTest.php | 95 +++++++++++++++++++ 7 files changed, 178 insertions(+) create mode 100644 Tests/Unit/Domain/Search/SearchServiceTest.php diff --git a/Classes/Connection/SearchRequestInterface.php b/Classes/Connection/SearchRequestInterface.php index ee779f7..7ec34ff 100644 --- a/Classes/Connection/SearchRequestInterface.php +++ b/Classes/Connection/SearchRequestInterface.php @@ -41,4 +41,11 @@ interface SearchRequestInterface * @return array */ public function getFilter(); + + /** + * Defines how many results should be fetched. + * + * @return int + */ + public function getSize(); } diff --git a/Classes/Domain/Model/SearchRequest.php b/Classes/Domain/Model/SearchRequest.php index 163f65a..7850b98 100644 --- a/Classes/Domain/Model/SearchRequest.php +++ b/Classes/Domain/Model/SearchRequest.php @@ -35,6 +35,11 @@ class SearchRequest implements SearchRequestInterface */ protected $query = ''; + /** + * @var int + */ + protected $size = 10; + /** * @var array */ @@ -112,4 +117,20 @@ class SearchRequest implements SearchRequestInterface { return $this->facets; } + + /** + * @return int + */ + public function getSize() + { + return $this->size; + } + + /** + * @param int $size + */ + public function setSize($size) + { + $this->size = (int) $size; + } } diff --git a/Classes/Domain/Search/QueryFactory.php b/Classes/Domain/Search/QueryFactory.php index 7809fb6..128e556 100644 --- a/Classes/Domain/Search/QueryFactory.php +++ b/Classes/Domain/Search/QueryFactory.php @@ -63,6 +63,7 @@ class QueryFactory */ protected function createElasticaQuery(SearchRequestInterface $searchRequest) { + $this->addSize($searchRequest); $this->addSearch($searchRequest); $this->addFilter($searchRequest); $this->addFacets($searchRequest); @@ -71,6 +72,17 @@ class QueryFactory return new \Elastica\Query($this->query); } + /** + * @param SearchRequestInterface $searchRequest + */ + protected function addSize(SearchRequestInterface $searchRequest) + { + $this->query = ArrayUtility::arrayMergeRecursiveOverrule($this->query, [ + 'from' => 0, + 'size' => $searchRequest->getSize(), + ]); + } + /** * @param SearchRequestInterface $searchRequest */ diff --git a/Classes/Domain/Search/SearchService.php b/Classes/Domain/Search/SearchService.php index 158d9d1..aaed496 100644 --- a/Classes/Domain/Search/SearchService.php +++ b/Classes/Domain/Search/SearchService.php @@ -69,11 +69,23 @@ class SearchService */ public function search(SearchRequestInterface $searchRequest) { + $this->addSize($searchRequest); $this->addConfiguredFacets($searchRequest); return $this->connection->search($searchRequest); } + /** + * Add configured size of search result items to request. + * + * @param SearchRequestInterface $searchRequest + */ + protected function addSize(SearchRequestInterface $searchRequest) + { + $size = $this->configuration->getIfExists('searching.size') ?: 10; + $searchRequest->setSize($size); + } + /** * Add facets from configuration to request. * diff --git a/Documentation/source/configuration.rst b/Documentation/source/configuration.rst index 3d8db75..4555cb1 100644 --- a/Documentation/source/configuration.rst +++ b/Documentation/source/configuration.rst @@ -189,6 +189,21 @@ options are available: Searching ^^^^^^^^^ +.. _size: + +``size`` +"""""""" + + Used by: Elasticsearch connection while building search query. + + Defined how many search results should be fetched to be available in search result. + + Example:: + + plugin.tx_searchcore.settings.searching.size = 50 + + Default if not configured is 10. + .. _facets: ``facets`` diff --git a/Tests/Unit/Domain/Search/QueryFactoryTest.php b/Tests/Unit/Domain/Search/QueryFactoryTest.php index 32c8fbd..56b0b03 100644 --- a/Tests/Unit/Domain/Search/QueryFactoryTest.php +++ b/Tests/Unit/Domain/Search/QueryFactoryTest.php @@ -145,4 +145,20 @@ class QueryFactoryTest extends AbstractUnitTestCase 'Facets were not added to query.' ); } + + /** + * @test + */ + public function sizeIsAddedToQuery() + { + $searchRequest = new SearchRequest('SearchWord'); + $searchRequest->setSize(45); + + $query = $this->subject->create($searchRequest); + $this->assertSame( + 45, + $query->toArray()['size'], + 'Size was not added to query.' + ); + } } diff --git a/Tests/Unit/Domain/Search/SearchServiceTest.php b/Tests/Unit/Domain/Search/SearchServiceTest.php new file mode 100644 index 0000000..6a90a3c --- /dev/null +++ b/Tests/Unit/Domain/Search/SearchServiceTest.php @@ -0,0 +1,95 @@ + + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +use Codappix\SearchCore\Configuration\ConfigurationContainerInterface; +use Codappix\SearchCore\Connection\ConnectionInterface; +use Codappix\SearchCore\Domain\Model\SearchRequest; +use Codappix\SearchCore\Domain\Search\SearchService; +use Codappix\SearchCore\Tests\Unit\AbstractUnitTestCase; +use TYPO3\CMS\Extbase\Object\ObjectManagerInterface; + +class SearchServiceTest extends AbstractUnitTestCase +{ + /** + * @var SearchService + */ + protected $subject; + + public function setUp() + { + parent::setUp(); + + $this->connection = $this->getMockBuilder(ConnectionInterface::class) + ->disableOriginalConstructor() + ->getMock(); + $this->configuration = $this->getMockBuilder(ConfigurationContainerInterface::class) + ->disableOriginalConstructor() + ->getMock(); + $this->objectManager = $this->getMockBuilder(ObjectManagerInterface::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->subject = new SearchService( + $this->connection, + $this->configuration, + $this->objectManager + ); + } + + /** + * @test + */ + public function sizeIsAddedFromConfiguration() + { + $this->configuration->expects($this->exactly(2)) + ->method('getIfExists') + ->withConsecutive(['searching.size'], ['searching.facets']) + ->will($this->onConsecutiveCalls(45, null)); + $this->connection->expects($this->once()) + ->method('search') + ->with($this->callback(function ($searchRequest) { + return $searchRequest->getSize() === 45; + })); + + $searchRequest = new SearchRequest('SearchWord'); + $this->subject->search($searchRequest); + } + + /** + * @test + */ + public function defaultSizeIsAddedIfNothingIsConfigured() + { + $this->configuration->expects($this->exactly(2)) + ->method('getIfExists') + ->withConsecutive(['searching.size'], ['searching.facets']) + ->will($this->onConsecutiveCalls(null, null)); + $this->connection->expects($this->once()) + ->method('search') + ->with($this->callback(function ($searchRequest) { + return $searchRequest->getSize() === 10; + })); + + $searchRequest = new SearchRequest('SearchWord'); + $this->subject->search($searchRequest); + } +} From b832a6e6b3387f67897fcd371ab1adc4acd7e6c1 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 20 Jul 2017 09:48:44 +0200 Subject: [PATCH 3/8] TASK: Allow sub indexer to exchange limit As long as it's not configurable, allow concrete implementations to exchange. Necessary for one customer at the moment. --- Classes/Domain/Index/AbstractIndexer.php | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/Classes/Domain/Index/AbstractIndexer.php b/Classes/Domain/Index/AbstractIndexer.php index b780dc5..475532b 100644 --- a/Classes/Domain/Index/AbstractIndexer.php +++ b/Classes/Domain/Index/AbstractIndexer.php @@ -83,8 +83,7 @@ abstract class AbstractIndexer implements IndexerInterface protected function getRecordGenerator() { $offset = 0; - // TODO: Make configurable. - $limit = 50; + $limit = $this->getLimit(); while (($records = $this->getRecords($offset, $limit)) !== []) { yield $records; @@ -92,6 +91,17 @@ abstract class AbstractIndexer implements IndexerInterface } } + /** + * Returns the limit to use to fetch records. + * + * @return int + */ + protected function getLimit() + { + // TODO: Make configurable. + return 50; + } + /** * @param int $offset * @param int $limit From e8a84a8ecc3e1c9a2dad2c61c7d980961bd99c59 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 20 Jul 2017 13:48:27 +0200 Subject: [PATCH 4/8] TASK: Remove temp variable --- Classes/Domain/Search/SearchService.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Classes/Domain/Search/SearchService.php b/Classes/Domain/Search/SearchService.php index aaed496..bb8b138 100644 --- a/Classes/Domain/Search/SearchService.php +++ b/Classes/Domain/Search/SearchService.php @@ -82,8 +82,9 @@ class SearchService */ protected function addSize(SearchRequestInterface $searchRequest) { - $size = $this->configuration->getIfExists('searching.size') ?: 10; - $searchRequest->setSize($size); + $searchRequest->setSize( + $this->configuration->getIfExists('searching.size') ?: 10 + ); } /** From 6462052c9b050ed6906e3ca25ae501b71ad81563 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 25 Jul 2017 09:52:17 +0200 Subject: [PATCH 5/8] FEATURE: Add ngram Provide configuration for index. Provide minimum_should_match configuration. --- .../Connection/Elasticsearch/IndexFactory.php | 39 ++++-- .../Elasticsearch/MappingFactory.php | 8 +- Classes/Domain/Search/QueryFactory.php | 41 ++++-- Documentation/source/configuration.rst | 50 +++++++ .../Elasticsearch/IndexFactoryTest.php | 132 ++++++++++++++++++ .../Elasticsearch/MappingFactoryTest.php | 86 ++++++++++++ Tests/Unit/Domain/Search/QueryFactoryTest.php | 88 +++++++++--- 7 files changed, 397 insertions(+), 47 deletions(-) create mode 100644 Tests/Unit/Connection/Elasticsearch/IndexFactoryTest.php create mode 100644 Tests/Unit/Connection/Elasticsearch/MappingFactoryTest.php diff --git a/Classes/Connection/Elasticsearch/IndexFactory.php b/Classes/Connection/Elasticsearch/IndexFactory.php index 019b091..f9fac51 100644 --- a/Classes/Connection/Elasticsearch/IndexFactory.php +++ b/Classes/Connection/Elasticsearch/IndexFactory.php @@ -20,6 +20,8 @@ namespace Codappix\SearchCore\Connection\Elasticsearch; * 02110-1301, USA. */ +use Codappix\SearchCore\Configuration\ConfigurationContainerInterface; +use Codappix\SearchCore\Configuration\InvalidArgumentException; use Elastica\Exception\ResponseException; use TYPO3\CMS\Core\SingletonInterface as Singleton; use TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface; @@ -31,6 +33,19 @@ use TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface; */ class IndexFactory implements Singleton { + /** + * @var ConfigurationContainerInterface + */ + protected $configuration; + + /** + * @param ConfigurationContainerInterface $configuration + */ + public function __construct(ConfigurationContainerInterface $configuration) + { + $this->configuration = $configuration; + } + /** * Get an index bases on TYPO3 table name. * @@ -41,19 +56,25 @@ class IndexFactory implements Singleton */ public function getIndex(Connection $connection, $documentType) { - // TODO: Fetch index name from configuration, based on $documentType. $index = $connection->getClient()->getIndex('typo3content'); - try { - // TODO: Provide configuration?! - // http://elastica.io/getting-started/storing-and-indexing-documents.html#section-analysis - $index->create(); - } catch (ResponseException $exception) { - if (stripos($exception->getMessage(), 'already exists') === false) { - throw $exception; - } + if ($index->exists() === false) { + $index->create($this->getConfigurationFor($documentType)); } return $index; } + + /** + * @param string $documentType + * @return array + */ + protected function getConfigurationFor($documentType) + { + try { + return $this->configuration->get('indexing.' . $documentType . '.index'); + } catch (InvalidArgumentException $e) { + return []; + } + } } diff --git a/Classes/Connection/Elasticsearch/MappingFactory.php b/Classes/Connection/Elasticsearch/MappingFactory.php index d4ac037..44a05c1 100644 --- a/Classes/Connection/Elasticsearch/MappingFactory.php +++ b/Classes/Connection/Elasticsearch/MappingFactory.php @@ -53,7 +53,13 @@ class MappingFactory implements Singleton { $mapping = new \Elastica\Type\Mapping(); $mapping->setType($type); - $mapping->setProperties($this->getConfiguration($type->getName())); + + $configuration = $this->getConfiguration($type->getName()); + if (isset($configuration['_all'])) { + $mapping->setAllField($configuration['_all']); + unset($configuration['_all']); + } + $mapping->setProperties($configuration); return $mapping; } diff --git a/Classes/Domain/Search/QueryFactory.php b/Classes/Domain/Search/QueryFactory.php index 7809fb6..a64bcf1 100644 --- a/Classes/Domain/Search/QueryFactory.php +++ b/Classes/Domain/Search/QueryFactory.php @@ -20,6 +20,7 @@ namespace Codappix\SearchCore\Domain\Search; * 02110-1301, USA. */ +use Codappix\SearchCore\Configuration\ConfigurationContainerInterface; use Codappix\SearchCore\Connection\ConnectionInterface; use Codappix\SearchCore\Connection\Elasticsearch\Query; use Codappix\SearchCore\Connection\SearchRequestInterface; @@ -32,6 +33,11 @@ class QueryFactory */ protected $logger; + /** + * @var ConfigurationContainerInterface + */ + protected $configuration; + /** * @var array */ @@ -39,10 +45,14 @@ class QueryFactory /** * @param \TYPO3\CMS\Core\Log\LogManager $logManager + * @param ConfigurationContainerInterface $configuration */ - public function __construct(\TYPO3\CMS\Core\Log\LogManager $logManager) - { + public function __construct( + \TYPO3\CMS\Core\Log\LogManager $logManager, + ConfigurationContainerInterface $configuration + ) { $this->logger = $logManager->getLogger(__CLASS__); + $this->configuration = $configuration; } /** @@ -76,19 +86,20 @@ class QueryFactory */ protected function addSearch(SearchRequestInterface $searchRequest) { - $this->query = ArrayUtility::arrayMergeRecursiveOverrule($this->query, [ - 'query' => [ - 'bool' => [ - 'must' => [ - [ - 'match' => [ - '_all' => $searchRequest->getSearchTerm() - ], - ], - ], - ], - ], - ]); + $this->query = ArrayUtility::setValueByPath( + $this->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.bool.must.0.match._all.minimum_should_match', + $minimumShouldMatch + ); + } } /** diff --git a/Documentation/source/configuration.rst b/Documentation/source/configuration.rst index 3d8db75..e7b8a7f 100644 --- a/Documentation/source/configuration.rst +++ b/Documentation/source/configuration.rst @@ -184,6 +184,43 @@ options are available: makes building a facet possible. +.. _index: + +``index`` +""""""""" + + Used by: Elasticsearch connection while indexing. + + Define index for Elasticsearch, have a look at the official docs: https://www.elastic.co/guide/en/elasticsearch/reference/5.2/indices-create-index.html + + Example:: + + plugin.tx_searchcore.settings.indexing.tt_content.index { + analysis { + analyzer { + ngram4 { + type = custom + tokenizer = ngram4 + char_filter { + html_strip + } + + filter { + lowercase + } + } + } + + tokenizer { + ngram4 { + type = ngram + min_gram = 4 + max_gram = 4 + } + } + } + } + .. _configuration_options_search: Searching @@ -209,3 +246,16 @@ Searching The above example will provide a facet with options for all found ``CType`` results together with a count. + +.. _minimumShouldMatch: + +``minimumShouldMatch`` +"""""""""""""""""""""" + + Used by: Elasticsearch connection while building search query. + + Define the minimum match for Elasticsearch, have a look at the official docs: https://www.elastic.co/guide/en/elasticsearch/reference/5.2/query-dsl-minimum-should-match.html + + Example:: + + plugin.tx_searchcore.settings.searching.minimumShouldMatch = 50% diff --git a/Tests/Unit/Connection/Elasticsearch/IndexFactoryTest.php b/Tests/Unit/Connection/Elasticsearch/IndexFactoryTest.php new file mode 100644 index 0000000..ee55156 --- /dev/null +++ b/Tests/Unit/Connection/Elasticsearch/IndexFactoryTest.php @@ -0,0 +1,132 @@ + + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +use Codappix\SearchCore\Configuration\ConfigurationContainerInterface; +use Codappix\SearchCore\Connection\Elasticsearch\Connection; +use Codappix\SearchCore\Connection\Elasticsearch\IndexFactory; +use Codappix\SearchCore\Tests\Unit\AbstractUnitTestCase; + +class IndexFactoryTest extends AbstractUnitTestCase +{ + /** + * @var IndexFactory + */ + protected $subject; + + public function setUp() + { + parent::setUp(); + + $this->configuration = $this->getMockBuilder(ConfigurationContainerInterface::class)->getMock(); + $this->subject = new IndexFactory($this->configuration); + } + + /** + * @test + */ + public function indexIsNotCreatedIfAlreadyExisting() + { + $indexMock = $this->getMockBuilder(\Elastica\Index::class) + ->disableOriginalConstructor() + ->getMock(); + $indexMock->expects($this->once()) + ->method('exists') + ->willReturn(true); + $indexMock->expects($this->never()) + ->method('create'); + $clientMock = $this->getMockBuilder(\Elastica\Client::class) + ->disableOriginalConstructor() + ->getMock(); + $clientMock->expects($this->once()) + ->method('getIndex') + ->with('typo3content') + ->willReturn($indexMock); + $connection = $this->getMockBuilder(Connection::class) + ->disableOriginalConstructor() + ->getMock(); + $connection->expects($this->once()) + ->method('getClient') + ->willReturn($clientMock); + + $this->subject->getIndex($connection, 'someIndex'); + } + + /** + * @test + */ + public function typoScriptConfigurationIsProvidedToIndex() + { + $configuration = [ + 'analysis' => [ + 'analyzer' => [ + 'ngram4' => [ + 'type' => 'custom', + 'tokenizer' => 'ngram4', + 'char_filter' => [ + 'html_strip', + ], + 'filter' => [ + 'lowercase', + ], + ], + ], + 'tokenizer' => [ + 'ngram4' => [ + 'type' => 'ngram', + 'min_gram' => 4, + 'max_gram' => 4, + ], + ], + ], + ]; + + $indexMock = $this->getMockBuilder(\Elastica\Index::class) + ->disableOriginalConstructor() + ->getMock(); + $indexMock->expects($this->once()) + ->method('exists') + ->willReturn(false); + $indexMock->expects($this->once()) + ->method('create') + ->with($configuration); + $clientMock = $this->getMockBuilder(\Elastica\Client::class) + ->disableOriginalConstructor() + ->getMock(); + $clientMock->expects($this->once()) + ->method('getIndex') + ->with('typo3content') + ->willReturn($indexMock); + $connection = $this->getMockBuilder(Connection::class) + ->disableOriginalConstructor() + ->getMock(); + $connection->expects($this->once()) + ->method('getClient') + ->willReturn($clientMock); + + $this->configuration->expects($this->once()) + ->method('get') + ->with('indexing.someIndex.index') + ->willReturn($configuration); + + $this->subject->getIndex($connection, 'someIndex'); + } +} diff --git a/Tests/Unit/Connection/Elasticsearch/MappingFactoryTest.php b/Tests/Unit/Connection/Elasticsearch/MappingFactoryTest.php new file mode 100644 index 0000000..8dde1cb --- /dev/null +++ b/Tests/Unit/Connection/Elasticsearch/MappingFactoryTest.php @@ -0,0 +1,86 @@ + + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +use Codappix\SearchCore\Configuration\ConfigurationContainerInterface; +use Codappix\SearchCore\Connection\Elasticsearch\MappingFactory; +use Codappix\SearchCore\Tests\Unit\AbstractUnitTestCase; + +class MappingFactoryTest extends AbstractUnitTestCase +{ + /** + * @var MappingFactory + */ + protected $subject; + + public function setUp() + { + parent::setUp(); + + $this->configuration = $this->getMockBuilder(ConfigurationContainerInterface::class)->getMock(); + $this->subject = new MappingFactory($this->configuration); + } + + /** + * @test + */ + public function typoScriptConfigurationIsProvidedToIndex() + { + $indexName = 'someIndex'; + $configuration = [ + '_all' => [ + 'type' => 'text', + 'analyzer' => 'ngram4', + ], + 'channel' => [ + 'type' => 'keyword', + ], + ]; + $type = $this->getMockBuilder(\Elastica\Type::class) + ->disableOriginalConstructor() + ->getMock(); + $type->expects($this->any()) + ->method('getName') + ->willReturn($indexName); + $this->configuration->expects($this->once()) + ->method('get') + ->with('indexing.' . $indexName . '.mapping') + ->willReturn($configuration); + + $mapping = $this->subject->getMapping($type)->toArray()[$indexName]; + $this->assertArraySubset( + [ + '_all' => $configuration['_all'] + ], + $mapping, + true, + 'Configuration of _all field was not set for mapping.' + ); + $this->assertArraySubset( + [ + 'channel' => $configuration['channel'] + ], + $mapping['properties'], + true, + 'Configuration for properties was not set for mapping.' + ); + } +} diff --git a/Tests/Unit/Domain/Search/QueryFactoryTest.php b/Tests/Unit/Domain/Search/QueryFactoryTest.php index 32c8fbd..8dcf218 100644 --- a/Tests/Unit/Domain/Search/QueryFactoryTest.php +++ b/Tests/Unit/Domain/Search/QueryFactoryTest.php @@ -20,6 +20,7 @@ namespace Codappix\SearchCore\Tests\Unit\Domain\Search; * 02110-1301, USA. */ +use Codappix\SearchCore\Configuration\ConfigurationContainerInterface; use Codappix\SearchCore\Domain\Model\FacetRequest; use Codappix\SearchCore\Domain\Model\SearchRequest; use Codappix\SearchCore\Domain\Search\QueryFactory; @@ -32,11 +33,17 @@ class QueryFactoryTest extends AbstractUnitTestCase */ protected $subject; + /** + * @var ConfigurationContainerInterface + */ + protected $configuration; + public function setUp() { parent::setUp(); - $this->subject = new QueryFactory($this->getMockedLogger()); + $this->configuration = $this->getMockBuilder(ConfigurationContainerInterface::class)->getMock(); + $this->subject = new QueryFactory($this->getMockedLogger(), $this->configuration); } /** @@ -97,27 +104,6 @@ class QueryFactoryTest extends AbstractUnitTestCase ); } - /** - * @test - */ - public function userInputIsAlwaysString() - { - $searchRequest = new SearchRequest(10); - $searchRequest->setFilter(['field' => 20]); - - $query = $this->subject->create($searchRequest); - $this->assertSame( - '10', - $query->toArray()['query']['bool']['must'][0]['match']['_all'], - 'Search word was not escaped as expected.' - ); - $this->assertSame( - '20', - $query->toArray()['query']['bool']['filter'][0]['term']['field'], - 'Search word was not escaped as expected.' - ); - } - /** * @test */ @@ -145,4 +131,62 @@ class QueryFactoryTest extends AbstractUnitTestCase 'Facets were not added to query.' ); } + + /** + * @test + */ + public function searchTermIsAddedToQuery() + { + $searchRequest = new SearchRequest('SearchWord'); + $query = $this->subject->create($searchRequest); + + $this->assertSame( + [ + 'bool' => [ + 'must' => [ + [ + 'match' => [ + '_all' => [ + 'query' => 'SearchWord', + ], + ], + ], + ], + ], + ], + $query->toArray()['query'], + 'Search term was not added to query as expected.' + ); + } + + /** + * @test + */ + public function minimumShouldMatchIsAddedToQuery() + { + $searchRequest = new SearchRequest('SearchWord'); + $this->configuration->expects($this->once()) + ->method('getIfExists') + ->with('searching.minimumShouldMatch') + ->willReturn('50%'); + $query = $this->subject->create($searchRequest); + + $this->assertArraySubset( + [ + 'bool' => [ + 'must' => [ + [ + 'match' => [ + '_all' => [ + 'minimum_should_match' => '50%', + ], + ], + ], + ], + ], + ], + $query->toArray()['query'], + 'minimum_should_match was not added to query as configured.' + ); + } } From c1cc16efa59154cb8a6516a37a86e5ad79510465 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 25 Jul 2017 15:00:25 +0200 Subject: [PATCH 6/8] BUGFIX: Fix nun working options due to miss match of ts and es As TypoScript does not provide a way to configure key less options, we use a comma separated list and explode them to stay compatible with elasticsearch. --- .../Connection/Elasticsearch/IndexFactory.php | 30 ++++++++++++++++++- Documentation/source/configuration.rst | 11 +++---- .../Elasticsearch/IndexFactoryTest.php | 14 ++++----- 3 files changed, 40 insertions(+), 15 deletions(-) diff --git a/Classes/Connection/Elasticsearch/IndexFactory.php b/Classes/Connection/Elasticsearch/IndexFactory.php index f9fac51..66b448b 100644 --- a/Classes/Connection/Elasticsearch/IndexFactory.php +++ b/Classes/Connection/Elasticsearch/IndexFactory.php @@ -24,6 +24,7 @@ use Codappix\SearchCore\Configuration\ConfigurationContainerInterface; use Codappix\SearchCore\Configuration\InvalidArgumentException; use Elastica\Exception\ResponseException; use TYPO3\CMS\Core\SingletonInterface as Singleton; +use TYPO3\CMS\Core\Utility\GeneralUtility; use TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface; /** @@ -67,14 +68,41 @@ class IndexFactory implements Singleton /** * @param string $documentType + * * @return array */ protected function getConfigurationFor($documentType) { try { - return $this->configuration->get('indexing.' . $documentType . '.index'); + $configuration = $this->configuration->get('indexing.' . $documentType . '.index'); + + if (isset($configuration['analysis']['analyzer'])) { + foreach ($configuration['analysis']['analyzer'] as $key => $analyzer) { + $configuration['analysis']['analyzer'][$key] = $this->prepareAnalyzerConfiguration($analyzer); + } + } + + return $configuration; } catch (InvalidArgumentException $e) { return []; } } + + /** + * @param array $analyzer + * + * @return array + */ + protected function prepareAnalyzerConfiguration(array $analyzer) + { + $fieldsToExplode = ['char_filter', 'filter']; + + foreach ($fieldsToExplode as $fieldToExplode) { + if (isset($analyzer[$fieldToExplode])) { + $analyzer[$fieldToExplode] = GeneralUtility::trimExplode(',', $analyzer[$fieldToExplode], true); + } + } + + return $analyzer; + } } diff --git a/Documentation/source/configuration.rst b/Documentation/source/configuration.rst index e7b8a7f..68994d7 100644 --- a/Documentation/source/configuration.rst +++ b/Documentation/source/configuration.rst @@ -201,13 +201,8 @@ options are available: ngram4 { type = custom tokenizer = ngram4 - char_filter { - html_strip - } - - filter { - lowercase - } + char_filter = html_strip + filter = lowercase, asciifolding } } @@ -221,6 +216,8 @@ options are available: } } + ``char_filter`` and ``filter`` are a comma separated list of options. + .. _configuration_options_search: Searching diff --git a/Tests/Unit/Connection/Elasticsearch/IndexFactoryTest.php b/Tests/Unit/Connection/Elasticsearch/IndexFactoryTest.php index ee55156..c73fe36 100644 --- a/Tests/Unit/Connection/Elasticsearch/IndexFactoryTest.php +++ b/Tests/Unit/Connection/Elasticsearch/IndexFactoryTest.php @@ -81,12 +81,8 @@ class IndexFactoryTest extends AbstractUnitTestCase 'ngram4' => [ 'type' => 'custom', 'tokenizer' => 'ngram4', - 'char_filter' => [ - 'html_strip', - ], - 'filter' => [ - 'lowercase', - ], + 'char_filter' => 'html_strip', + 'filter' => 'lowercase, , asciifolding', ], ], 'tokenizer' => [ @@ -99,6 +95,10 @@ class IndexFactoryTest extends AbstractUnitTestCase ], ]; + $expectedConfiguration = $configuration; + $expectedConfiguration['analysis']['analyzer']['ngram4']['char_filter'] = ['html_strip']; + $expectedConfiguration['analysis']['analyzer']['ngram4']['filter'] = ['lowercase', 'asciifolding']; + $indexMock = $this->getMockBuilder(\Elastica\Index::class) ->disableOriginalConstructor() ->getMock(); @@ -107,7 +107,7 @@ class IndexFactoryTest extends AbstractUnitTestCase ->willReturn(false); $indexMock->expects($this->once()) ->method('create') - ->with($configuration); + ->with($expectedConfiguration); $clientMock = $this->getMockBuilder(\Elastica\Client::class) ->disableOriginalConstructor() ->getMock(); From f138cd9034ea6f6beb151878acba1884cd104c57 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 25 Jul 2017 15:38:40 +0200 Subject: [PATCH 7/8] FEATURE: Add possibility to boost certain fields Allow configuration via TS to boost certain fields during searching. --- Classes/Domain/Search/QueryFactory.php | 56 ++++++++++++++++++- Documentation/source/configuration.rst | 19 +++++++ Tests/Unit/Domain/Search/QueryFactoryTest.php | 49 +++++++++++++++- 3 files changed, 120 insertions(+), 4 deletions(-) diff --git a/Classes/Domain/Search/QueryFactory.php b/Classes/Domain/Search/QueryFactory.php index 7809fb6..7f4df73 100644 --- a/Classes/Domain/Search/QueryFactory.php +++ b/Classes/Domain/Search/QueryFactory.php @@ -20,6 +20,7 @@ namespace Codappix\SearchCore\Domain\Search; * 02110-1301, USA. */ +use Codappix\SearchCore\Configuration\ConfigurationContainerInterface; use Codappix\SearchCore\Connection\ConnectionInterface; use Codappix\SearchCore\Connection\Elasticsearch\Query; use Codappix\SearchCore\Connection\SearchRequestInterface; @@ -32,6 +33,11 @@ class QueryFactory */ protected $logger; + /** + * @var ConfigurationContainerInterface + */ + protected $configuration; + /** * @var array */ @@ -39,13 +45,21 @@ class QueryFactory /** * @param \TYPO3\CMS\Core\Log\LogManager $logManager + * @param ConfigurationContainerInterface $configuration */ - public function __construct(\TYPO3\CMS\Core\Log\LogManager $logManager) - { + public function __construct( + \TYPO3\CMS\Core\Log\LogManager $logManager, + ConfigurationContainerInterface $configuration + ) { $this->logger = $logManager->getLogger(__CLASS__); + $this->configuration = $configuration; } /** + * TODO: This is not in scope Elasticsearch, therefore it should not return + * \Elastica\Query, but decide to use a more specific QueryFactory like + * ElasticaQueryFactory, once the second query is added? + * * @param SearchRequestInterface $searchRequest * * @return \Elastica\Query @@ -58,12 +72,12 @@ class QueryFactory /** * @param SearchRequestInterface $searchRequest * - * TODO: This is not in scope Elasticsearch, therefore should not return elastica. * @return \Elastica\Query */ protected function createElasticaQuery(SearchRequestInterface $searchRequest) { $this->addSearch($searchRequest); + $this->addBoosts($searchRequest); $this->addFilter($searchRequest); $this->addFacets($searchRequest); @@ -91,6 +105,42 @@ class QueryFactory ]); } + /** + * @param SearchRequestInterface $searchRequest + */ + protected function addBoosts(SearchRequestInterface $searchRequest) + { + try { + $fields = $this->configuration->get('searching.boost'); + if (!$fields) { + return; + } + } catch (InvalidArgumentException $e) { + return; + } + + $boostQueryParts = []; + + foreach ($fields as $fieldName => $boostValue) { + $boostQueryParts[] = [ + 'match' => [ + $fieldName => [ + 'query' => $searchRequest->getSearchTerm(), + 'boost' => $boostValue, + ], + ], + ]; + } + + $this->query = ArrayUtility::arrayMergeRecursiveOverrule($this->query, [ + 'query' => [ + 'bool' => [ + 'should' => $boostQueryParts, + ], + ], + ]); + } + /** * @param SearchRequestInterface $searchRequest */ diff --git a/Documentation/source/configuration.rst b/Documentation/source/configuration.rst index 3d8db75..14197fe 100644 --- a/Documentation/source/configuration.rst +++ b/Documentation/source/configuration.rst @@ -209,3 +209,22 @@ Searching The above example will provide a facet with options for all found ``CType`` results together with a count. + +.. _boost: + +``boost`` +""""""""" + + Used by: Elasticsearch connection while building search query. + + Define fields that should boost the score for results. + + Example:: + + plugin.tx_searchcore.settings.searching.boost { + search_title = 3 + search_abstract = 1.5 + } + + For further information take a look at + https://www.elastic.co/guide/en/elasticsearch/guide/2.x/_boosting_query_clauses.html diff --git a/Tests/Unit/Domain/Search/QueryFactoryTest.php b/Tests/Unit/Domain/Search/QueryFactoryTest.php index 32c8fbd..6c6b834 100644 --- a/Tests/Unit/Domain/Search/QueryFactoryTest.php +++ b/Tests/Unit/Domain/Search/QueryFactoryTest.php @@ -20,6 +20,7 @@ namespace Codappix\SearchCore\Tests\Unit\Domain\Search; * 02110-1301, USA. */ +use Codappix\SearchCore\Configuration\ConfigurationContainerInterface; use Codappix\SearchCore\Domain\Model\FacetRequest; use Codappix\SearchCore\Domain\Model\SearchRequest; use Codappix\SearchCore\Domain\Search\QueryFactory; @@ -32,11 +33,17 @@ class QueryFactoryTest extends AbstractUnitTestCase */ protected $subject; + /** + * @var ConfigurationContainerInterface + */ + protected $configuration; + public function setUp() { parent::setUp(); - $this->subject = new QueryFactory($this->getMockedLogger()); + $this->configuration = $this->getMockBuilder(ConfigurationContainerInterface::class)->getMock(); + $this->subject = new QueryFactory($this->getMockedLogger(), $this->configuration); } /** @@ -145,4 +152,44 @@ class QueryFactoryTest extends AbstractUnitTestCase 'Facets were not added to query.' ); } + + /** + * @test + */ + public function boostsAreAddedToQuery() + { + $searchRequest = new SearchRequest('SearchWord'); + + $this->configuration->expects($this->once()) + ->method('get') + ->with('searching.boost') + ->willReturn([ + 'search_title' => 3, + 'search_abstract' => 1.5, + ]); + + $query = $this->subject->create($searchRequest); + $this->assertSame( + [ + [ + 'match' => [ + 'search_title' => [ + 'query' => 'SearchWord', + 'boost' => 3, + ], + ], + ], + [ + 'match' => [ + 'search_abstract' => [ + 'query' => 'SearchWord', + 'boost' => 1.5, + ], + ], + ], + ], + $query->toArray()['query']['bool']['should'], + 'Boosts were not added to query.' + ); + } } From f436a02f5568960da77c681013229c309e5daf0b Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 27 Jul 2017 14:20:37 +0200 Subject: [PATCH 8/8] FEATURE: Add field_value_factor support through configuration --- Classes/Domain/Search/QueryFactory.php | 22 +++++- Documentation/source/configuration.rst | 19 +++++ Tests/Unit/Domain/Search/QueryFactoryTest.php | 78 +++++++++++++++++-- 3 files changed, 110 insertions(+), 9 deletions(-) diff --git a/Classes/Domain/Search/QueryFactory.php b/Classes/Domain/Search/QueryFactory.php index 7f4df73..d8acbf2 100644 --- a/Classes/Domain/Search/QueryFactory.php +++ b/Classes/Domain/Search/QueryFactory.php @@ -21,6 +21,7 @@ namespace Codappix\SearchCore\Domain\Search; */ use Codappix\SearchCore\Configuration\ConfigurationContainerInterface; +use Codappix\SearchCore\Configuration\InvalidArgumentException; use Codappix\SearchCore\Connection\ConnectionInterface; use Codappix\SearchCore\Connection\Elasticsearch\Query; use Codappix\SearchCore\Connection\SearchRequestInterface; @@ -81,6 +82,10 @@ class QueryFactory $this->addFilter($searchRequest); $this->addFacets($searchRequest); + // 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->logger->debug('Generated elasticsearch query.', [$this->query]); return new \Elastica\Query($this->query); } @@ -112,9 +117,6 @@ class QueryFactory { try { $fields = $this->configuration->get('searching.boost'); - if (!$fields) { - return; - } } catch (InvalidArgumentException $e) { return; } @@ -141,6 +143,20 @@ class QueryFactory ]); } + protected function addFactorBoost() + { + try { + $this->query['query'] = [ + 'function_score' => [ + 'query' => $this->query['query'], + 'field_value_factor' => $this->configuration->get('searching.fieldValueFactor'), + ], + ]; + } catch (InvalidArgumentException $e) { + return; + } + } + /** * @param SearchRequestInterface $searchRequest */ diff --git a/Documentation/source/configuration.rst b/Documentation/source/configuration.rst index 14197fe..4b215cb 100644 --- a/Documentation/source/configuration.rst +++ b/Documentation/source/configuration.rst @@ -228,3 +228,22 @@ Searching For further information take a look at https://www.elastic.co/guide/en/elasticsearch/guide/2.x/_boosting_query_clauses.html + +.. _fieldValueFactor: + +``fieldValueFactor`` +"""""""""""""""""""" + + Used by: Elasticsearch connection while building search query. + + Define a field to use as a factor for scoring. The configuration is passed through to elastic + search ``field_value_factor``, see: https://www.elastic.co/guide/en/elasticsearch/reference/5.2/query-dsl-function-score-query.html#function-field-value-factor + + Example:: + + plugin.tx_searchcore.settings.searching.field_value_factor { + field = rootlineLevel + modifier = reciprocal + factor = 2 + missing = 1 + } diff --git a/Tests/Unit/Domain/Search/QueryFactoryTest.php b/Tests/Unit/Domain/Search/QueryFactoryTest.php index 6c6b834..2b27af2 100644 --- a/Tests/Unit/Domain/Search/QueryFactoryTest.php +++ b/Tests/Unit/Domain/Search/QueryFactoryTest.php @@ -21,6 +21,7 @@ namespace Codappix\SearchCore\Tests\Unit\Domain\Search; */ use Codappix\SearchCore\Configuration\ConfigurationContainerInterface; +use Codappix\SearchCore\Configuration\InvalidArgumentException; use Codappix\SearchCore\Domain\Model\FacetRequest; use Codappix\SearchCore\Domain\Model\SearchRequest; use Codappix\SearchCore\Domain\Search\QueryFactory; @@ -53,6 +54,10 @@ class QueryFactoryTest extends AbstractUnitTestCase { $searchRequest = new SearchRequest('SearchWord'); + $this->configuration->expects($this->any()) + ->method('get') + ->will($this->throwException(new InvalidArgumentException)); + $query = $this->subject->create($searchRequest); $this->assertInstanceOf( \Elastica\Query::class, @@ -66,6 +71,10 @@ class QueryFactoryTest extends AbstractUnitTestCase */ public function filterIsAddedToQuery() { + $this->configuration->expects($this->any()) + ->method('get') + ->will($this->throwException(new InvalidArgumentException)); + $searchRequest = new SearchRequest('SearchWord'); $searchRequest->setFilter(['field' => 'content']); @@ -84,6 +93,10 @@ class QueryFactoryTest extends AbstractUnitTestCase */ public function emptyFilterIsNotAddedToQuery() { + $this->configuration->expects($this->any()) + ->method('get') + ->will($this->throwException(new InvalidArgumentException)); + $searchRequest = new SearchRequest('SearchWord'); $searchRequest->setFilter([ 'field' => '', @@ -109,6 +122,10 @@ class QueryFactoryTest extends AbstractUnitTestCase */ public function userInputIsAlwaysString() { + $this->configuration->expects($this->any()) + ->method('get') + ->will($this->throwException(new InvalidArgumentException)); + $searchRequest = new SearchRequest(10); $searchRequest->setFilter(['field' => 20]); @@ -130,6 +147,9 @@ class QueryFactoryTest extends AbstractUnitTestCase */ public function facetsAreAddedToQuery() { + $this->configuration->expects($this->any()) + ->method('get') + ->will($this->throwException(new InvalidArgumentException)); $searchRequest = new SearchRequest('SearchWord'); $searchRequest->addFacet(new FacetRequest('Identifier', 'FieldName')); $searchRequest->addFacet(new FacetRequest('Identifier 2', 'FieldName 2')); @@ -160,13 +180,16 @@ class QueryFactoryTest extends AbstractUnitTestCase { $searchRequest = new SearchRequest('SearchWord'); - $this->configuration->expects($this->once()) + $this->configuration->expects($this->exactly(2)) ->method('get') - ->with('searching.boost') - ->willReturn([ - 'search_title' => 3, - 'search_abstract' => 1.5, - ]); + ->withConsecutive(['searching.boost'], ['searching.fieldValueFactor']) + ->will($this->onConsecutiveCalls( + [ + 'search_title' => 3, + 'search_abstract' => 1.5, + ], + $this->throwException(new InvalidArgumentException) + )); $query = $this->subject->create($searchRequest); $this->assertSame( @@ -192,4 +215,47 @@ class QueryFactoryTest extends AbstractUnitTestCase 'Boosts were not added to query.' ); } + + /** + * @test + */ + public function factorBoostIsAddedToQuery() + { + $searchRequest = new SearchRequest('SearchWord'); + $fieldConfig = [ + 'field' => 'rootlineLevel', + 'modifier' => 'reciprocal', + 'factor' => '2', + 'missing' => '1', + ]; + $this->configuration->expects($this->exactly(2)) + ->method('get') + ->withConsecutive(['searching.boost'], ['searching.fieldValueFactor']) + ->will($this->onConsecutiveCalls( + $this->throwException(new InvalidArgumentException), + $fieldConfig + )); + + $query = $this->subject->create($searchRequest); + $this->assertSame( + [ + 'function_score' => [ + 'query' => [ + 'bool' => [ + 'must' => [ + [ + 'match' => [ + '_all' => 'SearchWord', + ], + ], + ], + ], + ], + 'field_value_factor' => $fieldConfig, + ], + ], + $query->toArray()['query'], + 'Boosts were not added to query.' + ); + } }