From a9027ddf76554a8290ce2c95c4d3b9f9d784161b Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Wed, 7 Jan 2026 15:45:46 +0000 Subject: [PATCH] Refine handling of API version errors Among HandlerMapping's some may not expect an API version. This is why those that do must be careful not to raise API validation errors if they don't match the request. Closes gh-36059 --- .../web/accept/ApiVersionHolder.java | 84 +++++++++++++++++++ .../web/reactive/HandlerMapping.java | 6 +- .../function/server/RequestPredicates.java | 10 ++- .../handler/AbstractHandlerMapping.java | 33 +++++--- .../condition/VersionRequestCondition.java | 14 ++-- .../ApiVersionMethodArgumentResolver.java | 4 +- .../server/RequestPredicatesTests.java | 3 +- .../condition/RequestMappingInfoTests.java | 26 +++--- .../VersionRequestConditionTests.java | 7 +- ...ApiVersionMethodArgumentResolverTests.java | 10 ++- .../RequestMappingHandlerMappingTests.java | 56 +++++++++++++ .../web/servlet/HandlerMapping.java | 7 +- .../servlet/function/RequestPredicates.java | 10 ++- .../handler/AbstractHandlerMapping.java | 36 +++++--- .../condition/VersionRequestCondition.java | 14 ++-- .../ApiVersionMethodArgumentResolver.java | 4 +- .../function/RequestPredicatesTests.java | 4 +- .../VersionRequestConditionTests.java | 13 +-- ...ApiVersionMethodArgumentResolverTests.java | 7 +- ...MappingHandlerAdapterIntegrationTests.java | 3 +- .../RequestMappingHandlerMappingTests.java | 53 ++++++++++++ 21 files changed, 337 insertions(+), 67 deletions(-) create mode 100644 spring-web/src/main/java/org/springframework/web/accept/ApiVersionHolder.java diff --git a/spring-web/src/main/java/org/springframework/web/accept/ApiVersionHolder.java b/spring-web/src/main/java/org/springframework/web/accept/ApiVersionHolder.java new file mode 100644 index 00000000000..a0140a6ad23 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/web/accept/ApiVersionHolder.java @@ -0,0 +1,84 @@ +/* + * Copyright 2002-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.web.accept; + +import org.jspecify.annotations.Nullable; + +import org.springframework.util.Assert; + +/** + * Simple container of the API version for a request (possibly {@code null}), + * or an exception that resulted from trying to resolve, parse, and validate + * the version. + * + *

While an API version needs to be initialized early, given that each + * {@code HandlerMapping} may or may not expect an API version, it is important + * to defer raising API version errors until it is known if the + * {@code HandlerMapping} will handle the request. + * + * @author Rossen Stoyanchev + * @since 7.0 + */ +public final class ApiVersionHolder { + + /** Static instance for a request without an API version. */ + public static final ApiVersionHolder EMPTY = ApiVersionHolder.fromVersion(null); + + + private final @Nullable Comparable version; + + private final @Nullable RuntimeException exception; + + + private ApiVersionHolder(@Nullable Comparable version, @Nullable RuntimeException ex) { + this.version = version; + this.exception = ex; + } + + + public boolean hasVersion() { + return (this.version != null); + } + + public boolean hasError() { + return (this.exception != null); + } + + public Comparable getVersion() { + Assert.state(this.version != null, "No version"); + return this.version; + } + + public @Nullable Comparable getVersionIfPresent() { + return this.version; + } + + public RuntimeException getError() { + Assert.state(this.exception != null, "No error"); + return this.exception; + } + + + public static ApiVersionHolder fromVersion(@Nullable Comparable version) { + return new ApiVersionHolder(version, null); + } + + public static ApiVersionHolder fromError(@Nullable RuntimeException ex) { + return new ApiVersionHolder(null, ex); + } + +} diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/HandlerMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/HandlerMapping.java index bb249c3c4a7..184316949fe 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/HandlerMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/HandlerMapping.java @@ -18,6 +18,7 @@ package org.springframework.web.reactive; import reactor.core.publisher.Mono; +import org.springframework.web.accept.ApiVersionHolder; import org.springframework.web.server.ServerWebExchange; /** @@ -84,8 +85,9 @@ public interface HandlerMapping { String PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE = HandlerMapping.class.getName() + ".producibleMediaTypes"; /** - * Name of the {@link ServerWebExchange#getAttributes() attribute} containing - * the resolved and parsed API version. + * Name of the {@link ServerWebExchange#getAttributes() attribute} that + * contains an {@link ApiVersionHolder} with the result of obtaining and + * parsing the API version of the request. * @since 7.0 */ String API_VERSION_ATTRIBUTE = HandlerMapping.class.getName() + ".apiVersion"; diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java index 2fd02b0a35a..3355ed81fe0 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java @@ -52,6 +52,7 @@ import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.util.MimeTypeUtils; import org.springframework.util.MultiValueMap; +import org.springframework.web.accept.ApiVersionHolder; import org.springframework.web.bind.WebDataBinder; import org.springframework.web.cors.reactive.CorsUtils; import org.springframework.web.reactive.HandlerMapping; @@ -892,14 +893,17 @@ public abstract class RequestPredicates { this.parsedVersion = strategy.parseVersion(this.version); } - Comparable requestVersion = - (Comparable) request.attribute(HandlerMapping.API_VERSION_ATTRIBUTE).orElse(null); + ApiVersionHolder requestVersionHolder = + (ApiVersionHolder) request.attribute(HandlerMapping.API_VERSION_ATTRIBUTE) + .orElseThrow(() -> new IllegalStateException("Expect API version attribute")); - if (requestVersion == null) { + if (!requestVersionHolder.hasVersion()) { traceMatch("Version", this.version, null, true); return true; } + Comparable requestVersion = requestVersionHolder.getVersion(); + int result = compareVersions(this.parsedVersion, requestVersion); boolean match = (this.baselineVersion ? result <= 0 : result == 0); traceMatch("Version", this.version, requestVersion, match); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractHandlerMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractHandlerMapping.java index b20577bea99..e54dce96fe3 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractHandlerMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractHandlerMapping.java @@ -28,6 +28,7 @@ import org.springframework.core.Ordered; import org.springframework.core.log.LogDelegateFactory; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.util.Assert; +import org.springframework.web.accept.ApiVersionHolder; import org.springframework.web.cors.CorsConfiguration; import org.springframework.web.cors.reactive.CorsConfigurationSource; import org.springframework.web.cors.reactive.CorsProcessor; @@ -184,11 +185,14 @@ public abstract class AbstractHandlerMapping extends ApplicationObjectSupport @Override public Mono getHandler(ServerWebExchange exchange) { - initApiVersion(exchange); + ApiVersionHolder versionHolder = initApiVersion(exchange); return getHandlerInternal(exchange).map(handler -> { if (logger.isDebugEnabled()) { logger.debug(exchange.getLogPrefix() + "Mapped to " + handler); } + if (versionHolder.hasError()) { + throw versionHolder.getError(); + } ServerHttpRequest request = exchange.getRequest(); if (hasCorsConfigurationSource(handler) || CorsUtils.isPreFlightRequest(request)) { CorsConfiguration config = (this.corsConfigurationSource != null ? @@ -204,8 +208,8 @@ public abstract class AbstractHandlerMapping extends ApplicationObjectSupport } } if (getApiVersionStrategy() != null) { - Comparable version = exchange.getAttribute(API_VERSION_ATTRIBUTE); - if (version != null) { + if (versionHolder.hasVersion()) { + Comparable version = versionHolder.getVersion(); getApiVersionStrategy().handleDeprecations(version, handler, exchange); } } @@ -213,16 +217,25 @@ public abstract class AbstractHandlerMapping extends ApplicationObjectSupport }); } - private void initApiVersion(ServerWebExchange exchange) { - if (this.apiVersionStrategy != null) { - Comparable version = exchange.getAttribute(API_VERSION_ATTRIBUTE); - if (version == null) { - version = this.apiVersionStrategy.resolveParseAndValidateVersion(exchange); - if (version != null) { - exchange.getAttributes().put(API_VERSION_ATTRIBUTE, version); + private ApiVersionHolder initApiVersion(ServerWebExchange exchange) { + ApiVersionHolder versionHolder = exchange.getAttribute(API_VERSION_ATTRIBUTE); + if (versionHolder == null) { + if (this.apiVersionStrategy == null) { + versionHolder = ApiVersionHolder.EMPTY; + } + else { + Comparable version; + try { + version = this.apiVersionStrategy.resolveParseAndValidateVersion(exchange); + versionHolder = ApiVersionHolder.fromVersion(version); + } + catch (RuntimeException ex) { + versionHolder = ApiVersionHolder.fromError(ex); } } } + exchange.getAttributes().put(API_VERSION_ATTRIBUTE, versionHolder); + return versionHolder; } /** 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 1d0843f289a..e3dbb29237d 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,6 +24,7 @@ import org.jspecify.annotations.Nullable; import org.springframework.util.Assert; import org.springframework.util.StringUtils; +import org.springframework.web.accept.ApiVersionHolder; import org.springframework.web.accept.NotAcceptableApiVersionException; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.reactive.HandlerMapping; @@ -99,12 +100,14 @@ public final class VersionRequestCondition extends AbstractRequestCondition requestVersion = exchange.getAttribute(HandlerMapping.API_VERSION_ATTRIBUTE); + ApiVersionHolder versionHolder = exchange.getRequiredAttribute(HandlerMapping.API_VERSION_ATTRIBUTE); - if (this.version == null || requestVersion == null) { + if (this.version == null || !versionHolder.hasVersion()) { return this; } + Comparable requestVersion = versionHolder.getVersion(); + // 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() @@ -130,8 +133,8 @@ public final class VersionRequestCondition extends AbstractRequestCondition version = exchange.getAttribute(HandlerMapping.API_VERSION_ATTRIBUTE); - return (version == null ? -1 * result : result); + ApiVersionHolder holder = exchange.getRequiredAttribute(HandlerMapping.API_VERSION_ATTRIBUTE); + return (!holder.hasVersion() ? -1 * result : result); } } @@ -151,7 +154,8 @@ public final class VersionRequestCondition extends AbstractRequestCondition version = exchange.getAttribute(HandlerMapping.API_VERSION_ATTRIBUTE); + ApiVersionHolder holder = exchange.getRequiredAttribute(HandlerMapping.API_VERSION_ATTRIBUTE); + Comparable version = holder.getVersionIfPresent(); if (version != null && !this.version.equals(version)) { throw new NotAcceptableApiVersionException(version.toString()); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ApiVersionMethodArgumentResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ApiVersionMethodArgumentResolver.java index 80341cc83e9..911fff8ca6b 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ApiVersionMethodArgumentResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ApiVersionMethodArgumentResolver.java @@ -21,6 +21,7 @@ import java.util.Optional; import org.jspecify.annotations.Nullable; import org.springframework.core.MethodParameter; +import org.springframework.web.accept.ApiVersionHolder; import org.springframework.web.accept.MissingApiVersionException; import org.springframework.web.accept.SemanticApiVersionParser; import org.springframework.web.reactive.BindingContext; @@ -45,7 +46,8 @@ public class ApiVersionMethodArgumentResolver implements SyncHandlerMethodArgume public @Nullable Object resolveArgumentValue( MethodParameter parameter, BindingContext bindingContext, ServerWebExchange exchange) { - Object version = exchange.getAttribute(HandlerMapping.API_VERSION_ATTRIBUTE); + ApiVersionHolder versionHolder = exchange.getRequiredAttribute(HandlerMapping.API_VERSION_ATTRIBUTE); + Object version = versionHolder.getVersionIfPresent(); if (parameter.getParameterType() == Optional.class) { return Optional.ofNullable(version); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/RequestPredicatesTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/RequestPredicatesTests.java index ec779c637f9..7a2fbc496ad 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/RequestPredicatesTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/RequestPredicatesTests.java @@ -26,6 +26,7 @@ import org.junit.jupiter.api.Test; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; +import org.springframework.web.accept.ApiVersionHolder; import org.springframework.web.accept.SemanticApiVersionParser; import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.reactive.accept.ApiVersionStrategy; @@ -374,7 +375,7 @@ class RequestPredicatesTests { ApiVersionStrategy versionStrategy = apiVersionStrategy(); Comparable parsedVersion = versionStrategy.parseVersion(version); MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("https://localhost")); - exchange.getAttributes().put(HandlerMapping.API_VERSION_ATTRIBUTE, parsedVersion); + exchange.getAttributes().put(HandlerMapping.API_VERSION_ATTRIBUTE, ApiVersionHolder.fromVersion(parsedVersion)); return new DefaultServerRequest(exchange, Collections.emptyList(), versionStrategy); } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/RequestMappingInfoTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/RequestMappingInfoTests.java index 500fd6d36b7..540e3b4758e 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/RequestMappingInfoTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/RequestMappingInfoTests.java @@ -26,7 +26,9 @@ import org.junit.jupiter.api.Test; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; +import org.springframework.web.accept.ApiVersionHolder; import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.reactive.result.method.RequestMappingInfo; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest; @@ -99,7 +101,7 @@ class RequestMappingInfoTests { @Test void matchPatternsCondition() { - MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/foo")); + MockServerWebExchange exchange = initExchange(MockServerHttpRequest.get("/foo")); RequestMappingInfo info = paths("/foo*", "/bar").build(); RequestMappingInfo expected = paths("/foo*").build(); @@ -114,7 +116,7 @@ class RequestMappingInfoTests { @Test void matchParamsCondition() { - ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/foo?foo=bar")); + ServerWebExchange exchange = initExchange(MockServerHttpRequest.get("/foo?foo=bar")); RequestMappingInfo info = paths("/foo").params("foo=bar").build(); RequestMappingInfo match = info.getMatchingCondition(exchange); @@ -129,8 +131,7 @@ class RequestMappingInfoTests { @Test void matchHeadersCondition() { - MockServerHttpRequest request = MockServerHttpRequest.get("/foo").header("foo", "bar").build(); - ServerWebExchange exchange = MockServerWebExchange.from(request); + ServerWebExchange exchange = initExchange(MockServerHttpRequest.get("/foo").header("foo", "bar")); RequestMappingInfo info = paths("/foo").headers("foo=bar").build(); RequestMappingInfo match = info.getMatchingCondition(exchange); @@ -145,8 +146,8 @@ class RequestMappingInfoTests { @Test void matchConsumesCondition() { - MockServerHttpRequest request = MockServerHttpRequest.post("/foo").contentType(MediaType.TEXT_PLAIN).build(); - ServerWebExchange exchange = MockServerWebExchange.from(request); + ServerWebExchange exchange = initExchange( + MockServerHttpRequest.post("/foo").contentType(MediaType.TEXT_PLAIN)); RequestMappingInfo info = paths("/foo").consumes("text/plain").build(); RequestMappingInfo match = info.getMatchingCondition(exchange); @@ -161,8 +162,7 @@ class RequestMappingInfoTests { @Test void matchProducesCondition() { - MockServerHttpRequest request = MockServerHttpRequest.get("/foo").accept(MediaType.TEXT_PLAIN).build(); - ServerWebExchange exchange = MockServerWebExchange.from(request); + ServerWebExchange exchange = initExchange(MockServerHttpRequest.get("/foo").accept(MediaType.TEXT_PLAIN)); RequestMappingInfo info = paths("/foo").produces("text/plain").build(); RequestMappingInfo match = info.getMatchingCondition(exchange); @@ -177,7 +177,7 @@ class RequestMappingInfoTests { @Test void matchCustomCondition() { - ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/foo?foo=bar")); + ServerWebExchange exchange = initExchange(MockServerHttpRequest.get("/foo?foo=bar")); RequestMappingInfo info = paths("/foo").params("foo=bar").build(); RequestMappingInfo match = info.getMatchingCondition(exchange); @@ -198,7 +198,7 @@ class RequestMappingInfoTests { RequestMappingInfo oneMethod = paths().methods(RequestMethod.GET).build(); RequestMappingInfo oneMethodOneParam = paths().methods(RequestMethod.GET).params("foo").build(); - ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/foo")); + ServerWebExchange exchange = initExchange(MockServerHttpRequest.get("/foo")); Comparator comparator = (info, otherInfo) -> info.compareTo(otherInfo, exchange); List list = asList(none, oneMethod, oneMethodOneParam); @@ -327,4 +327,10 @@ class RequestMappingInfoTests { .containsOnly(MediaType.parseMediaType("application/hal+json")); } + private static MockServerWebExchange initExchange(MockServerHttpRequest.BaseBuilder requestBuilder) { + MockServerWebExchange exchange = MockServerWebExchange.from(requestBuilder.build()); + exchange.getAttributes().put(HandlerMapping.API_VERSION_ATTRIBUTE, ApiVersionHolder.EMPTY); + return exchange; + } + } 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 e658ec41da0..4fe3ba26221 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 @@ -24,6 +24,7 @@ import org.jspecify.annotations.Nullable; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.springframework.web.accept.ApiVersionHolder; import org.springframework.web.accept.NotAcceptableApiVersionException; import org.springframework.web.accept.SemanticApiVersionParser; import org.springframework.web.reactive.HandlerMapping; @@ -179,13 +180,15 @@ public class VersionRequestConditionTests { } private static MockServerWebExchange exchange() { - return MockServerWebExchange.from(MockServerHttpRequest.get("/path")); + MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/path")); + exchange.getAttributes().put(HandlerMapping.API_VERSION_ATTRIBUTE, ApiVersionHolder.EMPTY); + return exchange; } private ServerWebExchange exchangeWithVersion(String v) { Comparable version = this.strategy.parseVersion(v); MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/path")); - exchange.getAttributes().put(HandlerMapping.API_VERSION_ATTRIBUTE, version); + exchange.getAttributes().put(HandlerMapping.API_VERSION_ATTRIBUTE, ApiVersionHolder.fromVersion(version)); return exchange; } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ApiVersionMethodArgumentResolverTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ApiVersionMethodArgumentResolverTests.java index a1044162f5c..a62501b5d2d 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ApiVersionMethodArgumentResolverTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ApiVersionMethodArgumentResolverTests.java @@ -24,6 +24,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.core.MethodParameter; +import org.springframework.web.accept.ApiVersionHolder; import org.springframework.web.accept.SemanticApiVersionParser; import org.springframework.web.accept.SemanticApiVersionParser.Version; import org.springframework.web.reactive.BindingContext; @@ -43,7 +44,7 @@ class ApiVersionMethodArgumentResolverTests { private final ApiVersionMethodArgumentResolver resolver = new ApiVersionMethodArgumentResolver(); - private final MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")); + private MockServerWebExchange exchange; private MethodParameter param; private MethodParameter nullableParam; @@ -54,6 +55,9 @@ class ApiVersionMethodArgumentResolverTests { @BeforeEach void setUp() throws Exception { + this.exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")); + this.exchange.getAttributes().put(HandlerMapping.API_VERSION_ATTRIBUTE, ApiVersionHolder.EMPTY); + Method method = getClass().getDeclaredMethod( "handle", Version.class, Version.class, Optional.class, int.class); @@ -74,7 +78,7 @@ class ApiVersionMethodArgumentResolverTests { @Test void resolveArgument() throws Exception { Version version = new SemanticApiVersionParser().parseVersion("1.2"); - this.exchange.getAttributes().put(HandlerMapping.API_VERSION_ATTRIBUTE, version); + this.exchange.getAttributes().put(HandlerMapping.API_VERSION_ATTRIBUTE, ApiVersionHolder.fromVersion(version)); Object actual = this.resolver.resolveArgumentValue(this.param, new BindingContext(), exchange); @@ -92,7 +96,7 @@ class ApiVersionMethodArgumentResolverTests { @Test void resolveOptionalArgument() { Version version = new SemanticApiVersionParser().parseVersion("1.2"); - this.exchange.getAttributes().put(HandlerMapping.API_VERSION_ATTRIBUTE, version); + this.exchange.getAttributes().put(HandlerMapping.API_VERSION_ATTRIBUTE, ApiVersionHolder.fromVersion(version)); Object actual = this.resolver.resolveArgumentValue(this.optionalParam, new BindingContext(), exchange); assertThat(actual).asInstanceOf(OPTIONAL).hasValue(version); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java index 107aa33c700..ea5225e8b41 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java @@ -22,11 +22,13 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.lang.reflect.Method; import java.security.Principal; +import java.util.List; import java.util.Map; import java.util.Set; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import reactor.test.StepVerifier; import org.springframework.cglib.proxy.Enhancer; import org.springframework.cglib.proxy.NoOp; @@ -35,6 +37,9 @@ import org.springframework.http.MediaType; import org.springframework.stereotype.Controller; import org.springframework.util.ClassUtils; import org.springframework.util.ReflectionUtils; +import org.springframework.web.accept.ApiVersionHolder; +import org.springframework.web.accept.InvalidApiVersionException; +import org.springframework.web.accept.SemanticApiVersionParser; import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PatchMapping; @@ -44,7 +49,11 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.context.support.StaticWebApplicationContext; +import org.springframework.web.method.HandlerMethod; import org.springframework.web.method.HandlerTypePredicate; +import org.springframework.web.reactive.HandlerMapping; +import org.springframework.web.reactive.accept.DefaultApiVersionStrategy; +import org.springframework.web.reactive.accept.HeaderApiVersionResolver; import org.springframework.web.reactive.result.condition.ConsumesRequestCondition; import org.springframework.web.reactive.result.condition.MediaTypeExpression; import org.springframework.web.reactive.result.method.RequestMappingInfo; @@ -107,6 +116,35 @@ class RequestMappingHandlerMappingTests { assertThat(info.getPatternsCondition().getPatterns()).containsOnly(new PathPatternParser().parse("/api/user/{id}")); } + @Test + void version() { + ServerWebExchange exchange = initExchangeForVersionTest("1.1"); + HandlerMethod handlerMethod = (HandlerMethod) this.handlerMapping.getHandler(exchange).block(); + assertThat(handlerMethod.getMethod().getName()).isEqualTo("foo1_1"); + } + + @Test + void versionInvalid() { + ServerWebExchange exchange = initExchangeForVersionTest("99"); + StepVerifier.create(this.handlerMapping.getHandler(exchange)) + .verifyError(InvalidApiVersionException.class); + } + + private ServerWebExchange initExchangeForVersionTest(String version) { + + ((StaticWebApplicationContext) this.handlerMapping.getApplicationContext()) + .registerSingleton("controller", VersionController.class); + + DefaultApiVersionStrategy versionStrategy = new DefaultApiVersionStrategy( + List.of(new HeaderApiVersionResolver("API-Version")), new SemanticApiVersionParser(), + true, null, true, null, null); + this.handlerMapping.setApiVersionStrategy(versionStrategy); + this.handlerMapping.afterPropertiesSet(); + + MockServerHttpRequest request = MockServerHttpRequest.get("/foo").header("API-Version", version).build(); + return MockServerWebExchange.from(request); + } + @Test void resolveRequestMappingViaComposedAnnotation() { RequestMappingInfo info = assertComposedAnnotationMapping("postJson", "/postJson", RequestMethod.POST); @@ -367,6 +405,7 @@ class RequestMappingHandlerMappingTests { MockServerHttpRequest request = MockServerHttpRequest.post(path) .contentType(mediaType).build(); ServerWebExchange exchange = MockServerWebExchange.from(request); + exchange.getAttributes().put(HandlerMapping.API_VERSION_ATTRIBUTE, ApiVersionHolder.EMPTY); RequestMappingInfo matchingInfo = info.getMatchingCondition(exchange); // Since the request has no body AND the required flag is false, the // ConsumesCondition in the matching condition in an EMPTY_CONDITION. @@ -409,6 +448,7 @@ class RequestMappingHandlerMappingTests { MockServerHttpRequest request = MockServerHttpRequest.post(path) .contentType(mediaType).build(); ServerWebExchange exchange = MockServerWebExchange.from(request); + exchange.getAttributes().put(HandlerMapping.API_VERSION_ATTRIBUTE, ApiVersionHolder.EMPTY); RequestMappingInfo matchingInfo = info.getMatchingCondition(exchange); assertThat(matchingInfo).isEqualTo(paths(path).methods(POST).consumes(mediaType.toString()).build()); } @@ -568,6 +608,22 @@ class RequestMappingHandlerMappingTests { } + @RestController + @RequestMapping("/foo") + static class VersionController { + + @GetMapping + public String foo() { + return "foo"; + } + + @GetMapping(version = "1.1") + public String foo1_1() { + return "foo1_1"; + } + } + + @RestController @HttpExchange("/exchange") static class HttpExchangeController { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/HandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/HandlerMapping.java index 081ac53ff02..fb9cb372694 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/HandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/HandlerMapping.java @@ -20,6 +20,8 @@ import jakarta.servlet.ServletRequest; import jakarta.servlet.http.HttpServletRequest; import org.jspecify.annotations.Nullable; +import org.springframework.web.accept.ApiVersionHolder; + /** * Interface to be implemented by objects that define a mapping between * requests and handler objects. @@ -136,8 +138,9 @@ public interface HandlerMapping { String PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE = HandlerMapping.class.getName() + ".producibleMediaTypes"; /** - * Name of the {@link HttpServletRequest} attribute that contains the - * resolved and parsed API version. + * Name of the {@link HttpServletRequest} attribute that contains an + * {@link ApiVersionHolder} with the + * result of obtaining and parsing the API version of the request. * @since 7.0 */ String API_VERSION_ATTRIBUTE = HandlerMapping.class.getName() + ".apiVersion"; diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/RequestPredicates.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/RequestPredicates.java index 3cf40bd24ae..a56aedabb0e 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/RequestPredicates.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/RequestPredicates.java @@ -54,6 +54,7 @@ import org.springframework.util.CollectionUtils; import org.springframework.util.MimeTypeUtils; import org.springframework.util.MultiValueMap; import org.springframework.validation.BindException; +import org.springframework.web.accept.ApiVersionHolder; import org.springframework.web.accept.ApiVersionStrategy; import org.springframework.web.bind.WebDataBinder; import org.springframework.web.cors.CorsUtils; @@ -890,14 +891,17 @@ public abstract class RequestPredicates { this.parsedVersion = strategy.parseVersion(this.version); } - Comparable requestVersion = - (Comparable) request.attribute(HandlerMapping.API_VERSION_ATTRIBUTE).orElse(null); + ApiVersionHolder requestVersionHolder = + (ApiVersionHolder) request.attribute(HandlerMapping.API_VERSION_ATTRIBUTE) + .orElseThrow(() -> new IllegalStateException("Expect API version attribute")); - if (requestVersion == null) { + if (!requestVersionHolder.hasVersion()) { traceMatch("Version", this.version, null, true); return true; } + Comparable requestVersion = requestVersionHolder.getVersion(); + int result = compareVersions(this.parsedVersion, requestVersion); boolean match = (this.baselineVersion ? result <= 0 : result == 0); traceMatch("Version", this.version, requestVersion, match); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMapping.java index da4aaa0f269..d250df937cb 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMapping.java @@ -39,6 +39,7 @@ import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.util.PathMatcher; import org.springframework.web.HttpRequestHandler; +import org.springframework.web.accept.ApiVersionHolder; import org.springframework.web.accept.ApiVersionStrategy; import org.springframework.web.context.request.WebRequestInterceptor; import org.springframework.web.context.request.async.WebAsyncManager; @@ -539,7 +540,7 @@ public abstract class AbstractHandlerMapping extends WebApplicationObjectSupport */ @Override public final @Nullable HandlerExecutionChain getHandler(HttpServletRequest request) throws Exception { - initApiVersion(request); + ApiVersionHolder versionHolder = initApiVersion(request); Object handler = getHandlerInternal(request); if (handler == null) { handler = getDefaultHandler(); @@ -547,6 +548,11 @@ public abstract class AbstractHandlerMapping extends WebApplicationObjectSupport if (handler == null) { return null; } + + if (versionHolder.hasError()) { + throw versionHolder.getError(); + } + // Bean name or resolved handler? if (handler instanceof String handlerName) { handler = obtainApplicationContext().getBean(handlerName); @@ -584,16 +590,25 @@ public abstract class AbstractHandlerMapping extends WebApplicationObjectSupport return executionChain; } - private void initApiVersion(HttpServletRequest request) { - if (this.versionStrategy != null) { - Comparable version = (Comparable) request.getAttribute(API_VERSION_ATTRIBUTE); - if (version == null) { - version = this.versionStrategy.resolveParseAndValidateVersion(request); - if (version != null) { - request.setAttribute(API_VERSION_ATTRIBUTE, version); + private ApiVersionHolder initApiVersion(HttpServletRequest request) { + ApiVersionHolder versionHolder = (ApiVersionHolder) request.getAttribute(API_VERSION_ATTRIBUTE); + if (versionHolder == null) { + if (this.versionStrategy == null) { + versionHolder = ApiVersionHolder.EMPTY; + } + else { + Comparable version; + try { + version = this.versionStrategy.resolveParseAndValidateVersion(request); + versionHolder = ApiVersionHolder.fromVersion(version); + } + catch (RuntimeException ex) { + versionHolder = ApiVersionHolder.fromError(ex); } } } + request.setAttribute(API_VERSION_ATTRIBUTE, versionHolder); + return versionHolder; } /** @@ -683,8 +698,9 @@ public abstract class AbstractHandlerMapping extends WebApplicationObjectSupport } if (this.versionStrategy != null) { - Comparable version = (Comparable) request.getAttribute(API_VERSION_ATTRIBUTE); - if (version != null) { + ApiVersionHolder versionHolder = (ApiVersionHolder) request.getAttribute(API_VERSION_ATTRIBUTE); + if (versionHolder.hasVersion()) { + Comparable version = versionHolder.getVersion(); chain.addInterceptor(new ApiVersionDeprecationHandlerInterceptor(this.versionStrategy, version)); } } 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 cc71d473bf7..825d82f0921 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 @@ -25,6 +25,7 @@ import org.jspecify.annotations.Nullable; import org.springframework.util.Assert; import org.springframework.util.StringUtils; +import org.springframework.web.accept.ApiVersionHolder; import org.springframework.web.accept.ApiVersionStrategy; import org.springframework.web.accept.NotAcceptableApiVersionException; import org.springframework.web.bind.annotation.RequestMapping; @@ -98,12 +99,14 @@ public final class VersionRequestCondition extends AbstractRequestCondition requestVersion = (Comparable) request.getAttribute(HandlerMapping.API_VERSION_ATTRIBUTE); + ApiVersionHolder versionHolder = (ApiVersionHolder) request.getAttribute(HandlerMapping.API_VERSION_ATTRIBUTE); - if (this.version == null || requestVersion == null) { + if (this.version == null || !versionHolder.hasVersion()) { return this; } + Comparable requestVersion = versionHolder.getVersion(); + // 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() @@ -129,8 +132,8 @@ public final class VersionRequestCondition extends AbstractRequestCondition version = (Comparable) request.getAttribute(HandlerMapping.API_VERSION_ATTRIBUTE); - return (version == null ? -1 * result : result); + ApiVersionHolder holder = (ApiVersionHolder) request.getAttribute(HandlerMapping.API_VERSION_ATTRIBUTE); + return (!holder.hasVersion() ? -1 * result : result); } } @@ -150,7 +153,8 @@ public final class VersionRequestCondition extends AbstractRequestCondition version = (Comparable) request.getAttribute(HandlerMapping.API_VERSION_ATTRIBUTE); + ApiVersionHolder holder = (ApiVersionHolder) request.getAttribute(HandlerMapping.API_VERSION_ATTRIBUTE); + Comparable version = holder.getVersionIfPresent(); if (version != null && !this.version.equals(version)) { throw new NotAcceptableApiVersionException(version.toString()); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ApiVersionMethodArgumentResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ApiVersionMethodArgumentResolver.java index 09fcca92d6f..b58f2cf935b 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ApiVersionMethodArgumentResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ApiVersionMethodArgumentResolver.java @@ -23,6 +23,7 @@ import org.jspecify.annotations.Nullable; import org.springframework.core.MethodParameter; import org.springframework.util.Assert; +import org.springframework.web.accept.ApiVersionHolder; import org.springframework.web.accept.MissingApiVersionException; import org.springframework.web.accept.SemanticApiVersionParser; import org.springframework.web.bind.support.WebDataBinderFactory; @@ -51,7 +52,8 @@ public class ApiVersionMethodArgumentResolver implements HandlerMethodArgumentRe HttpServletRequest request = webRequest.getNativeRequest(HttpServletRequest.class); Assert.state(request != null, "No HttpServletRequest"); - Object version = request.getAttribute(HandlerMapping.API_VERSION_ATTRIBUTE); + ApiVersionHolder versionHolder = (ApiVersionHolder) request.getAttribute(HandlerMapping.API_VERSION_ATTRIBUTE); + Object version = versionHolder.getVersionIfPresent(); if (parameter.getParameterType() == Optional.class) { return Optional.ofNullable(version); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/function/RequestPredicatesTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/function/RequestPredicatesTests.java index 55f9eb0d22d..763328d6c7c 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/function/RequestPredicatesTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/function/RequestPredicatesTests.java @@ -27,6 +27,7 @@ import org.junit.jupiter.api.Test; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; +import org.springframework.web.accept.ApiVersionHolder; import org.springframework.web.accept.ApiVersionStrategy; import org.springframework.web.accept.DefaultApiVersionStrategy; import org.springframework.web.accept.SemanticApiVersionParser; @@ -284,7 +285,8 @@ class RequestPredicatesTests { MockHttpServletRequest servletRequest = PathPatternsTestUtils.initRequest("GET", null, "/path", true, - req -> req.setAttribute(API_VERSION_ATTRIBUTE, strategy.parseVersion(version))); + req -> req.setAttribute(API_VERSION_ATTRIBUTE, + ApiVersionHolder.fromVersion(strategy.parseVersion(version)))); return new DefaultServerRequest(servletRequest, Collections.emptyList(), strategy); } 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 5fbb0769826..0c5142f049d 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,6 +24,7 @@ import org.jspecify.annotations.Nullable; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.springframework.web.accept.ApiVersionHolder; import org.springframework.web.accept.DefaultApiVersionStrategy; import org.springframework.web.accept.NotAcceptableApiVersionException; import org.springframework.web.accept.SemanticApiVersionParser; @@ -125,7 +126,8 @@ public class VersionRequestConditionTests { String version = "1.2"; this.strategy = initVersionStrategy(version); VersionRequestCondition condition = condition(version); - VersionRequestCondition match = condition.getMatchingCondition(new MockHttpServletRequest("GET", "/path")); + + VersionRequestCondition match = condition.getMatchingCondition(requestWithVersion(null)); assertThat(match).isSameAs(condition); } @@ -141,7 +143,7 @@ public class VersionRequestConditionTests { private void testCompare(String expected, String... versions) { List list = Arrays.stream(versions) .map(this::condition) - .sorted((c1, c2) -> c1.compareTo(c2, new MockHttpServletRequest())) + .sorted((c1, c2) -> c1.compareTo(c2, requestWithVersion(null))) .toList(); assertThat(list.get(0)).isEqualTo(condition(expected)); @@ -150,7 +152,7 @@ public class VersionRequestConditionTests { @Test void compareWithoutRequestVersion() { VersionRequestCondition condition = Stream.of(condition("1.1"), condition("1.2"), emptyCondition()) - .min((c1, c2) -> c1.compareTo(c2, new MockHttpServletRequest())) + .min((c1, c2) -> c1.compareTo(c2, requestWithVersion(null))) .get(); assertThat(condition).isEqualTo(emptyCondition()); @@ -158,7 +160,7 @@ public class VersionRequestConditionTests { @Test // gh-35236 void noRequestVersion() { - MockHttpServletRequest request = new MockHttpServletRequest("GET", "/path"); + MockHttpServletRequest request = requestWithVersion(null); VersionRequestCondition condition = condition("1.1"); VersionRequestCondition match = condition.getMatchingCondition(request); @@ -178,7 +180,8 @@ public class VersionRequestConditionTests { private MockHttpServletRequest requestWithVersion(String v) { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/path"); - request.setAttribute(HandlerMapping.API_VERSION_ATTRIBUTE, this.strategy.parseVersion(v)); + Comparable version = (v != null ? strategy.parseVersion(v) : null); + request.setAttribute(HandlerMapping.API_VERSION_ATTRIBUTE, ApiVersionHolder.fromVersion(version)); return request; } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ApiVersionMethodArgumentResolverTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ApiVersionMethodArgumentResolverTests.java index 18663afee2b..10d298a4191 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ApiVersionMethodArgumentResolverTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ApiVersionMethodArgumentResolverTests.java @@ -24,6 +24,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.core.MethodParameter; +import org.springframework.web.accept.ApiVersionHolder; import org.springframework.web.accept.SemanticApiVersionParser; import org.springframework.web.accept.SemanticApiVersionParser.Version; import org.springframework.web.context.request.ServletWebRequest; @@ -82,7 +83,7 @@ class ApiVersionMethodArgumentResolverTests { @Test void resolveArgument() throws Exception { Version version = new SemanticApiVersionParser().parseVersion("1.2"); - this.servletRequest.setAttribute(HandlerMapping.API_VERSION_ATTRIBUTE, version); + this.servletRequest.setAttribute(HandlerMapping.API_VERSION_ATTRIBUTE, ApiVersionHolder.fromVersion(version)); Object actual = this.resolver.resolveArgument(this.param, this.mav, this.webRequest, null); @@ -93,6 +94,7 @@ class ApiVersionMethodArgumentResolverTests { @Test void resolveNullableArgument() throws Exception { + this.servletRequest.setAttribute(HandlerMapping.API_VERSION_ATTRIBUTE, ApiVersionHolder.EMPTY); Object actual = this.resolver.resolveArgument(this.nullableParam, this.mav, this.webRequest, null); assertThat(actual).isNull(); } @@ -100,7 +102,7 @@ class ApiVersionMethodArgumentResolverTests { @Test void resolveOptionalArgument() throws Exception { Version version = new SemanticApiVersionParser().parseVersion("1.2"); - this.servletRequest.setAttribute(HandlerMapping.API_VERSION_ATTRIBUTE, version); + this.servletRequest.setAttribute(HandlerMapping.API_VERSION_ATTRIBUTE, ApiVersionHolder.fromVersion(version)); Object actual = this.resolver.resolveArgument(this.optionalParam, this.mav, this.webRequest, null); assertThat(actual).asInstanceOf(OPTIONAL).hasValue(version); @@ -108,6 +110,7 @@ class ApiVersionMethodArgumentResolverTests { @Test void resolveOptionalArgumentWhenEmpty() throws Exception { + this.servletRequest.setAttribute(HandlerMapping.API_VERSION_ATTRIBUTE, ApiVersionHolder.EMPTY); Object actual = this.resolver.resolveArgument(this.optionalParam, this.mav, this.webRequest, null); assertThat(actual).asInstanceOf(OPTIONAL).isEmpty(); } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterIntegrationTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterIntegrationTests.java index 6e63ffe1eb2..c4cb0d7ad5c 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterIntegrationTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterIntegrationTests.java @@ -57,6 +57,7 @@ import org.springframework.ui.ModelMap; import org.springframework.validation.BindingResult; import org.springframework.validation.Errors; import org.springframework.validation.Validator; +import org.springframework.web.accept.ApiVersionHolder; import org.springframework.web.accept.SemanticApiVersionParser; import org.springframework.web.bind.WebDataBinder; import org.springframework.web.bind.annotation.CookieValue; @@ -176,7 +177,7 @@ class RequestMappingHandlerAdapterIntegrationTests { request.getSession().setAttribute("sessionAttribute", sessionAttribute); request.setAttribute("requestAttribute", requestAttribute); SemanticApiVersionParser.Version version = new SemanticApiVersionParser().parseVersion("1.2"); - request.setAttribute(HandlerMapping.API_VERSION_ATTRIBUTE, version); + request.setAttribute(HandlerMapping.API_VERSION_ATTRIBUTE, ApiVersionHolder.fromVersion(version)); HandlerMethod handlerMethod = handlerMethod("handle", parameterTypes); ModelAndView mav = handlerAdapter.handle(request, response, handlerMethod); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMappingTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMappingTests.java index 7663dc2767f..249cab04570 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMappingTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMappingTests.java @@ -23,6 +23,7 @@ import java.lang.annotation.Target; import java.lang.reflect.Method; import java.security.Principal; import java.util.Collections; +import java.util.List; import java.util.Set; import java.util.stream.Stream; @@ -35,6 +36,10 @@ import org.springframework.core.annotation.AliasFor; import org.springframework.http.MediaType; import org.springframework.stereotype.Controller; import org.springframework.util.ClassUtils; +import org.springframework.web.accept.DefaultApiVersionStrategy; +import org.springframework.web.accept.HeaderApiVersionResolver; +import org.springframework.web.accept.InvalidApiVersionException; +import org.springframework.web.accept.SemanticApiVersionParser; import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PatchMapping; @@ -44,6 +49,7 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.context.support.StaticWebApplicationContext; +import org.springframework.web.method.HandlerMethod; import org.springframework.web.method.HandlerTypePredicate; import org.springframework.web.service.annotation.HttpExchange; import org.springframework.web.service.annotation.PostExchange; @@ -60,6 +66,7 @@ import org.springframework.web.util.pattern.PathPatternParser; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Named.named; import static org.junit.jupiter.params.provider.Arguments.arguments; import static org.mockito.Mockito.mock; @@ -160,6 +167,36 @@ class RequestMappingHandlerMappingTests { } } + @PathPatternsParameterizedTest + void version(RequestMappingHandlerMapping mapping) throws Exception { + MockHttpServletRequest request = initRequestForVersionTest(mapping, "1.1"); + HandlerMethod handlerMethod = (HandlerMethod) mapping.getHandler(request).getHandler(); + assertThat(handlerMethod.getMethod().getName()).isEqualTo("foo1_1"); + } + + @PathPatternsParameterizedTest + void versionInvalid(RequestMappingHandlerMapping mapping) throws Exception { + MockHttpServletRequest request = initRequestForVersionTest(mapping, "99"); + assertThatThrownBy(() -> mapping.getHandler(request)).isInstanceOf(InvalidApiVersionException.class); + } + + private static MockHttpServletRequest initRequestForVersionTest( + RequestMappingHandlerMapping mapping, String version) { + + ((StaticWebApplicationContext) mapping.getApplicationContext()) + .registerSingleton("controller", VersionController.class); + + DefaultApiVersionStrategy versionStrategy = new DefaultApiVersionStrategy( + List.of(new HeaderApiVersionResolver("API-Version")), new SemanticApiVersionParser(), + true, null, true, null, null); + mapping.setApiVersionStrategy(versionStrategy); + mapping.afterPropertiesSet(); + + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo"); + request.addHeader("API-Version", version); + return request; + } + @PathPatternsParameterizedTest void resolveRequestMappingViaComposedAnnotation(RequestMappingHandlerMapping mapping) { RequestMappingInfo info = assertComposedAnnotationMapping( @@ -626,6 +663,22 @@ class RequestMappingHandlerMappingTests { } + @RestController + @RequestMapping("/foo") + static class VersionController { + + @GetMapping + public String foo() { + return "foo"; + } + + @GetMapping(version = "1.1") + public String foo1_1() { + return "foo1_1"; + } + } + + @RestController @HttpExchange("/exchange") static class HttpExchangeController {