Skip to content

Commit f3b7861

Browse files
committed
Add missing matching pattern attribute
Prior to this commit, we introduced in gh-906 custom `RequestPredicates` for faster and more efficient matching. These custom WebFlux and MVC predicates did not set the matching `PathPattern` as a request attribute for positive matches. This value is used by observability conventions for metrics and traces KeyValues; as a result, observation metadata is missing the "uri" metadata and is using the "UNKNOWN" value instead. This commit adds the missing request attribute in our custom request predicates. Fixes gh-987
1 parent 6b3e277 commit f3b7861

File tree

4 files changed

+88
-0
lines changed

4 files changed

+88
-0
lines changed

spring-graphql/src/main/java/org/springframework/graphql/server/webflux/GraphQlRequestPredicates.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.springframework.util.MimeTypeUtils;
3232
import org.springframework.web.cors.reactive.CorsUtils;
3333
import org.springframework.web.reactive.function.server.RequestPredicate;
34+
import org.springframework.web.reactive.function.server.RouterFunctions;
3435
import org.springframework.web.reactive.function.server.ServerRequest;
3536
import org.springframework.web.util.pattern.PathPattern;
3637
import org.springframework.web.util.pattern.PathPatternParser;
@@ -165,6 +166,9 @@ private static boolean pathMatch(ServerRequest request, PathPattern pattern) {
165166
PathContainer pathContainer = request.requestPath().pathWithinApplication();
166167
boolean pathMatch = pattern.matches(pathContainer);
167168
traceMatch("Pattern", pattern.getPatternString(), request.path(), pathMatch);
169+
if (pathMatch) {
170+
request.attributes().put(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE, pattern);
171+
}
168172
return pathMatch;
169173
}
170174

spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlRequestPredicates.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.springframework.util.MimeTypeUtils;
3232
import org.springframework.web.cors.CorsUtils;
3333
import org.springframework.web.servlet.function.RequestPredicate;
34+
import org.springframework.web.servlet.function.RouterFunctions;
3435
import org.springframework.web.servlet.function.ServerRequest;
3536
import org.springframework.web.util.pattern.PathPattern;
3637
import org.springframework.web.util.pattern.PathPatternParser;
@@ -165,6 +166,9 @@ private static boolean pathMatch(ServerRequest request, PathPattern pattern) {
165166
PathContainer pathContainer = request.requestPath().pathWithinApplication();
166167
boolean pathMatch = pattern.matches(pathContainer);
167168
traceMatch("Pattern", pattern.getPatternString(), request.path(), pathMatch);
169+
if (pathMatch) {
170+
request.attributes().put(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE, pattern);
171+
}
168172
return pathMatch;
169173
}
170174

spring-graphql/src/test/java/org/springframework/graphql/server/webflux/GraphQlRequestPredicatesTests.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,10 @@
2828
import org.springframework.mock.http.server.reactive.MockServerHttpRequest;
2929
import org.springframework.mock.web.server.MockServerWebExchange;
3030
import org.springframework.web.reactive.function.server.RequestPredicate;
31+
import org.springframework.web.reactive.function.server.RouterFunctions;
3132
import org.springframework.web.reactive.function.server.ServerRequest;
3233
import org.springframework.web.server.ServerWebExchange;
34+
import org.springframework.web.util.pattern.PathPatternParser;
3335

3436
import static org.assertj.core.api.Assertions.assertThat;
3537

@@ -107,6 +109,25 @@ void shouldRejectRequestWithIncompatibleAccept() {
107109
assertThat(httpPredicate.test(serverRequest)).isFalse();
108110
}
109111

112+
@Test
113+
void shouldSetMatchingPatternAttribute() {
114+
ServerWebExchange exchange = createMatchingHttpExchange();
115+
ServerRequest serverRequest = ServerRequest.create(exchange, Collections.emptyList());
116+
httpPredicate.test(serverRequest);
117+
assertThat(serverRequest.attribute(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE))
118+
.hasValue(PathPatternParser.defaultInstance.parse("/graphql"));
119+
}
120+
121+
@Test
122+
void shouldNotSetAttributeWhenNoMatch() {
123+
ServerWebExchange exchange = createMatchingHttpExchange()
124+
.mutate().request(req -> req.path("/invalid")).build();
125+
ServerRequest serverRequest = ServerRequest.create(exchange, Collections.emptyList());
126+
httpPredicate.test(serverRequest);
127+
assertThat(serverRequest.attribute(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE))
128+
.isEmpty();
129+
}
130+
110131
private MockServerWebExchange createMatchingHttpExchange() {
111132
MockServerHttpRequest request = MockServerHttpRequest.post("/graphql")
112133
.contentType(MediaType.APPLICATION_JSON)
@@ -172,6 +193,25 @@ void shouldRejectRequestWithIncompatibleAccept() {
172193
assertThat(ssePredicate.test(serverRequest)).isFalse();
173194
}
174195

196+
@Test
197+
void shouldSetMatchingPatternAttribute() {
198+
ServerWebExchange exchange = createMatchingSseExchange();
199+
ServerRequest serverRequest = ServerRequest.create(exchange, Collections.emptyList());
200+
ssePredicate.test(serverRequest);
201+
assertThat(serverRequest.attribute(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE))
202+
.hasValue(PathPatternParser.defaultInstance.parse("/graphql"));
203+
}
204+
205+
@Test
206+
void shouldNotSetAttributeWhenNoMatch() {
207+
ServerWebExchange exchange = createMatchingSseExchange()
208+
.mutate().request(req -> req.path("/invalid")).build();
209+
ServerRequest serverRequest = ServerRequest.create(exchange, Collections.emptyList());
210+
ssePredicate.test(serverRequest);
211+
assertThat(serverRequest.attribute(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE))
212+
.isEmpty();
213+
}
214+
175215
private MockServerWebExchange createMatchingSseExchange() {
176216
MockServerHttpRequest request = MockServerHttpRequest.post("/graphql")
177217
.contentType(MediaType.APPLICATION_JSON)

spring-graphql/src/test/java/org/springframework/graphql/server/webmvc/GraphQlRequestPredicatesTests.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@
2525
import org.springframework.http.HttpHeaders;
2626
import org.springframework.mock.web.MockHttpServletRequest;
2727
import org.springframework.web.servlet.function.RequestPredicate;
28+
import org.springframework.web.servlet.function.RouterFunctions;
2829
import org.springframework.web.servlet.function.ServerRequest;
30+
import org.springframework.web.util.pattern.PathPatternParser;
2931

3032
import static org.assertj.core.api.Assertions.assertThat;
3133

@@ -99,6 +101,25 @@ void shouldRejectRequestWithIncompatibleAccept() {
99101
assertThat(httpPredicate.test(serverRequest)).isFalse();
100102
}
101103

104+
@Test
105+
void shouldSetMatchingPatternAttribute() {
106+
MockHttpServletRequest request = createMatchingHttpRequest();
107+
ServerRequest serverRequest = ServerRequest.create(request, Collections.emptyList());
108+
httpPredicate.test(serverRequest);
109+
assertThat(serverRequest.attribute(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE))
110+
.hasValue(PathPatternParser.defaultInstance.parse("/graphql"));
111+
}
112+
113+
@Test
114+
void shouldNotSetAttributeWhenNoMatch() {
115+
MockHttpServletRequest request = createMatchingHttpRequest();
116+
request.setRequestURI("/invalid");
117+
ServerRequest serverRequest = ServerRequest.create(request, Collections.emptyList());
118+
httpPredicate.test(serverRequest);
119+
assertThat(serverRequest.attribute(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE))
120+
.isEmpty();
121+
}
122+
102123
private MockHttpServletRequest createMatchingHttpRequest() {
103124
MockHttpServletRequest request = new MockHttpServletRequest("POST", "/graphql");
104125
request.setContentType("application/json");
@@ -163,6 +184,25 @@ void shouldRejectRequestWithIncompatibleAccept() {
163184
assertThat(ssePredicate.test(serverRequest)).isFalse();
164185
}
165186

187+
@Test
188+
void shouldSetMatchingPatternAttribute() {
189+
MockHttpServletRequest request = createMatchingSseRequest();
190+
ServerRequest serverRequest = ServerRequest.create(request, Collections.emptyList());
191+
ssePredicate.test(serverRequest);
192+
assertThat(serverRequest.attribute(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE))
193+
.hasValue(PathPatternParser.defaultInstance.parse("/graphql"));
194+
}
195+
196+
@Test
197+
void shouldNotSetAttributeWhenNoMatch() {
198+
MockHttpServletRequest request = createMatchingSseRequest();
199+
request.setRequestURI("/invalid");
200+
ServerRequest serverRequest = ServerRequest.create(request, Collections.emptyList());
201+
ssePredicate.test(serverRequest);
202+
assertThat(serverRequest.attribute(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE))
203+
.isEmpty();
204+
}
205+
166206
private MockHttpServletRequest createMatchingSseRequest() {
167207
MockHttpServletRequest request = new MockHttpServletRequest("POST", "/graphql");
168208
request.addHeader("Content-Type", "application/json");

0 commit comments

Comments
 (0)