Deprecate types in document delete requests. (#36087)

jtibshirani · web-flow · commit 0e1ddfd825df · 2018-11-30T15:11:29.000-08:00
* Make sure to use _doc as a type name in the CRUD HLRC tests.
* Deprecate types in document delete requests.
diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java
diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/CRUDDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/CRUDDocumentationIT.java
diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/MigrationDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/MigrationDocumentationIT.java
@@ -82,7 +82,7 @@ public void testRequests() throws Exception {
         RestHighLevelClient client = highLevelClient();
         {
             //tag::migration-request-ctor
-            IndexRequest request = new IndexRequest("index", "doc", "id"); // <1>
+            IndexRequest request = new IndexRequest("index", "_doc", "id"); // <1>
             request.source("{\"field\":\"value\"}", XContentType.JSON);
             //end::migration-request-ctor
 
@@ -93,7 +93,7 @@ public void testRequests() throws Exception {
         }
         {
             //tag::migration-request-async-execution
-            DeleteRequest request = new DeleteRequest("index", "doc", "id"); // <1>
+            DeleteRequest request = new DeleteRequest("index", "id"); // <1>
             client.deleteAsync(request, RequestOptions.DEFAULT, new ActionListener<DeleteResponse>() { // <2>
                 @Override
                 public void onResponse(DeleteResponse deleteResponse) {
@@ -106,11 +106,11 @@ public void onFailure(Exception e) {
                 }
             });
             //end::migration-request-async-execution
-            assertBusy(() -> assertFalse(client.exists(new GetRequest("index", "doc", "id"), RequestOptions.DEFAULT)));
+            assertBusy(() -> assertFalse(client.exists(new GetRequest("index", "_doc", "id"), RequestOptions.DEFAULT)));
         }
         {
             //tag::migration-request-sync-execution
-            DeleteRequest request = new DeleteRequest("index", "doc", "id");
+            DeleteRequest request = new DeleteRequest("index", "id");
             DeleteResponse response = client.delete(request, RequestOptions.DEFAULT); // <1>
             //end::migration-request-sync-execution
             assertEquals(RestStatus.NOT_FOUND, response.status());
diff --git a/docs/java-rest/high-level/document/delete.asciidoc b/docs/java-rest/high-level/document/delete.asciidoc
@@ -10,15 +10,14 @@
 [id="{upid}-{api}-request"]
 ==== Delete Request
 
-A +{request}+ has no arguments
+A +{request}+ has two required arguments:
 
 ["source","java",subs="attributes,callouts,macros"]
 --------------------------------------------------
 include-tagged::{doc-tests-file}[{api}-request]
 --------------------------------------------------
 <1> Index
-<2> Type
-<3> Document id
+<2> Document id
 
 ==== Optional arguments
 The following arguments can optionally be provided:
diff --git a/docs/reference/docs/delete.asciidoc b/docs/reference/docs/delete.asciidoc
@@ -1,9 +1,9 @@
 [[docs-delete]]
 == Delete API
 
-The delete API allows to delete a typed JSON document from a specific
+The delete API allows to delete a JSON document from a specific
 index based on its id. The following example deletes the JSON document
-from an index called `twitter`, under a type called `_doc`, with id `1`:
+from an index called `twitter` with ID `1`:
 
 [source,js]
 --------------------------------------------------
@@ -92,10 +92,7 @@ the request.
 If an <<docs-index_,external versioning variant>> is used,
 the delete operation automatically creates an index if it has not been
 created before (check out the <<indices-create-index,create index API>>
-for manually creating an index), and also automatically creates a
-dynamic type mapping for the specific type if it has not been created
-before (check out the <<indices-put-mapping,put mapping>>
-API for manually creating type mapping).
+for manually creating an index).
 
 [float]
 [[delete-distributed]]
diff --git a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java
@@ -129,13 +129,13 @@ public void testIndexing() throws IOException {
 
         if (CLUSTER_TYPE != ClusterType.OLD) {
             bulk("test_index", "_" + CLUSTER_TYPE, 5);
-            Request toBeDeleted = new Request("PUT", "/test_index/doc/to_be_deleted");
+            Request toBeDeleted = new Request("PUT", "/test_index/_doc/to_be_deleted");
             toBeDeleted.addParameter("refresh", "true");
             toBeDeleted.setJsonEntity("{\"f1\": \"delete-me\"}");
             client().performRequest(toBeDeleted);
             assertCount("test_index", expectedCount + 6);
 
-            Request delete = new Request("DELETE", "/test_index/doc/to_be_deleted");
+            Request delete = new Request("DELETE", "/test_index/_doc/to_be_deleted");
             delete.addParameter("refresh", "true");
             client().performRequest(delete);
 
@@ -146,7 +146,7 @@ public void testIndexing() throws IOException {
     private void bulk(String index, String valueSuffix, int count) throws IOException {
         StringBuilder b = new StringBuilder();
         for (int i = 0; i < count; i++) {
-            b.append("{\"index\": {\"_index\": \"").append(index).append("\", \"_type\": \"doc\"}}\n");
+            b.append("{\"index\": {\"_index\": \"").append(index).append("\", \"_type\": \"_doc\"}}\n");
             b.append("{\"f1\": \"v").append(i).append(valueSuffix).append("\", \"f2\": ").append(i).append("}\n");
         }
         Request bulk = new Request("POST", "/_bulk");
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/delete.json b/rest-api-spec/src/main/resources/rest-api-spec/api/delete.json
@@ -3,7 +3,7 @@
     "documentation": "http://www.elastic.co/guide/en/elasticsearch/reference/master/docs-delete.html",
     "methods": ["DELETE"],
     "url": {
-      "path": "/{index}/{type}/{id}",
+      "path": "/{index}/_doc/{id}",
       "paths": ["/{index}/{type}/{id}", "/{index}/_doc/{id}"],
       "parts": {
         "id": {
diff --git a/server/src/main/java/org/elasticsearch/action/DocWriteResponse.java b/server/src/main/java/org/elasticsearch/action/DocWriteResponse.java
@@ -156,7 +156,10 @@ public ShardId getShardId() {
 
     /**
      * The type of the document changed.
+     *
+     * @deprecated Types are in the process of being removed.
      */
+    @Deprecated
     public String getType() {
         return this.type;
     }
diff --git a/server/src/main/java/org/elasticsearch/action/delete/DeleteRequest.java b/server/src/main/java/org/elasticsearch/action/delete/DeleteRequest.java
@@ -30,6 +30,7 @@
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.lucene.uid.Versions;
 import org.elasticsearch.index.VersionType;
+import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.shard.ShardId;
 
 import java.io.IOException;
@@ -50,7 +51,7 @@
 public class DeleteRequest extends ReplicatedWriteRequest<DeleteRequest>
         implements DocWriteRequest<DeleteRequest>, CompositeIndicesRequest {
 
-    private String type;
+    private String type = MapperService.SINGLE_MAPPING_NAME;
     private String id;
     @Nullable
     private String routing;
@@ -74,13 +75,27 @@ public DeleteRequest(String index) {
      * @param index The index to get the document from
      * @param type  The type of the document
      * @param id    The id of the document
+     *
+     * @deprecated Types are in the process of being removed. Use {@link #DeleteRequest(String, String)} instead.
      */
+    @Deprecated
     public DeleteRequest(String index, String type, String id) {
         this.index = index;
         this.type = type;
         this.id = id;
     }
 
+    /**
+     * Constructs a new delete request against the specified index and id.
+     *
+     * @param index The index to get the document from
+     * @param id    The id of the document
+     */
+    public DeleteRequest(String index, String id) {
+        this.index = index;
+        this.id = id;
+    }
+
     @Override
     public ActionRequestValidationException validate() {
         ActionRequestValidationException validationException = super.validate();
@@ -102,15 +117,21 @@ public ActionRequestValidationException validate() {
 
     /**
      * The type of the document to delete.
+     *
+     * @deprecated Types are in the process of being removed.
      */
+    @Deprecated
     @Override
     public String type() {
         return type;
     }
 
     /**
      * Sets the type of the document to delete.
+     *
+     * @deprecated Types are in the process of being removed.
      */
+    @Deprecated
     @Override
     public DeleteRequest type(String type) {
         this.type = type;
diff --git a/server/src/main/java/org/elasticsearch/action/get/GetResponse.java b/server/src/main/java/org/elasticsearch/action/get/GetResponse.java
@@ -149,7 +149,6 @@ public DocumentField getField(String name) {
      * @deprecated Use {@link GetResponse#getSource()} instead
      */
     @Deprecated
-    @Override
     public Iterator<DocumentField> iterator() {
         return getResult.iterator();
     }
diff --git a/server/src/main/java/org/elasticsearch/rest/action/document/RestDeleteAction.java b/server/src/main/java/org/elasticsearch/rest/action/document/RestDeleteAction.java
@@ -19,11 +19,14 @@
 
 package org.elasticsearch.rest.action.document;
 
+import org.apache.logging.log4j.LogManager;
 import org.elasticsearch.action.delete.DeleteRequest;
 import org.elasticsearch.action.support.ActiveShardCount;
 import org.elasticsearch.client.node.NodeClient;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.VersionType;
+import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.rest.BaseRestHandler;
 import org.elasticsearch.rest.RestController;
 import org.elasticsearch.rest.RestRequest;
@@ -35,6 +38,11 @@
 import static org.elasticsearch.rest.RestRequest.Method.DELETE;
 
 public class RestDeleteAction extends BaseRestHandler {
+    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(
+        LogManager.getLogger(RestDeleteAction.class));
+    static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Specifying types in " +
+        "document index requests is deprecated, use the /{index}/_doc/{id} endpoint instead.";
+
     public RestDeleteAction(Settings settings, RestController controller) {
         super(settings);
         controller.registerHandler(DELETE, "/{index}/{type}/{id}", this);
@@ -47,9 +55,12 @@ public String getName() {
 
     @Override
     public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
-        DeleteRequest deleteRequest = new DeleteRequest(request.param("index"),
-            request.param("type"),
-            request.param("id"));
+        String type = request.param("type");
+        if (!type.equals(MapperService.SINGLE_MAPPING_NAME)) {
+            deprecationLogger.deprecated(TYPES_DEPRECATION_MESSAGE);
+        }
+
+        DeleteRequest deleteRequest = new DeleteRequest(request.param("index"), type, request.param("id"));
         deleteRequest.routing(request.param("routing"));
         deleteRequest.timeout(request.paramAsTime("timeout", DeleteRequest.DEFAULT_TIMEOUT));
         deleteRequest.setRefreshPolicy(request.param("refresh"));
diff --git a/server/src/test/java/org/elasticsearch/rest/action/document/RestDeleteActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/document/RestDeleteActionTests.java
@@ -0,0 +1,71 @@
+/*
+ * 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.rest.action.document;
+
+import org.elasticsearch.client.node.NodeClient;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.util.concurrent.ThreadContext;
+import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
+import org.elasticsearch.rest.RestChannel;
+import org.elasticsearch.rest.RestController;
+import org.elasticsearch.rest.RestRequest;
+import org.elasticsearch.rest.RestRequest.Method;
+import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.rest.FakeRestChannel;
+import org.elasticsearch.test.rest.FakeRestRequest;
+import org.elasticsearch.usage.UsageService;
+
+import java.util.Collections;
+
+import static org.mockito.Mockito.mock;
+
+public class RestDeleteActionTests extends ESTestCase {
+    private RestController controller;
+
+    public void setUp() throws Exception {
+        super.setUp();
+        controller = new RestController(Collections.emptySet(), null,
+            mock(NodeClient.class),
+            new NoneCircuitBreakerService(),
+            new UsageService());
+        new RestDeleteAction(Settings.EMPTY, controller);
+    }
+
+    public void testTypeInPath() {
+        RestRequest deprecatedRequest = new FakeRestRequest.Builder(xContentRegistry())
+            .withMethod(Method.DELETE)
+            .withPath("/some_index/some_type/some_id")
+            .build();
+        performRequest(deprecatedRequest);
+        assertWarnings(RestDeleteAction.TYPES_DEPRECATION_MESSAGE);
+
+        RestRequest validRequest = new FakeRestRequest.Builder(xContentRegistry())
+            .withMethod(Method.DELETE)
+            .withPath("/some_index/_doc/some_id")
+            .build();
+        performRequest(validRequest);
+    }
+
+    private void performRequest(RestRequest request) {
+        RestChannel channel = new FakeRestChannel(request, false, 1);
+        ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
+        controller.dispatchRequest(request, channel, threadContext);
+    }
+}
diff --git a/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java b/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java
@@ -114,13 +114,13 @@ public void testIndexing() throws IOException {
 
         if (CLUSTER_TYPE != ClusterType.OLD) {
             bulk("test_index", "_" + CLUSTER_TYPE, 5);
-            Request toBeDeleted = new Request("PUT", "/test_index/doc/to_be_deleted");
+            Request toBeDeleted = new Request("PUT", "/test_index/_doc/to_be_deleted");
             toBeDeleted.addParameter("refresh", "true");
             toBeDeleted.setJsonEntity("{\"f1\": \"delete-me\"}");
             client().performRequest(toBeDeleted);
             assertCount("test_index", expectedCount + 6);
 
-            Request delete = new Request("DELETE", "/test_index/doc/to_be_deleted");
+            Request delete = new Request("DELETE", "/test_index/_doc/to_be_deleted");
             delete.addParameter("refresh", "true");
             client().performRequest(delete);
 
@@ -131,7 +131,7 @@ public void testIndexing() throws IOException {
     private void bulk(String index, String valueSuffix, int count) throws IOException {
         StringBuilder b = new StringBuilder();
         for (int i = 0; i < count; i++) {
-            b.append("{\"index\": {\"_index\": \"").append(index).append("\", \"_type\": \"doc\"}}\n");
+            b.append("{\"index\": {\"_index\": \"").append(index).append("\", \"_type\": \"_doc\"}}\n");
             b.append("{\"f1\": \"v").append(i).append(valueSuffix).append("\", \"f2\": ").append(i).append("}\n");
         }
         Request bulk = new Request("POST", "/_bulk");

Original file line number	Diff line number	Diff line change
`@@ -82,7 +82,7 @@ public void testRequests() throws Exception {`
`82`	`82`	`RestHighLevelClient client = highLevelClient();`
`83`	`83`	`{`
`84`	`84`	`//tag::migration-request-ctor`
`85`		`- IndexRequest request = new IndexRequest("index", "doc", "id"); // <1>`
	`85`	`+ IndexRequest request = new IndexRequest("index", "_doc", "id"); // <1>`
`86`	`86`	`request.source("{\"field\":\"value\"}", XContentType.JSON);`
`87`	`87`	`//end::migration-request-ctor`
`88`	`88`
`@@ -93,7 +93,7 @@ public void testRequests() throws Exception {`
`93`	`93`	`}`
`94`	`94`	`{`
`95`	`95`	`//tag::migration-request-async-execution`
`96`		`- DeleteRequest request = new DeleteRequest("index", "doc", "id"); // <1>`
	`96`	`+ DeleteRequest request = new DeleteRequest("index", "id"); // <1>`
`97`	`97`	`client.deleteAsync(request, RequestOptions.DEFAULT, new ActionListener<DeleteResponse>() { // <2>`
`98`	`98`	`@Override`
`99`	`99`	`public void onResponse(DeleteResponse deleteResponse) {`
`@@ -106,11 +106,11 @@ public void onFailure(Exception e) {`
`106`	`106`	`}`
`107`	`107`	`});`
`108`	`108`	`//end::migration-request-async-execution`
`109`		`- assertBusy(() -> assertFalse(client.exists(new GetRequest("index", "doc", "id"), RequestOptions.DEFAULT)));`
	`109`	`+ assertBusy(() -> assertFalse(client.exists(new GetRequest("index", "_doc", "id"), RequestOptions.DEFAULT)));`
`110`	`110`	`}`
`111`	`111`	`{`
`112`	`112`	`//tag::migration-request-sync-execution`
`113`		`- DeleteRequest request = new DeleteRequest("index", "doc", "id");`
	`113`	`+ DeleteRequest request = new DeleteRequest("index", "id");`
`114`	`114`	`DeleteResponse response = client.delete(request, RequestOptions.DEFAULT); // <1>`
`115`	`115`	`//end::migration-request-sync-execution`
`116`	`116`	`assertEquals(RestStatus.NOT_FOUND, response.status());`
Original file line number	Diff line number	Diff line change
`@@ -149,7 +149,6 @@ public DocumentField getField(String name) {`
`149`	`149`	`* @deprecated Use {@link GetResponse#getSource()} instead`
`150`	`150`	`*/`
`151`	`151`	`@Deprecated`
`152`		`- @Override`
`153`	`152`	`public Iterator<DocumentField> iterator() {`
`154`	`153`	`return getResult.iterator();`
`155`	`154`	`}`