Remove ignored type parameter in search_shards api (#21688)

The `type` parameter has always been accepted by the search_shards api, probably to make the api and its urls the same as search. Truth is that the type never had any effect, it's been ignored from day one while accepting it may make users think that we actually do something with it.

This commit removes support for the type parameter from the REST layer and the Java API. Backwards compatibility is maintained on the transport layer though.

The new added serialization test also uncovered a bug in the java API where the `ClusterSearchShardsRequest` could be created with no arguments, but the indices were required to be not null otherwise the request couldn't be serialized as `writeTo` would throw NPE. Fixed by setting a default value (empty array) for indices.

core/src/main/java/org/elasticsearch/action/admin/cluster/shards/ClusterSearchShardsRequest.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.action.admin.cluster.shards;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.IndicesRequest;
 import org.elasticsearch.action.support.IndicesOptions;
@@ -29,14 +30,15 @@ import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 
 import java.io.IOException;
+import java.util.Objects;
 
 public class ClusterSearchShardsRequest extends MasterNodeReadRequest<ClusterSearchShardsRequest> implements IndicesRequest.Replaceable {
-    private String[] indices;
+    public static final Version V_5_1_0_UNRELEASED = Version.fromId(5010099);
+    private String[] indices = Strings.EMPTY_ARRAY;
     @Nullable
     private String routing;
     @Nullable
     private String preference;
-    private String[] types = Strings.EMPTY_ARRAY;
     private IndicesOptions indicesOptions = IndicesOptions.lenientExpandOpen();
 
 
@@ -57,14 +59,9 @@ public class ClusterSearchShardsRequest extends MasterNodeReadRequest<ClusterSea
      */
     @Override
     public ClusterSearchShardsRequest indices(String... indices) {
-        if (indices == null) {
+        Objects.requireNonNull(indices, "indices must not be null");
-            throw new IllegalArgumentException("indices must not be null");
+        for (int i = 0; i < indices.length; i++) {
-        } else {
+            Objects.requireNonNull(indices[i], "indices[" + i + "] must not be null");
-            for (int i = 0; i < indices.length; i++) {
-                if (indices[i] == null) {
-                    throw new IllegalArgumentException("indices[" + i + "] must not be null");
-                }
-            }
         }
         this.indices = indices;
         return this;
@@ -88,23 +85,6 @@ public class ClusterSearchShardsRequest extends MasterNodeReadRequest<ClusterSea
         return this;
     }
 
-    /**
-     * The document types to execute the search against. Defaults to be executed against
-     * all types.
-     */
-    public String[] types() {
-        return types;
-    }
-
-    /**
-     * The document types to execute the search against. Defaults to be executed against
-     * all types.
-     */
-    public ClusterSearchShardsRequest types(String... types) {
-        this.types = types;
-        return this;
-    }
-
     /**
      * A comma separated list of routing values to control the shards the search will be executed on.
      */
@@ -154,7 +134,10 @@ public class ClusterSearchShardsRequest extends MasterNodeReadRequest<ClusterSea
         routing = in.readOptionalString();
         preference = in.readOptionalString();
 
-        types = in.readStringArray();
+        if (in.getVersion().onOrBefore(V_5_1_0_UNRELEASED)) {
+            //types
+            in.readStringArray();
+        }
         indicesOptions = IndicesOptions.readIndicesOptions(in);
     }
 
@@ -170,7 +153,10 @@ public class ClusterSearchShardsRequest extends MasterNodeReadRequest<ClusterSea
         out.writeOptionalString(routing);
         out.writeOptionalString(preference);
 
-        out.writeStringArray(types);
+        if (out.getVersion().onOrBefore(V_5_1_0_UNRELEASED)) {
+            //types
+            out.writeStringArray(Strings.EMPTY_ARRAY);
+        }
         indicesOptions.writeIndicesOptions(out);
     }
 

core/src/main/java/org/elasticsearch/action/admin/cluster/shards/ClusterSearchShardsRequestBuilder.java

@@ -37,15 +37,6 @@ public class ClusterSearchShardsRequestBuilder extends MasterNodeReadOperationRe
         return this;
     }
 
-    /**
-     * The document types to execute the search against. Defaults to be executed against
-     * all types.
-     */
-    public ClusterSearchShardsRequestBuilder setTypes(String... types) {
-        request.types(types);
-        return this;
-    }
-
     /**
      * A comma separated list of routing values to control the shards the search will be executed on.
      */

core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterSearchShardsAction.java

@@ -45,8 +45,6 @@ public class RestClusterSearchShardsAction extends BaseRestHandler {
         controller.registerHandler(POST, "/_search_shards", this);
         controller.registerHandler(GET, "/{index}/_search_shards", this);
         controller.registerHandler(POST, "/{index}/_search_shards", this);
-        controller.registerHandler(GET, "/{index}/{type}/_search_shards", this);
-        controller.registerHandler(POST, "/{index}/{type}/_search_shards", this);
     }
 
     @Override
@@ -54,12 +52,9 @@ public class RestClusterSearchShardsAction extends BaseRestHandler {
         String[] indices = Strings.splitStringByCommaToArray(request.param("index"));
         final ClusterSearchShardsRequest clusterSearchShardsRequest = Requests.clusterSearchShardsRequest(indices);
         clusterSearchShardsRequest.local(request.paramAsBoolean("local", clusterSearchShardsRequest.local()));
-
-        clusterSearchShardsRequest.types(Strings.splitStringByCommaToArray(request.param("type")));
         clusterSearchShardsRequest.routing(request.param("routing"));
         clusterSearchShardsRequest.preference(request.param("preference"));
         clusterSearchShardsRequest.indicesOptions(IndicesOptions.fromRequest(request, clusterSearchShardsRequest.indicesOptions()));
-
         return channel -> client.admin().cluster().searchShards(clusterSearchShardsRequest, new RestToXContentListener<>(channel));
     }
 }

core/src/test/java/org/elasticsearch/VersionTests.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch;
 
+import org.elasticsearch.action.admin.cluster.shards.ClusterSearchShardsRequest;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.lucene.Lucene;
 import org.elasticsearch.common.settings.Settings;
@@ -289,6 +290,7 @@ public class VersionTests extends ESTestCase {
         // once we released 5.0.0 and it's added to Version.java we need to remove this constant
         assertUnknownVersion(Script.V_5_1_0_UNRELEASED);
         // once we released 5.0.0 and it's added to Version.java we need to remove this constant
+        assertUnknownVersion(ClusterSearchShardsRequest.V_5_1_0_UNRELEASED);
     }
 
     public static void assertUnknownVersion(Version version) {

core/src/test/java/org/elasticsearch/action/admin/cluster/shards/ClusterSearchShardsRequestTests.java

@@ -0,0 +1,80 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.action.admin.cluster.shards;
+
+import org.elasticsearch.Version;
+import org.elasticsearch.action.support.IndicesOptions;
+import org.elasticsearch.common.io.stream.BytesStreamOutput;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.VersionUtils;
+
+public class ClusterSearchShardsRequestTests extends ESTestCase {
+
+    public void testSerialization() throws Exception {
+        ClusterSearchShardsRequest request = new ClusterSearchShardsRequest();
+        if (randomBoolean()) {
+            int numIndices = randomIntBetween(1, 5);
+            String[] indices = new String[numIndices];
+            for (int i = 0; i < numIndices; i++) {
+                indices[i] = randomAsciiOfLengthBetween(3, 10);
+            }
+            request.indices(indices);
+        }
+        if (randomBoolean()) {
+            request.indicesOptions(
+                    IndicesOptions.fromOptions(randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean()));
+        }
+        if (randomBoolean()) {
+            request.preference(randomAsciiOfLengthBetween(3, 10));
+        }
+        if (randomBoolean()) {
+            int numRoutings = randomIntBetween(1, 3);
+            String[] routings = new String[numRoutings];
+            for (int i = 0; i < numRoutings; i++) {
+                routings[i] = randomAsciiOfLengthBetween(3, 10);
+            }
+            request.routing(routings);
+        }
+
+        Version version = VersionUtils.randomVersion(random());
+        try (BytesStreamOutput out = new BytesStreamOutput()) {
+            out.setVersion(version);
+            request.writeTo(out);
+            try (StreamInput in = out.bytes().streamInput()) {
+                in.setVersion(version);
+                ClusterSearchShardsRequest deserialized = new ClusterSearchShardsRequest();
+                deserialized.readFrom(in);
+                assertArrayEquals(request.indices(), deserialized.indices());
+                assertSame(request.indicesOptions(), deserialized.indicesOptions());
+                assertEquals(request.routing(), deserialized.routing());
+                assertEquals(request.preference(), deserialized.preference());
+            }
+        }
+    }
+
+    public void testIndicesMustNotBeNull() {
+        ClusterSearchShardsRequest request = new ClusterSearchShardsRequest();
+        assertNotNull(request.indices());
+        expectThrows(NullPointerException.class, () -> request.indices((String[])null));
+        expectThrows(NullPointerException.class, () -> request.indices((String)null));
+        expectThrows(NullPointerException.class, () -> request.indices(new String[]{"index1", null, "index3"}));
+    }
+}

docs/reference/migration/migrate_6_0/search.asciidoc

@@ -9,3 +9,9 @@
 * Queries on boolean fields now strictly parse boolean-like values. This means
   only the strings `"true"` and `"false"` will be parsed into their boolean
   counterparts. Other strings will cause an error to be thrown.
+
+
+==== Search shards API
+
+The search shards API no longer accepts the `type` url parameter, which didn't
+have any effect in previous versions.

docs/reference/search/search-shards.asciidoc

@@ -5,7 +5,7 @@ The search shards api returns the indices and shards that a search request would
 be executed against. This can give useful feedback for working out issues or
 planning optimizations with routing and shard preferences.
 
-The `index` and `type` parameters may be single values, or comma-separated.
+The `index` may be a single value, or comma-separated.
 
 [float]
 === Usage

rest-api-spec/src/main/resources/rest-api-spec/api/search_shards.json

@@ -3,16 +3,12 @@
     "documentation": "http://www.elastic.co/guide/en/elasticsearch/reference/master/search-shards.html",
     "methods": ["GET", "POST"],
     "url": {
-      "path": "/{index}/{type}/_search_shards",
+      "path": "/{index}/_search_shards",
-      "paths": ["/_search_shards", "/{index}/_search_shards", "/{index}/{type}/_search_shards"],
+      "paths": ["/_search_shards", "/{index}/_search_shards"],
       "parts": {
         "index": {
           "type" : "list",
           "description" : "A comma-separated list of index names to search; use `_all` or empty string to perform the operation on all indices"
-        },
-        "type": {
-          "type" : "list",
-          "description" : "A comma-separated list of document types to search; leave empty to perform the operation on all types"
         }
       },
       "params": {