Compatible logic for include_type_param and RestCreateIndexAction (#5…

…4197)

Refactoring of the compatible infrastructure to allow registering multiple RestAction under the same path. It only extends the current mechanism which allowed registering RestAction under the same path but with different method. Now it also uses a version together with a method to find a matching RestAction

This PR also provides a V7 compatible RestCreateIndexAction that needs include_Type_param and a different logic for parsing mapping from its body.

fixed get/index tests
CompatRestIT. test {yaml=get/21_stored_fields_with_types/Stored fields}
CompatRestIT. test {yaml=get/71_source_filtering_with_types/Source
filtering}

CompatRestIT. test {yaml=index/70_mix_typeless_typeful/Index call that
introduces new field mappings}
CompatRestIT. test {yaml=index/70_mix_typeless_typeful/Index with
typeless API on an index that has types}

however the last one from get is still failing
CompatRestIT. test {yaml=get/100_mix_typeless_typeful/GET with typeless
API on an index that has

relates #54160

modules/rest-compatibility/src/main/java/org/elasticsearch/rest/compat/RestCompatPlugin.java

@@ -30,6 +30,7 @@
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.rest.RestController;
 import org.elasticsearch.rest.RestHandler;
+import org.elasticsearch.rest.compat.version7.RestCreateIndexActionV7;
 import org.elasticsearch.rest.compat.version7.RestGetActionV7;
 import org.elasticsearch.rest.compat.version7.RestIndexActionV7;
 
@@ -54,7 +55,8 @@ public List<RestHandler> getRestHandlers(
                 new RestGetActionV7(),
                 new RestIndexActionV7.CompatibleRestIndexAction(),
                 new RestIndexActionV7.CompatibleCreateHandler(),
-                new RestIndexActionV7.CompatibleAutoIdHandler(nodesInCluster)
+                new RestIndexActionV7.CompatibleAutoIdHandler(nodesInCluster),
+                new RestCreateIndexActionV7()
             );
         }
         return Collections.emptyList();

modules/rest-compatibility/src/main/java/org/elasticsearch/rest/compat/version7/RestCreateIndexActionV7.java

@@ -0,0 +1,119 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.rest.compat.version7;
+
+import org.elasticsearch.Version;
+import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
+import org.elasticsearch.action.support.ActiveShardCount;
+import org.elasticsearch.client.node.NodeClient;
+import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
+import org.elasticsearch.common.xcontent.XContentHelper;
+import org.elasticsearch.index.mapper.MapperService;
+import org.elasticsearch.rest.RestRequest;
+import org.elasticsearch.rest.action.RestToXContentListener;
+import org.elasticsearch.rest.action.admin.indices.RestCreateIndexAction;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import static java.util.Collections.singletonMap;
+
+public class RestCreateIndexActionV7 extends RestCreateIndexAction {
+
+    /**
+     * Parameter that controls whether certain REST apis should include type names in their requests.
+     */
+    public static final String INCLUDE_TYPE_NAME_PARAMETER = "include_type_name";
+
+    @Override
+    public String getName() {
+        return "create_index_action_v7";
+    }
+
+    @Override
+    public Version compatibleWithVersion() {
+        return Version.V_7_0_0;
+    }
+
+    @Override
+    public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
+        CreateIndexRequest createIndexRequest = prepareRequest(request);
+        return channel -> client.admin().indices().create(createIndexRequest, new RestToXContentListener<>(channel));
+    }
+
+    // default scope for testing
+    CreateIndexRequest prepareRequest(RestRequest request) {
+        CreateIndexRequest createIndexRequest = new CreateIndexRequest(request.param("index"));
+
+        if (request.hasContent()) {
+            Map<String, Object> sourceAsMap = XContentHelper.convertToMap(request.requiredContent(), false, request.getXContentType()).v2();
+
+            request.param(INCLUDE_TYPE_NAME_PARAMETER);// just consume, it is always replaced with _doc
+            sourceAsMap = prepareMappings(sourceAsMap, request);
+
+            createIndexRequest.source(sourceAsMap, LoggingDeprecationHandler.INSTANCE);
+        }
+
+        createIndexRequest.timeout(request.paramAsTime("timeout", createIndexRequest.timeout()));
+        createIndexRequest.masterNodeTimeout(request.paramAsTime("master_timeout", createIndexRequest.masterNodeTimeout()));
+        createIndexRequest.waitForActiveShards(ActiveShardCount.parseString(request.param("wait_for_active_shards")));
+        return createIndexRequest;
+    }
+
+    static Map<String, Object> prepareMappings(Map<String, Object> source, RestRequest request) {
+        final boolean includeTypeName = request.paramAsBoolean(INCLUDE_TYPE_NAME_PARAMETER, false);
+
+        @SuppressWarnings("unchecked")
+        Map<String, Object> mappings = (Map<String, Object>) source.get("mappings");
+
+        if (includeTypeName && mappings.size() == 1) {
+            Map<String, Object> newSource = new HashMap<>();
+
+            String typeName = mappings.keySet().iterator().next();
+            @SuppressWarnings("unchecked")
+            Map<String, Object> typedMappings = (Map<String, Object>) mappings.get(typeName);
+
+            // no matter what the type was, replace it with _doc, because the internal representation still uses single type `_doc`.
+            newSource.put("mappings", Collections.singletonMap(MapperService.SINGLE_MAPPING_NAME, typedMappings));
+            return newSource;
+        } else {
+            return prepareMappings(source);
+        }
+    }
+
+    static Map<String, Object> prepareMappings(Map<String, Object> source) {
+        if (source.containsKey("mappings") == false || (source.get("mappings") instanceof Map) == false) {
+            return source;
+        }
+
+        Map<String, Object> newSource = new HashMap<>(source);
+
+        @SuppressWarnings("unchecked")
+        Map<String, Object> mappings = (Map<String, Object>) source.get("mappings");
+        if (MapperService.isMappingSourceTyped(MapperService.SINGLE_MAPPING_NAME, mappings)) {
+            throw new IllegalArgumentException("The mapping definition cannot be nested under a type");
+        }
+
+        newSource.put("mappings", singletonMap(MapperService.SINGLE_MAPPING_NAME, mappings));
+        return newSource;
+    }
+}

modules/rest-compatibility/src/main/java/org/elasticsearch/rest/compat/version7/RestGetActionV7.java

@@ -45,8 +45,6 @@ public String getName() {
 
     @Override
     public List<Route> routes() {
-        assert Version.CURRENT.major == 8 : "REST API compatibility for version 7 is only supported on version 8";
-
         return List.of(new Route(GET, "/{index}/{type}/{id}"), new Route(HEAD, "/{index}/{type}/{id}"));
     }
 
@@ -58,7 +56,7 @@ public RestChannelConsumer prepareRequest(RestRequest request, final NodeClient
     }
 
     @Override
-    public boolean compatibilityRequired() {
-        return true;
+    public Version compatibleWithVersion() {
+        return Version.V_7_0_0;
     }
 }

modules/rest-compatibility/src/main/java/org/elasticsearch/rest/compat/version7/RestIndexActionV7.java

@@ -31,9 +31,7 @@
 import java.util.List;
 import java.util.function.Supplier;
 
-import static java.util.Arrays.asList;
 import static java.util.Collections.singletonList;
-import static java.util.Collections.unmodifiableList;
 import static org.elasticsearch.rest.RestRequest.Method.POST;
 import static org.elasticsearch.rest.RestRequest.Method.PUT;
 
@@ -55,8 +53,6 @@ public String getName() {
 
         @Override
         public List<Route> routes() {
-            assert Version.CURRENT.major == 8 : "REST API compatilbity for version 7 is only supported on version 8";
-
             return List.of(new Route(POST, "/{index}/{type}/{id}"), new Route(PUT, "/{index}/{type}/{id}"));
         }
 
@@ -68,8 +64,8 @@ public RestChannelConsumer prepareRequest(RestRequest request, final NodeClient
         }
 
         @Override
-        public boolean compatibilityRequired() {
-            return true;
+        public Version compatibleWithVersion() {
+            return Version.V_7_0_0;
         }
     }
 
@@ -82,9 +78,7 @@ public String getName() {
 
         @Override
         public List<Route> routes() {
-            return unmodifiableList(
-                asList(new Route(POST, "/{index}/{type}/{id}/_create"), new Route(PUT, "/{index}/{type}/{id}/_create"))
-            );
+            return List.of(new Route(POST, "/{index}/{type}/{id}/_create"), new Route(PUT, "/{index}/{type}/{id}/_create"));
         }
 
         @Override
@@ -95,8 +89,8 @@ public RestChannelConsumer prepareRequest(RestRequest request, final NodeClient
         }
 
         @Override
-        public boolean compatibilityRequired() {
-            return true;
+        public Version compatibleWithVersion() {
+            return Version.V_7_0_0;
         }
     }
 
@@ -124,8 +118,8 @@ public RestChannelConsumer prepareRequest(RestRequest request, final NodeClient
         }
 
         @Override
-        public boolean compatibilityRequired() {
-            return true;
+        public Version compatibleWithVersion() {
+            return Version.V_7_0_0;
         }
     }
 }

modules/rest-compatibility/src/test/java/org/elasticsearch/rest/compat/version7/RestCreateIndexActionV7Tests.java

@@ -0,0 +1,75 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.rest.compat.version7;
+
+import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
+import org.elasticsearch.common.bytes.BytesArray;
+import org.elasticsearch.rest.RestRequest;
+import org.elasticsearch.test.rest.FakeRestRequest;
+import org.elasticsearch.test.rest.RestActionTestCase;
+import org.junit.Before;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.hamcrest.Matchers.equalTo;
+
+public class RestCreateIndexActionV7Tests extends RestActionTestCase {
+
+    String mimeType = "application/vnd.elasticsearch+json;compatible-with=7";
+    List<String> contentTypeHeader = Collections.singletonList(mimeType);
+
+    RestCreateIndexActionV7 restHandler = new RestCreateIndexActionV7();
+
+    @Before
+    public void setUpAction() {
+        controller().registerHandler(restHandler);
+    }
+
+    public void testTypeInMapping() throws IOException {
+        String content = "{\n"
+            + "  \"mappings\": {\n"
+            + "    \"some_type\": {\n"
+            + "      \"properties\": {\n"
+            + "        \"field1\": {\n"
+            + "          \"type\": \"text\"\n"
+            + "        }\n"
+            + "      }\n"
+            + "    }\n"
+            + "  }\n"
+            + "}";
+
+        Map<String, String> params = new HashMap<>();
+        params.put(RestCreateIndexActionV7.INCLUDE_TYPE_NAME_PARAMETER, "true");
+        RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.PUT)
+            .withHeaders(Map.of("Content-Type", contentTypeHeader, "Accept", contentTypeHeader))
+            .withPath("/some_index")
+            .withParams(params)
+            .withContent(new BytesArray(content), null)
+            .build();
+
+        CreateIndexRequest createIndexRequest = restHandler.prepareRequest(request);
+        // some_type is replaced with _doc
+        assertThat(createIndexRequest.mappings(), equalTo("{\"_doc\":{\"properties\":{\"field1\":{\"type\":\"text\"}}}}"));
+    }
+}

modules/rest-compatibility/src/test/java/org/elasticsearch/rest/compat/version7/RestGetActionV7Tests.java

@@ -29,7 +29,7 @@
 import java.util.Map;
 
 public class RestGetActionV7Tests extends RestActionTestCase {
-    final String mimeType = randomFrom("application/vnd.elasticsearch+json;compatible-with=7");
+    final String mimeType = "application/vnd.elasticsearch+json;compatible-with=7";
     final List<String> contentTypeHeader = Collections.singletonList(mimeType);
 
     @Before

modules/rest-compatibility/src/test/java/org/elasticsearch/rest/compat/version7/RestIndexActionV7Tests.java

@@ -32,7 +32,7 @@
 
 public class RestIndexActionV7Tests extends RestActionTestCase {
 
-    final String mimeType = randomFrom("application/vnd.elasticsearch+json;compatible-with=7");
+    final String mimeType = "application/vnd.elasticsearch+json;compatible-with=7";
     final List<String> contentTypeHeader = Collections.singletonList(mimeType);
 
     private final AtomicReference<ClusterState> clusterStateSupplier = new AtomicReference<>();

qa/rest-compat-tests/src/main/java/org/elasticsearch/rest/compat/AbstractCompatRestTest.java

@@ -22,6 +22,7 @@
 import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.elasticsearch.Version;
 import org.elasticsearch.rest.CompatibleConstants;
 import org.elasticsearch.test.rest.yaml.ClientYamlTestCandidate;
 import org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase;
@@ -82,7 +83,7 @@ private static Consumer<? super ExecutableSection> updateDoSection() {
             doSection.setIgnoreWarnings(true);
 
             String compatibleHeader = createCompatibleHeader();
-            // for cat apis accept headers would break tests which expect txt response
+            // TODO for cat apis accept headers would break tests which expect txt response
             if (doSection.getApiCallSection().getApi().startsWith("cat") == false) {
                 doSection.getApiCallSection()
                     .addHeaders(
@@ -99,7 +100,7 @@ private static Consumer<? super ExecutableSection> updateDoSection() {
     }
 
     private static String createCompatibleHeader() {
-        return "application/vnd.elasticsearch+json;compatible-with=" + CompatibleConstants.COMPATIBLE_VERSION;
+        return "application/vnd.elasticsearch+json;compatible-with=" + Version.minimumRestCompatibilityVersion().major;
     }
 
     private static Map<ClientYamlTestCandidate, ClientYamlTestCandidate> getLocalCompatibilityTests() throws Exception {

server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java

@@ -127,8 +127,8 @@ public XContentBuilder newBuilder(@Nullable XContentType requestContentType, @Nu
         }
 
         builder.humanReadable(human);
-        String compatibleVersion = request.param(CompatibleConstants.COMPATIBLE_PARAMS_KEY);
-        builder.setCompatibleMajorVersion(compatibleVersion == null ? -1 : Byte.parseByte(compatibleVersion));
+
+        builder.setCompatibleMajorVersion(request.getCompatibleApiVersion().major);
         return builder;
     }
 

server/src/main/java/org/elasticsearch/rest/CompatibleConstants.java

@@ -19,16 +19,12 @@
 
 package org.elasticsearch.rest;
 
-import org.elasticsearch.Version;
-
 public class CompatibleConstants {
 
     /**
      * TODO revisit when https://github.com/elastic/elasticsearch/issues/52370 is resolved
      */
     public static final String COMPATIBLE_ACCEPT_HEADER = "Accept";
     public static final String COMPATIBLE_CONTENT_TYPE_HEADER = "Content-Type";
-    public static final String COMPATIBLE_PARAMS_KEY = "Compatible-With";
-    public static final String COMPATIBLE_VERSION = String.valueOf(Version.minimumRestCompatibilityVersion().major);
 
 }