Remove type from validate query API (opensearch-project#2255)

dreamer-89 · web-flow · commit 5b0da85df857 · 2022-02-25T10:18:41.000-08:00
* Remove type mapping from RestValidateAction

Signed-off-by: Suraj Singh &lt;surajrider@gmail.com&gt;

* Spotless check apply

Signed-off-by: Suraj Singh &lt;surajrider@gmail.com&gt;

* Include suggested review comment

Signed-off-by: Suraj Singh &lt;surajrider@gmail.com&gt;
diff --git a/client/rest-high-level/src/main/java/org/opensearch/client/IndicesRequestConverters.java b/client/rest-high-level/src/main/java/org/opensearch/client/IndicesRequestConverters.java
@@ -669,8 +669,7 @@ static Request simulateIndexTemplate(SimulateIndexTemplateRequest simulateIndexT
 
     static Request validateQuery(ValidateQueryRequest validateQueryRequest) throws IOException {
         String[] indices = validateQueryRequest.indices() == null ? Strings.EMPTY_ARRAY : validateQueryRequest.indices();
-        String[] types = validateQueryRequest.types() == null || indices.length <= 0 ? Strings.EMPTY_ARRAY : validateQueryRequest.types();
-        String endpoint = RequestConverters.endpoint(indices, types, "_validate/query");
+        String endpoint = RequestConverters.endpoint(indices, "_validate/query");
         Request request = new Request(HttpGet.METHOD_NAME, endpoint);
         RequestConverters.Params params = new RequestConverters.Params();
         params.withIndicesOptions(validateQueryRequest.indicesOptions());
diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/IndicesRequestConvertersTests.java b/client/rest-high-level/src/test/java/org/opensearch/client/IndicesRequestConvertersTests.java
@@ -1198,15 +1198,13 @@ public void testPutTemplateRequest() throws Exception {
 
     public void testValidateQuery() throws Exception {
         String[] indices = OpenSearchTestCase.randomBoolean() ? null : RequestConvertersTests.randomIndicesNames(0, 5);
-        String[] types = OpenSearchTestCase.randomBoolean() ? OpenSearchTestCase.generateRandomStringArray(5, 5, false, false) : null;
         ValidateQueryRequest validateQueryRequest;
         if (OpenSearchTestCase.randomBoolean()) {
             validateQueryRequest = new ValidateQueryRequest(indices);
         } else {
             validateQueryRequest = new ValidateQueryRequest();
             validateQueryRequest.indices(indices);
         }
-        validateQueryRequest.types(types);
         Map<String, String> expectedParams = new HashMap<>();
         RequestConvertersTests.setRandomIndicesOptions(
             validateQueryRequest::indicesOptions,
@@ -1223,9 +1221,6 @@ public void testValidateQuery() throws Exception {
         StringJoiner endpoint = new StringJoiner("/", "/", "");
         if (indices != null && indices.length > 0) {
             endpoint.add(String.join(",", indices));
-            if (types != null && types.length > 0) {
-                endpoint.add(String.join(",", types));
-            }
         }
         endpoint.add("_validate/query");
         Assert.assertThat(request.getEndpoint(), equalTo(endpoint.toString()));
diff --git a/server/src/internalClusterTest/java/org/opensearch/validate/SimpleValidateQueryIT.java b/server/src/internalClusterTest/java/org/opensearch/validate/SimpleValidateQueryIT.java
@@ -447,7 +447,6 @@ private static void assertExplanation(QueryBuilder queryBuilder, Matcher<String>
         ValidateQueryResponse response = client().admin()
             .indices()
             .prepareValidateQuery("test")
-            .setTypes("type1")
             .setQuery(queryBuilder)
             .setExplain(true)
             .setRewrite(withRewrite)
@@ -468,7 +467,6 @@ private static void assertExplanations(
         ValidateQueryResponse response = client().admin()
             .indices()
             .prepareValidateQuery("test")
-            .setTypes("type1")
             .setQuery(queryBuilder)
             .setExplain(true)
             .setRewrite(withRewrite)
@@ -497,7 +495,6 @@ public void testExplainTermsQueryWithLookup() throws Exception {
         ValidateQueryResponse response = client().admin()
             .indices()
             .prepareValidateQuery("twitter")
-            .setTypes("_doc")
             .setQuery(termsLookupQuery)
             .setExplain(true)
             .execute()
diff --git a/server/src/main/java/org/opensearch/action/admin/indices/validate/query/ShardValidateQueryRequest.java b/server/src/main/java/org/opensearch/action/admin/indices/validate/query/ShardValidateQueryRequest.java
@@ -32,8 +32,8 @@
 
 package org.opensearch.action.admin.indices.validate.query;
 
+import org.opensearch.Version;
 import org.opensearch.action.support.broadcast.BroadcastShardRequest;
-import org.opensearch.common.Strings;
 import org.opensearch.common.io.stream.StreamInput;
 import org.opensearch.common.io.stream.StreamOutput;
 import org.opensearch.index.query.QueryBuilder;
@@ -49,7 +49,6 @@
 public class ShardValidateQueryRequest extends BroadcastShardRequest {
 
     private QueryBuilder query;
-    private String[] types = Strings.EMPTY_ARRAY;
     private boolean explain;
     private boolean rewrite;
     private long nowInMillis;
@@ -58,12 +57,12 @@ public class ShardValidateQueryRequest extends BroadcastShardRequest {
     public ShardValidateQueryRequest(StreamInput in) throws IOException {
         super(in);
         query = in.readNamedWriteable(QueryBuilder.class);
-
-        int typesSize = in.readVInt();
-        if (typesSize > 0) {
-            types = new String[typesSize];
-            for (int i = 0; i < typesSize; i++) {
-                types[i] = in.readString();
+        if (in.getVersion().before(Version.V_2_0_0)) {
+            int typesSize = in.readVInt();
+            if (typesSize > 0) {
+                for (int i = 0; i < typesSize; i++) {
+                    in.readString();
+                }
             }
         }
         filteringAliases = new AliasFilter(in);
@@ -75,7 +74,6 @@ public ShardValidateQueryRequest(StreamInput in) throws IOException {
     public ShardValidateQueryRequest(ShardId shardId, AliasFilter filteringAliases, ValidateQueryRequest request) {
         super(shardId, request);
         this.query = request.query();
-        this.types = request.types();
         this.explain = request.explain();
         this.rewrite = request.rewrite();
         this.filteringAliases = Objects.requireNonNull(filteringAliases, "filteringAliases must not be null");
@@ -86,10 +84,6 @@ public QueryBuilder query() {
         return query;
     }
 
-    public String[] types() {
-        return this.types;
-    }
-
     public boolean explain() {
         return this.explain;
     }
@@ -110,9 +104,8 @@ public long nowInMillis() {
     public void writeTo(StreamOutput out) throws IOException {
         super.writeTo(out);
         out.writeNamedWriteable(query);
-        out.writeVInt(types.length);
-        for (String type : types) {
-            out.writeString(type);
+        if (out.getVersion().before(Version.V_2_0_0)) {
+            out.writeVInt(0); // no types to filter
         }
         filteringAliases.writeTo(out);
         out.writeBoolean(explain);
diff --git a/server/src/main/java/org/opensearch/action/admin/indices/validate/query/ValidateQueryRequest.java b/server/src/main/java/org/opensearch/action/admin/indices/validate/query/ValidateQueryRequest.java
@@ -32,6 +32,7 @@
 
 package org.opensearch.action.admin.indices.validate.query;
 
+import org.opensearch.Version;
 import org.opensearch.action.ActionRequestValidationException;
 import org.opensearch.action.ValidateActions;
 import org.opensearch.action.support.IndicesOptions;
@@ -60,8 +61,6 @@ public class ValidateQueryRequest extends BroadcastRequest<ValidateQueryRequest>
     private boolean rewrite;
     private boolean allShards;
 
-    private String[] types = Strings.EMPTY_ARRAY;
-
     long nowInMillis;
 
     public ValidateQueryRequest() {
@@ -71,11 +70,12 @@ public ValidateQueryRequest() {
     public ValidateQueryRequest(StreamInput in) throws IOException {
         super(in);
         query = in.readNamedWriteable(QueryBuilder.class);
-        int typesSize = in.readVInt();
-        if (typesSize > 0) {
-            types = new String[typesSize];
-            for (int i = 0; i < typesSize; i++) {
-                types[i] = in.readString();
+        if (in.getVersion().before(Version.V_2_0_0)) {
+            int typesSize = in.readVInt();
+            if (typesSize > 0) {
+                for (int i = 0; i < typesSize; i++) {
+                    in.readString();
+                }
             }
         }
         explain = in.readBoolean();
@@ -113,29 +113,6 @@ public ValidateQueryRequest query(QueryBuilder query) {
         return this;
     }
 
-    /**
-     * The types of documents the query will run against. Defaults to all types.
-     *
-     * @deprecated Types are in the process of being removed. Instead of using a type, prefer to
-     * filter on a field on the document.
-     */
-    @Deprecated
-    public String[] types() {
-        return this.types;
-    }
-
-    /**
-     * The types of documents the query will run against. Defaults to all types.
-     *
-     * @deprecated Types are in the process of being removed. Instead of using a type, prefer to
-     * filter on a field on the document.
-     */
-    @Deprecated
-    public ValidateQueryRequest types(String... types) {
-        this.types = types;
-        return this;
-    }
-
     /**
      * Indicate if detailed information about query is requested
      */
@@ -182,9 +159,8 @@ public boolean allShards() {
     public void writeTo(StreamOutput out) throws IOException {
         super.writeTo(out);
         out.writeNamedWriteable(query);
-        out.writeVInt(types.length);
-        for (String type : types) {
-            out.writeString(type);
+        if (out.getVersion().before(Version.V_2_0_0)) {
+            out.writeVInt(0);   // no types to filter
         }
         out.writeBoolean(explain);
         out.writeBoolean(rewrite);
@@ -196,8 +172,7 @@ public String toString() {
         return "["
             + Arrays.toString(indices)
             + "]"
-            + Arrays.toString(types)
-            + ", query["
+            + " query["
             + query
             + "], explain:"
             + explain
diff --git a/server/src/main/java/org/opensearch/action/admin/indices/validate/query/ValidateQueryRequestBuilder.java b/server/src/main/java/org/opensearch/action/admin/indices/validate/query/ValidateQueryRequestBuilder.java
@@ -45,14 +45,6 @@ public ValidateQueryRequestBuilder(OpenSearchClient client, ValidateQueryAction
         super(client, action, new ValidateQueryRequest());
     }
 
-    /**
-     * The types of documents the query will run against. Defaults to all types.
-     */
-    public ValidateQueryRequestBuilder setTypes(String... types) {
-        request.types(types);
-        return this;
-    }
-
     /**
      * The query to validate.
      *
diff --git a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestValidateQueryAction.java b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestValidateQueryAction.java
@@ -39,7 +39,6 @@
 import org.opensearch.client.node.NodeClient;
 import org.opensearch.common.ParsingException;
 import org.opensearch.common.Strings;
-import org.opensearch.common.logging.DeprecationLogger;
 import org.opensearch.common.xcontent.XContentBuilder;
 import org.opensearch.rest.BaseRestHandler;
 import org.opensearch.rest.BytesRestResponse;
@@ -58,19 +57,14 @@
 import static org.opensearch.rest.RestStatus.OK;
 
 public class RestValidateQueryAction extends BaseRestHandler {
-    private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestValidateQueryAction.class);
-    static final String TYPES_DEPRECATION_MESSAGE = "[types removal]" + " Specifying types in validate query requests is deprecated.";
-
     @Override
     public List<Route> routes() {
         return unmodifiableList(
             asList(
                 new Route(GET, "/_validate/query"),
                 new Route(POST, "/_validate/query"),
                 new Route(GET, "/{index}/_validate/query"),
-                new Route(POST, "/{index}/_validate/query"),
-                new Route(GET, "/{index}/{type}/_validate/query"),
-                new Route(POST, "/{index}/{type}/_validate/query")
+                new Route(POST, "/{index}/_validate/query")
             )
         );
     }
@@ -86,11 +80,6 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
         validateQueryRequest.indicesOptions(IndicesOptions.fromRequest(request, validateQueryRequest.indicesOptions()));
         validateQueryRequest.explain(request.paramAsBoolean("explain", false));
 
-        if (request.hasParam("type")) {
-            deprecationLogger.deprecate("validate_query_with_types", TYPES_DEPRECATION_MESSAGE);
-            validateQueryRequest.types(Strings.splitStringByCommaToArray(request.param("type")));
-        }
-
         validateQueryRequest.rewrite(request.paramAsBoolean("rewrite", false));
         validateQueryRequest.allShards(request.paramAsBoolean("all_shards", false));
 
diff --git a/server/src/test/java/org/opensearch/action/ShardValidateQueryRequestTests.java b/server/src/test/java/org/opensearch/action/ShardValidateQueryRequestTests.java
@@ -69,17 +69,15 @@ public void testSerialize() throws IOException {
             validateQueryRequest.query(QueryBuilders.termQuery("field", "value"));
             validateQueryRequest.rewrite(true);
             validateQueryRequest.explain(false);
-            validateQueryRequest.types("type1", "type2");
             ShardValidateQueryRequest request = new ShardValidateQueryRequest(
                 new ShardId("index", "foobar", 1),
-                new AliasFilter(QueryBuilders.termQuery("filter_field", "value"), new String[] { "alias0", "alias1" }),
+                new AliasFilter(QueryBuilders.termQuery("filter_field", "value"), "alias0", "alias1"),
                 validateQueryRequest
             );
             request.writeTo(output);
             try (StreamInput in = new NamedWriteableAwareStreamInput(output.bytes().streamInput(), namedWriteableRegistry)) {
                 ShardValidateQueryRequest readRequest = new ShardValidateQueryRequest(in);
                 assertEquals(request.filteringAliases(), readRequest.filteringAliases());
-                assertArrayEquals(request.types(), readRequest.types());
                 assertEquals(request.explain(), readRequest.explain());
                 assertEquals(request.query(), readRequest.query());
                 assertEquals(request.rewrite(), readRequest.rewrite());

Original file line number	Diff line number	Diff line change
`@@ -45,14 +45,6 @@ public ValidateQueryRequestBuilder(OpenSearchClient client, ValidateQueryAction`
`45`	`45`	`super(client, action, new ValidateQueryRequest());`
`46`	`46`	`}`
`47`	`47`
`48`		`- /**`
`49`		`- * The types of documents the query will run against. Defaults to all types.`
`50`		`- */`
`51`		`- public ValidateQueryRequestBuilder setTypes(String... types) {`
`52`		`- request.types(types);`
`53`		`- return this;`
`54`		`- }`
`55`		`-`
`56`	`48`	`/**`
`57`	`49`	`* The query to validate.`
`58`	`50`	`*`