Skip to content

Commit

Permalink
[Rest Api Compatibility] Typed endpoints for get_source api
Browse files Browse the repository at this point in the history 
retrofits typed get_source api removed in elastic#46931 and elastic#46587
relates elastic#51816
  • Loading branch information
@pgomulka
pgomulka committed Jun 9, 2021
1 parent 4598b46 commit a8f5016
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 10 deletions.
18 changes: 9 additions & 9 deletions rest-api-spec/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
15 changes: 14 additions & 1 deletion server/src/main/java/org/elasticsearch/rest/action/document/RestGetSourceAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<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 +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"));
Expand Down
56 changes: 56 additions & 0 deletions server/src/test/java/org/elasticsearch/rest/action/document/RestGetSourceActionTests.java
View file
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,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<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 +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 =
Expand Down Expand Up @@ -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<String, String> 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);
// }
// }
}

0 comments on commit a8f5016

Please sign in to comment.