Skip to content

[Rest Api Compatibility] Typed endpoint for multiget api #73878

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

Merged
merged 6 commits into from
Jun 9, 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
33 changes: 16 additions & 17 deletions rest-api-spec/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,11 @@ tasks.named("yamlRestCompatTest").configure {
'search/160_exists_query/Test exists query on _type field',
//type information is not stored, hence the the index will be found
'termvectors/50_mix_typeless_typeful/Term vectors with typeless API on an index that has types',
// 85 - 13 = 72 tests won't be fixed
// mget - these use cases are no longer valid, because we always default to _doc.
// This mean test cases where there is assertion on not finging by type won't work
'mget/11_default_index_type/Default index/type',
'mget/16_basic_with_types/Basic multi-get',
// 88 - 14 = 74 tests won't be fixed
'cluster.voting_config_exclusions/10_basic/Throw exception when adding voting config exclusion and specifying both node_ids and node_names',
'cluster.voting_config_exclusions/10_basic/Throw exception when adding voting config exclusion without specifying nodes',
'count/11_basic_with_types/count body without query element',
Expand Down Expand Up @@ -169,22 +173,6 @@ tasks.named("yamlRestCompatTest").configure {
'indices.upgrade/10_basic/Upgrade indices disallow no indices',
'indices.upgrade/10_basic/Upgrade indices disallow unavailable',
'indices.upgrade/10_basic/Upgrade indices ignore unavailable',
'mget/10_basic/Basic multi-get',
'mget/11_default_index_type/Default index/type',
'mget/14_alias_to_multiple_indices/Multi Get with alias that resolves to multiple indices',
'mget/16_basic_with_types/Basic multi-get',
'mget/17_default_index/Default index/type',
'mget/18_non_existent_index_with_types/Non-existent index',
'mget/19_missing_metadata_with_types/Missing metadata',
'mget/21_alias_to_multiple_indices_with_types/Multi Get with alias that resolves to multiple indices',
'mget/22_ids_with_types/IDs',
'mget/23_stored_fields_with_types/Stored fields',
'mget/41_routing_with_types/Routing',
'mget/61_realtime_refresh_with_types/Realtime Refresh',
'mget/71_source_filtering_with_types/Source filtering - exclude field',
'mget/71_source_filtering_with_types/Source filtering - include field',
'mget/71_source_filtering_with_types/Source filtering - include nested field',
'mget/71_source_filtering_with_types/Source filtering - true/false',
'mlt/20_docs/Basic mlt query with docs',
'mlt/30_unlike/Basic mlt query with unlike',
'search.aggregation/10_histogram/Deprecated _time order',
Expand Down Expand Up @@ -308,6 +296,17 @@ tasks.named("transformV7RestTests").configure({ task ->
// overrides for indices.get_mapping
task.replaceIsTrue("test_1.mappings.doc", "test_1.mappings._doc")
task.replaceIsTrue("test_2.mappings.doc", "test_2.mappings._doc")
// overrides for mget
task.replaceValueInMatch("docs.0._type", "_doc" , "Basic multi-get") // index found, but no doc
task.replaceValueInMatch("docs.0._type", "_doc", "Default index/type")
task.replaceValueInMatch("docs.0._type", "_doc", "Non-existent index")
task.replaceValueInMatch("docs.0._type", "_doc", "Missing metadata")
task.replaceValueInMatch("docs.0._type", "_doc", "Multi Get with alias that resolves to multiple indices")
task.replaceValueInMatch("docs.1._type", "_doc", "Multi Get with alias that resolves to multiple indices")
task.replaceValueInMatch("docs.2._type", "_doc", "Multi Get with alias that resolves to multiple indices")
task.replaceValueInMatch("docs.0._type", "_doc", "IDs")
task.replaceValueInMatch("docs.1._type", "_doc", "IDs")
task.replaceValueInMatch("docs.2._type", "_doc", "Routing")
})

tasks.register('enforceYamlTestConvention').configure {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,20 @@
import org.elasticsearch.core.Nullable;
import org.elasticsearch.common.xcontent.ParseField;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.core.RestApiVersion;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.lucene.uid.Versions;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentParser.Token;
import org.elasticsearch.index.VersionType;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.rest.action.document.RestMultiGetAction;
import org.elasticsearch.search.fetch.subphase.FetchSourceContext;

import java.io.IOException;
Expand All @@ -43,9 +46,11 @@

public class MultiGetRequest extends ActionRequest
implements Iterable<MultiGetRequest.Item>, CompositeIndicesRequest, RealtimeRequest, ToXContentObject {
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(MultiGetRequest.class);

private static final ParseField DOCS = new ParseField("docs");
private static final ParseField INDEX = new ParseField("_index");
private static final ParseField TYPE = new ParseField("_type");
private static final ParseField ID = new ParseField("_id");
private static final ParseField ROUTING = new ParseField("routing");
private static final ParseField VERSION = new ParseField("version");
Expand Down Expand Up @@ -383,6 +388,9 @@ private static void parseDocuments(XContentParser parser, List<Item> items, @Nul
index = parser.text();
} else if (ID.match(currentFieldName, parser.getDeprecationHandler())) {
id = parser.text();
} else if(parser.getRestApiVersion() == RestApiVersion.V_7 &&
TYPE.match(currentFieldName,parser.getDeprecationHandler())) {
deprecationLogger.compatibleApiWarning("mget_with_types", RestMultiGetAction.TYPES_DEPRECATION_MESSAGE);
} else if (ROUTING.match(currentFieldName, parser.getDeprecationHandler())) {
routing = parser.text();
} else if (FIELDS.match(currentFieldName, parser.getDeprecationHandler())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,18 @@
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionResponse;
import org.elasticsearch.common.xcontent.ParseField;
import org.elasticsearch.core.RestApiVersion;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentParser.Token;
import org.elasticsearch.index.get.GetResult;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.rest.action.document.RestMultiGetAction;

import java.io.IOException;
import java.util.ArrayList;
Expand All @@ -29,8 +32,10 @@
import java.util.List;

public class MultiGetResponse extends ActionResponse implements Iterable<MultiGetItemResponse>, ToXContentObject {
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(MultiGetResponse.class);

private static final ParseField INDEX = new ParseField("_index");
private static final ParseField TYPE = new ParseField("_type");
private static final ParseField ID = new ParseField("_id");
private static final ParseField ERROR = new ParseField("error");
private static final ParseField DOCS = new ParseField("docs");
Expand Down Expand Up @@ -94,6 +99,9 @@ public void writeTo(StreamOutput out) throws IOException {
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
builder.field(INDEX.getPreferredName(), index);
if (builder.getRestApiVersion() == RestApiVersion.V_7) {
builder.field(MapperService.TYPE_FIELD_NAME, MapperService.SINGLE_MAPPING_NAME);
}
builder.field(ID.getPreferredName(), id);
ElasticsearchException.generateFailureXContent(builder, params, exception, true);
builder.endObject();
Expand Down Expand Up @@ -188,6 +196,8 @@ private static MultiGetItemResponse parseItem(XContentParser parser) throws IOEx
case VALUE_STRING:
if (INDEX.match(currentFieldName, parser.getDeprecationHandler())) {
index = parser.text();
} else if (TYPE.match(currentFieldName, parser.getDeprecationHandler())) {
deprecationLogger.compatibleApiWarning("mget_with_types", RestMultiGetAction.TYPES_DEPRECATION_MESSAGE);
} else if (ID.match(currentFieldName, parser.getDeprecationHandler())) {
id = parser.text();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.mapper.IgnoredFieldMapper;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.SourceFieldMapper;
import org.elasticsearch.rest.action.document.RestMultiGetAction;
import org.elasticsearch.search.lookup.SourceLookup;

import java.io.IOException;
Expand All @@ -40,6 +42,7 @@
import static org.elasticsearch.index.seqno.SequenceNumbers.UNASSIGNED_SEQ_NO;

public class GetResult implements Writeable, Iterable<DocumentField>, ToXContentObject {
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(GetResult.class);

public static final String _INDEX = "_index";
public static final String _ID = "_id";
Expand Down Expand Up @@ -323,6 +326,8 @@ public static GetResult fromXContentEmbedded(XContentParser parser, String index
} else if (token.isValue()) {
if (_INDEX.equals(currentFieldName)) {
index = parser.text();
} else if (parser.getRestApiVersion() == RestApiVersion.V_7 && MapperService.TYPE_FIELD_NAME.equals(currentFieldName)) {
deprecationLogger.compatibleApiWarning("mget_with_types", RestMultiGetAction.TYPES_DEPRECATION_MESSAGE);
} else if (_ID.equals(currentFieldName)) {
id = parser.text();
} else if (_VERSION.equals(currentFieldName)) {
Expand Down
3 changes: 2 additions & 1 deletion server/src/main/java/org/elasticsearch/rest/RestRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,8 @@ public final boolean hasContentOrSourceParam() {
*/
public final XContentParser contentOrSourceParamParser() throws IOException {
Tuple<XContentType, BytesReference> tuple = contentOrSourceParam();
return tuple.v1().xContent().createParser(xContentRegistry, LoggingDeprecationHandler.INSTANCE, tuple.v2().streamInput());
return tuple.v1().xContent().createParserForCompatibility(xContentRegistry, LoggingDeprecationHandler.INSTANCE,
tuple.v2().streamInput(), restApiVersion);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import org.elasticsearch.action.get.MultiGetRequest;
import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.core.RestApiVersion;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentParser;
Expand All @@ -25,6 +26,8 @@
import static org.elasticsearch.rest.RestRequest.Method.POST;

public class RestMultiGetAction extends BaseRestHandler {
public static final String TYPES_DEPRECATION_MESSAGE = "[types removal]" +
" Specifying types in multi get requests is deprecated.";

private final boolean allowExplicitIndex;

Expand All @@ -38,7 +41,13 @@ public List<Route> routes() {
new Route(GET, "/_mget"),
new Route(POST, "/_mget"),
new Route(GET, "/{index}/_mget"),
new Route(POST, "/{index}/_mget"));
new Route(POST, "/{index}/_mget"),
Route.builder(GET, "/{index}/{type}/_mget")
.deprecated(TYPES_DEPRECATION_MESSAGE, RestApiVersion.V_7)
.build(),
Route.builder(POST, "/{index}/{type}/_mget")
.deprecated(TYPES_DEPRECATION_MESSAGE, RestApiVersion.V_7)
.build());
}

@Override
Expand All @@ -48,7 +57,9 @@ public String getName() {

@Override
public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {

if (request.getRestApiVersion() == RestApiVersion.V_7 && request.param("type") != null) {
request.param("type");
}
MultiGetRequest multiGetRequest = new MultiGetRequest();
multiGetRequest.refresh(request.paramAsBoolean("refresh", multiGetRequest.refresh()));
multiGetRequest.preference(request.param("preference"));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.rest.action.document;

import org.elasticsearch.action.get.MultiGetRequest;
import org.elasticsearch.action.get.MultiGetResponse;
import org.elasticsearch.core.RestApiVersion;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.test.rest.FakeRestRequest;
import org.elasticsearch.test.rest.RestActionTestCase;
import org.junit.Before;
import org.mockito.Mockito;

import java.util.Collections;
import java.util.List;
import java.util.Map;

import static org.hamcrest.Matchers.instanceOf;

public class RestMultiGetActionTests extends RestActionTestCase {
XContentType VND_TYPE = randomVendorType();
List<String> contentTypeHeader = Collections.singletonList(compatibleMediaType(VND_TYPE, RestApiVersion.V_7));

@Before
public void setUpAction() {
controller().registerHandler(new RestMultiGetAction(Settings.EMPTY));
verifyingClient.setExecuteVerifier((actionType, request) -> {
assertThat(request, instanceOf(MultiGetRequest.class));
return Mockito.mock(MultiGetResponse.class);
});
}
public void testTypeInPath() {
RestRequest deprecatedRequest = new FakeRestRequest.Builder(xContentRegistry())
.withHeaders( Map.of("Content-Type", contentTypeHeader, "Accept", contentTypeHeader))
.withMethod(RestRequest.Method.GET)
.withPath("some_index/some_type/_mget")
.build();
dispatchRequest(deprecatedRequest);
assertWarnings(RestMultiGetAction.TYPES_DEPRECATION_MESSAGE);
}

public void testTypeInBody() throws Exception {
XContentBuilder content = XContentFactory.contentBuilder(VND_TYPE).startObject()
.startArray("docs")
.startObject()
.field("_index", "some_index")
.field("_type", "_doc")
.field("_id", "2")
.endObject()
.startObject()
.field("_index", "test")
.field("_id", "2")
.endObject()
.endArray()
.endObject();

RestRequest request = new FakeRestRequest.Builder(xContentRegistry())
.withPath("_mget")
.withHeaders( Map.of("Content-Type", contentTypeHeader, "Accept", contentTypeHeader))
.withContent(BytesReference.bytes(content), null)
.build();
dispatchRequest(request);
assertWarnings(RestMultiGetAction.TYPES_DEPRECATION_MESSAGE);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -1157,6 +1157,10 @@ public String compatibleMediaType(XContentType type, RestApiVersion version) {
.responseContentTypeHeader(Map.of(MediaType.COMPATIBLE_WITH_PARAMETER_NAME, String.valueOf(version.major)));
}

public XContentType randomVendorType() {
return randomFrom(XContentType.VND_JSON, XContentType.VND_SMILE, XContentType.VND_CBOR, XContentType.VND_YAML);
}

public static class GeohashGenerator extends CodepointSetGenerator {
private static final char[] ASCII_SET = "0123456789bcdefghjkmnpqrstuvwxyz".toCharArray();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,6 @@ protected void doAssert(Object actualValue, Object expectedValue) {
assertThat(actualValue, instanceOf(List.class));
assertMap((List<?>) actualValue, matchesList((List<?>) expectedValue));
}
assertThat(expectedValue, equalTo(actualValue));
assertThat(actualValue, equalTo(expectedValue));
}
}