From 164b7734b38c02d9970e566b85c6f245da1f5b50 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 26 Sep 2024 16:16:26 -0500 Subject: [PATCH] refactor (no tests) --- .../rest/DeprecationRestHandler.java | 20 ++++---- .../elasticsearch/rest/RestController.java | 51 +++++-------------- .../org/elasticsearch/rest/RestHandler.java | 35 +++++++------ 3 files changed, 42 insertions(+), 64 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/DeprecationRestHandler.java b/server/src/main/java/org/elasticsearch/rest/DeprecationRestHandler.java index 98ab7d53ffbe6..edd370bc447bd 100644 --- a/server/src/main/java/org/elasticsearch/rest/DeprecationRestHandler.java +++ b/server/src/main/java/org/elasticsearch/rest/DeprecationRestHandler.java @@ -29,7 +29,6 @@ public class DeprecationRestHandler extends FilterRestHandler implements RestHan private final DeprecationLogger deprecationLogger; private final boolean compatibleVersionWarning; private final String deprecationKey; - @Nullable private final Level deprecationLevel; /** @@ -51,7 +50,7 @@ public DeprecationRestHandler( RestHandler handler, RestRequest.Method method, String path, - @Nullable Level deprecationLevel, + Level deprecationLevel, String deprecationMessage, DeprecationLogger deprecationLogger, boolean compatibleVersionWarning @@ -61,7 +60,7 @@ public DeprecationRestHandler( this.deprecationLogger = Objects.requireNonNull(deprecationLogger); this.compatibleVersionWarning = compatibleVersionWarning; this.deprecationKey = DEPRECATED_ROUTE_KEY + "_" + method + "_" + path; - if (deprecationLevel != null && (deprecationLevel != Level.WARN && deprecationLevel != DeprecationLogger.CRITICAL)) { + if (deprecationLevel != Level.WARN || deprecationLevel != DeprecationLogger.CRITICAL) { throw new IllegalArgumentException( "unexpected deprecation logger level: " + deprecationLevel + ", expected either 'CRITICAL' or 'WARN'" ); @@ -77,19 +76,18 @@ public DeprecationRestHandler( @Override public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { if (compatibleVersionWarning == false) { - // The default value for deprecated requests without a version warning is WARN - if (deprecationLevel == null || deprecationLevel == Level.WARN) { + // emit a standard deprecation warning + if (Level.WARN == deprecationLevel) { deprecationLogger.warn(DeprecationCategory.API, deprecationKey, deprecationMessage); - } else { + } else if (DeprecationLogger.CRITICAL == deprecationLevel) { deprecationLogger.critical(DeprecationCategory.API, deprecationKey, deprecationMessage); } } else { - // The default value for deprecated requests with a version warning is CRITICAL, - // because they have a specific version where the endpoint is removed - if (deprecationLevel == null || deprecationLevel == DeprecationLogger.CRITICAL) { - deprecationLogger.compatibleCritical(deprecationKey, deprecationMessage); - } else { + // emit a compatibility warning + if (Level.WARN == deprecationLevel) { deprecationLogger.compatible(Level.WARN, deprecationKey, deprecationMessage); + } else if (DeprecationLogger.CRITICAL == deprecationLevel) { + deprecationLogger.compatibleCritical(deprecationKey, deprecationMessage); } } diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java index 7fcc75ad65872..12ba6bf4d8245 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestController.java +++ b/server/src/main/java/org/elasticsearch/rest/RestController.java @@ -34,6 +34,7 @@ import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.core.Streams; import org.elasticsearch.core.TimeValue; +import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.http.HttpHeadersValidationException; import org.elasticsearch.http.HttpRouteStats; import org.elasticsearch.http.HttpServerTransport; @@ -144,24 +145,6 @@ public ServerlessApiProtections getApiProtections() { return apiProtections; } - /** - * Registers a REST handler to be executed when the provided {@code method} and {@code path} match the request. - * - * @param method GET, POST, etc. - * @param path Path to handle (e.g. "/{index}/{type}/_bulk") - * @param version API version to handle (e.g. RestApiVersion.V_8) - * @param handler The handler to actually execute - * @param deprecationMessage The message to log and send as a header in the response - */ - protected void registerAsDeprecatedHandler( - RestRequest.Method method, - String path, - RestApiVersion version, - RestHandler handler, - String deprecationMessage - ) { - registerAsDeprecatedHandler(method, path, version, handler, deprecationMessage, null); - } /** * Registers a REST handler to be executed when the provided {@code method} and {@code path} match the request. @@ -173,17 +156,18 @@ protected void registerAsDeprecatedHandler( * @param deprecationMessage The message to log and send as a header in the response * @param deprecationLevel The deprecation log level to use for the deprecation warning, either WARN or CRITICAL */ + @UpdateForV9 // comment in the "assert false" below when V_7 is no longer supported protected void registerAsDeprecatedHandler( RestRequest.Method method, String path, RestApiVersion version, RestHandler handler, String deprecationMessage, - @Nullable Level deprecationLevel + Level deprecationLevel ) { assert (handler instanceof DeprecationRestHandler) == false; if (version == RestApiVersion.current()) { - // e.g. it was marked as deprecated in 8.x, and we're currently running 8.x + // e.g. it was marked as deprecated in 9.x, and we're currently running 9.x registerHandler( method, path, @@ -191,7 +175,7 @@ protected void registerAsDeprecatedHandler( new DeprecationRestHandler(handler, method, path, deprecationLevel, deprecationMessage, deprecationLogger, false) ); } else if (version == RestApiVersion.minimumSupported()) { - // e.g. it was marked as deprecated in 7.x, and we're currently running 8.x + // e.g. it was marked as last fully supported in 8.x, and we're currently running 9.x registerHandler( method, path, @@ -199,21 +183,10 @@ protected void registerAsDeprecatedHandler( new DeprecationRestHandler(handler, method, path, deprecationLevel, deprecationMessage, deprecationLogger, true) ); } else { - // e.g. it was marked as deprecated in 7.x, and we're currently running *9.x* - logger.debug( - "Deprecated route [" - + method - + " " - + path - + "] for handler [" - + handler.getClass() - + "] " - + "with version [" - + version - + "], which is less than the minimum supported version [" - + RestApiVersion.minimumSupported() - + "]" - ); + // e.g. it was marked as deprecated in 7.x, and we're currently running 9.x + // should never happen as we only support the current and current -1 versions + //TODO: comment this back in when V_7 is no longer supported + //assert false : "Unsupported REST API version " + version; } } @@ -264,7 +237,8 @@ protected void registerAsReplacedHandler( + "] instead."; registerHandler(method, path, version, handler); - registerAsDeprecatedHandler(replacedMethod, replacedPath, replacedVersion, handler, replacedMessage); + Level deprecationLevel = version == RestApiVersion.current() ? Level.WARN : DeprecationLogger.CRITICAL; + registerAsDeprecatedHandler(replacedMethod, replacedPath, replacedVersion, handler, replacedMessage, deprecationLevel); } /** @@ -308,7 +282,8 @@ public void registerHandler(final Route route, final RestHandler handler) { handler, replaced.getMethod(), replaced.getPath(), - replaced.getRestApiVersion() + replaced.getRestApiVersion(), + replaced.getDeprecationLevel() ); } else if (route.isDeprecated()) { registerAsDeprecatedHandler( diff --git a/server/src/main/java/org/elasticsearch/rest/RestHandler.java b/server/src/main/java/org/elasticsearch/rest/RestHandler.java index 28919c6df2d5c..c9f45ce43c0c8 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestHandler.java +++ b/server/src/main/java/org/elasticsearch/rest/RestHandler.java @@ -11,6 +11,7 @@ import org.apache.logging.log4j.Level; import org.elasticsearch.client.internal.node.NodeClient; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.core.Nullable; import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.rest.RestRequest.Method; @@ -136,11 +137,11 @@ class Route { private final Method method; private final String path; private final RestApiVersion restApiVersion; - + @Nullable private final String deprecationMessage; @Nullable private final Level deprecationLevel; - + @Nullable private final Route replacedRoute; private Route( @@ -148,15 +149,15 @@ private Route( String path, RestApiVersion restApiVersion, String deprecationMessage, - Level deprecationLevel, + @Nullable Level deprecationLevel, Route replacedRoute ) { this.method = Objects.requireNonNull(method); this.path = Objects.requireNonNull(path); + // the last version in which this route was fully supported, defaults to the current version this.restApiVersion = Objects.requireNonNull(restApiVersion); - // a deprecated route will have a deprecation message, and the restApiVersion - // will represent the version when the route was deprecated + // a route marked as deprecated to keep or remove will have a deprecation message and level (warn for keep, critical for remove) this.deprecationMessage = deprecationMessage; this.deprecationLevel = deprecationLevel; @@ -183,7 +184,6 @@ public static class RouteBuilder { private RestApiVersion restApiVersion; private String deprecationMessage; - @Nullable private Level deprecationLevel; private Route replacedRoute; @@ -225,6 +225,7 @@ public RouteBuilder deprecatedForRemoval(String deprecationMessage, RestApiVersi assert this.replacedRoute == null; this.restApiVersion = Objects.requireNonNull(lastFullySupportedVersion); this.deprecationMessage = Objects.requireNonNull(deprecationMessage); + this.deprecationLevel = DeprecationLogger.CRITICAL; return this; } @@ -246,6 +247,7 @@ public RouteBuilder deprecatedForRemoval(String deprecationMessage, RestApiVersi */ public RouteBuilder replaces(Method method, String path, RestApiVersion lastFullySupportedVersion) { assert this.deprecationMessage == null; + // the deprecation message and level will be built later this.replacedRoute = new Route(method, path, lastFullySupportedVersion, null, null, null); return this; } @@ -271,14 +273,16 @@ public RouteBuilder deprecateAndKeep(String deprecationMessage) { } public Route build() { - if (replacedRoute != null) { - return new Route(method, path, restApiVersion, null, null, replacedRoute); - } else if (deprecationMessage != null) { - return new Route(method, path, restApiVersion, deprecationMessage, deprecationLevel, null); - } else { - // this is a little silly, but perfectly legal - return new Route(method, path, restApiVersion, null, null, null); - } + assert (deprecationMessage != null) == (deprecationLevel != null); // both must be set or neither + return new Route( + method, + path, + restApiVersion, + deprecationMessage, + deprecationLevel, + replacedRoute + ); + } } @@ -298,11 +302,11 @@ public RestApiVersion getRestApiVersion() { return restApiVersion; } + @Nullable public String getDeprecationMessage() { return deprecationMessage; } - @Nullable public Level getDeprecationLevel() { return deprecationLevel; } @@ -311,6 +315,7 @@ public boolean isDeprecated() { return deprecationMessage != null; } + @Nullable public Route getReplacedRoute() { return replacedRoute; }