Skip to content

Commit

Permalink
Fix nested http.route (#8282)
Browse files Browse the repository at this point in the history
Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
  • Loading branch information
heyams and trask committed Apr 21, 2023
1 parent 1fdb6ee commit ffb63d6
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,12 @@ public static void methodEnter(

Context parentContext = Context.current();

// HttpRouteSource.CONTROLLER has useFirst true, and it will update http.route only once
// using the last portion of the nested path.
// HttpRouteSource.NESTED_CONTROLLER has useFirst false, and it will make http.route updated
// twice: 1st using the last portion of the nested path, 2nd time using the full nested path.
HttpRouteHolder.updateHttpRoute(
parentContext, HttpRouteSource.CONTROLLER, httpRouteGetter(), exchange);
parentContext, HttpRouteSource.NESTED_CONTROLLER, httpRouteGetter(), exchange);

if (handler == null) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,14 @@

package server.base;

import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.NESTED_PATH;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assumptions.assumeTrue;

import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint;
import io.opentelemetry.testing.internal.armeria.common.AggregatedHttpRequest;
import io.opentelemetry.testing.internal.armeria.common.AggregatedHttpResponse;
import org.junit.jupiter.api.Test;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.web.embedded.netty.NettyReactiveWebServerFactory;
import org.springframework.context.annotation.Bean;
Expand Down Expand Up @@ -54,4 +61,18 @@ protected Mono<ServerResponse> wrapResponse(
});
}
}

@Test
void nestedPath() {
assumeTrue(Boolean.getBoolean("testLatestDeps"));

String method = "GET";
AggregatedHttpRequest request = request(NESTED_PATH, method);
AggregatedHttpResponse response = client.execute(request).aggregate().join();
assertThat(response.status().code()).isEqualTo(NESTED_PATH.getStatus());
assertThat(response.contentUtf8()).isEqualTo(NESTED_PATH.getBody());
assertResponseHasCustomizedHeaders(response, NESTED_PATH, null);

assertTheTraces(1, null, null, null, method, NESTED_PATH, response);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,18 @@ public Mono<String> capture_headers(ServerHttpRequest request, ServerHttpRespons
});
}

@GetMapping("/nestedPath")
public Mono<String> nested_path(ServerHttpRequest request, ServerHttpResponse response) {
ServerEndpoint endpoint = ServerEndpoint.NESTED_PATH;

return wrapControllerMethod(
endpoint,
() -> {
setStatus(response, endpoint);
return endpoint.getBody();
});
}

protected abstract <T> Mono<T> wrapControllerMethod(ServerEndpoint endpoint, Supplier<T> handler);

private static void setStatus(ServerHttpResponse response, ServerEndpoint endpoint) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
package server.base;

import static org.springframework.web.reactive.function.server.RequestPredicates.GET;
import static org.springframework.web.reactive.function.server.RequestPredicates.path;
import static org.springframework.web.reactive.function.server.RouterFunctions.nest;
import static org.springframework.web.reactive.function.server.RouterFunctions.route;

import io.opentelemetry.api.trace.Span;
Expand Down Expand Up @@ -98,7 +100,17 @@ public RouterFunction<ServerResponse> createRoutes() {
request.headers().asHttpHeaders().getFirst("X-Test-Request")),
null,
null);
});
})
.andNest(
path("/nestedPath"),
nest(
path("/hello"),
route(
path("/world"),
request -> {
ServerEndpoint endpoint = ServerEndpoint.NESTED_PATH;
return respond(endpoint, null, null, null);
})));
}

protected Mono<ServerResponse> respond(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ public String expectedHttpRoute(ServerEndpoint endpoint) {
return getContextPath() + "/path/{id}/param";
case NOT_FOUND:
return "/**";
case NESTED_PATH:
return "/nestedPath/hello/world";
default:
return super.expectedHttpRoute(endpoint);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public static <T> T controller(ServerEndpoint endpoint, Supplier<T> closure) {
return GlobalTraceUtil.runWithSpan("controller", () -> closure.get());
}

private AggregatedHttpRequest request(ServerEndpoint uri, String method) {
protected AggregatedHttpRequest request(ServerEndpoint uri, String method) {
return AggregatedHttpRequest.of(HttpMethod.valueOf(method), resolveAddress(uri));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ public enum ServerEndpoint {
EXCEPTION("exception", 500, "controller exception"),
NOT_FOUND("notFound", 404, "not found"),
CAPTURE_HEADERS("captureHeaders", 200, "headers captured"),
NESTED_PATH(
"nestedPath/hello/world",
200,
"nested path"), // TODO (heya) move it to webflux test module after this has been converted to
// a class
CAPTURE_PARAMETERS("captureParameters", 200, "parameters captured"),

// TODO: add tests for the following cases:
Expand Down

0 comments on commit ffb63d6

Please sign in to comment.