Don't rely on shared groupingrequest instances to look up results

Instead of setting the id of the grouping result in the query
when the result is parsed, which relies on sharing the same grouping
instance across all query copies to be useful, create it on
demand.

4 files changed, 18 insertions, 18 deletions

diff --git a/container-search/src/main/java/com/yahoo/search/grouping/GroupingRequest.java b/container-search/src/main/java/com/yahoo/search/grouping/GroupingRequest.java
index 754d3b562d3..97e3ecdc491 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/GroupingRequest.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/GroupingRequest.java
@@ -8,6 +8,7 @@ import com.yahoo.search.Result;
 import com.yahoo.search.grouping.request.GroupingOperation;
 import com.yahoo.search.grouping.result.Group;
 import com.yahoo.search.grouping.result.RootGroup;
+import com.yahoo.search.grouping.result.RootId;
 import com.yahoo.search.query.Select;
 import com.yahoo.search.result.Hit;
 
@@ -115,8 +116,8 @@ public class GroupingRequest {
      * @return the result {@link RootGroup} of this request, or null if not found.
      */
     public RootGroup getResultGroup(Result result) {
-        Hit root = result.hits().get(resultId, -1);
-        if (!(root instanceof RootGroup)) {
+        Hit root = result.hits().get(new URI(new RootId(getRequestId()).toString()), -1);
+        if ( ! (root instanceof RootGroup)) {
             return null;
         }
         return (RootGroup)root;
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/vespa/GroupingExecutor.java b/container-search/src/main/java/com/yahoo/search/grouping/vespa/GroupingExecutor.java
index 61856467652..895459f5568 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/vespa/GroupingExecutor.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/vespa/GroupingExecutor.java
@@ -103,7 +103,7 @@ public class GroupingExecutor extends Searcher {
         for (Iterator<Hit> it = result.hits().unorderedDeepIterator(); it.hasNext(); ) {
             Hit hit = it.next();
             Object metaData = hit.getSearcherSpecificMetaData(this);
-            if (metaData != null && metaData instanceof String) {
+            if (metaData instanceof String) {
                 // Use the summary class specified by grouping, set in HitConverter, for the first fill request
                 // after grouping. This assumes the first fill request is using the default summary class,
                 // which may be a fragile assumption. But currently we cannot do better because the difference
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/vespa/ResultBuilder.java b/container-search/src/main/java/com/yahoo/search/grouping/vespa/ResultBuilder.java
index 7e2404c34e8..bbcead37acc 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/vespa/ResultBuilder.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/vespa/ResultBuilder.java
@@ -65,8 +65,8 @@ class ResultBuilder {
     /**
      * Sets the id of the {@link GroupingRequest} that this builder is creating the result for.
      *
-     * @param requestId The id of the corresponding GroupingRequest.
-     * @return This, to allow chaining.
+     * @param requestId the id of the corresponding GroupingRequest
+     * @return this, to allow chaining
      */
     public ResultBuilder setRequestId(int requestId) {
         root = new RootGroup(requestId, continuation);
@@ -77,8 +77,8 @@ class ResultBuilder {
     /**
      * Sets the transform that details how the result should be built.
      *
-     * @param transform The transform to set.
-     * @return This, to allow chaining.
+     * @param transform the transform to set
+     * @return this, to allow chaining
      */
     public ResultBuilder setTransform(GroupingTransform transform) {
         this.transform = transform;
@@ -88,8 +88,8 @@ class ResultBuilder {
     /**
      * Sets the converts that details how hits are converted.
      *
-     * @param hitConverter The converter to set.
-     * @return This, to allow chaining.
+     * @param hitConverter the converter to set
+     * @return this, to allow chaining
      */
     public ResultBuilder setHitConverter(HitConverter hitConverter) {
         this.hitConverter = hitConverter;
@@ -100,7 +100,7 @@ class ResultBuilder {
      * Adds a grouping result to this transform. This method will recurse through the given object and retrieve all the
      * information it needs to produce the desired result when calling {@link #build()}.
      *
-     * @param executionResult The grouping result to process.
+     * @param executionResult the grouping result to process
      */
     public void addGroupingResult(Grouping executionResult) {
         executionResult.unifyNull();
@@ -110,7 +110,7 @@ class ResultBuilder {
     /**
      * Returns the root {@link RootGroup} that was created when {@link #build()} was called.
      *
-     * @return The root that was built.
+     * @return the root that was built
      */
     public RootGroup getRoot() {
         return root;
diff --git a/container-search/src/main/java/com/yahoo/search/result/HitGroup.java b/container-search/src/main/java/com/yahoo/search/result/HitGroup.java
index 42d62cc0ec5..022ef9950b3 100644
--- a/container-search/src/main/java/com/yahoo/search/result/HitGroup.java
+++ b/container-search/src/main/java/com/yahoo/search/result/HitGroup.java
@@ -274,13 +274,13 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable<
     /**
      * Returns the hit with the given id, or null if there is no hit with this id
      * in this group or any subgroup.
-     * This method is o(min(number of nested hits in this result,depth)).
+     * This method is o(min(number of nested hits in this result, depth)).
      *
      * @param id the id of the hit to return from this or any nested group
      * @param depth the max depth to recurse into nested groups: -1: Recurse infinitely deep, 0: Only look at hits in
      *        the list of this group, 1: Look at hits in this group, and the hits of any immediate nested HitGroups,
      *        etc.
-     * @return The hit, or null if not found.
+     * @return the hit, or null if not found
      */
     public Hit get(URI id, int depth) {
         updateHits();
@@ -292,7 +292,7 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable<
                 return hit;
             }
 
-            if (hit instanceof HitGroup && depth!=0) {
+            if (hit instanceof HitGroup && depth != 0) {
                 Hit found=((HitGroup)hit).get(id,depth-1);
                 if (found!=null) return found;
             }
@@ -319,10 +319,9 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable<
 
     /**
      * Adds a hit to this group in the specified index,
-     * all existing hits on this index and higher will have their index
-     * increased by one.
-     * <b>Note:</b> If the group was sorted, it will still be considered sorted
-     * after this call.
+     * all existing hits on this index and higher will have their index increased by one.
+     *
+     * <b>Note:</b> If the group was sorted, it will still be considered sorted after this call.
      */
     public void add(int index, Hit hit) {
         if (hit instanceof ErrorHit) { // Merge instead