Skip to content

Deprecate types in document delete requests. #36087

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
234 changes: 117 additions & 117 deletions client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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) {
Expand All @@ -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());
Expand Down
5 changes: 2 additions & 3 deletions docs/java-rest/high-level/document/delete.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
9 changes: 3 additions & 6 deletions docs/reference/docs/delete.asciidoc
Original file line number Diff line number Diff line change
@@ -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]
--------------------------------------------------
Expand Down Expand Up @@ -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]]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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();
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ public DocumentField getField(String name) {
* @deprecated Use {@link GetResponse#getSource()} instead
*/
@Deprecated
@Override
public Iterator<DocumentField> iterator() {
return getResult.iterator();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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"));
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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");
Expand Down