Deprecate types in get_source and exist_source (#36426)

This change adds a new untyped endpoint `{index}/_source/{id}` for both the
GET and the HEAD methods to get the source of a document or check for its
existance. It also adds deprecation warnings to RestGetSourceAction that emit
a warning when the old deprecated "type" parameter is still used. Also updating
documentation and tests where appropriate.

Relates to #35190
This commit is contained in:
Christoph Büscher 2018-12-18 00:57:42 +01:00 committed by GitHub
parent f0f2b26159
commit 2f5300e3a6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
25 changed files with 393 additions and 44 deletions

View File

@ -268,8 +268,14 @@ final class RequestConverters {
}
static Request sourceExists(GetRequest getRequest) {
Request request = new Request(HttpHead.METHOD_NAME, endpoint(getRequest.index(), getRequest.type(), getRequest.id(), "_source"));
String optionalType = getRequest.type();
String endpoint;
if (optionalType.equals(MapperService.SINGLE_MAPPING_NAME)) {
endpoint = endpoint(getRequest.index(), "_source", getRequest.id());
} else {
endpoint = endpoint(getRequest.index(), optionalType, getRequest.id(), "_source");
}
Request request = new Request(HttpHead.METHOD_NAME, endpoint);
Params parameters = new Params(request);
parameters.withPreference(getRequest.preference());
parameters.withRouting(getRequest.routing());

View File

@ -73,6 +73,7 @@ import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.index.VersionType;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.index.query.TermQueryBuilder;
import org.elasticsearch.index.rankeval.PrecisionAtK;
@ -115,6 +116,7 @@ import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.StringJoiner;
import java.util.function.BiFunction;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Supplier;
@ -156,6 +158,58 @@ public class RequestConvertersTests extends ESTestCase {
getAndExistsWithTypeTest(RequestConverters::get, HttpGet.METHOD_NAME);
}
public void testSourceExists() throws IOException {
doTestSourceExists((index, id) -> new GetRequest(index, id));
}
public void testSourceExistsWithType() throws IOException {
String type = frequently() ? randomAlphaOfLengthBetween(3, 10) : MapperService.SINGLE_MAPPING_NAME;
doTestSourceExists((index, id) -> new GetRequest(index, type, id));
}
private static void doTestSourceExists(BiFunction<String, String, GetRequest> requestFunction) throws IOException {
String index = randomAlphaOfLengthBetween(3, 10);
String id = randomAlphaOfLengthBetween(3, 10);
final GetRequest getRequest = requestFunction.apply(index, id);
Map<String, String> expectedParams = new HashMap<>();
if (randomBoolean()) {
String preference = randomAlphaOfLengthBetween(3, 10);
getRequest.preference(preference);
expectedParams.put("preference", preference);
}
if (randomBoolean()) {
String routing = randomAlphaOfLengthBetween(3, 10);
getRequest.routing(routing);
expectedParams.put("routing", routing);
}
if (randomBoolean()) {
boolean realtime = randomBoolean();
getRequest.realtime(realtime);
if (realtime == false) {
expectedParams.put("realtime", "false");
}
}
if (randomBoolean()) {
boolean refresh = randomBoolean();
getRequest.refresh(refresh);
if (refresh) {
expectedParams.put("refresh", "true");
}
}
Request request = RequestConverters.sourceExists(getRequest);
assertEquals(HttpHead.METHOD_NAME, request.getMethod());
String type = getRequest.type();
if (type.equals(MapperService.SINGLE_MAPPING_NAME)) {
assertEquals("/" + index + "/_source/" + id, request.getEndpoint());
} else {
assertEquals("/" + index + "/" + type + "/" + id + "/_source", request.getEndpoint());
}
assertEquals(expectedParams, request.getParameters());
assertNull(request.getEntity());
}
public void testMultiGet() throws IOException {
Map<String, String> expectedParams = new HashMap<>();
MultiGetRequest multiGetRequest = new MultiGetRequest();

View File

@ -1265,7 +1265,6 @@ public class CRUDDocumentationIT extends ESRestHighLevelClientTestCase {
assertEquals(3, getResponse.getSourceAsMap().size());
//tag::get-response
String index = getResponse.getIndex();
String type = getResponse.getType();
String id = getResponse.getId();
if (getResponse.isExists()) {
long version = getResponse.getVersion();

View File

@ -1,9 +1,9 @@
[[docs-get]]
== Get API
The get API allows to get a typed JSON document from the index based on
The get API allows to get a JSON document from the index based on
its id. The following example gets a JSON document from an index called
twitter, under a type called `_doc`, with id valued 0:
twitter with id valued 0:
[source,js]
--------------------------------------------------
@ -34,7 +34,7 @@ The result of the above get operation is:
--------------------------------------------------
// TESTRESPONSE[s/"_seq_no" : \d+/"_seq_no" : $body._seq_no/ s/"_primary_term" : 1/"_primary_term" : $body._primary_term/]
The above result includes the `_index`, `_type`, `_id` and `_version`
The above result includes the `_index`, `_id` and `_version`
of the document we wish to retrieve, including the actual `_source`
of the document if it could be found (as indicated by the `found`
field in the response).
@ -223,13 +223,13 @@ will fail.
[[_source]]
=== Getting the +_source+ directly
Use the `/{index}/{type}/{id}/_source` endpoint to get
Use the `/{index}/_source/{id}` endpoint to get
just the `_source` field of the document,
without any additional content around it. For example:
[source,js]
--------------------------------------------------
GET twitter/_doc/1/_source
GET twitter/_source/1
--------------------------------------------------
// CONSOLE
// TEST[continued]
@ -238,7 +238,7 @@ You can also use the same source filtering parameters to control which parts of
[source,js]
--------------------------------------------------
GET twitter/_doc/1/_source?_source_includes=*.id&_source_excludes=entities'
GET twitter/_source/1/?_source_includes=*.id&_source_excludes=entities'
--------------------------------------------------
// CONSOLE
// TEST[continued]
@ -248,7 +248,7 @@ An existing document will not have a _source if it is disabled in the <<mapping-
[source,js]
--------------------------------------------------
HEAD twitter/_doc/1/_source
HEAD twitter/_source/1
--------------------------------------------------
// CONSOLE
// TEST[continued]

View File

@ -149,8 +149,8 @@ public class Netty4HeadBodyIsEmptyIT extends ESRestTestCase {
public void testGetSourceAction() throws IOException {
createTestDoc();
headTestCase("/test/test/1/_source", emptyMap(), greaterThan(0));
headTestCase("/test/test/2/_source", emptyMap(), NOT_FOUND.getStatus(), greaterThan(0));
headTestCase("/test/_source/1", emptyMap(), greaterThan(0));
headTestCase("/test/_source/2", emptyMap(), NOT_FOUND.getStatus(), greaterThan(0));
try (XContentBuilder builder = jsonBuilder()) {
builder.startObject();
@ -175,7 +175,7 @@ public class Netty4HeadBodyIsEmptyIT extends ESRestTestCase {
request.setJsonEntity(Strings.toString(builder));
client().performRequest(request);
createTestDoc("test-no-source", "test-no-source");
headTestCase("/test-no-source/test-no-source/1/_source", emptyMap(), NOT_FOUND.getStatus(), greaterThan(0));
headTestCase("/test-no-source/_source/1", emptyMap(), NOT_FOUND.getStatus(), greaterThan(0));
}
}

View File

@ -3,8 +3,8 @@
"documentation": "http://www.elastic.co/guide/en/elasticsearch/reference/master/docs-get.html",
"methods": ["HEAD"],
"url": {
"path": "/{index}/{type}/{id}/_source",
"paths": ["/{index}/{type}/{id}/_source"],
"path": "/{index}/_source/{id}",
"paths": ["/{index}/_source/{id}", "/{index}/{type}/{id}/_source"],
"parts": {
"id": {
"type" : "string",
@ -18,8 +18,8 @@
},
"type": {
"type" : "string",
"required" : true,
"description" : "The type of the document; use `_all` to fetch the first document matching the ID across all types"
"required" : false,
"description" : "The type of the document; deprecated and optional starting with 7.0"
}
},
"params": {

View File

@ -3,8 +3,8 @@
"documentation": "http://www.elastic.co/guide/en/elasticsearch/reference/master/docs-get.html",
"methods": ["GET"],
"url": {
"path": "/{index}/{type}/{id}/_source",
"paths": ["/{index}/{type}/{id}/_source"],
"path": "/{index}/_source/{id}",
"paths": ["/{index}/_source/{id}", "/{index}/{type}/{id}/_source"],
"parts": {
"id": {
"type" : "string",
@ -18,8 +18,8 @@
},
"type": {
"type" : "string",
"required" : true,
"description" : "The type of the document; use `_all` to fetch the first document matching the ID across all types"
"required" : false,
"description" : "The type of the document; deprecated and optional starting with 7.0"
}
},
"params": {

View File

@ -1,16 +1,19 @@
---
"Basic":
- skip:
version: " - 6.99.99"
reason: types are required in requests before 7.0.0
- do:
index:
index: test_1
type: test
id: 1
body: { "foo": "bar" }
- do:
get_source:
index: test_1
type: test
id: 1
- match: { '': { foo: bar } }
@ -18,7 +21,6 @@
- do:
get_source:
index: test_1
type: _all
id: 1
- match: { '': { foo: bar } }

View File

@ -0,0 +1,17 @@
---
"Basic with types":
- do:
index:
index: test_1
type: test
id: 1
body: { "foo": "bar" }
- do:
get_source:
index: test_1
type: test
id: 1
- match: { '': { foo: bar } }

View File

@ -1,5 +1,11 @@
---
"Default values":
- skip:
version: " - 6.99.99"
reason: types are required in requests before 7.0.0
- do:
index:
index: test_1
@ -10,7 +16,6 @@
- do:
get_source:
index: test_1
type: _all
id: 1
- match: { '': { foo: bar } }

View File

@ -0,0 +1,16 @@
---
"Default values":
- do:
index:
index: test_1
type: test
id: 1
body: { "foo": "bar" }
- do:
get_source:
index: test_1
type: test
id: 1
- match: { '': { foo: bar } }

View File

@ -1,6 +1,11 @@
---
"Routing":
- skip:
version: " - 6.99.99"
reason: types are required in requests before 7.0.0
- do:
indices.create:
index: test_1
@ -26,7 +31,6 @@
- do:
get_source:
index: test_1
type: test
id: 1
routing: 5
@ -36,6 +40,5 @@
catch: missing
get_source:
index: test_1
type: test
id: 1

View File

@ -0,0 +1,42 @@
---
"Routing":
- do:
indices.create:
index: test_1
body:
settings:
index:
number_of_shards: 5
number_of_routing_shards: 5
number_of_replicas: 0
- do:
cluster.health:
wait_for_status: green
- do:
index:
index: test_1
type: test
id: 1
routing: 5
body: { foo: bar }
- do:
get_source:
index: test_1
type: test
id: 1
routing: 5
- match: { '': {foo: bar}}
- do:
catch: missing
get_source:
index: test_1
type: test
id: 1

View File

@ -1,6 +1,9 @@
---
"Realtime":
- skip:
version: " - 6.99.99"
reason: types are required in requests before 7.0.0
- do:
indices.create:
@ -25,14 +28,12 @@
catch: missing
get_source:
index: test_1
type: test
id: 1
realtime: false
- do:
get_source:
index: test_1
type: test
id: 1
realtime: true
@ -41,7 +42,6 @@
- do:
get_source:
index: test_1
type: test
id: 1
realtime: false
refresh: true

View File

@ -0,0 +1,49 @@
---
"Realtime":
- do:
indices.create:
index: test_1
body:
settings:
refresh_interval: -1
number_of_replicas: 0
- do:
cluster.health:
wait_for_status: green
- do:
index:
index: test_1
type: test
id: 1
body: { foo: bar }
- do:
catch: missing
get_source:
index: test_1
type: test
id: 1
realtime: false
- do:
get_source:
index: test_1
type: test
id: 1
realtime: true
- match: { '': {foo: bar}}
- do:
get_source:
index: test_1
type: test
id: 1
realtime: false
refresh: true
- match: { '': {foo: bar}}

View File

@ -1,6 +1,11 @@
---
"Source filtering":
- skip:
version: " - 6.99.99"
reason: types are required in requests before 7.0.0
- do:
index:
index: test_1
@ -9,18 +14,18 @@
body: { "include": { "field1": "v1", "field2": "v2" }, "count": 1 }
- do:
get_source: { index: test_1, type: test, id: 1, _source_includes: include.field1 }
get_source: { index: test_1, id: 1, _source_includes: include.field1 }
- match: { include.field1: v1 }
- is_false: include.field2
- do:
get_source: { index: test_1, type: test, id: 1, _source_includes: "include.field1,include.field2" }
get_source: { index: test_1, id: 1, _source_includes: "include.field1,include.field2" }
- match: { include.field1: v1 }
- match: { include.field2: v2 }
- is_false: count
- do:
get_source: { index: test_1, type: test, id: 1, _source_includes: include, _source_excludes: "*.field2" }
get_source: { index: test_1, id: 1, _source_includes: include, _source_excludes: "*.field2" }
- match: { include.field1: v1 }
- is_false: include.field2
- is_false: count

View File

@ -0,0 +1,27 @@
---
"Source filtering":
- do:
index:
index: test_1
type: test
id: 1
body: { "include": { "field1": "v1", "field2": "v2" }, "count": 1 }
- do:
get_source: { index: test_1, type: test, id: 1, _source_includes: include.field1 }
- match: { include.field1: v1 }
- is_false: include.field2
- do:
get_source: { index: test_1, type: test, id: 1, _source_includes: "include.field1,include.field2" }
- match: { include.field1: v1 }
- match: { include.field2: v2 }
- is_false: count
- do:
get_source: { index: test_1, type: test, id: 1, _source_includes: include, _source_excludes: "*.field2" }
- match: { include.field1: v1 }
- is_false: include.field2
- is_false: count

View File

@ -1,19 +1,27 @@
---
"Missing document with catch":
- skip:
features: warnings
version: " - 6.99.99"
reason: types are required in requests before 7.0.0
- do:
catch: missing
get_source:
index: test_1
type: test
id: 1
---
"Missing document with ignore":
- skip:
features: warnings
version: " - 6.99.99"
reason: types are required in requests before 7.0.0
- do:
get_source:
index: test_1
type: test
id: 1
ignore: 404

View File

@ -0,0 +1,19 @@
---
"Missing document with catch":
- do:
catch: missing
get_source:
index: test_1
type: test
id: 1
---
"Missing document with ignore":
- do:
get_source:
index: test_1
type: test
id: 1
ignore: 404

View File

@ -1,5 +1,10 @@
---
setup:
- skip:
version: " - 6.99.99"
reason: types are required in requests before 7.0.0
- do:
indices.create:
index: test_1
@ -23,7 +28,6 @@ setup:
catch: missing
get_source:
index: test_1
type: test
id: 1
---
@ -32,6 +36,5 @@ setup:
- do:
get_source:
index: test_1
type: test
id: 1
ignore: 404

View File

@ -0,0 +1,38 @@
---
setup:
- do:
indices.create:
index: test_1
body:
mappings:
test:
_source: { enabled: false }
- do:
index:
index: test_1
type: test
id: 1
body: { foo: bar }
---
"Missing document source with catch":
- do:
catch: missing
get_source:
index: test_1
type: test
id: 1
---
"Missing document source with ignore":
- do:
get_source:
index: test_1
type: test
id: 1
ignore: 404

View File

@ -38,7 +38,7 @@ import java.io.IOException;
import static org.elasticsearch.action.ValidateActions.addValidationError;
/**
* A request to get a document (its source) from an index based on its type (optional) and id. Best created using
* A request to get a document (its source) from an index based on its id. Best created using
* {@link org.elasticsearch.client.Requests#getRequest(String)}.
* <p>
* The operation requires the {@link #index()}, {@link #type(String)} and {@link #id(String)}
@ -84,7 +84,6 @@ public class GetRequest extends SingleShardRequest<GetRequest> implements Realti
* @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 GetRequest(String, String)} instead.
*/
@Deprecated
@ -127,7 +126,6 @@ public class GetRequest extends SingleShardRequest<GetRequest> implements Realti
/**
* Sets the type of the document to fetch.
*
* @deprecated Types are in the process of being removed.
*/
@Deprecated

View File

@ -19,12 +19,14 @@
package org.elasticsearch.rest.action.document;
import org.apache.logging.log4j.LogManager;
import org.elasticsearch.ResourceNotFoundException;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.get.GetRequest;
import org.elasticsearch.action.get.GetResponse;
import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentHelper;
@ -49,8 +51,14 @@ import static org.elasticsearch.rest.RestStatus.OK;
*/
public class RestGetSourceAction extends BaseRestHandler {
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(RestGetSourceAction.class));
static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Specifying types in get_source and exist_source"
+ "requests is deprecated.";
public RestGetSourceAction(final Settings settings, final RestController controller) {
super(settings);
controller.registerHandler(GET, "/{index}/_source/{id}", this);
controller.registerHandler(HEAD, "/{index}/_source/{id}", this);
controller.registerHandler(GET, "/{index}/{type}/{id}/_source", this);
controller.registerHandler(HEAD, "/{index}/{type}/{id}/_source", this);
}
@ -62,7 +70,13 @@ public class RestGetSourceAction extends BaseRestHandler {
@Override
public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
final GetRequest getRequest = new GetRequest(request.param("index"), request.param("type"), request.param("id"));
final GetRequest getRequest;
if (request.hasParam("type")) {
deprecationLogger.deprecatedAndMaybeLog("get_source_with_types", TYPES_DEPRECATION_MESSAGE);
getRequest = new GetRequest(request.param("index"), request.param("type"), request.param("id"));
} else {
getRequest = new GetRequest(request.param("index"), request.param("id"));
}
getRequest.refresh(request.paramAsBoolean("refresh", getRequest.refresh()));
getRequest.routing(request.param("routing"));
getRequest.preference(request.param("preference"));

View File

@ -40,6 +40,7 @@ public class GetRequestTests extends ESTestCase {
final ActionRequestValidationException validate = request.validate();
assertThat(validate, not(nullValue()));
assertEquals(2, validate.validationErrors().size());
assertThat(validate.validationErrors(), hasItems("type is missing", "id is missing"));
}
}

View File

@ -23,26 +23,38 @@ import org.elasticsearch.ResourceNotFoundException;
import org.elasticsearch.action.get.GetResponse;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.get.GetResult;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.RestRequest.Method;
import org.elasticsearch.rest.RestResponse;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.rest.action.document.RestGetSourceAction.RestGetSourceResponseListener;
import org.elasticsearch.test.rest.FakeRestChannel;
import org.elasticsearch.test.rest.FakeRestRequest;
import org.elasticsearch.test.rest.RestActionTestCase;
import org.junit.AfterClass;
import org.junit.Before;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import static java.util.Collections.emptyMap;
import static org.elasticsearch.index.seqno.SequenceNumbers.UNASSIGNED_SEQ_NO;
import static org.elasticsearch.rest.RestStatus.OK;
import static org.elasticsearch.rest.action.document.RestGetSourceAction.RestGetSourceResponseListener;
import static org.hamcrest.Matchers.equalTo;
public class RestGetSourceActionTests extends ESTestCase {
public class RestGetSourceActionTests extends RestActionTestCase {
private static RestRequest request = new FakeRestRequest();
private static FakeRestChannel channel = new FakeRestChannel(request, true, 0);
private static RestGetSourceResponseListener listener = new RestGetSourceResponseListener(channel, request);
@Before
public void setUpAction() {
new RestGetSourceAction(Settings.EMPTY, controller());
}
@AfterClass
public static void cleanupReferences() {
request = null;
@ -50,6 +62,37 @@ public class RestGetSourceActionTests extends ESTestCase {
listener = null;
}
/**
* test deprecation is logged if type is used in path
*/
public void testTypeInPath() {
for (Method method : Arrays.asList(Method.GET, Method.HEAD)) {
RestRequest request = new FakeRestRequest.Builder(xContentRegistry())
.withMethod(method)
.withPath("/some_index/some_type/id/_source")
.build();
dispatchRequest(request);
assertWarnings(RestGetSourceAction.TYPES_DEPRECATION_MESSAGE);
}
}
/**
* test deprecation is logged if type is used as parameter
*/
public void testTypeParameter() {
Map<String, String> params = new HashMap<>();
params.put("type", "some_type");
for (Method method : Arrays.asList(Method.GET, Method.HEAD)) {
RestRequest request = new FakeRestRequest.Builder(xContentRegistry())
.withMethod(method)
.withPath("/some_index/_source/id")
.withParams(params)
.build();
dispatchRequest(request);
assertWarnings(RestGetSourceAction.TYPES_DEPRECATION_MESSAGE);
}
}
public void testRestGetSourceAction() throws Exception {
final BytesReference source = new BytesArray("{\"foo\": \"bar\"}");
final GetResponse response =