From 47d974a466ea231d50938fb0c267a58d71ca1f94 Mon Sep 17 00:00:00 2001 From: aina-esokia Date: Tue, 23 Apr 2019 12:10:25 +0400 Subject: [PATCH] PHRAS-2497 #comment port to 4.1 Search issue when query mix data from Public and Private field #time 2h --- .../Elastic/AST/QuotedTextNode.php | 2 +- .../SearchEngine/Elastic/AST/RawNode.php | 2 +- .../SearchEngine/Elastic/AST/TermNode.php | 6 ++++-- .../SearchEngine/Elastic/AST/TextNode.php | 9 ++++----- .../Elastic/Search/QueryHelper.php | 6 +++--- .../SearchEngine/AST/QuotedTextNodeTest.php | 16 +++++++++++++--- .../Phrasea/SearchEngine/AST/TermNodeTest.php | 10 ++++++++-- .../Phrasea/SearchEngine/AST/TextNodeTest.php | 19 ++++++++++++++++--- 8 files changed, 50 insertions(+), 20 deletions(-) diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/QuotedTextNode.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/QuotedTextNode.php index 8d2d5fd9ad..8c330545fd 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/QuotedTextNode.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/QuotedTextNode.php @@ -43,7 +43,7 @@ class QuotedTextNode extends Node $private_fields = $context->getPrivateFields(); $private_fields = ValueChecker::filterByValueCompatibility($private_fields, $this->text); - foreach (QueryHelper::wrapPrivateFieldQueries($private_fields, $query_builder) as $private_field_query) { + foreach (QueryHelper::wrapPrivateFieldQueries($private_fields, $unrestricted_fields, $query_builder) as $private_field_query) { $query = QueryHelper::applyBooleanClause($query, 'should', $private_field_query); } diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/RawNode.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/RawNode.php index 784fadcbfa..c6cdd7d5cc 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/RawNode.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/RawNode.php @@ -61,7 +61,7 @@ class RawNode extends Node $private_fields = $context->getPrivateFields(); $private_fields = ValueChecker::filterByValueCompatibility($private_fields, $this->text); - foreach (QueryHelper::wrapPrivateFieldQueries($private_fields, $query_builder) as $private_field_query) { + foreach (QueryHelper::wrapPrivateFieldQueries($private_fields, $unrestricted_fields, $query_builder) as $private_field_query) { $query = QueryHelper::applyBooleanClause($query, 'should', $private_field_query); } diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/TermNode.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/TermNode.php index 8066fa756c..c587c54592 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/TermNode.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/TermNode.php @@ -19,9 +19,11 @@ class TermNode extends AbstractTermNode return $query; }; - $query = $query_builder($context->getUnrestrictedFields()); + $unrestricted_fields = $context->getUnrestrictedFields(); $private_fields = $context->getPrivateFields(); - foreach (QueryHelper::wrapPrivateFieldQueries($private_fields, $query_builder) as $concept_query) { + $query = $query_builder($unrestricted_fields); + + foreach (QueryHelper::wrapPrivateFieldQueries($private_fields, $unrestricted_fields, $query_builder) as $concept_query) { $query = QueryHelper::applyBooleanClause($query, 'should', $concept_query); } diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/TextNode.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/TextNode.php index 49b8d49da9..4bc98ec6b9 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/TextNode.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/TextNode.php @@ -66,12 +66,11 @@ class TextNode extends AbstractTermNode implements ContextAbleInterface return $query; }; - // Unrestricted fields - $query = $query_builder($context->getUnrestrictedFields()); - - // Private fields + $unrestricted_fields = $context->getUnrestrictedFields(); $private_fields = $context->getPrivateFields(); - foreach (QueryHelper::wrapPrivateFieldQueries($private_fields, $query_builder) as $private_field_query) { + + $query = $query_builder($unrestricted_fields); + foreach (QueryHelper::wrapPrivateFieldQueries($private_fields, $unrestricted_fields, $query_builder) as $private_field_query) { $query = QueryHelper::applyBooleanClause($query, 'should', $private_field_query); } diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryHelper.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryHelper.php index 746266e341..1acdd5935c 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryHelper.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryHelper.php @@ -10,13 +10,13 @@ class QueryHelper { private function __construct() {} - public static function wrapPrivateFieldQueries(array $fields, \Closure $query_builder) + public static function wrapPrivateFieldQueries(array $private_fields, array $unrestricted_fields, \Closure $query_builder) { // We make a boolean clause for each collection set to shrink query size // (instead of a clause for each field, with his collection set) $fields_map = []; $collections_map = []; - foreach ($fields as $field) { + foreach ($private_fields as $field) { $collections = $field->getDependantCollections(); $hash = self::hashCollections($collections); $collections_map[$hash] = $collections; @@ -31,7 +31,7 @@ class QueryHelper foreach ($fields_map as $hash => $fields) { // 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); + $query = $query_builder(array_merge($fields, $unrestricted_fields)); if ($query !== null) { $queries[] = self::restrictQueryToCollections($query, $collections_map[$hash]); } diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/QuotedTextNodeTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/QuotedTextNodeTest.php index dc5e9995a5..c667e3ff62 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/QuotedTextNodeTest.php +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/QuotedTextNodeTest.php @@ -47,7 +47,9 @@ class QuotedTextNodeTest extends \PHPUnit_Framework_TestCase public function testQueryBuildWithPrivateFields() { - $public_field = new Field('foo', FieldMapping::TYPE_STRING, ['private' => false]); + $public_field = new Field('foo', FieldMapping::TYPE_STRING, [ + 'private' => false + ]); $private_field = new Field('bar', FieldMapping::TYPE_STRING, [ 'private' => true, 'used_by_collections' => [1, 2, 3] @@ -75,7 +77,10 @@ class QuotedTextNodeTest extends \PHPUnit_Framework_TestCase "should": [{ "multi_match": { "type": "phrase", - "fields": ["foo.fr", "foo.en"], + "fields": [ + "foo.fr", + "foo.en" + ], "query": "baz", "lenient": true } @@ -89,7 +94,12 @@ class QuotedTextNodeTest extends \PHPUnit_Framework_TestCase "query": { "multi_match": { "type": "phrase", - "fields": ["private_caption.bar.fr", "private_caption.bar.en"], + "fields": [ + "private_caption.bar.fr", + "private_caption.bar.en", + "foo.fr", + "foo.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 d78422d30a..aa68edbfbd 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/TermNodeTest.php +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/TermNodeTest.php @@ -126,12 +126,18 @@ class TermNodeTest extends \PHPUnit_Framework_TestCase "bool": { "should": [{ "multi_match": { - "fields": ["concept_path.bar"], + "fields": [ + "concept_path.bar", + "concept_path.foo" + ], "query": "/baz" } }, { "multi_match": { - "fields": ["concept_path.bar"], + "fields": [ + "concept_path.bar", + "concept_path.foo" + ], "query": "/qux" } }] diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/TextNodeTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/TextNodeTest.php index 6f954dd398..b2cde860b1 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/TextNodeTest.php +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/TextNodeTest.php @@ -109,7 +109,12 @@ class TextNodeTest extends \PHPUnit_Framework_TestCase }, "query": { "multi_match": { - "fields": ["private_caption.bar.fr", "private_caption.bar.en"], + "fields": [ + "private_caption.bar.fr", + "private_caption.bar.en", + "foo.fr", + "foo.en" + ], "query": "baz", "type": "cross_fields", "operator": "and", @@ -216,7 +221,12 @@ class TextNodeTest extends \PHPUnit_Framework_TestCase "bool": { "should": [{ "multi_match": { - "fields": ["private_caption.bar.fr", "private_caption.bar.en"], + "fields": [ + "private_caption.bar.fr", + "private_caption.bar.en", + "foo.fr", + "foo.en" + ], "query": "baz", "type": "cross_fields", "operator": "and", @@ -224,7 +234,10 @@ class TextNodeTest extends \PHPUnit_Framework_TestCase } }, { "multi_match": { - "fields": ["concept_path.bar"], + "fields": [ + "concept_path.bar", + "concept_path.foo" + ], "query": "/qux" } }]