From 2e1dc8912e580226554fb9fce34b6bdebb23e7f0 Mon Sep 17 00:00:00 2001 From: Tejas Shah Date: Mon, 1 Jul 2024 13:14:46 -0700 Subject: [PATCH] Adds integration test for invalid method parameters Signed-off-by: Tejas Shah --- .../knn/index/util/DefaultIVFContext.java | 2 +- .../knn/index/query/InvalidSearchQueryIT.java | 139 ++++++++++++++++++ .../parser/MethodParametersParserTests.java | 13 ++ 3 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 src/test/java/org/opensearch/knn/index/query/InvalidSearchQueryIT.java diff --git a/src/main/java/org/opensearch/knn/index/util/DefaultIVFContext.java b/src/main/java/org/opensearch/knn/index/util/DefaultIVFContext.java index 2179bba460..dbccbe9cbc 100644 --- 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> supportedMethodParameters() { + public Map> 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 new file mode 100644 index 0000000000..6c1d486f95 --- /dev/null +++ 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 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 index f2924fb4f7..984c72b893 100644 --- 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)); } }