Skip to content

Commit 8d91e6f

Browse files
vandonrvandonr
committed
test fix
1 parent b0a0881 commit 8d91e6f

5 files changed

Lines changed: 283 additions & 337 deletions

File tree

‎dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/build.gradle‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ dependencies {
4242
testImplementation group: 'io.vertx', name: 'vertx-web', version: '3.4.0'
4343
testImplementation group: 'io.vertx', name: 'vertx-web-client', version: '3.4.0'
4444

45-
testImplementation group: 'org.mockito', name: 'mockito-inline', version: '4.11.0'
45+
testImplementation libs.bundles.mockito
4646

4747
testImplementation project(':dd-java-agent:appsec:appsec-test-fixtures')
4848

‎dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RouteUpdateHelper.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,11 @@ public static void updateRouteFromContext(
2121
final RoutingContext routingContext,
2222
final AgentSpan parentSpan,
2323
final AgentSpan handlerSpan) {
24-
final Route currentRoute = routingContext.currentRoute();
25-
if (currentRoute == null) {
24+
Route currentRoute;
25+
try {
26+
currentRoute = routingContext.currentRoute();
27+
} catch (final RuntimeException ignored) {
28+
// Vert.x can call RouteState.matches before currentRoute is set on the context.
2629
return;
2730
}
2831
final String contextRoute = routePath(routingContext, currentRoute.getPath());

‎dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/server/RouteHandlerWrapperTest.java‎

Lines changed: 135 additions & 166 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
package server;
22

3-
import static org.junit.jupiter.api.Assertions.assertEquals;
3+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
4+
import static org.mockito.ArgumentMatchers.any;
5+
import static org.mockito.ArgumentMatchers.eq;
6+
import static org.mockito.Mockito.mock;
7+
import static org.mockito.Mockito.never;
8+
import static org.mockito.Mockito.verify;
9+
import static org.mockito.Mockito.verifyNoMoreInteractions;
10+
import static org.mockito.Mockito.when;
411

512
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
613
import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities;
@@ -10,180 +17,171 @@
1017
import io.vertx.ext.web.Route;
1118
import io.vertx.ext.web.RoutingContext;
1219
import java.lang.reflect.InvocationHandler;
13-
import java.lang.reflect.Method;
1420
import java.lang.reflect.Proxy;
15-
import java.util.ArrayList;
1621
import java.util.HashMap;
17-
import java.util.List;
1822
import java.util.Map;
1923
import org.junit.jupiter.api.Test;
2024

2125
class RouteHandlerWrapperTest {
2226

2327
@Test
2428
void updateRouteWritesRouteToBothSpans() {
25-
RecordingProxy<RoutingContext> context = RecordingProxy.of(RoutingContext.class);
26-
RecordingProxy<HttpServerRequest> request = RecordingProxy.of(HttpServerRequest.class);
27-
RecordingProxy<Route> route = RecordingProxy.of(Route.class);
28-
RecordingProxy<AgentSpan> parentSpan = RecordingProxy.of(AgentSpan.class);
29-
RecordingProxy<AgentSpan> handlerSpan = RecordingProxy.of(AgentSpan.class);
30-
route.returns("getPath", "/items/:id");
31-
context.returns("mountPoint", null);
32-
context.returns("request", request.instance);
33-
request.returns("path", "/items/123");
34-
request.returns("rawMethod", "GET");
35-
context.returns("get", null);
36-
handlerSpan.returns("getSpanName", "vertx.route-handler");
37-
handlerSpan.returns("getResourceNamePriority", Byte.MIN_VALUE);
29+
TestRoutingContext context = new TestRoutingContext("/items/123", "GET").route("/items/:id");
30+
AgentSpan parentSpan = mock(AgentSpan.class);
31+
AgentSpan handlerSpan = mock(AgentSpan.class);
32+
when(handlerSpan.getSpanName()).thenReturn("vertx.route-handler");
33+
when(handlerSpan.getResourceNamePriority()).thenReturn(Byte.MIN_VALUE);
3834

3935
RouteUpdateHelper.updateRouteFromMatchedRoute(
40-
context.instance, route.instance, parentSpan.instance, handlerSpan.instance);
41-
42-
assertEquals(1, route.count("getPath"));
43-
assertEquals(1, context.count("mountPoint"));
44-
assertEquals(2, context.count("request"));
45-
assertEquals(1, request.count("path"));
46-
assertEquals(1, request.count("rawMethod"));
47-
assertEquals(1, context.count("get", "dd." + Tags.HTTP_ROUTE));
48-
assertEquals(1, context.count("put", "dd.vertx.matched_route", "/items/:id"));
49-
assertEquals(1, context.count("put", "dd." + Tags.HTTP_ROUTE, "/items/:id"));
50-
assertEquals(1, parentSpan.count("setTag", Tags.HTTP_ROUTE, "/items/:id"));
51-
assertEquals(
52-
1,
53-
parentSpan.count(
54-
"setResourceName", "GET /items/:id", ResourceNamePriorities.HTTP_FRAMEWORK_ROUTE));
55-
assertEquals(1, handlerSpan.count("getSpanName"));
56-
assertEquals(1, handlerSpan.count("getResourceNamePriority"));
57-
assertEquals(1, handlerSpan.count("setTag", Tags.HTTP_ROUTE, "/items/:id"));
58-
assertEquals(
59-
1,
60-
handlerSpan.count(
61-
"setResourceName", "GET /items/:id", ResourceNamePriorities.HTTP_FRAMEWORK_ROUTE));
36+
context.instance(), context.route(), parentSpan, handlerSpan);
37+
38+
context.verify("dd.vertx.matched_route", "/items/:id");
39+
context.verify("dd." + Tags.HTTP_ROUTE, "/items/:id");
40+
verify(parentSpan).setTag(Tags.HTTP_ROUTE, (CharSequence) "/items/:id");
41+
verify(parentSpan)
42+
.setResourceName(any(CharSequence.class), eq(ResourceNamePriorities.HTTP_FRAMEWORK_ROUTE));
43+
verify(handlerSpan).getSpanName();
44+
verify(handlerSpan).getResourceNamePriority();
45+
verify(handlerSpan).setTag(Tags.HTTP_ROUTE, (CharSequence) "/items/:id");
46+
verify(handlerSpan)
47+
.setResourceName(any(CharSequence.class), eq(ResourceNamePriorities.HTTP_FRAMEWORK_ROUTE));
48+
verifyNoMoreInteractions(parentSpan, handlerSpan);
6249
}
6350

6451
@Test
6552
void updateRouteDoesNotWriteRouteToNonVertxHandlerSpan() {
66-
RecordingProxy<RoutingContext> context = RecordingProxy.of(RoutingContext.class);
67-
RecordingProxy<HttpServerRequest> request = RecordingProxy.of(HttpServerRequest.class);
68-
RecordingProxy<Route> route = RecordingProxy.of(Route.class);
69-
RecordingProxy<AgentSpan> parentSpan = RecordingProxy.of(AgentSpan.class);
70-
RecordingProxy<AgentSpan> handlerSpan = RecordingProxy.of(AgentSpan.class);
71-
route.returns("getPath", "/items/:id");
72-
context.returns("mountPoint", null);
73-
context.returns("request", request.instance);
74-
request.returns("path", "/items/123");
75-
request.returns("rawMethod", "GET");
76-
context.returns("get", null);
77-
handlerSpan.returns("getSpanName", "some.other.span");
53+
TestRoutingContext context = new TestRoutingContext("/items/123", "GET").route("/items/:id");
54+
AgentSpan parentSpan = mock(AgentSpan.class);
55+
AgentSpan handlerSpan = mock(AgentSpan.class);
56+
when(handlerSpan.getSpanName()).thenReturn("some.other.span");
7857

7958
RouteUpdateHelper.updateRouteFromMatchedRoute(
80-
context.instance, route.instance, parentSpan.instance, handlerSpan.instance);
81-
82-
assertEquals(1, route.count("getPath"));
83-
assertEquals(1, context.count("mountPoint"));
84-
assertEquals(2, context.count("request"));
85-
assertEquals(1, request.count("path"));
86-
assertEquals(1, request.count("rawMethod"));
87-
assertEquals(1, context.count("get", "dd." + Tags.HTTP_ROUTE));
88-
assertEquals(1, context.count("put", "dd.vertx.matched_route", "/items/:id"));
89-
assertEquals(1, context.count("put", "dd." + Tags.HTTP_ROUTE, "/items/:id"));
90-
assertEquals(1, parentSpan.count("setTag", Tags.HTTP_ROUTE, "/items/:id"));
91-
assertEquals(
92-
1,
93-
parentSpan.count(
94-
"setResourceName", "GET /items/:id", ResourceNamePriorities.HTTP_FRAMEWORK_ROUTE));
95-
assertEquals(1, handlerSpan.count("getSpanName"));
96-
assertEquals(0, handlerSpan.count("setTag"));
97-
assertEquals(0, handlerSpan.count("setResourceName"));
59+
context.instance(), context.route(), parentSpan, handlerSpan);
60+
61+
context.verify("dd.vertx.matched_route", "/items/:id");
62+
context.verify("dd." + Tags.HTTP_ROUTE, "/items/:id");
63+
verify(parentSpan).setTag(Tags.HTTP_ROUTE, (CharSequence) "/items/:id");
64+
verify(parentSpan)
65+
.setResourceName(any(CharSequence.class), eq(ResourceNamePriorities.HTTP_FRAMEWORK_ROUTE));
66+
verify(handlerSpan).getSpanName();
67+
verify(handlerSpan, never()).setTag(any(String.class), any(CharSequence.class));
68+
verifyNoMoreInteractions(parentSpan, handlerSpan);
9869
}
9970

10071
@Test
10172
void updateRouteDoesNotReplaceRootRouteWhenOneExists() {
102-
RecordingProxy<RoutingContext> context = RecordingProxy.of(RoutingContext.class);
103-
RecordingProxy<HttpServerRequest> request = RecordingProxy.of(HttpServerRequest.class);
104-
RecordingProxy<Route> route = RecordingProxy.of(Route.class);
105-
RecordingProxy<AgentSpan> parentSpan = RecordingProxy.of(AgentSpan.class);
106-
RecordingProxy<AgentSpan> handlerSpan = RecordingProxy.of(AgentSpan.class);
107-
route.returns("getPath", "/");
108-
context.returns("mountPoint", null);
109-
context.returns("request", request.instance);
110-
request.returns("path", "/");
111-
request.returns("rawMethod", "GET");
112-
context.returns("get", null);
113-
parentSpan.returns("getTag", "/existing");
73+
TestRoutingContext context = new TestRoutingContext("/", "GET").route("/");
74+
AgentSpan parentSpan = mock(AgentSpan.class);
75+
AgentSpan handlerSpan = mock(AgentSpan.class);
76+
when(parentSpan.getTag(Tags.HTTP_ROUTE)).thenReturn("/existing");
11477

11578
RouteUpdateHelper.updateRouteFromMatchedRoute(
116-
context.instance, route.instance, parentSpan.instance, handlerSpan.instance);
117-
118-
assertEquals(1, route.count("getPath"));
119-
assertEquals(1, context.count("mountPoint"));
120-
assertEquals(2, context.count("request"));
121-
assertEquals(1, request.count("path"));
122-
assertEquals(1, request.count("rawMethod"));
123-
assertEquals(1, context.count("get", "dd." + Tags.HTTP_ROUTE));
124-
assertEquals(1, context.count("put", "dd.vertx.matched_route", "/"));
125-
assertEquals(1, parentSpan.count("getTag", Tags.HTTP_ROUTE));
126-
assertEquals(0, context.count("put", "dd." + Tags.HTTP_ROUTE, "/"));
127-
assertEquals(0, parentSpan.count("setTag"));
128-
assertEquals(0, handlerSpan.count("setTag"));
79+
context.instance(), context.route(), parentSpan, handlerSpan);
80+
81+
context.verify("dd.vertx.matched_route", "/");
82+
context.verifyUnset("dd." + Tags.HTTP_ROUTE);
83+
verify(parentSpan).getTag(Tags.HTTP_ROUTE);
84+
verify(parentSpan, never()).setTag(any(String.class), any(CharSequence.class));
85+
verify(handlerSpan, never()).setTag(any(String.class), any(CharSequence.class));
86+
verifyNoMoreInteractions(parentSpan, handlerSpan);
12987
}
13088

131-
private static final class RecordingProxy<T> implements InvocationHandler {
132-
private final Map<String, Object> returnValues = new HashMap<>();
133-
private final List<Call> calls = new ArrayList<>();
134-
private final T instance;
89+
@Test
90+
void updateRouteIgnoresUnavailableCurrentRouteFallback() {
91+
TestRoutingContext context = new TestRoutingContext("/items/123", "GET");
92+
context.currentRouteFailure = new NullPointerException("currentRoute");
93+
AgentSpan parentSpan = mock(AgentSpan.class);
94+
AgentSpan handlerSpan = mock(AgentSpan.class);
95+
96+
assertDoesNotThrow(
97+
() ->
98+
RouteUpdateHelper.updateRouteFromMatchedRoute(
99+
context.instance(), new Object(), parentSpan, handlerSpan));
100+
101+
context.verifyUnset("dd." + Tags.HTTP_ROUTE);
102+
verifyNoMoreInteractions(parentSpan, handlerSpan);
103+
}
135104

136-
private RecordingProxy(Class<T> type) {
137-
this.instance =
138-
type.cast(Proxy.newProxyInstance(type.getClassLoader(), new Class<?>[] {type}, this));
105+
private static final class TestRoutingContext implements InvocationHandler {
106+
private final RoutingContext instance;
107+
private final HttpServerRequest request;
108+
private final Map<String, Object> data = new HashMap<>();
109+
private String routePath;
110+
private RuntimeException currentRouteFailure;
111+
112+
private TestRoutingContext(String requestPath, String rawMethod) {
113+
this.instance = proxy(RoutingContext.class, this);
114+
this.request = request(requestPath, rawMethod);
139115
}
140116

141-
static <T> RecordingProxy<T> of(Class<T> type) {
142-
return new RecordingProxy<T>(type);
117+
private TestRoutingContext route(String routePath) {
118+
this.routePath = routePath;
119+
return this;
143120
}
144121

145-
void returns(String method, Object value) {
146-
returnValues.put(method, value);
122+
private RoutingContext instance() {
123+
return instance;
147124
}
148125

149-
int count(String method, Object... args) {
150-
int count = 0;
151-
for (Call call : calls) {
152-
if (call.matches(method, args)) {
153-
count++;
154-
}
155-
}
156-
return count;
126+
private Route route() {
127+
return routePath == null ? null : routeProxy(routePath);
128+
}
129+
130+
private void verify(String key, Object value) {
131+
org.junit.jupiter.api.Assertions.assertEquals(value, data.get(key));
132+
}
133+
134+
private void verifyUnset(String key) {
135+
org.junit.jupiter.api.Assertions.assertFalse(data.containsKey(key));
157136
}
158137

159138
@Override
160-
public Object invoke(Object proxy, Method method, Object[] args) {
161-
if (method.getDeclaringClass() == Object.class) {
162-
return invokeObjectMethod(proxy, method, args);
139+
public Object invoke(Object proxy, java.lang.reflect.Method method, Object[] args) {
140+
switch (method.getName()) {
141+
case "mountPoint":
142+
return null;
143+
case "request":
144+
return request;
145+
case "currentRoute":
146+
if (currentRouteFailure != null) {
147+
throw currentRouteFailure;
148+
}
149+
return route();
150+
case "get":
151+
return data.get(args[0]);
152+
case "put":
153+
data.put((String) args[0], args[1]);
154+
return proxy;
155+
default:
156+
return defaultValue(method.getReturnType());
163157
}
164-
Object[] arguments = args == null ? new Object[0] : args;
165-
calls.add(new Call(method.getName(), arguments));
166-
if (returnValues.containsKey(method.getName())) {
167-
return returnValues.get(method.getName());
168-
}
169-
Class<?> returnType = method.getReturnType();
170-
if (returnType.isInstance(proxy)) {
171-
return proxy;
172-
}
173-
return defaultValue(returnType);
174158
}
175159

176-
private static Object invokeObjectMethod(Object proxy, Method method, Object[] args) {
177-
if ("toString".equals(method.getName())) {
178-
return proxy.getClass().getInterfaces()[0].getName() + " proxy";
179-
}
180-
if ("hashCode".equals(method.getName())) {
181-
return System.identityHashCode(proxy);
182-
}
183-
if ("equals".equals(method.getName())) {
184-
return proxy == args[0];
185-
}
186-
throw new UnsupportedOperationException(method.getName());
160+
private static HttpServerRequest request(String path, String rawMethod) {
161+
return proxy(
162+
HttpServerRequest.class,
163+
(proxy, method, args) -> {
164+
switch (method.getName()) {
165+
case "path":
166+
return path;
167+
case "rawMethod":
168+
return rawMethod;
169+
default:
170+
return defaultValue(method.getReturnType());
171+
}
172+
});
173+
}
174+
175+
private static Route routeProxy(String path) {
176+
return proxy(
177+
Route.class,
178+
(proxy, method, args) ->
179+
"getPath".equals(method.getName()) ? path : defaultValue(method.getReturnType()));
180+
}
181+
182+
private static <T> T proxy(Class<T> type, InvocationHandler handler) {
183+
return type.cast(
184+
Proxy.newProxyInstance(type.getClassLoader(), new Class<?>[] {type}, handler));
187185
}
188186

189187
private static Object defaultValue(Class<?> type) {
@@ -217,33 +215,4 @@ private static Object defaultValue(Class<?> type) {
217215
return null;
218216
}
219217
}
220-
221-
private static final class Call {
222-
private final String method;
223-
private final Object[] args;
224-
225-
private Call(String method, Object[] args) {
226-
this.method = method;
227-
this.args = args;
228-
}
229-
230-
private boolean matches(String method, Object[] args) {
231-
if (!this.method.equals(method) || this.args.length != args.length) {
232-
return false;
233-
}
234-
for (int i = 0; i < args.length; i++) {
235-
if (!argumentMatches(this.args[i], args[i])) {
236-
return false;
237-
}
238-
}
239-
return true;
240-
}
241-
242-
private static boolean argumentMatches(Object actual, Object expected) {
243-
if (actual instanceof CharSequence && expected instanceof CharSequence) {
244-
return actual.toString().contentEquals((CharSequence) expected);
245-
}
246-
return java.util.Objects.equals(actual, expected);
247-
}
248-
}
249218
}

‎dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RouteUpdateHelper.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,11 @@ public static void updateRouteFromContext(
2121
final RoutingContext routingContext,
2222
final AgentSpan parentSpan,
2323
final AgentSpan handlerSpan) {
24-
final Route currentRoute = routingContext.currentRoute();
25-
if (currentRoute == null) {
24+
Route currentRoute;
25+
try {
26+
currentRoute = routingContext.currentRoute();
27+
} catch (final RuntimeException ignored) {
28+
// Vert.x can call RouteState.matches before currentRoute is set on the context.
2629
return;
2730
}
2831
final String contextRoute = routePath(routingContext, routePath(currentRoute));

0 commit comments

Comments
 (0)