diff --git a/spring-web/src/test/java/org/springframework/web/accept/DefaultApiVersionStrategiesTests.java b/spring-web/src/test/java/org/springframework/web/accept/DefaultApiVersionStrategiesTests.java index 812831953f5..d24f139b695 100644 --- a/spring-web/src/test/java/org/springframework/web/accept/DefaultApiVersionStrategiesTests.java +++ b/spring-web/src/test/java/org/springframework/web/accept/DefaultApiVersionStrategiesTests.java @@ -36,7 +36,7 @@ public class DefaultApiVersionStrategiesTests { @Test - void defaultVersion() { + void defaultVersionIsParsed() { SemanticApiVersionParser.Version version = this.parser.parseVersion("1.2.3"); ApiVersionStrategy strategy = initVersionStrategy(version.toString()); @@ -44,21 +44,29 @@ public class DefaultApiVersionStrategiesTests { } @Test - void supportedVersions() { - SemanticApiVersionParser.Version v1 = this.parser.parseVersion("1"); - SemanticApiVersionParser.Version v2 = this.parser.parseVersion("2"); - SemanticApiVersionParser.Version v9 = this.parser.parseVersion("9"); + void validateSupportedVersion() { + SemanticApiVersionParser.Version v12 = this.parser.parseVersion("1.2"); DefaultApiVersionStrategy strategy = initVersionStrategy(null); - strategy.addSupportedVersion(v1.toString()); - strategy.addSupportedVersion(v2.toString()); + strategy.addSupportedVersion(v12.toString()); MockHttpServletRequest request = new MockHttpServletRequest(); - strategy.validateVersion(v1, request); - strategy.validateVersion(v2, request); + strategy.validateVersion(v12, request); + } + + @Test + void validateUnsupportedVersion() { + assertThatThrownBy(() -> initVersionStrategy(null).validateVersion("1.2", new MockHttpServletRequest())) + .isInstanceOf(InvalidApiVersionException.class) + .hasMessage("400 BAD_REQUEST \"Invalid API version: '1.2'.\""); + } - assertThatThrownBy(() -> strategy.validateVersion(v9, request)) - .isInstanceOf(InvalidApiVersionException.class); + @Test + void missingRequiredVersion() { + DefaultApiVersionStrategy strategy = initVersionStrategy(null); + assertThatThrownBy(() -> strategy.validateVersion(null, new MockHttpServletRequest())) + .isInstanceOf(MissingApiVersionException.class) + .hasMessage("400 BAD_REQUEST \"API version is required.\""); } private static DefaultApiVersionStrategy initVersionStrategy(@Nullable String defaultValue) { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/VersionRequestCondition.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/VersionRequestCondition.java index 0a24c8475a9..5d701f28f30 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/VersionRequestCondition.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/VersionRequestCondition.java @@ -24,7 +24,6 @@ import org.jspecify.annotations.Nullable; import org.springframework.util.Assert; import org.springframework.util.StringUtils; -import org.springframework.web.accept.InvalidApiVersionException; import org.springframework.web.accept.NotAcceptableApiVersionException; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.reactive.HandlerMapping; @@ -42,21 +41,12 @@ import org.springframework.web.server.ServerWebExchange; */ public final class VersionRequestCondition extends AbstractRequestCondition { - private static final String VERSION_ATTRIBUTE_NAME = VersionRequestCondition.class.getName() + ".VERSION"; - - private static final String NO_VERSION_ATTRIBUTE = "NO_VERSION"; - - private static final ApiVersionStrategy NO_OP_VERSION_STRATEGY = new NoOpApiVersionStrategy(); - - private final @Nullable String versionValue; private final @Nullable Object version; private final boolean baselineVersion; - private final ApiVersionStrategy versionStrategy; - private final Set content; @@ -67,14 +57,12 @@ public final class VersionRequestCondition extends AbstractRequestCondition requestVersion = exchange.getAttribute(VERSION_ATTRIBUTE_NAME); - if (requestVersion == null) { - String value = this.versionStrategy.resolveVersion(exchange); - requestVersion = (value != null ? parseVersion(value) : this.versionStrategy.getDefaultVersion()); - this.versionStrategy.validateVersion(requestVersion, exchange); - requestVersion = (requestVersion != null ? requestVersion : NO_VERSION_ATTRIBUTE); - exchange.getAttributes().put(VERSION_ATTRIBUTE_NAME, requestVersion); - } + Comparable requestVersion = exchange.getAttribute(HandlerMapping.API_VERSION_ATTRIBUTE); - if (this.version == null || requestVersion == NO_VERSION_ATTRIBUTE) { + if (this.version == null || requestVersion == null) { return this; } - exchange.getAttributes().put(HandlerMapping.API_VERSION_ATTRIBUTE, requestVersion); - - // At this stage, match all versions as baseline versions. - // Strict matching for fixed versions is enforced at the end in handleMatch. + // Always use a baseline match here in order to select the highest version (baseline or fixed) + // The fixed version match is enforced at the end in handleMatch() int result = compareVersions(this.version, requestVersion); return (result <= 0 ? this : null); } - private Comparable parseVersion(String value) { - try { - return this.versionStrategy.parseVersion(value); - } - catch (Exception ex) { - throw new InvalidApiVersionException(value, null, ex); - } - } - @SuppressWarnings("unchecked") private > int compareVersions(Object v1, Object v2) { return ((V) v1).compareTo((V) v2); @@ -178,7 +148,7 @@ public final class VersionRequestCondition extends AbstractRequestCondition version = exchange.getAttribute(VERSION_ATTRIBUTE_NAME); + Comparable version = exchange.getAttribute(HandlerMapping.API_VERSION_ATTRIBUTE); Assert.state(version != null, "No API version attribute"); if (!this.version.equals(version)) { throw new NotAcceptableApiVersionException(version.toString()); @@ -186,31 +156,4 @@ public final class VersionRequestCondition extends AbstractRequestCondition requestVersion, ServerWebExchange exchange) { - } - - @Override - public @Nullable Comparable getDefaultVersion() { - return null; - } - - @Override - public void handleDeprecations(Comparable version, ServerWebExchange exchange) { - } - } - } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMapping.java index 117c6661145..56f2f98c5a9 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMapping.java @@ -28,6 +28,7 @@ import java.util.function.Predicate; import java.util.stream.Stream; import org.jspecify.annotations.Nullable; +import reactor.core.publisher.Mono; import org.springframework.context.EmbeddedValueResolverAware; import org.springframework.core.annotation.AnnotatedElementUtils; @@ -41,6 +42,7 @@ import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.util.StringUtils; import org.springframework.util.StringValueResolver; +import org.springframework.web.accept.InvalidApiVersionException; import org.springframework.web.bind.annotation.CrossOrigin; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; @@ -173,6 +175,37 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi return AnnotatedElementUtils.hasAnnotation(beanType, Controller.class); } + @Override + public Mono getHandlerInternal(ServerWebExchange exchange) { + if (this.apiVersionStrategy != null) { + Comparable requestVersion = exchange.getAttribute(API_VERSION_ATTRIBUTE); + if (requestVersion == null) { + requestVersion = getApiVersion(exchange, this.apiVersionStrategy); + if (requestVersion != null) { + exchange.getAttributes().put(API_VERSION_ATTRIBUTE, requestVersion); + } + } + } + return super.getHandlerInternal(exchange); + } + + private static @Nullable Comparable getApiVersion( + ServerWebExchange exchange, ApiVersionStrategy versionStrategy) { + + String value = versionStrategy.resolveVersion(exchange); + if (value == null) { + return versionStrategy.getDefaultVersion(); + } + try { + Comparable version = versionStrategy.parseVersion(value); + versionStrategy.validateVersion(version, exchange); + return version; + } + catch (Exception ex) { + throw new InvalidApiVersionException(value, null, ex); + } + } + /** * Uses type-level and method-level {@link RequestMapping @RequestMapping} * and {@link HttpExchange @HttpExchange} annotations to create the diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/accept/DefaultApiVersionStrategiesTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/accept/DefaultApiVersionStrategiesTests.java index 0da6278b761..82d62ef6c7a 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/accept/DefaultApiVersionStrategiesTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/accept/DefaultApiVersionStrategiesTests.java @@ -22,6 +22,7 @@ import org.jspecify.annotations.Nullable; import org.junit.jupiter.api.Test; import org.springframework.web.accept.InvalidApiVersionException; +import org.springframework.web.accept.MissingApiVersionException; import org.springframework.web.accept.SemanticApiVersionParser; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest; @@ -59,7 +60,8 @@ public class DefaultApiVersionStrategiesTests { @Test void validateUnsupportedVersion() { assertThatThrownBy(() -> validateVersion("1.2", apiVersionStrategy())) - .isInstanceOf(InvalidApiVersionException.class); + .isInstanceOf(InvalidApiVersionException.class) + .hasMessage("400 BAD_REQUEST \"Invalid API version: '1.2.0'.\""); } @Test @@ -80,6 +82,14 @@ public class DefaultApiVersionStrategiesTests { .isInstanceOf(InvalidApiVersionException.class); } + @Test + void missingRequiredVersion() { + DefaultApiVersionStrategy strategy = apiVersionStrategy(); + assertThatThrownBy(() -> strategy.validateVersion(null, exchange)) + .isInstanceOf(MissingApiVersionException.class) + .hasMessage("400 BAD_REQUEST \"API version is required.\""); + } + private static DefaultApiVersionStrategy apiVersionStrategy() { return apiVersionStrategy(null, false); } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/VersionRequestConditionTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/VersionRequestConditionTests.java index 933c91635a4..ef5fa635480 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/VersionRequestConditionTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/VersionRequestConditionTests.java @@ -23,10 +23,9 @@ import org.jspecify.annotations.Nullable; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.springframework.web.accept.InvalidApiVersionException; -import org.springframework.web.accept.MissingApiVersionException; import org.springframework.web.accept.NotAcceptableApiVersionException; import org.springframework.web.accept.SemanticApiVersionParser; +import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.reactive.accept.DefaultApiVersionStrategy; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest; @@ -99,12 +98,6 @@ public class VersionRequestConditionTests { testMatch("v1.1", condition, true, false); testMatch("v1.3", condition, true, false); - - assertThatThrownBy(() -> condition.getMatchingCondition(exchangeWithVersion("1.2"))) - .isInstanceOf(InvalidApiVersionException.class); - - assertThatThrownBy(() -> condition.getMatchingCondition(MockServerWebExchange.from(MockServerHttpRequest.get("/")))) - .isInstanceOf(MissingApiVersionException.class); } private void testMatch( @@ -128,12 +121,6 @@ public class VersionRequestConditionTests { condition.handleMatch(exchange); } - @Test - void missingRequiredVersion() { - assertThatThrownBy(() -> condition("1.2").getMatchingCondition(exchange())) - .hasMessage("400 BAD_REQUEST \"API version is required.\""); - } - @Test void defaultVersion() { String version = "1.2"; @@ -144,12 +131,6 @@ public class VersionRequestConditionTests { assertThat(match).isSameAs(condition); } - @Test - void unsupportedVersion() { - assertThatThrownBy(() -> condition("1.2").getMatchingCondition(exchangeWithVersion("1.3"))) - .hasMessage("400 BAD_REQUEST \"Invalid API version: '1.3.0'.\""); - } - @Test void compare() { testCompare("1.1", "1", "1.1"); @@ -181,8 +162,10 @@ public class VersionRequestConditionTests { } private ServerWebExchange exchangeWithVersion(String v) { - return MockServerWebExchange.from( - MockServerHttpRequest.get("/path").queryParam("api-version", v)); + Comparable version = this.strategy.parseVersion(v); + MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/path")); + exchange.getAttributes().put(HandlerMapping.API_VERSION_ATTRIBUTE, version); + return exchange; } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/VersionRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/VersionRequestCondition.java index 0dcd117e7e1..fd824ee8305 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/VersionRequestCondition.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/VersionRequestCondition.java @@ -21,13 +21,11 @@ import java.util.Collections; import java.util.Set; import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; import org.jspecify.annotations.Nullable; import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.web.accept.ApiVersionStrategy; -import org.springframework.web.accept.InvalidApiVersionException; import org.springframework.web.accept.NotAcceptableApiVersionException; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.servlet.HandlerMapping; @@ -42,21 +40,12 @@ import org.springframework.web.servlet.HandlerMapping; */ public final class VersionRequestCondition extends AbstractRequestCondition { - private static final String VERSION_ATTRIBUTE_NAME = VersionRequestCondition.class.getName() + ".VERSION"; - - private static final String NO_VERSION_ATTRIBUTE = "NO_VERSION"; - - private static final ApiVersionStrategy NO_OP_VERSION_STRATEGY = new NoOpApiVersionStrategy(); - - private final @Nullable String versionValue; private final @Nullable Comparable version; private final boolean baselineVersion; - private final ApiVersionStrategy versionStrategy; - private final Set content; @@ -67,14 +56,12 @@ public final class VersionRequestCondition extends AbstractRequestCondition requestVersion = (Comparable) request.getAttribute(VERSION_ATTRIBUTE_NAME); - if (requestVersion == null) { - String value = this.versionStrategy.resolveVersion(request); - requestVersion = (value != null ? parseVersion(value) : this.versionStrategy.getDefaultVersion()); - this.versionStrategy.validateVersion(requestVersion, request); - requestVersion = (requestVersion != null ? requestVersion : NO_VERSION_ATTRIBUTE); - request.setAttribute(VERSION_ATTRIBUTE_NAME, requestVersion); - } + Comparable requestVersion = (Comparable) request.getAttribute(HandlerMapping.API_VERSION_ATTRIBUTE); - if (this.version == null || requestVersion == NO_VERSION_ATTRIBUTE) { + if (this.version == null || requestVersion == null) { return this; } - request.setAttribute(HandlerMapping.API_VERSION_ATTRIBUTE, requestVersion); - - // At this stage, match all versions as baseline versions. - // Strict matching for fixed versions is enforced at the end in handleMatch. + // Always use a baseline match here in order to select the highest version (baseline or fixed) + // The fixed version match is enforced at the end in handleMatch() int result = compareVersions(this.version, requestVersion); return (result <= 0 ? this : null); } - private Comparable parseVersion(String value) { - try { - return this.versionStrategy.parseVersion(value); - } - catch (Exception ex) { - throw new InvalidApiVersionException(value, null, ex); - } - } - @SuppressWarnings("unchecked") private > int compareVersions(Object v1, Object v2) { return ((V) v1).compareTo((V) v2); @@ -178,7 +147,7 @@ public final class VersionRequestCondition extends AbstractRequestCondition version = (Comparable) request.getAttribute(VERSION_ATTRIBUTE_NAME); + Comparable version = (Comparable) request.getAttribute(HandlerMapping.API_VERSION_ATTRIBUTE); Assert.state(version != null, "No API version attribute"); if (!this.version.equals(version)) { throw new NotAcceptableApiVersionException(version.toString()); @@ -186,31 +155,4 @@ public final class VersionRequestCondition extends AbstractRequestCondition requestVersion, HttpServletRequest request) { - } - - @Override - public @Nullable Comparable getDefaultVersion() { - return null; - } - - @Override - public void handleDeprecations(Comparable version, HttpServletRequest request, HttpServletResponse response) { - } - } - } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java index a51170d7737..ffbff6537b1 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java @@ -46,6 +46,7 @@ import org.springframework.util.StringValueResolver; import org.springframework.web.accept.ApiVersionStrategy; import org.springframework.web.accept.ContentNegotiationManager; import org.springframework.web.accept.DefaultApiVersionStrategy; +import org.springframework.web.accept.InvalidApiVersionException; import org.springframework.web.bind.annotation.CrossOrigin; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; @@ -200,6 +201,38 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi return AnnotatedElementUtils.hasAnnotation(beanType, Controller.class); } + + @Override + protected @Nullable HandlerMethod getHandlerInternal(HttpServletRequest request) throws Exception { + if (this.apiVersionStrategy != null) { + Comparable requestVersion = (Comparable) request.getAttribute(API_VERSION_ATTRIBUTE); + if (requestVersion == null) { + requestVersion = getApiVersion(request, this.apiVersionStrategy); + if (requestVersion != null) { + request.setAttribute(API_VERSION_ATTRIBUTE, requestVersion); + } + } + } + return super.getHandlerInternal(request); + } + + private static @Nullable Comparable getApiVersion( + HttpServletRequest request, ApiVersionStrategy versionStrategy) { + + String value = versionStrategy.resolveVersion(request); + if (value == null) { + return versionStrategy.getDefaultVersion(); + } + try { + Comparable version = versionStrategy.parseVersion(value); + versionStrategy.validateVersion(version, request); + return version; + } + catch (Exception ex) { + throw new InvalidApiVersionException(value, null, ex); + } + } + /** * Uses type-level and method-level {@link RequestMapping @RequestMapping} * and {@link HttpExchange @HttpExchange} annotations to create the diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/VersionRequestConditionTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/VersionRequestConditionTests.java index 394ad04b9d3..5e14295c48f 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/VersionRequestConditionTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/VersionRequestConditionTests.java @@ -24,10 +24,9 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.web.accept.DefaultApiVersionStrategy; -import org.springframework.web.accept.InvalidApiVersionException; -import org.springframework.web.accept.MissingApiVersionException; import org.springframework.web.accept.NotAcceptableApiVersionException; import org.springframework.web.accept.SemanticApiVersionParser; +import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.testfixture.servlet.MockHttpServletRequest; import static org.assertj.core.api.Assertions.assertThat; @@ -97,12 +96,6 @@ public class VersionRequestConditionTests { testMatch("v1.1", condition, true, false); testMatch("v1.3", condition, true, false); - - assertThatThrownBy(() -> condition.getMatchingCondition(requestWithVersion("1.2"))) - .isInstanceOf(InvalidApiVersionException.class); - - assertThatThrownBy(() -> condition.getMatchingCondition(new MockHttpServletRequest("GET", "/path"))) - .isInstanceOf(MissingApiVersionException.class); } private void testMatch( @@ -126,12 +119,6 @@ public class VersionRequestConditionTests { condition.handleMatch(request); } - @Test - void missingRequiredVersion() { - assertThatThrownBy(() -> condition("1.2").getMatchingCondition(new MockHttpServletRequest("GET", "/path"))) - .hasMessage("400 BAD_REQUEST \"API version is required.\""); - } - @Test void defaultVersion() { String version = "1.2"; @@ -142,12 +129,6 @@ public class VersionRequestConditionTests { assertThat(match).isSameAs(condition); } - @Test - void unsupportedVersion() { - assertThatThrownBy(() -> condition("1.2").getMatchingCondition(requestWithVersion("1.3"))) - .hasMessage("400 BAD_REQUEST \"Invalid API version: '1.3.0'.\""); - } - @Test void compare() { testCompare("1.1", "1", "1.1"); @@ -176,7 +157,7 @@ public class VersionRequestConditionTests { private MockHttpServletRequest requestWithVersion(String v) { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/path"); - request.addParameter("api-version", v); + request.setAttribute(HandlerMapping.API_VERSION_ATTRIBUTE, this.strategy.parseVersion(v)); return request; }