Allow indices.get_mapping response parsing without types (#37492)

This change adds deprecation warning to the indices.get_mapping API in case the
"inlcude_type_name" parameter is set to "true" and changes the parsing code in
GetMappingsResponse to parse the type-less response instead of the one
containing types. As a consequence the HLRC client doesn't need to force
"include_type_name=true" any more and the GetMappingsResponseTests can be
adapted to the new format as well. Also removing some "include_type_name"
parameters in yaml test and docs where not necessary.

client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesRequestConverters.java

@@ -150,7 +150,6 @@ final class IndicesRequestConverters {
         parameters.withMasterTimeout(getMappingsRequest.masterNodeTimeout());
         parameters.withIndicesOptions(getMappingsRequest.indicesOptions());
         parameters.withLocal(getMappingsRequest.local());
-        parameters.putParam(INCLUDE_TYPE_NAME_PARAMETER, "true");
 
         return request;
     }

client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java

@@ -443,9 +443,7 @@ public class IndicesClientIT extends ESRestHighLevelClientTestCase {
         Map<String, Object> getIndexResponse = getAsMap(indexName);
         assertEquals("text", XContentMapValues.extractValue(indexName + ".mappings.properties.field.type", getIndexResponse));
 
-        GetMappingsRequest request = new GetMappingsRequest()
-            .indices(indexName)
-            .types("_doc");
+        GetMappingsRequest request = new GetMappingsRequest().indices(indexName);
 
         GetMappingsResponse getMappingsResponse =
             execute(request, highLevelClient().indices()::getMapping, highLevelClient().indices()::getMappingAsync);

client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesRequestConvertersTests.java

@@ -217,7 +217,6 @@ public class IndicesRequestConvertersTests extends ESTestCase {
             getMappingRequest::indicesOptions, expectedParams);
         RequestConvertersTests.setRandomMasterTimeout(getMappingRequest, expectedParams);
         RequestConvertersTests.setRandomLocal(getMappingRequest, expectedParams);
-        expectedParams.put(INCLUDE_TYPE_NAME_PARAMETER, "true");
 
         Request request = IndicesRequestConverters.getMappings(getMappingRequest);
         StringJoiner endpoint = new StringJoiner("/", "/", "");

client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java

@@ -609,7 +609,6 @@ public class IndicesClientDocumentationIT extends ESRestHighLevelClientTestCase
             // tag::get-mappings-request
             GetMappingsRequest request = new GetMappingsRequest(); // <1>
             request.indices("twitter"); // <2>
-            request.types("_doc"); // <3>
             // end::get-mappings-request
 
             // tag::get-mappings-request-masterTimeout
@@ -665,7 +664,6 @@ public class IndicesClientDocumentationIT extends ESRestHighLevelClientTestCase
         {
             GetMappingsRequest request = new GetMappingsRequest();
             request.indices("twitter");
-            request.types("_doc");
 
             // tag::get-mappings-execute-listener
             ActionListener<GetMappingsResponse> listener =

docs/java-rest/high-level/indices/get_mappings.asciidoc

@@ -18,7 +18,6 @@ include-tagged::{doc-tests-file}[{api}-request]
 --------------------------------------------------
 <1> An empty request that will return all indices and types
 <2> Setting the indices to fetch mapping for
-<3> The types to be returned
 
 ==== Optional arguments
 The following arguments can also optionally be provided:

rest-api-spec/src/main/resources/rest-api-spec/api/indices.get_mapping.json

@@ -18,7 +18,7 @@
       "params": {
         "include_type_name": {
           "type" : "boolean",
-          "description" : "Whether to add the type name to the response"
+          "description" : "Whether to add the type name to the response (default: false)"
         },
         "ignore_unavailable": {
             "type" : "boolean",

rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_mapping/10_basic.yml

@@ -5,13 +5,11 @@ setup:
       reason: include_type_name defaults to true before 7.0
   - do:
         indices.create:
-          include_type_name: false
           index: test_1
           body:
             mappings: {}
   - do:
         indices.create:
-          include_type_name: false
           index: test_2
           body:
             mappings: {}

rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_mapping/60_empty.yml

@@ -1,8 +1,5 @@
 ---
 setup:
-  - skip:
-      version: " - 6.99.99"
-      reason: include_type_name defaults to true before 7.0
   - do:
       indices.create:
         index: test_1

server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetMappingsResponse.java

@@ -31,6 +31,7 @@ import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.ToXContentFragment;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.rest.BaseRestHandler;
 
 import java.io.IOException;
@@ -101,22 +102,17 @@ public class GetMappingsResponse extends ActionResponse implements ToXContentFra
         for (Map.Entry<String, Object> entry : parts.entrySet()) {
             final String indexName = entry.getKey();
             assert entry.getValue() instanceof Map : "expected a map as type mapping, but got: " + entry.getValue().getClass();
-            @SuppressWarnings("unchecked")
-            final Map<String, Object> mapping = (Map<String, Object>) ((Map<String, ?>) entry.getValue()).get(MAPPINGS.getPreferredName());
-
             ImmutableOpenMap.Builder<String, MappingMetaData> typeBuilder = new ImmutableOpenMap.Builder<>();
-            for (Map.Entry<String, Object> typeEntry : mapping.entrySet()) {
-                final String typeName = typeEntry.getKey();
-                assert typeEntry.getValue() instanceof Map : "expected a map as inner type mapping, but got: " +
-                    typeEntry.getValue().getClass();
-                @SuppressWarnings("unchecked")
-                final Map<String, Object> fieldMappings = (Map<String, Object>) typeEntry.getValue();
-                MappingMetaData mmd = new MappingMetaData(typeName, fieldMappings);
-                typeBuilder.put(typeName, mmd);
+            @SuppressWarnings("unchecked")
+            final Map<String, Object> fieldMappings = (Map<String, Object>) ((Map<String, ?>) entry.getValue())
+                    .get(MAPPINGS.getPreferredName());
+            if (fieldMappings.isEmpty() == false) {
+                assert fieldMappings instanceof Map : "expected a map as inner type mapping, but got: " + fieldMappings.getClass();
+                MappingMetaData mmd = new MappingMetaData(MapperService.SINGLE_MAPPING_NAME, fieldMappings);
+                typeBuilder.put(MapperService.SINGLE_MAPPING_NAME, mmd);
             }
             builder.put(indexName, typeBuilder.build());
         }
-
         return new GetMappingsResponse(builder.build());
     }
 

server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetMappingAction.java

@@ -20,6 +20,7 @@
 package org.elasticsearch.rest.action.admin.indices;
 
 import com.carrotsearch.hppc.cursors.ObjectCursor;
+
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsRequest;
@@ -59,6 +60,8 @@ import static org.elasticsearch.rest.RestRequest.Method.HEAD;
 public class RestGetMappingAction extends BaseRestHandler {
     private static final Logger logger = LogManager.getLogger(RestGetMappingAction.class);
     private static final DeprecationLogger deprecationLogger = new DeprecationLogger(logger);
+    static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Using include_type_name in get mapping requests is deprecated. "
+            + "The parameter will be removed in the next major version.";
 
     public RestGetMappingAction(final Settings settings, final RestController controller) {
         super(settings);
@@ -90,6 +93,9 @@ public class RestGetMappingAction extends BaseRestHandler {
             throw new IllegalArgumentException("Types cannot be provided in get mapping requests, unless" +
                 " include_type_name is set to true.");
         }
+        if (request.hasParam(INCLUDE_TYPE_NAME_PARAMETER)) {
+            deprecationLogger.deprecatedAndMaybeLog("get_mapping_with_types", TYPES_DEPRECATION_MESSAGE);
+        }
 
         final GetMappingsRequest getMappingsRequest = new GetMappingsRequest();
         getMappingsRequest.indices(indices).types(types);

server/src/test/java/org/elasticsearch/action/admin/indices/mapping/get/GetMappingsResponseTests.java

@@ -24,8 +24,11 @@ import com.carrotsearch.hppc.cursors.ObjectCursor;
 import org.elasticsearch.cluster.metadata.MappingMetaData;
 import org.elasticsearch.common.collect.ImmutableOpenMap;
 import org.elasticsearch.common.xcontent.ToXContent;
+import org.elasticsearch.common.xcontent.ToXContent.Params;
 import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.mapper.MapperService;
+import org.elasticsearch.rest.BaseRestHandler;
 import org.elasticsearch.test.AbstractStreamableXContentTestCase;
 import org.elasticsearch.test.EqualsHashCodeTestUtils;
 
@@ -38,7 +41,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 
-import static org.elasticsearch.rest.BaseRestHandler.INCLUDE_TYPE_NAME_PARAMETER;
+import static org.elasticsearch.test.AbstractXContentTestCase.xContentTester;
 
 public class GetMappingsResponseTests extends AbstractStreamableXContentTestCase<GetMappingsResponse> {
 
@@ -86,12 +89,6 @@ public class GetMappingsResponseTests extends AbstractStreamableXContentTestCase
         return mutate(instance);
     }
 
-    public static ImmutableOpenMap<String, MappingMetaData> createMappingsForIndex() {
-        // rarely have no types
-        int typeCount = rarely() ? 0 : scaledRandomIntBetween(1, 3);
-        return createMappingsForIndex(typeCount, true);
-    }
-
     public static ImmutableOpenMap<String, MappingMetaData> createMappingsForIndex(int typeCount, boolean randomTypeName) {
         List<MappingMetaData> typeMappings = new ArrayList<>(typeCount);
 
@@ -122,22 +119,18 @@ public class GetMappingsResponseTests extends AbstractStreamableXContentTestCase
 
     @Override
     protected GetMappingsResponse createTestInstance() {
+        return createTestInstance(true);
+    }
+
+    private GetMappingsResponse createTestInstance(boolean randomTypeNames) {
         ImmutableOpenMap.Builder<String, ImmutableOpenMap<String, MappingMetaData>> indexBuilder = ImmutableOpenMap.builder();
-        indexBuilder.put("index-" + randomAlphaOfLength(5), createMappingsForIndex());
+        int typeCount = rarely() ? 0 : 1;
+        indexBuilder.put("index-" + randomAlphaOfLength(5), createMappingsForIndex(typeCount, randomTypeNames));
         GetMappingsResponse resp = new GetMappingsResponse(indexBuilder.build());
         logger.debug("--> created: {}", resp);
         return resp;
     }
 
-    /**
-     * For now, we only unit test the legacy typed responses. This will soon no longer be the
-     * case, as we introduce support for typeless xContent parsing in {@link GetMappingsResponse}.
-     */
-    @Override
-    protected ToXContent.Params getToXContentParams() {
-        return new ToXContent.MapParams(Collections.singletonMap(INCLUDE_TYPE_NAME_PARAMETER, "true"));
-    }
-
     // Not meant to be exhaustive
     private static Map<String, Object> randomFieldMapping() {
         Map<String, Object> mappings = new HashMap<>();
@@ -170,4 +163,57 @@ public class GetMappingsResponseTests extends AbstractStreamableXContentTestCase
         }
         return mappings;
     }
+
+    @Override
+    protected GetMappingsResponse createXContextTestInstance(XContentType xContentType) {
+        // don't use random type names for XContent roundtrip tests because we cannot parse them back anymore
+        return createTestInstance(false);
+    }
+
+    /**
+     * check that the "old" legacy response format with types works as expected
+     */
+    public void testToXContentWithTypes() throws IOException {
+        Params params = new ToXContent.MapParams(Collections.singletonMap(BaseRestHandler.INCLUDE_TYPE_NAME_PARAMETER, "true"));
+        xContentTester(this::createParser, t -> createTestInstance(), params, this::fromXContentWithTypes)
+                .numberOfTestRuns(NUMBER_OF_TEST_RUNS)
+                .supportsUnknownFields(supportsUnknownFields())
+                .shuffleFieldsExceptions(getShuffleFieldsExceptions())
+                .randomFieldsExcludeFilter(getRandomFieldsExcludeFilter())
+                .assertEqualsConsumer(this::assertEqualInstances)
+                .assertToXContentEquivalence(true)
+                .test();
+    }
+
+    /**
+     * including the pre-7.0 parsing code here to test that older HLRC clients using this can parse the responses that are
+     * returned when "include_type_name=true"
+     */
+    private GetMappingsResponse fromXContentWithTypes(XContentParser parser) throws IOException {
+        if (parser.currentToken() == null) {
+            parser.nextToken();
+        }
+        assert parser.currentToken() == XContentParser.Token.START_OBJECT;
+        Map<String, Object> parts = parser.map();
+
+        ImmutableOpenMap.Builder<String, ImmutableOpenMap<String, MappingMetaData>> builder = new ImmutableOpenMap.Builder<>();
+        for (Map.Entry<String, Object> entry : parts.entrySet()) {
+            final String indexName = entry.getKey();
+            assert entry.getValue() instanceof Map : "expected a map as type mapping, but got: " + entry.getValue().getClass();
+            final Map<String, Object> mapping = (Map<String, Object>) ((Map) entry.getValue()).get("mappings");
+
+            ImmutableOpenMap.Builder<String, MappingMetaData> typeBuilder = new ImmutableOpenMap.Builder<>();
+            for (Map.Entry<String, Object> typeEntry : mapping.entrySet()) {
+                final String typeName = typeEntry.getKey();
+                assert typeEntry.getValue() instanceof Map : "expected a map as inner type mapping, but got: "
+                        + typeEntry.getValue().getClass();
+                final Map<String, Object> fieldMappings = (Map<String, Object>) typeEntry.getValue();
+                MappingMetaData mmd = new MappingMetaData(typeName, fieldMappings);
+                typeBuilder.put(typeName, mmd);
+            }
+            builder.put(indexName, typeBuilder.build());
+        }
+
+        return new GetMappingsResponse(builder.build());
+    }
 }

server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestGetMappingActionTests.java

@@ -28,6 +28,7 @@ import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.test.rest.FakeRestChannel;
 import org.elasticsearch.test.rest.FakeRestRequest;
 import org.elasticsearch.test.rest.RestActionTestCase;
+import org.junit.Before;
 
 import java.util.HashMap;
 import java.util.Map;
@@ -37,6 +38,11 @@ import static org.mockito.Mockito.mock;
 
 public class RestGetMappingActionTests extends RestActionTestCase {
 
+    @Before
+    public void setUpAction() {
+        new RestGetMappingAction(Settings.EMPTY, controller());
+    }
+
     public void testTypeExistsDeprecation() throws Exception {
         Map<String, String> params = new HashMap<>();
         params.put("type", "_doc");
@@ -69,4 +75,24 @@ public class RestGetMappingActionTests extends RestActionTestCase {
         assertEquals(1, channel.errors().get());
         assertEquals(RestStatus.BAD_REQUEST, channel.capturedResponse().status());
     }
+
+    /**
+     * Setting "include_type_name" to true or false should cause a deprecation warning starting in 7.0
+     */
+    public void testTypeUrlParameterDeprecation() throws Exception {
+        Map<String, String> params = new HashMap<>();
+        params.put(INCLUDE_TYPE_NAME_PARAMETER, Boolean.toString(randomBoolean()));
+        RestRequest request = new FakeRestRequest.Builder(xContentRegistry())
+            .withMethod(RestRequest.Method.GET)
+            .withParams(params)
+            .withPath("/some_index/_mappings")
+            .build();
+
+        FakeRestChannel channel = new FakeRestChannel(request, false, 1);
+        ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
+        controller().dispatchRequest(request, channel, threadContext);
+
+        assertWarnings(RestGetMappingAction.TYPES_DEPRECATION_MESSAGE);
+    }
+
 }