diff --git a/modules/rest-compatibility/src/main/java/org/elasticsearch/rest/compat/RestCompatPlugin.java b/modules/rest-compatibility/src/main/java/org/elasticsearch/rest/compat/RestCompatPlugin.java index 18b803f41ab4d..3fe0c356a5a34 100644 --- a/modules/rest-compatibility/src/main/java/org/elasticsearch/rest/compat/RestCompatPlugin.java +++ b/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 getRestHandlers( new RestGetActionV7(), new RestIndexActionV7.CompatibleRestIndexAction(), new RestIndexActionV7.CompatibleCreateHandler(), - new RestIndexActionV7.CompatibleAutoIdHandler(nodesInCluster) + new RestIndexActionV7.CompatibleAutoIdHandler(nodesInCluster), + new RestCreateIndexActionV7() ); } return Collections.emptyList(); diff --git a/modules/rest-compatibility/src/main/java/org/elasticsearch/rest/compat/version7/RestCreateIndexActionV7.java b/modules/rest-compatibility/src/main/java/org/elasticsearch/rest/compat/version7/RestCreateIndexActionV7.java new file mode 100644 index 0000000000000..6f5d3d1fe272f --- /dev/null +++ b/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 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 prepareMappings(Map source, RestRequest request) { + final boolean includeTypeName = request.paramAsBoolean(INCLUDE_TYPE_NAME_PARAMETER, false); + + @SuppressWarnings("unchecked") + Map mappings = (Map) source.get("mappings"); + + if (includeTypeName && mappings.size() == 1) { + Map newSource = new HashMap<>(); + + String typeName = mappings.keySet().iterator().next(); + @SuppressWarnings("unchecked") + Map typedMappings = (Map) 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 prepareMappings(Map source) { + if (source.containsKey("mappings") == false || (source.get("mappings") instanceof Map) == false) { + return source; + } + + Map newSource = new HashMap<>(source); + + @SuppressWarnings("unchecked") + Map mappings = (Map) 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; + } +} diff --git a/modules/rest-compatibility/src/main/java/org/elasticsearch/rest/compat/version7/RestGetActionV7.java b/modules/rest-compatibility/src/main/java/org/elasticsearch/rest/compat/version7/RestGetActionV7.java index d8d01f42698f7..b0eda49b522c5 100644 --- a/modules/rest-compatibility/src/main/java/org/elasticsearch/rest/compat/version7/RestGetActionV7.java +++ b/modules/rest-compatibility/src/main/java/org/elasticsearch/rest/compat/version7/RestGetActionV7.java @@ -45,8 +45,6 @@ public String getName() { @Override public List 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; } } diff --git a/modules/rest-compatibility/src/main/java/org/elasticsearch/rest/compat/version7/RestIndexActionV7.java b/modules/rest-compatibility/src/main/java/org/elasticsearch/rest/compat/version7/RestIndexActionV7.java index 51d4a56baa8fa..33d9ff76c2cc7 100644 --- a/modules/rest-compatibility/src/main/java/org/elasticsearch/rest/compat/version7/RestIndexActionV7.java +++ b/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 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 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; } } } diff --git a/modules/rest-compatibility/src/test/java/org/elasticsearch/rest/compat/version7/RestCreateIndexActionV7Tests.java b/modules/rest-compatibility/src/test/java/org/elasticsearch/rest/compat/version7/RestCreateIndexActionV7Tests.java new file mode 100644 index 0000000000000..781ed2e12e3cc --- /dev/null +++ b/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 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 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\"}}}}")); + } +} diff --git a/modules/rest-compatibility/src/test/java/org/elasticsearch/rest/compat/version7/RestGetActionV7Tests.java b/modules/rest-compatibility/src/test/java/org/elasticsearch/rest/compat/version7/RestGetActionV7Tests.java index c84972ad4bb33..124138ea2a142 100644 --- a/modules/rest-compatibility/src/test/java/org/elasticsearch/rest/compat/version7/RestGetActionV7Tests.java +++ b/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 contentTypeHeader = Collections.singletonList(mimeType); @Before diff --git a/modules/rest-compatibility/src/test/java/org/elasticsearch/rest/compat/version7/RestIndexActionV7Tests.java b/modules/rest-compatibility/src/test/java/org/elasticsearch/rest/compat/version7/RestIndexActionV7Tests.java index 59946abdd3d8e..6fb7b2dfac2f2 100644 --- a/modules/rest-compatibility/src/test/java/org/elasticsearch/rest/compat/version7/RestIndexActionV7Tests.java +++ b/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 contentTypeHeader = Collections.singletonList(mimeType); private final AtomicReference clusterStateSupplier = new AtomicReference<>(); diff --git a/qa/rest-compat-tests/src/main/java/org/elasticsearch/rest/compat/AbstractCompatRestTest.java b/qa/rest-compat-tests/src/main/java/org/elasticsearch/rest/compat/AbstractCompatRestTest.java index 36bfd8b56c45f..a770139c44aa3 100644 --- a/qa/rest-compat-tests/src/main/java/org/elasticsearch/rest/compat/AbstractCompatRestTest.java +++ b/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 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 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 getLocalCompatibilityTests() throws Exception { diff --git a/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java b/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java index dc378ab0d4117..7a6abc34d6b60 100644 --- a/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java +++ b/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; } diff --git a/server/src/main/java/org/elasticsearch/rest/CompatibleConstants.java b/server/src/main/java/org/elasticsearch/rest/CompatibleConstants.java index 2f3f13b893fe3..9719383c3a3ed 100644 --- a/server/src/main/java/org/elasticsearch/rest/CompatibleConstants.java +++ b/server/src/main/java/org/elasticsearch/rest/CompatibleConstants.java @@ -19,8 +19,6 @@ package org.elasticsearch.rest; -import org.elasticsearch.Version; - public class CompatibleConstants { /** @@ -28,7 +26,5 @@ public class CompatibleConstants { */ 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); } diff --git a/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java b/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java index 0d6233e62f925..a68ad4fa132b2 100644 --- a/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java +++ b/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java @@ -19,6 +19,7 @@ package org.elasticsearch.rest; +import org.elasticsearch.Version; import org.elasticsearch.common.Nullable; import java.util.HashMap; @@ -31,13 +32,14 @@ final class MethodHandlers { private final String path; - private final Map methodHandlers; + private final Map> methodHandlers; - MethodHandlers(String path, RestHandler handler, RestRequest.Method... methods) { + MethodHandlers(String path, RestHandler handler, Version version, RestRequest.Method... methods) { this.path = path; this.methodHandlers = new HashMap<>(methods.length); for (RestRequest.Method method : methods) { - methodHandlers.put(method, handler); + methodHandlers.computeIfAbsent(method, k -> new HashMap<>()) + .put(version, handler); } } @@ -45,9 +47,10 @@ final class MethodHandlers { * Add a handler for an additional array of methods. Note that {@code MethodHandlers} * does not allow replacing the handler for an already existing method. */ - MethodHandlers addMethods(RestHandler handler, RestRequest.Method... methods) { + MethodHandlers addMethods(RestHandler handler, Version version, RestRequest.Method... methods) { for (RestRequest.Method method : methods) { - RestHandler existing = methodHandlers.putIfAbsent(method, handler); + RestHandler existing = methodHandlers.computeIfAbsent(method, k -> new HashMap<>()) + .putIfAbsent(version, handler); if (existing != null) { throw new IllegalArgumentException("Cannot replace existing handler for [" + path + "] for method: " + method); } @@ -56,11 +59,26 @@ MethodHandlers addMethods(RestHandler handler, RestRequest.Method... methods) { } /** - * Returns the handler for the given method or {@code null} if none exists. + * Return a handler for a given method and a version + * If a handler for a given version is not found, the handler for Version.CURRENT is returned. + * We only expect Version.CURRENT or Version.CURRENT -1. This is validated. + * + * Handlers can be registered under the same path and method, but will require to have different versions (CURRENT or CURRENT-1) + * + * //todo What if a handler was added in V8 but was not present in V7? + * + * @param method a REST method under which a handler was registered + * @param version a Version under which a handler was registered + * @return a handler */ @Nullable - RestHandler getHandler(RestRequest.Method method) { - return methodHandlers.get(method); + RestHandler getHandler(RestRequest.Method method, Version version) { + Map versionToHandlers = methodHandlers.get(method); + if (versionToHandlers == null) { + return null; //method not found + } + final RestHandler handler = versionToHandlers.get(version); + return handler != null || version.equals(Version.CURRENT) ? handler : versionToHandlers.get(Version.CURRENT); } /** diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java index e083ea8d770d4..9b50af95d43ea 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestController.java +++ b/server/src/main/java/org/elasticsearch/rest/RestController.java @@ -23,6 +23,7 @@ import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.Version; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; @@ -32,7 +33,6 @@ import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.path.PathTrie; import org.elasticsearch.common.util.concurrent.ThreadContext; -import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.core.internal.io.Streams; @@ -54,8 +54,6 @@ import java.util.stream.Collectors; import static org.elasticsearch.rest.BytesRestResponse.TEXT_CONTENT_TYPE; -import static org.elasticsearch.rest.CompatibleConstants.COMPATIBLE_PARAMS_KEY; -import static org.elasticsearch.rest.CompatibleConstants.COMPATIBLE_VERSION; import static org.elasticsearch.rest.RestStatus.BAD_REQUEST; import static org.elasticsearch.rest.RestStatus.INTERNAL_SERVER_ERROR; import static org.elasticsearch.rest.RestStatus.METHOD_NOT_ALLOWED; @@ -147,12 +145,17 @@ protected void registerWithDeprecatedHandler(RestRequest.Method method, String p * @param method GET, POST, etc. */ protected void registerHandler(RestRequest.Method method, String path, RestHandler handler) { + assert Version.minimumRestCompatibilityVersion() == handler.compatibleWithVersion() || + Version.CURRENT == handler.compatibleWithVersion() + : "REST API compatibility is only supported for version " + Version.minimumRestCompatibilityVersion().major; + if (handler instanceof BaseRestHandler) { usageService.addRestHandler((BaseRestHandler) handler); } + final Version version = handler.compatibleWithVersion(); final RestHandler maybeWrappedHandler = handlerWrapper.apply(handler); - handlers.insertOrUpdate(path, new MethodHandlers(path, maybeWrappedHandler, method), - (mHandlers, newMHandler) -> mHandlers.addMethods(maybeWrappedHandler, method)); + handlers.insertOrUpdate(path, new MethodHandlers(path, maybeWrappedHandler, version, method), + (mHandlers, newMHandler) -> mHandlers.addMethods(maybeWrappedHandler, version, method)); } /** @@ -297,6 +300,8 @@ private void tryAllHandlers(final RestRequest request, final RestChannel channel final String rawPath = request.rawPath(); final String uri = request.uri(); + Version version = request.getCompatibleApiVersion(); + final RestRequest.Method requestMethod; try { // Resolves the HTTP method and fails if the method is invalid @@ -309,20 +314,14 @@ private void tryAllHandlers(final RestRequest request, final RestChannel channel if (handlers == null) { handler = null; } else { - handler = handlers.getHandler(requestMethod); + handler = handlers.getHandler(requestMethod, version); } if (handler == null) { if (handleNoHandlerFound(rawPath, requestMethod, uri, channel)) { return; } } else { - if(handler.compatibilityRequired() == false //regular (not removed) handlers are always dispatched - //handlers that were registered compatible, require request to be compatible - || isCompatible(request)) { - dispatchRequest(request, channel, handler); - } else { - handleBadRequest(uri, requestMethod, channel); - } + dispatchRequest(request, channel, handler); return; } } @@ -334,11 +333,6 @@ private void tryAllHandlers(final RestRequest request, final RestChannel channel handleBadRequest(uri, requestMethod, channel); } - private boolean isCompatible(ToXContent.Params params) { - String param = params.param(COMPATIBLE_PARAMS_KEY); - return COMPATIBLE_VERSION.equals(param); - } - Iterator getAllHandlers(@Nullable Map requestParamsRef, String rawPath) { final Supplier> paramsSupplier; if (requestParamsRef == null) { diff --git a/server/src/main/java/org/elasticsearch/rest/RestHandler.java b/server/src/main/java/org/elasticsearch/rest/RestHandler.java index 58c452897308e..a0bdc7ef986a2 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestHandler.java +++ b/server/src/main/java/org/elasticsearch/rest/RestHandler.java @@ -19,6 +19,7 @@ package org.elasticsearch.rest; +import org.elasticsearch.Version; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.rest.RestRequest.Method; @@ -89,8 +90,14 @@ default List replacedRoutes() { return Collections.emptyList(); } - default boolean compatibilityRequired(){ - return false; + /** + * Returns a version a handler is compatible with. + * This version is then used to math a handler with a request that specified a version. + * If no version is specified, handler is assumed to be compatible with Version.CURRENT + * @return a version + */ + default Version compatibleWithVersion(){ + return Version.CURRENT; } class Route { diff --git a/server/src/main/java/org/elasticsearch/rest/RestRequest.java b/server/src/main/java/org/elasticsearch/rest/RestRequest.java index cd52770fd6683..dcfd2f144dbe5 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -70,12 +70,12 @@ public class RestRequest implements ToXContent.Params { private final Set consumedParams = new HashSet<>(); private final SetOnce xContentType = new SetOnce<>(); private final HttpChannel httpChannel; + private final long requestId; + private final Version compatibleApiVersion; private HttpRequest httpRequest; - private boolean contentConsumed = false; - private final long requestId; public boolean isContentConsumed() { return contentConsumed; @@ -91,6 +91,7 @@ private RestRequest(NamedXContentRegistry xContentRegistry, Map this(xContentRegistry, params, path, headers, httpRequest, httpChannel, requestId, true); } + private RestRequest(NamedXContentRegistry xContentRegistry, Map params, String path, Map> headers, HttpRequest httpRequest, HttpChannel httpChannel, long requestId, boolean headersValidation) { @@ -110,7 +111,7 @@ private RestRequest(NamedXContentRegistry xContentRegistry, Map this.rawPath = path; this.headers = Collections.unmodifiableMap(headers); this.requestId = requestId; - addCompatibleParameter(headersValidation); + this.compatibleApiVersion = addCompatibleParameter(headersValidation); } protected RestRequest(RestRequest restRequest) { @@ -144,16 +145,14 @@ public static RestRequest request(NamedXContentRegistry xContentRegistry, HttpRe requestIdGenerator.incrementAndGet()); } - private void addCompatibleParameter(boolean headersValidation) { - if(headersValidation){ - if(isRequestingCompatibility()) { - params().put(CompatibleConstants.COMPATIBLE_PARAMS_KEY, - String.valueOf(Version.minimumRestCompatibilityVersion().major)); - //use it so it won't fail request validation with unused parameter - param(CompatibleConstants.COMPATIBLE_PARAMS_KEY); - } + private Version addCompatibleParameter(boolean headersValidation) { + if (headersValidation && isRequestingCompatibility()) { + return Version.minimumRestCompatibilityVersion(); + } else { + return Version.CURRENT; } } + private boolean isRequestingCompatibility() { String acceptHeader = header(CompatibleConstants.COMPATIBLE_ACCEPT_HEADER); String aVersion = XContentType.parseVersion(acceptHeader); @@ -196,6 +195,15 @@ private boolean isRequestingCompatibility() { return acceptVersion < Version.CURRENT.major; } + /** + * An http request can be accompanied with a compatible version indicating with what version a client is using. + * Only a major Versions are supported. Internally we use Versions objects, but only use Version(major,0,0) + * @return a version with what a client is compatible with. + */ + public Version getCompatibleApiVersion() { + return this.compatibleApiVersion; + } + private static Map params(final String uri) { final Map params = new HashMap<>(); int index = uri.indexOf('?'); diff --git a/server/src/test/java/org/elasticsearch/http/DefaultRestChannelTests.java b/server/src/test/java/org/elasticsearch/http/DefaultRestChannelTests.java index 8d7a026aee034..c3373cfc5840a 100644 --- a/server/src/test/java/org/elasticsearch/http/DefaultRestChannelTests.java +++ b/server/src/test/java/org/elasticsearch/http/DefaultRestChannelTests.java @@ -38,7 +38,6 @@ import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; import org.elasticsearch.rest.BytesRestResponse; -import org.elasticsearch.rest.CompatibleConstants; import org.elasticsearch.rest.RestChannel; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestResponse; @@ -216,12 +215,12 @@ public void testHeadersSet() { } public void testCompatibleParamIsSet() { - String version = String.valueOf(Version.CURRENT.major-1); + int majorVersion = Version.CURRENT.major - 1; final TestRequest httpRequest = new TestRequest(HttpRequest.HttpVersion.HTTP_1_1, RestRequest.Method.GET, "/"); - httpRequest.getHeaders().put(HttpHeaders.ACCEPT, List.of("application/vnd.elasticsearch+json;compatible-with=" + version)); + httpRequest.getHeaders().put(HttpHeaders.ACCEPT, List.of("application/vnd.elasticsearch+json;compatible-with=" + majorVersion)); final RestRequest request = RestRequest.request(xContentRegistry(), httpRequest, httpChannel); - assertEquals(version, request.param(CompatibleConstants.COMPATIBLE_PARAMS_KEY)); + assertEquals(Version.fromString(majorVersion+".0.0"), request.getCompatibleApiVersion()); } public void testCookiesSet() { diff --git a/server/src/test/java/org/elasticsearch/rest/CompatibleHeaderCombinationTests.java b/server/src/test/java/org/elasticsearch/rest/CompatibleHeaderCombinationTests.java index e38b7f9efab38..b43e7d10823b5 100644 --- a/server/src/test/java/org/elasticsearch/rest/CompatibleHeaderCombinationTests.java +++ b/server/src/test/java/org/elasticsearch/rest/CompatibleHeaderCombinationTests.java @@ -175,9 +175,8 @@ private Matcher isCompatible() { } private Matcher requestHasVersion(int version) { - return ElasticsearchMatchers.HasPropertyLambdaMatcher.hasProperty(build -> - build.param(CompatibleConstants.COMPATIBLE_PARAMS_KEY) //TODO to be refactored into getVersion - , equalTo(String.valueOf(version))); + return ElasticsearchMatchers.HasPropertyLambdaMatcher.hasProperty(restRequest -> + restRequest.getCompatibleApiVersion(), equalTo(Version.fromString(version + ".0.0"))); } private String bodyNotPresent() { diff --git a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java index 4811dbc466623..e4ab71b438113 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java @@ -45,6 +45,7 @@ import org.elasticsearch.test.rest.FakeRestRequest; import org.elasticsearch.usage.UsageService; import org.junit.Before; +import org.mockito.Mockito; import java.io.IOException; import java.util.Arrays; @@ -128,7 +129,7 @@ public MethodHandlers next() { assertEquals("true", threadContext.getHeader("header.1")); assertEquals("true", threadContext.getHeader("header.2")); assertNull(threadContext.getHeader("header.3")); - }, RestRequest.Method.GET); + }, Version.CURRENT, RestRequest.Method.GET); } }); AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST); @@ -179,7 +180,7 @@ public void testRegisterAsDeprecatedHandler() { RestRequest.Method method = randomFrom(RestRequest.Method.values()); String path = "/_" + randomAlphaOfLengthBetween(1, 6); - RestHandler handler = mock(RestHandler.class); + RestHandler handler = v8mockHandler(); String deprecationMessage = randomAlphaOfLengthBetween(1, 10); // don't want to test everything -- just that it actually wraps the handler @@ -195,7 +196,7 @@ public void testRegisterWithDeprecatedHandler() { final RestRequest.Method method = randomFrom(RestRequest.Method.values()); final String path = "/_" + randomAlphaOfLengthBetween(1, 6); - final RestHandler handler = mock(RestHandler.class); + final RestHandler handler = v8mockHandler(); final RestRequest.Method deprecatedMethod = randomFrom(RestRequest.Method.values()); final String deprecatedPath = "/_" + randomAlphaOfLengthBetween(1, 6); @@ -220,7 +221,7 @@ public void testRegisterSecondMethodWithDifferentNamedWildcard() { final String path = "/_" + randomAlphaOfLengthBetween(1, 6); - RestHandler handler = mock(RestHandler.class); + RestHandler handler = v8mockHandler(); restController.registerHandler(firstMethod, path + "/{wildcard1}", handler); IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, @@ -229,6 +230,12 @@ public void testRegisterSecondMethodWithDifferentNamedWildcard() { assertThat(exception.getMessage(), equalTo("Trying to use conflicting wildcard names for same path: wildcard1 and wildcard2")); } + private RestHandler v8mockHandler() { + RestHandler mock = mock(RestHandler.class); + Mockito.when(mock.compatibleWithVersion()).thenReturn(Version.CURRENT); + return mock; + } + public void testRestHandlerWrapper() throws Exception { AtomicBoolean handlerCalled = new AtomicBoolean(false); AtomicBoolean wrapperCalled = new AtomicBoolean(false); @@ -627,8 +634,8 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c } @Override - public boolean compatibilityRequired() { - return true; + public Version compatibleWithVersion() { + return Version.V_7_0_0; } }); @@ -637,6 +644,22 @@ public boolean compatibilityRequired() { assertTrue(channel.getSendResponseCalled()); } + public void testRegisterIncompatibleVersionHandler() { + final byte version = (byte) (Version.CURRENT.major - 2); + + expectThrows(AssertionError.class, + () -> restController.registerHandler(RestRequest.Method.GET, "/foo", new RestHandler() { + @Override + public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { + } + + @Override + public Version compatibleWithVersion() { + return Version.fromString(version + ".0.0"); + } + })); + } + private String randomCompatibleMimeType(byte version) { String subtype = randomFrom(Stream.of(XContentType.values()) .map(XContentType::shortName) diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java b/test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java index 093b5ce28651b..5c09720d8d1d7 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java @@ -183,7 +183,7 @@ public Builder(NamedXContentRegistry xContentRegistry) { } public Builder withHeaders(Map> headers) { - this.headers = headers; + this.headers.putAll(headers); return this; } 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 a5d932a3d1a3d..5aea384de6a56 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 @@ -38,7 +38,7 @@ * that can be used to register individual REST actions, and test request handling. */ public abstract class RestActionTestCase extends ESTestCase { - private RestController controller; + protected RestController controller; protected NodeClient nodeClient; @Before diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java index 9fbd15dc187a7..f0f80c278c60a 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java @@ -9,6 +9,7 @@ import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.util.Supplier; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.util.concurrent.ThreadContext; @@ -129,7 +130,7 @@ private RestRequest maybeWrapRestRequest(RestRequest restRequest) throws IOExcep } @Override - public boolean compatibilityRequired() { - return restHandler.compatibilityRequired(); + public Version compatibleWithVersion() { + return restHandler.compatibleWithVersion(); } }