Skip to content

Commit

Permalink
fix test
Browse files Browse the repository at this point in the history
  • Loading branch information
jakelandis committed Sep 27, 2024
1 parent 5082673 commit 808f41f
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 67 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.logging.DeprecationCategory;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.core.Nullable;

import java.util.Objects;

Expand Down
26 changes: 8 additions & 18 deletions server/src/main/java/org/elasticsearch/rest/RestController.java
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +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.
*
Expand Down Expand Up @@ -185,8 +184,8 @@ protected void registerAsDeprecatedHandler(
} else {
// 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;
// TODO: comment this back in when V_7 is no longer supported
// assert false : "Unsupported REST API version " + version;
}
}

Expand Down Expand Up @@ -223,22 +222,11 @@ protected void registerAsReplacedHandler(
RestHandler handler,
RestRequest.Method replacedMethod,
String replacedPath,
RestApiVersion replacedVersion
RestApiVersion replacedVersion,
String replacedMessage,
Level deprecationLevel
) {
// e.g. [POST /_optimize] is deprecated! Use [POST /_forcemerge] instead.
final String replacedMessage = "["
+ replacedMethod.name()
+ " "
+ replacedPath
+ "] is deprecated! Use ["
+ method.name()
+ " "
+ path
+ "] instead.";

registerHandler(method, path, version, handler);
//TODO: test this variant
Level deprecationLevel = replacedVersion == RestApiVersion.current() ? Level.WARN : DeprecationLogger.CRITICAL;
registerAsDeprecatedHandler(replacedMethod, replacedPath, replacedVersion, handler, replacedMessage, deprecationLevel);
}

Expand Down Expand Up @@ -283,7 +271,9 @@ public void registerHandler(final Route route, final RestHandler handler) {
handler,
replaced.getMethod(),
replaced.getPath(),
replaced.getRestApiVersion()
replaced.getRestApiVersion(),
replaced.getDeprecationMessage(),
replaced.getDeprecationLevel()
);
} else if (route.isDeprecated()) {
registerAsDeprecatedHandler(
Expand Down
40 changes: 32 additions & 8 deletions server/src/main/java/org/elasticsearch/rest/RestHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -153,12 +153,16 @@ private Route(
) {
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
// the last version in which this route was fully supported
this.restApiVersion = Objects.requireNonNull(restApiVersion);

// 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;
this.deprecationLevel = Objects.requireNonNull(deprecationLevel);

if (deprecationMessage == null && deprecationLevel != Level.OFF) {
throw new IllegalArgumentException("deprecationMessage must be set if deprecationLevel is not OFF");
}

// a route that replaces another route will have a reference to the route that was replaced
this.replacedRoute = replacedRoute;
Expand Down Expand Up @@ -237,17 +241,38 @@ public RouteBuilder deprecatedForRemoval(String deprecationMessage, RestApiVersi
* Route.builder(GET, "/_security/user/")
* .replaces(GET, "/_xpack/security/user/", RestApiVersion.V_7).build()}</pre>
*
* @param method the method being replaced
* @param path the path being replaced
* @param replacedMethod the method being replaced
* @param replacedPath the path being replaced
* @param lastFullySupportedVersion the last {@link RestApiVersion} (i.e. 7) for which this route is fully supported.
* The next major version (i.e. 8) will require compatibility header(s). (;compatible-with=7)
* The next major version (i.e. 9) will have no support whatsoever for this route.
* @return a reference to this object.
*/
public RouteBuilder replaces(Method method, String path, RestApiVersion lastFullySupportedVersion) {
public RouteBuilder replaces(Method replacedMethod, String replacedPath, 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);

// if being replaced in the current version, then it's a warning, otherwise it's critical
Level deprecationLevel = lastFullySupportedVersion == RestApiVersion.current() ? Level.WARN : DeprecationLogger.CRITICAL;

// e.g. [POST /_optimize] is deprecated! Use [POST /_forcemerge] instead.
final String replacedMessage = "["
+ replacedMethod.name()
+ " "
+ replacedPath
+ "] is deprecated! Use ["
+ this.method.name()
+ " "
+ this.path
+ "] instead.";

this.replacedRoute = new Route(
replacedMethod,
replacedPath,
lastFullySupportedVersion,
replacedMessage,
deprecationLevel,
null
);
return this;
}

Expand Down Expand Up @@ -281,7 +306,6 @@ public Route build() {
deprecationLevel == null ? Level.OFF : deprecationLevel,
replacedRoute
);

}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ public void testSupportsBulkContentFalse() {
);
}

public void testDeprecationLevel(){
public void testDeprecationLevel() {
DeprecationRestHandler handler = new DeprecationRestHandler(
this.handler,
METHOD,
Expand All @@ -195,26 +195,16 @@ public void testDeprecationLevel(){
);
assertEquals(DeprecationLogger.CRITICAL, handler.getDeprecationLevel());

IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> new DeprecationRestHandler(
this.handler,
METHOD,
PATH,
null,
deprecationMessage,
deprecationLogger,
false
));
IllegalArgumentException exception = expectThrows(
IllegalArgumentException.class,
() -> new DeprecationRestHandler(this.handler, METHOD, PATH, null, deprecationMessage, deprecationLogger, false)
);
assertEquals(exception.getMessage(), "unexpected deprecation logger level: null, expected either 'CRITICAL' or 'WARN'");

exception = expectThrows(IllegalArgumentException.class, () -> new DeprecationRestHandler(
this.handler,
METHOD,
PATH,
Level.OFF,
deprecationMessage,
deprecationLogger,
false
));
exception = expectThrows(
IllegalArgumentException.class,
() -> new DeprecationRestHandler(this.handler, METHOD, PATH, Level.OFF, deprecationMessage, deprecationLogger, false)
);
assertEquals(exception.getMessage(), "unexpected deprecation logger level: OFF, expected either 'CRITICAL' or 'WARN'");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -390,36 +390,35 @@ public void testRegisterAsReplacedHandler() {
for (RestApiVersion replacedInVersion : replacedInVersions) {
clearInvocations(controller);
Route route = Route.builder(method, path).replaces(replacedMethod, replacedPath, replacedInVersion).build();

// don't want to test everything -- just that it actually wraps the handler
doCallRealMethod().when(controller).registerHandler(route, handler);
Level level = replacedInVersion == current ? Level.WARN : DeprecationLogger.CRITICAL;
doCallRealMethod().when(controller)
.registerAsReplacedHandler(method, path, current, handler, replacedMethod, replacedPath, replacedInVersion);

controller.registerHandler(route, handler);

verify(controller).registerHandler(method, path, current, handler);
if (replacedInVersion.equals(current)) {
// is replaced in current version, which results in a critical deprecation warning
verify(controller).registerAsDeprecatedHandler(
replacedMethod,
replacedPath,
replacedInVersion,
.registerAsReplacedHandler(
method,
path,
current,
handler,
deprecationMessage,
Level.WARN
);
} else {
// is replaced in previous version, which results in a critical deprecation warning
verify(controller).registerAsDeprecatedHandler(
replacedMethod,
replacedPath,
replacedInVersion,
handler,
deprecationMessage,
DeprecationLogger.CRITICAL
level
);
}

controller.registerHandler(route, handler);

// verify we registered the primary handler
verify(controller).registerHandler(method, path, current, handler);
// verify we register the replaced handler with the correct deprecation message and level
verify(controller).registerAsDeprecatedHandler(
replacedMethod,
replacedPath,
replacedInVersion,
handler,
deprecationMessage,
level
);
}
}

Expand Down

0 comments on commit 808f41f

Please sign in to comment.