Skip to content

Commit

Permalink
Deprecate types in get_source and exist_source (#36426)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Christoph Büscher authored Dec 17, 2018
1 parent f0f2b26 commit 2f5300e
Show file tree
Hide file tree
Showing 25 changed files with 393 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,14 @@ private static Request getStyleRequest(String method, GetRequest getRequest) {
}

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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
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;
Expand Down Expand Up @@ -115,6 +116,7 @@
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;
Expand Down Expand Up @@ -156,6 +158,58 @@ public void testGetWithType() {
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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1265,7 +1265,6 @@ public void testGet() throws Exception {
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();
Expand Down
14 changes: 7 additions & 7 deletions docs/reference/docs/get.asciidoc
Original file line number Diff line number Diff line change
@@ -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]
--------------------------------------------------
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ public void testTemplateExists() throws IOException {

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();
Expand All @@ -175,7 +175,7 @@ public void testGetSourceAction() throws IOException {
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));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,24 +1,26 @@
---
"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 } }

- do:
get_source:
index: test_1
type: _all
id: 1

- match: { '': { foo: bar } }
Original file line number Diff line number Diff line change
@@ -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 } }
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -10,7 +16,6 @@
- do:
get_source:
index: test_1
type: _all
id: 1

- match: { '': { foo: bar } }
Original file line number Diff line number Diff line change
@@ -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 } }
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -26,7 +31,6 @@
- do:
get_source:
index: test_1
type: test
id: 1
routing: 5

Expand All @@ -36,6 +40,5 @@
catch: missing
get_source:
index: test_1
type: test
id: 1

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

Loading

0 comments on commit 2f5300e

Please sign in to comment.