Add some tests

7 files changed, 154 insertions, 54 deletions

diff --git a/container-search/src/main/java/com/yahoo/prelude/searcher/BlendingSearcher.java b/container-search/src/main/java/com/yahoo/prelude/searcher/BlendingSearcher.java
index 65634c56d4c..932f4ea7c38 100644
--- a/container-search/src/main/java/com/yahoo/prelude/searcher/BlendingSearcher.java
+++ b/container-search/src/main/java/com/yahoo/prelude/searcher/BlendingSearcher.java
@@ -57,7 +57,6 @@ public class BlendingSearcher extends Searcher {
     @Override
     public Result search(Query query, Execution execution) {
         Result result = execution.search(query);
-
         Result blended = blendResults(result, query, query.getOffset(), query.getHits(), execution);
         blended.trace("Blended result");
         return blended;
@@ -78,7 +77,7 @@ public class BlendingSearcher extends Searcher {
      * This assumes that all hits are organized into hitgroups. If not, blending will not be performed.
      */
     protected Result blendResults(Result result, Query q, int offset, int hits, Execution execution) {
-        // Assert that there are more than one hitgroup and that there are only hitgroups on the lowest level
+        // Assert that there are more than one hitgroup and that there are only hitgroups on the highest level
         boolean foundNonGroup = false;
         Iterator<Hit> hitIterator = result.hits().iterator();
         List<HitGroup> groups = new ArrayList<>();
@@ -186,11 +185,11 @@ public class BlendingSearcher extends Searcher {
         void scanResult(Execution execution) {
             List<Hit> hits = group.asUnorderedHits();
             for (int i = hits.size()-1; i >= 0; i--) {
-                Hit sniffHit = hits.get(i);
-                if (!sniffHit.isMeta()) {
-                    scan(sniffHit, i, execution);
+                Hit hit = hits.get(i);
+                if (!hit.isMeta()) {
+                    scan(hit, i, execution);
                 } else {
-                    result.hits().add(sniffHit);
+                    result.hits().add(hit);
                 }
             }
         }
@@ -199,7 +198,7 @@ public class BlendingSearcher extends Searcher {
             // note, different loop direction from scanResult()
             for(HitGroup group : groups.subList(1, groups.size())) {
                 for(Hit hit : group.asList()) {
-                    if(hit.isMeta()) {
+                    if (hit.isMeta()) {
                         result.hits().add(hit);
                     } else {
                         put(group, hit, execution);
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/result/GroupId.java b/container-search/src/main/java/com/yahoo/search/grouping/result/GroupId.java
index 2cf88372014..512bc0c5867 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/result/GroupId.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/result/GroupId.java
@@ -28,11 +28,7 @@ public abstract class GroupId {
         this.image = image.toString();
     }
 
-    /**
-     * Returns the type name of this group id. This is the second part of the {@link #toString()} value of this.
-     *
-     * @return The type name.
-     */
+    /** Returns the type name of this group id. This is the second part of the {@link #toString()} value of this. */
     public String getTypeName() {
         return type;
     }
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/result/StringBucketId.java b/container-search/src/main/java/com/yahoo/search/grouping/result/StringBucketId.java
index 78dd6671d6e..e34148aa487 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/result/StringBucketId.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/result/StringBucketId.java
@@ -18,4 +18,5 @@ public class StringBucketId extends BucketGroupId<String> {
     public StringBucketId(String from, String to) {
         super("string_bucket", from, to);
     }
+
 }
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/result/ValueGroupId.java b/container-search/src/main/java/com/yahoo/search/grouping/result/ValueGroupId.java
index 0230683dd47..72709c49919 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/result/ValueGroupId.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/result/ValueGroupId.java
@@ -1,8 +1,6 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.search.grouping.result;
 
-import static com.yahoo.text.Lowercase.toLowerCase;
-
 /**
  * This abstract class is used in {@link Group} instances where the identifying expression evaluated to a singe value.
  *
@@ -15,8 +13,8 @@ public abstract class ValueGroupId<T> extends GroupId {
     /**
      * Constructs a new instance of this class.
      *
-     * @param type  The type of this id's value.
-     * @param value The identifying value.
+     * @param type  the type of this id's value
+     * @param value the identifying value
      */
     public ValueGroupId(String type, T value) {
         this(type, value, String.valueOf(value.toString()));
@@ -25,21 +23,18 @@ public abstract class ValueGroupId<T> extends GroupId {
     /**
      * Constructs a new instance of this class.
      *
-     * @param type       The type of this id's value.
-     * @param value      The identifying value.
-     * @param valueImage The String representation of the <code>value</code> argument.
+     * @param type       the type of this id's value
+     * @param value      the identifying value
+     * @param valueImage the String representation of the <code>value</code> argument
      */
     public ValueGroupId(String type, T value, String valueImage) {
         super(type, valueImage);
         this.value = value;
     }
 
-    /**
-     * Returns the identifying value.
-     *
-     * @return The value.
-     */
+    /** Returns the identifying value. */
     public T getValue() {
         return value;
     }
+
 }
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 9859678f492..b3399390bd9 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
@@ -141,10 +141,10 @@ public class GroupingExecutor extends Searcher {
      * context that corresponds to the given request, whereas the created {@link Grouping} objects are written directly
      * to the given map.
      *
-     * @param query The query being executed.
-     * @param req   The request to convert.
-     * @param map   The grouping map to write to.
-     * @return The context required to identify the request results.
+     * @param query the query being executed
+     * @param req   the request to convert
+     * @param map   the grouping map to write to
+     * @return the context required to identify the request results
      */
     private RequestContext convertRequest(Query query, GroupingRequest req, int requestId, Map<Integer, Grouping> map) {
         RequestBuilder builder = new RequestBuilder(requestId);
@@ -168,10 +168,10 @@ public class GroupingExecutor extends Searcher {
     /**
      * Converts the results of the given request context into a single {@link Group}.
      *
-     * @param requestContext   The context that identifies the results to convert.
-     * @param groupingMap  The map of all {@link Grouping} objects available.
-     * @param hitConverter The converter to use for {@link Hit} conversion.
-     * @return The corresponding root RootGroup.
+     * @param requestContext the context that identifies the results to convert
+     * @param groupingMap    the map of all {@link Grouping} objects available
+     * @param hitConverter   the converter to use for {@link Hit} conversion
+     * @return the corresponding root RootGroup.
      */
     private RootGroup convertResult(RequestContext requestContext, Map<Integer, Grouping> groupingMap,
                                     HitConverter hitConverter) {
@@ -191,10 +191,10 @@ public class GroupingExecutor extends Searcher {
      * grouping map argument as both an input and an output variable, as the contained {@link Grouping} objects are
      * updates as results arrive from the back end.
      *
-     * @param query       The query to execute.
-     * @param execution   The execution context used to run the queries.
-     * @param groupingMap The map of grouping requests to perform.
-     * @return The search result to pass back from this searcher.
+     * @param query       the query to execute
+     * @param execution   the execution context used to run the queries
+     * @param groupingMap the map of grouping requests to perform
+     * @return the search result to pass back from this searcher
      */
     private Result performSearch(Query query, Execution execution, Map<Integer, Grouping> groupingMap) {
         // Determine how many passes to perform.
@@ -263,8 +263,8 @@ public class GroupingExecutor extends Searcher {
      * Merges the content of result into state. This needs to be done in order to conserve the context objects contained
      * in the state as they are not part of the serialized object representation.
      *
-     * @param state  the current state.
-     * @param result the results from the current pass.
+     * @param state  the current state
+     * @param result the results from the current pass
      */
     private void mergeGroupingMaps(Map<Integer, Grouping> state, Map<Integer, Grouping> result) {
         for (Grouping grouping : result.values()) {
@@ -282,9 +282,9 @@ public class GroupingExecutor extends Searcher {
     /**
      * Returns a list of {@link Grouping} objects that are to be used for the given pass.
      *
-     * @param groupingMap The map of all grouping objects.
-     * @param pass        The pass about to be performed.
-     * @return A list of grouping objects.
+     * @param groupingMap the map of all grouping objects
+     * @param pass        the pass about to be performed
+     * @return a list of grouping objects
      */
     private List<Grouping> getGroupingListForPassN(Map<Integer, Grouping> groupingMap, int pass) {
         List<Grouping> ret = new ArrayList<>();
@@ -310,8 +310,8 @@ public class GroupingExecutor extends Searcher {
      * Merges the grouping content of the given result object. The first grouping hit found by iterating over the result
      * content is kept, and all consecutive matching hits are merged into this.
      *
-     * @param result The result to traverse.
-     * @return A map of merged grouping objects.
+     * @param result the result to traverse
+     * @return a map of merged grouping objects
      */
     private Map<Integer, Grouping> mergeGroupingResults(Result result) {
         Map<Integer, Grouping> ret = new HashMap<>();
@@ -341,8 +341,8 @@ public class GroupingExecutor extends Searcher {
      * Returns the list of {@link Grouping} objects assigned to the given query. If no list has been assigned, this
      * method returns an empty list.
      *
-     * @param query The query whose grouping list to return.
-     * @return The list of assigned grouping objects.
+     * @param query the query whose grouping list to return
+     * @return the list of assigned grouping objects
      */
     @SuppressWarnings({ "unchecked" })
     public static List<Grouping> getGroupingList(Query query) {
@@ -362,11 +362,11 @@ public class GroupingExecutor extends Searcher {
      * Sets the list of {@link Grouping} objects assigned to the given query. This method overwrites any grouping
      * objects already assigned to the query.
      *
-     * @param query The query whose grouping list to set.
-     * @param lst   The grouping list to set.
+     * @param query the query whose grouping list to set
+     * @param list   the grouping list to set
      */
-    public static void setGroupingList(Query query, List<Grouping> lst) {
-        query.properties().set(PROP_GROUPINGLIST, lst);
+    public static void setGroupingList(Query query, List<Grouping> list) {
+        query.properties().set(PROP_GROUPINGLIST, list);
     }
 
     private static CompoundName newCompoundName(String name) {
@@ -403,4 +403,5 @@ public class GroupingExecutor extends Searcher {
             this.transform = transform;
         }
     }
+
 }
diff --git a/container-search/src/test/java/com/yahoo/prelude/searcher/test/BlendingSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/searcher/test/BlendingSearcherTestCase.java
index 01380a52a33..7dad3c9b52b 100644
--- a/container-search/src/test/java/com/yahoo/prelude/searcher/test/BlendingSearcherTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/searcher/test/BlendingSearcherTestCase.java
@@ -13,7 +13,6 @@ import com.yahoo.component.provider.ComponentRegistry;
 import com.yahoo.search.federation.FederationConfig;
 import com.yahoo.container.QrSearchersConfig;
 import com.yahoo.search.federation.StrictContractsConfig;
-import com.yahoo.prelude.IndexFacts;
 import com.yahoo.search.Query;
 import com.yahoo.search.Result;
 import com.yahoo.prelude.fastsearch.FastHit;
@@ -21,8 +20,14 @@ import com.yahoo.prelude.searcher.BlendingSearcher;
 import com.yahoo.prelude.searcher.FillSearcher;
 import com.yahoo.search.Searcher;
 import com.yahoo.search.federation.FederationSearcher;
+import com.yahoo.search.grouping.result.Group;
+import com.yahoo.search.grouping.result.RootGroup;
+import com.yahoo.search.grouping.result.StringBucketId;
+import com.yahoo.search.grouping.result.StringId;
+import com.yahoo.search.grouping.result.ValueGroupId;
 import com.yahoo.search.result.ErrorMessage;
 import com.yahoo.search.result.Hit;
+import com.yahoo.search.result.Relevance;
 import com.yahoo.search.searchchain.Execution;
 import com.yahoo.search.searchchain.SearchChain;
 import com.yahoo.search.searchchain.SearchChainRegistry;
@@ -489,4 +494,107 @@ public class BlendingSearcherTestCase {
                      e.getDetailedMessage());
     }
 
+    @Test
+    public void testBlendingFederationWithGrouping() {
+        DocumentSourceSearcher docSource1 = new DocumentSourceSearcher();
+        DocumentSourceSearcher docSource2 = new DocumentSourceSearcher();
+
+        Query q = new Query("/search?query=test");
+
+        Result r1 = new Result(q);
+        Result r2 = new Result(q);
+
+        RootGroup root1 = new RootGroup(0, null);
+        Group subGroup11 = new Group(new StringBucketId("a", "b"), new Relevance(1.0));
+        subGroup11.add(new Group(new StringId("unique1"), new Relevance(1.0)));
+        subGroup11.add(new Group(new StringId("unique5"), new Relevance(1.0)));
+        subGroup11.add(new Group(new StringId("unique6"), new Relevance(1.0)));
+        subGroup11.add(new Group(new StringId("unique7"), new Relevance(1.0)));
+        root1.add(subGroup11);
+        r1.hits().add(root1);
+        docSource1.addResult(q, r1);
+
+        RootGroup root2 = new RootGroup(0, null);
+        Group subGroup21 = new Group(new StringBucketId("a", "b"), new Relevance(1.0));
+        subGroup21.add(new Group(new StringId("unique1"), new Relevance(1.0)));
+        subGroup21.add(new Group(new StringId("unique2"), new Relevance(1.0)));
+        subGroup21.add(new Group(new StringId("unique3"), new Relevance(1.0)));
+        root2.add(subGroup21);
+        Group subGroup22 = new Group(new StringBucketId("c", "d"), new Relevance(1.0));
+        subGroup22.add(new Group(new StringId("unique3"), new Relevance(1.0)));
+        subGroup22.add(new Group(new StringId("unique4"), new Relevance(1.0)));
+        root2.add(subGroup22);
+        r2.hits().add(root2);
+        docSource2.addResult(q, r2);
+
+        BlendingSearcherWrapper blender = new BlendingSearcherWrapper();
+        blender.addSource(new Chain<>("a", new FillSearcher(), docSource1));
+        blender.addSource(new Chain<>("b", new FillSearcher(), docSource2));
+        blender.initialize();
+        q.setWindow( 0, 10);
+        Result result = new Execution(blender, Execution.Context.createContextStub()).search(q);
+        assertEquals(2, result.hits().size());
+
+        assertTrue(result.hits().get(0) instanceof RootGroup);
+        RootGroup resultRoot1 = (RootGroup)result.hits().get(0);
+        assertEquals(1, resultRoot1.asList().size());
+
+        assertTrue(result.hits().get(1) instanceof RootGroup);
+        RootGroup resultRoot2 = (RootGroup)result.hits().get(1);
+        assertEquals(2, resultRoot2.asList().size());
+    }
+
+    /** Multiple document types in the same cluster are returned without a top level group representing each */
+    @Test
+    public void testBlendingMultipleDocumentTypesWithGrouping() {
+        DocumentSourceSearcher docSource = new DocumentSourceSearcher();
+
+        Query q = new Query("/search?query=test");
+
+        Result r = new Result(q);
+
+        RootGroup root1 = new RootGroup(0, null);
+        Group subGroup11 = new Group(new StringBucketId("a", "b"), new Relevance(1.0));
+        subGroup11.add(new Group(new StringId("unique1"), new Relevance(1.0)));
+        subGroup11.add(new Group(new StringId("unique5"), new Relevance(1.0)));
+        subGroup11.add(new Group(new StringId("unique6"), new Relevance(1.0)));
+        subGroup11.add(new Group(new StringId("unique7"), new Relevance(1.0)));
+        root1.add(subGroup11);
+        r.hits().add(root1);
+
+        RootGroup root2 = new RootGroup(0, null);
+        Group subGroup21 = new Group(new StringBucketId("a", "b"), new Relevance(1.0));
+        subGroup21.add(new Group(new StringId("unique1"), new Relevance(1.0)));
+        subGroup21.add(new Group(new StringId("unique2"), new Relevance(1.0)));
+        subGroup21.add(new Group(new StringId("unique3"), new Relevance(1.0)));
+        root2.add(subGroup21);
+        Group subGroup22 = new Group(new StringBucketId("c", "d"), new Relevance(1.0));
+        subGroup22.add(new Group(new StringId("unique3"), new Relevance(1.0)));
+        subGroup22.add(new Group(new StringId("unique4"), new Relevance(1.0)));
+        root2.add(subGroup22);
+        r.hits().add(root2);
+
+        docSource.addResult(q, r);
+
+        Chain<Searcher> chain = new Chain<>("main",
+                                            new FillSearcher(),
+                                            new BlendingSearcher(ComponentId.fromString("test"), new QrSearchersConfig.Builder().build()),
+                                            docSource);
+        q.setWindow( 0, 10);
+        Result result = new Execution(chain, Execution.Context.createContextStub()).search(q);
+        assertEquals(3, result.hits().size());
+
+        assertTrue(result.hits().get(0) instanceof Group);
+        Group resultSubGroup1 = (Group)result.hits().get(0);
+        assertEquals(4, resultSubGroup1.asList().size());
+
+        assertTrue(result.hits().get(1) instanceof Group);
+        Group resultSubGroup2 = (Group)result.hits().get(1);
+        assertEquals(3, resultSubGroup2.asList().size());
+
+        assertTrue(result.hits().get(2) instanceof Group);
+        Group resultSubGroup3 = (Group)result.hits().get(2);
+        assertEquals(2, resultSubGroup3.asList().size());
+    }
+
 }
diff --git a/document/src/main/java/com/yahoo/document/BucketId.java b/document/src/main/java/com/yahoo/document/BucketId.java
index af3c53aa013..50dc9ad336a 100755
--- a/document/src/main/java/com/yahoo/document/BucketId.java
+++ b/document/src/main/java/com/yahoo/document/BucketId.java
@@ -5,9 +5,10 @@ package com.yahoo.document;
  * Representation of a bucket identifier.
  */
 public class BucketId implements Comparable<BucketId> {
+
     public static final int COUNT_BITS = 6;
     private long id = 0;
-    private static long[] usedMask;
+    private final static long[] usedMask;
 
     static {
         usedMask = new long[59];
@@ -18,9 +19,7 @@ public class BucketId implements Comparable<BucketId> {
         }
     }
 
-    /**
-     * Default-constructed BucketId signifies an invalid bucket ID.
-     */
+    /** Default-constructed BucketId signifies an invalid bucket ID. */
     public BucketId() {
     }
 
@@ -127,4 +126,5 @@ public class BucketId implements Comparable<BucketId> {
     public boolean contains(DocumentId docId, BucketIdFactory factory) {
         return contains(factory.getBucketId(docId));
     }
+
 }