From 8e8ec90767fcfe5da121850ab51b91f3648d5ee8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Fri, 7 Dec 2018 14:14:24 +0100 Subject: [PATCH 1/9] WIP --- .../documentation/CRUDDocumentationIT.java | 4 +- .../high-level/document/get.asciidoc | 3 +- docs/reference/docs/get.asciidoc | 12 ++-- .../rest/Netty4HeadBodyIsEmptyIT.java | 4 +- .../rest-api-spec/api/exists_source.json | 8 +-- .../rest-api-spec/api/get_source.json | 8 +-- .../test/get_source/10_basic.yml | 2 - .../11_basic_deprecated_endpoints.yml | 33 ++++++++++ .../test/get_source/15_default_values.yml | 1 - .../test/get_source/40_routing.yml | 2 - .../test/get_source/60_realtime_refresh.yml | 3 - .../test/get_source/70_source_filtering.yml | 6 +- .../test/get_source/80_missing.yml | 2 - .../test/get_source/85_source_missing.yml | 2 - .../elasticsearch/action/get/GetRequest.java | 33 +++++++--- .../action/document/RestGetSourceAction.java | 15 ++++- .../action/get/GetRequestTests.java | 2 +- .../document/RestGetSourceActionTests.java | 60 ++++++++++++++++++- 18 files changed, 154 insertions(+), 46 deletions(-) create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/get_source/11_basic_deprecated_endpoints.yml 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 index 4fc7fd21a399e..cee1fc9855b22 100644 --- 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 @@ -1258,8 +1258,7 @@ public void testGet() throws Exception { //tag::get-request GetRequest getRequest = new GetRequest( "posts", // <1> - "_doc", // <2> - "1"); // <3> + "1"); // <2> //end::get-request //tag::get-execute @@ -1269,7 +1268,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(); diff --git a/docs/java-rest/high-level/document/get.asciidoc b/docs/java-rest/high-level/document/get.asciidoc index f3ea9434f71c2..2916eb9335c76 100644 --- a/docs/java-rest/high-level/document/get.asciidoc +++ b/docs/java-rest/high-level/document/get.asciidoc @@ -17,8 +17,7 @@ A +{request}+ requires the following arguments: include-tagged::{doc-tests-file}[{api}-request] -------------------------------------------------- <1> Index -<2> Type -<3> Document id +<2> Document id [id="{upid}-{api}-request-optional-arguments"] ==== Optional arguments diff --git a/docs/reference/docs/get.asciidoc b/docs/reference/docs/get.asciidoc index ec6ef28534fd6..8f5946e4d3138 100644 --- a/docs/reference/docs/get.asciidoc +++ b/docs/reference/docs/get.asciidoc @@ -1,13 +1,13 @@ [[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] -------------------------------------------------- -GET twitter/_doc/0 +GET twitter/0 -------------------------------------------------- // CONSOLE // TEST[setup:twitter] @@ -32,7 +32,7 @@ The result of the above get operation is: -------------------------------------------------- // TESTRESPONSE -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). @@ -42,7 +42,7 @@ The API also allows to check for the existence of a document using [source,js] -------------------------------------------------- -HEAD twitter/_doc/0 +HEAD twitter/0 -------------------------------------------------- // CONSOLE // TEST[setup:twitter] @@ -217,7 +217,7 @@ will fail. [[_source]] === Getting the +_source+ directly -Use the `/{index}/{type}/{id}/_source` endpoint to get +Use the `/{index}/{id}/_source` endpoint to get just the `_source` field of the document, without any additional content around it. For example: diff --git a/modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java b/modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java index 3a9315015c06d..605bf0b628b39 100644 --- a/modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java +++ b/modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java @@ -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/1/_source", emptyMap(), greaterThan(0)); + headTestCase("/test/2/_source", emptyMap(), NOT_FOUND.getStatus(), greaterThan(0)); try (XContentBuilder builder = jsonBuilder()) { builder.startObject(); diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/exists_source.json b/rest-api-spec/src/main/resources/rest-api-spec/api/exists_source.json index f08aa8392521c..625b55e06c4b2 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/exists_source.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/exists_source.json @@ -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}/{id}/_source", + "paths": ["/{index}/{id}/_source", "/{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": { diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/get_source.json b/rest-api-spec/src/main/resources/rest-api-spec/api/get_source.json index f8d116abe87ac..0176e61bead93 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/get_source.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/get_source.json @@ -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}/{id}/_source", + "paths": ["/{index}/{id}/_source", "/{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": { diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/10_basic.yml index b20357cdf99a6..733c83dbfc6e6 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/10_basic.yml @@ -10,7 +10,6 @@ - do: get_source: index: test_1 - type: test id: 1 - match: { '': { foo: bar } } @@ -18,7 +17,6 @@ - do: get_source: index: test_1 - type: _all id: 1 - match: { '': { foo: bar } } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/11_basic_deprecated_endpoints.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/11_basic_deprecated_endpoints.yml new file mode 100644 index 0000000000000..059ac6e731590 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/11_basic_deprecated_endpoints.yml @@ -0,0 +1,33 @@ +--- +"Basic": + - skip: + version: " - 6.99.99" + reason: deprecated in 7.0 + features: "warnings" + + - do: + index: + index: test_1 + type: test + id: 1 + body: { "foo": "bar" } + + - do: + warnings: + - '[types removal] Specifying types in get_source requests is deprecated.' + get_source: + index: test_1 + type: test + id: 1 + + - match: { '': { foo: bar } } + +# - do: +# warnings: +# - '[types removal] Specifying types in get_source requests is deprecated.' +# get_source: +# index: test_1 +# type: test +# id: 1 +# +# - match: { '': { foo: bar } } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/15_default_values.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/15_default_values.yml index 3e67c62ffc2eb..2120d40dcf4f9 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/15_default_values.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/15_default_values.yml @@ -10,7 +10,6 @@ - do: get_source: index: test_1 - type: _all id: 1 - match: { '': { foo: bar } } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/40_routing.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/40_routing.yml index b4eebfa8ab947..a53147642aa40 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/40_routing.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/40_routing.yml @@ -26,7 +26,6 @@ - do: get_source: index: test_1 - type: test id: 1 routing: 5 @@ -36,6 +35,5 @@ catch: missing get_source: index: test_1 - type: test id: 1 diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/60_realtime_refresh.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/60_realtime_refresh.yml index fbf3948d7b1e8..11f483229565c 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/60_realtime_refresh.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/60_realtime_refresh.yml @@ -25,14 +25,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 +39,6 @@ - do: get_source: index: test_1 - type: test id: 1 realtime: false refresh: true diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/70_source_filtering.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/70_source_filtering.yml index c9cb2b00db038..15c99b15ffd87 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/70_source_filtering.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/70_source_filtering.yml @@ -9,18 +9,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 diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/80_missing.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/80_missing.yml index fba854041117f..d2d2ad83bf290 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/80_missing.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/80_missing.yml @@ -5,7 +5,6 @@ catch: missing get_source: index: test_1 - type: test id: 1 --- @@ -14,6 +13,5 @@ - do: get_source: index: test_1 - type: test id: 1 ignore: 404 diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/85_source_missing.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/85_source_missing.yml index 6ec261ac61dbd..3c54e1950e524 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/85_source_missing.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/85_source_missing.yml @@ -23,7 +23,6 @@ setup: catch: missing get_source: index: test_1 - type: test id: 1 --- @@ -32,6 +31,5 @@ setup: - do: get_source: index: test_1 - type: test id: 1 ignore: 404 diff --git a/server/src/main/java/org/elasticsearch/action/get/GetRequest.java b/server/src/main/java/org/elasticsearch/action/get/GetRequest.java index 5162fd46ecab0..f4cd0c2740ff4 100644 --- a/server/src/main/java/org/elasticsearch/action/get/GetRequest.java +++ b/server/src/main/java/org/elasticsearch/action/get/GetRequest.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.search.fetch.subphase.FetchSourceContext; import java.io.IOException; @@ -37,7 +38,7 @@ 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)}. *

* The operation requires the {@link #index()}, {@link #type(String)} and {@link #id(String)} @@ -66,7 +67,7 @@ public class GetRequest extends SingleShardRequest implements Realti private long version = Versions.MATCH_ANY; public GetRequest() { - type = "_all"; + type = MapperService.SINGLE_MAPPING_NAME; } /** @@ -75,7 +76,7 @@ public GetRequest() { */ public GetRequest(String index) { super(index); - this.type = "_all"; + type = MapperService.SINGLE_MAPPING_NAME; } /** @@ -84,19 +85,30 @@ public GetRequest(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 use the typeless constructor {@link #GetRequest(String, String)} where no type is given */ + @Deprecated public GetRequest(String index, String type, String id) { super(index); this.type = type; this.id = id; } + /** + * Constructs a new get request against the specified index with the type and id. + * + * @param index The index to get the document from + * @param id The id of the document + */ + public GetRequest(String index, String id) { + super(index); + this.type = MapperService.SINGLE_MAPPING_NAME;; + this.id = id; + } + @Override public ActionRequestValidationException validate() { ActionRequestValidationException validationException = super.validateNonNullIndex(); - if (Strings.isEmpty(type)) { - validationException = addValidationError("type is missing", validationException); - } if (Strings.isEmpty(id)) { validationException = addValidationError("id is missing", validationException); } @@ -112,10 +124,12 @@ public ActionRequestValidationException validate() { /** * Sets the type of the document to fetch. + * @deprecated don't use types in {@link GetRequest} any more as they are in the process of being removed */ + @Deprecated public GetRequest type(@Nullable String type) { if (type == null) { - type = "_all"; + type = MapperService.SINGLE_MAPPING_NAME;; } this.type = type; return this; @@ -148,6 +162,11 @@ public GetRequest preference(String preference) { return this; } + /** + * @return the type + * @deprecated don't use types any more as they are in the process of being removed + */ + @Deprecated public String type() { return type; } diff --git a/server/src/main/java/org/elasticsearch/rest/action/document/RestGetSourceAction.java b/server/src/main/java/org/elasticsearch/rest/action/document/RestGetSourceAction.java index c48529d420c1e..f0050c6c25542 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/document/RestGetSourceAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/document/RestGetSourceAction.java @@ -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,13 @@ */ 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 requests is deprecated."; + public RestGetSourceAction(final Settings settings, final RestController controller) { super(settings); + controller.registerHandler(GET, "/{index}/{id}/_source", this); + controller.registerHandler(HEAD, "/{index}/{id}/_source", this); controller.registerHandler(GET, "/{index}/{type}/{id}/_source", this); controller.registerHandler(HEAD, "/{index}/{type}/{id}/_source", this); } @@ -62,7 +69,13 @@ public String getName() { @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_type", 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")); diff --git a/server/src/test/java/org/elasticsearch/action/get/GetRequestTests.java b/server/src/test/java/org/elasticsearch/action/get/GetRequestTests.java index fc2162e866240..bbd3b23153f3a 100644 --- a/server/src/test/java/org/elasticsearch/action/get/GetRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/get/GetRequestTests.java @@ -40,7 +40,7 @@ public void testValidation() { final ActionRequestValidationException validate = request.validate(); assertThat(validate, not(nullValue())); - assertThat(validate.validationErrors(), hasItems("type is missing", "id is missing")); + assertThat(validate.validationErrors(), hasItems("id is missing")); } } } diff --git a/server/src/test/java/org/elasticsearch/rest/action/document/RestGetSourceActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/document/RestGetSourceActionTests.java index b9fd724fb656d..7ab9802c48317 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/document/RestGetSourceActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/document/RestGetSourceActionTests.java @@ -21,26 +21,51 @@ import org.elasticsearch.ResourceNotFoundException; import org.elasticsearch.action.get.GetResponse; +import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.index.get.GetResult; +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.rest.RestResponse; +import org.elasticsearch.rest.action.document.RestGetSourceAction.RestGetSourceResponseListener; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.rest.FakeRestChannel; import org.elasticsearch.test.rest.FakeRestRequest; +import org.elasticsearch.usage.UsageService; import org.junit.AfterClass; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + import static java.util.Collections.emptyMap; import static org.elasticsearch.rest.RestStatus.OK; -import static org.elasticsearch.rest.action.document.RestGetSourceAction.RestGetSourceResponseListener; import static org.hamcrest.Matchers.equalTo; +import static org.mockito.Mockito.mock; public class RestGetSourceActionTests extends ESTestCase { private static RestRequest request = new FakeRestRequest(); private static FakeRestChannel channel = new FakeRestChannel(request, true, 0); private static RestGetSourceResponseListener listener = new RestGetSourceResponseListener(channel, request); + private RestController controller; + + @Override + public void setUp() throws Exception { + super.setUp(); + controller = new RestController(Collections.emptySet(), null, + mock(NodeClient.class), + new NoneCircuitBreakerService(), + new UsageService()); + new RestGetSourceAction(Settings.EMPTY, controller); + } @AfterClass public static void cleanupReferences() { @@ -49,6 +74,39 @@ public static void cleanupReferences() { listener = null; } + /** + * test deprecation is logged if type is used in path + */ + public void testTypeInPath() { + RestRequest request = new FakeRestRequest.Builder(xContentRegistry()) + .withMethod(randomFrom(Arrays.asList(Method.GET, Method.HEAD))) + .withPath("/some_index/some_type/id/_source") + .build(); + performRequest(request); + assertWarnings(RestGetSourceAction.TYPES_DEPRECATION_MESSAGE); + } + + /** + * test deprecation is logged if type is used as parameter + */ + public void testTypeParameter() { + Map params = new HashMap<>(); + params.put("type", "some_type"); + RestRequest request = new FakeRestRequest.Builder(xContentRegistry()) + .withMethod(randomFrom(Arrays.asList(Method.GET, Method.HEAD))) + .withPath("/some_index/id/_source") + .withParams(params) + .build(); + performRequest(request); + assertWarnings(RestGetSourceAction.TYPES_DEPRECATION_MESSAGE); + } + + private void performRequest(RestRequest request) { + RestChannel channel = new FakeRestChannel(request, false, 1); + ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + controller.dispatchRequest(request, channel, threadContext); + } + public void testRestGetSourceAction() throws Exception { final BytesReference source = new BytesArray("{\"foo\": \"bar\"}"); final GetResponse response = new GetResponse(new GetResult("index1", "_doc", "1", -1, true, source, emptyMap())); From d499679e55697db8db382a86554ce9ab70d1ea09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Mon, 10 Dec 2018 13:05:55 +0100 Subject: [PATCH 2/9] iter --- .../java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java | 2 +- .../rest/action/document/RestGetSourceAction.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java b/modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java index 605bf0b628b39..9341e92ba9cc1 100644 --- a/modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java +++ b/modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java @@ -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/1/_source", emptyMap(), NOT_FOUND.getStatus(), greaterThan(0)); } } diff --git a/server/src/main/java/org/elasticsearch/rest/action/document/RestGetSourceAction.java b/server/src/main/java/org/elasticsearch/rest/action/document/RestGetSourceAction.java index f0050c6c25542..493fea76b4d27 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/document/RestGetSourceAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/document/RestGetSourceAction.java @@ -52,7 +52,8 @@ 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 requests is deprecated."; + 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); From 9177931526f8b9a40fe879d58843e631e3295b96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Thu, 13 Dec 2018 13:21:23 +0100 Subject: [PATCH 3/9] iter --- docs/reference/docs/get.asciidoc | 2 +- .../rest-api-spec/api/exists_source.json | 4 +- .../rest-api-spec/api/get_source.json | 4 +- .../test/get_source/10_basic.yml | 6 +- .../11_basic_deprecated_endpoints.yml | 16 +---- .../elasticsearch/action/get/GetRequest.java | 5 -- .../action/document/RestGetSourceAction.java | 4 +- .../document/RestGetSourceActionTests.java | 63 +++++++------------ 8 files changed, 37 insertions(+), 67 deletions(-) diff --git a/docs/reference/docs/get.asciidoc b/docs/reference/docs/get.asciidoc index 8f5946e4d3138..20252e46a660d 100644 --- a/docs/reference/docs/get.asciidoc +++ b/docs/reference/docs/get.asciidoc @@ -42,7 +42,7 @@ The API also allows to check for the existence of a document using [source,js] -------------------------------------------------- -HEAD twitter/0 +HEAD twitter/_doc/0 -------------------------------------------------- // CONSOLE // TEST[setup:twitter] diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/exists_source.json b/rest-api-spec/src/main/resources/rest-api-spec/api/exists_source.json index 625b55e06c4b2..a3edff1d111a5 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/exists_source.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/exists_source.json @@ -3,8 +3,8 @@ "documentation": "http://www.elastic.co/guide/en/elasticsearch/reference/master/docs-get.html", "methods": ["HEAD"], "url": { - "path": "/{index}/{id}/_source", - "paths": ["/{index}/{id}/_source", "/{index}/{type}/{id}/_source"], + "path": "/{index}/_source/{id}", + "paths": ["/{index}/_source/{id}", "/{index}/{type}/{id}/_source"], "parts": { "id": { "type" : "string", diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/get_source.json b/rest-api-spec/src/main/resources/rest-api-spec/api/get_source.json index 0176e61bead93..cd737e32d6ca8 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/get_source.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/get_source.json @@ -3,8 +3,8 @@ "documentation": "http://www.elastic.co/guide/en/elasticsearch/reference/master/docs-get.html", "methods": ["GET"], "url": { - "path": "/{index}/{id}/_source", - "paths": ["/{index}/{id}/_source", "/{index}/{type}/{id}/_source"], + "path": "/{index}/_source/{id}", + "paths": ["/{index}/_source/{id}", "/{index}/{type}/{id}/_source"], "parts": { "id": { "type" : "string", diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/10_basic.yml index 733c83dbfc6e6..6f81c430c883a 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/10_basic.yml @@ -1,9 +1,13 @@ --- "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" } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/11_basic_deprecated_endpoints.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/11_basic_deprecated_endpoints.yml index 059ac6e731590..4b4e148a3bbf9 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/11_basic_deprecated_endpoints.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/11_basic_deprecated_endpoints.yml @@ -1,9 +1,5 @@ --- -"Basic": - - skip: - version: " - 6.99.99" - reason: deprecated in 7.0 - features: "warnings" +"Basic with types": - do: index: @@ -21,13 +17,3 @@ id: 1 - match: { '': { foo: bar } } - -# - do: -# warnings: -# - '[types removal] Specifying types in get_source requests is deprecated.' -# get_source: -# index: test_1 -# type: test -# id: 1 -# -# - match: { '': { foo: bar } } diff --git a/server/src/main/java/org/elasticsearch/action/get/GetRequest.java b/server/src/main/java/org/elasticsearch/action/get/GetRequest.java index ac313923e8385..6ba0f7b5a32c4 100644 --- a/server/src/main/java/org/elasticsearch/action/get/GetRequest.java +++ b/server/src/main/java/org/elasticsearch/action/get/GetRequest.java @@ -162,12 +162,7 @@ public GetRequest preference(String preference) { } /** -<<<<<<< HEAD - * @return the type - * @deprecated don't use types any more as they are in the process of being removed -======= * @deprecated Types are in the process of being removed. ->>>>>>> master */ @Deprecated public String type() { diff --git a/server/src/main/java/org/elasticsearch/rest/action/document/RestGetSourceAction.java b/server/src/main/java/org/elasticsearch/rest/action/document/RestGetSourceAction.java index 493fea76b4d27..37d2d0379f2cd 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/document/RestGetSourceAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/document/RestGetSourceAction.java @@ -57,8 +57,8 @@ public class RestGetSourceAction extends BaseRestHandler { public RestGetSourceAction(final Settings settings, final RestController controller) { super(settings); - controller.registerHandler(GET, "/{index}/{id}/_source", this); - controller.registerHandler(HEAD, "/{index}/{id}/_source", this); + 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); } diff --git a/server/src/test/java/org/elasticsearch/rest/action/document/RestGetSourceActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/document/RestGetSourceActionTests.java index 7ab9802c48317..50e6761f46eba 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/document/RestGetSourceActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/document/RestGetSourceActionTests.java @@ -21,50 +21,37 @@ import org.elasticsearch.ResourceNotFoundException; import org.elasticsearch.action.get.GetResponse; -import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.index.get.GetResult; -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.rest.RestResponse; import org.elasticsearch.rest.action.document.RestGetSourceAction.RestGetSourceResponseListener; -import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.rest.FakeRestChannel; import org.elasticsearch.test.rest.FakeRestRequest; -import org.elasticsearch.usage.UsageService; +import org.elasticsearch.test.rest.RestActionTestCase; import org.junit.AfterClass; +import org.junit.Before; import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.Map; import static java.util.Collections.emptyMap; import static org.elasticsearch.rest.RestStatus.OK; import static org.hamcrest.Matchers.equalTo; -import static org.mockito.Mockito.mock; -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); - private RestController controller; - - @Override - public void setUp() throws Exception { - super.setUp(); - controller = new RestController(Collections.emptySet(), null, - mock(NodeClient.class), - new NoneCircuitBreakerService(), - new UsageService()); - new RestGetSourceAction(Settings.EMPTY, controller); + + @Before + public void setUpAction() { + new RestGetSourceAction(Settings.EMPTY, controller()); } @AfterClass @@ -78,12 +65,14 @@ public static void cleanupReferences() { * test deprecation is logged if type is used in path */ public void testTypeInPath() { - RestRequest request = new FakeRestRequest.Builder(xContentRegistry()) - .withMethod(randomFrom(Arrays.asList(Method.GET, Method.HEAD))) - .withPath("/some_index/some_type/id/_source") - .build(); - performRequest(request); - assertWarnings(RestGetSourceAction.TYPES_DEPRECATION_MESSAGE); + 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); + } } /** @@ -92,19 +81,15 @@ public void testTypeInPath() { public void testTypeParameter() { Map params = new HashMap<>(); params.put("type", "some_type"); - RestRequest request = new FakeRestRequest.Builder(xContentRegistry()) - .withMethod(randomFrom(Arrays.asList(Method.GET, Method.HEAD))) - .withPath("/some_index/id/_source") - .withParams(params) - .build(); - performRequest(request); - assertWarnings(RestGetSourceAction.TYPES_DEPRECATION_MESSAGE); - } - - private void performRequest(RestRequest request) { - RestChannel channel = new FakeRestChannel(request, false, 1); - ThreadContext threadContext = new ThreadContext(Settings.EMPTY); - controller.dispatchRequest(request, channel, threadContext); + 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 { From 4ed6c18fba1f87d8942995f246a8e977c2053781 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Thu, 13 Dec 2018 18:52:15 +0100 Subject: [PATCH 4/9] add type and typless yml tests --- ..._endpoints.yml => 11_basic_with_types.yml} | 0 .../test/get_source/15_default_values.yml | 6 +++ .../16_default_values_with_types.yml | 16 ++++++ .../test/get_source/40_routing.yml | 5 ++ .../test/get_source/41_routing_with_types.yml | 46 +++++++++++++++++ .../test/get_source/60_realtime_refresh.yml | 3 ++ .../61_realtime_refresh_with_types.yml | 49 +++++++++++++++++++ .../test/get_source/70_source_filtering.yml | 5 ++ .../71_source_filtering_with_types.yml | 31 ++++++++++++ .../test/get_source/80_missing.yml | 4 ++ .../test/get_source/81_missing_with_types.yml | 19 +++++++ .../test/get_source/85_source_missing.yml | 5 ++ .../86_source_missing_with_types.yml | 38 ++++++++++++++ 13 files changed, 227 insertions(+) rename rest-api-spec/src/main/resources/rest-api-spec/test/get_source/{11_basic_deprecated_endpoints.yml => 11_basic_with_types.yml} (100%) create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/get_source/16_default_values_with_types.yml create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/get_source/41_routing_with_types.yml create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/get_source/61_realtime_refresh_with_types.yml create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/get_source/71_source_filtering_with_types.yml create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/get_source/81_missing_with_types.yml create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/get_source/86_source_missing_with_types.yml diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/11_basic_deprecated_endpoints.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/11_basic_with_types.yml similarity index 100% rename from rest-api-spec/src/main/resources/rest-api-spec/test/get_source/11_basic_deprecated_endpoints.yml rename to rest-api-spec/src/main/resources/rest-api-spec/test/get_source/11_basic_with_types.yml diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/15_default_values.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/15_default_values.yml index 2120d40dcf4f9..a3739823766f1 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/15_default_values.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/15_default_values.yml @@ -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 diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/16_default_values_with_types.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/16_default_values_with_types.yml new file mode 100644 index 0000000000000..e2de7a9f0007c --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/16_default_values_with_types.yml @@ -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 } } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/40_routing.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/40_routing.yml index a53147642aa40..7a4cb421f2eac 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/40_routing.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/40_routing.yml @@ -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 diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/41_routing_with_types.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/41_routing_with_types.yml new file mode 100644 index 0000000000000..398a0860f9a6d --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/41_routing_with_types.yml @@ -0,0 +1,46 @@ +--- +"Routing": + + + - skip: + version: " - 6.99.99" + reason: types are required in requests before 7.0.0 + + - 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 + diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/60_realtime_refresh.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/60_realtime_refresh.yml index 11f483229565c..6afaafe5da434 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/60_realtime_refresh.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/60_realtime_refresh.yml @@ -1,6 +1,9 @@ --- "Realtime": + - skip: + version: " - 6.99.99" + reason: types are required in requests before 7.0.0 - do: indices.create: diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/61_realtime_refresh_with_types.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/61_realtime_refresh_with_types.yml new file mode 100644 index 0000000000000..f5b406de28b4a --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/61_realtime_refresh_with_types.yml @@ -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}} + diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/70_source_filtering.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/70_source_filtering.yml index 15c99b15ffd87..3687ec40f0bb9 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/70_source_filtering.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/70_source_filtering.yml @@ -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 diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/71_source_filtering_with_types.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/71_source_filtering_with_types.yml new file mode 100644 index 0000000000000..01147138f2c5f --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/71_source_filtering_with_types.yml @@ -0,0 +1,31 @@ +--- +"Source filtering": + + + - 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: { "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 diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/80_missing.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/80_missing.yml index d2d2ad83bf290..99f0ff658cb56 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/80_missing.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/80_missing.yml @@ -1,3 +1,7 @@ + - skip: + version: " - 6.99.99" + reason: types are required in requests before 7.0.0 + --- "Missing document with catch": diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/81_missing_with_types.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/81_missing_with_types.yml new file mode 100644 index 0000000000000..16eb5ea51e898 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/81_missing_with_types.yml @@ -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 diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/85_source_missing.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/85_source_missing.yml index 3c54e1950e524..78f2760a04944 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/85_source_missing.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/85_source_missing.yml @@ -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 diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/86_source_missing_with_types.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/86_source_missing_with_types.yml new file mode 100644 index 0000000000000..63a47c7c95836 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/get_source/86_source_missing_with_types.yml @@ -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 From e901dbab39eb209c7d10721e0f4aa5e4bcf05702 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Thu, 13 Dec 2018 19:48:40 +0100 Subject: [PATCH 5/9] iter --- docs/reference/docs/get.asciidoc | 10 +++++----- .../test/get_source/11_basic_with_types.yml | 2 -- .../test/get_source/41_routing_with_types.yml | 4 ---- .../get_source/71_source_filtering_with_types.yml | 4 ---- .../rest-api-spec/test/get_source/80_missing.yml | 12 +++++++++--- 5 files changed, 14 insertions(+), 18 deletions(-) diff --git a/docs/reference/docs/get.asciidoc b/docs/reference/docs/get.asciidoc index 20252e46a660d..cd9f7fae1bcda 100644 --- a/docs/reference/docs/get.asciidoc +++ b/docs/reference/docs/get.asciidoc @@ -7,7 +7,7 @@ twitter with id valued 0: [source,js] -------------------------------------------------- -GET twitter/0 +GET twitter/_doc/0 -------------------------------------------------- // CONSOLE // TEST[setup:twitter] @@ -217,13 +217,13 @@ will fail. [[_source]] === Getting the +_source+ directly -Use the `/{index}/{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] @@ -232,7 +232,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] @@ -242,7 +242,7 @@ An existing document will not have a _source if it is disabled in the < Date: Fri, 14 Dec 2018 15:53:01 +0100 Subject: [PATCH 6/9] Add change in enpoint mapping for hlrc --- .../java/org/elasticsearch/client/RequestConverters.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java index f0cff9a315858..169eb945dfc94 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java @@ -267,7 +267,12 @@ 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 endpoint = new EndpointBuilder() + .addPathPart(getRequest.index()) + .addPathPartAsIs("_source") + .addPathPart(getRequest.id()) + .build(); + Request request = new Request(HttpHead.METHOD_NAME, endpoint); Params parameters = new Params(request); parameters.withPreference(getRequest.preference()); From 582912f2414a765f8a64e5fab96c184bc84bc3a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Fri, 14 Dec 2018 17:17:59 +0100 Subject: [PATCH 7/9] Fix Netty4HeadBodyIsEmptyIT --- .../org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java b/modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java index 9341e92ba9cc1..79b75487f6568 100644 --- a/modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java +++ b/modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java @@ -149,8 +149,8 @@ public void testTemplateExists() throws IOException { public void testGetSourceAction() throws IOException { createTestDoc(); - headTestCase("/test/1/_source", emptyMap(), greaterThan(0)); - headTestCase("/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 void testGetSourceAction() throws IOException { request.setJsonEntity(Strings.toString(builder)); client().performRequest(request); createTestDoc("test-no-source", "test-no-source"); - headTestCase("/test-no-source/1/_source", emptyMap(), NOT_FOUND.getStatus(), greaterThan(0)); + headTestCase("/test-no-source/_source/1", emptyMap(), NOT_FOUND.getStatus(), greaterThan(0)); } } From fb86003b924334f0dee1406ce57f0b8ddde3b0ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Mon, 17 Dec 2018 10:09:13 +0100 Subject: [PATCH 8/9] Addressing review comments --- .../client/RequestConverters.java | 13 ++--- .../client/RequestConvertersTests.java | 49 +++++++++++++++++++ .../elasticsearch/action/get/GetRequest.java | 3 ++ .../action/document/RestGetSourceAction.java | 2 +- 4 files changed, 60 insertions(+), 7 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java index 1c3c39e494ec6..06764ca02743a 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java @@ -268,13 +268,14 @@ private static Request getStyleRequest(String method, GetRequest getRequest) { } static Request sourceExists(GetRequest getRequest) { - String endpoint = new EndpointBuilder() - .addPathPart(getRequest.index()) - .addPathPartAsIs("_source") - .addPathPart(getRequest.id()) - .build(); + String optionalType = getRequest.type(); + String endpoint; + if (optionalType == null || 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()); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java index b50d2c1265ed2..93dddf51cd6c2 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java @@ -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; @@ -156,6 +157,54 @@ public void testGetWithType() { getAndExistsWithTypeTest(RequestConverters::get, HttpGet.METHOD_NAME); } + public void testSourceExists() throws IOException { + String index = randomAlphaOfLengthBetween(3, 10); + String id = randomAlphaOfLengthBetween(3, 10); + final GetRequest getRequest; + if (randomBoolean()) { + getRequest = new GetRequest(index, id); + } else { + getRequest = new GetRequest(index, randomFrom(MapperService.SINGLE_MAPPING_NAME, null), id); + } + + Map 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 == null || 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 expectedParams = new HashMap<>(); MultiGetRequest multiGetRequest = new MultiGetRequest(); diff --git a/server/src/main/java/org/elasticsearch/action/get/GetRequest.java b/server/src/main/java/org/elasticsearch/action/get/GetRequest.java index 6ba0f7b5a32c4..2a4ba570d7011 100644 --- a/server/src/main/java/org/elasticsearch/action/get/GetRequest.java +++ b/server/src/main/java/org/elasticsearch/action/get/GetRequest.java @@ -108,6 +108,9 @@ public GetRequest(String index, String id) { @Override public ActionRequestValidationException validate() { ActionRequestValidationException validationException = super.validateNonNullIndex(); + if (Strings.isEmpty(type)) { + validationException = addValidationError("type is missing", validationException); + } if (Strings.isEmpty(id)) { validationException = addValidationError("id is missing", validationException); } diff --git a/server/src/main/java/org/elasticsearch/rest/action/document/RestGetSourceAction.java b/server/src/main/java/org/elasticsearch/rest/action/document/RestGetSourceAction.java index 37d2d0379f2cd..af376bf7c3ccf 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/document/RestGetSourceAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/document/RestGetSourceAction.java @@ -72,7 +72,7 @@ public String getName() { public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { final GetRequest getRequest; if (request.hasParam("type")) { - deprecationLogger.deprecatedAndMaybeLog("get_source_type", TYPES_DEPRECATION_MESSAGE); + 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")); From 1aed03ad25b3d7cdfe9e2c379afa3e285c10b40e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Mon, 17 Dec 2018 20:32:27 +0100 Subject: [PATCH 9/9] another iteration --- .../client/RequestConverters.java | 4 ++-- .../client/RequestConvertersTests.java | 19 ++++++++++++------- .../action/get/GetRequestTests.java | 3 ++- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java index 06764ca02743a..c7a54a9ac32fe 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java @@ -270,8 +270,8 @@ private static Request getStyleRequest(String method, GetRequest getRequest) { static Request sourceExists(GetRequest getRequest) { String optionalType = getRequest.type(); String endpoint; - if (optionalType == null || optionalType.equals(MapperService.SINGLE_MAPPING_NAME)) { - endpoint = endpoint(getRequest.index(), "_source", getRequest.id()); + if (optionalType.equals(MapperService.SINGLE_MAPPING_NAME)) { + endpoint = endpoint(getRequest.index(), "_source", getRequest.id()); } else { endpoint = endpoint(getRequest.index(), optionalType, getRequest.id(), "_source"); } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java index 93dddf51cd6c2..fa0f1c5708c2b 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java @@ -116,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; @@ -158,14 +159,18 @@ public void testGetWithType() { } 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 requestFunction) throws IOException { String index = randomAlphaOfLengthBetween(3, 10); String id = randomAlphaOfLengthBetween(3, 10); - final GetRequest getRequest; - if (randomBoolean()) { - getRequest = new GetRequest(index, id); - } else { - getRequest = new GetRequest(index, randomFrom(MapperService.SINGLE_MAPPING_NAME, null), id); - } + final GetRequest getRequest = requestFunction.apply(index, id); Map expectedParams = new HashMap<>(); if (randomBoolean()) { @@ -195,7 +200,7 @@ public void testSourceExists() throws IOException { Request request = RequestConverters.sourceExists(getRequest); assertEquals(HttpHead.METHOD_NAME, request.getMethod()); String type = getRequest.type(); - if (type == null || type.equals(MapperService.SINGLE_MAPPING_NAME)) { + if (type.equals(MapperService.SINGLE_MAPPING_NAME)) { assertEquals("/" + index + "/_source/" + id, request.getEndpoint()); } else { assertEquals("/" + index + "/" + type + "/" + id + "/_source", request.getEndpoint()); diff --git a/server/src/test/java/org/elasticsearch/action/get/GetRequestTests.java b/server/src/test/java/org/elasticsearch/action/get/GetRequestTests.java index bbd3b23153f3a..499932ccdf026 100644 --- a/server/src/test/java/org/elasticsearch/action/get/GetRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/get/GetRequestTests.java @@ -40,7 +40,8 @@ public void testValidation() { final ActionRequestValidationException validate = request.validate(); assertThat(validate, not(nullValue())); - assertThat(validate.validationErrors(), hasItems("id is missing")); + assertEquals(2, validate.validationErrors().size()); + assertThat(validate.validationErrors(), hasItems("type is missing", "id is missing")); } } }