From 40783d59166190fc62d366f823d35aee44f46470 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Wed, 15 Jul 2020 15:39:21 +0200 Subject: [PATCH 1/4] draft --- .../compat/RestCompatPlugin.java | 8 +++- x-pack/plugin/rest-compat/build.gradle | 18 +++++++++ .../compat/CompatRequestWrapper.java | 19 ++++++++++ .../compat/RestCompatRequestPlugin.java | 37 +++++++++++++++++++ 4 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 x-pack/plugin/rest-compat/build.gradle create mode 100644 x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/CompatRequestWrapper.java create mode 100644 x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/RestCompatRequestPlugin.java diff --git a/modules/rest-compatibility/src/main/java/org/elasticsearch/compat/RestCompatPlugin.java b/modules/rest-compatibility/src/main/java/org/elasticsearch/compat/RestCompatPlugin.java index e846c925166d4..d45d60cff2a52 100644 --- a/modules/rest-compatibility/src/main/java/org/elasticsearch/compat/RestCompatPlugin.java +++ b/modules/rest-compatibility/src/main/java/org/elasticsearch/compat/RestCompatPlugin.java @@ -19,6 +19,8 @@ package org.elasticsearch.compat; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.node.DiscoveryNodes; @@ -50,7 +52,7 @@ import java.util.function.Supplier; public class RestCompatPlugin extends Plugin implements ActionPlugin { - + Logger log = LogManager.getLogger(RestCompatPlugin.class); @Override public List getRestHandlers( Settings settings, @@ -61,7 +63,9 @@ public List getRestHandlers( IndexNameExpressionResolver indexNameExpressionResolver, Supplier nodesInCluster ) { - if (Version.CURRENT.major == 8) { + boolean compatibilityEnabled = Boolean.parseBoolean(settings.get("compat.setting")); + log.warn(" compat setting "+compatibilityEnabled); + if (compatibilityEnabled && Version.CURRENT.major == 8) { return validateCompatibleHandlers( 7, new RestDeleteByQueryActionV7(), diff --git a/x-pack/plugin/rest-compat/build.gradle b/x-pack/plugin/rest-compat/build.gradle new file mode 100644 index 0000000000000..59b4ddb0f32e9 --- /dev/null +++ b/x-pack/plugin/rest-compat/build.gradle @@ -0,0 +1,18 @@ +evaluationDependsOn(xpackModule('core')) + +apply plugin: 'elasticsearch.esplugin' +apply plugin: 'elasticsearch.rest-resources' + +esplugin { + name 'rest-compat' + description 'A plugin for Rest compat request features' + classname 'org.elasticsearch.compat.RestCompatRequestPlugin' + extendedPlugins = ['x-pack-core'] +} + +dependencies { + compileOnly project(path: xpackModule('core'), configuration: 'default') + testImplementation project(path: xpackModule('core'), configuration: 'testArtifacts') +} + + diff --git a/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/CompatRequestWrapper.java b/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/CompatRequestWrapper.java new file mode 100644 index 0000000000000..4e85e0e3444e3 --- /dev/null +++ b/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/CompatRequestWrapper.java @@ -0,0 +1,19 @@ +/* + * 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; + +import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.rest.RestChannel; +import org.elasticsearch.rest.RestHandler; +import org.elasticsearch.rest.RestRequest; + +public class CompatRequestWrapper implements RestHandler { + @Override + public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { + System.out.println("heee"); + } +} diff --git a/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/RestCompatRequestPlugin.java b/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/RestCompatRequestPlugin.java new file mode 100644 index 0000000000000..c8f6c1a856b0a --- /dev/null +++ b/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/RestCompatRequestPlugin.java @@ -0,0 +1,37 @@ +/* + * 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; + +import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.plugins.ActionPlugin; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.rest.RestHandler; +import org.elasticsearch.rest.RestHeaderDefinition; +import org.elasticsearch.xpack.core.security.SecuritySettings; + +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.function.UnaryOperator; + +public class RestCompatRequestPlugin extends Plugin implements ActionPlugin { + static final Setting SIMPLE_SETTING = Setting.simpleString("compat.setting", Setting.Property.NodeScope); + @Override + public List> getSettings() { + return Arrays.asList(SIMPLE_SETTING); + } + + @Override + public Settings additionalSettings() { + final Settings.Builder builder = Settings.builder(); + builder.put("compat.setting", true); + return builder.build(); + } + +} From 486252091ccc6df408e9e4f7984bd1ec2dbe72dd Mon Sep 17 00:00:00 2001 From: pgomulka Date: Thu, 16 Jul 2020 16:01:39 +0200 Subject: [PATCH 2/4] rest controller with a wrapping restRequest factory --- .../elasticsearch/action/ActionModule.java | 19 ++++- .../elasticsearch/plugins/ActionPlugin.java | 3 + .../elasticsearch/rest/RestController.java | 8 +- .../org/elasticsearch/rest/RestRequest.java | 2 +- .../rest/RestRequestFactory.java | 24 ++++++ .../rest/RestControllerTests.java | 14 ++-- .../rest/RestHttpResponseHeadersTests.java | 2 +- .../indices/RestValidateQueryActionTests.java | 2 +- .../test/rest/RestActionTestCase.java | 2 +- .../compat/CompatRequestWrapper.java | 19 ----- .../compat/RestCompatRequestPlugin.java | 84 +++++++++++++++++-- 11 files changed, 136 insertions(+), 43 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/rest/RestRequestFactory.java delete mode 100644 x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/CompatRequestWrapper.java diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index 5dfd676e6e5db..7adfc9b2b2d62 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -264,6 +264,7 @@ import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; import org.elasticsearch.rest.RestHeaderDefinition; +import org.elasticsearch.rest.RestRequestFactory; import org.elasticsearch.rest.action.RestFieldCapabilitiesAction; import org.elasticsearch.rest.action.RestMainAction; import org.elasticsearch.rest.action.admin.cluster.RestAddVotingConfigExclusionAction; @@ -477,10 +478,24 @@ public ActionModule(Settings settings, IndexNameExpressionResolver indexNameExpr actionPlugins.stream().flatMap(p -> p.mappingRequestValidators().stream()).collect(Collectors.toList())); indicesAliasesRequestRequestValidators = new RequestValidators<>( actionPlugins.stream().flatMap(p -> p.indicesAliasesRequestValidators().stream()).collect(Collectors.toList())); - - restController = new RestController(headers, restWrapper, nodeClient, circuitBreakerService, usageService); + RestRequestFactory restRequestFactory = getRestRequestFactory(actionPlugins); + restController = new RestController(headers, restWrapper, nodeClient, circuitBreakerService, usageService, restRequestFactory); } + private RestRequestFactory getRestRequestFactory(List actionPlugins) { + RestRequestFactory restRequestFactory = null; + for (ActionPlugin plugin : actionPlugins) { + RestRequestFactory newRestRequestFactory = plugin.getRestRequestFactory(); + if (newRestRequestFactory != null) { + logger.debug("Using REST wrapper from plugin " + plugin.getClass().getName()); + if (restRequestFactory != null) { + throw new IllegalArgumentException("Cannot have more than one plugin implementing a REST wrapper"); + } + restRequestFactory = newRestRequestFactory; + } + } + return restRequestFactory != null ? restRequestFactory : r -> r; + } public Map> getActions() { return actions; diff --git a/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java b/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java index ef9861f266222..ba5407c523851 100644 --- a/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java @@ -37,6 +37,7 @@ import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; import org.elasticsearch.rest.RestHeaderDefinition; +import org.elasticsearch.rest.RestRequestFactory; import java.util.Collection; import java.util.Collections; @@ -128,6 +129,8 @@ default UnaryOperator getRestHandlerWrapper(ThreadContext threadCon return null; } + RestRequestFactory getRestRequestFactory(); + final class ActionHandler { private final ActionType action; private final Class> transportAction; diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java index 1110cbe3e8cdd..23cbcfcf1f863 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestController.java +++ b/server/src/main/java/org/elasticsearch/rest/RestController.java @@ -76,11 +76,13 @@ public class RestController implements HttpServerTransport.Dispatcher { /** Rest headers that are copied to internal requests made during a rest request. */ private final Set headersToCopy; private final UsageService usageService; + private RestRequestFactory restRequestFactory; public RestController(Set headersToCopy, UnaryOperator handlerWrapper, - NodeClient client, CircuitBreakerService circuitBreakerService, UsageService usageService) { + NodeClient client, CircuitBreakerService circuitBreakerService, UsageService usageService, RestRequestFactory restRequestFactory) { this.headersToCopy = headersToCopy; this.usageService = usageService; + this.restRequestFactory = restRequestFactory; if (handlerWrapper == null) { handlerWrapper = h -> h; // passthrough if no wrapper set } @@ -176,8 +178,10 @@ public void dispatchRequest(RestRequest request, RestChannel channel, ThreadCont handleFavicon(request.method(), request.uri(), channel); return; } + RestRequest restRequest = restRequestFactory.createRestRequest(request); + try { - tryAllHandlers(request, channel, threadContext); + tryAllHandlers(restRequest, channel, threadContext); } catch (Exception e) { try { channel.sendResponse(new BytesRestResponse(channel, e)); diff --git a/server/src/main/java/org/elasticsearch/rest/RestRequest.java b/server/src/main/java/org/elasticsearch/rest/RestRequest.java index dcfd2f144dbe5..74c897dea018b 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -638,7 +638,7 @@ public static class BadParameterException extends RuntimeException { public static class CompatibleApiHeadersCombinationException extends RuntimeException { - CompatibleApiHeadersCombinationException(String cause) { + public CompatibleApiHeadersCombinationException(String cause) { super(cause); } diff --git a/server/src/main/java/org/elasticsearch/rest/RestRequestFactory.java b/server/src/main/java/org/elasticsearch/rest/RestRequestFactory.java new file mode 100644 index 0000000000000..c2972374407c6 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/rest/RestRequestFactory.java @@ -0,0 +1,24 @@ +/* + * 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; + +public interface RestRequestFactory { + RestRequest createRestRequest(RestRequest restRequest); +} diff --git a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java index 9848ce0d83392..0807d9652e7ed 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java @@ -95,7 +95,7 @@ public void setup() { inFlightRequestsBreaker = circuitBreakerService.getBreaker(CircuitBreaker.IN_FLIGHT_REQUESTS); HttpServerTransport httpServerTransport = new TestHttpServerTransport(); - restController = new RestController(Collections.emptySet(), null, null, circuitBreakerService, usageService); + restController = new RestController(Collections.emptySet(), null, null, circuitBreakerService, usageService, r -> r); restController.registerHandler(RestRequest.Method.GET, "/", (request, channel, client) -> channel.sendResponse( new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY))); @@ -110,7 +110,7 @@ public void testApplyRelevantHeaders() throws Exception { final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); Set headers = new HashSet<>(Arrays.asList(new RestHeaderDefinition("header.1", true), new RestHeaderDefinition("header.2", true))); - final RestController restController = new RestController(headers, null, null, circuitBreakerService, usageService); + final RestController restController = new RestController(headers, null, null, circuitBreakerService, usageService, r -> r); Map> restHeaders = new HashMap<>(); restHeaders.put("header.1", Collections.singletonList("true")); restHeaders.put("header.2", Collections.singletonList("true")); @@ -146,7 +146,7 @@ public void testRequestWithDisallowedMultiValuedHeader() { final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); Set headers = new HashSet<>(Arrays.asList(new RestHeaderDefinition("header.1", true), new RestHeaderDefinition("header.2", false))); - final RestController restController = new RestController(headers, null, null, circuitBreakerService, usageService); + final RestController restController = new RestController(headers, null, null, circuitBreakerService, usageService, r -> r); Map> restHeaders = new HashMap<>(); restHeaders.put("header.1", Collections.singletonList("boo")); restHeaders.put("header.2", List.of("foo", "bar")); @@ -160,7 +160,7 @@ public void testRequestWithDisallowedMultiValuedHeaderButSameValues() { final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); Set headers = new HashSet<>(Arrays.asList(new RestHeaderDefinition("header.1", true), new RestHeaderDefinition("header.2", false))); - final RestController restController = new RestController(headers, null, null, circuitBreakerService, usageService); + final RestController restController = new RestController(headers, null, null, circuitBreakerService, usageService, r -> r); Map> restHeaders = new HashMap<>(); restHeaders.put("header.1", Collections.singletonList("boo")); restHeaders.put("header.2", List.of("foo", "foo")); @@ -214,7 +214,7 @@ public void testRegisterWithDeprecatedHandler() { } public void testRegisterSecondMethodWithDifferentNamedWildcard() { - final RestController restController = new RestController(null, null, null, circuitBreakerService, usageService); + final RestController restController = new RestController(null, null, null, circuitBreakerService, usageService, r -> r); RestRequest.Method firstMethod = randomFrom(RestRequest.Method.values()); RestRequest.Method secondMethod = @@ -247,7 +247,7 @@ public void testRestHandlerWrapper() throws Exception { h -> { assertSame(handler, h); return (RestRequest request, RestChannel channel, NodeClient client) -> wrapperCalled.set(true); - }, null, circuitBreakerService, usageService); + }, null, circuitBreakerService, usageService, r -> r); restController.registerHandler(RestRequest.Method.GET, "/wrapped", handler); RestRequest request = testRestRequest("/wrapped", "{}", XContentType.JSON); AssertingChannel channel = new AssertingChannel(request, true, RestStatus.BAD_REQUEST); @@ -310,7 +310,7 @@ public void testDispatchRequiresContentTypeForRequestsWithContent() { String content = randomAlphaOfLength((int) Math.round(BREAKER_LIMIT.getBytes() / inFlightRequestsBreaker.getOverhead())); RestRequest request = testRestRequest("/", content, null); AssertingChannel channel = new AssertingChannel(request, true, RestStatus.NOT_ACCEPTABLE); - restController = new RestController(Collections.emptySet(), null, null, circuitBreakerService, usageService); + restController = new RestController(Collections.emptySet(), null, null, circuitBreakerService, usageService, r -> r); restController.registerHandler(RestRequest.Method.GET, "/", (r, c, client) -> c.sendResponse( new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY))); diff --git a/server/src/test/java/org/elasticsearch/rest/RestHttpResponseHeadersTests.java b/server/src/test/java/org/elasticsearch/rest/RestHttpResponseHeadersTests.java index ce54b896ef36e..0a31379c70839 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestHttpResponseHeadersTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestHttpResponseHeadersTests.java @@ -90,7 +90,7 @@ public void testUnsupportedMethodResponseHttpHeader() throws Exception { final Settings settings = Settings.EMPTY; UsageService usageService = new UsageService(); RestController restController = new RestController(Collections.emptySet(), - null, null, circuitBreakerService, usageService); + null, null, circuitBreakerService, usageService, r -> r); // A basic RestHandler handles requests to the endpoint RestHandler restHandler = new RestHandler() { diff --git a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestValidateQueryActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestValidateQueryActionTests.java index 4813e11e15bfc..4f2f22e295a38 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestValidateQueryActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestValidateQueryActionTests.java @@ -58,7 +58,7 @@ public class RestValidateQueryActionTests extends AbstractSearchTestCase { private static UsageService usageService = new UsageService(); private static RestController controller = new RestController(emptySet(), null, client, - new NoneCircuitBreakerService(), usageService); + new NoneCircuitBreakerService(), usageService, r -> r); private static RestValidateQueryAction action = new RestValidateQueryAction(); /** diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java index 5aea384de6a56..57ea6ebf77295 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java @@ -47,7 +47,7 @@ public void setUpController() { controller = new RestController(Collections.emptySet(), null, nodeClient, new NoneCircuitBreakerService(), - new UsageService()); + new UsageService(), r -> r); } /** diff --git a/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/CompatRequestWrapper.java b/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/CompatRequestWrapper.java deleted file mode 100644 index 4e85e0e3444e3..0000000000000 --- a/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/CompatRequestWrapper.java +++ /dev/null @@ -1,19 +0,0 @@ -/* - * 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; - -import org.elasticsearch.client.node.NodeClient; -import org.elasticsearch.rest.RestChannel; -import org.elasticsearch.rest.RestHandler; -import org.elasticsearch.rest.RestRequest; - -public class CompatRequestWrapper implements RestHandler { - @Override - public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { - System.out.println("heee"); - } -} diff --git a/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/RestCompatRequestPlugin.java b/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/RestCompatRequestPlugin.java index c8f6c1a856b0a..34844e2beb84a 100644 --- a/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/RestCompatRequestPlugin.java +++ b/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/RestCompatRequestPlugin.java @@ -6,32 +6,98 @@ package org.elasticsearch.compat; + +import org.elasticsearch.Version; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.http.HttpChannel; +import org.elasticsearch.http.HttpRequest; import org.elasticsearch.plugins.ActionPlugin; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.rest.RestHandler; import org.elasticsearch.rest.RestHeaderDefinition; import org.elasticsearch.xpack.core.security.SecuritySettings; +import org.elasticsearch.rest.CompatibleConstants; +import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.RestRequestFactory; import java.util.Arrays; import java.util.Collection; import java.util.List; import java.util.function.UnaryOperator; +import java.util.Locale; public class RestCompatRequestPlugin extends Plugin implements ActionPlugin { - static final Setting SIMPLE_SETTING = Setting.simpleString("compat.setting", Setting.Property.NodeScope); - @Override - public List> getSettings() { - return Arrays.asList(SIMPLE_SETTING); - } @Override - public Settings additionalSettings() { - final Settings.Builder builder = Settings.builder(); - builder.put("compat.setting", true); - return builder.build(); + public RestRequestFactory getRestRequestFactory() { + return new RestRequestFactory() { + @Override + public RestRequest createRestRequest(RestRequest restRequest) { + return new CompatibleRestRequest(restRequest); + } + }; } + public static class CompatibleRestRequest extends RestRequest{ + + protected CompatibleRestRequest(RestRequest restRequest) { + super(restRequest); + } + + @Override + public Version getCompatibleApiVersion() { + if (isRequestingCompatibility()) { + return Version.minimumRestCompatibilityVersion(); + } else { + return Version.CURRENT; + } + } + + private boolean isRequestingCompatibility() { + String acceptHeader = header(CompatibleConstants.COMPATIBLE_ACCEPT_HEADER); + String aVersion = XContentType.parseVersion(acceptHeader); + byte acceptVersion = aVersion == null ? Version.CURRENT.major : Integer.valueOf(aVersion).byteValue(); + String contentTypeHeader = header(CompatibleConstants.COMPATIBLE_CONTENT_TYPE_HEADER); + String cVersion = XContentType.parseVersion(contentTypeHeader); + byte contentTypeVersion = cVersion == null ? Version.CURRENT.major : Integer.valueOf(cVersion).byteValue(); + if(Version.CURRENT.major < acceptVersion || Version.CURRENT.major - acceptVersion > 1 ){ + throw new CompatibleApiHeadersCombinationException( + String.format(Locale.ROOT, "Unsupported version provided. " + + "Accept=%s Content-Type=%s hasContent=%b path=%s params=%s method=%s", acceptHeader, + contentTypeHeader, hasContent(), path(), params().toString(), method().toString())); + } + if (hasContent()) { + if(Version.CURRENT.major < contentTypeVersion || Version.CURRENT.major - contentTypeVersion > 1 ){ + throw new CompatibleApiHeadersCombinationException( + String.format(Locale.ROOT, "Unsupported version provided. " + + "Accept=%s Content-Type=%s hasContent=%b path=%s params=%s method=%s", acceptHeader, + contentTypeHeader, hasContent(), path(), params().toString(), method().toString())); + } + + if (contentTypeVersion != acceptVersion) { + throw new CompatibleApiHeadersCombinationException( + String.format(Locale.ROOT, "Content-Type and Accept headers have to match when content is present. " + + "Accept=%s Content-Type=%s hasContent=%b path=%s params=%s method=%s", acceptHeader, + contentTypeHeader, hasContent(), path(), params().toString(), method().toString())); + } + // both headers should be versioned or none + if ((cVersion == null && aVersion!=null) || (aVersion ==null && cVersion!=null) ){ + throw new RestRequest.CompatibleApiHeadersCombinationException( + String.format(Locale.ROOT, "Versioning is required on both Content-Type and Accept headers. " + + "Accept=%s Content-Type=%s hasContent=%b path=%s params=%s method=%s", acceptHeader, + contentTypeHeader, hasContent(), path(), params().toString(), method().toString())); + } + + return contentTypeVersion < Version.CURRENT.major; + } + + return acceptVersion < Version.CURRENT.major; + } + + + } } From 9f2bfee59051778f1398e9ef42e623ad20fed202 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Mon, 20 Jul 2020 10:51:29 +0200 Subject: [PATCH 3/4] cleanup and todos --- .../compat/RestCompatPlugin.java | 4 +-- .../elasticsearch/action/ActionModule.java | 14 +++++---- .../http/AbstractHttpServerTransport.java | 1 + .../java/org/elasticsearch/node/Node.java | 4 ++- .../elasticsearch/plugins/ActionPlugin.java | 2 +- .../plugins/RestRequestPlugin.java | 29 +++++++++++++++++++ .../action/ActionModuleTests.java | 6 ++-- .../compat/RestCompatRequestPlugin.java | 20 ++++--------- 8 files changed, 51 insertions(+), 29 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/plugins/RestRequestPlugin.java diff --git a/modules/rest-compatibility/src/main/java/org/elasticsearch/compat/RestCompatPlugin.java b/modules/rest-compatibility/src/main/java/org/elasticsearch/compat/RestCompatPlugin.java index d45d60cff2a52..d6018949395d1 100644 --- a/modules/rest-compatibility/src/main/java/org/elasticsearch/compat/RestCompatPlugin.java +++ b/modules/rest-compatibility/src/main/java/org/elasticsearch/compat/RestCompatPlugin.java @@ -63,9 +63,7 @@ public List getRestHandlers( IndexNameExpressionResolver indexNameExpressionResolver, Supplier nodesInCluster ) { - boolean compatibilityEnabled = Boolean.parseBoolean(settings.get("compat.setting")); - log.warn(" compat setting "+compatibilityEnabled); - if (compatibilityEnabled && Version.CURRENT.major == 8) { + if (Version.CURRENT.major == 8) { return validateCompatibleHandlers( 7, new RestDeleteByQueryActionV7(), diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index 7adfc9b2b2d62..226b2130a1c21 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -261,6 +261,7 @@ import org.elasticsearch.persistent.UpdatePersistentTaskStatusAction; import org.elasticsearch.plugins.ActionPlugin; import org.elasticsearch.plugins.ActionPlugin.ActionHandler; +import org.elasticsearch.plugins.RestRequestPlugin; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; import org.elasticsearch.rest.RestHeaderDefinition; @@ -447,7 +448,7 @@ public class ActionModule extends AbstractModule { public ActionModule(Settings settings, IndexNameExpressionResolver indexNameExpressionResolver, IndexScopedSettings indexScopedSettings, ClusterSettings clusterSettings, SettingsFilter settingsFilter, ThreadPool threadPool, List actionPlugins, NodeClient nodeClient, - CircuitBreakerService circuitBreakerService, UsageService usageService, ClusterService clusterService) { + CircuitBreakerService circuitBreakerService, UsageService usageService, ClusterService clusterService, List restRequestPlugins) { this.settings = settings; this.indexNameExpressionResolver = indexNameExpressionResolver; this.indexScopedSettings = indexScopedSettings; @@ -478,18 +479,19 @@ public ActionModule(Settings settings, IndexNameExpressionResolver indexNameExpr actionPlugins.stream().flatMap(p -> p.mappingRequestValidators().stream()).collect(Collectors.toList())); indicesAliasesRequestRequestValidators = new RequestValidators<>( actionPlugins.stream().flatMap(p -> p.indicesAliasesRequestValidators().stream()).collect(Collectors.toList())); - RestRequestFactory restRequestFactory = getRestRequestFactory(actionPlugins); + RestRequestFactory restRequestFactory = getRestRequestFactory(restRequestPlugins); restController = new RestController(headers, restWrapper, nodeClient, circuitBreakerService, usageService, restRequestFactory); } - private RestRequestFactory getRestRequestFactory(List actionPlugins) { + private RestRequestFactory getRestRequestFactory(List restRequestPlugins) { RestRequestFactory restRequestFactory = null; - for (ActionPlugin plugin : actionPlugins) { + for (RestRequestPlugin plugin : restRequestPlugins) { RestRequestFactory newRestRequestFactory = plugin.getRestRequestFactory(); if (newRestRequestFactory != null) { - logger.debug("Using REST wrapper from plugin " + plugin.getClass().getName()); + logger.debug("Using REST request factory from plugin " + plugin.getClass().getName()); if (restRequestFactory != null) { - throw new IllegalArgumentException("Cannot have more than one plugin implementing a REST wrapper"); + //TODO maybe we could combine/chain them? + throw new IllegalArgumentException("Cannot have more than one plugin implementing a RestRequestPlugin"); } restRequestFactory = newRestRequestFactory; } diff --git a/server/src/main/java/org/elasticsearch/http/AbstractHttpServerTransport.java b/server/src/main/java/org/elasticsearch/http/AbstractHttpServerTransport.java index e146e383f9e91..9a11878922043 100644 --- a/server/src/main/java/org/elasticsearch/http/AbstractHttpServerTransport.java +++ b/server/src/main/java/org/elasticsearch/http/AbstractHttpServerTransport.java @@ -334,6 +334,7 @@ private void handleIncomingRequest(final HttpRequest httpRequest, final HttpChan { RestRequest innerRestRequest; try { + //TODO another option would be to use a factory here, but it requires to modify all the subclasses.. innerRestRequest = RestRequest.request(xContentRegistry, httpRequest, httpChannel); } catch (final RestRequest.ContentTypeHeaderException e) { badRequestCause = ExceptionsHelper.useOrSuppress(badRequestCause, e); diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index 68258a55047a6..afe65a52e743e 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -141,6 +141,7 @@ import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.PluginsService; import org.elasticsearch.plugins.RepositoryPlugin; +import org.elasticsearch.plugins.RestRequestPlugin; import org.elasticsearch.plugins.ScriptPlugin; import org.elasticsearch.plugins.SearchPlugin; import org.elasticsearch.plugins.SystemIndexPlugin; @@ -511,7 +512,8 @@ 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, clusterService); + threadPool, pluginsService.filterPlugins(ActionPlugin.class), client, circuitBreakerService, usageService, clusterService, + pluginsService.filterPlugins(RestRequestPlugin.class)); modules.add(actionModule); final RestController restController = actionModule.getRestController(); diff --git a/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java b/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java index ba5407c523851..1fb5c142679e2 100644 --- a/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java @@ -129,7 +129,7 @@ default UnaryOperator getRestHandlerWrapper(ThreadContext threadCon return null; } - RestRequestFactory getRestRequestFactory(); +// RestRequestFactory getRestRequestFactory(); final class ActionHandler { private final ActionType action; diff --git a/server/src/main/java/org/elasticsearch/plugins/RestRequestPlugin.java b/server/src/main/java/org/elasticsearch/plugins/RestRequestPlugin.java new file mode 100644 index 0000000000000..05974ba6211ec --- /dev/null +++ b/server/src/main/java/org/elasticsearch/plugins/RestRequestPlugin.java @@ -0,0 +1,29 @@ +/* + * 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.plugins; + +import org.elasticsearch.rest.RestRequestFactory; + +public interface RestRequestPlugin { + default RestRequestFactory getRestRequestFactory() { + return null; + } + +} diff --git a/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java b/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java index aa0e592e01a79..c73a3497b4ab5 100644 --- a/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java +++ b/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java @@ -109,7 +109,7 @@ public void testSetupRestHandlerContainsKnownBuiltin() { UsageService usageService = new UsageService(); ActionModule actionModule = new ActionModule(settings.getSettings(), new IndexNameExpressionResolver(), settings.getIndexScopedSettings(), settings.getClusterSettings(), settings.getSettingsFilter(), null, emptyList(), null, - null, usageService, null); + null, usageService, null, 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, () -> @@ -148,7 +148,7 @@ public String getName() { UsageService usageService = new UsageService(); ActionModule actionModule = new ActionModule(settings.getSettings(), new IndexNameExpressionResolver(), settings.getIndexScopedSettings(), settings.getClusterSettings(), settings.getSettingsFilter(), threadPool, - singletonList(dupsMainAction), null, null, usageService, null); + singletonList(dupsMainAction), null, null, usageService, null, null); Exception e = expectThrows(IllegalArgumentException.class, () -> actionModule.initRestHandlers(null)); assertThat(e.getMessage(), startsWith("Cannot replace existing handler for [/] for method: GET")); } finally { @@ -182,7 +182,7 @@ public List getRestHandlers(Settings settings, RestController restC UsageService usageService = new UsageService(); ActionModule actionModule = new ActionModule(settings.getSettings(), new IndexNameExpressionResolver(), settings.getIndexScopedSettings(), settings.getClusterSettings(), settings.getSettingsFilter(), threadPool, - singletonList(registersFakeHandler), null, null, usageService, null); + singletonList(registersFakeHandler), null, null, usageService, null, 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, () -> diff --git a/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/RestCompatRequestPlugin.java b/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/RestCompatRequestPlugin.java index 34844e2beb84a..089ebd277600a 100644 --- a/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/RestCompatRequestPlugin.java +++ b/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/RestCompatRequestPlugin.java @@ -8,29 +8,16 @@ import org.elasticsearch.Version; -import org.elasticsearch.common.settings.Setting; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.util.concurrent.ThreadContext; -import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentType; -import org.elasticsearch.http.HttpChannel; -import org.elasticsearch.http.HttpRequest; -import org.elasticsearch.plugins.ActionPlugin; import org.elasticsearch.plugins.Plugin; -import org.elasticsearch.rest.RestHandler; -import org.elasticsearch.rest.RestHeaderDefinition; -import org.elasticsearch.xpack.core.security.SecuritySettings; +import org.elasticsearch.plugins.RestRequestPlugin; import org.elasticsearch.rest.CompatibleConstants; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestRequestFactory; -import java.util.Arrays; -import java.util.Collection; -import java.util.List; -import java.util.function.UnaryOperator; import java.util.Locale; -public class RestCompatRequestPlugin extends Plugin implements ActionPlugin { +public class RestCompatRequestPlugin extends Plugin implements RestRequestPlugin { @Override public RestRequestFactory getRestRequestFactory() { @@ -41,8 +28,11 @@ public RestRequest createRestRequest(RestRequest restRequest) { } }; } + + public static class CompatibleRestRequest extends RestRequest{ + //TODO this requires copying the content of original rest request. Isn't this against why multiple rest wrappers were disallowed? protected CompatibleRestRequest(RestRequest restRequest) { super(restRequest); } From 629e53f4b59cc1f34b4096ee59240cf9682bd361 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Mon, 20 Jul 2020 12:26:42 +0200 Subject: [PATCH 4/4] precommit --- .../compat/RestCompatPlugin.java | 1 + .../elasticsearch/action/ActionModule.java | 3 +- .../elasticsearch/plugins/ActionPlugin.java | 1 - .../elasticsearch/rest/RestController.java | 3 +- x-pack/plugin/rest-compat/build.gradle | 2 +- .../compat/RestCompatRequestPlugin.java | 71 +++++++++++++------ .../compat/RestCompatRequestPluginTests.java | 16 +++++ 7 files changed, 73 insertions(+), 24 deletions(-) create mode 100644 x-pack/plugin/rest-compat/src/test/java/org/elasticsearch/compat/RestCompatRequestPluginTests.java diff --git a/modules/rest-compatibility/src/main/java/org/elasticsearch/compat/RestCompatPlugin.java b/modules/rest-compatibility/src/main/java/org/elasticsearch/compat/RestCompatPlugin.java index d6018949395d1..1314d17fa7867 100644 --- a/modules/rest-compatibility/src/main/java/org/elasticsearch/compat/RestCompatPlugin.java +++ b/modules/rest-compatibility/src/main/java/org/elasticsearch/compat/RestCompatPlugin.java @@ -53,6 +53,7 @@ public class RestCompatPlugin extends Plugin implements ActionPlugin { Logger log = LogManager.getLogger(RestCompatPlugin.class); + @Override public List getRestHandlers( Settings settings, diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index 226b2130a1c21..2c40993549fa2 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -448,7 +448,8 @@ public class ActionModule extends AbstractModule { public ActionModule(Settings settings, IndexNameExpressionResolver indexNameExpressionResolver, IndexScopedSettings indexScopedSettings, ClusterSettings clusterSettings, SettingsFilter settingsFilter, ThreadPool threadPool, List actionPlugins, NodeClient nodeClient, - CircuitBreakerService circuitBreakerService, UsageService usageService, ClusterService clusterService, List restRequestPlugins) { + CircuitBreakerService circuitBreakerService, UsageService usageService, ClusterService clusterService, + List restRequestPlugins) { this.settings = settings; this.indexNameExpressionResolver = indexNameExpressionResolver; this.indexScopedSettings = indexScopedSettings; diff --git a/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java b/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java index 1fb5c142679e2..a8e2a70191873 100644 --- a/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java @@ -37,7 +37,6 @@ import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; import org.elasticsearch.rest.RestHeaderDefinition; -import org.elasticsearch.rest.RestRequestFactory; import java.util.Collection; import java.util.Collections; diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java index 23cbcfcf1f863..7b2f8abc10e08 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestController.java +++ b/server/src/main/java/org/elasticsearch/rest/RestController.java @@ -79,7 +79,8 @@ public class RestController implements HttpServerTransport.Dispatcher { private RestRequestFactory restRequestFactory; public RestController(Set headersToCopy, UnaryOperator handlerWrapper, - NodeClient client, CircuitBreakerService circuitBreakerService, UsageService usageService, RestRequestFactory restRequestFactory) { + NodeClient client, CircuitBreakerService circuitBreakerService, UsageService usageService, + RestRequestFactory restRequestFactory) { this.headersToCopy = headersToCopy; this.usageService = usageService; this.restRequestFactory = restRequestFactory; diff --git a/x-pack/plugin/rest-compat/build.gradle b/x-pack/plugin/rest-compat/build.gradle index 59b4ddb0f32e9..421e5492a183a 100644 --- a/x-pack/plugin/rest-compat/build.gradle +++ b/x-pack/plugin/rest-compat/build.gradle @@ -1,7 +1,6 @@ evaluationDependsOn(xpackModule('core')) apply plugin: 'elasticsearch.esplugin' -apply plugin: 'elasticsearch.rest-resources' esplugin { name 'rest-compat' @@ -16,3 +15,4 @@ dependencies { } +integTest.enabled = false diff --git a/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/RestCompatRequestPlugin.java b/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/RestCompatRequestPlugin.java index 089ebd277600a..cff5c24f39667 100644 --- a/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/RestCompatRequestPlugin.java +++ b/x-pack/plugin/rest-compat/src/main/java/org/elasticsearch/compat/RestCompatRequestPlugin.java @@ -6,7 +6,6 @@ package org.elasticsearch.compat; - import org.elasticsearch.Version; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.plugins.Plugin; @@ -29,10 +28,9 @@ public RestRequest createRestRequest(RestRequest restRequest) { }; } + public static class CompatibleRestRequest extends RestRequest { - public static class CompatibleRestRequest extends RestRequest{ - - //TODO this requires copying the content of original rest request. Isn't this against why multiple rest wrappers were disallowed? + // TODO this requires copying the content of original rest request. Isn't this against why multiple rest wrappers were disallowed? protected CompatibleRestRequest(RestRequest restRequest) { super(restRequest); } @@ -54,32 +52,66 @@ private boolean isRequestingCompatibility() { String cVersion = XContentType.parseVersion(contentTypeHeader); byte contentTypeVersion = cVersion == null ? Version.CURRENT.major : Integer.valueOf(cVersion).byteValue(); - if(Version.CURRENT.major < acceptVersion || Version.CURRENT.major - acceptVersion > 1 ){ + if (Version.CURRENT.major < acceptVersion || Version.CURRENT.major - acceptVersion > 1) { throw new CompatibleApiHeadersCombinationException( - String.format(Locale.ROOT, "Unsupported version provided. " + - "Accept=%s Content-Type=%s hasContent=%b path=%s params=%s method=%s", acceptHeader, - contentTypeHeader, hasContent(), path(), params().toString(), method().toString())); + String.format( + Locale.ROOT, + "Unsupported version provided. " + "Accept=%s Content-Type=%s hasContent=%b path=%s params=%s method=%s", + acceptHeader, + contentTypeHeader, + hasContent(), + path(), + params().toString(), + method().toString() + ) + ); } if (hasContent()) { - if(Version.CURRENT.major < contentTypeVersion || Version.CURRENT.major - contentTypeVersion > 1 ){ + if (Version.CURRENT.major < contentTypeVersion || Version.CURRENT.major - contentTypeVersion > 1) { throw new CompatibleApiHeadersCombinationException( - String.format(Locale.ROOT, "Unsupported version provided. " + - "Accept=%s Content-Type=%s hasContent=%b path=%s params=%s method=%s", acceptHeader, - contentTypeHeader, hasContent(), path(), params().toString(), method().toString())); + String.format( + Locale.ROOT, + "Unsupported version provided. " + "Accept=%s Content-Type=%s hasContent=%b path=%s params=%s method=%s", + acceptHeader, + contentTypeHeader, + hasContent(), + path(), + params().toString(), + method().toString() + ) + ); } if (contentTypeVersion != acceptVersion) { throw new CompatibleApiHeadersCombinationException( - String.format(Locale.ROOT, "Content-Type and Accept headers have to match when content is present. " + - "Accept=%s Content-Type=%s hasContent=%b path=%s params=%s method=%s", acceptHeader, - contentTypeHeader, hasContent(), path(), params().toString(), method().toString())); + String.format( + Locale.ROOT, + "Content-Type and Accept headers have to match when content is present. " + + "Accept=%s Content-Type=%s hasContent=%b path=%s params=%s method=%s", + acceptHeader, + contentTypeHeader, + hasContent(), + path(), + params().toString(), + method().toString() + ) + ); } // both headers should be versioned or none - if ((cVersion == null && aVersion!=null) || (aVersion ==null && cVersion!=null) ){ + if ((cVersion == null && aVersion != null) || (aVersion == null && cVersion != null)) { throw new RestRequest.CompatibleApiHeadersCombinationException( - String.format(Locale.ROOT, "Versioning is required on both Content-Type and Accept headers. " + - "Accept=%s Content-Type=%s hasContent=%b path=%s params=%s method=%s", acceptHeader, - contentTypeHeader, hasContent(), path(), params().toString(), method().toString())); + String.format( + Locale.ROOT, + "Versioning is required on both Content-Type and Accept headers. " + + "Accept=%s Content-Type=%s hasContent=%b path=%s params=%s method=%s", + acceptHeader, + contentTypeHeader, + hasContent(), + path(), + params().toString(), + method().toString() + ) + ); } return contentTypeVersion < Version.CURRENT.major; @@ -88,6 +120,5 @@ private boolean isRequestingCompatibility() { return acceptVersion < Version.CURRENT.major; } - } } diff --git a/x-pack/plugin/rest-compat/src/test/java/org/elasticsearch/compat/RestCompatRequestPluginTests.java b/x-pack/plugin/rest-compat/src/test/java/org/elasticsearch/compat/RestCompatRequestPluginTests.java new file mode 100644 index 0000000000000..88fb908aa88a0 --- /dev/null +++ b/x-pack/plugin/rest-compat/src/test/java/org/elasticsearch/compat/RestCompatRequestPluginTests.java @@ -0,0 +1,16 @@ +/* + * 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; + +import org.elasticsearch.test.ESTestCase; + +public class RestCompatRequestPluginTests extends ESTestCase { + + public void testEmpty() { + + } +}