Validate rank profiles

4 files changed, 48 insertions, 158 deletions

diff --git a/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java b/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java
index a1cad6de419..496e057a243 100644
--- a/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java
+++ b/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java
@@ -38,7 +38,6 @@ import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executor;
 import java.util.concurrent.FutureTask;
 import java.util.concurrent.RejectedExecutionException;
-import java.util.stream.Collectors;
 
 import static com.yahoo.container.QrSearchersConfig.Searchcluster.Indexingmode.STREAMING;
 
@@ -166,63 +165,6 @@ public class ClusterSearcher extends Searcher {
         this(schemas, null, null);
     }
 
-    /**
-     * Returns an error if the document types do not have the requested rank
-     * profile. For the case of multiple document types, only returns an
-     * error if we have restricted the set of documents somehow. This is
-     * because when searching over all doc types, common ancestors might
-     * not have the requested rank profile and failing on that basis is
-     * probably not reasonable.
-     *
-     * @param  query    query
-     * @param  schemas set of requested schemas for this query
-     * @return          null if requested rank profile is ok for the requested
-     *                  schemas, a result with error message if not.
-     */
-    // TODO: This should be in a separate searcher
-    // TODO Vespa 8: This should simply fail if the specified profile isn't present in all schemas
-    private Result checkValidRankProfiles(Query query, Set<String> schemas, Execution.Context context) {
-        String rankProfile = query.getRanking().getProfile();
-        Set<String> invalidInSchemas = null;
-        Set<String> schemasHavingProfile = schemasHavingProfile(rankProfile, context);
-
-        if (schemasHavingProfile.isEmpty()) {
-            invalidInSchemas = schemas;
-        }
-        else if (schemas.size() == 1) {
-            if ( ! schemasHavingProfile.containsAll(schemas))
-                invalidInSchemas = schemas;
-        }
-        else {
-            // multiple schemas, only fail when restricting doc types
-            Set<String> restrict = query.getModel().getRestrict();
-            Set<String> sources = query.getModel().getSources();
-            boolean validate = restrict != null && !restrict.isEmpty();
-            validate = validate || sources != null && !sources.isEmpty();
-            if (validate && !schemasHavingProfile.containsAll(schemas)) {
-                invalidInSchemas = new HashSet<>(schemas);
-                invalidInSchemas.removeAll(schemasHavingProfile);
-            }
-        }
-
-        if (invalidInSchemas != null && !invalidInSchemas.isEmpty()) {
-            String plural = invalidInSchemas.size() > 1 ? "s" : "";
-            return new Result(query,
-                              ErrorMessage.createInvalidQueryParameter("Requested rank profile '" + rankProfile +
-                                                                       "' is undefined for document type" + plural + " '" +
-                                                                       String.join(", ", invalidInSchemas) + "'"));
-        }
-
-        return null;
-    }
-
-    private Set<String> schemasHavingProfile(String profile, Execution.Context context) {
-        return context.schemaInfo().schemas().values().stream()
-                                             .filter(schema -> schema.rankProfiles().containsKey(profile))
-                                             .map(schema -> schema.name())
-                                             .collect(Collectors.toSet());
-    }
-
     @Override
     public void fill(com.yahoo.search.Result result, String summaryClass, Execution execution) {
         Query query = result.getQuery();
@@ -285,12 +227,6 @@ public class ClusterSearcher extends Searcher {
             return searchMultipleDocumentTypes(searcher, query, execution);
         } else {
             String docType = schemas.iterator().next();
-
-            Result invalidRankProfile = checkValidRankProfiles(query, schemas, execution.context());
-            if (invalidRankProfile != null) {
-                return invalidRankProfile;
-            }
-
             query.getModel().setRestrict(docType);
             return searcher.search(query, execution);
         }
@@ -310,10 +246,6 @@ public class ClusterSearcher extends Searcher {
 
     private Result searchMultipleDocumentTypes(Searcher searcher, Query query, Execution execution) {
         Set<String> schemas = resolveSchemas(query, execution.context().getIndexFacts());
-
-        Result invalidRankProfile = checkValidRankProfiles(query, schemas, execution.context());
-        if (invalidRankProfile != null) return invalidRankProfile;
-
         List<Query> queries = createQueries(query, schemas);
         if (queries.size() == 1) {
             return searcher.search(queries.get(0), execution);
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 21037be1a8b..584249fa1f3 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
@@ -14,7 +14,6 @@ import com.yahoo.protect.Validator;
 import com.yahoo.search.Query;
 import com.yahoo.search.Result;
 import com.yahoo.search.cluster.PingableSearcher;
-import com.yahoo.search.config.SchemaInfoConfig;
 import com.yahoo.search.schema.RankProfile;
 import com.yahoo.search.grouping.vespa.GroupingExecutor;
 import com.yahoo.search.result.ErrorMessage;
@@ -168,6 +167,11 @@ public abstract class VespaBackEndSearcher extends PingableSearcher {
             return new Result(query, ErrorMessage.createNullQuery(query.getHttpRequest().getUri().toString()));
         }
 
+        if ( ! getDocumentDatabase(query).schema().rankProfiles().containsKey(query.getRanking().getProfile()))
+            return new Result(query, ErrorMessage.createInvalidQueryParameter(getDocumentDatabase(query).schema() +
+                                                                              " does not contain requested rank profile '" +
+                                                                              query.getRanking().getProfile() + "'"));
+
         QueryRewrite.optimizeByRestrict(query);
         QueryRewrite.optimizeAndNot(query);
         QueryRewrite.collapseSingleComposites(query);
@@ -185,8 +189,6 @@ public abstract class VespaBackEndSearcher extends PingableSearcher {
             return new Result(query);
 
         Result result = doSearch2(query, execution);
-        if (isLoggingFine())
-            getLogger().fine("Result NOT retrieved from cache");
 
         if (query.getTraceLevel() >= 1)
             query.trace(getName() + " dispatch response: " + result, false, 1);
diff --git a/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java
index 434f4bcc8e0..a09c2ff9b79 100644
--- a/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java
@@ -8,7 +8,6 @@ import com.yahoo.concurrent.InThreadExecutorService;
 import com.yahoo.container.QrSearchersConfig;
 import com.yahoo.container.handler.ClustersStatus;
 import com.yahoo.container.handler.VipStatus;
-import com.yahoo.container.protect.Error;
 import com.yahoo.prelude.IndexFacts;
 import com.yahoo.prelude.IndexModel;
 import com.yahoo.prelude.SearchDefinition;
@@ -428,81 +427,6 @@ public class ClusterSearcherTestCase {
         assertResult(6, new ArrayList<>(),  getResult(6, 2, extra, ex));
     }
 
-    @Test
-    public void testRequireThatSearchFailsForUndefinedRankProfileWithOneDocType() {
-        Execution execution = createExecution(Arrays.asList("type1"), false);
-
-        // "default" rank profile
-        Query query = new Query("?query=hello");
-        com.yahoo.search.Result result = execution.search(query);
-        assertEquals(3, result.getTotalHitCount());
-
-        // specified "default" rank profile
-        query = new Query("?query=hello&ranking.profile=default");
-        result = execution.search(query);
-        assertEquals(3, result.getTotalHitCount());
-
-        // empty rank profile, should fail
-        query = new Query("?query=hello&ranking.profile=");
-        result = execution.search(query);
-        assertEquals(0, result.getTotalHitCount());
-        assertEquals(result.hits().getError().getCode(), Error.INVALID_QUERY_PARAMETER.code);
-
-        // invalid rank profile
-        query = new Query("?query=hello&ranking.profile=undefined");
-        result = execution.search(query);
-        assertEquals(0, result.getTotalHitCount());
-        assertEquals(result.hits().getError().getCode(), Error.INVALID_QUERY_PARAMETER.code);
-
-        // valid rank profile for type1
-        query = new Query("?query=hello&ranking.profile=testprofile");
-        result = execution.search(query);
-        assertEquals(3, result.getTotalHitCount());
-    }
-
-    @Test
-    public void testRequireThatSearchFailsForUndefinedRankProfileWithMultipleDocTypes() {
-        Execution execution = createExecution(Arrays.asList("type1", "type2", "type3"), false);
-
-        // "default" rank profile
-        Query query = new Query("?query=hello");
-        com.yahoo.search.Result result = execution.search(query);
-        assertEquals(9, result.getTotalHitCount());
-
-        // specified "default" rank profile
-        query = new Query("?query=hello&ranking.profile=default");
-        result = execution.search(query);
-        assertEquals(9, result.getTotalHitCount());
-
-        // empty rank profile, should fail
-        query = new Query("?query=hello&ranking.profile=");
-        result = execution.search(query);
-        assertEquals(0, result.getTotalHitCount());
-        assertEquals(result.hits().getError().getCode(), Error.INVALID_QUERY_PARAMETER.code);
-
-        // invalid rank profile
-        query = new Query("?query=hello&ranking.profile=undefined");
-        result = execution.search(query);
-        assertEquals(0, result.getTotalHitCount());
-        assertEquals(result.hits().getError().getCode(), Error.INVALID_QUERY_PARAMETER.code);
-
-        // testprofile is only defined for type1, but should pass as it exists in at least one document type
-        query = new Query("?query=hello&ranking.profile=testprofile");
-        result = execution.search(query);
-        assertEquals(9, result.getTotalHitCount());
-
-        // testprofile is only defined for type1, but should fail when restricting doc types
-        query = new Query("?query=hello&ranking.profile=testprofile&restrict=type1,type3");
-        result = execution.search(query);
-        assertEquals(0, result.getTotalHitCount());
-        assertEquals(result.hits().getError().getCode(), Error.INVALID_QUERY_PARAMETER.code);
-
-        // testprofile is only defined for type1, ok if restricted to type1
-        query = new Query("?query=hello&ranking.profile=testprofile&restrict=type1");
-        result = execution.search(query);
-        assertEquals(3, result.getTotalHitCount());
-    }
-
     private static ClusterSearcher createSearcher(String clusterName, Double maxQueryTimeout, Double maxQueryCacheTimeout,
                                                   boolean streamingMode, VipStatus vipStatus) {
         QrSearchersConfig.Builder qrSearchersConfig = new QrSearchersConfig.Builder();
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 c9193d08381..271b932dff5 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
@@ -16,6 +16,7 @@ import com.yahoo.prelude.fastsearch.VespaBackEndSearcher;
 import com.yahoo.search.Query;
 import com.yahoo.search.Result;
 import com.yahoo.search.Searcher;
+import com.yahoo.search.config.SchemaInfoConfig;
 import com.yahoo.search.dispatch.rpc.RpcResourcePool;
 import com.yahoo.search.dispatch.searchcluster.Node;
 import com.yahoo.search.grouping.GroupingRequest;
@@ -41,6 +42,7 @@ import java.util.logging.Logger;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 
@@ -51,9 +53,6 @@ import static org.junit.Assert.assertTrue;
  */
 public class FastSearcherTestCase {
 
-    private final static DocumentdbInfoConfig documentdbInfoConfig = new DocumentdbInfoConfig(new DocumentdbInfoConfig.Builder());
-
-
     @Test
     public void testNullQuery() {
         Logger.getLogger(FastSearcher.class.getName()).setLevel(Level.ALL);
@@ -61,11 +60,11 @@ public class FastSearcherTestCase {
                                                      MockDispatcher.create(Collections.emptyList()),
                                                      new SummaryParameters(null),
                                                      new ClusterParams("testhittype"),
-                                                     documentdbInfoConfig,
-                                                     SchemaInfo.empty());
+                                                     documentdbInfoConfig("test"),
+                                                     schemaInfo("test"));
 
         String query = "?junkparam=ignored";
-        Result result = doSearch(fastSearcher,new Query(query), 0, 10);
+        Result result = doSearch(fastSearcher, new Query(query), 0, 10);
         ErrorMessage message = result.hits().getError();
 
         assertNotNull("Got error", message);
@@ -93,11 +92,11 @@ public class FastSearcherTestCase {
     @Test
     public void testSinglePassGroupingIsForcedWithSingleNodeGroups() {
         FastSearcher fastSearcher = new FastSearcher("container.0",
-                                                     MockDispatcher.create(Collections.singletonList(new Node(0, "host0", 0))),
+                                                     MockDispatcher.create(List.of(new Node(0, "host0", 0))),
                                                      new SummaryParameters(null),
                                                      new ClusterParams("testhittype"),
-                                                     documentdbInfoConfig,
-                                                     SchemaInfo.empty());
+                                                     documentdbInfoConfig("test"),
+                                                     schemaInfo("test"));
         Query q = new Query("?query=foo");
         GroupingRequest request1 = GroupingRequest.newInstance(q);
         request1.setRootOperation(new AllOperation());
@@ -114,6 +113,19 @@ public class FastSearcherTestCase {
     }
 
     @Test
+    public void testRankProfileValidation() {
+        FastSearcher fastSearcher = new FastSearcher("container.0",
+                                                     MockDispatcher.create(List.of(new Node(0, "host0", 0))),
+                                                     new SummaryParameters(null),
+                                                     new ClusterParams("testhittype"),
+                                                     documentdbInfoConfig("test"),
+                                                     schemaInfo("test"));
+        assertFalse(searchError("?query=q", fastSearcher).contains("does not contain requested rank profile"));
+        assertFalse(searchError("?query=q&ranking.profile=default", fastSearcher).contains("does not contain requested rank profile"));
+        assertTrue(searchError("?query=q&ranking.profile=nosuch", fastSearcher).contains("does not contain requested rank profile"));
+    }
+
+    @Test
     public void testSummaryNeedsQuery() {
         var documentDb = new DocumentdbInfoConfig(new DocumentdbInfoConfig.Builder().documentdb(new DocumentdbInfoConfig.Documentdb.Builder().name("test")));
         var schema = new Schema.Builder("test")
@@ -145,8 +157,8 @@ public class FastSearcherTestCase {
                                                      dispatcher,
                                                      new SummaryParameters(null),
                                                      new ClusterParams("testhittype"),
-                                                     documentdbInfoConfig,
-                                                     SchemaInfo.empty());
+                                                     documentdbInfoConfig("test"),
+                                                     schemaInfo("test"));
         Query q = new Query("?query=foo");
         GroupingRequest request1 = GroupingRequest.newInstance(q);
         request1.setRootOperation(new AllOperation());
@@ -192,4 +204,24 @@ public class FastSearcherTestCase {
         assertTrue(vipStatus.isInRotation()); //Verify that deconstruct does not touch vipstatus
     }
 
+    private String searchError(String query, Searcher searcher) {
+        return search(query, searcher).hits().getError().getDetailedMessage();
+    }
+
+    private Result search(String query, Searcher searcher) {
+        return searcher.search(new Query(query), new Execution(Execution.Context.createContextStub()));
+    }
+
+    private DocumentdbInfoConfig documentdbInfoConfig(String schemaName) {
+        var db = new DocumentdbInfoConfig.Documentdb.Builder().name(schemaName);
+        db.rankprofile(new DocumentdbInfoConfig.Documentdb.Rankprofile.Builder().name("default"));
+        return new DocumentdbInfoConfig.Builder().documentdb(db).build();
+    }
+
+    private SchemaInfo schemaInfo(String schemaName) {
+        var schema = new Schema.Builder(schemaName);
+        schema.add(new RankProfile.Builder("default").build());
+        return new SchemaInfo(List.of(schema.build()),  Map.of());
+    }
+
 }