Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Rest Api Compatibility] Typed endpoints for get_source api #73957

Merged
merged 5 commits into from
Jun 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions rest-api-spec/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -134,15 +134,6 @@ tasks.named("yamlRestCompatTest").configure {
'count/11_basic_with_types/count with body',
'count/11_basic_with_types/count with empty body',
'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',
'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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@
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;
import org.elasticsearch.rest.BaseRestHandler;
import org.elasticsearch.rest.BytesRestResponse;
import org.elasticsearch.rest.RestChannel;
Expand All @@ -36,12 +38,21 @@
* The REST handler for get source and head source APIs.
*/
public class RestGetSourceAction extends BaseRestHandler {
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
public List<Route> 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
Expand All @@ -51,6 +62,11 @@ 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
deprecationLogger.compatibleApiWarning("get_source_with_types", TYPES_DEPRECATION_MESSAGE);
}

final GetRequest getRequest = new GetRequest(request.param("index"), request.param("id"));
getRequest.refresh(request.paramAsBoolean("refresh", getRequest.refresh()));
getRequest.routing(request.param("routing"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -21,21 +23,33 @@
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.HashMap;
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<String> 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
Expand All @@ -44,6 +58,7 @@ public static void cleanupReferences() {
channel = null;
listener = null;
}

public void testRestGetSourceAction() throws Exception {
final BytesReference source = new BytesArray("{\"foo\": \"bar\"}");
final GetResponse response =
Expand Down Expand Up @@ -73,4 +88,57 @@ 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
*/
public void testTypeParameterAndGet() {
Map<String, String> 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<String, String> 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);
}

}