Adds integration test for invalid method parameters

Signed-off-by: Tejas Shah <[email protected]>
opensearch-project · Jul 1, 2024 · 2e1dc89 · 2e1dc89
1 parent 7ee1ab4
commit 2e1dc89
Show file tree

Hide file tree

Showing 3 changed files with 153 additions and 1 deletion.
diff --git a/src/main/java/org/opensearch/knn/index/util/DefaultIVFContext.java b/src/main/java/org/opensearch/knn/index/util/DefaultIVFContext.java
@@ -24,7 +24,7 @@ public final class DefaultIVFContext implements EngineSpecificMethodContext {
         .build();
 
     @Override
-    public Map<String, Parameter<?>> supportedMethodParameters() {
+    public Map<String, Parameter<?>> supportedMethodParameters(QueryContext context) {
         return supportedMethodParameters;
     }
 }
diff --git a/src/test/java/org/opensearch/knn/index/query/InvalidSearchQueryIT.java b/src/test/java/org/opensearch/knn/index/query/InvalidSearchQueryIT.java
@@ -0,0 +1,139 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ *
+ * Modifications Copyright OpenSearch Contributors. See
+ * GitHub history for details.
+ */
+
+package org.opensearch.knn.index.query;
+
+import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
+import lombok.AllArgsConstructor;
+import lombok.SneakyThrows;
+import org.opensearch.client.Request;
+import org.opensearch.client.ResponseException;
+import org.opensearch.common.xcontent.XContentFactory;
+import org.opensearch.core.xcontent.XContentBuilder;
+import org.opensearch.knn.KNNRestTestCase;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collection;
+
+import static com.carrotsearch.randomizedtesting.RandomizedTest.$;
+import static com.carrotsearch.randomizedtesting.RandomizedTest.$$;
+
+@AllArgsConstructor
+public class InvalidSearchQueryIT extends KNNRestTestCase {
+
+    private String description;
+    private XContentBuilder xContentBuilder;
+
+    @ParametersFactory(argumentFormatting = "description:%1$s; request:%2$s, expectedexception:%3$s")
+    public static Collection<Object[]> parameters() throws IOException {
+        return Arrays.asList(
+            $$(
+                $(
+                    "Empty method_parameter",
+                    XContentFactory.jsonBuilder()
+                        .startObject()
+                        .startObject("query")
+                        .startObject("knn")
+                        .startObject(FIELD_NAME)
+                        .field("vector", new float[] { 1.0f, 2.0f })
+                        .field("k", 1)
+                        .startObject("method_parameter")
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                ),
+                $(
+                    "ef_search string",
+                    XContentFactory.jsonBuilder()
+                        .startObject()
+                        .startObject("query")
+                        .startObject("knn")
+                        .startObject(FIELD_NAME)
+                        .field("vector", new float[] { 1.0f, 2.0f })
+                        .field("k", 1)
+                        .startObject("method_parameter")
+                        .field("ef_search", "string value")
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                ),
+                $(
+                    "ef_search less than 0",
+                    XContentFactory.jsonBuilder()
+                        .startObject()
+                        .startObject("query")
+                        .startObject("knn")
+                        .startObject(FIELD_NAME)
+                        .field("vector", new float[] { 1.0f, 2.0f })
+                        .field("k", 1)
+                        .startObject("method_parameter")
+                        .field("ef_search", -1)
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                ),
+                $(
+                    "nprobes string",
+                    XContentFactory.jsonBuilder()
+                        .startObject()
+                        .startObject("query")
+                        .startObject("knn")
+                        .startObject(FIELD_NAME)
+                        .field("vector", new float[] { 1.0f, 2.0f })
+                        .field("k", 1)
+                        .startObject("method_parameter")
+                        .field("nprobes", "string value")
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                ),
+                $(
+                    "nprobes less than 0",
+                    XContentFactory.jsonBuilder()
+                        .startObject()
+                        .startObject("query")
+                        .startObject("knn")
+                        .startObject(FIELD_NAME)
+                        .field("vector", new float[] { 1.0f, 2.0f })
+                        .field("k", 1)
+                        .startObject("method_parameter")
+                        .field("nprobes", -10)
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                )
+            )
+        );
+    }
+
+    @SneakyThrows
+    public void testEndToEnd_whenMethodIsHNSWFlat_thenSucceed() {
+        Request request = new Request("POST", "/dummy_index/_search");
+        request.setJsonEntity(xContentBuilder.toString());
+
+        request.addParameter("size", Integer.toString(10));
+        request.addParameter("explain", Boolean.toString(true));
+        request.addParameter("search_type", "query_then_fetch");
+
+        expectThrows(ResponseException.class, () -> client().performRequest(request));
+    }
+}
diff --git a/src/test/java/org/opensearch/knn/index/query/parser/MethodParametersParserTests.java b/src/test/java/org/opensearch/knn/index/query/parser/MethodParametersParserTests.java
@@ -35,6 +35,9 @@ public void testValidateMethodParameters() {
 
         ValidationException validationException3 = validateMethodParameters(Map.of("ef_search", 10));
         assertNull(validationException3);
+
+        ValidationException validationException4 = validateMethodParameters(Map.of("nprobes", 0));
+        assertTrue(validationException4.getMessage().contains("Validation Failed: 1: nprobes should be greater than 0"));
     }
 
     @SneakyThrows
@@ -80,5 +83,15 @@ public void testFromXContent() {
         builder = XContentFactory.jsonBuilder().startObject().endObject();
         XContentParser parser4 = createParser(builder);
         expectThrows(ParsingException.class, () -> MethodParametersParser.fromXContent(parser4));
+
+        // nprobes string
+        builder = XContentFactory.jsonBuilder().startObject().field("nprobes", "string").endObject();
+        XContentParser parser5 = createParser(builder);
+        expectThrows(ParsingException.class, () -> MethodParametersParser.fromXContent(parser5));
+
+        // nprobes Valid
+        builder = XContentFactory.jsonBuilder().startObject().field("nprobes", 10).endObject();
+        XContentParser parser6 = createParser(builder);
+        assertEquals(Map.of("nprobes", 10), MethodParametersParser.fromXContent(parser6));
     }
 }