Skip to content

Commit ffdf941

Browse files
rstoyanchevrstoyanchev
committed
Resolve API version in RequestMappingHandlerMapping
API version resolution and parsing is already applied as long as an ApiVersionStrategy is configured and irrespective of whether a given RequestMapping has a version or not. RequestMappingHandlerMapping also needs to be aware of the API version in order to apply deprecated version handling. So it is better to resolve, parse, and validate the version in the beginning of handler mapping rather than in the first call to any VersionRequestCondition. Closes gh-35049
1 parent 492e51f commit ffdf941

File tree

8 files changed

+115
-182
lines changed

8 files changed

+115
-182
lines changed

‎spring-web/src/test/java/org/springframework/web/accept/DefaultApiVersionStrategiesTests.java

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,29 +36,37 @@ public class DefaultApiVersionStrategiesTests {
3636

3737

3838
@Test
39-
void defaultVersion() {
39+
void defaultVersionIsParsed() {
4040
SemanticApiVersionParser.Version version = this.parser.parseVersion("1.2.3");
4141
ApiVersionStrategy strategy = initVersionStrategy(version.toString());
4242

4343
assertThat(strategy.getDefaultVersion()).isEqualTo(version);
4444
}
4545

4646
@Test
47-
void supportedVersions() {
48-
SemanticApiVersionParser.Version v1 = this.parser.parseVersion("1");
49-
SemanticApiVersionParser.Version v2 = this.parser.parseVersion("2");
50-
SemanticApiVersionParser.Version v9 = this.parser.parseVersion("9");
47+
void validateSupportedVersion() {
48+
SemanticApiVersionParser.Version v12 = this.parser.parseVersion("1.2");
5149

5250
DefaultApiVersionStrategy strategy = initVersionStrategy(null);
53-
strategy.addSupportedVersion(v1.toString());
54-
strategy.addSupportedVersion(v2.toString());
51+
strategy.addSupportedVersion(v12.toString());
5552

5653
MockHttpServletRequest request = new MockHttpServletRequest();
57-
strategy.validateVersion(v1, request);
58-
strategy.validateVersion(v2, request);
54+
strategy.validateVersion(v12, request);
55+
}
56+
57+
@Test
58+
void validateUnsupportedVersion() {
59+
assertThatThrownBy(() -> initVersionStrategy(null).validateVersion("1.2", new MockHttpServletRequest()))
60+
.isInstanceOf(InvalidApiVersionException.class)
61+
.hasMessage("400 BAD_REQUEST \"Invalid API version: '1.2'.\"");
62+
}
5963

60-
assertThatThrownBy(() -> strategy.validateVersion(v9, request))
61-
.isInstanceOf(InvalidApiVersionException.class);
64+
@Test
65+
void missingRequiredVersion() {
66+
DefaultApiVersionStrategy strategy = initVersionStrategy(null);
67+
assertThatThrownBy(() -> strategy.validateVersion(null, new MockHttpServletRequest()))
68+
.isInstanceOf(MissingApiVersionException.class)
69+
.hasMessage("400 BAD_REQUEST \"API version is required.\"");
6270
}
6371

6472
private static DefaultApiVersionStrategy initVersionStrategy(@Nullable String defaultValue) {

‎spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/VersionRequestCondition.java

Lines changed: 6 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424

2525
import org.springframework.util.Assert;
2626
import org.springframework.util.StringUtils;
27-
import org.springframework.web.accept.InvalidApiVersionException;
2827
import org.springframework.web.accept.NotAcceptableApiVersionException;
2928
import org.springframework.web.bind.annotation.RequestMapping;
3029
import org.springframework.web.reactive.HandlerMapping;
@@ -42,21 +41,12 @@
4241
*/
4342
public final class VersionRequestCondition extends AbstractRequestCondition<VersionRequestCondition> {
4443

45-
private static final String VERSION_ATTRIBUTE_NAME = VersionRequestCondition.class.getName() + ".VERSION";
46-
47-
private static final String NO_VERSION_ATTRIBUTE = "NO_VERSION";
48-
49-
private static final ApiVersionStrategy NO_OP_VERSION_STRATEGY = new NoOpApiVersionStrategy();
50-
51-
5244
private final @Nullable String versionValue;
5345

5446
private final @Nullable Object version;
5547

5648
private final boolean baselineVersion;
5749

58-
private final ApiVersionStrategy versionStrategy;
59-
6050
private final Set<String> content;
6151

6252

@@ -67,14 +57,12 @@ public final class VersionRequestCondition extends AbstractRequestCondition<Vers
6757
public VersionRequestCondition(@Nullable String version, @Nullable ApiVersionStrategy strategy) {
6858
if (StringUtils.hasText(version)) {
6959
Assert.isTrue(strategy != null, "ApiVersionStrategy is required for mapping by version");
70-
this.versionStrategy = strategy;
7160
this.baselineVersion = version.endsWith("+");
7261
this.versionValue = updateVersion(version, this.baselineVersion);
73-
this.version = this.versionStrategy.parseVersion(this.versionValue);
62+
this.version = strategy.parseVersion(this.versionValue);
7463
this.content = Set.of(version);
7564
}
7665
else {
77-
this.versionStrategy = (strategy != null ? strategy : NO_OP_VERSION_STRATEGY);
7866
this.versionValue = null;
7967
this.version = null;
8068
this.baselineVersion = false;
@@ -111,37 +99,19 @@ public VersionRequestCondition combine(VersionRequestCondition other) {
11199

112100
@Override
113101
public @Nullable VersionRequestCondition getMatchingCondition(ServerWebExchange exchange) {
114-
Comparable<?> requestVersion = exchange.getAttribute(VERSION_ATTRIBUTE_NAME);
115-
if (requestVersion == null) {
116-
String value = this.versionStrategy.resolveVersion(exchange);
117-
requestVersion = (value != null ? parseVersion(value) : this.versionStrategy.getDefaultVersion());
118-
this.versionStrategy.validateVersion(requestVersion, exchange);
119-
requestVersion = (requestVersion != null ? requestVersion : NO_VERSION_ATTRIBUTE);
120-
exchange.getAttributes().put(VERSION_ATTRIBUTE_NAME, requestVersion);
121-
}
102+
Comparable<?> requestVersion = exchange.getAttribute(HandlerMapping.API_VERSION_ATTRIBUTE);
122103

123-
if (this.version == null || requestVersion == NO_VERSION_ATTRIBUTE) {
104+
if (this.version == null || requestVersion == null) {
124105
return this;
125106
}
126107

127-
exchange.getAttributes().put(HandlerMapping.API_VERSION_ATTRIBUTE, requestVersion);
128-
129-
// At this stage, match all versions as baseline versions.
130-
// Strict matching for fixed versions is enforced at the end in handleMatch.
108+
// Always use a baseline match here in order to select the highest version (baseline or fixed)
109+
// The fixed version match is enforced at the end in handleMatch()
131110

132111
int result = compareVersions(this.version, requestVersion);
133112
return (result <= 0 ? this : null);
134113
}
135114

136-
private Comparable<?> parseVersion(String value) {
137-
try {
138-
return this.versionStrategy.parseVersion(value);
139-
}
140-
catch (Exception ex) {
141-
throw new InvalidApiVersionException(value, null, ex);
142-
}
143-
}
144-
145115
@SuppressWarnings("unchecked")
146116
private <V extends Comparable<V>> int compareVersions(Object v1, Object v2) {
147117
return ((V) v1).compareTo((V) v2);
@@ -178,39 +148,12 @@ else if (this.version != null && otherVersion != null) {
178148
*/
179149
public void handleMatch(ServerWebExchange exchange) {
180150
if (this.version != null && !this.baselineVersion) {
181-
Comparable<?> version = exchange.getAttribute(VERSION_ATTRIBUTE_NAME);
151+
Comparable<?> version = exchange.getAttribute(HandlerMapping.API_VERSION_ATTRIBUTE);
182152
Assert.state(version != null, "No API version attribute");
183153
if (!this.version.equals(version)) {
184154
throw new NotAcceptableApiVersionException(version.toString());
185155
}
186156
}
187157
}
188158

189-
190-
private static final class NoOpApiVersionStrategy implements ApiVersionStrategy {
191-
192-
@Override
193-
public @Nullable String resolveVersion(ServerWebExchange exchange) {
194-
return null;
195-
}
196-
197-
@Override
198-
public String parseVersion(String version) {
199-
return version;
200-
}
201-
202-
@Override
203-
public void validateVersion(@Nullable Comparable<?> requestVersion, ServerWebExchange exchange) {
204-
}
205-
206-
@Override
207-
public @Nullable Comparable<?> getDefaultVersion() {
208-
return null;
209-
}
210-
211-
@Override
212-
public void handleDeprecations(Comparable<?> version, ServerWebExchange exchange) {
213-
}
214-
}
215-
216159
}

‎spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMapping.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.stream.Stream;
2929

3030
import org.jspecify.annotations.Nullable;
31+
import reactor.core.publisher.Mono;
3132

3233
import org.springframework.context.EmbeddedValueResolverAware;
3334
import org.springframework.core.annotation.AnnotatedElementUtils;
@@ -41,6 +42,7 @@
4142
import org.springframework.util.CollectionUtils;
4243
import org.springframework.util.StringUtils;
4344
import org.springframework.util.StringValueResolver;
45+
import org.springframework.web.accept.InvalidApiVersionException;
4446
import org.springframework.web.bind.annotation.CrossOrigin;
4547
import org.springframework.web.bind.annotation.RequestBody;
4648
import org.springframework.web.bind.annotation.RequestMapping;
@@ -173,6 +175,37 @@ protected boolean isHandler(Class<?> beanType) {
173175
return AnnotatedElementUtils.hasAnnotation(beanType, Controller.class);
174176
}
175177

178+
@Override
179+
public Mono<HandlerMethod> getHandlerInternal(ServerWebExchange exchange) {
180+
if (this.apiVersionStrategy != null) {
181+
Comparable<?> requestVersion = exchange.getAttribute(API_VERSION_ATTRIBUTE);
182+
if (requestVersion == null) {
183+
requestVersion = getApiVersion(exchange, this.apiVersionStrategy);
184+
if (requestVersion != null) {
185+
exchange.getAttributes().put(API_VERSION_ATTRIBUTE, requestVersion);
186+
}
187+
}
188+
}
189+
return super.getHandlerInternal(exchange);
190+
}
191+
192+
private static @Nullable Comparable<?> getApiVersion(
193+
ServerWebExchange exchange, ApiVersionStrategy versionStrategy) {
194+
195+
String value = versionStrategy.resolveVersion(exchange);
196+
if (value == null) {
197+
return versionStrategy.getDefaultVersion();
198+
}
199+
try {
200+
Comparable<?> version = versionStrategy.parseVersion(value);
201+
versionStrategy.validateVersion(version, exchange);
202+
return version;
203+
}
204+
catch (Exception ex) {
205+
throw new InvalidApiVersionException(value, null, ex);
206+
}
207+
}
208+
176209
/**
177210
* Uses type-level and method-level {@link RequestMapping @RequestMapping}
178211
* and {@link HttpExchange @HttpExchange} annotations to create the

‎spring-webflux/src/test/java/org/springframework/web/reactive/accept/DefaultApiVersionStrategiesTests.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.junit.jupiter.api.Test;
2323

2424
import org.springframework.web.accept.InvalidApiVersionException;
25+
import org.springframework.web.accept.MissingApiVersionException;
2526
import org.springframework.web.accept.SemanticApiVersionParser;
2627
import org.springframework.web.server.ServerWebExchange;
2728
import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest;
@@ -59,7 +60,8 @@ void validateSupportedVersion() {
5960
@Test
6061
void validateUnsupportedVersion() {
6162
assertThatThrownBy(() -> validateVersion("1.2", apiVersionStrategy()))
62-
.isInstanceOf(InvalidApiVersionException.class);
63+
.isInstanceOf(InvalidApiVersionException.class)
64+
.hasMessage("400 BAD_REQUEST \"Invalid API version: '1.2.0'.\"");
6365
}
6466

6567
@Test
@@ -80,6 +82,14 @@ void validateWhenDetectSupportedVersionsIsOff() {
8082
.isInstanceOf(InvalidApiVersionException.class);
8183
}
8284

85+
@Test
86+
void missingRequiredVersion() {
87+
DefaultApiVersionStrategy strategy = apiVersionStrategy();
88+
assertThatThrownBy(() -> strategy.validateVersion(null, exchange))
89+
.isInstanceOf(MissingApiVersionException.class)
90+
.hasMessage("400 BAD_REQUEST \"API version is required.\"");
91+
}
92+
8393
private static DefaultApiVersionStrategy apiVersionStrategy() {
8494
return apiVersionStrategy(null, false);
8595
}

‎spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/VersionRequestConditionTests.java

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,9 @@
2323
import org.junit.jupiter.api.BeforeEach;
2424
import org.junit.jupiter.api.Test;
2525

26-
import org.springframework.web.accept.InvalidApiVersionException;
27-
import org.springframework.web.accept.MissingApiVersionException;
2826
import org.springframework.web.accept.NotAcceptableApiVersionException;
2927
import org.springframework.web.accept.SemanticApiVersionParser;
28+
import org.springframework.web.reactive.HandlerMapping;
3029
import org.springframework.web.reactive.accept.DefaultApiVersionStrategy;
3130
import org.springframework.web.server.ServerWebExchange;
3231
import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest;
@@ -99,12 +98,6 @@ void notVersionedMatch() {
9998

10099
testMatch("v1.1", condition, true, false);
101100
testMatch("v1.3", condition, true, false);
102-
103-
assertThatThrownBy(() -> condition.getMatchingCondition(exchangeWithVersion("1.2")))
104-
.isInstanceOf(InvalidApiVersionException.class);
105-
106-
assertThatThrownBy(() -> condition.getMatchingCondition(MockServerWebExchange.from(MockServerHttpRequest.get("/"))))
107-
.isInstanceOf(MissingApiVersionException.class);
108101
}
109102

110103
private void testMatch(
@@ -128,12 +121,6 @@ private void testMatch(
128121
condition.handleMatch(exchange);
129122
}
130123

131-
@Test
132-
void missingRequiredVersion() {
133-
assertThatThrownBy(() -> condition("1.2").getMatchingCondition(exchange()))
134-
.hasMessage("400 BAD_REQUEST \"API version is required.\"");
135-
}
136-
137124
@Test
138125
void defaultVersion() {
139126
String version = "1.2";
@@ -144,12 +131,6 @@ void defaultVersion() {
144131
assertThat(match).isSameAs(condition);
145132
}
146133

147-
@Test
148-
void unsupportedVersion() {
149-
assertThatThrownBy(() -> condition("1.2").getMatchingCondition(exchangeWithVersion("1.3")))
150-
.hasMessage("400 BAD_REQUEST \"Invalid API version: '1.3.0'.\"");
151-
}
152-
153134
@Test
154135
void compare() {
155136
testCompare("1.1", "1", "1.1");
@@ -181,8 +162,10 @@ private static MockServerWebExchange exchange() {
181162
}
182163

183164
private ServerWebExchange exchangeWithVersion(String v) {
184-
return MockServerWebExchange.from(
185-
MockServerHttpRequest.get("/path").queryParam("api-version", v));
165+
Comparable<?> version = this.strategy.parseVersion(v);
166+
MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/path"));
167+
exchange.getAttributes().put(HandlerMapping.API_VERSION_ATTRIBUTE, version);
168+
return exchange;
186169
}
187170

188171
}

0 commit comments

Comments
 (0)