Skip to content

Commit 3c5b74b

Browse files
authored
Better support for RouteBuilder for deprecation removal vs. keep (#113712)
This commit is a follow up to #113151 to better clarify how to deprecate HTTP routes. That change introduced RouteBuilder.deprecateAndKeep to enable the ability to deprecate an HTTP API, but not require REST compatibilty headers in the next major version. This commit deprecates the java methnod RouteBuilder.deprecated and introduces RouteBuilder.deprecatedForRemoval. The valid options are now RouteBuilder.deprecateAndKeep vs. RouteBuilder.deprecatedForRemoval where the later will require compatiblity headers to access the route in any newer REST API versions than the lastFullySupportedVersion declared. The javadoc should help to provide some clarification. Additionally, the deprecation level should not be configurable. The deprecation level of WARN, when applied to routes, is informational only (and may require compatibility headers in the next version). The deprecation level of CRITICAL means means no access what so ever in the next major version. Generally, CRTICIAL is used for any cases where the compatibility is required (which means it is the last version for any access), or no compatibility is planned. Some examples: ``` Route.builder(GET, "/foo/bar").build() -> no deprecations Route.builder(GET, "/foo/bar").deprecateAndKeep("my message").build() -> deprecated, but removal is not influenced by REST API compatibilty Route.builder(GET, "/foo/bar").deprecatedForRemoval("my message", V_8).build() -> will be available in V_8, but emit a warn level deprecation, V_9 will require compatiblity headers and emit a CRITICAL deprecation, and V_10 this will no longer be available Route.builder(GET, "/foo/bar").replaces(GET, "/foo/baz", V_8).build() -> /foo/bar will be available in all versions. /foo/baz will be available in V_8 but emit a warn level deprecation, V_9 will require compatibility headers and emit a CRITICAL deprecation, and V_10 this will no longer be available. This is effectively a short cut to register a new route ("/foo/bar") and deprecatedForRemoval the path being replaced. ``` The functionality remains unchanged and this refactoring only provides better contracts and cleans up some of the implementation.
1 parent 663ab04 commit 3c5b74b

File tree

7 files changed

+318
-124
lines changed

7 files changed

+318
-124
lines changed

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

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import org.elasticsearch.common.Strings;
1414
import org.elasticsearch.common.logging.DeprecationCategory;
1515
import org.elasticsearch.common.logging.DeprecationLogger;
16-
import org.elasticsearch.core.Nullable;
1716

1817
import java.util.Objects;
1918

@@ -29,7 +28,6 @@ public class DeprecationRestHandler extends FilterRestHandler implements RestHan
2928
private final DeprecationLogger deprecationLogger;
3029
private final boolean compatibleVersionWarning;
3130
private final String deprecationKey;
32-
@Nullable
3331
private final Level deprecationLevel;
3432

3533
/**
@@ -39,6 +37,8 @@ public class DeprecationRestHandler extends FilterRestHandler implements RestHan
3937
* @param handler The rest handler to deprecate (it's possible that the handler is reused with a different name!)
4038
* @param method a method of a deprecated endpoint
4139
* @param path a path of a deprecated endpoint
40+
* @param deprecationLevel The level of the deprecation warning, must be non-null
41+
* and either {@link Level#WARN} or {@link DeprecationLogger#CRITICAL}
4242
* @param deprecationMessage The message to warn users with when they use the {@code handler}
4343
* @param deprecationLogger The deprecation logger
4444
* @param compatibleVersionWarning set to false so that a deprecation warning will be issued for the handled request,
@@ -51,7 +51,7 @@ public DeprecationRestHandler(
5151
RestHandler handler,
5252
RestRequest.Method method,
5353
String path,
54-
@Nullable Level deprecationLevel,
54+
Level deprecationLevel,
5555
String deprecationMessage,
5656
DeprecationLogger deprecationLogger,
5757
boolean compatibleVersionWarning
@@ -61,7 +61,7 @@ public DeprecationRestHandler(
6161
this.deprecationLogger = Objects.requireNonNull(deprecationLogger);
6262
this.compatibleVersionWarning = compatibleVersionWarning;
6363
this.deprecationKey = DEPRECATED_ROUTE_KEY + "_" + method + "_" + path;
64-
if (deprecationLevel != null && (deprecationLevel != Level.WARN && deprecationLevel != DeprecationLogger.CRITICAL)) {
64+
if (deprecationLevel != Level.WARN && deprecationLevel != DeprecationLogger.CRITICAL) {
6565
throw new IllegalArgumentException(
6666
"unexpected deprecation logger level: " + deprecationLevel + ", expected either 'CRITICAL' or 'WARN'"
6767
);
@@ -77,19 +77,18 @@ public DeprecationRestHandler(
7777
@Override
7878
public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception {
7979
if (compatibleVersionWarning == false) {
80-
// The default value for deprecated requests without a version warning is WARN
81-
if (deprecationLevel == null || deprecationLevel == Level.WARN) {
80+
// emit a standard deprecation warning
81+
if (Level.WARN == deprecationLevel) {
8282
deprecationLogger.warn(DeprecationCategory.API, deprecationKey, deprecationMessage);
83-
} else {
83+
} else if (DeprecationLogger.CRITICAL == deprecationLevel) {
8484
deprecationLogger.critical(DeprecationCategory.API, deprecationKey, deprecationMessage);
8585
}
8686
} else {
87-
// The default value for deprecated requests with a version warning is CRITICAL,
88-
// because they have a specific version where the endpoint is removed
89-
if (deprecationLevel == null || deprecationLevel == DeprecationLogger.CRITICAL) {
90-
deprecationLogger.compatibleCritical(deprecationKey, deprecationMessage);
91-
} else {
87+
// emit a compatibility warning
88+
if (Level.WARN == deprecationLevel) {
9289
deprecationLogger.compatible(Level.WARN, deprecationKey, deprecationMessage);
90+
} else if (DeprecationLogger.CRITICAL == deprecationLevel) {
91+
deprecationLogger.compatibleCritical(deprecationKey, deprecationMessage);
9392
}
9493
}
9594

@@ -139,4 +138,9 @@ public static String requireValidHeader(String value) {
139138

140139
return value;
141140
}
141+
142+
// test only
143+
Level getDeprecationLevel() {
144+
return deprecationLevel;
145+
}
142146
}

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

Lines changed: 28 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -144,25 +144,6 @@ public ServerlessApiProtections getApiProtections() {
144144
return apiProtections;
145145
}
146146

147-
/**
148-
* Registers a REST handler to be executed when the provided {@code method} and {@code path} match the request.
149-
*
150-
* @param method GET, POST, etc.
151-
* @param path Path to handle (e.g. "/{index}/{type}/_bulk")
152-
* @param version API version to handle (e.g. RestApiVersion.V_8)
153-
* @param handler The handler to actually execute
154-
* @param deprecationMessage The message to log and send as a header in the response
155-
*/
156-
protected void registerAsDeprecatedHandler(
157-
RestRequest.Method method,
158-
String path,
159-
RestApiVersion version,
160-
RestHandler handler,
161-
String deprecationMessage
162-
) {
163-
registerAsDeprecatedHandler(method, path, version, handler, deprecationMessage, null);
164-
}
165-
166147
/**
167148
* Registers a REST handler to be executed when the provided {@code method} and {@code path} match the request.
168149
*
@@ -179,40 +160,23 @@ protected void registerAsDeprecatedHandler(
179160
RestApiVersion version,
180161
RestHandler handler,
181162
String deprecationMessage,
182-
@Nullable Level deprecationLevel
163+
Level deprecationLevel
183164
) {
184165
assert (handler instanceof DeprecationRestHandler) == false;
185-
if (version == RestApiVersion.current()) {
186-
// e.g. it was marked as deprecated in 8.x, and we're currently running 8.x
187-
registerHandler(
188-
method,
189-
path,
190-
version,
191-
new DeprecationRestHandler(handler, method, path, deprecationLevel, deprecationMessage, deprecationLogger, false)
192-
);
193-
} else if (version == RestApiVersion.minimumSupported()) {
194-
// e.g. it was marked as deprecated in 7.x, and we're currently running 8.x
166+
if (RestApiVersion.onOrAfter(RestApiVersion.minimumSupported()).test(version)) {
195167
registerHandler(
196168
method,
197169
path,
198170
version,
199-
new DeprecationRestHandler(handler, method, path, deprecationLevel, deprecationMessage, deprecationLogger, true)
200-
);
201-
} else {
202-
// e.g. it was marked as deprecated in 7.x, and we're currently running *9.x*
203-
logger.debug(
204-
"Deprecated route ["
205-
+ method
206-
+ " "
207-
+ path
208-
+ "] for handler ["
209-
+ handler.getClass()
210-
+ "] "
211-
+ "with version ["
212-
+ version
213-
+ "], which is less than the minimum supported version ["
214-
+ RestApiVersion.minimumSupported()
215-
+ "]"
171+
new DeprecationRestHandler(
172+
handler,
173+
method,
174+
path,
175+
deprecationLevel,
176+
deprecationMessage,
177+
deprecationLogger,
178+
version != RestApiVersion.current()
179+
)
216180
);
217181
}
218182
}
@@ -250,21 +214,12 @@ protected void registerAsReplacedHandler(
250214
RestHandler handler,
251215
RestRequest.Method replacedMethod,
252216
String replacedPath,
253-
RestApiVersion replacedVersion
217+
RestApiVersion replacedVersion,
218+
String replacedMessage,
219+
Level deprecationLevel
254220
) {
255-
// e.g. [POST /_optimize] is deprecated! Use [POST /_forcemerge] instead.
256-
final String replacedMessage = "["
257-
+ replacedMethod.name()
258-
+ " "
259-
+ replacedPath
260-
+ "] is deprecated! Use ["
261-
+ method.name()
262-
+ " "
263-
+ path
264-
+ "] instead.";
265-
266221
registerHandler(method, path, version, handler);
267-
registerAsDeprecatedHandler(replacedMethod, replacedPath, replacedVersion, handler, replacedMessage);
222+
registerAsDeprecatedHandler(replacedMethod, replacedPath, replacedVersion, handler, replacedMessage, deprecationLevel);
268223
}
269224

270225
/**
@@ -284,7 +239,15 @@ protected void registerHandler(RestRequest.Method method, String path, RestApiVe
284239

285240
private void registerHandlerNoWrap(RestRequest.Method method, String path, RestApiVersion version, RestHandler handler) {
286241
assert RestApiVersion.minimumSupported() == version || RestApiVersion.current() == version
287-
: "REST API compatibility is only supported for version " + RestApiVersion.minimumSupported().major;
242+
: "REST API compatibility is only supported for version "
243+
+ RestApiVersion.minimumSupported().major
244+
+ " [method="
245+
+ method
246+
+ ", path="
247+
+ path
248+
+ ", handler="
249+
+ handler.getClass().getCanonicalName()
250+
+ "]";
288251

289252
if (RESERVED_PATHS.contains(path)) {
290253
throw new IllegalArgumentException("path [" + path + "] is a reserved path and may not be registered");
@@ -299,7 +262,7 @@ private void registerHandlerNoWrap(RestRequest.Method method, String path, RestA
299262
}
300263

301264
public void registerHandler(final Route route, final RestHandler handler) {
302-
if (route.isReplacement()) {
265+
if (route.hasReplacement()) {
303266
Route replaced = route.getReplacedRoute();
304267
registerAsReplacedHandler(
305268
route.getMethod(),
@@ -308,7 +271,9 @@ public void registerHandler(final Route route, final RestHandler handler) {
308271
handler,
309272
replaced.getMethod(),
310273
replaced.getPath(),
311-
replaced.getRestApiVersion()
274+
replaced.getRestApiVersion(),
275+
replaced.getDeprecationMessage(),
276+
replaced.getDeprecationLevel()
312277
);
313278
} else if (route.isDeprecated()) {
314279
registerAsDeprecatedHandler(

0 commit comments

Comments
 (0)