From 5abaceb7f7ae65aa0060c3141610aa82498a19d9 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Fri, 9 Feb 2024 10:41:23 +0100 Subject: - 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. --- .../result/FlatteningSearcherTestCase.java | 4 +- .../grouping/vespa/GroupingExecutorTestCase.java | 63 +++++++++------------- .../grouping/vespa/HitConverterTestCase.java | 40 +++++++------- 3 files changed, 49 insertions(+), 58 deletions(-) (limited to 'container-search/src/test') 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 it = result.hits().deepIterator(); it.hasNext();) { Hit hit = it.next(); Query query = hit.getQuery(); - List lst = hitsByQuery.get(query); - if (lst == null) { - lst = new LinkedList<>(); - hitsByQuery.put(query, lst); - } + List 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 it = result.hits().deepIterator(); it.hasNext();) { Hit hit = it.next(); - List lst = hitsBySummary.get(summaryClass); - if (lst == null) { - lst = new LinkedList<>(); - hitsBySummary.put(summaryClass, lst); - } + List 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); -- cgit v1.2.3