Return an error when ranking.queryCache is not enabled when it needs to be.

3 files changed, 74 insertions, 29 deletions

diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java
index 9eb550374d0..bbc5af64c62 100644
--- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java
+++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java
@@ -56,9 +56,6 @@ public class FastSearcher extends VespaBackEndSearcher {
     /** If this is turned on this will make search queries directly to the local search node when possible */
     private final static CompoundName dispatchDirect = new CompoundName("dispatch.direct");
 
-    /** If this is turned on this will fill summaries by dispatching directly to search nodes over RPC */
-    private final static CompoundName dispatchSummaries = new CompoundName("dispatch.summaries");
-
     /** The compression method which will be used with rpc dispatch. "lz4" (default) and "none" is supported. */
     private final static CompoundName dispatchCompression = new CompoundName("dispatch.compression");
 
@@ -265,7 +262,7 @@ public class FastSearcher extends VespaBackEndSearcher {
         Query query = result.getQuery();
         traceQuery(getName(), "fill", query, query.getOffset(), query.getHits(), 2, quotedSummaryClass(summaryClass));
 
-        if (query.properties().getBoolean(dispatchSummaries)) {
+        if (wantsRPCSummarFill(query)) {
             CompressionType compression =
                 CompressionType.valueOf(query.properties().getString(dispatchCompression, "LZ4").toUpperCase());
             fillSDDocName(result);
@@ -483,31 +480,6 @@ public class FastSearcher extends VespaBackEndSearcher {
         return convertBasicPackets(receivedPackets);
     }
 
-    /**
-     * Returns whether we need to send the query when fetching summaries.
-     * This is necessary if the query requests summary features or dynamic snippeting
-     */
-    private boolean summaryNeedsQuery(Query query) {
-        if (query.getRanking().getQueryCache()) return false;  // Query is cached in backend
-
-        DocumentDatabase documentDb = getDocumentDatabase(query);
-
-        // Needed to generate a dynamic summary?
-        DocsumDefinition docsumDefinition = documentDb.getDocsumDefinitionSet().getDocsumDefinition(query.getPresentation().getSummary());
-        if (docsumDefinition == null) return true; // stay safe
-        if (docsumDefinition.isDynamic()) return true;
-
-        // Needed to generate ranking features?
-        RankProfile rankProfile = documentDb.rankProfiles().get(query.getRanking().getProfile());
-        if (rankProfile == null) return true; // stay safe
-        if (rankProfile.hasSummaryFeatures()) return true;
-        if (query.getRanking().getListFeatures()) return true;
-
-        // (Don't just add other checks here as there is a return false above)
-
-        return false;
-    }
-
     public String toString() {
         return "fast searcher (" + getName() + ") " + dispatchBackend;
     }
diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java
index 38d666c845a..ae6d029fde8 100644
--- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java
+++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java
@@ -55,6 +55,9 @@ public abstract class VespaBackEndSearcher extends PingableSearcher {
 
     private static final CompoundName grouping=new CompoundName("grouping");
     private static final CompoundName combinerows=new CompoundName("combinerows");
+    /** If this is turned on this will fill summaries by dispatching directly to search nodes over RPC */
+    private final static CompoundName dispatchSummaries = new CompoundName("dispatch.summaries");
+
     protected static final CompoundName PACKET_COMPRESSION_LIMIT = new CompoundName("packetcompressionlimit");
     protected static final CompoundName PACKET_COMPRESSION_TYPE = new CompoundName("packetcompressiontype");
     protected static final CompoundName TRACE_DISABLE = new CompoundName("trace.disable");
@@ -109,6 +112,35 @@ public abstract class VespaBackEndSearcher extends PingableSearcher {
 
     protected abstract void doPartialFill(Result result, String summaryClass);
 
+    protected static boolean wantsRPCSummarFill(Query query) {
+        return query.properties().getBoolean(dispatchSummaries);
+    }
+
+    /**
+     * Returns whether we need to send the query when fetching summaries.
+     * This is necessary if the query requests summary features or dynamic snippeting
+     */
+    protected boolean summaryNeedsQuery(Query query) {
+        if (query.getRanking().getQueryCache()) return false;  // Query is cached in backend
+
+        DocumentDatabase documentDb = getDocumentDatabase(query);
+
+        // Needed to generate a dynamic summary?
+        DocsumDefinition docsumDefinition = documentDb.getDocsumDefinitionSet().getDocsumDefinition(query.getPresentation().getSummary());
+        if (docsumDefinition == null) return true; // stay safe
+        if (docsumDefinition.isDynamic()) return true;
+
+        // Needed to generate ranking features?
+        RankProfile rankProfile = documentDb.rankProfiles().get(query.getRanking().getProfile());
+        if (rankProfile == null) return true; // stay safe
+        if (rankProfile.hasSummaryFeatures()) return true;
+        if (query.getRanking().getListFeatures()) return true;
+
+        // (Don't just add other checks here as there is a return false above)
+
+        return false;
+    }
+
     private Result cacheLookupFirstPhase(CacheKey key, QueryPacketData queryPacketData, Query query, int offset, int hits, String summaryClass) throws IOException {
         PacketWrapper packetWrapper = cacheControl.lookup(key, query);
 
@@ -186,6 +218,12 @@ public abstract class VespaBackEndSearcher extends PingableSearcher {
             return new Result(query, ErrorMessage.createNullQuery(query.getHttpRequest().getUri().toString()));
         }
 
+        if (wantsRPCSummarFill(query) && summaryNeedsQuery(query)) {
+            return new Result(query, ErrorMessage.createInvalidQueryParameter(
+                    "When using dispatch.summaries and your summary/rankprofile require the query, " +
+                    " you need to enable ranking.queryCache."));
+        }
+
         QueryRewrite.optimizeByRestrict(query);
         QueryRewrite.optimizeAndNot(query);
         QueryRewrite.collapseSingleComposites(query);
diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java
index ca8ed15f67b..2526e690868 100644
--- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java
@@ -103,6 +103,41 @@ public class FastSearcherTestCase {
     }
 
     @Test
+    public void testDispatchDotSummaries() {
+        Logger.getLogger(FastSearcher.class.getName()).setLevel(Level.ALL);
+        DocumentdbInfoConfig documentdbConfigWithOneDb =
+                new DocumentdbInfoConfig(new DocumentdbInfoConfig.Builder().documentdb(new DocumentdbInfoConfig.Documentdb.Builder()
+                        .name("testDb")
+                        .summaryclass(new DocumentdbInfoConfig.Documentdb.Summaryclass.Builder().name("simple").id(7))
+                        .rankprofile(new DocumentdbInfoConfig.Documentdb.Rankprofile.Builder()
+                                .name("simpler").hasRankFeatures(false).hasSummaryFeatures(false))));
+        FastSearcher fastSearcher = new FastSearcher(new MockBackend(),
+                new FS4ResourcePool(1),
+                new MockDispatcher(Collections.emptyList()),
+                new SummaryParameters(null),
+                new ClusterParams("testhittype"),
+                new CacheParams(100, 1e64),
+                documentdbConfigWithOneDb);
+
+        String query = "?query=sddocname:a&dispatch.summaries";
+        Result result = doSearch(fastSearcher,new Query(query), 0, 10);
+        ErrorMessage message = result.hits().getError();
+
+        assertNotNull("Got error", message);
+        assertEquals("Invalid query parameter", message.getMessage());
+        assertEquals("When using dispatch.summaries and your summary/rankprofile require the query,  you need to enable ranking.queryCache.", message.getDetailedMessage());
+        assertEquals(Error.INVALID_QUERY_PARAMETER.code, message.getCode());
+
+        query = "?query=sddocname:a&dispatch.summaries&ranking.queryCache";
+        result = doSearch(fastSearcher,new Query(query), 0, 10);
+        assertNull(result.hits().getError());
+
+        query = "?query=sddocname:a&dispatch.summaries&summary=simple&ranking=simpler";
+        result = doSearch(fastSearcher,new Query(query), 0, 10);
+        assertNull(result.hits().getError());
+    }
+
+    @Test
     public void testQueryWithRestrict() {
         mockBackend = new MockBackend();
         DocumentdbInfoConfig documentdbConfigWithOneDb =