From 967540d06d9578fa8b590083a0c75d0dc821f2b1 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Mon, 8 Feb 2021 11:14:47 +0100 Subject: [PATCH 1/8] Refactor usage of compatible version Compatible API version is at the moment represented by both Version and byte - representing a major version. This can lead to a confusion which representation to use, as well as to incorrect assumptions that minor versions are supperted (with the use of Version.V_7_0_0) Current usage of XContentParser.useCompatible is also not allowing to define two compatibile implementations. This is not about support N-2 compatibility, but to allow to continue development when a major release is performed. This commit is introducing the CompatibleVersion object responsible for wrapping around a major version of compatible API. --- .../compatibility/CompatibleVersion.java | 34 +++++++++++++++++++ .../common/xcontent/XContentBuilder.java | 22 +++++++----- .../main/java/org/elasticsearch/Version.java | 6 ---- .../elasticsearch/rest/MethodHandlers.java | 10 +++--- .../rest/RestCompatibleVersionHelper.java | 27 ++++++++------- .../elasticsearch/rest/RestController.java | 24 ++++++------- .../org/elasticsearch/rest/RestHandler.java | 6 ++-- .../org/elasticsearch/rest/RestRequest.java | 8 ++--- .../CompatibleNamedXContentRegistryTests.java | 3 +- .../rest/MethodHandlersTests.java | 20 +++++------ .../RestCompatibleVersionHelperTests.java | 7 ++-- .../rest/RestControllerTests.java | 29 ++++++++-------- .../security/rest/SecurityRestFilter.java | 8 ++--- 13 files changed, 119 insertions(+), 85 deletions(-) create mode 100644 libs/core/src/main/java/org/elasticsearch/common/compatibility/CompatibleVersion.java diff --git a/libs/core/src/main/java/org/elasticsearch/common/compatibility/CompatibleVersion.java b/libs/core/src/main/java/org/elasticsearch/common/compatibility/CompatibleVersion.java new file mode 100644 index 0000000000000..878647bfb77b8 --- /dev/null +++ b/libs/core/src/main/java/org/elasticsearch/common/compatibility/CompatibleVersion.java @@ -0,0 +1,34 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.common.compatibility; + +public enum CompatibleVersion { + + V_8(8), + V_7(7); + + public static final CompatibleVersion CURRENT = V_8; + public byte major; + + CompatibleVersion(int major) { + this.major = (byte) major; + } + + public CompatibleVersion previousMajor() { + return fromMajorVersion(major - 1); + } + + public static CompatibleVersion fromMajorVersion(int majorVersion) { + return valueOf("V_" + majorVersion); + } + + public static CompatibleVersion minimumRestCompatibilityVersion() { + return CURRENT.previousMajor(); + } +} diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java index f3ddff5b58746..2a20a06473bb7 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java @@ -8,6 +8,8 @@ package org.elasticsearch.common.xcontent; +import org.elasticsearch.common.compatibility.CompatibleVersion; + import java.io.ByteArrayOutputStream; import java.io.Closeable; import java.io.Flushable; @@ -155,7 +157,7 @@ public interface HumanReadableTransformer { */ private boolean humanReadable = false; - private byte compatibleMajorVersion; + private CompatibleVersion compatibleVersion; private ParsedMediaType responseContentType; @@ -1006,21 +1008,23 @@ public XContentBuilder copyCurrentStructure(XContentParser parser) throws IOExce /** * Sets a version used for serialising a response compatible with a previous version. + * @param compatibleVersion - indicates requested a version of API that the builder will be creating */ - public XContentBuilder withCompatibleMajorVersion(byte compatibleMajorVersion) { - assert this.compatibleMajorVersion == 0 : "Compatible version has already been set"; - if (compatibleMajorVersion == 0) { - throw new IllegalArgumentException("Compatible major version must not be equal to 0"); + public XContentBuilder withCompatibleVersion(CompatibleVersion compatibleVersion) { + assert this.compatibleVersion == null : "Compatible version has already been set"; + if (compatibleVersion == null) { + throw new IllegalArgumentException("Compatible major version must not be null"); } - this.compatibleMajorVersion = compatibleMajorVersion; + this.compatibleVersion = compatibleVersion; return this; } /** - * Returns a version used for serialising a response compatible with a previous version. + * Returns a version used for serialising a response. + * @return a compatible version */ - public byte getCompatibleMajorVersion() { - return compatibleMajorVersion; + public CompatibleVersion getCompatibleVersion() { + return compatibleVersion; } @Override diff --git a/server/src/main/java/org/elasticsearch/Version.java b/server/src/main/java/org/elasticsearch/Version.java index 792fa066ac161..8b9074ebd87e5 100644 --- a/server/src/main/java/org/elasticsearch/Version.java +++ b/server/src/main/java/org/elasticsearch/Version.java @@ -364,12 +364,6 @@ public boolean isCompatible(Version version) { return compatible; } - /** - * Returns the minimum version that can be used for compatible REST API - */ - public Version minimumRestCompatibilityVersion() { - return this.previousMajor(); - } /** * Returns a first major version previous to the version stored in this object. diff --git a/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java b/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java index e893c5fa639c6..1bcf06bc725e4 100644 --- a/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java +++ b/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java @@ -8,7 +8,7 @@ package org.elasticsearch.rest; -import org.elasticsearch.Version; +import org.elasticsearch.common.compatibility.CompatibleVersion; import java.util.HashMap; import java.util.Map; @@ -20,7 +20,7 @@ final class MethodHandlers { private final String path; - private final Map> methodHandlers; + private final Map> methodHandlers; MethodHandlers(String path, RestHandler handler, RestRequest.Method... methods) { this.path = path; @@ -54,13 +54,13 @@ MethodHandlers addMethods(RestHandler handler, RestRequest.Method... methods) { * (as opposed to non-compatible/breaking) * or {@code null} if none exists. */ - RestHandler getHandler(RestRequest.Method method, Version version) { - Map versionToHandlers = methodHandlers.get(method); + RestHandler getHandler(RestRequest.Method method, CompatibleVersion version) { + Map versionToHandlers = methodHandlers.get(method); if (versionToHandlers == null) { return null; //method not found } final RestHandler handler = versionToHandlers.get(version); - return handler == null ? versionToHandlers.get(Version.CURRENT) : handler; + return handler == null ? versionToHandlers.get(CompatibleVersion.CURRENT) : handler; } diff --git a/server/src/main/java/org/elasticsearch/rest/RestCompatibleVersionHelper.java b/server/src/main/java/org/elasticsearch/rest/RestCompatibleVersionHelper.java index fe53ae212939f..147567c18bfbf 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestCompatibleVersionHelper.java +++ b/server/src/main/java/org/elasticsearch/rest/RestCompatibleVersionHelper.java @@ -17,6 +17,7 @@ import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.Version; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.compatibility.CompatibleVersion; import org.elasticsearch.common.xcontent.MediaType; import org.elasticsearch.common.xcontent.ParsedMediaType; @@ -27,23 +28,23 @@ */ class RestCompatibleVersionHelper { - static Version getCompatibleVersion( + static CompatibleVersion getCompatibleVersion( @Nullable ParsedMediaType acceptHeader, @Nullable ParsedMediaType contentTypeHeader, boolean hasContent ) { Byte aVersion = parseVersion(acceptHeader); - byte acceptVersion = aVersion == null ? Version.CURRENT.major : Integer.valueOf(aVersion).byteValue(); + byte acceptVersion = aVersion == null ? CompatibleVersion.CURRENT.major : Integer.valueOf(aVersion).byteValue(); Byte cVersion = parseVersion(contentTypeHeader); - byte contentTypeVersion = cVersion == null ? Version.CURRENT.major : Integer.valueOf(cVersion).byteValue(); + byte contentTypeVersion = cVersion == null ? CompatibleVersion.CURRENT.major : Integer.valueOf(cVersion).byteValue(); // accept version must be current or prior - if (acceptVersion > Version.CURRENT.major || acceptVersion < Version.CURRENT.minimumRestCompatibilityVersion().major) { + if (acceptVersion > CompatibleVersion.CURRENT.major || acceptVersion < CompatibleVersion.minimumRestCompatibilityVersion().major) { throw new ElasticsearchStatusException( "Accept version must be either version {} or {}, but found {}. Accept={}", RestStatus.BAD_REQUEST, - Version.CURRENT.major, - Version.CURRENT.minimumRestCompatibilityVersion().major, + CompatibleVersion.CURRENT.major, + CompatibleVersion.minimumRestCompatibilityVersion().major, acceptVersion, acceptHeader ); @@ -51,13 +52,13 @@ static Version getCompatibleVersion( if (hasContent) { // content-type version must be current or prior - if (contentTypeVersion > Version.CURRENT.major - || contentTypeVersion < Version.CURRENT.minimumRestCompatibilityVersion().major) { + if (contentTypeVersion > CompatibleVersion.CURRENT.major + || contentTypeVersion < CompatibleVersion.minimumRestCompatibilityVersion().major) { throw new ElasticsearchStatusException( "Content-Type version must be either version {} or {}, but found {}. Content-Type={}", RestStatus.BAD_REQUEST, - Version.CURRENT.major, - Version.CURRENT.minimumRestCompatibilityVersion().major, + CompatibleVersion.CURRENT.major, + CompatibleVersion.minimumRestCompatibilityVersion().major, contentTypeVersion, contentTypeHeader ); @@ -84,15 +85,15 @@ static Version getCompatibleVersion( ); } if (contentTypeVersion < Version.CURRENT.major) { - return Version.CURRENT.previousMajor(); + return CompatibleVersion.fromMajorVersion(Version.CURRENT.previousMajor().major); } } if (acceptVersion < Version.CURRENT.major) { - return Version.CURRENT.previousMajor(); + return CompatibleVersion.fromMajorVersion(Version.CURRENT.previousMajor().major); } - return Version.CURRENT; + return CompatibleVersion.fromMajorVersion(Version.CURRENT.major); } static Byte parseVersion(ParsedMediaType parsedMediaType) { diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java index b10ab89f6ada0..ef86cc0f91647 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestController.java +++ b/server/src/main/java/org/elasticsearch/rest/RestController.java @@ -12,13 +12,13 @@ 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; import org.elasticsearch.common.breaker.CircuitBreaker; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.compatibility.CompatibleVersion; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.path.PathTrie; @@ -158,9 +158,9 @@ protected void registerHandler(RestRequest.Method method, String path, RestHandl } private void registerHandlerNoWrap(RestRequest.Method method, String path, RestHandler maybeWrappedHandler) { - final Version version = maybeWrappedHandler.compatibleWithVersion(); - assert Version.CURRENT.minimumRestCompatibilityVersion() == version || Version.CURRENT == version - : "REST API compatibility is only supported for version " + Version.CURRENT.minimumRestCompatibilityVersion().major; + final CompatibleVersion version = maybeWrappedHandler.compatibleWithVersion(); + assert CompatibleVersion.minimumRestCompatibilityVersion() == version || CompatibleVersion.CURRENT == version + : "REST API compatibility is only supported for version " + CompatibleVersion.minimumRestCompatibilityVersion().major; handlers.insertOrUpdate(path, new MethodHandlers(path, maybeWrappedHandler, method), (mHandlers, newMHandler) -> mHandlers.addMethods(maybeWrappedHandler, method)); @@ -214,7 +214,7 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th } } - private void dispatchRequest(RestRequest request, RestChannel channel, RestHandler handler, Version compatibleVersion) + private void dispatchRequest(RestRequest request, RestChannel channel, RestHandler handler, CompatibleVersion compatibleVersion) throws Exception { final int contentLength = request.contentLength(); if (contentLength > 0) { @@ -316,7 +316,7 @@ private void tryAllHandlers(final RestRequest request, final RestChannel channel final String uri = request.uri(); final RestRequest.Method requestMethod; - Version compatibleVersion = request.getCompatibleVersion(); + CompatibleVersion compatibleVersion = request.getCompatibleVersion(); try { // Resolves the HTTP method and fails if the method is invalid requestMethod = request.method(); @@ -453,11 +453,11 @@ private static final class ResourceHandlingHttpChannel implements RestChannel { private final RestChannel delegate; private final CircuitBreakerService circuitBreakerService; private final int contentLength; - private final Version compatibleVersion; + private final CompatibleVersion compatibleVersion; private final AtomicBoolean closed = new AtomicBoolean(); ResourceHandlingHttpChannel(RestChannel delegate, CircuitBreakerService circuitBreakerService, int contentLength, - Version compatibleVersion) { + CompatibleVersion compatibleVersion) { this.delegate = delegate; this.circuitBreakerService = circuitBreakerService; this.contentLength = contentLength; @@ -467,26 +467,26 @@ private static final class ResourceHandlingHttpChannel implements RestChannel { @Override public XContentBuilder newBuilder() throws IOException { return delegate.newBuilder() - .withCompatibleMajorVersion(compatibleVersion.major); + .withCompatibleVersion(compatibleVersion); } @Override public XContentBuilder newErrorBuilder() throws IOException { return delegate.newErrorBuilder() - .withCompatibleMajorVersion(compatibleVersion.major); + .withCompatibleVersion(compatibleVersion); } @Override public XContentBuilder newBuilder(@Nullable XContentType xContentType, boolean useFiltering) throws IOException { return delegate.newBuilder(xContentType, useFiltering) - .withCompatibleMajorVersion(compatibleVersion.major); + .withCompatibleVersion(compatibleVersion); } @Override public XContentBuilder newBuilder(XContentType xContentType, XContentType responseContentType, boolean useFiltering) throws IOException { return delegate.newBuilder(xContentType, responseContentType, useFiltering) - .withCompatibleMajorVersion(compatibleVersion.major); + .withCompatibleVersion(compatibleVersion); } @Override diff --git a/server/src/main/java/org/elasticsearch/rest/RestHandler.java b/server/src/main/java/org/elasticsearch/rest/RestHandler.java index 88e271b63e919..90d277d8dba1d 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestHandler.java +++ b/server/src/main/java/org/elasticsearch/rest/RestHandler.java @@ -8,8 +8,8 @@ package org.elasticsearch.rest; -import org.elasticsearch.Version; import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.compatibility.CompatibleVersion; import org.elasticsearch.common.xcontent.MediaType; import org.elasticsearch.common.xcontent.MediaTypeRegistry; import org.elasticsearch.common.xcontent.XContent; @@ -102,8 +102,8 @@ default MediaTypeRegistry validAcceptMediaTypes() { * If no version is specified, handler is assumed to be compatible with Version.CURRENT * @return a version */ - default Version compatibleWithVersion() { - return Version.CURRENT; + default CompatibleVersion compatibleWithVersion() { + return CompatibleVersion.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 8a2d460dba737..a8e40a443fe8c 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -10,7 +10,6 @@ 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; @@ -18,6 +17,7 @@ import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.collect.Tuple; +import org.elasticsearch.common.compatibility.CompatibleVersion; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; @@ -62,7 +62,7 @@ public class RestRequest implements ToXContent.Params { private final HttpChannel httpChannel; private final ParsedMediaType parsedAccept; private final ParsedMediaType parsedContentType; - private final Version compatibleVersion; + private final CompatibleVersion compatibleVersion; private HttpRequest httpRequest; private boolean contentConsumed = false; @@ -439,7 +439,7 @@ public NamedXContentRegistry getXContentRegistry() { public final XContentParser contentParser() throws IOException { BytesReference content = requiredContent(); // will throw exception if body or content type missing XContent xContent = xContentType.get().xContent(); - if (compatibleVersion == Version.CURRENT.minimumRestCompatibilityVersion()) { + if (compatibleVersion == CompatibleVersion.minimumRestCompatibilityVersion()) { return xContent.createParserForCompatibility(xContentRegistry, LoggingDeprecationHandler.INSTANCE, content.streamInput()); } else { return xContent.createParser(xContentRegistry, LoggingDeprecationHandler.INSTANCE, content.streamInput()); @@ -551,7 +551,7 @@ public static XContentType parseContentType(List header) { throw new IllegalArgumentException("empty Content-Type header"); } - public Version getCompatibleVersion() { + public CompatibleVersion getCompatibleVersion() { return compatibleVersion; } diff --git a/server/src/test/java/org/elasticsearch/common/xcontent/CompatibleNamedXContentRegistryTests.java b/server/src/test/java/org/elasticsearch/common/xcontent/CompatibleNamedXContentRegistryTests.java index 5792ddbe95c9b..ee8845bc44b07 100644 --- a/server/src/test/java/org/elasticsearch/common/xcontent/CompatibleNamedXContentRegistryTests.java +++ b/server/src/test/java/org/elasticsearch/common/xcontent/CompatibleNamedXContentRegistryTests.java @@ -11,6 +11,7 @@ import org.elasticsearch.Version; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.compatibility.CompatibleVersion; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.rest.FakeRestRequest; @@ -138,7 +139,7 @@ public void testCompatibleRequest() throws IOException { b.endObject(); String mediaType = XContentType.VND_JSON.toParsedMediaType() .responseContentTypeHeader(Map.of(MediaType.COMPATIBLE_WITH_PARAMETER_NAME, - String.valueOf(Version.CURRENT.minimumRestCompatibilityVersion().major))); + String.valueOf(CompatibleVersion.minimumRestCompatibilityVersion().major))); List mediaTypeList = Collections.singletonList(mediaType); RestRequest restRequest2 = new FakeRestRequest.Builder(compatibleRegistry) diff --git a/server/src/test/java/org/elasticsearch/rest/MethodHandlersTests.java b/server/src/test/java/org/elasticsearch/rest/MethodHandlersTests.java index 312f26ea272ff..7d3613bd9f72f 100644 --- a/server/src/test/java/org/elasticsearch/rest/MethodHandlersTests.java +++ b/server/src/test/java/org/elasticsearch/rest/MethodHandlersTests.java @@ -8,8 +8,8 @@ package org.elasticsearch.rest; -import org.elasticsearch.Version; import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.compatibility.CompatibleVersion; import org.elasticsearch.test.ESTestCase; import static org.hamcrest.Matchers.sameInstance; @@ -22,7 +22,7 @@ public void testLookupForDifferentMethodsSameVersion() { MethodHandlers methodHandlers = new MethodHandlers("path", putHandler, RestRequest.Method.PUT); methodHandlers.addMethods(postHandler, RestRequest.Method.POST); - RestHandler handler = methodHandlers.getHandler(RestRequest.Method.PUT, Version.CURRENT); + RestHandler handler = methodHandlers.getHandler(RestRequest.Method.PUT, CompatibleVersion.CURRENT); assertThat(handler, sameInstance(putHandler)); } @@ -30,10 +30,10 @@ public void testLookupForHandlerUnderMultipleMethods() { RestHandler handler = new CurrentVersionHandler(); MethodHandlers methodHandlers = new MethodHandlers("path", handler, RestRequest.Method.PUT, RestRequest.Method.POST); - RestHandler handlerFound = methodHandlers.getHandler(RestRequest.Method.PUT, Version.CURRENT); + RestHandler handlerFound = methodHandlers.getHandler(RestRequest.Method.PUT, CompatibleVersion.CURRENT); assertThat(handlerFound, sameInstance(handler)); - handlerFound = methodHandlers.getHandler(RestRequest.Method.POST, Version.CURRENT); + handlerFound = methodHandlers.getHandler(RestRequest.Method.POST, CompatibleVersion.CURRENT); assertThat(handlerFound, sameInstance(handler)); } @@ -43,10 +43,10 @@ public void testLookupForHandlersUnderDifferentVersions() { MethodHandlers methodHandlers = new MethodHandlers("path", currentVersionHandler, RestRequest.Method.PUT); methodHandlers.addMethods(previousVersionHandler, RestRequest.Method.PUT); - RestHandler handler = methodHandlers.getHandler(RestRequest.Method.PUT, Version.CURRENT); + RestHandler handler = methodHandlers.getHandler(RestRequest.Method.PUT, CompatibleVersion.CURRENT); assertThat(handler, sameInstance(currentVersionHandler)); - handler = methodHandlers.getHandler(RestRequest.Method.PUT, Version.CURRENT.previousMajor()); + handler = methodHandlers.getHandler(RestRequest.Method.PUT, CompatibleVersion.CURRENT.previousMajor()); assertThat(handler, sameInstance(previousVersionHandler)); } @@ -60,14 +60,14 @@ public void testExceptionOnOverride() { public void testMissingCurrentHandler(){ RestHandler previousVersionHandler = new PreviousVersionHandler(); MethodHandlers methodHandlers = new MethodHandlers("path", previousVersionHandler, RestRequest.Method.PUT, RestRequest.Method.POST); - RestHandler handler = methodHandlers.getHandler(RestRequest.Method.PUT, Version.CURRENT); + RestHandler handler = methodHandlers.getHandler(RestRequest.Method.PUT, CompatibleVersion.CURRENT); assertNull(handler); } public void testMissingPriorHandlerReturnsCurrentHandler(){ RestHandler currentVersionHandler = new CurrentVersionHandler(); MethodHandlers methodHandlers = new MethodHandlers("path", currentVersionHandler, RestRequest.Method.PUT, RestRequest.Method.POST); - RestHandler handler = methodHandlers.getHandler(RestRequest.Method.PUT, Version.CURRENT.previousMajor()); + RestHandler handler = methodHandlers.getHandler(RestRequest.Method.PUT, CompatibleVersion.CURRENT.previousMajor()); assertThat(handler, sameInstance(currentVersionHandler)); } @@ -85,8 +85,8 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c } @Override - public Version compatibleWithVersion() { - return Version.CURRENT.previousMajor(); + public CompatibleVersion compatibleWithVersion() { + return CompatibleVersion.CURRENT.previousMajor(); } } } diff --git a/server/src/test/java/org/elasticsearch/rest/RestCompatibleVersionHelperTests.java b/server/src/test/java/org/elasticsearch/rest/RestCompatibleVersionHelperTests.java index 2548b2dfd5873..576ff1d5d60dd 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestCompatibleVersionHelperTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestCompatibleVersionHelperTests.java @@ -9,6 +9,7 @@ import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.Version; +import org.elasticsearch.common.compatibility.CompatibleVersion; import org.elasticsearch.common.xcontent.ParsedMediaType; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.hamcrest.ElasticsearchMatchers; @@ -322,11 +323,11 @@ public void testVersionParsing() { } - private Matcher isCompatible() { + private Matcher isCompatible() { return requestHasVersion(PREVIOUS_VERSION); } - private Matcher requestHasVersion(int version) { + private Matcher requestHasVersion(int version) { return ElasticsearchMatchers.HasPropertyLambdaMatcher.hasProperty(v -> (int) v.major, equalTo(version)); } @@ -361,7 +362,7 @@ private String mediaType(String version) { return null; } - private Version requestWith(String accept, String contentType, String body) { + private CompatibleVersion requestWith(String accept, String contentType, String body) { ParsedMediaType parsedAccept = ParsedMediaType.parseMediaType(accept); ParsedMediaType parsedContentType = ParsedMediaType.parseMediaType(contentType); return RestCompatibleVersionHelper.getCompatibleVersion(parsedAccept, parsedContentType, body.isEmpty() == false); diff --git a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java index fa3d5c37c3802..a45cae1a8bad1 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java @@ -13,6 +13,7 @@ import org.elasticsearch.common.breaker.CircuitBreaker; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.compatibility.CompatibleVersion; import org.elasticsearch.common.component.AbstractLifecycleComponent; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; @@ -233,7 +234,7 @@ public void testRegisterSecondMethodWithDifferentNamedWildcard() { private RestHandler v8mockHandler() { RestHandler mock = mock(RestHandler.class); - Mockito.when(mock.compatibleWithVersion()).thenReturn(Version.CURRENT); + Mockito.when(mock.compatibleWithVersion()).thenReturn(CompatibleVersion.CURRENT); return mock; } @@ -627,7 +628,7 @@ public void testDispatchCompatibleHandler() { RestController restController = new RestController(Collections.emptySet(), null, client, circuitBreakerService, usageService); - final byte version = Version.CURRENT.minimumRestCompatibilityVersion().major; + final byte version = CompatibleVersion.minimumRestCompatibilityVersion().major; final String mediaType = randomCompatibleMediaType(version); FakeRestRequest fakeRestRequest = requestWithContent(mediaType); @@ -638,13 +639,13 @@ public void testDispatchCompatibleHandler() { public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { assertThat(request.contentParser().useCompatibility(), is(true)); XContentBuilder xContentBuilder = channel.newBuilder(); - assertThat(xContentBuilder.getCompatibleMajorVersion(), equalTo(version)); + assertThat(xContentBuilder.getCompatibleVersion(), equalTo(CompatibleVersion.fromMajorVersion(version))); channel.sendResponse(new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); } @Override - public Version compatibleWithVersion() { - return Version.CURRENT.minimumRestCompatibilityVersion(); + public CompatibleVersion compatibleWithVersion() { + return CompatibleVersion.minimumRestCompatibilityVersion(); } }); @@ -657,7 +658,7 @@ public void testDispatchCompatibleRequestToNewlyAddedHandler() { RestController restController = new RestController(Collections.emptySet(), null, client, circuitBreakerService, usageService); - final byte version = Version.CURRENT.minimumRestCompatibilityVersion().major; + final byte version = CompatibleVersion.minimumRestCompatibilityVersion().major; final String mediaType = randomCompatibleMediaType(version); FakeRestRequest fakeRestRequest = requestWithContent(mediaType); @@ -673,13 +674,13 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c // even though the handler is CURRENT, the xContentBuilder has the version requested by a client. // This allows to implement the compatible logic within the serialisation without introducing V7 (compatible) handler // when only response shape has changed - assertThat(xContentBuilder.getCompatibleMajorVersion(), equalTo(version)); + assertThat(xContentBuilder.getCompatibleVersion(), equalTo(CompatibleVersion.fromMajorVersion(version))); channel.sendResponse(new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); } @Override - public Version compatibleWithVersion() { - return Version.CURRENT; + public CompatibleVersion compatibleWithVersion() { + return CompatibleVersion.CURRENT; } }); @@ -716,13 +717,13 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c assertThat(request.contentParser().useCompatibility(), is(false)); XContentBuilder xContentBuilder = channel.newBuilder(); - assertThat(xContentBuilder.getCompatibleMajorVersion(), equalTo(version)); + assertThat(xContentBuilder.getCompatibleVersion(), equalTo(CompatibleVersion.fromMajorVersion(version))); channel.sendResponse(new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); } @Override - public Version compatibleWithVersion() { - return Version.CURRENT; + public CompatibleVersion compatibleWithVersion() { + return CompatibleVersion.CURRENT; } }); @@ -742,8 +743,8 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c } @Override - public Version compatibleWithVersion() { - return Version.fromString(version + ".0.0"); + public CompatibleVersion compatibleWithVersion() { + return CompatibleVersion.fromMajorVersion(version); } })); } 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 32b0003dff6cc..6b57237606fac 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 @@ -11,9 +11,9 @@ import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.util.Supplier; import org.elasticsearch.ExceptionsHelper; -import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.compatibility.CompatibleVersion; import org.elasticsearch.common.util.Maps; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.MediaType; @@ -25,15 +25,13 @@ import org.elasticsearch.rest.RestHandler; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestRequest.Method; -import org.elasticsearch.rest.RestStatus; import org.elasticsearch.rest.RestRequestFilter; - +import org.elasticsearch.rest.RestStatus; import org.elasticsearch.xpack.security.authc.AuthenticationService; import org.elasticsearch.xpack.security.authc.support.SecondaryAuthenticator; import org.elasticsearch.xpack.security.transport.SSLEngineUtils; import java.io.IOException; - import java.util.List; import java.util.Map; @@ -163,7 +161,7 @@ public MediaTypeRegistry validAcceptMediaTypes() { } @Override - public Version compatibleWithVersion() { + public CompatibleVersion compatibleWithVersion() { return restHandler.compatibleWithVersion(); } } From d6db590b45e1412e3940b348a3715cac4e1694b2 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Mon, 8 Feb 2021 12:27:20 +0100 Subject: [PATCH 2/8] use compatible version in builder --- .../compatibility/CompatibleVersion.java | 1 + .../common/xcontent/XContentBuilder.java | 14 ++-- .../java/org/elasticsearch/node/Node.java | 9 ++- .../org/elasticsearch/node/NodeTests.java | 70 +++++++++++++++++++ .../rest/RestControllerTests.java | 15 ++-- 5 files changed, 98 insertions(+), 11 deletions(-) diff --git a/libs/core/src/main/java/org/elasticsearch/common/compatibility/CompatibleVersion.java b/libs/core/src/main/java/org/elasticsearch/common/compatibility/CompatibleVersion.java index 878647bfb77b8..097f2e51cb53b 100644 --- a/libs/core/src/main/java/org/elasticsearch/common/compatibility/CompatibleVersion.java +++ b/libs/core/src/main/java/org/elasticsearch/common/compatibility/CompatibleVersion.java @@ -13,6 +13,7 @@ public enum CompatibleVersion { V_8(8), V_7(7); + // This needs to be aligned with Version.CURRENT public static final CompatibleVersion CURRENT = V_8; public byte major; diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java index 2a20a06473bb7..528f3f554eeb0 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java @@ -157,7 +157,7 @@ public interface HumanReadableTransformer { */ private boolean humanReadable = false; - private CompatibleVersion compatibleVersion; + private CompatibleVersion restCompatibilityVersion; private ParsedMediaType responseContentType; @@ -1011,11 +1011,11 @@ public XContentBuilder copyCurrentStructure(XContentParser parser) throws IOExce * @param compatibleVersion - indicates requested a version of API that the builder will be creating */ public XContentBuilder withCompatibleVersion(CompatibleVersion compatibleVersion) { - assert this.compatibleVersion == null : "Compatible version has already been set"; + assert this.restCompatibilityVersion == null : "Compatible version has already been set"; if (compatibleVersion == null) { throw new IllegalArgumentException("Compatible major version must not be null"); } - this.compatibleVersion = compatibleVersion; + this.restCompatibilityVersion = compatibleVersion; return this; } @@ -1023,8 +1023,12 @@ public XContentBuilder withCompatibleVersion(CompatibleVersion compatibleVersion * Returns a version used for serialising a response. * @return a compatible version */ - public CompatibleVersion getCompatibleVersion() { - return compatibleVersion; + public CompatibleVersion getRestCompatibilityVersion() { + return restCompatibilityVersion; + } + + public boolean useCompatibility(CompatibleVersion codeChangeVersion) { + return this.restCompatibilityVersion == codeChangeVersion; } @Override diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index db2b89104cba3..f0ecdf761a8d6 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -454,8 +454,7 @@ protected Node(final Environment initialEnvironment, .flatMap(p -> p.getNamedXContent().stream()), ClusterModule.getNamedXWriteables().stream()) .flatMap(Function.identity()).collect(toList()), - pluginsService.filterPlugins(Plugin.class).stream() - .flatMap(p -> p.getNamedXContentForCompatibility().stream()).collect(toList()) + getCompatibleNamedXContents() ); final MetaStateService metaStateService = new MetaStateService(nodeEnvironment, xContentRegistry); final PersistedClusterStateService lucenePersistedStateFactory @@ -730,6 +729,12 @@ protected Node(final Environment initialEnvironment, } } + // package scope for testing + List getCompatibleNamedXContents() { + return pluginsService.filterPlugins(Plugin.class).stream() + .flatMap(p -> p.getNamedXContentForCompatibility().stream()).collect(toList()); + } + protected TransportService newTransportService(Settings settings, Transport transport, ThreadPool threadPool, TransportInterceptor interceptor, Function localNodeFactory, diff --git a/server/src/test/java/org/elasticsearch/node/NodeTests.java b/server/src/test/java/org/elasticsearch/node/NodeTests.java index fd0865061e377..3ce4d9b502744 100644 --- a/server/src/test/java/org/elasticsearch/node/NodeTests.java +++ b/server/src/test/java/org/elasticsearch/node/NodeTests.java @@ -13,9 +13,11 @@ import org.elasticsearch.bootstrap.BootstrapContext; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.common.breaker.CircuitBreaker; +import org.elasticsearch.common.compatibility.CompatibleVersion; import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.BoundTransportAddress; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.env.Environment; import org.elasticsearch.index.IndexService; import org.elasticsearch.index.engine.Engine.Searcher; @@ -29,6 +31,7 @@ import org.elasticsearch.test.InternalTestCluster; import org.elasticsearch.test.MockHttpTransport; import org.elasticsearch.threadpool.ThreadPool; +import org.mockito.Mockito; import java.io.IOException; import java.nio.file.Path; @@ -44,6 +47,7 @@ import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS; import static org.elasticsearch.test.NodeRoles.dataNode; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; @@ -328,4 +332,70 @@ public void setCircuitBreaker(CircuitBreaker circuitBreaker) { myCircuitBreaker.set(circuitBreaker); } } + + + + + interface MockCompatibleVersion { + CompatibleVersion minimumRestCompatibilityVersion() ; + } + // This test shows an example on how multiple compatible namedxcontent can be present at the same time. + public void testLoadingMultipleRestCompatibilityPlugins() throws IOException { + + MockCompatibleVersion MockCompatibleVersion = Mockito.mock(MockCompatibleVersion.class); + Mockito.when(MockCompatibleVersion.minimumRestCompatibilityVersion()) + .thenReturn(CompatibleVersion.V_7); + + NamedXContentRegistry.Entry v7CompatibleEntries = Mockito.mock(NamedXContentRegistry.Entry.class); + NamedXContentRegistry.Entry v8CompatibleEntries = Mockito.mock(NamedXContentRegistry.Entry.class); + + class TestRestCompatibility1 extends Plugin { + @Override + public List getNamedXContentForCompatibility() { + // real plugin will use CompatibleVersion.minimumRestCompatibilityVersion() + if(/*CompatibleVersion.minimumRestCompatibilityVersion()*/ + MockCompatibleVersion.minimumRestCompatibilityVersion().equals(CompatibleVersion.V_7)){ + //return set of N-1 entries + return List.of(v7CompatibleEntries); + } + // after major release, new compatible apis can be added before the old ones are removed. + if(/*CompatibleVersion.minimumRestCompatibilityVersion()*/ + MockCompatibleVersion.minimumRestCompatibilityVersion().equals(CompatibleVersion.V_8)){ + return List.of(v8CompatibleEntries); + + } + return super.getNamedXContentForCompatibility(); + } + } + + { + Settings.Builder settings = baseSettings(); + + // throw an exception when two plugins are registered + List> plugins = basePlugins(); + plugins.add(TestRestCompatibility1.class); + + try (Node node = new MockNode(settings.build(), plugins)) { + List compatibleNamedXContents = node.getCompatibleNamedXContents(); + assertThat(compatibleNamedXContents, contains(v7CompatibleEntries)); + } + } + // after version bump CompatibleVersion.minimumRestCompatibilityVersion() will return V_8 + Mockito.when(MockCompatibleVersion.minimumRestCompatibilityVersion()) + .thenReturn(CompatibleVersion.V_8); + { + Settings.Builder settings = baseSettings(); + + // throw an exception when two plugins are registered + List> plugins = basePlugins(); + plugins.add(TestRestCompatibility1.class); + + try (Node node = new MockNode(settings.build(), plugins)) { + List compatibleNamedXContents = node.getCompatibleNamedXContents(); + assertThat(compatibleNamedXContents, contains(v8CompatibleEntries)); + } + } + } + + } diff --git a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java index a45cae1a8bad1..5e04dca46399f 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java @@ -637,9 +637,11 @@ public void testDispatchCompatibleHandler() { restController.registerHandler(RestRequest.Method.GET, "/foo", new RestHandler() { @Override public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { + // in real use case we will use exact version CompatibleVersion.V_7 assertThat(request.contentParser().useCompatibility(), is(true)); XContentBuilder xContentBuilder = channel.newBuilder(); - assertThat(xContentBuilder.getCompatibleVersion(), equalTo(CompatibleVersion.fromMajorVersion(version))); + assertFalse(xContentBuilder.useCompatibility(CompatibleVersion.minimumRestCompatibilityVersion())); + assertThat(xContentBuilder.getRestCompatibilityVersion(), equalTo(CompatibleVersion.minimumRestCompatibilityVersion())); channel.sendResponse(new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); } @@ -668,13 +670,17 @@ public void testDispatchCompatibleRequestToNewlyAddedHandler() { restController.registerHandler(RestRequest.Method.GET, "/foo", new RestHandler() { @Override public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { - assertThat(request.contentParser().useCompatibility(), is(true)); XContentBuilder xContentBuilder = channel.newBuilder(); // even though the handler is CURRENT, the xContentBuilder has the version requested by a client. // This allows to implement the compatible logic within the serialisation without introducing V7 (compatible) handler // when only response shape has changed - assertThat(xContentBuilder.getCompatibleVersion(), equalTo(CompatibleVersion.fromMajorVersion(version))); + assertThat(xContentBuilder.getRestCompatibilityVersion(), equalTo(CompatibleVersion.fromMajorVersion(version))); + + // we don't expect compatible api code to be implemented for newly added APIs, + // yet the request is compatible so the compatible flags are set on builder/parser + assertTrue(xContentBuilder.useCompatibility(CompatibleVersion.minimumRestCompatibilityVersion())); + assertThat(request.contentParser().useCompatibility(), is(true)); channel.sendResponse(new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); } @@ -717,7 +723,8 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c assertThat(request.contentParser().useCompatibility(), is(false)); XContentBuilder xContentBuilder = channel.newBuilder(); - assertThat(xContentBuilder.getCompatibleVersion(), equalTo(CompatibleVersion.fromMajorVersion(version))); + assertFalse(xContentBuilder.useCompatibility(CompatibleVersion.minimumRestCompatibilityVersion())); + assertThat(xContentBuilder.getRestCompatibilityVersion(), equalTo(CompatibleVersion.fromMajorVersion(version))); channel.sendResponse(new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); } From 722e45adefe3f7e6ef34c0fa69118885226c6c36 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Mon, 8 Feb 2021 13:33:47 +0100 Subject: [PATCH 3/8] fix tests --- .../compatibility/CompatibleVersion.java | 3 +- .../org/elasticsearch/node/NodeTests.java | 75 ++++++++++--------- .../rest/RestControllerTests.java | 2 +- 3 files changed, 43 insertions(+), 37 deletions(-) diff --git a/libs/core/src/main/java/org/elasticsearch/common/compatibility/CompatibleVersion.java b/libs/core/src/main/java/org/elasticsearch/common/compatibility/CompatibleVersion.java index 097f2e51cb53b..52c2cb0322f0d 100644 --- a/libs/core/src/main/java/org/elasticsearch/common/compatibility/CompatibleVersion.java +++ b/libs/core/src/main/java/org/elasticsearch/common/compatibility/CompatibleVersion.java @@ -11,7 +11,8 @@ public enum CompatibleVersion { V_8(8), - V_7(7); + V_7(7), + V_6(6);//used in testing, to prove validation // This needs to be aligned with Version.CURRENT public static final CompatibleVersion CURRENT = V_8; diff --git a/server/src/test/java/org/elasticsearch/node/NodeTests.java b/server/src/test/java/org/elasticsearch/node/NodeTests.java index 3ce4d9b502744..9d73b803dc37e 100644 --- a/server/src/test/java/org/elasticsearch/node/NodeTests.java +++ b/server/src/test/java/org/elasticsearch/node/NodeTests.java @@ -12,11 +12,13 @@ import org.elasticsearch.bootstrap.BootstrapCheck; import org.elasticsearch.bootstrap.BootstrapContext; import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.common.ParseField; import org.elasticsearch.common.breaker.CircuitBreaker; import org.elasticsearch.common.compatibility.CompatibleVersion; import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.BoundTransportAddress; +import org.elasticsearch.common.xcontent.ContextParser; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.env.Environment; import org.elasticsearch.index.IndexService; @@ -146,10 +148,10 @@ public void testServerNameNodeAttribute() throws IOException { private static Settings.Builder baseSettings() { final Path tempDir = createTempDir(); return Settings.builder() - .put(ClusterName.CLUSTER_NAME_SETTING.getKey(), InternalTestCluster.clusterName("single-node-cluster", randomLong())) - .put(Environment.PATH_HOME_SETTING.getKey(), tempDir) - .put(NetworkModule.TRANSPORT_TYPE_KEY, getTestTransportType()) - .put(dataNode()); + .put(ClusterName.CLUSTER_NAME_SETTING.getKey(), InternalTestCluster.clusterName("single-node-cluster", randomLong())) + .put(Environment.PATH_HOME_SETTING.getKey(), tempDir) + .put(NetworkModule.TRANSPORT_TYPE_KEY, getTestTransportType()) + .put(dataNode()); } public void testCloseOnOutstandingTask() throws Exception { @@ -160,7 +162,7 @@ public void testCloseOnOutstandingTask() throws Exception { final CountDownLatch threadRunning = new CountDownLatch(1); threadpool.executor(ThreadPool.Names.SEARCH).execute(() -> { threadRunning.countDown(); - while (shouldRun.get()); + while (shouldRun.get()) ; }); threadRunning.await(); node.close(); @@ -183,7 +185,7 @@ public void testCloseRaceWithTaskExecution() throws Exception { } try { threadpool.executor(ThreadPool.Names.SEARCH).execute(() -> { - while (shouldRun.get()); + while (shouldRun.get()) ; }); } catch (RejectedExecutionException e) { assertThat(e.getMessage(), containsString("[Terminated,")); @@ -222,7 +224,7 @@ public void testAwaitCloseTimeoutsOnNonInterruptibleTask() throws Exception { final CountDownLatch threadRunning = new CountDownLatch(1); threadpool.executor(ThreadPool.Names.SEARCH).execute(() -> { threadRunning.countDown(); - while (shouldRun.get()); + while (shouldRun.get()) ; }); threadRunning.await(); node.close(); @@ -265,7 +267,7 @@ public void testCloseOnLeakedIndexReaderReference() throws Exception { node.start(); IndicesService indicesService = node.injector().getInstance(IndicesService.class); assertAcked(node.client().admin().indices().prepareCreate("test") - .setSettings(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 1).put(SETTING_NUMBER_OF_REPLICAS, 0))); + .setSettings(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 1).put(SETTING_NUMBER_OF_REPLICAS, 0))); IndexService indexService = indicesService.iterator().next(); IndexShard shard = indexService.getShard(0); Searcher searcher = shard.acquireSearcher("test"); @@ -281,7 +283,7 @@ public void testCloseOnLeakedStoreReference() throws Exception { node.start(); IndicesService indicesService = node.injector().getInstance(IndicesService.class); assertAcked(node.client().admin().indices().prepareCreate("test") - .setSettings(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 1).put(SETTING_NUMBER_OF_REPLICAS, 0))); + .setSettings(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 1).put(SETTING_NUMBER_OF_REPLICAS, 0))); IndexService indexService = indicesService.iterator().next(); IndexShard shard = indexService.getShard(0); shard.store().incRef(); @@ -304,7 +306,7 @@ public void testCreateWithCircuitBreakerPlugins() throws IOException { CircuitBreakerPlugin breakerPlugin = node.getPluginsService().filterPlugins(CircuitBreakerPlugin.class).get(0); assertTrue(breakerPlugin instanceof MockCircuitBreakerPlugin); assertSame("plugin circuit breaker instance is not the same as breaker service's instance", - ((MockCircuitBreakerPlugin)breakerPlugin).myCircuitBreaker.get(), + ((MockCircuitBreakerPlugin) breakerPlugin).myCircuitBreaker.get(), service.getBreaker("test_breaker")); } } @@ -334,40 +336,43 @@ public void setCircuitBreaker(CircuitBreaker circuitBreaker) { } + interface MockCompatibleVersion { + CompatibleVersion minimumRestCompatibilityVersion(); + } + static MockCompatibleVersion MockCompatibleVersion = Mockito.mock(MockCompatibleVersion.class); - interface MockCompatibleVersion { - CompatibleVersion minimumRestCompatibilityVersion() ; + static NamedXContentRegistry.Entry v7CompatibleEntries = new NamedXContentRegistry.Entry(Integer.class, + new ParseField("name"), Mockito.mock(ContextParser.class)); + static NamedXContentRegistry.Entry v8CompatibleEntries = new NamedXContentRegistry.Entry(Integer.class, + new ParseField("name2"), Mockito.mock(ContextParser.class)); + + public static class TestRestCompatibility1 extends Plugin { + + @Override + public List getNamedXContentForCompatibility() { + // real plugin will use CompatibleVersion.minimumRestCompatibilityVersion() + if (/*CompatibleVersion.minimumRestCompatibilityVersion()*/ + MockCompatibleVersion.minimumRestCompatibilityVersion().equals(CompatibleVersion.V_7)) { + //return set of N-1 entries + return List.of(v7CompatibleEntries); + } + // after major release, new compatible apis can be added before the old ones are removed. + if (/*CompatibleVersion.minimumRestCompatibilityVersion()*/ + MockCompatibleVersion.minimumRestCompatibilityVersion().equals(CompatibleVersion.V_8)) { + return List.of(v8CompatibleEntries); + + } + return super.getNamedXContentForCompatibility(); + } } + // This test shows an example on how multiple compatible namedxcontent can be present at the same time. public void testLoadingMultipleRestCompatibilityPlugins() throws IOException { - MockCompatibleVersion MockCompatibleVersion = Mockito.mock(MockCompatibleVersion.class); Mockito.when(MockCompatibleVersion.minimumRestCompatibilityVersion()) .thenReturn(CompatibleVersion.V_7); - NamedXContentRegistry.Entry v7CompatibleEntries = Mockito.mock(NamedXContentRegistry.Entry.class); - NamedXContentRegistry.Entry v8CompatibleEntries = Mockito.mock(NamedXContentRegistry.Entry.class); - - class TestRestCompatibility1 extends Plugin { - @Override - public List getNamedXContentForCompatibility() { - // real plugin will use CompatibleVersion.minimumRestCompatibilityVersion() - if(/*CompatibleVersion.minimumRestCompatibilityVersion()*/ - MockCompatibleVersion.minimumRestCompatibilityVersion().equals(CompatibleVersion.V_7)){ - //return set of N-1 entries - return List.of(v7CompatibleEntries); - } - // after major release, new compatible apis can be added before the old ones are removed. - if(/*CompatibleVersion.minimumRestCompatibilityVersion()*/ - MockCompatibleVersion.minimumRestCompatibilityVersion().equals(CompatibleVersion.V_8)){ - return List.of(v8CompatibleEntries); - - } - return super.getNamedXContentForCompatibility(); - } - } - { Settings.Builder settings = baseSettings(); diff --git a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java index 5e04dca46399f..79c001a455106 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java @@ -640,7 +640,7 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c // in real use case we will use exact version CompatibleVersion.V_7 assertThat(request.contentParser().useCompatibility(), is(true)); XContentBuilder xContentBuilder = channel.newBuilder(); - assertFalse(xContentBuilder.useCompatibility(CompatibleVersion.minimumRestCompatibilityVersion())); + assertTrue(xContentBuilder.useCompatibility(CompatibleVersion.minimumRestCompatibilityVersion())); assertThat(xContentBuilder.getRestCompatibilityVersion(), equalTo(CompatibleVersion.minimumRestCompatibilityVersion())); channel.sendResponse(new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); } From 211f6ff87c26467ad12809cd078f517d9ab8528c Mon Sep 17 00:00:00 2001 From: pgomulka Date: Mon, 8 Feb 2021 18:13:12 +0100 Subject: [PATCH 4/8] javadoc --- .../common/compatibility/CompatibleVersion.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libs/core/src/main/java/org/elasticsearch/common/compatibility/CompatibleVersion.java b/libs/core/src/main/java/org/elasticsearch/common/compatibility/CompatibleVersion.java index 52c2cb0322f0d..ebe7f29d9b016 100644 --- a/libs/core/src/main/java/org/elasticsearch/common/compatibility/CompatibleVersion.java +++ b/libs/core/src/main/java/org/elasticsearch/common/compatibility/CompatibleVersion.java @@ -8,6 +8,12 @@ package org.elasticsearch.common.compatibility; +/** + * A enum representing versions which are used by a REST Compatible API. + * A CURRENT instance, represents a major Version.CURRENT from server module. + * + * Only major versions are supported. + */ public enum CompatibleVersion { V_8(8), From 5de9e93dc5085de2abf1cabd27d74d6aa07b6001 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Tue, 9 Feb 2021 11:11:53 +0100 Subject: [PATCH 5/8] parsers to have version instead of flag, review follow up --- ...ion.java => RestApiCompatibleVersion.java} | 19 ++++---- .../xcontent/NamedXContentRegistry.java | 4 +- .../common/xcontent/XContent.java | 7 ++- .../common/xcontent/XContentBuilder.java | 24 +++++----- .../common/xcontent/XContentParser.java | 3 +- .../common/xcontent/XContentSubParser.java | 5 ++- .../common/xcontent/cbor/CborXContent.java | 6 ++- .../xcontent/cbor/CborXContentParser.java | 6 ++- .../common/xcontent/json/JsonXContent.java | 6 ++- .../xcontent/json/JsonXContentParser.java | 8 ++-- .../common/xcontent/smile/SmileXContent.java | 6 ++- .../xcontent/smile/SmileXContentParser.java | 6 ++- .../support/AbstractXContentParser.java | 14 +++--- .../common/xcontent/yaml/YamlXContent.java | 6 ++- .../xcontent/yaml/YamlXContentParser.java | 6 ++- .../main/java/org/elasticsearch/Version.java | 5 ++- .../elasticsearch/rest/MethodHandlers.java | 10 ++--- .../rest/RestCompatibleVersionHelper.java | 30 +++++++------ .../elasticsearch/rest/RestController.java | 33 +++++++------- .../org/elasticsearch/rest/RestHandler.java | 6 +-- .../org/elasticsearch/rest/RestRequest.java | 18 ++++---- .../CompatibleNamedXContentRegistryTests.java | 4 +- .../org/elasticsearch/node/NodeTests.java | 16 +++---- .../rest/MethodHandlersTests.java | 20 ++++----- .../rest/RestControllerTests.java | 45 +++++++++---------- ...tRestApiCompatibleVersionHelperTests.java} | 10 ++--- .../xcontent/WatcherXContentParser.java | 5 ++- .../security/rest/SecurityRestFilter.java | 4 +- 28 files changed, 179 insertions(+), 153 deletions(-) rename libs/core/src/main/java/org/elasticsearch/common/compatibility/{CompatibleVersion.java => RestApiCompatibleVersion.java} (64%) rename server/src/test/java/org/elasticsearch/rest/{RestCompatibleVersionHelperTests.java => RestRestApiCompatibleVersionHelperTests.java} (97%) diff --git a/libs/core/src/main/java/org/elasticsearch/common/compatibility/CompatibleVersion.java b/libs/core/src/main/java/org/elasticsearch/common/compatibility/RestApiCompatibleVersion.java similarity index 64% rename from libs/core/src/main/java/org/elasticsearch/common/compatibility/CompatibleVersion.java rename to libs/core/src/main/java/org/elasticsearch/common/compatibility/RestApiCompatibleVersion.java index ebe7f29d9b016..2be75e7954223 100644 --- a/libs/core/src/main/java/org/elasticsearch/common/compatibility/CompatibleVersion.java +++ b/libs/core/src/main/java/org/elasticsearch/common/compatibility/RestApiCompatibleVersion.java @@ -14,29 +14,32 @@ * * Only major versions are supported. */ -public enum CompatibleVersion { +public enum RestApiCompatibleVersion { V_8(8), V_7(7), V_6(6);//used in testing, to prove validation - // This needs to be aligned with Version.CURRENT - public static final CompatibleVersion CURRENT = V_8; public byte major; + private static RestApiCompatibleVersion CURRENT = V_8; - CompatibleVersion(int major) { + RestApiCompatibleVersion(int major) { this.major = (byte) major; } - public CompatibleVersion previousMajor() { + public RestApiCompatibleVersion previousMajor() { return fromMajorVersion(major - 1); } - public static CompatibleVersion fromMajorVersion(int majorVersion) { + public static RestApiCompatibleVersion fromMajorVersion(int majorVersion) { return valueOf("V_" + majorVersion); } - public static CompatibleVersion minimumRestCompatibilityVersion() { - return CURRENT.previousMajor(); + public static RestApiCompatibleVersion minimumSupported() { + return currentVersion().previousMajor(); } + + public static RestApiCompatibleVersion currentVersion() { + return CURRENT; + }; } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/NamedXContentRegistry.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/NamedXContentRegistry.java index 0d7a454ac1bab..6a7964883f05f 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/NamedXContentRegistry.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/NamedXContentRegistry.java @@ -10,6 +10,7 @@ import org.elasticsearch.common.CheckedFunction; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import java.io.IOException; import java.util.ArrayList; @@ -138,7 +139,8 @@ private Map, Map> getRegistry(List entries){ */ public T parseNamedObject(Class categoryClass, String name, XContentParser parser, C context) throws IOException { - Map parsers = parser.useCompatibility() ? compatibleRegistry.get(categoryClass) : registry.get(categoryClass); + Map parsers = parser.getRestApiCompatibleVersion() == RestApiCompatibleVersion.minimumSupported() ? + compatibleRegistry.get(categoryClass) : registry.get(categoryClass); if (parsers == null) { if (registry.isEmpty()) { // The "empty" registry will never work so we throw a better exception as a hint. diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContent.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContent.java index 49dbb5a0a99dc..839a9b13a84b1 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContent.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContent.java @@ -8,6 +8,8 @@ package org.elasticsearch.common.xcontent; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; + import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -76,9 +78,10 @@ XContentParser createParser(NamedXContentRegistry xContentRegistry, DeprecationH /** * Creates a parser over the provided input stream and with the indication that a request is using REST compatible API. - * Parses XContent using the N-1 compatible logic. + * Depending on restApiCompatibleVersionParses + * @param restApiCompatibleVersion - indicates if the N-1 or N compatible XContent parsing logic will be used. */ XContentParser createParserForCompatibility(NamedXContentRegistry xContentRegistry, DeprecationHandler deprecationHandler, - InputStream is) throws IOException; + InputStream is, RestApiCompatibleVersion restApiCompatibleVersion) throws IOException; } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java index 528f3f554eeb0..e09a11530ace0 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java @@ -8,7 +8,7 @@ package org.elasticsearch.common.xcontent; -import org.elasticsearch.common.compatibility.CompatibleVersion; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import java.io.ByteArrayOutputStream; import java.io.Closeable; @@ -157,7 +157,7 @@ public interface HumanReadableTransformer { */ private boolean humanReadable = false; - private CompatibleVersion restCompatibilityVersion; + private RestApiCompatibleVersion restApiCompatibilityVersion; private ParsedMediaType responseContentType; @@ -1008,14 +1008,12 @@ public XContentBuilder copyCurrentStructure(XContentParser parser) throws IOExce /** * Sets a version used for serialising a response compatible with a previous version. - * @param compatibleVersion - indicates requested a version of API that the builder will be creating + * @param restApiCompatibleVersion - indicates requested a version of API that the builder will be creating */ - public XContentBuilder withCompatibleVersion(CompatibleVersion compatibleVersion) { - assert this.restCompatibilityVersion == null : "Compatible version has already been set"; - if (compatibleVersion == null) { - throw new IllegalArgumentException("Compatible major version must not be null"); - } - this.restCompatibilityVersion = compatibleVersion; + public XContentBuilder withCompatibleVersion(RestApiCompatibleVersion restApiCompatibleVersion) { + assert this.restApiCompatibilityVersion == null : "restApiCompatibleVersion has already been set"; + Objects.requireNonNull(restApiCompatibleVersion, "restApiCompatibleVersion cannot be null"); + this.restApiCompatibilityVersion = restApiCompatibleVersion; return this; } @@ -1023,12 +1021,12 @@ public XContentBuilder withCompatibleVersion(CompatibleVersion compatibleVersion * Returns a version used for serialising a response. * @return a compatible version */ - public CompatibleVersion getRestCompatibilityVersion() { - return restCompatibilityVersion; + public RestApiCompatibleVersion getRestApiCompatibilityVersion() { + return restApiCompatibilityVersion; } - public boolean useCompatibility(CompatibleVersion codeChangeVersion) { - return this.restCompatibilityVersion == codeChangeVersion; + public boolean useCompatibility(RestApiCompatibleVersion codeChangeVersion) { + return this.restApiCompatibilityVersion == codeChangeVersion; } @Override diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java index d653f8becfe3f..b7dea17ee5d70 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java @@ -9,6 +9,7 @@ package org.elasticsearch.common.xcontent; import org.elasticsearch.common.CheckedFunction; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import java.io.Closeable; import java.io.IOException; @@ -251,7 +252,7 @@ Map map( boolean isClosed(); - boolean useCompatibility(); + RestApiCompatibleVersion getRestApiCompatibleVersion(); /** * The callback to notify when parsing encounters a deprecated field. diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentSubParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentSubParser.java index c9c214470426c..b318bc5ad343d 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentSubParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentSubParser.java @@ -9,6 +9,7 @@ package org.elasticsearch.common.xcontent; import org.elasticsearch.common.CheckedFunction; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import java.io.IOException; import java.nio.CharBuffer; @@ -259,8 +260,8 @@ public boolean isClosed() { } @Override - public boolean useCompatibility() { - return parser.useCompatibility(); + public RestApiCompatibleVersion getRestApiCompatibleVersion() { + return parser.getRestApiCompatibleVersion(); } @Override diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContent.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContent.java index 864e3089f2279..bad16abe7d216 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContent.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContent.java @@ -12,6 +12,7 @@ import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.dataformat.cbor.CBORFactory; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.xcontent.DeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContent; @@ -98,9 +99,10 @@ public XContentParser createParser(NamedXContentRegistry xContentRegistry, @Override public XContentParser createParserForCompatibility(NamedXContentRegistry xContentRegistry, - DeprecationHandler deprecationHandler, InputStream is) + DeprecationHandler deprecationHandler, InputStream is, + RestApiCompatibleVersion restApiCompatibleVersion) throws IOException { - return new CborXContentParser(xContentRegistry, deprecationHandler, cborFactory.createParser(is), true); + return new CborXContentParser(xContentRegistry, deprecationHandler, cborFactory.createParser(is), restApiCompatibleVersion); } } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContentParser.java index 67e848cc5c368..a12b50efc7aac 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContentParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContentParser.java @@ -9,6 +9,7 @@ package org.elasticsearch.common.xcontent.cbor; import com.fasterxml.jackson.core.JsonParser; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.xcontent.DeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentType; @@ -22,8 +23,9 @@ public CborXContentParser(NamedXContentRegistry xContentRegistry, } public CborXContentParser(NamedXContentRegistry xContentRegistry, - DeprecationHandler deprecationHandler, JsonParser parser, boolean useCompatibility) { - super(xContentRegistry, deprecationHandler, parser, useCompatibility); + DeprecationHandler deprecationHandler, JsonParser parser, + RestApiCompatibleVersion restApiCompatibleVersion) { + super(xContentRegistry, deprecationHandler, parser, restApiCompatibleVersion); } @Override diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContent.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContent.java index c237169b7abe5..21fb8e9c3e4e5 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContent.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContent.java @@ -12,6 +12,7 @@ import com.fasterxml.jackson.core.JsonFactory; import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonParser; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.xcontent.DeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContent; @@ -99,8 +100,9 @@ public XContentParser createParser(NamedXContentRegistry xContentRegistry, @Override public XContentParser createParserForCompatibility(NamedXContentRegistry xContentRegistry, - DeprecationHandler deprecationHandler, InputStream is) throws IOException { - return new JsonXContentParser(xContentRegistry, deprecationHandler, jsonFactory.createParser(is), true); + DeprecationHandler deprecationHandler, InputStream is, + RestApiCompatibleVersion restApiCompatibleVersion) throws IOException { + return new JsonXContentParser(xContentRegistry, deprecationHandler, jsonFactory.createParser(is), restApiCompatibleVersion); } } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentParser.java index 801c6058f3da4..20038b1dc3c1c 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentParser.java @@ -11,6 +11,7 @@ import com.fasterxml.jackson.core.JsonLocation; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.core.JsonToken; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.xcontent.DeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentLocation; @@ -27,13 +28,14 @@ public class JsonXContentParser extends AbstractXContentParser { public JsonXContentParser(NamedXContentRegistry xContentRegistry, DeprecationHandler deprecationHandler, JsonParser parser) { - super(xContentRegistry, deprecationHandler, false); + super(xContentRegistry, deprecationHandler, RestApiCompatibleVersion.currentVersion()); this.parser = parser; } public JsonXContentParser(NamedXContentRegistry xContentRegistry, - DeprecationHandler deprecationHandler, JsonParser parser, boolean useCompatibility) { - super(xContentRegistry, deprecationHandler, useCompatibility); + DeprecationHandler deprecationHandler, JsonParser parser, + RestApiCompatibleVersion restApiCompatibleVersion) { + super(xContentRegistry, deprecationHandler, restApiCompatibleVersion); this.parser = parser; } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContent.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContent.java index 0b240b49a79cf..c077ba41a4eb8 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContent.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContent.java @@ -13,6 +13,7 @@ import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.dataformat.smile.SmileFactory; import com.fasterxml.jackson.dataformat.smile.SmileGenerator; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.xcontent.DeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContent; @@ -100,7 +101,8 @@ public XContentParser createParser(NamedXContentRegistry xContentRegistry, @Override public XContentParser createParserForCompatibility(NamedXContentRegistry xContentRegistry, - DeprecationHandler deprecationHandler, InputStream is) throws IOException { - return new SmileXContentParser(xContentRegistry, deprecationHandler, smileFactory.createParser(is), true); + DeprecationHandler deprecationHandler, InputStream is, + RestApiCompatibleVersion restApiCompatibleVersion) throws IOException { + return new SmileXContentParser(xContentRegistry, deprecationHandler, smileFactory.createParser(is), restApiCompatibleVersion); } } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContentParser.java index 68c424b73f890..12f901fb1c3c0 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContentParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContentParser.java @@ -9,6 +9,7 @@ package org.elasticsearch.common.xcontent.smile; import com.fasterxml.jackson.core.JsonParser; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.xcontent.DeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentType; @@ -22,8 +23,9 @@ public SmileXContentParser(NamedXContentRegistry xContentRegistry, } public SmileXContentParser(NamedXContentRegistry xContentRegistry, - DeprecationHandler deprecationHandler, JsonParser parser, boolean useCompatibility) { - super(xContentRegistry, deprecationHandler, parser, useCompatibility); + DeprecationHandler deprecationHandler, JsonParser parser, + RestApiCompatibleVersion restApiCompatibleVersion) { + super(xContentRegistry, deprecationHandler, parser, restApiCompatibleVersion); } @Override diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java index 43a150f72db55..c2ae840dcc12e 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java @@ -10,6 +10,7 @@ import org.elasticsearch.common.Booleans; import org.elasticsearch.common.CheckedFunction; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.xcontent.DeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentParseException; @@ -46,16 +47,17 @@ private static void checkCoerceString(boolean coerce, Class cl private final NamedXContentRegistry xContentRegistry; private final DeprecationHandler deprecationHandler; - private final boolean useCompatibility; + private final RestApiCompatibleVersion restApiCompatibleVersion; - public AbstractXContentParser(NamedXContentRegistry xContentRegistry, DeprecationHandler deprecationHandler, boolean useCompatibility) { + public AbstractXContentParser(NamedXContentRegistry xContentRegistry, DeprecationHandler deprecationHandler, + RestApiCompatibleVersion restApiCompatibleVersion) { this.xContentRegistry = xContentRegistry; this.deprecationHandler = deprecationHandler; - this.useCompatibility = useCompatibility; + this.restApiCompatibleVersion = restApiCompatibleVersion; } public AbstractXContentParser(NamedXContentRegistry xContentRegistry, DeprecationHandler deprecationHandler) { - this(xContentRegistry, deprecationHandler, false); + this(xContentRegistry, deprecationHandler, RestApiCompatibleVersion.currentVersion()); } // The 3rd party parsers we rely on are known to silently truncate fractions: see @@ -413,8 +415,8 @@ public NamedXContentRegistry getXContentRegistry() { public abstract boolean isClosed(); @Override - public boolean useCompatibility() { - return useCompatibility; + public RestApiCompatibleVersion getRestApiCompatibleVersion() { + return restApiCompatibleVersion; } @Override diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/yaml/YamlXContent.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/yaml/YamlXContent.java index f271851698aa2..8ce0c4144118e 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/yaml/YamlXContent.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/yaml/YamlXContent.java @@ -11,6 +11,7 @@ import com.fasterxml.jackson.core.JsonEncoding; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.xcontent.DeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContent; @@ -93,8 +94,9 @@ public XContentParser createParser(NamedXContentRegistry xContentRegistry, @Override public XContentParser createParserForCompatibility(NamedXContentRegistry xContentRegistry, - DeprecationHandler deprecationHandler, InputStream is) throws IOException { - return new YamlXContentParser(xContentRegistry, deprecationHandler, yamlFactory.createParser(is), true); + DeprecationHandler deprecationHandler, InputStream is, + RestApiCompatibleVersion restApiCompatibleVersion) throws IOException { + return new YamlXContentParser(xContentRegistry, deprecationHandler, yamlFactory.createParser(is), restApiCompatibleVersion); } } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/yaml/YamlXContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/yaml/YamlXContentParser.java index 02f9b3b3f5907..35d85dca59fbb 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/yaml/YamlXContentParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/yaml/YamlXContentParser.java @@ -9,6 +9,7 @@ package org.elasticsearch.common.xcontent.yaml; import com.fasterxml.jackson.core.JsonParser; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.xcontent.DeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentType; @@ -22,8 +23,9 @@ public YamlXContentParser(NamedXContentRegistry xContentRegistry, } public YamlXContentParser(NamedXContentRegistry xContentRegistry, - DeprecationHandler deprecationHandler, JsonParser parser, boolean useCompatibility) { - super(xContentRegistry, deprecationHandler, parser, useCompatibility); + DeprecationHandler deprecationHandler, JsonParser parser, + RestApiCompatibleVersion restApiCompatibleVersion) { + super(xContentRegistry, deprecationHandler, parser, restApiCompatibleVersion); } @Override diff --git a/server/src/main/java/org/elasticsearch/Version.java b/server/src/main/java/org/elasticsearch/Version.java index 8b9074ebd87e5..95ac300f66925 100644 --- a/server/src/main/java/org/elasticsearch/Version.java +++ b/server/src/main/java/org/elasticsearch/Version.java @@ -12,6 +12,7 @@ import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.collect.ImmutableOpenIntMap; import org.elasticsearch.common.collect.ImmutableOpenMap; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ToXContentFragment; @@ -117,7 +118,9 @@ public class Version implements Comparable, ToXContentFragment { } assert CURRENT.luceneVersion.equals(org.apache.lucene.util.Version.LATEST) : "Version must be upgraded to [" + org.apache.lucene.util.Version.LATEST + "] is still set to [" + CURRENT.luceneVersion + "]"; - + assert RestApiCompatibleVersion.currentVersion().major == CURRENT.major : "RestApiCompatibleVersion must be upgraded " + + "to reflect major from Version.CURRENT [" + CURRENT.major + "]" + + " but is still set to [" + RestApiCompatibleVersion.currentVersion().major + "]"; builder.put(V_EMPTY_ID, V_EMPTY); builderByString.put(V_EMPTY.toString(), V_EMPTY); idToVersion = builder.build(); diff --git a/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java b/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java index 1bcf06bc725e4..aa18c8ce43c1e 100644 --- a/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java +++ b/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java @@ -8,7 +8,7 @@ package org.elasticsearch.rest; -import org.elasticsearch.common.compatibility.CompatibleVersion; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import java.util.HashMap; import java.util.Map; @@ -20,7 +20,7 @@ final class MethodHandlers { private final String path; - private final Map> methodHandlers; + private final Map> methodHandlers; MethodHandlers(String path, RestHandler handler, RestRequest.Method... methods) { this.path = path; @@ -54,13 +54,13 @@ MethodHandlers addMethods(RestHandler handler, RestRequest.Method... methods) { * (as opposed to non-compatible/breaking) * or {@code null} if none exists. */ - RestHandler getHandler(RestRequest.Method method, CompatibleVersion version) { - Map versionToHandlers = methodHandlers.get(method); + RestHandler getHandler(RestRequest.Method method, RestApiCompatibleVersion version) { + Map versionToHandlers = methodHandlers.get(method); if (versionToHandlers == null) { return null; //method not found } final RestHandler handler = versionToHandlers.get(version); - return handler == null ? versionToHandlers.get(CompatibleVersion.CURRENT) : handler; + return handler == null ? versionToHandlers.get(RestApiCompatibleVersion.currentVersion()) : handler; } diff --git a/server/src/main/java/org/elasticsearch/rest/RestCompatibleVersionHelper.java b/server/src/main/java/org/elasticsearch/rest/RestCompatibleVersionHelper.java index 147567c18bfbf..2c88186ab4094 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestCompatibleVersionHelper.java +++ b/server/src/main/java/org/elasticsearch/rest/RestCompatibleVersionHelper.java @@ -17,7 +17,7 @@ import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.Version; import org.elasticsearch.common.Nullable; -import org.elasticsearch.common.compatibility.CompatibleVersion; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.xcontent.MediaType; import org.elasticsearch.common.xcontent.ParsedMediaType; @@ -28,23 +28,25 @@ */ class RestCompatibleVersionHelper { - static CompatibleVersion getCompatibleVersion( + static RestApiCompatibleVersion getCompatibleVersion( @Nullable ParsedMediaType acceptHeader, @Nullable ParsedMediaType contentTypeHeader, boolean hasContent ) { Byte aVersion = parseVersion(acceptHeader); - byte acceptVersion = aVersion == null ? CompatibleVersion.CURRENT.major : Integer.valueOf(aVersion).byteValue(); + byte acceptVersion = aVersion == null ? RestApiCompatibleVersion.currentVersion().major : Integer.valueOf(aVersion).byteValue(); Byte cVersion = parseVersion(contentTypeHeader); - byte contentTypeVersion = cVersion == null ? CompatibleVersion.CURRENT.major : Integer.valueOf(cVersion).byteValue(); + byte contentTypeVersion = cVersion == null ? + RestApiCompatibleVersion.currentVersion().major : Integer.valueOf(cVersion).byteValue(); // accept version must be current or prior - if (acceptVersion > CompatibleVersion.CURRENT.major || acceptVersion < CompatibleVersion.minimumRestCompatibilityVersion().major) { + if (acceptVersion > RestApiCompatibleVersion.currentVersion().major || + acceptVersion < RestApiCompatibleVersion.minimumSupported().major) { throw new ElasticsearchStatusException( "Accept version must be either version {} or {}, but found {}. Accept={}", RestStatus.BAD_REQUEST, - CompatibleVersion.CURRENT.major, - CompatibleVersion.minimumRestCompatibilityVersion().major, + RestApiCompatibleVersion.currentVersion().major, + RestApiCompatibleVersion.minimumSupported().major, acceptVersion, acceptHeader ); @@ -52,13 +54,13 @@ static CompatibleVersion getCompatibleVersion( if (hasContent) { // content-type version must be current or prior - if (contentTypeVersion > CompatibleVersion.CURRENT.major - || contentTypeVersion < CompatibleVersion.minimumRestCompatibilityVersion().major) { + if (contentTypeVersion > RestApiCompatibleVersion.currentVersion().major + || contentTypeVersion < RestApiCompatibleVersion.minimumSupported().major) { throw new ElasticsearchStatusException( "Content-Type version must be either version {} or {}, but found {}. Content-Type={}", RestStatus.BAD_REQUEST, - CompatibleVersion.CURRENT.major, - CompatibleVersion.minimumRestCompatibilityVersion().major, + RestApiCompatibleVersion.currentVersion().major, + RestApiCompatibleVersion.minimumSupported().major, contentTypeVersion, contentTypeHeader ); @@ -85,15 +87,15 @@ static CompatibleVersion getCompatibleVersion( ); } if (contentTypeVersion < Version.CURRENT.major) { - return CompatibleVersion.fromMajorVersion(Version.CURRENT.previousMajor().major); + return RestApiCompatibleVersion.fromMajorVersion(Version.CURRENT.previousMajor().major); } } if (acceptVersion < Version.CURRENT.major) { - return CompatibleVersion.fromMajorVersion(Version.CURRENT.previousMajor().major); + return RestApiCompatibleVersion.fromMajorVersion(Version.CURRENT.previousMajor().major); } - return CompatibleVersion.fromMajorVersion(Version.CURRENT.major); + return RestApiCompatibleVersion.fromMajorVersion(Version.CURRENT.major); } static Byte parseVersion(ParsedMediaType parsedMediaType) { diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java index ef86cc0f91647..fa2d533d99f06 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestController.java +++ b/server/src/main/java/org/elasticsearch/rest/RestController.java @@ -18,7 +18,7 @@ import org.elasticsearch.common.breaker.CircuitBreaker; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.compatibility.CompatibleVersion; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.path.PathTrie; @@ -158,9 +158,9 @@ protected void registerHandler(RestRequest.Method method, String path, RestHandl } private void registerHandlerNoWrap(RestRequest.Method method, String path, RestHandler maybeWrappedHandler) { - final CompatibleVersion version = maybeWrappedHandler.compatibleWithVersion(); - assert CompatibleVersion.minimumRestCompatibilityVersion() == version || CompatibleVersion.CURRENT == version - : "REST API compatibility is only supported for version " + CompatibleVersion.minimumRestCompatibilityVersion().major; + final RestApiCompatibleVersion version = maybeWrappedHandler.compatibleWithVersion(); + assert RestApiCompatibleVersion.minimumSupported() == version || RestApiCompatibleVersion.currentVersion() == version + : "REST API compatibility is only supported for version " + RestApiCompatibleVersion.minimumSupported().major; handlers.insertOrUpdate(path, new MethodHandlers(path, maybeWrappedHandler, method), (mHandlers, newMHandler) -> mHandlers.addMethods(maybeWrappedHandler, method)); @@ -214,7 +214,8 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th } } - private void dispatchRequest(RestRequest request, RestChannel channel, RestHandler handler, CompatibleVersion compatibleVersion) + private void dispatchRequest(RestRequest request, RestChannel channel, RestHandler handler, + RestApiCompatibleVersion restApiCompatibleVersion) throws Exception { final int contentLength = request.contentLength(); if (contentLength > 0) { @@ -239,7 +240,7 @@ private void dispatchRequest(RestRequest request, RestChannel channel, RestHandl inFlightRequestsBreaker(circuitBreakerService).addWithoutBreaking(contentLength); } // iff we could reserve bytes for the request we need to send the response also over this channel - responseChannel = new ResourceHandlingHttpChannel(channel, circuitBreakerService, contentLength, compatibleVersion); + responseChannel = new ResourceHandlingHttpChannel(channel, circuitBreakerService, contentLength, restApiCompatibleVersion); // TODO: Count requests double in the circuit breaker if they need copying? if (handler.allowsUnsafeBuffers() == false) { request.ensureSafeBuffers(); @@ -316,7 +317,7 @@ private void tryAllHandlers(final RestRequest request, final RestChannel channel final String uri = request.uri(); final RestRequest.Method requestMethod; - CompatibleVersion compatibleVersion = request.getCompatibleVersion(); + RestApiCompatibleVersion restApiCompatibleVersion = request.getRestApiCompatibleVersion(); try { // Resolves the HTTP method and fails if the method is invalid requestMethod = request.method(); @@ -328,14 +329,14 @@ private void tryAllHandlers(final RestRequest request, final RestChannel channel if (handlers == null) { handler = null; } else { - handler = handlers.getHandler(requestMethod, compatibleVersion); + handler = handlers.getHandler(requestMethod, restApiCompatibleVersion); } if (handler == null) { if (handleNoHandlerFound(rawPath, requestMethod, uri, channel)) { return; } } else { - dispatchRequest(request, channel, handler, compatibleVersion); + dispatchRequest(request, channel, handler, restApiCompatibleVersion); return; } } @@ -453,40 +454,40 @@ private static final class ResourceHandlingHttpChannel implements RestChannel { private final RestChannel delegate; private final CircuitBreakerService circuitBreakerService; private final int contentLength; - private final CompatibleVersion compatibleVersion; + private final RestApiCompatibleVersion restApiCompatibleVersion; private final AtomicBoolean closed = new AtomicBoolean(); ResourceHandlingHttpChannel(RestChannel delegate, CircuitBreakerService circuitBreakerService, int contentLength, - CompatibleVersion compatibleVersion) { + RestApiCompatibleVersion restApiCompatibleVersion) { this.delegate = delegate; this.circuitBreakerService = circuitBreakerService; this.contentLength = contentLength; - this.compatibleVersion = compatibleVersion; + this.restApiCompatibleVersion = restApiCompatibleVersion; } @Override public XContentBuilder newBuilder() throws IOException { return delegate.newBuilder() - .withCompatibleVersion(compatibleVersion); + .withCompatibleVersion(restApiCompatibleVersion); } @Override public XContentBuilder newErrorBuilder() throws IOException { return delegate.newErrorBuilder() - .withCompatibleVersion(compatibleVersion); + .withCompatibleVersion(restApiCompatibleVersion); } @Override public XContentBuilder newBuilder(@Nullable XContentType xContentType, boolean useFiltering) throws IOException { return delegate.newBuilder(xContentType, useFiltering) - .withCompatibleVersion(compatibleVersion); + .withCompatibleVersion(restApiCompatibleVersion); } @Override public XContentBuilder newBuilder(XContentType xContentType, XContentType responseContentType, boolean useFiltering) throws IOException { return delegate.newBuilder(xContentType, responseContentType, useFiltering) - .withCompatibleVersion(compatibleVersion); + .withCompatibleVersion(restApiCompatibleVersion); } @Override diff --git a/server/src/main/java/org/elasticsearch/rest/RestHandler.java b/server/src/main/java/org/elasticsearch/rest/RestHandler.java index 90d277d8dba1d..9b1902e27a85d 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestHandler.java +++ b/server/src/main/java/org/elasticsearch/rest/RestHandler.java @@ -9,7 +9,7 @@ package org.elasticsearch.rest; import org.elasticsearch.client.node.NodeClient; -import org.elasticsearch.common.compatibility.CompatibleVersion; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.xcontent.MediaType; import org.elasticsearch.common.xcontent.MediaTypeRegistry; import org.elasticsearch.common.xcontent.XContent; @@ -102,8 +102,8 @@ default MediaTypeRegistry validAcceptMediaTypes() { * If no version is specified, handler is assumed to be compatible with Version.CURRENT * @return a version */ - default CompatibleVersion compatibleWithVersion() { - return CompatibleVersion.CURRENT; + default RestApiCompatibleVersion compatibleWithVersion() { + return RestApiCompatibleVersion.currentVersion(); } 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 a8e40a443fe8c..c8fc07dfbff01 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -17,7 +17,7 @@ import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.collect.Tuple; -import org.elasticsearch.common.compatibility.CompatibleVersion; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; @@ -62,7 +62,7 @@ public class RestRequest implements ToXContent.Params { private final HttpChannel httpChannel; private final ParsedMediaType parsedAccept; private final ParsedMediaType parsedContentType; - private final CompatibleVersion compatibleVersion; + private final RestApiCompatibleVersion restApiCompatibleVersion; private HttpRequest httpRequest; private boolean contentConsumed = false; @@ -100,7 +100,7 @@ private RestRequest(NamedXContentRegistry xContentRegistry, Map this.rawPath = path; this.headers = Collections.unmodifiableMap(headers); this.requestId = requestId; - this.compatibleVersion = RestCompatibleVersionHelper.getCompatibleVersion(parsedAccept, parsedContentType, hasContent()); + this.restApiCompatibleVersion = RestCompatibleVersionHelper.getCompatibleVersion(parsedAccept, parsedContentType, hasContent()); } private static @Nullable ParsedMediaType parseHeaderWithMediaType(Map> headers, String headerName) { @@ -439,11 +439,9 @@ public NamedXContentRegistry getXContentRegistry() { public final XContentParser contentParser() throws IOException { BytesReference content = requiredContent(); // will throw exception if body or content type missing XContent xContent = xContentType.get().xContent(); - if (compatibleVersion == CompatibleVersion.minimumRestCompatibilityVersion()) { - return xContent.createParserForCompatibility(xContentRegistry, LoggingDeprecationHandler.INSTANCE, content.streamInput()); - } else { - return xContent.createParser(xContentRegistry, LoggingDeprecationHandler.INSTANCE, content.streamInput()); - } + return xContent.createParserForCompatibility(xContentRegistry, LoggingDeprecationHandler.INSTANCE, content.streamInput(), + restApiCompatibleVersion); + } /** @@ -551,8 +549,8 @@ public static XContentType parseContentType(List header) { throw new IllegalArgumentException("empty Content-Type header"); } - public CompatibleVersion getCompatibleVersion() { - return compatibleVersion; + public RestApiCompatibleVersion getRestApiCompatibleVersion() { + return restApiCompatibleVersion; } public static class MediaTypeHeaderException extends RuntimeException { diff --git a/server/src/test/java/org/elasticsearch/common/xcontent/CompatibleNamedXContentRegistryTests.java b/server/src/test/java/org/elasticsearch/common/xcontent/CompatibleNamedXContentRegistryTests.java index ee8845bc44b07..25ca050a848f0 100644 --- a/server/src/test/java/org/elasticsearch/common/xcontent/CompatibleNamedXContentRegistryTests.java +++ b/server/src/test/java/org/elasticsearch/common/xcontent/CompatibleNamedXContentRegistryTests.java @@ -11,7 +11,7 @@ import org.elasticsearch.Version; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.compatibility.CompatibleVersion; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.rest.FakeRestRequest; @@ -139,7 +139,7 @@ public void testCompatibleRequest() throws IOException { b.endObject(); String mediaType = XContentType.VND_JSON.toParsedMediaType() .responseContentTypeHeader(Map.of(MediaType.COMPATIBLE_WITH_PARAMETER_NAME, - String.valueOf(CompatibleVersion.minimumRestCompatibilityVersion().major))); + String.valueOf(RestApiCompatibleVersion.minimumSupported().major))); List mediaTypeList = Collections.singletonList(mediaType); RestRequest restRequest2 = new FakeRestRequest.Builder(compatibleRegistry) diff --git a/server/src/test/java/org/elasticsearch/node/NodeTests.java b/server/src/test/java/org/elasticsearch/node/NodeTests.java index 9d73b803dc37e..408d7ba19a0b8 100644 --- a/server/src/test/java/org/elasticsearch/node/NodeTests.java +++ b/server/src/test/java/org/elasticsearch/node/NodeTests.java @@ -14,7 +14,7 @@ import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.breaker.CircuitBreaker; -import org.elasticsearch.common.compatibility.CompatibleVersion; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.BoundTransportAddress; @@ -336,11 +336,11 @@ public void setCircuitBreaker(CircuitBreaker circuitBreaker) { } - interface MockCompatibleVersion { - CompatibleVersion minimumRestCompatibilityVersion(); + interface MockRestApiCompatibleVersion { + RestApiCompatibleVersion minimumRestCompatibilityVersion(); } - static MockCompatibleVersion MockCompatibleVersion = Mockito.mock(MockCompatibleVersion.class); + static MockRestApiCompatibleVersion MockCompatibleVersion = Mockito.mock(MockRestApiCompatibleVersion.class); static NamedXContentRegistry.Entry v7CompatibleEntries = new NamedXContentRegistry.Entry(Integer.class, new ParseField("name"), Mockito.mock(ContextParser.class)); @@ -353,13 +353,13 @@ public static class TestRestCompatibility1 extends Plugin { public List getNamedXContentForCompatibility() { // real plugin will use CompatibleVersion.minimumRestCompatibilityVersion() if (/*CompatibleVersion.minimumRestCompatibilityVersion()*/ - MockCompatibleVersion.minimumRestCompatibilityVersion().equals(CompatibleVersion.V_7)) { + MockCompatibleVersion.minimumRestCompatibilityVersion().equals(RestApiCompatibleVersion.V_7)) { //return set of N-1 entries return List.of(v7CompatibleEntries); } // after major release, new compatible apis can be added before the old ones are removed. if (/*CompatibleVersion.minimumRestCompatibilityVersion()*/ - MockCompatibleVersion.minimumRestCompatibilityVersion().equals(CompatibleVersion.V_8)) { + MockCompatibleVersion.minimumRestCompatibilityVersion().equals(RestApiCompatibleVersion.V_8)) { return List.of(v8CompatibleEntries); } @@ -371,7 +371,7 @@ public List getNamedXContentForCompatibility() { public void testLoadingMultipleRestCompatibilityPlugins() throws IOException { Mockito.when(MockCompatibleVersion.minimumRestCompatibilityVersion()) - .thenReturn(CompatibleVersion.V_7); + .thenReturn(RestApiCompatibleVersion.V_7); { Settings.Builder settings = baseSettings(); @@ -387,7 +387,7 @@ public void testLoadingMultipleRestCompatibilityPlugins() throws IOException { } // after version bump CompatibleVersion.minimumRestCompatibilityVersion() will return V_8 Mockito.when(MockCompatibleVersion.minimumRestCompatibilityVersion()) - .thenReturn(CompatibleVersion.V_8); + .thenReturn(RestApiCompatibleVersion.V_8); { Settings.Builder settings = baseSettings(); diff --git a/server/src/test/java/org/elasticsearch/rest/MethodHandlersTests.java b/server/src/test/java/org/elasticsearch/rest/MethodHandlersTests.java index 7d3613bd9f72f..a827eaa523060 100644 --- a/server/src/test/java/org/elasticsearch/rest/MethodHandlersTests.java +++ b/server/src/test/java/org/elasticsearch/rest/MethodHandlersTests.java @@ -9,7 +9,7 @@ package org.elasticsearch.rest; import org.elasticsearch.client.node.NodeClient; -import org.elasticsearch.common.compatibility.CompatibleVersion; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.test.ESTestCase; import static org.hamcrest.Matchers.sameInstance; @@ -22,7 +22,7 @@ public void testLookupForDifferentMethodsSameVersion() { MethodHandlers methodHandlers = new MethodHandlers("path", putHandler, RestRequest.Method.PUT); methodHandlers.addMethods(postHandler, RestRequest.Method.POST); - RestHandler handler = methodHandlers.getHandler(RestRequest.Method.PUT, CompatibleVersion.CURRENT); + RestHandler handler = methodHandlers.getHandler(RestRequest.Method.PUT, RestApiCompatibleVersion.currentVersion()); assertThat(handler, sameInstance(putHandler)); } @@ -30,10 +30,10 @@ public void testLookupForHandlerUnderMultipleMethods() { RestHandler handler = new CurrentVersionHandler(); MethodHandlers methodHandlers = new MethodHandlers("path", handler, RestRequest.Method.PUT, RestRequest.Method.POST); - RestHandler handlerFound = methodHandlers.getHandler(RestRequest.Method.PUT, CompatibleVersion.CURRENT); + RestHandler handlerFound = methodHandlers.getHandler(RestRequest.Method.PUT, RestApiCompatibleVersion.currentVersion()); assertThat(handlerFound, sameInstance(handler)); - handlerFound = methodHandlers.getHandler(RestRequest.Method.POST, CompatibleVersion.CURRENT); + handlerFound = methodHandlers.getHandler(RestRequest.Method.POST, RestApiCompatibleVersion.currentVersion()); assertThat(handlerFound, sameInstance(handler)); } @@ -43,10 +43,10 @@ public void testLookupForHandlersUnderDifferentVersions() { MethodHandlers methodHandlers = new MethodHandlers("path", currentVersionHandler, RestRequest.Method.PUT); methodHandlers.addMethods(previousVersionHandler, RestRequest.Method.PUT); - RestHandler handler = methodHandlers.getHandler(RestRequest.Method.PUT, CompatibleVersion.CURRENT); + RestHandler handler = methodHandlers.getHandler(RestRequest.Method.PUT, RestApiCompatibleVersion.currentVersion()); assertThat(handler, sameInstance(currentVersionHandler)); - handler = methodHandlers.getHandler(RestRequest.Method.PUT, CompatibleVersion.CURRENT.previousMajor()); + handler = methodHandlers.getHandler(RestRequest.Method.PUT, RestApiCompatibleVersion.currentVersion().previousMajor()); assertThat(handler, sameInstance(previousVersionHandler)); } @@ -60,14 +60,14 @@ public void testExceptionOnOverride() { public void testMissingCurrentHandler(){ RestHandler previousVersionHandler = new PreviousVersionHandler(); MethodHandlers methodHandlers = new MethodHandlers("path", previousVersionHandler, RestRequest.Method.PUT, RestRequest.Method.POST); - RestHandler handler = methodHandlers.getHandler(RestRequest.Method.PUT, CompatibleVersion.CURRENT); + RestHandler handler = methodHandlers.getHandler(RestRequest.Method.PUT, RestApiCompatibleVersion.currentVersion()); assertNull(handler); } public void testMissingPriorHandlerReturnsCurrentHandler(){ RestHandler currentVersionHandler = new CurrentVersionHandler(); MethodHandlers methodHandlers = new MethodHandlers("path", currentVersionHandler, RestRequest.Method.PUT, RestRequest.Method.POST); - RestHandler handler = methodHandlers.getHandler(RestRequest.Method.PUT, CompatibleVersion.CURRENT.previousMajor()); + RestHandler handler = methodHandlers.getHandler(RestRequest.Method.PUT, RestApiCompatibleVersion.currentVersion().previousMajor()); assertThat(handler, sameInstance(currentVersionHandler)); } @@ -85,8 +85,8 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c } @Override - public CompatibleVersion compatibleWithVersion() { - return CompatibleVersion.CURRENT.previousMajor(); + public RestApiCompatibleVersion compatibleWithVersion() { + return RestApiCompatibleVersion.currentVersion().previousMajor(); } } } diff --git a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java index 79c001a455106..f680c02bb3a47 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java @@ -13,7 +13,7 @@ import org.elasticsearch.common.breaker.CircuitBreaker; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.compatibility.CompatibleVersion; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.component.AbstractLifecycleComponent; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; @@ -234,7 +234,7 @@ public void testRegisterSecondMethodWithDifferentNamedWildcard() { private RestHandler v8mockHandler() { RestHandler mock = mock(RestHandler.class); - Mockito.when(mock.compatibleWithVersion()).thenReturn(CompatibleVersion.CURRENT); + Mockito.when(mock.compatibleWithVersion()).thenReturn(RestApiCompatibleVersion.currentVersion()); return mock; } @@ -369,7 +369,7 @@ public void testDispatchWorksWithNewlineDelimitedJson() { restController.registerHandler(RestRequest.Method.GET, "/foo", new RestHandler() { @Override public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { - assertThat(request.contentParser().useCompatibility(), is(false)); + assertThat(request.contentParser().getRestApiCompatibleVersion(), is(RestApiCompatibleVersion.currentVersion())); channel.sendResponse(new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); } @@ -396,7 +396,7 @@ public void testDispatchWithContentStream() { restController.registerHandler(RestRequest.Method.GET, "/foo", new RestHandler() { @Override public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { - assertThat(request.contentParser().useCompatibility(), is(false)); + assertThat(request.contentParser().getRestApiCompatibleVersion(), is(RestApiCompatibleVersion.currentVersion())); channel.sendResponse(new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); } @@ -628,7 +628,7 @@ public void testDispatchCompatibleHandler() { RestController restController = new RestController(Collections.emptySet(), null, client, circuitBreakerService, usageService); - final byte version = CompatibleVersion.minimumRestCompatibilityVersion().major; + final byte version = RestApiCompatibleVersion.minimumSupported().major; final String mediaType = randomCompatibleMediaType(version); FakeRestRequest fakeRestRequest = requestWithContent(mediaType); @@ -638,16 +638,15 @@ public void testDispatchCompatibleHandler() { @Override public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { // in real use case we will use exact version CompatibleVersion.V_7 - assertThat(request.contentParser().useCompatibility(), is(true)); XContentBuilder xContentBuilder = channel.newBuilder(); - assertTrue(xContentBuilder.useCompatibility(CompatibleVersion.minimumRestCompatibilityVersion())); - assertThat(xContentBuilder.getRestCompatibilityVersion(), equalTo(CompatibleVersion.minimumRestCompatibilityVersion())); + assertThat(xContentBuilder.getRestApiCompatibilityVersion(), equalTo(RestApiCompatibleVersion.minimumSupported())); + assertThat(request.contentParser().getRestApiCompatibleVersion(), equalTo(RestApiCompatibleVersion.minimumSupported())); channel.sendResponse(new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); } @Override - public CompatibleVersion compatibleWithVersion() { - return CompatibleVersion.minimumRestCompatibilityVersion(); + public RestApiCompatibleVersion compatibleWithVersion() { + return RestApiCompatibleVersion.minimumSupported(); } }); @@ -660,7 +659,7 @@ public void testDispatchCompatibleRequestToNewlyAddedHandler() { RestController restController = new RestController(Collections.emptySet(), null, client, circuitBreakerService, usageService); - final byte version = CompatibleVersion.minimumRestCompatibilityVersion().major; + final byte version = RestApiCompatibleVersion.minimumSupported().major; final String mediaType = randomCompatibleMediaType(version); FakeRestRequest fakeRestRequest = requestWithContent(mediaType); @@ -675,18 +674,15 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c // even though the handler is CURRENT, the xContentBuilder has the version requested by a client. // This allows to implement the compatible logic within the serialisation without introducing V7 (compatible) handler // when only response shape has changed - assertThat(xContentBuilder.getRestCompatibilityVersion(), equalTo(CompatibleVersion.fromMajorVersion(version))); + assertThat(xContentBuilder.getRestApiCompatibilityVersion(), equalTo(RestApiCompatibleVersion.minimumSupported())); + assertThat(request.contentParser().getRestApiCompatibleVersion(), equalTo(RestApiCompatibleVersion.minimumSupported())); - // we don't expect compatible api code to be implemented for newly added APIs, - // yet the request is compatible so the compatible flags are set on builder/parser - assertTrue(xContentBuilder.useCompatibility(CompatibleVersion.minimumRestCompatibilityVersion())); - assertThat(request.contentParser().useCompatibility(), is(true)); channel.sendResponse(new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); } @Override - public CompatibleVersion compatibleWithVersion() { - return CompatibleVersion.CURRENT; + public RestApiCompatibleVersion compatibleWithVersion() { + return RestApiCompatibleVersion.currentVersion(); } }); @@ -720,17 +716,16 @@ public void testCurrentVersionVNDMediaTypeIsNotUsingCompatibility() { public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { // the media type is in application/vnd.elasticsearch form but with compatible-with=CURRENT. // Hence compatibility is not used. - assertThat(request.contentParser().useCompatibility(), is(false)); XContentBuilder xContentBuilder = channel.newBuilder(); - assertFalse(xContentBuilder.useCompatibility(CompatibleVersion.minimumRestCompatibilityVersion())); - assertThat(xContentBuilder.getRestCompatibilityVersion(), equalTo(CompatibleVersion.fromMajorVersion(version))); + assertThat(request.contentParser().getRestApiCompatibleVersion(), equalTo(RestApiCompatibleVersion.currentVersion())); + assertThat(xContentBuilder.getRestApiCompatibilityVersion(), equalTo(RestApiCompatibleVersion.currentVersion())); channel.sendResponse(new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); } @Override - public CompatibleVersion compatibleWithVersion() { - return CompatibleVersion.CURRENT; + public RestApiCompatibleVersion compatibleWithVersion() { + return RestApiCompatibleVersion.currentVersion(); } }); @@ -750,8 +745,8 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c } @Override - public CompatibleVersion compatibleWithVersion() { - return CompatibleVersion.fromMajorVersion(version); + public RestApiCompatibleVersion compatibleWithVersion() { + return RestApiCompatibleVersion.fromMajorVersion(version); } })); } diff --git a/server/src/test/java/org/elasticsearch/rest/RestCompatibleVersionHelperTests.java b/server/src/test/java/org/elasticsearch/rest/RestRestApiCompatibleVersionHelperTests.java similarity index 97% rename from server/src/test/java/org/elasticsearch/rest/RestCompatibleVersionHelperTests.java rename to server/src/test/java/org/elasticsearch/rest/RestRestApiCompatibleVersionHelperTests.java index 576ff1d5d60dd..41295bea3eede 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestCompatibleVersionHelperTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestRestApiCompatibleVersionHelperTests.java @@ -9,7 +9,7 @@ import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.Version; -import org.elasticsearch.common.compatibility.CompatibleVersion; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.xcontent.ParsedMediaType; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.hamcrest.ElasticsearchMatchers; @@ -20,7 +20,7 @@ import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; -public class RestCompatibleVersionHelperTests extends ESTestCase { +public class RestRestApiCompatibleVersionHelperTests extends ESTestCase { int CURRENT_VERSION = Version.CURRENT.major; int PREVIOUS_VERSION = Version.CURRENT.major - 1; int OBSOLETE_VERSION = Version.CURRENT.major - 2; @@ -323,11 +323,11 @@ public void testVersionParsing() { } - private Matcher isCompatible() { + private Matcher isCompatible() { return requestHasVersion(PREVIOUS_VERSION); } - private Matcher requestHasVersion(int version) { + private Matcher requestHasVersion(int version) { return ElasticsearchMatchers.HasPropertyLambdaMatcher.hasProperty(v -> (int) v.major, equalTo(version)); } @@ -362,7 +362,7 @@ private String mediaType(String version) { return null; } - private CompatibleVersion requestWith(String accept, String contentType, String body) { + private RestApiCompatibleVersion requestWith(String accept, String contentType, String body) { ParsedMediaType parsedAccept = ParsedMediaType.parseMediaType(accept); ParsedMediaType parsedContentType = ParsedMediaType.parseMediaType(contentType); return RestCompatibleVersionHelper.getCompatibleVersion(parsedAccept, parsedContentType, body.isEmpty() == false); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/support/xcontent/WatcherXContentParser.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/support/xcontent/WatcherXContentParser.java index c06f3a2af84a4..8d76a1b7b312b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/support/xcontent/WatcherXContentParser.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/support/xcontent/WatcherXContentParser.java @@ -9,6 +9,7 @@ import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.CheckedFunction; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.xcontent.DeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentLocation; @@ -288,8 +289,8 @@ public void close() throws IOException { } @Override - public boolean useCompatibility() { - return false; + public RestApiCompatibleVersion getRestApiCompatibleVersion() { + return RestApiCompatibleVersion.currentVersion(); } @Override 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 6b57237606fac..2f567c5e53c4c 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 @@ -13,7 +13,7 @@ import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.node.NodeClient; -import org.elasticsearch.common.compatibility.CompatibleVersion; +import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.util.Maps; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.MediaType; @@ -161,7 +161,7 @@ public MediaTypeRegistry validAcceptMediaTypes() { } @Override - public CompatibleVersion compatibleWithVersion() { + public RestApiCompatibleVersion compatibleWithVersion() { return restHandler.compatibleWithVersion(); } } From 47ac2987c762189f00bd72349067be248ccd9da3 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Tue, 9 Feb 2021 11:16:30 +0100 Subject: [PATCH 6/8] rename --- ...onHelperTests.java => RestCompatibleVersionHelperTests.java} | 2 +- .../test/java/org/elasticsearch/rest/RestControllerTests.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename server/src/test/java/org/elasticsearch/rest/{RestRestApiCompatibleVersionHelperTests.java => RestCompatibleVersionHelperTests.java} (99%) diff --git a/server/src/test/java/org/elasticsearch/rest/RestRestApiCompatibleVersionHelperTests.java b/server/src/test/java/org/elasticsearch/rest/RestCompatibleVersionHelperTests.java similarity index 99% rename from server/src/test/java/org/elasticsearch/rest/RestRestApiCompatibleVersionHelperTests.java rename to server/src/test/java/org/elasticsearch/rest/RestCompatibleVersionHelperTests.java index 41295bea3eede..fd2c193b2e949 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestRestApiCompatibleVersionHelperTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestCompatibleVersionHelperTests.java @@ -20,7 +20,7 @@ import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; -public class RestRestApiCompatibleVersionHelperTests extends ESTestCase { +public class RestCompatibleVersionHelperTests extends ESTestCase { int CURRENT_VERSION = Version.CURRENT.major; int PREVIOUS_VERSION = Version.CURRENT.major - 1; int OBSOLETE_VERSION = Version.CURRENT.major - 2; diff --git a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java index f680c02bb3a47..6fe9088f90e83 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java @@ -637,7 +637,7 @@ public void testDispatchCompatibleHandler() { restController.registerHandler(RestRequest.Method.GET, "/foo", new RestHandler() { @Override public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { - // in real use case we will use exact version CompatibleVersion.V_7 + // in real use case we will use exact version RestApiCompatibleVersion.V_7 XContentBuilder xContentBuilder = channel.newBuilder(); assertThat(xContentBuilder.getRestApiCompatibilityVersion(), equalTo(RestApiCompatibleVersion.minimumSupported())); assertThat(request.contentParser().getRestApiCompatibleVersion(), equalTo(RestApiCompatibleVersion.minimumSupported())); From a1085ba30bf9489834a9152d0c11df51b4c9f35c Mon Sep 17 00:00:00 2001 From: pgomulka Date: Tue, 9 Feb 2021 11:23:30 +0100 Subject: [PATCH 7/8] do not use version in rest compatible version helper tests --- .../rest/RestCompatibleVersionHelper.java | 11 +++++------ .../rest/RestCompatibleVersionHelperTests.java | 7 +++---- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/RestCompatibleVersionHelper.java b/server/src/main/java/org/elasticsearch/rest/RestCompatibleVersionHelper.java index 2c88186ab4094..24f88cc6ff00d 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestCompatibleVersionHelper.java +++ b/server/src/main/java/org/elasticsearch/rest/RestCompatibleVersionHelper.java @@ -15,7 +15,6 @@ package org.elasticsearch.rest; import org.elasticsearch.ElasticsearchStatusException; -import org.elasticsearch.Version; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.xcontent.MediaType; @@ -86,16 +85,16 @@ static RestApiCompatibleVersion getCompatibleVersion( contentTypeHeader ); } - if (contentTypeVersion < Version.CURRENT.major) { - return RestApiCompatibleVersion.fromMajorVersion(Version.CURRENT.previousMajor().major); + if (contentTypeVersion < RestApiCompatibleVersion.currentVersion().major) { + return RestApiCompatibleVersion.minimumSupported(); } } - if (acceptVersion < Version.CURRENT.major) { - return RestApiCompatibleVersion.fromMajorVersion(Version.CURRENT.previousMajor().major); + if (acceptVersion < RestApiCompatibleVersion.currentVersion().major) { + return RestApiCompatibleVersion.minimumSupported(); } - return RestApiCompatibleVersion.fromMajorVersion(Version.CURRENT.major); + return RestApiCompatibleVersion.currentVersion(); } static Byte parseVersion(ParsedMediaType parsedMediaType) { diff --git a/server/src/test/java/org/elasticsearch/rest/RestCompatibleVersionHelperTests.java b/server/src/test/java/org/elasticsearch/rest/RestCompatibleVersionHelperTests.java index fd2c193b2e949..ae19cb7bde33a 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestCompatibleVersionHelperTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestCompatibleVersionHelperTests.java @@ -8,7 +8,6 @@ package org.elasticsearch.rest; import org.elasticsearch.ElasticsearchStatusException; -import org.elasticsearch.Version; import org.elasticsearch.common.compatibility.RestApiCompatibleVersion; import org.elasticsearch.common.xcontent.ParsedMediaType; import org.elasticsearch.test.ESTestCase; @@ -21,9 +20,9 @@ import static org.hamcrest.Matchers.nullValue; public class RestCompatibleVersionHelperTests extends ESTestCase { - int CURRENT_VERSION = Version.CURRENT.major; - int PREVIOUS_VERSION = Version.CURRENT.major - 1; - int OBSOLETE_VERSION = Version.CURRENT.major - 2; + int CURRENT_VERSION = RestApiCompatibleVersion.currentVersion().major; + int PREVIOUS_VERSION = RestApiCompatibleVersion.currentVersion().major - 1; + int OBSOLETE_VERSION = RestApiCompatibleVersion.currentVersion().major - 2; public void testAcceptAndContentTypeCombinations() { assertThat(requestWith(acceptHeader(PREVIOUS_VERSION), contentTypeHeader(PREVIOUS_VERSION), bodyPresent()), isCompatible()); From ad582ded70dee8fdf7a8ea63a5131bc95c51c27b Mon Sep 17 00:00:00 2001 From: pgomulka Date: Wed, 10 Feb 2021 08:58:07 +0100 Subject: [PATCH 8/8] remove unused method and enum instance --- .../compatibility/RestApiCompatibleVersion.java | 3 +-- .../common/xcontent/XContentBuilder.java | 4 ---- .../elasticsearch/rest/RestControllerTests.java | 17 ----------------- 3 files changed, 1 insertion(+), 23 deletions(-) diff --git a/libs/core/src/main/java/org/elasticsearch/common/compatibility/RestApiCompatibleVersion.java b/libs/core/src/main/java/org/elasticsearch/common/compatibility/RestApiCompatibleVersion.java index 2be75e7954223..f78661aef1079 100644 --- a/libs/core/src/main/java/org/elasticsearch/common/compatibility/RestApiCompatibleVersion.java +++ b/libs/core/src/main/java/org/elasticsearch/common/compatibility/RestApiCompatibleVersion.java @@ -17,8 +17,7 @@ public enum RestApiCompatibleVersion { V_8(8), - V_7(7), - V_6(6);//used in testing, to prove validation + V_7(7); public byte major; private static RestApiCompatibleVersion CURRENT = V_8; diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java index e09a11530ace0..708f42df752df 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java @@ -1025,10 +1025,6 @@ public RestApiCompatibleVersion getRestApiCompatibilityVersion() { return restApiCompatibilityVersion; } - public boolean useCompatibility(RestApiCompatibleVersion codeChangeVersion) { - return this.restApiCompatibilityVersion == codeChangeVersion; - } - @Override public void flush() throws IOException { generator.flush(); diff --git a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java index 6fe9088f90e83..301132a233f99 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java @@ -734,23 +734,6 @@ public RestApiCompatibleVersion compatibleWithVersion() { assertTrue(channel.getSendResponseCalled()); } - public void testRegisterIncompatibleVersionHandler() { - //using restController which uses a compatible version function returning always Version.CURRENT - 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 RestApiCompatibleVersion compatibleWithVersion() { - return RestApiCompatibleVersion.fromMajorVersion(version); - } - })); - } - private static final class TestHttpServerTransport extends AbstractLifecycleComponent implements HttpServerTransport {