From 11cb06235706cd826c2351b56697cb2e82d68e16 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Thu, 28 Aug 2025 15:49:37 +0300 Subject: [PATCH] Assert versionRequired and defaultVersion Closes gh-35387 --- .../web/accept/DefaultApiVersionStrategy.java | 12 +++++++----- .../accept/DefaultApiVersionStrategiesTests.java | 13 ++++++++++++- .../reactive/accept/DefaultApiVersionStrategy.java | 12 +++++++----- .../web/reactive/config/ApiVersionConfigurer.java | 2 +- .../accept/DefaultApiVersionStrategiesTests.java | 14 ++++++++++++-- .../condition/VersionRequestConditionTests.java | 2 +- .../config/annotation/ApiVersionConfigurer.java | 2 +- .../condition/VersionRequestConditionTests.java | 2 +- 8 files changed, 42 insertions(+), 17 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/accept/DefaultApiVersionStrategy.java b/spring-web/src/main/java/org/springframework/web/accept/DefaultApiVersionStrategy.java index b9c41fb31bb..bb65d4c0a31 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/DefaultApiVersionStrategy.java +++ b/spring-web/src/main/java/org/springframework/web/accept/DefaultApiVersionStrategy.java @@ -61,9 +61,9 @@ public class DefaultApiVersionStrategy implements ApiVersionStrategy { * @param versionResolvers one or more resolvers to try; the first non-null * value returned by any resolver becomes the value used * @param versionParser parser for raw version values - * @param versionRequired whether a version is required; if a request - * does not have a version, and a {@code defaultVersion} is not specified, - * validation fails with {@link MissingApiVersionException} + * @param versionRequired whether a version is required leading to + * {@link MissingApiVersionException} for requests that don't have one; + * by default set to true unless there is a defaultVersion * @param defaultVersion a default version to assign to requests that * don't specify one * @param detectSupportedVersions whether to use API versions that appear in @@ -74,16 +74,18 @@ public class DefaultApiVersionStrategy implements ApiVersionStrategy { */ public DefaultApiVersionStrategy( List versionResolvers, ApiVersionParser versionParser, - boolean versionRequired, @Nullable String defaultVersion, + @Nullable Boolean versionRequired, @Nullable String defaultVersion, boolean detectSupportedVersions, @Nullable Predicate> supportedVersionPredicate, @Nullable ApiVersionDeprecationHandler deprecationHandler) { Assert.notEmpty(versionResolvers, "At least one ApiVersionResolver is required"); Assert.notNull(versionParser, "ApiVersionParser is required"); + Assert.isTrue(defaultVersion == null || versionRequired == null || !versionRequired, + "versionRequired cannot be set to true if a defaultVersion is also configured"); this.versionResolvers = new ArrayList<>(versionResolvers); this.versionParser = versionParser; - this.versionRequired = (versionRequired && defaultVersion == null); + this.versionRequired = (versionRequired != null ? versionRequired : defaultVersion == null); this.defaultVersion = (defaultVersion != null ? versionParser.parseVersion(defaultVersion) : null); this.detectSupportedVersions = detectSupportedVersions; this.supportedVersionPredicate = initSupportedVersionPredicate(supportedVersionPredicate); 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 5edea422003..be1597479f9 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 @@ -25,6 +25,7 @@ import org.junit.jupiter.api.Test; import org.springframework.web.testfixture.servlet.MockHttpServletRequest; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatThrownBy; /** @@ -93,6 +94,16 @@ public class DefaultApiVersionStrategiesTests { assertThatThrownBy(() -> validateVersion("1.2", strategy)).isInstanceOf(InvalidApiVersionException.class); } + @Test + void versionRequiredAndDefaultVersionSet() { + assertThatIllegalArgumentException() + .isThrownBy(() -> + new DefaultApiVersionStrategy( + List.of(request -> request.getParameter("api-version")), new SemanticApiVersionParser(), + true, "1.2", true, version -> true, null)) + .withMessage("versionRequired cannot be set to true if a defaultVersion is also configured"); + } + private static DefaultApiVersionStrategy apiVersionStrategy() { return apiVersionStrategy(null, false, null); } @@ -107,7 +118,7 @@ public class DefaultApiVersionStrategiesTests { return new DefaultApiVersionStrategy( List.of(request -> request.getParameter("api-version")), new SemanticApiVersionParser(), - true, defaultVersion, detectSupportedVersions, supportedVersionPredicate, null); + null, defaultVersion, detectSupportedVersions, supportedVersionPredicate, null); } private void validateVersion(@Nullable String version, DefaultApiVersionStrategy strategy) { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/DefaultApiVersionStrategy.java b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/DefaultApiVersionStrategy.java index 114f0c95dd5..da7ce06b2dd 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/DefaultApiVersionStrategy.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/DefaultApiVersionStrategy.java @@ -63,9 +63,9 @@ public class DefaultApiVersionStrategy implements ApiVersionStrategy { * @param versionResolvers one or more resolvers to try; the first non-null * value returned by any resolver becomes the resolved used * @param versionParser parser for to raw version values - * @param versionRequired whether a version is required; if a request - * does not have a version, and a {@code defaultVersion} is not specified, - * validation fails with {@link MissingApiVersionException} + * @param versionRequired whether a version is required leading to + * {@link MissingApiVersionException} for requests that don't have one; + * by default set to true unless there is a defaultVersion * @param defaultVersion a default version to assign to requests that * don't specify one * @param detectSupportedVersions whether to use API versions that appear in @@ -76,16 +76,18 @@ public class DefaultApiVersionStrategy implements ApiVersionStrategy { */ public DefaultApiVersionStrategy( List versionResolvers, ApiVersionParser versionParser, - boolean versionRequired, @Nullable String defaultVersion, + @Nullable Boolean versionRequired, @Nullable String defaultVersion, boolean detectSupportedVersions, @Nullable Predicate> supportedVersionPredicate, @Nullable ApiVersionDeprecationHandler deprecationHandler) { Assert.notEmpty(versionResolvers, "At least one ApiVersionResolver is required"); Assert.notNull(versionParser, "ApiVersionParser is required"); + Assert.isTrue(defaultVersion == null || versionRequired == null || !versionRequired, + "versionRequired cannot be set to true if a defaultVersion is also configured"); this.versionResolvers = new ArrayList<>(versionResolvers); this.versionParser = versionParser; - this.versionRequired = (versionRequired && defaultVersion == null); + this.versionRequired = (versionRequired != null ? versionRequired : defaultVersion == null); this.defaultVersion = (defaultVersion != null ? versionParser.parseVersion(defaultVersion) : null); this.detectSupportedVersions = detectSupportedVersions; this.supportedVersionPredicate = initSupportedVersionPredicate(supportedVersionPredicate); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/config/ApiVersionConfigurer.java b/spring-webflux/src/main/java/org/springframework/web/reactive/config/ApiVersionConfigurer.java index 2fe21fc5393..549120c5857 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/config/ApiVersionConfigurer.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/config/ApiVersionConfigurer.java @@ -212,7 +212,7 @@ public class ApiVersionConfigurer { DefaultApiVersionStrategy strategy = new DefaultApiVersionStrategy(this.versionResolvers, (this.versionParser != null ? this.versionParser : new SemanticApiVersionParser()), - (this.versionRequired != null ? this.versionRequired : true), this.defaultVersion, + this.versionRequired, this.defaultVersion, this.detectSupportedVersions, this.supportedVersionPredicate, this.deprecationHandler); 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 f826dccf311..377cc2f8704 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 @@ -29,6 +29,7 @@ import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRe import org.springframework.web.testfixture.server.MockServerWebExchange; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatThrownBy; /** @@ -97,6 +98,16 @@ public class DefaultApiVersionStrategiesTests { assertThatThrownBy(() -> validateVersion("1.2", strategy)).isInstanceOf(InvalidApiVersionException.class); } + @Test + void versionRequiredAndDefaultVersionSet() { + assertThatIllegalArgumentException() + .isThrownBy(() -> + new org.springframework.web.accept.DefaultApiVersionStrategy( + List.of(request -> request.getParameter("api-version")), new SemanticApiVersionParser(), + true, "1.2", true, version -> true, null)) + .withMessage("versionRequired cannot be set to true if a defaultVersion is also configured"); + } + private static DefaultApiVersionStrategy apiVersionStrategy() { return apiVersionStrategy(null, false, null); } @@ -107,7 +118,7 @@ public class DefaultApiVersionStrategiesTests { return new DefaultApiVersionStrategy( List.of(exchange -> exchange.getRequest().getQueryParams().getFirst("api-version")), - parser, true, defaultVersion, detectSupportedVersions, supportedVersionPredicate, null); + parser, null, defaultVersion, detectSupportedVersions, supportedVersionPredicate, null); } private void validateVersion(@Nullable String version, DefaultApiVersionStrategy strategy) { @@ -115,7 +126,6 @@ public class DefaultApiVersionStrategiesTests { if (version != null) { requestBuilder.queryParam("api-version", version); } - Comparable parsedVersion = (version != null ? parser.parseVersion(version) : null); strategy.resolveParseAndValidateVersion(MockServerWebExchange.builder(requestBuilder).build()); } 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 b96de727c5a..e658ec41da0 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 @@ -52,7 +52,7 @@ public class VersionRequestConditionTests { private static DefaultApiVersionStrategy initVersionStrategy(@Nullable String defaultVersion) { return new DefaultApiVersionStrategy( List.of(exchange -> exchange.getRequest().getQueryParams().getFirst("api-version")), - new SemanticApiVersionParser(), true, defaultVersion, false, null, null); + new SemanticApiVersionParser(), null, defaultVersion, false, null, null); } @Test diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ApiVersionConfigurer.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ApiVersionConfigurer.java index e08e1708163..f3db4095d4a 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ApiVersionConfigurer.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ApiVersionConfigurer.java @@ -213,7 +213,7 @@ public class ApiVersionConfigurer { DefaultApiVersionStrategy strategy = new DefaultApiVersionStrategy(this.versionResolvers, (this.versionParser != null ? this.versionParser : new SemanticApiVersionParser()), - (this.versionRequired != null ? this.versionRequired : true), this.defaultVersion, + this.versionRequired, this.defaultVersion, this.detectSupportedVersions, this.supportedVersionPredicate, this.deprecationHandler); 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 47b41ba4563..5fbb0769826 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 @@ -50,7 +50,7 @@ public class VersionRequestConditionTests { private static DefaultApiVersionStrategy initVersionStrategy(@Nullable String defaultVersion) { return new DefaultApiVersionStrategy( List.of(request -> request.getParameter("api-version")), - new SemanticApiVersionParser(), true, defaultVersion, false, null, null); + new SemanticApiVersionParser(), null, defaultVersion, false, null, null); } @Test