From a8f50167f35ef1ad5b711e45dd605eb0e7f4d397 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Wed, 9 Jun 2021 16:16:36 +0200 Subject: [PATCH 1/3] [Rest Api Compatibility] Typed endpoints for get_source api retrofits typed get_source api removed in #46931 and #46587 relates #51816 --- rest-api-spec/build.gradle | 18 +++--- .../action/document/RestGetSourceAction.java | 15 ++++- .../document/RestGetSourceActionTests.java | 56 +++++++++++++++++++ 3 files changed, 79 insertions(+), 10 deletions(-) diff --git a/rest-api-spec/build.gradle b/rest-api-spec/build.gradle index 73a9dd8fcdc8c..8053fc5b695bf 100644 --- a/rest-api-spec/build.gradle +++ b/rest-api-spec/build.gradle @@ -140,15 +140,15 @@ tasks.named("yamlRestCompatTest").configure { 'explain/31_query_string_with_types/explain with query_string parameters', 'explain/40_mix_typeless_typeful/Explain with typeless API on an index that has types', 'field_caps/30_filter/Field caps with index filter', - 'get_source/11_basic_with_types/Basic with types', - 'get_source/16_default_values_with_types/Default values', - 'get_source/41_routing_with_types/Routing', - 'get_source/61_realtime_refresh_with_types/Realtime', - 'get_source/71_source_filtering_with_types/Source filtering', - 'get_source/81_missing_with_types/Missing document with catch', - 'get_source/81_missing_with_types/Missing document with ignore', - 'get_source/86_source_missing_with_types/Missing document source with catch', - 'get_source/86_source_missing_with_types/Missing document source with ignore', +// 'get_source/11_basic_with_types/Basic with types', +// 'get_source/16_default_values_with_types/Default values', +// 'get_source/41_routing_with_types/Routing', +// 'get_source/61_realtime_refresh_with_types/Realtime', +// 'get_source/71_source_filtering_with_types/Source filtering', +// 'get_source/81_missing_with_types/Missing document with catch', +// 'get_source/81_missing_with_types/Missing document with ignore', +// 'get_source/86_source_missing_with_types/Missing document source with catch', +// 'get_source/86_source_missing_with_types/Missing document source with ignore', 'indices.create/10_basic/Create index without soft deletes', 'indices.flush/10_basic/Index synced flush rest test', 'indices.forcemerge/10_basic/Check deprecation warning when incompatible only_expunge_deletes and max_num_segments values are both set', 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 94f293ffc852f..2db782cebe026 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 @@ -16,6 +16,7 @@ import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.BytesRestResponse; import org.elasticsearch.rest.RestChannel; @@ -36,12 +37,20 @@ * The REST handler for get source and head source APIs. */ public class RestGetSourceAction extends BaseRestHandler { + static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Specifying types in get_source and exist_source" + + "requests is deprecated."; @Override public List routes() { return List.of( new Route(GET, "/{index}/_source/{id}"), - new Route(HEAD, "/{index}/_source/{id}")); + new Route(HEAD, "/{index}/_source/{id}"), + Route.builder(GET, "/{index}/{type}/{id}/_source") + .deprecated(TYPES_DEPRECATION_MESSAGE, RestApiVersion.V_7) + .build(), + Route.builder(HEAD, "/{index}/{type}/{id}/_source") + .deprecated(TYPES_DEPRECATION_MESSAGE, RestApiVersion.V_7) + .build()); } @Override @@ -51,6 +60,10 @@ public String getName() { @Override public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { + if (request.getRestApiVersion() == RestApiVersion.V_7 && request.hasParam("type")) { + request.param("type"); // consume and ignore the type + } + final GetRequest getRequest = new GetRequest(request.param("index"), request.param("id")); getRequest.refresh(request.paramAsBoolean("refresh", getRequest.refresh())); getRequest.routing(request.param("routing")); 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 9d8cfb5ad60e0..260a28b2b569f 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 @@ -9,9 +9,11 @@ package org.elasticsearch.rest.action.document; import org.elasticsearch.ResourceNotFoundException; +import org.elasticsearch.action.get.GetRequest; import org.elasticsearch.action.get.GetResponse; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.index.get.GetResult; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestResponse; @@ -21,21 +23,32 @@ import org.elasticsearch.test.rest.RestActionTestCase; import org.junit.AfterClass; import org.junit.Before; +import org.mockito.Mockito; + +import java.util.Collections; +import java.util.List; +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.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; 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 final List compatibleMediaType = Collections.singletonList(randomCompatibleMediaType(RestApiVersion.V_7)); @Before public void setUpAction() { controller().registerHandler(new RestGetSourceAction()); + verifyingClient.setExecuteVerifier((actionType, request) -> { + assertThat(request, instanceOf(GetRequest.class)); + return Mockito.mock(GetResponse.class); + }); } @AfterClass @@ -44,6 +57,7 @@ public static void cleanupReferences() { channel = null; listener = null; } + public void testRestGetSourceAction() throws Exception { final BytesReference source = new BytesArray("{\"foo\": \"bar\"}"); final GetResponse response = @@ -73,4 +87,46 @@ public void testRestGetSourceActionWithMissingDocumentSource() { assertThat(exception.getMessage(), equalTo("Source not found [index1]/[1]")); } + + /** + * test deprecation is logged if type is used in path + */ + public void testTypeInGetPath() { + RestRequest request = new FakeRestRequest.Builder(xContentRegistry()) + .withHeaders(Map.of("Accept", compatibleMediaType)) + .withMethod(RestRequest.Method.HEAD) + .withPath("/some_index/some_type/id/_source") + .build(); + dispatchRequest(request); + assertWarnings(RestGetSourceAction.TYPES_DEPRECATION_MESSAGE); + } + + public void testTypeInHeadPath() { + RestRequest request = new FakeRestRequest.Builder(xContentRegistry()) + .withHeaders(Map.of("Accept", compatibleMediaType)) + .withMethod(RestRequest.Method.GET) + .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 + */ + //TODO is this a real scenario? to use ?type instead of /index/type +// public void testTypeParameter() { +// Map params = new HashMap<>(); +// params.put("type", "some_type"); +// for (RestRequest.Method method : Arrays.asList(RestRequest.Method.GET, RestRequest.Method.HEAD)) { +// RestRequest request = new FakeRestRequest.Builder(xContentRegistry()) +// .withHeaders(Map.of("Accept", compatibleMediaType)) +// .withMethod(method) +// .withPath("/some_index/_source/id") +// .withParams(params) +// .build(); +// dispatchRequest(request); +// assertWarnings(RestGetSourceAction.TYPES_DEPRECATION_MESSAGE); +// } +// } } From b147ff141c52adcc4a8524d8ee9e0bba918f8cf8 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Mon, 14 Jun 2021 13:45:13 +0200 Subject: [PATCH 2/3] ?type support --- .../action/document/RestGetSourceAction.java | 5 ++- .../document/RestGetSourceActionTests.java | 43 ++++++++++++------- 2 files changed, 32 insertions(+), 16 deletions(-) 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 2db782cebe026..7c7c447652aaf 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 @@ -14,6 +14,7 @@ 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.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.core.RestApiVersion; @@ -37,7 +38,8 @@ * The REST handler for get source and head source APIs. */ public class RestGetSourceAction extends BaseRestHandler { - static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Specifying types in get_source and exist_source" + private final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestGetSourceAction.class); + static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Specifying types in get_source and exist_source " + "requests is deprecated."; @Override @@ -62,6 +64,7 @@ public String getName() { public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { if (request.getRestApiVersion() == RestApiVersion.V_7 && request.hasParam("type")) { request.param("type"); // consume and ignore the type + deprecationLogger.compatibleApiWarning("get_source_with_types", TYPES_DEPRECATION_MESSAGE); } final GetRequest getRequest = new GetRequest(request.param("index"), request.param("id")); 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 260a28b2b569f..273311408b8e6 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 @@ -25,7 +25,9 @@ import org.junit.Before; import org.mockito.Mockito; +import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -114,19 +116,30 @@ public void testTypeInHeadPath() { /** * test deprecation is logged if type is used as parameter */ - //TODO is this a real scenario? to use ?type instead of /index/type -// public void testTypeParameter() { -// Map params = new HashMap<>(); -// params.put("type", "some_type"); -// for (RestRequest.Method method : Arrays.asList(RestRequest.Method.GET, RestRequest.Method.HEAD)) { -// RestRequest request = new FakeRestRequest.Builder(xContentRegistry()) -// .withHeaders(Map.of("Accept", compatibleMediaType)) -// .withMethod(method) -// .withPath("/some_index/_source/id") -// .withParams(params) -// .build(); -// dispatchRequest(request); -// assertWarnings(RestGetSourceAction.TYPES_DEPRECATION_MESSAGE); -// } -// } + public void testTypeParameterAndGet() { + Map params = new HashMap<>(); + params.put("type", "some_type"); + RestRequest request = new FakeRestRequest.Builder(xContentRegistry()) + .withHeaders(Map.of("Accept", compatibleMediaType)) + .withMethod(RestRequest.Method.GET) + .withPath("/some_index/_source/id") + .withParams(params) + .build(); + dispatchRequest(request); + assertWarnings(RestGetSourceAction.TYPES_DEPRECATION_MESSAGE); + } + + public void testTypeParameterAndHead() { + Map params = new HashMap<>(); + params.put("type", "some_type"); + RestRequest request = new FakeRestRequest.Builder(xContentRegistry()) + .withHeaders(Map.of("Accept", compatibleMediaType)) + .withMethod(RestRequest.Method.HEAD) + .withPath("/some_index/_source/id") + .withParams(params) + .build(); + dispatchRequest(request); + assertWarnings(RestGetSourceAction.TYPES_DEPRECATION_MESSAGE); + } + } From fd6dfe354415fe8616c1d72ae8b4e47b64b8f8aa Mon Sep 17 00:00:00 2001 From: pgomulka Date: Mon, 14 Jun 2021 14:09:18 +0200 Subject: [PATCH 3/3] checkstyle --- .../rest/action/document/RestGetSourceActionTests.java | 1 - 1 file changed, 1 deletion(-) 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 273311408b8e6..bec7259f92e4f 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 @@ -25,7 +25,6 @@ import org.junit.Before; import org.mockito.Mockito; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List;