From c596914ab9047e2fbad1fb41fa79e22dc16e4e7d Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Wed, 26 Aug 2015 15:21:02 +0200 Subject: [PATCH] Fix concept query builder & private field wrapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Concept queries on private fields where adding a « must » clause to restrict match on certain collections only. Boolean queries do not enforce one « should » clause match at least once a « must » clause is added to it. To make queries on private fields more robust & performants, these are now wrapped in a filtered query with a collection filter. --- .../Elastic/AST/AbstractTermNode.php | 3 ++ .../SearchEngine/Elastic/AST/TextNode.php | 2 +- .../Elastic/Search/QueryHelper.php | 8 ++--- .../SearchEngine/AST/QuotedTextNodeTest.php | 9 +++--- .../Phrasea/SearchEngine/AST/TermNodeTest.php | 30 +++++++++++-------- .../Phrasea/SearchEngine/AST/TextNodeTest.php | 9 +++--- 6 files changed, 34 insertions(+), 27 deletions(-) diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/AbstractTermNode.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/AbstractTermNode.php index 551b57278d..5a1892cce3 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/AbstractTermNode.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/AbstractTermNode.php @@ -37,6 +37,9 @@ abstract class AbstractTermNode extends Node implements TermInterface foreach ($fields as $field) { $index_fields[] = $field->getConceptPathIndexField(); } + if (!$index_fields) { + return null; + } $query = null; foreach ($concepts as $concept) { $concept_query = [ diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/TextNode.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/TextNode.php index 7bc91a1440..0811fb549e 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/TextNode.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/TextNode.php @@ -70,7 +70,7 @@ class TextNode extends AbstractTermNode implements ContextAbleInterface $concept_query = $this->buildConceptQuery($context); if ($concept_query !== null) { - $query = QueryHelper::applyBooleanClause($query, 'should', $this->buildConceptQuery($context)); + $query = QueryHelper::applyBooleanClause($query, 'should', $concept_query); } return $query; diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryHelper.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryHelper.php index d711d9bf6c..fb2103c3cf 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryHelper.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryHelper.php @@ -50,8 +50,8 @@ class QueryHelper private static function restrictQueryToCollections(array $query, array $collections) { $wrapper = []; - $wrapper['bool']['must'][0]['terms']['base_id'] = $collections; - $wrapper['bool']['must'][1] = $query; + $wrapper['filtered']['filter']['terms']['base_id'] = $collections; + $wrapper['filtered']['query'] = $query; return $wrapper; } @@ -86,9 +86,7 @@ class QueryHelper // Right to query on a private field is dependant of document collection // Here we make sure we can only match on allowed collections $query = $query_builder($fields); - $collection_query = []; - $collection_query['terms']['base_id'] = $collections_map[$hash]; - $query = self::applyBooleanClause($query, 'must', $collection_query); + $query = self::restrictQueryToCollections($query, $collections_map[$hash]); $queries[] = $query; } diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/QuotedTextNodeTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/QuotedTextNodeTest.php index 148ecdebc2..dd681b3e4c 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/QuotedTextNodeTest.php +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/QuotedTextNodeTest.php @@ -79,19 +79,20 @@ class QuotedTextNodeTest extends \PHPUnit_Framework_TestCase "lenient": true } }, { - "bool": { - "must": [{ + "filtered": { + "filter": { "terms": { "base_id": [1, 2, 3] } - }, { + }, + "query": { "multi_match": { "type": "phrase", "fields": ["private_caption.bar.fr", "private_caption.bar.en"], "query": "baz", "lenient": true } - }] + } } }] } diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/TermNodeTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/TermNodeTest.php index 1a421cbe49..77df623f0b 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/TermNodeTest.php +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/TermNodeTest.php @@ -121,23 +121,27 @@ class TermNodeTest extends \PHPUnit_Framework_TestCase "query": "/qux" } }, { - "bool": { - "must": [{ + "filtered": { + "filter": { "terms": { "base_id": [1, 2, 3] } - }], - "should": [{ - "multi_match": { - "fields": ["concept_path.bar"], - "query": "/baz" + }, + "query": { + "bool": { + "should": [{ + "multi_match": { + "fields": ["concept_path.bar"], + "query": "/baz" + } + }, { + "multi_match": { + "fields": ["concept_path.bar"], + "query": "/qux" + } + }] } - }, { - "multi_match": { - "fields": ["concept_path.bar"], - "query": "/qux" - } - }] + } } }] } diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/TextNodeTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/TextNodeTest.php index 5a32146097..396770ae1c 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/TextNodeTest.php +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/TextNodeTest.php @@ -98,19 +98,20 @@ class TextNodeTest extends \PHPUnit_Framework_TestCase "lenient": true } }, { - "bool": { - "must": [{ + "filtered": { + "filter": { "terms": { "base_id": [1, 2, 3] } - }, { + }, + "query": { "multi_match": { "fields": ["private_caption.bar.fr", "private_caption.bar.en"], "query": "baz", "operator": "and", "lenient": true } - }] + } } }] }