From 02ef86b67b8f2eab59528496b977ec5fe35c72f1 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Wed, 29 Nov 2017 18:57:09 +0100 Subject: [PATCH 1/4] FEATURE: Provide new feature to configure fields to search in This enables you to search only in some fields. Also if some fields contain mapping, you can add them in addition to e.g. `_all`. --- Classes/Domain/Search/QueryFactory.php | 18 ++++++++---------- Configuration/TypoScript/setup.txt | 6 ++++++ .../source/configuration/searching.rst | 13 ++++++++++++- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/Classes/Domain/Search/QueryFactory.php b/Classes/Domain/Search/QueryFactory.php index f73372d..980b763 100644 --- a/Classes/Domain/Search/QueryFactory.php +++ b/Classes/Domain/Search/QueryFactory.php @@ -99,20 +99,18 @@ class QueryFactory return; } - $query = ArrayUtility::setValueByPath( - $query, - 'query.bool.must.0.match._all.query', - $searchRequest->getSearchTerm() - ); + $matchExpression = [ + 'type' => 'most_fields', + 'query' => $searchRequest->getSearchTerm(), + 'fields' => GeneralUtility::trimExplode(',', $this->configuration->get('searching.fields.query')), + ]; $minimumShouldMatch = $this->configuration->getIfExists('searching.minimumShouldMatch'); if ($minimumShouldMatch) { - $query = ArrayUtility::setValueByPath( - $query, - 'query.bool.must.0.match._all.minimum_should_match', - $minimumShouldMatch - ); + $matchExpression['minimum_should_match'] = $minimumShouldMatch; } + + $query = ArrayUtility::setValueByPath($query, 'query.bool.must.0.multi_match', $matchExpression); } protected function addBoosts(SearchRequestInterface $searchRequest, array &$query) diff --git a/Configuration/TypoScript/setup.txt b/Configuration/TypoScript/setup.txt index 67612e1..1a1577f 100644 --- a/Configuration/TypoScript/setup.txt +++ b/Configuration/TypoScript/setup.txt @@ -21,6 +21,12 @@ plugin { abstractFields = {$plugin.tx_searchcore.settings.indexing.pages.abstractFields} } } + + searching { + fields { + query = _all + } + } } } } diff --git a/Documentation/source/configuration/searching.rst b/Documentation/source/configuration/searching.rst index e953dcb..ae73fad 100644 --- a/Documentation/source/configuration/searching.rst +++ b/Documentation/source/configuration/searching.rst @@ -151,7 +151,18 @@ filtering. This way you can use arbitrary filter names and map them to existing fields ------ -Defines the fields to fetch from elasticsearch. Two sub entries exist: +Defines the fields to fetch and search from elasticsearch. With the following sub keys: + +``query`` defines the fields to search in. Default is ``_all`` from 5.x times of elasticsearch. +Configure a comma separated list of fields to search in. This is necessary if you have configured +special mapping for some fields, or just want to search some fields. +The most hits get ranked highest. The following is an example configuration:: + + fields { + query = _all, city + } + +The following sub properties configure the fields to fetch from elasticsearch: First ``stored_fields`` which is a list of comma separated fields which actually exist and will be added. Typically you will use ``_source`` to fetch the whole indexed fields. From 0006148a525e38d5af3dc94fda56c16431b0d994 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Wed, 29 Nov 2017 19:43:16 +0100 Subject: [PATCH 2/4] TASK: Fix broken functional tests Add new default TypoScript to not break tests. --- Tests/Functional/Fixtures/BasicSetup.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Tests/Functional/Fixtures/BasicSetup.ts b/Tests/Functional/Fixtures/BasicSetup.ts index 1e2b3a9..b7b0c6a 100644 --- a/Tests/Functional/Fixtures/BasicSetup.ts +++ b/Tests/Functional/Fixtures/BasicSetup.ts @@ -42,6 +42,10 @@ plugin { field = CType } } + + fields { + query = _all + } } } } From e3151e802c55aec78d7ff99fa7dffc04fe5720a6 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Wed, 29 Nov 2017 19:52:10 +0100 Subject: [PATCH 3/4] TASK: Fix broken unit tests Adjust tests to match new queries built with multiple fields. --- Tests/Unit/Domain/Search/QueryFactoryTest.php | 84 ++++++++++--------- 1 file changed, 45 insertions(+), 39 deletions(-) diff --git a/Tests/Unit/Domain/Search/QueryFactoryTest.php b/Tests/Unit/Domain/Search/QueryFactoryTest.php index 6301cdd..ba845d4 100644 --- a/Tests/Unit/Domain/Search/QueryFactoryTest.php +++ b/Tests/Unit/Domain/Search/QueryFactoryTest.php @@ -56,9 +56,7 @@ class QueryFactoryTest extends AbstractUnitTestCase { $searchRequest = new SearchRequest('SearchWord'); - $this->configuration->expects($this->any()) - ->method('get') - ->will($this->throwException(new InvalidArgumentException)); + $this->configureConfigurationMockWithDefault(); $query = $this->subject->create($searchRequest); $this->assertInstanceOf( @@ -73,9 +71,7 @@ class QueryFactoryTest extends AbstractUnitTestCase */ public function filterIsAddedToQuery() { - $this->configuration->expects($this->any()) - ->method('get') - ->will($this->throwException(new InvalidArgumentException)); + $this->configureConfigurationMockWithDefault(); $searchRequest = new SearchRequest('SearchWord'); $searchRequest->setFilter(['field' => 'content']); @@ -95,9 +91,7 @@ class QueryFactoryTest extends AbstractUnitTestCase */ public function emptyFilterIsNotAddedToQuery() { - $this->configuration->expects($this->any()) - ->method('get') - ->will($this->throwException(new InvalidArgumentException)); + $this->configureConfigurationMockWithDefault(); $searchRequest = new SearchRequest('SearchWord'); $searchRequest->setFilter([ @@ -122,9 +116,7 @@ class QueryFactoryTest extends AbstractUnitTestCase */ public function facetsAreAddedToQuery() { - $this->configuration->expects($this->any()) - ->method('get') - ->will($this->throwException(new InvalidArgumentException)); + $this->configureConfigurationMockWithDefault(); $searchRequest = new SearchRequest('SearchWord'); $searchRequest->addFacet(new FacetRequest('Identifier', 'FieldName')); $searchRequest->addFacet(new FacetRequest('Identifier 2', 'FieldName 2')); @@ -153,9 +145,7 @@ class QueryFactoryTest extends AbstractUnitTestCase */ public function sizeIsAddedToQuery() { - $this->configuration->expects($this->any()) - ->method('get') - ->will($this->throwException(new InvalidArgumentException)); + $this->configureConfigurationMockWithDefault(); $searchRequest = new SearchRequest('SearchWord'); $searchRequest->setLimit(45); $searchRequest->setOffset(35); @@ -179,9 +169,7 @@ class QueryFactoryTest extends AbstractUnitTestCase public function searchTermIsAddedToQuery() { $searchRequest = new SearchRequest('SearchWord'); - $this->configuration->expects($this->any()) - ->method('get') - ->will($this->throwException(new InvalidArgumentException)); + $this->configureConfigurationMockWithDefault(); $query = $this->subject->create($searchRequest); $this->assertSame( @@ -189,9 +177,11 @@ class QueryFactoryTest extends AbstractUnitTestCase 'bool' => [ 'must' => [ [ - 'match' => [ - '_all' => [ - 'query' => 'SearchWord', + 'multi_match' => [ + 'type' => 'most_fields', + 'query' => 'SearchWord', + 'fields' => [ + '_all', ], ], ], @@ -219,9 +209,7 @@ class QueryFactoryTest extends AbstractUnitTestCase '50%', null )); - $this->configuration->expects($this->any()) - ->method('get') - ->will($this->throwException(new InvalidArgumentException)); + $this->configureConfigurationMockWithDefault(); $query = $this->subject->create($searchRequest); $this->assertArraySubset( @@ -229,10 +217,13 @@ class QueryFactoryTest extends AbstractUnitTestCase 'bool' => [ 'must' => [ [ - 'match' => [ - '_all' => [ - 'minimum_should_match' => '50%', + 'multi_match' => [ + 'type' => 'most_fields', + 'query' => 'SearchWord', + 'fields' => [ + '_all', ], + 'minimum_should_match' => '50%', ], ], ], @@ -253,12 +244,14 @@ class QueryFactoryTest extends AbstractUnitTestCase $this->configuration->expects($this->any()) ->method('get') ->withConsecutive( + ['searching.fields.query'], ['searching.boost'], ['searching.fields.stored_fields'], ['searching.fields.script_fields'], ['searching.fieldValueFactor'] ) ->will($this->onConsecutiveCalls( + '_all', [ 'search_title' => 3, 'search_abstract' => 1.5, @@ -308,12 +301,14 @@ class QueryFactoryTest extends AbstractUnitTestCase $this->configuration->expects($this->any()) ->method('get') ->withConsecutive( + ['searching.fields.query'], ['searching.boost'], ['searching.fields.stored_fields'], ['searching.fields.script_fields'], ['searching.fieldValueFactor'] ) ->will($this->onConsecutiveCalls( + '_all', $this->throwException(new InvalidArgumentException), $this->throwException(new InvalidArgumentException), $this->throwException(new InvalidArgumentException), @@ -328,9 +323,11 @@ class QueryFactoryTest extends AbstractUnitTestCase 'bool' => [ 'must' => [ [ - 'match' => [ - '_all' => [ - 'query' => 'SearchWord', + 'multi_match' => [ + 'type' => 'most_fields', + 'query' => 'SearchWord', + 'fields' => [ + '_all', ], ], ], @@ -352,9 +349,7 @@ class QueryFactoryTest extends AbstractUnitTestCase { $searchRequest = new SearchRequest(); - $this->configuration->expects($this->any()) - ->method('get') - ->will($this->throwException(new InvalidArgumentException)); + $this->configureConfigurationMockWithDefault(); $query = $this->subject->create($searchRequest); $this->assertInstanceOf( @@ -433,12 +428,14 @@ class QueryFactoryTest extends AbstractUnitTestCase $this->configuration->expects($this->any()) ->method('get') ->withConsecutive( + ['searching.fields.query'], ['searching.boost'], ['searching.fields.stored_fields'], ['searching.fields.script_fields'], ['searching.fieldValueFactor'] ) ->will($this->onConsecutiveCalls( + '_all', $this->throwException(new InvalidArgumentException), $this->throwException(new InvalidArgumentException), [ @@ -522,9 +519,7 @@ class QueryFactoryTest extends AbstractUnitTestCase ] )); - $this->configuration->expects($this->any()) - ->method('get') - ->will($this->throwException(new InvalidArgumentException)); + $this->configureConfigurationMockWithDefault(); $query = $this->subject->create($searchRequest); $this->assertSame( @@ -559,9 +554,7 @@ class QueryFactoryTest extends AbstractUnitTestCase null )); - $this->configuration->expects($this->any()) - ->method('get') - ->will($this->throwException(new InvalidArgumentException)); + $this->configureConfigurationMockWithDefault(); $query = $this->subject->create($searchRequest); $this->assertTrue( @@ -569,4 +562,17 @@ class QueryFactoryTest extends AbstractUnitTestCase 'Sort was added to query even if not configured.' ); } + + protected function configureConfigurationMockWithDefault() + { + $this->configuration->expects($this->any()) + ->method('get') + ->will($this->returnCallback(function ($configName) { + if ($configName === 'searching.fields.query') { + return '_all'; + } + + throw new InvalidArgumentException(); + })); + } } From 5ba860b8de7854ecb4ed76c6ae492259645bae37 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Wed, 29 Nov 2017 20:00:10 +0100 Subject: [PATCH 4/4] TASK: Add new test covering new feature --- Tests/Unit/Domain/Search/QueryFactoryTest.php | 49 ++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/Tests/Unit/Domain/Search/QueryFactoryTest.php b/Tests/Unit/Domain/Search/QueryFactoryTest.php index ba845d4..e0fc86d 100644 --- a/Tests/Unit/Domain/Search/QueryFactoryTest.php +++ b/Tests/Unit/Domain/Search/QueryFactoryTest.php @@ -359,6 +359,54 @@ class QueryFactoryTest extends AbstractUnitTestCase ); } + /** + * @test + */ + public function configuredQueryFieldsAreAddedToQuery() + { + $searchRequest = new SearchRequest('SearchWord'); + + $this->configuration->expects($this->any()) + ->method('get') + ->withConsecutive( + ['searching.fields.query'], + ['searching.boost'], + ['searching.fields.stored_fields'], + ['searching.fields.script_fields'], + ['searching.fieldValueFactor'] + ) + ->will($this->onConsecutiveCalls( + '_all, field1, field2', + $this->throwException(new InvalidArgumentException), + $this->throwException(new InvalidArgumentException), + $this->throwException(new InvalidArgumentException), + $this->throwException(new InvalidArgumentException) + )); + + $query = $this->subject->create($searchRequest); + $this->assertArraySubset( + [ + 'bool' => [ + 'must' => [ + [ + 'multi_match' => [ + 'type' => 'most_fields', + 'query' => 'SearchWord', + 'fields' => [ + '_all', + 'field1', + 'field2', + ], + ], + ], + ], + ], + ], + $query->toArray()['query'], + 'Configured fields were not added to query as configured.' + ); + } + /** * @test */ @@ -462,7 +510,6 @@ class QueryFactoryTest extends AbstractUnitTestCase $query->toArray()['script_fields'], 'Script fields were not added to query as expected.' ); - } /**