Merge pull request #30682 from vespa-engine/balder/common-summary-error-handling

Balder/common summary error handling

4 files changed, 27 insertions, 13 deletions

diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackend.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackend.java
index 761cb22be57..a463fb9d0e6 100644
--- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackend.java
+++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackend.java
@@ -9,6 +9,7 @@ import com.yahoo.prelude.query.Item;
 import com.yahoo.prelude.query.NullItem;
 import com.yahoo.prelude.query.textualrepresentation.TextualQueryRepresentation;
 import com.yahoo.prelude.querytransform.QueryRewrite;
+import com.yahoo.processing.IllegalInputException;
 import com.yahoo.protect.Validator;
 import com.yahoo.search.Query;
 import com.yahoo.search.Result;
@@ -212,13 +213,26 @@ public abstract class VespaBackend {
         List<Result> parts = partitionHits(result, summaryClass);
         if (!parts.isEmpty()) { // anything to fill at all?
             for (Result r : parts) {
-                doPartialFill(r, summaryClass);
+                doPartialFill(r, ensureLegalSummaryClass(r.getQuery(), summaryClass));
                 mergeErrorsInto(result, r);
             }
             result.hits().setSorted(false);
             result.analyzeHits();
         }
     }
+    protected String ensureLegalSummaryClass(Query query, String summaryClass) {
+        if (summaryClass != null) {
+            if (summaryClass.isEmpty()) {
+                return null;
+            } else {
+                var db = getDocumentDatabase(query);
+                if (db != null && ! db.getDocsumDefinitionSet().hasDocsum(summaryClass)) {
+                    throw new IllegalInputException("invalid presentation.summary=" + summaryClass);
+                }
+            }
+        }
+        return summaryClass;
+    }
 
     private void mergeErrorsInto(Result destination, Result source) {
         destination.hits().addErrorsFrom(source.hits());
diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcProtobufFillInvoker.java b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcProtobufFillInvoker.java
index 7220bc61fe5..fe6dc9abe19 100644
--- a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcProtobufFillInvoker.java
+++ b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcProtobufFillInvoker.java
@@ -12,7 +12,6 @@ import com.yahoo.data.access.slime.SlimeAdapter;
 import com.yahoo.prelude.fastsearch.DocumentDatabase;
 import com.yahoo.prelude.fastsearch.FastHit;
 import com.yahoo.prelude.fastsearch.TimeoutException;
-import com.yahoo.processing.IllegalInputException;
 import com.yahoo.search.Query;
 import com.yahoo.search.Result;
 import com.yahoo.search.dispatch.FillInvoker;
@@ -23,7 +22,6 @@ import com.yahoo.slime.ArrayTraverser;
 import com.yahoo.slime.BinaryFormat;
 import com.yahoo.slime.BinaryView;
 
-import java.util.Iterator;
 import java.util.List;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.LinkedBlockingQueue;
@@ -72,13 +70,6 @@ public class RpcProtobufFillInvoker extends FillInvoker {
 
     @Override
     protected void sendFillRequest(Result result, String summaryClass) {
-        if (summaryClass != null) {
-            if (summaryClass.equals("")) {
-                summaryClass = null;
-            } else if (! documentDb.getDocsumDefinitionSet().hasDocsum(summaryClass)) {
-                throw new IllegalInputException("invalid presentation.summary=" + summaryClass);
-            }
-        }
         ListMap<Integer, FastHit> hitsByNode = hitsByNode(result);
 
         result.getQuery().trace(false, 5, "Sending ", hitsByNode.size(), " summary fetch requests with jrt/protobuf");
diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/StreamingBackend.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/StreamingBackend.java
index 6b81ab0fa97..cdd812222cf 100644
--- a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/StreamingBackend.java
+++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/StreamingBackend.java
@@ -16,6 +16,7 @@ import com.yahoo.prelude.fastsearch.FastHit;
 import com.yahoo.prelude.fastsearch.GroupingListHit;
 import com.yahoo.prelude.fastsearch.TimeoutException;
 import com.yahoo.prelude.fastsearch.VespaBackend;
+import com.yahoo.processing.IllegalInputException;
 import com.yahoo.processing.request.CompoundName;
 import com.yahoo.search.Query;
 import com.yahoo.search.Result;
@@ -29,6 +30,7 @@ import com.yahoo.vdslib.DocumentSummary;
 import com.yahoo.vdslib.SearchResult;
 import com.yahoo.vdslib.VisitorStatistics;
 import com.yahoo.vespa.streamingvisitors.tracing.TraceDescription;
+import com.yahoo.yolean.Exceptions;
 
 import java.math.BigInteger;
 import java.util.List;
@@ -131,6 +133,11 @@ public class StreamingBackend extends VespaBackend {
             return new Result(query, ErrorMessage.createIllegalQuery("Streaming search requires either " +
                                                                      "streaming.groupname or streaming.selection"));
         }
+        try {
+            ensureLegalSummaryClass(query, query.getPresentation().getSummary());
+        } catch (IllegalInputException e) {
+            return new Result(query, ErrorMessage.createIllegalQuery(Exceptions.toMessageString(e)));
+        }
 
         if (query.getTrace().isTraceable(4))
             query.trace("Routing to search cluster " + getSearchClusterName() + " and document type " + schema, 4);
diff --git a/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/StreamingSearcherTestCase.java b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/StreamingSearcherTestCase.java
index 8d264f9860b..cd9ef708920 100644
--- a/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/StreamingSearcherTestCase.java
+++ b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/StreamingSearcherTestCase.java
@@ -229,11 +229,13 @@ public class StreamingSearcherTestCase {
     @Test
     void testBasics() {
         MockVisitorFactory factory = new MockVisitorFactory();
-        var schema = new Schema.Builder("test");
-        schema.add(new com.yahoo.search.schema.DocumentSummary.Builder("default").build());
+        var schema = new Schema.Builder("test")
+                .add(new com.yahoo.search.schema.DocumentSummary.Builder("default").build())
+                .add(new com.yahoo.search.schema.DocumentSummary.Builder("summary").build())
+                .build();
         ClusterParams clusterParams = new ClusterParams("clusterName", "server.0", "default",
                 new DocumentdbInfoConfig.Builder().documentdb(new DocumentdbInfoConfig.Documentdb.Builder().name("test")).build(),
-                new SchemaInfo(List.of(schema.build()), List.of()));
+                new SchemaInfo(List.of(schema), List.of()));
         StreamingBackend searcher = new StreamingBackend(clusterParams, "search-cluster-A", factory, "content-cluster-A");
 
         // Magic query values are used to trigger specific behaviors from mock visitor.