[Rest Api Compatibility] Typed endpoints for RestUpdateAction and RestDeleteAction

rest-api-spec/build.gradle

@@ -36,17 +36,6 @@ tasks.named("yamlRestCompatTest").configure {
   }
 
   systemProperty 'tests.rest.blacklist', [
-    'bulk/11_basic_with_types/Array of objects',
-    'bulk/11_basic_with_types/Empty _id',
-    'bulk/11_basic_with_types/Empty _id with op_type create',
-    'bulk/11_basic_with_types/empty action',
-    'bulk/21_list_of_strings_with_types/List of strings',
-    'bulk/31_big_string_with_types/One big string',
-    'bulk/41_source_with_types/Source filtering',
-    'bulk/51_refresh_with_types/refresh=empty string immediately makes changes are visible in search',
-    'bulk/51_refresh_with_types/refresh=true immediately makes changes are visible in search',
-    'bulk/51_refresh_with_types/refresh=wait_for waits until changes are visible in search',
-    'bulk/81_cas_with_types/Compare And Swap Sequence Numbers',
     'cat.aliases/10_basic/Alias against closed index',
     'cat.aliases/10_basic/Alias name',
     'cat.aliases/10_basic/Alias sorting',
@@ -105,24 +94,48 @@ tasks.named("yamlRestCompatTest").configure {
     'cat.templates/10_basic/Normal templates',
     'cat.templates/10_basic/Select columns',
     'cat.thread_pool/10_basic/Test cat thread_pool output',
+    // type information about the type is removed and not passed down. The logic to check for this is also removed.
+    'delete/70_mix_typeless_typeful/DELETE with typeless API on an index that has types',
+    // WILL NOT BE FIXED - failing due to not recognising missing type (the type path param is ignored)
+    'get/100_mix_typeless_typeful/GET with typeless API on an index that has types',
+    // type information about the type is removed and not passed down. The logic to check for this is also removed.
+    'indices.create/20_mix_typeless_typeful/Implicitly create a typed index while there is a typeless template',
+    'indices.create/20_mix_typeless_typeful/Implicitly create a typeless index while there is a typed template',
+    //
+    // This test returns test_index.mappings:{} when {} was expected. difference between 20_missing_field and 21_missing_field_with_types?
+    'indices.get_field_mapping/21_missing_field_with_types/Return empty object if field doesn\'t exist, but type and index do',
+    // The information about the type is not present in the index. hence it cannot know if the type exist or not.
+    'indices.get_field_mapping/30_missing_type/Raise 404 when type doesn\'t exist',
+    // The information about the type is not present in the index. hence it cannot know if the type exist or not.
+    'indices.get_mapping/20_missing_type/Existent and non-existent type returns 404 and the existing type',
+    'indices.get_mapping/20_missing_type/Existent and non-existent types returns 404 and the existing type',
+    'indices.get_mapping/20_missing_type/No type matching pattern returns 404',
+    'indices.get_mapping/20_missing_type/Non-existent type returns 404',
+    'indices.get_mapping/20_missing_type/Type missing when no types exist',
+    //
+    // The information about the type is not present in the index. hence it cannot know if the type was already used or not
+    'indices.put_mapping/20_mix_typeless_typeful/PUT mapping with _doc on an index that has types',
+    'indices.put_mapping/20_mix_typeless_typeful/PUT mapping with typeless API on an index that has types',
+    // there is a small distinction between empty mappings and no mappings at all. The code to implement this test was refactored #54003
+    // field search on _type field- not implementing. The data for _type is considered incorrect in this search
+    'search/160_exists_query/Test exists query on _type field',
+    // 83 - 12 = 71 tests won't be fixed
+    'bulk/11_basic_with_types/Array of objects',
+    'bulk/11_basic_with_types/Empty _id',
+    'bulk/11_basic_with_types/Empty _id with op_type create',
+    'bulk/11_basic_with_types/empty action',
+    'bulk/21_list_of_strings_with_types/List of strings',
+    'bulk/31_big_string_with_types/One big string',
+    'bulk/41_source_with_types/Source filtering',
+    'bulk/51_refresh_with_types/refresh=empty string immediately makes changes are visible in search',
+    'bulk/51_refresh_with_types/refresh=true immediately makes changes are visible in search',
+    'bulk/51_refresh_with_types/refresh=wait_for waits until changes are visible in search',
+    'bulk/81_cas_with_types/Compare And Swap Sequence Numbers',
     'cluster.voting_config_exclusions/10_basic/Throw exception when adding voting config exclusion and specifying both node_ids and node_names',
     'cluster.voting_config_exclusions/10_basic/Throw exception when adding voting config exclusion without specifying nodes',
     'count/11_basic_with_types/count body without query element',
     'count/11_basic_with_types/count with body',
     'count/11_basic_with_types/count with empty body',
-    'delete/13_basic_with_types/Basic',
-    'delete/14_shard_header_with_types/Delete check shard header',
-    'delete/15_result_with_types/Delete result field',
-    'delete/21_cas_with_types/Internal version',
-    'delete/27_external_version_with_types/External version',
-    'delete/28_external_gte_version_with_types/External GTE version',
-    'delete/31_routing_with_types/Routing',
-    'delete/51_refresh_with_types/Refresh',
-    'delete/51_refresh_with_types/When refresh url parameter is an empty string that means "refresh immediately"',
-    'delete/51_refresh_with_types/refresh=wait_for waits until changes are visible in search',
-    'delete/61_missing_with_types/Missing document with catch',
-    'delete/61_missing_with_types/Missing document with ignore',
-    'delete/70_mix_typeless_typeful/DELETE with typeless API on an index that has types',
     'explain/10_basic/Basic explain',
     'explain/10_basic/Basic explain with alias',
     'explain/11_basic_with_types/Basic explain',
@@ -132,8 +145,6 @@ tasks.named("yamlRestCompatTest").configure {
     'explain/31_query_string_with_types/explain with query_string parameters',
     'explain/40_mix_typeless_typeful/Explain with typeless API on an index that has types',
     'field_caps/30_filter/Field caps with index filter',
-    // WILL NOT BE FIXED - failing due to not recognising missing type (the type path param is ignored)
-    'get/100_mix_typeless_typeful/GET with typeless API on an index that has types',
     'get_source/11_basic_with_types/Basic with types',
     'get_source/16_default_values_with_types/Default values',
     'get_source/41_routing_with_types/Routing',
@@ -144,30 +155,10 @@ tasks.named("yamlRestCompatTest").configure {
     'get_source/86_source_missing_with_types/Missing document source with catch',
     'get_source/86_source_missing_with_types/Missing document source with ignore',
     'indices.create/10_basic/Create index without soft deletes',
-    // type information about the type is removed and not passed down. The logic to check for this is also removed.
-    'indices.create/20_mix_typeless_typeful/Implicitly create a typed index while there is a typeless template',
-    'indices.create/20_mix_typeless_typeful/Implicitly create a typeless index while there is a typed template',
-    //
     'indices.flush/10_basic/Index synced flush rest test',
     'indices.forcemerge/10_basic/Check deprecation warning when incompatible only_expunge_deletes and max_num_segments values are both set',
-    // This test returns test_index.mappings:{} when {} was expected. difference between 20_missing_field and 21_missing_field_with_types?
-    'indices.get_field_mapping/21_missing_field_with_types/Return empty object if field doesn\'t exist, but type and index do',
-    // The information about the type is not present in the index. hence it cannot know if the type exist or not.
-    'indices.get_field_mapping/30_missing_type/Raise 404 when type doesn\'t exist',
-    // The information about the type is not present in the index. hence it cannot know if the type exist or not.
-    'indices.get_mapping/20_missing_type/Existent and non-existent type returns 404 and the existing type',
-    'indices.get_mapping/20_missing_type/Existent and non-existent types returns 404 and the existing type',
-    'indices.get_mapping/20_missing_type/No type matching pattern returns 404',
-    'indices.get_mapping/20_missing_type/Non-existent type returns 404',
-    'indices.get_mapping/20_missing_type/Type missing when no types exist',
-    //
     'indices.open/10_basic/?wait_for_active_shards default is deprecated',
     'indices.open/10_basic/?wait_for_active_shards=index-setting',
-
-    // The information about the type is not present in the index. hence it cannot know if the type was already used or not
-    'indices.put_mapping/20_mix_typeless_typeful/PUT mapping with _doc on an index that has types',
-    'indices.put_mapping/20_mix_typeless_typeful/PUT mapping with typeless API on an index that has types',
-    // there is a small distinction between empty mappings and no mappings at all. The code to implement this test was refactored #54003
     // not fixing this in #70966
     'indices.put_template/11_basic_with_types/Put template with empty mappings',
     'indices.shrink/30_copy_settings/Copy settings during shrink index',
@@ -212,17 +203,13 @@ tasks.named("yamlRestCompatTest").configure {
     'mtermvectors/11_basic_with_types/Basic tests for multi termvector get',
     'mtermvectors/21_deprecated_with_types/Deprecated camel case and _ parameters should fail in Term Vectors query',
     'mtermvectors/30_mix_typeless_typeful/mtermvectors without types on an index that has types',
-    // field search on _type field- not implementing. The data for _type is considered incorrect in this search
-    'search/160_exists_query/Test exists query on _type field',
-    //
     'search/10_source_filtering/docvalue_fields with default format', //use_field_mapping change
     'search/40_indices_boost/Indices boost using object', //indices_boost
     'search/150_rewrite_on_coordinator/Ensure that we fetch the document only once', //terms_lookup
     'search/171_terms_query_with_types/Terms Query with No.of terms exceeding index.max_terms_count should FAIL', //bulk
     'search/260_parameter_validation/test size=-1 is deprecated', //size=-1 change
     'search/310_match_bool_prefix/multi_match multiple fields with cutoff_frequency throws exception', //cutoff_frequency
     'search/340_type_query/type query', // type_query - probably should behave like match_all
-
     'search_shards/10_basic/Search shards aliases with and without filters',
     'snapshot.get/10_basic/Get missing snapshot info succeeds when ignore_unavailable is true',
     'snapshot.get/10_basic/Get missing snapshot info throws an exception',
@@ -236,17 +223,7 @@ tasks.named("yamlRestCompatTest").configure {
     'termvectors/20_issue7121/Term vector API should return \'found: false\' for docs between index and refresh',
     'termvectors/21_issue7121_with_types/Term vector API should return \'found: false\' for docs between index and refresh',
     'termvectors/31_realtime_with_types/Realtime Term Vectors',
-    'termvectors/50_mix_typeless_typeful/Term vectors with typeless API on an index that has types',
-    'update/14_shard_header_with_types/Update check shard header',
-    'update/15_result_with_types/Update result field',
-    'update/16_noop/Noop',
-    'update/21_doc_upsert_with_types/Doc upsert',
-    'update/24_doc_as_upsert_with_types/Doc as upsert',
-    'update/41_routing_with_types/Routing',
-    'update/61_refresh_with_types/Refresh',
-    'update/61_refresh_with_types/When refresh url parameter is an empty string that means "refresh immediately"',
-    'update/61_refresh_with_types/refresh=wait_for waits until changes are visible in search',
-    'update/81_source_filtering_with_types/Source filtering',
+    'termvectors/50_mix_typeless_typeful/Term vectors with typeless API on an index that has types'
   ].join(',')
 }
 

server/src/main/java/org/elasticsearch/rest/action/document/RestDeleteAction.java

@@ -11,6 +11,7 @@
 import org.elasticsearch.action.delete.DeleteRequest;
 import org.elasticsearch.action.support.ActiveShardCount;
 import org.elasticsearch.client.node.NodeClient;
+import org.elasticsearch.common.RestApiVersion;
 import org.elasticsearch.index.VersionType;
 import org.elasticsearch.rest.BaseRestHandler;
 import org.elasticsearch.rest.RestRequest;
@@ -23,10 +24,16 @@
 import static org.elasticsearch.rest.RestRequest.Method.DELETE;
 
 public class RestDeleteAction extends BaseRestHandler {
+    public static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Specifying types in "
+        + "document index requests is deprecated, use the /{index}/_doc/{id} endpoint instead.";
 
     @Override
     public List<Route> routes() {
-        return List.of(new Route(DELETE, "/{index}/_doc/{id}"));
+        return List.of(
+            new Route(DELETE, "/{index}/_doc/{id}"),
+            Route.builder(DELETE, "/{index}/{type}/{id}")
+                .deprecated(TYPES_DEPRECATION_MESSAGE, RestApiVersion.V_7)
+                .build());
     }
 
     @Override
@@ -36,6 +43,9 @@ public String getName() {
 
     @Override
     public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
+        if (request.getRestApiVersion() == RestApiVersion.V_7 && request.hasParam("type")) {
+            request.param("type");
+        }
         DeleteRequest deleteRequest = new DeleteRequest(request.param("index"), request.param("id"));
         deleteRequest.routing(request.param("routing"));
         deleteRequest.timeout(request.paramAsTime("timeout", DeleteRequest.DEFAULT_TIMEOUT));

server/src/main/java/org/elasticsearch/rest/action/document/RestUpdateAction.java

@@ -14,6 +14,7 @@
 import org.elasticsearch.action.support.ActiveShardCount;
 import org.elasticsearch.action.update.UpdateRequest;
 import org.elasticsearch.client.node.NodeClient;
+import org.elasticsearch.common.RestApiVersion;
 import org.elasticsearch.index.VersionType;
 import org.elasticsearch.rest.BaseRestHandler;
 import org.elasticsearch.rest.RestRequest;
@@ -27,10 +28,17 @@
 import static org.elasticsearch.rest.RestRequest.Method.POST;
 
 public class RestUpdateAction extends BaseRestHandler {
+    public static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Specifying types in "
+        + "document update requests is deprecated, use the endpoint /{index}/_update/{id} instead.";
 
     @Override
     public List<Route> routes() {
-        return List.of(new Route(POST, "/{index}/_update/{id}"));
+        return List.of(
+            new Route(POST, "/{index}/_update/{id}"),
+            Route.builder(POST, "/{index}/{type}/{id}/_update")
+                .deprecated(TYPES_DEPRECATION_MESSAGE, RestApiVersion.V_7)
+                .build()
+            );
     }
 
     @Override
@@ -40,6 +48,9 @@ public String getName() {
 
     @Override
     public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
+        if (request.getRestApiVersion() == RestApiVersion.V_7 && request.hasParam("type")) {
+            request.param("type");
+        }
         UpdateRequest updateRequest = new UpdateRequest(request.param("index"), request.param("id"));
         updateRequest.routing(request.param("routing"));
         updateRequest.timeout(request.paramAsTime("timeout", updateRequest.timeout()));

server/src/test/java/org/elasticsearch/rest/action/document/RestDeleteActionTests.java

@@ -0,0 +1,50 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.rest.action.document;
+
+import org.elasticsearch.action.delete.DeleteResponse;
+import org.elasticsearch.common.RestApiVersion;
+import org.elasticsearch.rest.RestRequest;
+import org.elasticsearch.test.rest.FakeRestRequest;
+import org.elasticsearch.test.rest.RestActionTestCase;
+import org.junit.Before;
+import org.mockito.Mockito;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+public class RestDeleteActionTests extends RestActionTestCase {
+
+    final List<String> contentTypeHeader = Collections.singletonList(randomCompatibleMediaType(RestApiVersion.V_7));
+
+    @Before
+    public void setUpAction() {
+        controller().registerHandler(new RestDeleteAction());
+        verifyingClient.setExecuteVerifier((actionType, request) -> Mockito.mock(DeleteResponse.class));
+        verifyingClient.setExecuteLocallyVerifier((actionType, request) -> Mockito.mock(DeleteResponse.class));
+    }
+
+    public void testTypeInPath() {
+        RestRequest request = new FakeRestRequest.Builder(xContentRegistry())
+            .withHeaders(Map.of("Accept", contentTypeHeader))
+            .withMethod(RestRequest.Method.DELETE)
+            .withPath("/some_index/some_type/some_id")
+            .build();
+        dispatchRequest(request);
+        assertWarnings(RestDeleteAction.TYPES_DEPRECATION_MESSAGE);
+
+        RestRequest validRequest = new FakeRestRequest.Builder(xContentRegistry())
+            .withHeaders(Map.of("Accept", contentTypeHeader))
+            .withMethod(RestRequest.Method.DELETE)
+            .withPath("/some_index/_doc/some_id")
+            .build();
+        dispatchRequest(validRequest);
+    }
+}

server/src/test/java/org/elasticsearch/rest/action/document/RestMultiTermVectorsActionTests.java

@@ -32,7 +32,6 @@ public class RestMultiTermVectorsActionTests extends RestActionTestCase {
     @Before
     public void setUpAction() {
         controller().registerHandler(new RestMultiTermVectorsAction());
-        //TODO clarify why we need to set these? any workaround?
         verifyingClient.setExecuteVerifier((actionType, request) -> Mockito.mock(MultiTermVectorsResponse.class));
         verifyingClient.setExecuteLocallyVerifier((actionType, request) -> Mockito.mock(MultiTermVectorsResponse.class));
     }