Move REST compatible header parsing into server (#68047)

Currently the logic for parsing vendor specific headers - i.e. application/vnd.elasticsearch+json;compatible-with=7 is within rest-compatibility module.
This commit is removing the rest-compatibility plugin and moving the version parsing logic
into server module. It no longer needs to be injected from xpack into server, therefore can be directly used from RestRequest.

relates #51816

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

@@ -253,7 +253,6 @@
 import org.elasticsearch.persistent.UpdatePersistentTaskStatusAction;
 import org.elasticsearch.plugins.ActionPlugin;
 import org.elasticsearch.plugins.ActionPlugin.ActionHandler;
-import org.elasticsearch.rest.CompatibleVersion;
 import org.elasticsearch.rest.RestController;
 import org.elasticsearch.rest.RestHandler;
 import org.elasticsearch.rest.RestHeaderDefinition;
@@ -420,8 +419,7 @@ public class ActionModule extends AbstractModule {
     public ActionModule(Settings settings, IndexNameExpressionResolver indexNameExpressionResolver,
                         IndexScopedSettings indexScopedSettings, ClusterSettings clusterSettings, SettingsFilter settingsFilter,
                         ThreadPool threadPool, List<ActionPlugin> actionPlugins, NodeClient nodeClient,
-                        CircuitBreakerService circuitBreakerService, UsageService usageService, SystemIndices systemIndices,
-                        CompatibleVersion compatibleVersion) {
+                        CircuitBreakerService circuitBreakerService, UsageService usageService, SystemIndices systemIndices) {
         this.settings = settings;
         this.indexNameExpressionResolver = indexNameExpressionResolver;
         this.indexScopedSettings = indexScopedSettings;
@@ -453,7 +451,7 @@ public ActionModule(Settings settings, IndexNameExpressionResolver indexNameExpr
         indicesAliasesRequestRequestValidators = new RequestValidators<>(
                 actionPlugins.stream().flatMap(p -> p.indicesAliasesRequestValidators().stream()).collect(Collectors.toList()));
 
-        restController = new RestController(headers, restWrapper, nodeClient, circuitBreakerService, usageService, compatibleVersion);
+        restController = new RestController(headers, restWrapper, nodeClient, circuitBreakerService, usageService);
     }
 
 

server/src/main/java/org/elasticsearch/node/Node.java

@@ -146,13 +146,11 @@
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.plugins.PluginsService;
 import org.elasticsearch.plugins.RepositoryPlugin;
-import org.elasticsearch.plugins.RestCompatibilityPlugin;
 import org.elasticsearch.plugins.ScriptPlugin;
 import org.elasticsearch.plugins.SearchPlugin;
 import org.elasticsearch.plugins.SystemIndexPlugin;
 import org.elasticsearch.repositories.RepositoriesModule;
 import org.elasticsearch.repositories.RepositoriesService;
-import org.elasticsearch.rest.CompatibleVersion;
 import org.elasticsearch.rest.RestController;
 import org.elasticsearch.script.ScriptContext;
 import org.elasticsearch.script.ScriptEngine;
@@ -564,8 +562,7 @@ protected Node(final Environment initialEnvironment,
 
             ActionModule actionModule = new ActionModule(settings, clusterModule.getIndexNameExpressionResolver(),
                 settingsModule.getIndexScopedSettings(), settingsModule.getClusterSettings(), settingsModule.getSettingsFilter(),
-                threadPool, pluginsService.filterPlugins(ActionPlugin.class), client, circuitBreakerService, usageService, systemIndices,
-                getRestCompatibleFunction());
+                threadPool, pluginsService.filterPlugins(ActionPlugin.class), client, circuitBreakerService, usageService, systemIndices);
             modules.add(actionModule);
 
             final RestController restController = actionModule.getRestController();
@@ -741,23 +738,6 @@ protected Node(final Environment initialEnvironment,
         }
     }
 
-    /**
-     * @return A function that can be used to determine the requested REST compatible version
-     * package scope for testing
-     */
-    CompatibleVersion getRestCompatibleFunction() {
-        List<RestCompatibilityPlugin> restCompatibilityPlugins = pluginsService.filterPlugins(RestCompatibilityPlugin.class);
-        final CompatibleVersion compatibleVersion;
-        if (restCompatibilityPlugins.size() > 1) {
-            throw new IllegalStateException("Only one RestCompatibilityPlugin is allowed");
-        } else if (restCompatibilityPlugins.size() == 1) {
-            compatibleVersion = restCompatibilityPlugins.get(0)::getCompatibleVersion;
-        } else {
-            compatibleVersion = CompatibleVersion.CURRENT_VERSION;
-        }
-        return compatibleVersion;
-    }
-
     protected TransportService newTransportService(Settings settings, Transport transport, ThreadPool threadPool,
                                                    TransportInterceptor interceptor,
                                                    Function<BoundTransportAddress, DiscoveryNode> localNodeFactory,

x-pack/plugin/rest-compatibility/src/main/java/org/elasticsearch/compat/CompatibleVersionPlugin.java → server/src/main/java/org/elasticsearch/rest/RestCompatibleVersionHelper.java

@@ -1,23 +1,43 @@
+/*
+ * 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.
+ */
+
 /*
  * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-package org.elasticsearch.compat;
+package org.elasticsearch.rest;
 
 import org.elasticsearch.ElasticsearchStatusException;
 import org.elasticsearch.Version;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.xcontent.MediaType;
 import org.elasticsearch.common.xcontent.ParsedMediaType;
-import org.elasticsearch.plugins.Plugin;
-import org.elasticsearch.plugins.RestCompatibilityPlugin;
-import org.elasticsearch.rest.RestStatus;
 
-public class CompatibleVersionPlugin extends Plugin implements RestCompatibilityPlugin {
+/**
+ * A helper that is responsible for parsing a Compatible REST API version from RestRequest.
+ * It also performs a validation of allowed combination of versions provided on those headers.
+ * Package scope as it is only aimed to be used by RestRequest
+ */
+class RestCompatibleVersionHelper {
 
-    @Override
-    public Version getCompatibleVersion(
+    static Version getCompatibleVersion(
         @Nullable ParsedMediaType acceptHeader,
         @Nullable ParsedMediaType contentTypeHeader,
         boolean hasContent
@@ -85,7 +105,6 @@ public Version getCompatibleVersion(
         return Version.CURRENT;
     }
 
-    // scope for testing
     static Byte parseVersion(ParsedMediaType parsedMediaType) {
         if (parsedMediaType != null) {
             String version = parsedMediaType.getParameters().get(MediaType.COMPATIBLE_WITH_PARAMETER_NAME);

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

@@ -91,14 +91,11 @@ public class RestController implements HttpServerTransport.Dispatcher {
     /** Rest headers that are copied to internal requests made during a rest request. */
     private final Set<RestHeaderDefinition> headersToCopy;
     private final UsageService usageService;
-    private final CompatibleVersion compatibleVersion;
 
     public RestController(Set<RestHeaderDefinition> headersToCopy, UnaryOperator<RestHandler> handlerWrapper,
-                          NodeClient client, CircuitBreakerService circuitBreakerService, UsageService usageService,
-                          CompatibleVersion compatibleVersion) {
+                          NodeClient client, CircuitBreakerService circuitBreakerService, UsageService usageService) {
         this.headersToCopy = headersToCopy;
         this.usageService = usageService;
-        this.compatibleVersion = compatibleVersion;
         if (handlerWrapper == null) {
             handlerWrapper = h -> h; // passthrough if no wrapper set
         }
@@ -330,8 +327,7 @@ private void tryAllHandlers(final RestRequest request, final RestChannel channel
         final String uri = request.uri();
         final RestRequest.Method requestMethod;
 
-        Version compatibleVersion = this.compatibleVersion.
-            get(request.getParsedAccept(), request.getParsedContentType(), request.hasContent());
+        Version compatibleVersion = request.getCompatibleVersion();
         try {
             // Resolves the HTTP method and fails if the method is invalid
             requestMethod = request.method();

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

@@ -21,6 +21,7 @@
 
 import org.apache.lucene.util.SetOnce;
 import org.elasticsearch.ElasticsearchParseException;
+import org.elasticsearch.Version;
 import org.elasticsearch.common.Booleans;
 import org.elasticsearch.common.CheckedConsumer;
 import org.elasticsearch.common.Nullable;
@@ -71,6 +72,7 @@ public class RestRequest implements ToXContent.Params {
     private final HttpChannel httpChannel;
     private final ParsedMediaType parsedAccept;
     private final ParsedMediaType parsedContentType;
+    private final Version compatibleVersion;
     private HttpRequest httpRequest;
 
     private boolean contentConsumed = false;
@@ -108,6 +110,7 @@ private RestRequest(NamedXContentRegistry xContentRegistry, Map<String, String>
         this.rawPath = path;
         this.headers = Collections.unmodifiableMap(headers);
         this.requestId = requestId;
+        this.compatibleVersion = RestCompatibleVersionHelper.getCompatibleVersion(parsedAccept, parsedContentType, hasContent());
     }
 
     private static @Nullable ParsedMediaType parseHeaderWithMediaType(Map<String, List<String>> headers, String headerName) {
@@ -553,6 +556,10 @@ public static XContentType parseContentType(List<String> header) {
         throw new IllegalArgumentException("empty Content-Type header");
     }
 
+    public Version getCompatibleVersion() {
+        return compatibleVersion;
+    }
+
     public static class MediaTypeHeaderException extends RuntimeException {
 
         private String failedHeaderName;

server/src/test/java/org/elasticsearch/action/ActionModuleTests.java

@@ -34,7 +34,6 @@
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.plugins.ActionPlugin;
 import org.elasticsearch.plugins.ActionPlugin.ActionHandler;
-import org.elasticsearch.rest.CompatibleVersion;
 import org.elasticsearch.rest.RestChannel;
 import org.elasticsearch.rest.RestController;
 import org.elasticsearch.rest.RestHandler;
@@ -112,7 +111,7 @@ public void testSetupRestHandlerContainsKnownBuiltin() {
         ActionModule actionModule = new ActionModule(settings.getSettings(),
             new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY)), settings.getIndexScopedSettings(),
             settings.getClusterSettings(), settings.getSettingsFilter(), null, emptyList(), null,
-            null, usageService, null, CompatibleVersion.CURRENT_VERSION);
+            null, usageService, null);
         actionModule.initRestHandlers(null);
         // At this point the easiest way to confirm that a handler is loaded is to try to register another one on top of it and to fail
         Exception e = expectThrows(IllegalArgumentException.class, () ->
@@ -152,7 +151,7 @@ public String getName() {
             ActionModule actionModule = new ActionModule(settings.getSettings(),
                 new IndexNameExpressionResolver(threadPool.getThreadContext()), settings.getIndexScopedSettings(),
                 settings.getClusterSettings(), settings.getSettingsFilter(), threadPool, singletonList(dupsMainAction),
-                null, null, usageService, null, CompatibleVersion.CURRENT_VERSION);
+                null, null, usageService, null);
             Exception e = expectThrows(IllegalArgumentException.class, () -> actionModule.initRestHandlers(null));
             assertThat(e.getMessage(), startsWith("Cannot replace existing handler for [/] for method: GET"));
         } finally {
@@ -187,7 +186,7 @@ public List<RestHandler> getRestHandlers(Settings settings, RestController restC
             ActionModule actionModule = new ActionModule(settings.getSettings(),
                 new IndexNameExpressionResolver(threadPool.getThreadContext()), settings.getIndexScopedSettings(),
                 settings.getClusterSettings(), settings.getSettingsFilter(), threadPool, singletonList(registersFakeHandler),
-                null, null, usageService, null, CompatibleVersion.CURRENT_VERSION);
+                null, null, usageService, null);
             actionModule.initRestHandlers(null);
             // At this point the easiest way to confirm that a handler is loaded is to try to register another one on top of it and to fail
             Exception e = expectThrows(IllegalArgumentException.class, () ->

server/src/test/java/org/elasticsearch/node/NodeTests.java

@@ -20,15 +20,13 @@
 
 import org.apache.lucene.util.LuceneTestCase;
 import org.apache.lucene.util.SetOnce;
-import org.elasticsearch.Version;
 import org.elasticsearch.bootstrap.BootstrapCheck;
 import org.elasticsearch.bootstrap.BootstrapContext;
 import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.common.breaker.CircuitBreaker;
 import org.elasticsearch.common.network.NetworkModule;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.transport.BoundTransportAddress;
-import org.elasticsearch.common.xcontent.ParsedMediaType;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.index.IndexService;
 import org.elasticsearch.index.engine.Engine.Searcher;
@@ -38,8 +36,6 @@
 import org.elasticsearch.indices.breaker.CircuitBreakerService;
 import org.elasticsearch.plugins.CircuitBreakerPlugin;
 import org.elasticsearch.plugins.Plugin;
-import org.elasticsearch.plugins.RestCompatibilityPlugin;
-import org.elasticsearch.rest.CompatibleVersion;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.InternalTestCluster;
 import org.elasticsearch.test.MockHttpTransport;
@@ -343,56 +339,4 @@ public void setCircuitBreaker(CircuitBreaker circuitBreaker) {
             myCircuitBreaker.set(circuitBreaker);
         }
     }
-
-    public static class TestRestCompatibility1 extends Plugin implements RestCompatibilityPlugin {
-        @Override
-        public Version getCompatibleVersion(ParsedMediaType acceptHeader, ParsedMediaType contentTypeHeader, boolean hasContent) {
-            return Version.CURRENT.previousMajor();
-        }
-    }
-
-    public static class TestRestCompatibility2 extends Plugin implements RestCompatibilityPlugin {
-        @Override
-        public Version getCompatibleVersion(ParsedMediaType acceptHeader, ParsedMediaType contentTypeHeader, boolean hasContent) {
-            return null;
-        }
-    }
-
-    public void testLoadingMultipleRestCompatibilityPlugins() throws IOException {
-        Settings.Builder settings = baseSettings();
-
-        // throw an exception when two plugins are registered
-        List<Class<? extends Plugin>> plugins = basePlugins();
-        plugins.add(TestRestCompatibility1.class);
-        plugins.add(TestRestCompatibility2.class);
-
-        IllegalStateException e = expectThrows(IllegalStateException.class, () -> new MockNode(settings.build(), plugins));
-        assertThat(e.getMessage(), equalTo("Only one RestCompatibilityPlugin is allowed"));
-    }
-
-    public void testCorrectUsageOfRestCompatibilityPlugin() throws IOException {
-        Settings.Builder settings = baseSettings();
-
-        // the correct usage expects one plugin
-        List<Class<? extends Plugin>> plugins = basePlugins();
-        plugins.add(TestRestCompatibility1.class);
-
-        try (Node node = new MockNode(settings.build(), plugins)) {
-            CompatibleVersion restCompatibleFunction = node.getRestCompatibleFunction();
-            assertThat(restCompatibleFunction.get(null, null, false), equalTo(Version.CURRENT.previousMajor()));
-        }
-    }
-
-
-    public void testDefaultingRestCompatibilityPlugin() throws IOException {
-        Settings.Builder settings = baseSettings();
-
-        // default to CompatibleVersion.CURRENT_VERSION when no plugins provided
-        List<Class<? extends Plugin>> plugins = basePlugins();
-
-        try (Node node = new MockNode(settings.build(), plugins)) {
-            CompatibleVersion restCompatibleFunction = node.getRestCompatibleFunction();
-            assertThat(restCompatibleFunction.get(null, null, false), equalTo(Version.CURRENT));
-        }
-    }
 }