Browse Source

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

Retrofits typed api for M-get api removed in #46587

relates #51816
Przemyslaw Gomulka 4 years ago
parent
commit
4598b46e7f

+ 16 - 17
rest-api-spec/build.gradle

@@ -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',
@@ -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',
@@ -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 {

+ 8 - 0
server/src/main/java/org/elasticsearch/action/get/MultiGetRequest.java

@@ -20,10 +20,12 @@ import org.elasticsearch.action.support.IndicesOptions;
 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;
@@ -31,6 +33,7 @@ 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;
@@ -43,9 +46,11 @@ import java.util.Locale;
 
 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");
@@ -383,6 +388,9 @@ public class MultiGetRequest extends ActionRequest
                         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())) {

+ 10 - 0
server/src/main/java/org/elasticsearch/action/get/MultiGetResponse.java

@@ -12,15 +12,18 @@ import org.elasticsearch.ElasticsearchException;
 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;
@@ -29,8 +32,10 @@ import java.util.Iterator;
 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");
@@ -94,6 +99,9 @@ public class MultiGetResponse extends ActionResponse implements Iterable<MultiGe
         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();
@@ -188,6 +196,8 @@ public class MultiGetResponse extends ActionResponse implements Iterable<MultiGe
                 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();
                     }

+ 5 - 0
server/src/main/java/org/elasticsearch/index/get/GetResult.java

@@ -18,6 +18,7 @@ import org.elasticsearch.common.document.DocumentField;
 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;
@@ -25,6 +26,7 @@ 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;
@@ -40,6 +42,7 @@ import static org.elasticsearch.index.seqno.SequenceNumbers.UNASSIGNED_PRIMARY_T
 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";
@@ -323,6 +326,8 @@ public class GetResult implements Writeable, Iterable<DocumentField>, ToXContent
             } 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)) {

+ 2 - 1
server/src/main/java/org/elasticsearch/rest/RestRequest.java

@@ -482,7 +482,8 @@ public class RestRequest implements ToXContent.Params {
      */
     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);
     }
 
     /**

+ 13 - 2
server/src/main/java/org/elasticsearch/rest/action/document/RestMultiGetAction.java

@@ -10,6 +10,7 @@ package org.elasticsearch.rest.action.document;
 
 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;
@@ -25,6 +26,8 @@ import static org.elasticsearch.rest.RestRequest.Method.GET;
 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;
 
@@ -38,7 +41,13 @@ public class RestMultiGetAction extends BaseRestHandler {
             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
@@ -48,7 +57,9 @@ public class RestMultiGetAction extends BaseRestHandler {
 
     @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"));

+ 77 - 0
server/src/test/java/org/elasticsearch/rest/action/document/RestMultiGetActionTests.java

@@ -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);
+    }
+
+}

+ 4 - 0
test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java

@@ -1157,6 +1157,10 @@ public abstract class ESTestCase extends LuceneTestCase {
             .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();
 

+ 1 - 1
test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/MatchAssertion.java

@@ -88,6 +88,6 @@ public class MatchAssertion extends Assertion {
             assertThat(actualValue, instanceOf(List.class));
             assertMap((List<?>) actualValue, matchesList((List<?>) expectedValue));
         }
-        assertThat(expectedValue, equalTo(actualValue));
+        assertThat(actualValue, equalTo(expectedValue));
     }
 }