1 files changed, 67 insertions, 54 deletions
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 1469596d20e..8c7e05aee90 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
@@ -11,23 +11,26 @@ import com.yahoo.processing.response.DataList;
 import com.yahoo.processing.response.DefaultIncomingData;
 import com.yahoo.processing.response.IncomingData;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import static com.yahoo.collections.CollectionUtil.first;
 
 /**
  * <p>A group of ordered hits. Since hitGroup is itself a kind of Hit,
- * this can compose hierarchies of grouped hits.</p>
+ * this can compose hierarchies of grouped hits.
  *
  * <p>Group hits has a relevancy just as other hits - they can be ordered
  * between each other and in comparison to other hits.
  *
  * <p>Note that a group is by default a meta hit, but it can also contain its own content
- * in addition to subgroup content, in which case it should be set to non-meta.</p>
+ * in addition to subgroup content, in which case it should be set to non-meta.
  *
  * @author bratseth
  */
@@ -77,7 +80,7 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable<
      * A direct reference to the errors of this result, or null if there are no errors.
      * The error hit will also be listed in the set of this of this result
      */
-    private ErrorHit errorHit = null;
+    private DefaultErrorHit errorHit = null;
 
     private final ListenableFuture<DataList<Hit>> completedFuture;
 
@@ -223,12 +226,20 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable<
      */
     @Override
     public Hit add(Hit hit) {
-        if (hit.isMeta() && hit instanceof ErrorHit) {
-            boolean add = mergeErrors((ErrorHit) hit);
-            if (!add) return (Hit)errorHit;
+        System.out.println("Source of incoming hit: " + hit.getSource());
+        if (hit.isMeta() && hit instanceof DefaultErrorHit) {
+            if (errorHit != null) {
+                errorHit.addErrors((DefaultErrorHit)hit);
+                return errorHit; // don't add another error hit
+            }
+            else {
+                errorHit = merge(consumeAnyQueryErrors(), (DefaultErrorHit) hit);
+                hit = errorHit; // Add this hit below
+            }
         }
         handleNewHit(hit);
         hits.add(hit);
+        System.out.println("Source of hit added: " + hit.getSource());
         return hit;
     }
 
@@ -340,18 +351,19 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable<
     /**
      * Removes a hit from this group or any subgroup.
      *
-     * @param uri The uri of the hit to remove.
-     * @return The hit removed, or null if not found.
+     * @param uri the uri of the hit to remove.
+     * @return the hit removed, or null if not found.
      */
     public Hit remove(URI uri) {
         for (Iterator<Hit> it = hits.iterator(); it.hasNext(); ) {
             Hit hit = it.next();
+            System.out.println("Is " + uri + " equal to " + hit.getId() + "?");
             if (uri.equals(hit.getId())) {
                 it.remove();
                 handleRemovedHit(hit);
                 return hit;
             }
-            if (hit instanceof HitGroup) {
+            else if (hit instanceof HitGroup) {
                 Hit removed = ((HitGroup)hit).remove(uri);
                 if (removed != null) {
                     return removed;
@@ -391,7 +403,7 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable<
     public void addError(ErrorMessage error) {
         getError(); // update the list of errors
         if (errorHit == null)
-            add((Hit)createErrorHit(error));
+            add(new DefaultErrorHit(getSource(), error));
         else
             errorHit.addError(error);
     }
@@ -408,9 +420,11 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable<
      *
      * @return the error hit which was removed, or null if there were no errors
      */
-    public ErrorHit removeErrorHit() {
+    public DefaultErrorHit removeErrorHit() {
         updateHits(); // Consume and remove from the query producing this as well
-        ErrorHit removed = errorHit;
+        DefaultErrorHit removed = errorHit;
+        if (removed != null)
+            remove(removed.getId());
         errorHit = null;
         return removed;
     }
@@ -420,57 +434,53 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable<
      * or null if no searcher has produced an error AND the query doesn't contain an error
      */
     public ErrorMessage getError() {
-        // See updateHits if this method is changed
-        if (errorHit != null) {
-            return errorHit.errors().iterator().next();
-        }
-
-        if (getQuery() != null && getQuery().errors().size() != 0) {
-            updateHits();
-        } // Pull them over
-
-        if (errorHit == null) {
+        updateHits();
+        if (errorHit == null)
             return null;
-        }
-
-        return errorHit.errors().iterator().next();
+        else
+            return errorHit.errors().iterator().next();
     }
 
     /**
-     * Handles the addition of a new error hit, whether or not we already have one
+     * Combines two error hits to one. Any one argument may be null, in which case the other is returned.
      *
-     * @return true if this shouls also be added to the list of hits of this reslt
+     * @return true if this should also be added to the list of hits of this result
      */
-    private boolean mergeErrors(ErrorHit newHit) {
-        if (errorHit == null) {
-            errorHit = newHit;
-            return true;
-        } else {
-            errorHit.addErrors(newHit);
-            return false;
-        }
+    private DefaultErrorHit merge(DefaultErrorHit first, DefaultErrorHit second) {
+        if (first == null) return second;
+        if (second == null) return first;
+
+        String mergedSource = first.getSource()!=null ? first.getSource() : second.getSource();
+        List<ErrorMessage> mergedErrors = new ArrayList<>();
+        mergedErrors.addAll(first.errors());
+        mergedErrors.addAll(second.errors());
+        return new DefaultErrorHit(mergedSource, mergedErrors);
     }
 
     /**
      * Must be called before the list of hits, or anything dependent on the list of hits, is removed.
-     * Merges errors from the query if there is one set for this group
+     * Consumes errors from the query if there is one set for this group
      */
     private void updateHits() {
-        if (getQuery() == null) return;
-
-        if (getQuery().errors().size() == 0) return;
-
-        if (errorHit == null)
-            add((Hit)createErrorHit(toSearchError(getQuery().errors().get(0))));
-
-        // Add the rest of the errors
-        for (int i = 1; i < getQuery().errors().size(); i++)
-            errorHit.addError(toSearchError(getQuery().errors().get(i)));
-        getQuery().errors().clear();
+        DefaultErrorHit queryErrors = consumeAnyQueryErrors();
+        if (queryErrors != null)
+            add(queryErrors);
     }
 
-    protected ErrorHit createErrorHit(ErrorMessage errorMessage) {
-        return new DefaultErrorHit(getSource(), errorMessage);
+    /** 
+     * Consumes errors from the query and returns them in a new error hit
+     * 
+     * @return the error hit containing all query errors, or null if no query errors should be consumed
+     */
+    private DefaultErrorHit consumeAnyQueryErrors() {
+        if (errorHit != null) return null;
+        if (getQuery() == null) return null;
+        if (getQuery().errors().isEmpty()) return null;
+
+        // Move errors from the query into this
+        List<ErrorMessage> queryErrors = getQuery().errors().stream().map(this::toSearchError).collect(Collectors.toList());
+        getQuery().errors().clear(); // TODO: Remove this line (not promised, can be done at any time)
+        return new DefaultErrorHit(getSource(), queryErrors);
     }
 
     /** Compatibility */
@@ -478,7 +488,7 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable<
         if (error instanceof ErrorMessage) 
             return (ErrorMessage)error;
         else 
-            return new ErrorMessage(error.getCode(),error.getMessage(),error.getDetailedMessage(),error.getCause());
+            return new ErrorMessage(error.getCode(), error.getMessage(), error.getDetailedMessage(), error.getCause());
     }
 
     /**
@@ -720,14 +730,17 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable<
 
     // Filled is not kept in sync at removal
     private void handleRemovedHit(Hit hit) {
-        if (!hit.isAuxiliary()) {
+        if ( ! hit.isAuxiliary()) {
             concreteHitCount--;
-            if (!hit.isCached())
+            if ( ! hit.isCached())
                 notCachedCount--;
         }
         else if (hit instanceof HitGroup) {
             subgroupCount--;
         }
+        else if (hit instanceof DefaultErrorHit) {
+            errorHit = null;
+        }
 
         if (deletionBreaksOrdering) {
             hitsSorted = false;
@@ -812,8 +825,8 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable<
         }
         if (this.errorHit!=null) { // Find the cloned error and assign it
             for (Hit hit : hitGroupClone.asList()) {
-                if (hit instanceof ErrorHit)
-                    hitGroupClone.errorHit=(ErrorHit)hit;
+                if (hit instanceof DefaultErrorHit)
+                    hitGroupClone.errorHit=(DefaultErrorHit)hit;
             }
         }