Simplify and correct isFilled

2 files changed, 46 insertions, 53 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 022ef9950b3..483f55f9c92 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
@@ -845,62 +845,18 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable<
         return fillableHits().iterator().hasNext();
     }
 
+    /** Returns the set of summaries for which all concrete hits recursively below this is filled. */
     @Override
     public Set<String> getFilled() {
-        Iterator<Hit> hitIterator = hits.iterator();
-        Set<String> firstSummaryNames = getSummaryNamesNextFilledHit(hitIterator);
-        if (firstSummaryNames == null || firstSummaryNames.isEmpty())
-            return firstSummaryNames;
-
-        Set<String> intersection = firstSummaryNames;
-        while (true) {
-            Set<String> summaryNames = getSummaryNamesNextFilledHit(hitIterator);
-            if (summaryNames == null)
-                break;
-
-            if (intersection.size() == 1)
-                return getFilledSingle(first(intersection), hitIterator);
-
-
-            boolean notInSet = false;
-            if (intersection == firstSummaryNames) {
-                if (intersection.size() == summaryNames.size()) {
-                    for(String s : summaryNames) {
-                        if ( ! intersection.contains(s)) {
-                            intersection = new HashSet<>(firstSummaryNames);
-                            notInSet = true;
-                            break;
-                        }
-                    }
-                }
-            }
-            if (notInSet) {
-                intersection.retainAll(summaryNames);
-            }
-
-        }
-
-        return intersection;
-    }
-
-    private Set<String> getSummaryNamesNextFilledHit(Iterator<Hit> hitIterator) {
-        while (hitIterator.hasNext()) {
-            Set<String> filled = hitIterator.next().getFilled();
-            if (filled != null)
-                return filled;
-        }
-        return null;
-    }
-
-    private Set<String> getFilledSingle(String summaryName, Iterator<Hit> hitIterator) {
-        while (true) {
-            Set<String> summaryNames = getSummaryNamesNextFilledHit(hitIterator);
-            if (summaryNames == null) {
-                return Collections.singleton(summaryName);
-            } else if (!summaryNames.contains(summaryName)) {
-                return Collections.emptySet();
-            }
+        Set<String> filled = null;
+        for (Hit hit : hits) {
+            if (hit.getFilled() == null) continue;
+            if (filled == null)
+                filled = new HashSet<>(hit.getFilled());
+            else
+                filled.retainAll(hit.getFilled());
         }
+        return filled;
     }
 
     private Iterable<Hit> fillableHits() {
diff --git a/container-search/src/test/java/com/yahoo/search/result/test/FillingTestCase.java b/container-search/src/test/java/com/yahoo/search/result/test/FillingTestCase.java
index 01383ba29f2..8a068b7cb3b 100644
--- a/container-search/src/test/java/com/yahoo/search/result/test/FillingTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/result/test/FillingTestCase.java
@@ -5,6 +5,9 @@ import com.yahoo.search.result.Hit;
 import com.yahoo.search.result.HitGroup;
 import org.junit.Test;
 
+import java.util.Collections;
+
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
@@ -50,6 +53,40 @@ public class FillingTestCase {
         assertTrue(group.isFilled("otherSummary"));
     }
 
+    @Test
+    public void testPartiallyFilledWith2Hits() {
+        Hit hit1 = new Hit("id1");
+        Hit hit2 = new Hit("id2");
+
+        hit1.setFilled("summary");
+        hit2.setFillable();
+
+        HitGroup hits = new HitGroup();
+        hits.add(hit1);
+        hits.add(hit2);
+
+        assertEquals(Collections.emptySet(), hits.getFilled());
+    }
+
+    @Test
+    public void testPartiallyFilledDiverse() {
+        Hit hit1 = new Hit("id1");
+        Hit hit2 = new Hit("id2");
+        Hit hit3 = new Hit("id3");
+
+        hit1.setFilled("summary1");
+        hit1.setFilled("summary2");
+        hit2.setFilled("summary1");
+        hit3.setFilled("summary1");
+
+        HitGroup hits = new HitGroup();
+        hits.add(hit1);
+        hits.add(hit2);
+        hits.add(hit3);
+
+        assertEquals(Collections.singleton("summary1"), hits.getFilled());
+    }
+
     private Hit createNonFilled(String id) {
         Hit hit=new Hit(id);
         hit.setFillable();