- Let there only be one way to wire query to the grouping hits.

  Enforce that by requiring it in the constructor.
- Carry the DocumentDatabase along, not only the DocusumDefinitionSet.

3 files changed, 49 insertions, 58 deletions

diff --git a/container-search/src/test/java/com/yahoo/search/grouping/result/FlatteningSearcherTestCase.java b/container-search/src/test/java/com/yahoo/search/grouping/result/FlatteningSearcherTestCase.java
index 7ec35151eab..9e8fcb0ea21 100644
--- a/container-search/src/test/java/com/yahoo/search/grouping/result/FlatteningSearcherTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/grouping/result/FlatteningSearcherTestCase.java
@@ -76,8 +76,8 @@ public class FlatteningSearcherTestCase {
         Execution execution = newExecution(new FlatteningSearcher(),
                 new GroupingExecutor(ComponentId.fromString("grouping")),
                 new ResultProvider(List.of(
-                        new GroupingListHit(List.of(group0), null),
-                        new GroupingListHit(List.of(group1), null))),
+                        new GroupingListHit(List.of(group0), null, query),
+                        new GroupingListHit(List.of(group1), null, query))),
                 new HitsProvider(List.of(
                         new DefaultErrorHit("source 1", ErrorMessage.createBackendCommunicationError("backend communication error 1")),
                         new DefaultErrorHit("source 2", ErrorMessage.createBackendCommunicationError("backend communication error 1")))));
diff --git a/container-search/src/test/java/com/yahoo/search/grouping/vespa/GroupingExecutorTestCase.java b/container-search/src/test/java/com/yahoo/search/grouping/vespa/GroupingExecutorTestCase.java
index 17ab3823d57..ef2ef9724a9 100644
--- a/container-search/src/test/java/com/yahoo/search/grouping/vespa/GroupingExecutorTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/grouping/vespa/GroupingExecutorTestCase.java
@@ -8,9 +8,7 @@ import com.yahoo.document.DocumentId;
 import com.yahoo.document.GlobalId;
 import com.yahoo.prelude.fastsearch.FastHit;
 import com.yahoo.prelude.fastsearch.GroupingListHit;
-import com.yahoo.prelude.query.NotItem;
 import com.yahoo.prelude.query.NullItem;
-import com.yahoo.prelude.query.WordItem;
 import com.yahoo.search.Query;
 import com.yahoo.search.Result;
 import com.yahoo.search.Searcher;
@@ -183,8 +181,8 @@ public class GroupingExecutorTestCase {
         );
         Execution exec = newExecution(new GroupingExecutor(),
                 new ResultProvider(Arrays.asList(
-                        new GroupingListHit(List.of(grpA), null),
-                        new GroupingListHit(List.of(grpB), null))));
+                        new GroupingListHit(List.of(grpA), null, query),
+                        new GroupingListHit(List.of(grpB), null, query))));
         Group grp = req.getResultGroup(exec.search(query));
         assertEquals(1, grp.size());
         Hit hit = grp.get(0);
@@ -216,8 +214,8 @@ public class GroupingExecutorTestCase {
         );
         Execution exec = newExecution(new GroupingExecutor(),
                 new ResultProvider(Arrays.asList(
-                        new GroupingListHit(List.of(grpExpected), null),
-                        new GroupingListHit(List.of(grpUnexpected), null))));
+                        new GroupingListHit(List.of(grpExpected), null, query),
+                        new GroupingListHit(List.of(grpUnexpected), null, query))));
         Group grp = req.getResultGroup(exec.search(query));
         assertEquals(1, grp.size());
         Hit hit = grp.get(0);
@@ -247,8 +245,8 @@ public class GroupingExecutorTestCase {
                 ));
         Execution exec = newExecution(new GroupingExecutor(),
                 new ResultProvider(Arrays.asList(
-                        new GroupingListHit(List.of(grp0), null),
-                        new GroupingListHit(List.of(grp1), null))),
+                        new GroupingListHit(List.of(grp0), null, query),
+                        new GroupingListHit(List.of(grp1), null, query))),
                 new FillRequestThrower());
         Result res = exec.search(query);
 
@@ -287,8 +285,8 @@ public class GroupingExecutorTestCase {
                                         .addHit(new com.yahoo.searchlib.aggregation.FS4Hit()))));
         Execution exec = newExecution(new GroupingExecutor(),
                 new ResultProvider(Arrays.asList(
-                        new GroupingListHit(List.of(grp0), null),
-                        new GroupingListHit(List.of(grp1), null))),
+                        new GroupingListHit(List.of(grp0), null, query),
+                        new GroupingListHit(List.of(grp1), null, query))),
                 new FillErrorProvider());
         Result res = exec.search(query);
         exec.fill(res);
@@ -313,8 +311,8 @@ public class GroupingExecutorTestCase {
                         .addOrderBy(new AggregationRefNode(0), true)));
         Result res = newExecution(new GroupingExecutor(),
                 new ResultProvider(Arrays.asList(
-                        new GroupingListHit(List.of(grp), null),
-                        new GroupingListHit(List.of(grp), null)))).search(query);
+                        new GroupingListHit(List.of(grp), null, query),
+                        new GroupingListHit(List.of(grp), null, query)))).search(query);
 
         GroupList groupList = (GroupList) req.getResultGroup(res).get(0);
         assertEquals(1.0, groupList.get(0).getRelevance().getScore(), 1E-6);
@@ -342,8 +340,8 @@ public class GroupingExecutorTestCase {
         Execution exec = newExecution(new GroupingExecutor(),
                 err,
                 new ResultProvider(Arrays.asList(
-                        new GroupingListHit(List.of(grp0), null),
-                        new GroupingListHit(List.of(grp1), null))));
+                        new GroupingListHit(List.of(grp0), null, query),
+                        new GroupingListHit(List.of(grp1), null, query))));
         Result res = exec.search(query);
         assertNotNull(res.hits().getError());
         assertEquals(Error.TIMEOUT.code, res.hits().getError().getCode());
@@ -353,8 +351,8 @@ public class GroupingExecutorTestCase {
         exec = newExecution(new GroupingExecutor(),
                 err,
                 new ResultProvider(Arrays.asList(
-                        new GroupingListHit(List.of(grp0), null),
-                        new GroupingListHit(List.of(grp1), null))));
+                        new GroupingListHit(List.of(grp0), null, query),
+                        new GroupingListHit(List.of(grp1), null, query))));
         res = exec.search(query);
         assertNotNull(res.hits().getError());
         assertEquals(Error.TIMEOUT.code, res.hits().getError().getCode());
@@ -392,8 +390,8 @@ public class GroupingExecutorTestCase {
         SummaryMapper sm = new SummaryMapper();
         Execution exec = newExecution(new GroupingExecutor(),
                 new ResultProvider(Arrays.asList(
-                        new GroupingListHit(Arrays.asList(pass0A, pass0B), null),
-                        new GroupingListHit(Arrays.asList(pass1A, pass1B), null))),
+                        new GroupingListHit(Arrays.asList(pass0A, pass0B), null, query),
+                        new GroupingListHit(Arrays.asList(pass1A, pass1B), null, query))),
                 sm);
         exec.fill(exec.search(query), "default");
         assertEquals(2, sm.hitsBySummary.size());
@@ -436,8 +434,8 @@ public class GroupingExecutorTestCase {
                                         .addHit(new com.yahoo.searchlib.aggregation.FS4Hit()))));
         Execution exec = newExecution(new GroupingExecutor(),
                 new ResultProvider(Arrays.asList(
-                        new GroupingListHit(List.of(pass0), null),
-                        new GroupingListHit(List.of(pass1), null))));
+                        new GroupingListHit(List.of(pass0), null, query),
+                        new GroupingListHit(List.of(pass1), null, query))));
         Result res = exec.search(query);
         exec.fill(res);
 
@@ -457,7 +455,7 @@ public class GroupingExecutorTestCase {
                 .addChild(new com.yahoo.searchlib.aggregation.Group().setId(new StringResultNode("foo"))
                         .addAggregationResult(new HitsAggregationResult(1, "bar"))
                 ));
-        GroupingListHit pass0 = new GroupingListHit(List.of(grp), null);
+        GroupingListHit pass0 = new GroupingListHit(List.of(grp), null, queryA);
 
         GlobalId gid = new GlobalId((new DocumentId("id:ns:type::1")).getGlobalId());
         grp = new Grouping(0);
@@ -465,9 +463,8 @@ public class GroupingExecutorTestCase {
                 .addChild(new com.yahoo.searchlib.aggregation.Group().setId(new StringResultNode("foo"))
                         .addAggregationResult(new HitsAggregationResult(1, "bar").addHit(new com.yahoo.searchlib.aggregation.FS4Hit(4, gid, 6)))
                 ));
-        GroupingListHit pass1 = new GroupingListHit(List.of(grp), null);
         Query queryB = newQuery(); // required by GroupingListHit.getSearchQuery()
-        pass1.setQuery(queryB);
+        GroupingListHit pass1 = new GroupingListHit(List.of(grp), null, queryB);
 
         QueryMapper qm = new QueryMapper();
         Execution exec = newExecution(new GroupingExecutor(),
@@ -569,10 +566,10 @@ public class GroupingExecutorTestCase {
                 .addChild(new com.yahoo.searchlib.aggregation.Group().setId(new StringResultNode("uniqueC")).addAggregationResult(new MaxAggregationResult().setMax(new IntegerResultNode(7)).setTag(4)))
                 .addChild(new com.yahoo.searchlib.aggregation.Group().setId(new StringResultNode("common")).addAggregationResult(new MaxAggregationResult().setMax(new IntegerResultNode(11)).setTag(4)))
         );
-        resultsByDocumentType.put("typeA", List.of(new GroupingListHit(List.of(groupA1), null),
-                new GroupingListHit(List.of(groupA2), null)));
-        resultsByDocumentType.put("typeB", List.of(new GroupingListHit(List.of(groupB1), null),
-                new GroupingListHit(List.of(groupB2), null)));
+        resultsByDocumentType.put("typeA", List.of(new GroupingListHit(List.of(groupA1), null, query),
+                new GroupingListHit(List.of(groupA2), null, query)));
+        resultsByDocumentType.put("typeB", List.of(new GroupingListHit(List.of(groupB1), null, query),
+                new GroupingListHit(List.of(groupB2), null, query)));
         Execution execution = newExecution(new GroupingExecutor(),
                 new MockClusterSearcher(),
                 new MultiDocumentTypeResultProvider(resultsByDocumentType));
@@ -742,11 +739,7 @@ public class GroupingExecutorTestCase {
             for (Iterator<Hit> it = result.hits().deepIterator(); it.hasNext();) {
                 Hit hit = it.next();
                 Query query = hit.getQuery();
-                List<Hit> lst = hitsByQuery.get(query);
-                if (lst == null) {
-                    lst = new LinkedList<>();
-                    hitsByQuery.put(query, lst);
-                }
+                List<Hit> lst = hitsByQuery.computeIfAbsent(query, k -> new LinkedList<>());
                 lst.add(hit);
             }
         }
@@ -767,11 +760,7 @@ public class GroupingExecutorTestCase {
         public void fill(Result result, String summaryClass, Execution exec) {
             for (Iterator<Hit> it = result.hits().deepIterator(); it.hasNext();) {
                 Hit hit = it.next();
-                List<Hit> lst = hitsBySummary.get(summaryClass);
-                if (lst == null) {
-                    lst = new LinkedList<>();
-                    hitsBySummary.put(summaryClass, lst);
-                }
+                List<Hit> lst = hitsBySummary.computeIfAbsent(summaryClass, k -> new LinkedList<>());
                 lst.add(hit);
             }
         }
diff --git a/container-search/src/test/java/com/yahoo/search/grouping/vespa/HitConverterTestCase.java b/container-search/src/test/java/com/yahoo/search/grouping/vespa/HitConverterTestCase.java
index 8f775e9923a..57f6ee7d65f 100644
--- a/container-search/src/test/java/com/yahoo/search/grouping/vespa/HitConverterTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/grouping/vespa/HitConverterTestCase.java
@@ -4,8 +4,8 @@ package com.yahoo.search.grouping.vespa;
 import com.yahoo.document.DocumentId;
 import com.yahoo.document.GlobalId;
 import com.yahoo.net.URI;
+import com.yahoo.prelude.fastsearch.DocumentDatabase;
 import com.yahoo.prelude.fastsearch.GroupingListHit;
-import com.yahoo.prelude.fastsearch.DocsumDefinitionSet;
 import com.yahoo.prelude.fastsearch.FastHit;
 import com.yahoo.search.Query;
 import com.yahoo.search.Result;
@@ -19,7 +19,7 @@ import com.yahoo.searchlib.aggregation.FS4Hit;
 import com.yahoo.searchlib.aggregation.VdsHit;
 import org.junit.jupiter.api.Test;
 
-import java.util.Collections;
+import java.util.List;
 
 import static org.junit.jupiter.api.Assertions.*;
 
@@ -34,24 +34,24 @@ public class HitConverterTestCase {
 
     @Test
     void requireThatHitsAreConverted() {
-        HitConverter converter = new HitConverter(new MySearcher(), new Query());
-        Hit hit = converter.toSearchHit("default", new FS4Hit(1, createGlobalId(2), 3).setContext(context()));
+        Query query = new Query();
+        HitConverter converter = new HitConverter(new MySearcher());
+        Hit hit = converter.toSearchHit("default", new FS4Hit(1, createGlobalId(2), 3).setContext(context(query)));
         assertNotNull(hit);
         assertEquals(new URI("index:null/1/" + asHexString(createGlobalId(2))), hit.getId());
 
-        hit = converter.toSearchHit("default", new FS4Hit(4, createGlobalId(5), 6).setContext(context()));
+        hit = converter.toSearchHit("default", new FS4Hit(4, createGlobalId(5), 6).setContext(context(query)));
         assertNotNull(hit);
         assertEquals(new URI("index:null/4/" + asHexString(createGlobalId(5))), hit.getId());
     }
 
     @Test
     void requireThatContextDataIsCopied() {
-        Hit ctxHit = context();
-        ctxHit.setSource("69");
         Query ctxQuery = new Query();
-        ctxHit.setQuery(ctxQuery);
+        Hit ctxHit = context(ctxQuery);
+        ctxHit.setSource("69");
 
-        HitConverter converter = new HitConverter(new MySearcher(), new Query());
+        HitConverter converter = new HitConverter(new MySearcher());
         Hit hit = converter.toSearchHit("default", new FS4Hit(1, createGlobalId(2), 3).setContext(ctxHit));
         assertNotNull(hit);
         assertTrue(hit instanceof FastHit);
@@ -63,9 +63,10 @@ public class HitConverterTestCase {
 
     @Test
     void requireThatSummaryClassIsSet() {
+        Query query = new Query();
         Searcher searcher = new MySearcher();
-        HitConverter converter = new HitConverter(searcher, new Query());
-        Hit hit = converter.toSearchHit("69", new FS4Hit(1, createGlobalId(2), 3).setContext(context()));
+        HitConverter converter = new HitConverter(searcher);
+        Hit hit = converter.toSearchHit("69", new FS4Hit(1, createGlobalId(2), 3).setContext(context(query)));
         assertNotNull(hit);
         assertTrue(hit instanceof FastHit);
         assertEquals("69", hit.getSearcherSpecificMetaData(searcher));
@@ -73,7 +74,7 @@ public class HitConverterTestCase {
 
     @Test
     void requireThatHitHasContext() {
-        HitConverter converter = new HitConverter(new MySearcher(), new Query());
+        HitConverter converter = new HitConverter(new MySearcher());
         try {
             converter.toSearchHit("69", new FS4Hit(1, createGlobalId(2), 3));
             fail();
@@ -84,7 +85,7 @@ public class HitConverterTestCase {
 
     @Test
     void requireThatUnsupportedHitClassThrows() {
-        HitConverter converter = new HitConverter(new MySearcher(), new Query());
+        HitConverter converter = new HitConverter(new MySearcher());
         try {
             converter.toSearchHit("69", new com.yahoo.searchlib.aggregation.Hit() {
 
@@ -95,21 +96,22 @@ public class HitConverterTestCase {
         }
     }
 
-    private static GroupingListHit context() {
-        return new GroupingListHit(Collections.emptyList(), null);
+    private static GroupingListHit context(Query query) {
+        return new GroupingListHit(List.of(), null, query);
     }
 
-    private static DocsumDefinitionSet sixtynine() {
+    private static DocumentDatabase sixtynine() {
         var schema = new Schema.Builder("none");
         var summary = new DocumentSummary.Builder("69");
         schema.add(summary.build());
-        return new DocsumDefinitionSet(schema.build());
+        return new DocumentDatabase(schema.build());
     }
 
     @Test
     void requireThatVdsHitCanBeConverted() {
-        HitConverter converter = new HitConverter(new MySearcher(), new Query());
-        GroupingListHit context = new GroupingListHit(null, sixtynine());
+        Query query = new Query();
+        HitConverter converter = new HitConverter(new MySearcher());
+        GroupingListHit context = new GroupingListHit(null, sixtynine(), query);
         VdsHit lowHit = new VdsHit("id:ns:type::", new byte[]{0x55, 0x55, 0x55, 0x55}, 1);
         lowHit.setContext(context);
         Hit hit = converter.toSearchHit("69", lowHit);