Merge pull request #1424 from b2ihealthcare/issue/SO-6491-nested-query-scoring

SO-6491: Resolve issue with tracking document scores in EsQueryBuilder

Author: cmark

commons/com.b2international.index.tests/src/com/b2international/index/BoolQueryTest.java

@@ -16,6 +16,7 @@
 package com.b2international.index;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 import java.util.*;
 import java.util.function.BiFunction;
@@ -28,6 +29,7 @@
 import org.slf4j.LoggerFactory;
 
 import com.b2international.index.Fixtures.Data;
+import com.b2international.index.Fixtures.ParentData;
 import com.b2international.index.admin.IndexAdmin;
 import com.b2international.index.es.query.EsQueryBuilder;
 import com.b2international.index.mapping.DocumentMapping;
@@ -46,7 +48,7 @@ public class BoolQueryTest extends BaseIndexTest {
 
 	@Override
 	protected Collection<Class<?>> getTypes() {
-		return List.of(Fixtures.Data.class);
+		return List.of(Fixtures.Data.class, Fixtures.ParentData.class);
 	}
 
 	@Test
@@ -159,6 +161,26 @@ public void mergeDisjunctTermFilterClausesOnCollection() throws Exception {
 		assertEquals(BoolQueryBuilder.NAME, esQueryBuilder.getName());
 		assertEquals(3, ((BoolQueryBuilder) esQueryBuilder).filter().size());
 	}
+	
+	@Test
+	public void nestedScoringQuery() throws Exception {
+		Expression actual = Expressions.bool()
+			// This clause requires scoring
+			.should(Expressions.nestedMatch("nestedData", Expressions.matchTextAll("nestedData.analyzedField", "multi term query")))
+			// This one does not
+			.should(Expressions.nestedMatch("nestedData", Expressions.exactMatch("nestedData.field2", "field2value")))
+			.build();
+		
+		IndexAdmin indexAdmin = index().admin();
+		DocumentMapping mapping = indexAdmin.getIndexMapping().getMapping(ParentData.class);
+		Map<String, Object> settings = indexAdmin.settings();
+		
+		EsQueryBuilder esQueryBuilder = new EsQueryBuilder(mapping, settings, LOG);
+		esQueryBuilder.build(actual);
+		
+		// We are only interested in the value of the flag, not the actual query
+		assertTrue(esQueryBuilder.needsScoring());
+	}
 
 	private Expression generateDeepBooleanClause(int depth, BiFunction<ExpressionBuilder, Expression, ExpressionBuilder> boolClause) {
 		ExpressionBuilder root = Expressions.bool();

commons/com.b2international.index/src/com/b2international/index/es/query/EsQueryBuilder.java

@@ -74,6 +74,10 @@ public boolean needsScoring() {
 		return needsScoring;
 	}
 
+	public void enableScoring() {
+		needsScoring = true;
+	}
+	
 	public QueryBuilder build(Expression expression) {
 		checkNotNull(expression, "expression");
 		visit(expression);
@@ -156,7 +160,7 @@ private void visit(ScriptScoreExpression expression) {
 		visit(inner);
 		final QueryBuilder innerQuery = deque.pop();
 
-		needsScoring = true;
+		enableScoring();
 		deque.push(QueryBuilders
 				.functionScoreQuery(innerQuery, ScoreFunctionBuilders.scriptFunction(expression.toEsScript(mapping)))
 				.boostMode(CombineFunction.REPLACE));
@@ -180,8 +184,8 @@ private void visit(BoolExpression bool) {
 			// visit the item and immediately pop the deque item back
 			final EsQueryBuilder innerQueryBuilder = new EsQueryBuilder(mapping, settings, log, path);
 			innerQueryBuilder.visit(must);
-			if (innerQueryBuilder.needsScoring) {
-				needsScoring = innerQueryBuilder.needsScoring;
+			if (innerQueryBuilder.needsScoring()) {
+				enableScoring();
 				query.must(innerQueryBuilder.deque.pop());
 			} else {
 				query.filter(innerQueryBuilder.deque.pop());
@@ -266,9 +270,17 @@ private void visit(NestedPredicate predicate) {
 		final DocumentMapping nestedMapping = mapping.getNestedMapping(predicate.getField());
 		final EsQueryBuilder nestedQueryBuilder = new EsQueryBuilder(nestedMapping, settings, log, nestedPath);
 		nestedQueryBuilder.visit(predicate.getExpression());
-		needsScoring = nestedQueryBuilder.needsScoring;
+		
+		final ScoreMode scoreMode;
+		if (nestedQueryBuilder.needsScoring()) {
+			enableScoring();
+			scoreMode = ScoreMode.Max;
+		} else {
+			scoreMode = ScoreMode.None;
+		}
+		
 		final QueryBuilder nestedQuery = nestedQueryBuilder.deque.pop();
-		deque.push(QueryBuilders.nestedQuery(nestedPath, nestedQuery, nestedQueryBuilder.needsScoring ? ScoreMode.Max : ScoreMode.None));
+		deque.push(QueryBuilders.nestedQuery(nestedPath, nestedQuery, scoreMode));
 	}
 
 	private String toFieldPath(Predicate predicate) {
@@ -288,60 +300,70 @@ private void visit(TextPredicate predicate) {
 		final String term = predicate.term();
 		final MatchType type = predicate.type();
 		final int minShouldMatch = predicate.minShouldMatch();
+		final String searchAnalyzer = selectSearchAnalyzer(predicate);
 
 		QueryBuilder query;
 
-		String searchAnalyzer = selectSearchAnalyzer(predicate);
 		switch (type) {
-		case BOOLEAN_PREFIX:
-			query = QueryBuilders.matchBoolPrefixQuery(field, term)
-				.analyzer(searchAnalyzer)
-				.operator(Operator.AND);
-			break;
-		case PHRASE:
-			query = QueryBuilders.matchPhraseQuery(field, term)
-						.analyzer(searchAnalyzer);
-			break;
-		case ALL:
-			MatchQueryBuilder all = QueryBuilders.matchQuery(field, term)
-						.analyzer(searchAnalyzer)
-						.operator(Operator.AND);
-			if (!Strings.isNullOrEmpty(predicate.fuzziness())) {
-				all
-					.fuzziness(predicate.fuzziness())
-					.prefixLength(predicate.prefixLength())
-					.maxExpansions(predicate.maxExpansions());
-			}
-			query = all;
-			break;
-		case ANY:
-			MatchQueryBuilder any = QueryBuilders.matchQuery(field, term)
-						.analyzer(searchAnalyzer)
-						.operator(Operator.OR)
-						.minimumShouldMatch(Integer.toString(minShouldMatch));
-			if (!Strings.isNullOrEmpty(predicate.fuzziness())) {
-				any
-					.fuzziness(predicate.fuzziness())
+			case BOOLEAN_PREFIX:
+				query = QueryBuilders.matchBoolPrefixQuery(field, term)
+					.analyzer(searchAnalyzer)
+					.operator(Operator.AND);
+				break;
+				
+			case PHRASE:
+				query = QueryBuilders.matchPhraseQuery(field, term)
+					.analyzer(searchAnalyzer);
+				break;
+				
+			case ALL:
+				MatchQueryBuilder all = QueryBuilders.matchQuery(field, term)
+					.analyzer(searchAnalyzer)
+					.operator(Operator.AND);
+		
+				if (!Strings.isNullOrEmpty(predicate.fuzziness())) {
+					all.fuzziness(predicate.fuzziness())
+						.prefixLength(predicate.prefixLength())
+						.maxExpansions(predicate.maxExpansions());
+				}
+			
+				query = all;
+				break;
+				
+			case ANY:
+				MatchQueryBuilder any = QueryBuilders.matchQuery(field, term)
+					.analyzer(searchAnalyzer)
+					.operator(Operator.OR)
+					.minimumShouldMatch(Integer.toString(minShouldMatch));
+				
+				if (!Strings.isNullOrEmpty(predicate.fuzziness())) {
+					any.fuzziness(predicate.fuzziness())
 					.prefixLength(predicate.prefixLength())
 					.maxExpansions(predicate.maxExpansions());
-			}
-			query = any;
-			break;
-		case PARSED:
-			query = QueryBuilders.queryStringQuery(TextConstants.escape(term))
-						.analyzer(searchAnalyzer)
-						.field(field)
-						.escape(false)
-						.allowLeadingWildcard(true)
-						.defaultOperator(Operator.AND);
-			break;
-		default: throw new UnsupportedOperationException("Unexpected text match type: " + type);
+				}
+				
+				query = any;
+				break;
+				
+			case PARSED:
+				query = QueryBuilders.queryStringQuery(TextConstants.escape(term))
+					.analyzer(searchAnalyzer)
+					.field(field)
+					.escape(false)
+					.allowLeadingWildcard(true)
+					.defaultOperator(Operator.AND);
+				break;
+		
+			default: 
+				throw new UnsupportedOperationException("Unexpected text match type: " + type);
 		}
+		
 		if (query == null) {
 			query = MATCH_NONE;
 		} else {
-			needsScoring = true;
+			enableScoring();
 		}
+		
 		deque.push(query);
 	}