Merge pull request #18 from jakelandis/introduce_module

Introduce module for compat code

build.gradle

@@ -114,6 +114,7 @@ subprojects {
       ':distribution:tools:keystore-cli',
       ':distribution:tools:launchers',
       ':distribution:tools:plugin-cli',
+      ':modules:rest-compatibility',
       ':qa:os',
       ':qa:wildfly',
       ':x-pack:plugin:autoscaling',

docs/src/test/java/org/elasticsearch/smoketest/DocsClientYamlTestSuiteIT.java

@@ -74,7 +74,7 @@ public static Iterable<Object[]> parameters() throws Exception {
         entries.add(new NamedXContentRegistry.Entry(ExecutableSection.class,
                 new ParseField("compare_analyzers"), CompareAnalyzers::parse));
         NamedXContentRegistry executableSectionRegistry = new NamedXContentRegistry(entries);
-        return ESClientYamlSuiteTestCase.createParameters(executableSectionRegistry);
+        return ESClientYamlSuiteTestCase.createParameters(executableSectionRegistry, TESTS_PATH);
     }
 
     @Override

modules/rest-compatibility/build.gradle

@@ -0,0 +1,26 @@
+/*
+ * 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.
+ */
+
+esplugin {
+  description 'Adds a compatiblity layer for the prior major versions REST API'
+  classname 'org.elasticsearch.rest.compat.RestCompatPlugin'
+}
+
+integTest.enabled = false
+test.enabled = false

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

@@ -0,0 +1,52 @@
+/*
+ * 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;
+
+import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
+import org.elasticsearch.cluster.node.DiscoveryNodes;
+import org.elasticsearch.common.settings.ClusterSettings;
+import org.elasticsearch.common.settings.IndexScopedSettings;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.settings.SettingsFilter;
+import org.elasticsearch.plugins.ActionPlugin;
+import org.elasticsearch.plugins.Plugin;
+import org.elasticsearch.rest.RestController;
+import org.elasticsearch.rest.RestHandler;
+import org.elasticsearch.rest.compat.version7.RestGetActionV7;
+
+import java.util.List;
+import java.util.function.Supplier;
+
+public class RestCompatPlugin extends Plugin implements ActionPlugin {
+
+    @Override
+    public List<RestHandler> getRestHandlers(
+        Settings settings,
+        RestController restController,
+        ClusterSettings clusterSettings,
+        IndexScopedSettings indexScopedSettings,
+        SettingsFilter settingsFilter,
+        IndexNameExpressionResolver indexNameExpressionResolver,
+        Supplier<DiscoveryNodes> nodesInCluster
+    ) {
+
+        return List.of(new RestGetActionV7());
+    }
+}

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

@@ -0,0 +1,67 @@
+/*
+ * 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.apache.logging.log4j.LogManager;
+import org.elasticsearch.Version;
+import org.elasticsearch.client.node.NodeClient;
+import org.elasticsearch.common.logging.DeprecationLogger;
+import org.elasticsearch.rest.CompatibleHandlers;
+import org.elasticsearch.rest.RestRequest;
+import org.elasticsearch.rest.action.document.RestGetAction;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.function.Consumer;
+
+import static java.util.Arrays.asList;
+import static java.util.Collections.unmodifiableList;
+import static org.elasticsearch.rest.RestRequest.Method.GET;
+import static org.elasticsearch.rest.RestRequest.Method.HEAD;
+
+public class RestGetActionV7 extends RestGetAction {
+
+    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(RestGetAction.class));
+    private static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Specifying types in "
+        + "document get requests is deprecated, use the /{index}/_doc/{id} endpoint instead.";
+    private static final Consumer<RestRequest> DEPRECATION_WARNING = r -> deprecationLogger.deprecatedAndMaybeLog(
+        "get_with_types",
+        TYPES_DEPRECATION_MESSAGE
+    );
+
+    @Override
+    public List<Route> routes() {
+        assert Version.CURRENT.major == 8 : "REST API compatibility for version 7 is only supported on version 8";
+
+        return unmodifiableList(asList(new Route(GET, "/{index}/{type}/{id}"), new Route(HEAD, "/{index}/{type}/{id}")));
+    }
+
+    @Override
+    public RestChannelConsumer prepareRequest(RestRequest request, final NodeClient client) throws IOException {
+        DEPRECATION_WARNING.accept(request);
+        CompatibleHandlers.consumeParameterType(deprecationLogger).accept(request);
+        return super.prepareRequest(request, client);
+    }
+
+    @Override
+    public boolean compatibilityRequired() {
+        return true;
+    }
+}

qa/rest-compat-tests/build.gradle

@@ -20,3 +20,5 @@ integTest.dependsOn(copyRestTestsResources)
 dependencies {
   compile project(':test:framework')
 }
+
+test.enabled = false

server/src/main/java/org/elasticsearch/action/ActionModule.java

@@ -641,7 +641,6 @@ public void initRestHandlers(Supplier<DiscoveryNodes> nodesInCluster) {
         registerHandler.accept(new RestIndexAction.CompatibleCreateHandler());
         registerHandler.accept(new RestIndexAction.CompatibleAutoIdHandler(nodesInCluster));
         registerHandler.accept(new RestGetAction());
-        registerHandler.accept(new RestGetAction.CompatibleRestGetAction());
         registerHandler.accept(new RestGetSourceAction());
         registerHandler.accept(new RestMultiGetAction(settings));
         registerHandler.accept(new RestDeleteAction());

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

@@ -1,3 +1,22 @@
+/*
+ * 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;
 
 import org.elasticsearch.common.logging.DeprecationLogger;

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

@@ -329,7 +329,8 @@ private void tryAllHandlers(final RestRequest request, final RestChannel channel
                   }
                 } else {
                     if(handler.compatibilityRequired() == false //regular (not removed) handlers are always dispatched
-                        || CompatibleHandlers.isV7Compatible(request)) { //handlers that were registered compatible, require request to be compatible
+                        //handlers that were registered compatible, require request to be compatible
+                        || CompatibleHandlers.isV7Compatible(request)) {
                         dispatchRequest(request, channel, handler);
                     } else {
                         handleCompatibleNotAllowed(rawPath, channel);

server/src/main/java/org/elasticsearch/rest/action/document/RestGetAction.java

@@ -19,17 +19,12 @@
 
 package org.elasticsearch.rest.action.document;
 
-import org.apache.logging.log4j.LogManager;
-import org.elasticsearch.Version;
 import org.elasticsearch.action.get.GetRequest;
 import org.elasticsearch.action.get.GetResponse;
 import org.elasticsearch.client.node.NodeClient;
 import org.elasticsearch.common.Strings;
-import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.index.VersionType;
 import org.elasticsearch.rest.BaseRestHandler;
-import org.elasticsearch.rest.CompatibleHandlers;
-import org.elasticsearch.rest.RestController;
 import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.rest.action.RestActions;
@@ -38,24 +33,14 @@
 
 import java.io.IOException;
 import java.util.List;
-import java.util.List;
-import java.util.function.Consumer;
 
-import static java.util.Arrays.asList;
-import static java.util.Collections.unmodifiableList;
 import static org.elasticsearch.rest.RestRequest.Method.GET;
 import static org.elasticsearch.rest.RestRequest.Method.HEAD;
 import static org.elasticsearch.rest.RestStatus.NOT_FOUND;
 import static org.elasticsearch.rest.RestStatus.OK;
 
 public class RestGetAction extends BaseRestHandler {
 
-    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(RestGetAction.class));
-    private static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Specifying types in " +
-        "document get requests is deprecated, use the /{index}/_doc/{id} endpoint instead.";
-    private static final Consumer<RestRequest> DEPRECATION_WARNING = r -> deprecationLogger.deprecatedAndMaybeLog("get_with_types",TYPES_DEPRECATION_MESSAGE);
-
-
     @Override
     public String getName() {
         return "document_get_action";
@@ -92,42 +77,11 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
         getRequest.versionType(VersionType.fromString(request.param("version_type"), getRequest.versionType()));
 
         getRequest.fetchSourceContext(FetchSourceContext.parseFromRestRequest(request));
-
         return channel -> client.get(getRequest, new RestToXContentListener<GetResponse>(channel) {
             @Override
             protected RestStatus getStatus(final GetResponse response) {
                 return response.isExists() ? OK : NOT_FOUND;
             }
         });
     }
-
-    public static class CompatibleRestGetAction extends RestGetAction {
-
-        private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(RestGetAction.class));
-        private static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Specifying types in " +
-            "document get requests is deprecated, use the /{index}/_doc/{id} endpoint instead.";
-        private static final Consumer<RestRequest> DEPRECATION_WARNING = r -> deprecationLogger.deprecatedAndMaybeLog("get_with_types",TYPES_DEPRECATION_MESSAGE);
-
-
-        @Override
-        public List<Route> routes() {
-            assert Version.CURRENT.major == 8 : "REST API compatilbity for version 7 is only supported on version 8";
-
-            return unmodifiableList(asList(
-                new Route(GET, "/{index}/{type}/{id}"),
-                new Route(HEAD, "/{index}/{type}/{id}")));
-        }
-
-        @Override
-        public RestChannelConsumer prepareRequest(RestRequest request, final NodeClient client) throws IOException {
-            DEPRECATION_WARNING.accept(request);
-            CompatibleHandlers.consumeParameterType(deprecationLogger).accept(request);
-            return super.prepareRequest(request, client);
-        }
-
-        @Override
-        public boolean compatibilityRequired() {
-            return true;
-        }
-    }
 }

server/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java

@@ -25,12 +25,10 @@
 import org.elasticsearch.action.support.ActiveShardCount;
 import org.elasticsearch.client.node.NodeClient;
 import org.elasticsearch.cluster.node.DiscoveryNodes;
-import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.index.VersionType;
 import org.elasticsearch.rest.BaseRestHandler;
 import org.elasticsearch.rest.CompatibleHandlers;
-import org.elasticsearch.rest.RestController;
 import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.rest.action.RestActions;
 import org.elasticsearch.rest.action.RestStatusToXContentListener;

test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java

@@ -88,7 +88,7 @@ public abstract class ESClientYamlSuiteTestCase extends ESRestTestCase {
      */
     private static final String REST_TESTS_VALIDATE_SPEC = "tests.rest.validate_spec";
 
-    private static final String TESTS_PATH = "/rest-api-spec/test";
+    protected static final String TESTS_PATH = "/rest-api-spec/test";
     private static final String SPEC_PATH = "/rest-api-spec/api";
 
     /**

test/framework/src/test/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCaseTests.java

@@ -25,36 +25,37 @@
 
 import org.elasticsearch.test.ESTestCase;
 
+import static org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase.TESTS_PATH;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.Matchers.greaterThan;
 
 public class ESClientYamlSuiteTestCaseTests extends ESTestCase {
 
     public void testLoadAllYamlSuites() throws Exception {
-        Map<String,Set<Path>> yamlSuites = ESClientYamlSuiteTestCase.loadSuites("");
+        Map<String,Set<Path>> yamlSuites = ESClientYamlSuiteTestCase.loadSuites(TESTS_PATH, "");
         assertEquals(2, yamlSuites.size());
     }
 
     public void testLoadSingleYamlSuite() throws Exception {
-        Map<String,Set<Path>> yamlSuites = ESClientYamlSuiteTestCase.loadSuites("suite1/10_basic");
+        Map<String,Set<Path>> yamlSuites = ESClientYamlSuiteTestCase.loadSuites(TESTS_PATH, "suite1/10_basic");
         assertSingleFile(yamlSuites, "suite1", "10_basic.yml");
 
         //extension .yaml is optional
-        yamlSuites = ESClientYamlSuiteTestCase.loadSuites("suite1/10_basic");
+        yamlSuites = ESClientYamlSuiteTestCase.loadSuites(TESTS_PATH, "suite1/10_basic");
         assertSingleFile(yamlSuites, "suite1", "10_basic.yml");
     }
 
     public void testLoadMultipleYamlSuites() throws Exception {
         //single directory
-        Map<String,Set<Path>> yamlSuites = ESClientYamlSuiteTestCase.loadSuites("suite1");
+        Map<String,Set<Path>> yamlSuites = ESClientYamlSuiteTestCase.loadSuites(TESTS_PATH, "suite1");
         assertThat(yamlSuites, notNullValue());
         assertThat(yamlSuites.size(), equalTo(1));
         assertThat(yamlSuites.containsKey("suite1"), equalTo(true));
         assertThat(yamlSuites.get("suite1").size(), greaterThan(1));
 
         //multiple directories
-        yamlSuites = ESClientYamlSuiteTestCase.loadSuites("suite1", "suite2");
+        yamlSuites = ESClientYamlSuiteTestCase.loadSuites(TESTS_PATH, "suite1", "suite2");
         assertThat(yamlSuites, notNullValue());
         assertThat(yamlSuites.size(), equalTo(2));
         assertThat(yamlSuites.containsKey("suite1"), equalTo(true));
@@ -63,7 +64,7 @@ public void testLoadMultipleYamlSuites() throws Exception {
         assertEquals(2, yamlSuites.get("suite2").size());
 
         //multiple paths, which can be both directories or yaml test suites (with optional file extension)
-        yamlSuites = ESClientYamlSuiteTestCase.loadSuites("suite2/10_basic", "suite1");
+        yamlSuites = ESClientYamlSuiteTestCase.loadSuites(TESTS_PATH, "suite2/10_basic", "suite1");
         assertThat(yamlSuites, notNullValue());
         assertThat(yamlSuites.size(), equalTo(2));
         assertThat(yamlSuites.containsKey("suite2"), equalTo(true));