Polishing contribution

Closes gh-36398

spring-web/src/main/java/org/springframework/web/accept/PathApiVersionResolver.java

@@ -30,10 +30,13 @@ import org.springframework.web.util.ServletRequestPathUtils;
 /**
  * {@link ApiVersionResolver} that extract the version from a path segment.
  *
- * <p>Note that this resolver will either resolve the version from the specified
- * path segment, or raise an {@link InvalidApiVersionException}, e.g. if there
- * are not enough path segments. It never returns {@code null}, and therefore
- * cannot yield to other resolvers.
+ * <p>If the resolver is created with a path index only, it will always return
+ * a version, or raise an {@link InvalidApiVersionException}, but never
+ * return {@code null}.
+ *
+ * <p>The resolver can also be created with an additional
+ * {@code Predicate<RequestPath>} that provides more flexibility in deciding
+ * whether a given path is versioned or not, possibly resolving to {@code null}.
  *
  * @author Rossen Stoyanchev
  * @since 7.0
@@ -41,7 +44,8 @@ import org.springframework.web.util.ServletRequestPathUtils;
 public class PathApiVersionResolver implements ApiVersionResolver {
 
 	private final int pathSegmentIndex;
-	private @Nullable Predicate<RequestPath> includePath;
+
+	private @Nullable Predicate<RequestPath> versionPathPredicate;
 
 
 	/**
@@ -55,22 +59,19 @@ public class PathApiVersionResolver implements ApiVersionResolver {
 	}
 
 	/**
-	 * Create a resolver instance.
-	 * @param pathSegmentIndex the index of the path segment that contains the API version
-	 * @param includePath a {@link Predicate} that tests if the given path should be included
+	 * Constructor variant of {@link #PathApiVersionResolver(int)} with an
+	 * additional {@code Predicate<RequestPath>} to help determine whether
+	 * a given path is versioned (true) or not (false).
 	 */
-	public PathApiVersionResolver(int pathSegmentIndex, Predicate<RequestPath> includePath) {
+	public PathApiVersionResolver(int pathSegmentIndex, Predicate<RequestPath> versionPathPredicate) {
 		this(pathSegmentIndex);
-		this.includePath = includePath;
+		this.versionPathPredicate = versionPathPredicate;
 	}
 
 	@Override
 	public @Nullable String resolveVersion(HttpServletRequest request) {
-		if (!ServletRequestPathUtils.hasParsedRequestPath(request)) {
-			throw new IllegalStateException("Expected parsed request path");
-		}
 		RequestPath path = ServletRequestPathUtils.getParsedRequestPath(request);
-		if (this.includePath != null && !this.includePath.test(path)) {
+		if (!isVersionedPath(path)) {
 			return null;
 		}
 		int i = 0;
@@ -82,4 +83,8 @@ public class PathApiVersionResolver implements ApiVersionResolver {
 		throw new InvalidApiVersionException("No path segment at index " + this.pathSegmentIndex);
 	}
 
+	private boolean isVersionedPath(RequestPath path) {
+		return (this.versionPathPredicate == null || this.versionPathPredicate.test(path));
+	}
+
 }

spring-web/src/test/java/org/springframework/web/accept/PathApiVersionResolverTests.java

@@ -17,10 +17,12 @@
 package org.springframework.web.accept;
 
 import java.util.List;
+import java.util.function.Predicate;
 
 import org.junit.jupiter.api.Test;
 
 import org.springframework.http.server.PathContainer;
+import org.springframework.http.server.RequestPath;
 import org.springframework.web.testfixture.servlet.MockHttpServletRequest;
 import org.springframework.web.util.ServletRequestPathUtils;
 
@@ -45,52 +47,25 @@ public class PathApiVersionResolverTests {
 	}
 
 	@Test
-	void includePathFalse() {
-		String requestUri = "/v3/api-docs";
-		testResolveWithIncludePath(requestUri, null);
+	void resolveWithVersionPathPredicate() {
+		testVersionPathPredicate("/app/1.0/path", "1.0");
+		testVersionPathPredicate("/app", null);
+		testVersionPathPredicate("/v3/api-docs", null);
 	}
 
-	@Test
-	void includePathTrue() {
-		String requestUri = "/app/1.0/path";
-		testResolveWithIncludePath(requestUri, "1.0");
-	}
-
-	@Test
-	void includePathFalseShortPath() {
-		String requestUri = "/app";
-		testResolveWithIncludePath(requestUri, null);
-	}
-
-	@Test
-	void includePathInsufficientPathSegments() {
-		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/app");
-		try {
-			ServletRequestPathUtils.parseAndCache(request);
-			assertThatThrownBy(() -> new PathApiVersionResolver(1, requestPath -> true)
-					.resolveVersion(request))
-				.isInstanceOf(InvalidApiVersionException.class);
-		}
-		finally {
-			ServletRequestPathUtils.clearParsedRequestPath(request);
-		}
-	}
-
-	private static void testResolveWithIncludePath(String requestUri, String expected) {
+	private static void testVersionPathPredicate(String requestUri, String expectedVersion) {
+		Predicate<RequestPath> versionPathPredicate = path -> {
+			List<PathContainer.Element> elements = path.elements();
+			return (elements.size() > 3 &&
+					elements.get(1).value().equals("app") &&
+					elements.get(3).value().matches("\\d+\\.\\d+"));
+		};
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
 		try {
 			ServletRequestPathUtils.parseAndCache(request);
-			String actual = new PathApiVersionResolver(1, requestPath -> {
-				List<PathContainer.Element> elements = requestPath.elements();
-				if (elements.size() < 4) {
-					return false;
-				}
-				return elements.get(0).value().equals("/") &&
-						elements.get(1).value().equals("app") &&
-						elements.get(2).value().equals("/") &&
-						elements.get(3).value().equals("1.0");
-			}).resolveVersion(request);
-			assertThat(actual).isEqualTo(expected);
+			PathApiVersionResolver resolver = new PathApiVersionResolver(1, versionPathPredicate);
+			String actual = resolver.resolveVersion(request);
+			assertThat(actual).isEqualTo(expectedVersion);
 		}
 		finally {
 			ServletRequestPathUtils.clearParsedRequestPath(request);

spring-webflux/src/main/java/org/springframework/web/reactive/accept/PathApiVersionResolver.java

@@ -29,10 +29,13 @@ import org.springframework.web.server.ServerWebExchange;
 /**
  * {@link ApiVersionResolver} that extract the version from a path segment.
  *
- * <p>Note that this resolver will either resolve the version from the specified
- * path segment, or raise an {@link InvalidApiVersionException}, e.g. if there
- * are not enough path segments. It never returns {@code null}, and therefore
- * cannot yield to other resolvers.
+ * <p>If the resolver is created with a path index only, it will always return
+ * a version, or raise an {@link InvalidApiVersionException}, but never
+ * return {@code null}.
+ *
+ * <p>The resolver can also be created with an additional
+ * {@code Predicate<RequestPath>} that provides more flexibility in deciding
+ * whether a given path is versioned or not, possibly resolving to {@code null}.
  *
  * @author Rossen Stoyanchev
  * @author Martin Mois
@@ -41,7 +44,8 @@ import org.springframework.web.server.ServerWebExchange;
 public class PathApiVersionResolver implements ApiVersionResolver {
 
 	private final int pathSegmentIndex;
-	private @Nullable Predicate<RequestPath> includePath = null;
+
+	private @Nullable Predicate<RequestPath> versionPathPredicate;
 
 
 	/**
@@ -55,13 +59,13 @@ public class PathApiVersionResolver implements ApiVersionResolver {
 	}
 
 	/**
-	 * Create a resolver instance.
-	 * @param pathSegmentIndex the index of the path segment that contains the API version
-	 * @param includePath a {@link Predicate} that tests if the given path should be included
+	 * Constructor variant of {@link #PathApiVersionResolver(int)} with an
+	 * additional {@code Predicate<RequestPath>} to help determine whether
+	 * a given path is versioned (true) or not (false).
 	 */
-	public PathApiVersionResolver(int pathSegmentIndex, Predicate<RequestPath> includePath) {
+	public PathApiVersionResolver(int pathSegmentIndex, Predicate<RequestPath> versionPathPredicate) {
 		this(pathSegmentIndex);
-		this.includePath = includePath;
+		this.versionPathPredicate = versionPathPredicate;
 	}
 
 
@@ -69,7 +73,7 @@ public class PathApiVersionResolver implements ApiVersionResolver {
 	public @Nullable String resolveVersion(ServerWebExchange exchange) {
 		int i = 0;
 		RequestPath path = exchange.getRequest().getPath();
-		if (this.includePath != null && !this.includePath.test(path)) {
+		if (!isVersionedPath(path)) {
 			return null;
 		}
 		for (PathContainer.Element e : path.pathWithinApplication().elements()) {
@@ -80,4 +84,8 @@ public class PathApiVersionResolver implements ApiVersionResolver {
 		throw new InvalidApiVersionException("No path segment at index " + this.pathSegmentIndex);
 	}
 
+	private boolean isVersionedPath(RequestPath path) {
+		return (this.versionPathPredicate == null || this.versionPathPredicate.test(path));
+	}
+
 }

spring-webflux/src/main/java/org/springframework/web/reactive/config/ApiVersionConfigurer.java

@@ -110,16 +110,17 @@ public class ApiVersionConfigurer {
 	}
 
 	/**
-	 * Add a resolver that extracts the API version from a path segment
-	 * and that allows to include only certain paths based on the provided {@link Predicate}.
-	 * <p>Note that this resolver never returns {@code null}, and therefore
-	 * cannot yield to other resolvers, see {@link org.springframework.web.accept.PathApiVersionResolver}.
+	 * Variant of {@link #usePathSegment(int)} with a {@code Predicate<RequestPath>}
+	 * to determine whether a given path is versioned, providing additional
+	 * flexibility, and the option to resolve the version to {@code null}.
 	 * @param index the index of the path segment to check; e.g. for URL's like
 	 * {@code "/{version}/..."} use index 0, for {@code "/api/{version}/..."} index 1.
-	 * @param includePath a {@link Predicate} that allows to include a certain path
+	 * @param versionPathPredicate used to decide if a path is versioned (true)
+	 * or not (false).
+	 * @since 7.0.6
 	 */
-	public ApiVersionConfigurer usePathSegment(int index, Predicate<RequestPath> includePath) {
-		this.versionResolvers.add(new PathApiVersionResolver(index, includePath));
+	public ApiVersionConfigurer usePathSegment(int index, Predicate<RequestPath> versionPathPredicate) {
+		this.versionResolvers.add(new PathApiVersionResolver(index, versionPathPredicate));
 		return this;
 	}
 

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

@@ -17,10 +17,12 @@
 package org.springframework.web.reactive.accept;
 
 import java.util.List;
+import java.util.function.Predicate;
 
 import org.junit.jupiter.api.Test;
 
 import org.springframework.http.server.PathContainer;
+import org.springframework.http.server.RequestPath;
 import org.springframework.web.accept.InvalidApiVersionException;
 import org.springframework.web.server.ServerWebExchange;
 import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest;
@@ -48,42 +50,22 @@ public class PathApiVersionResolverTests {
 	}
 
 	@Test
-	void includePathFalse() {
-		String requestUri = "/v3/api-docs";
-		testResolveWithIncludePath(requestUri, null);
+	void resolveWithVersionPathPredicate() {
+		testVersionPathPredicate("/app/1.0/path", "1.0");
+		testVersionPathPredicate("/app", null);
+		testVersionPathPredicate("/v3/api-docs", null);
 	}
 
-	@Test
-	void includePathTrue() {
-		String requestUri = "/app/1.0/path";
-		testResolveWithIncludePath(requestUri, "1.0");
-	}
-
-	@Test
-	void includePathFalseShortPath() {
-		String requestUri = "/app";
-		testResolveWithIncludePath(requestUri, null);
-	}
-
-	@Test
-	void includePathInsufficientPathSegments() {
-		ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/too-short"));
-		assertThatThrownBy(() -> new PathApiVersionResolver(1, requestPath -> true).resolveVersion(exchange))
-				.isInstanceOf(InvalidApiVersionException.class);
-	}
-
-	private static void testResolveWithIncludePath(String requestUri, String expected) {
-		ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get(requestUri));
-		String actual = new PathApiVersionResolver(1, requestPath -> {
-			List<PathContainer.Element> elements = requestPath.elements();
-			if (elements.size() < 4) {
-				return false;
-			}
-			return elements.get(0).value().equals("/") &&
+	private static void testVersionPathPredicate(String requestUri, String expected) {
+		Predicate<RequestPath> versionPathPredicate = path -> {
+			List<PathContainer.Element> elements = path.elements();
+			return (elements.size() > 3 &&
 					elements.get(1).value().equals("app") &&
-					elements.get(2).value().equals("/") &&
-					elements.get(3).value().equals("1.0");
-		}).resolveVersion(exchange);
+					elements.get(3).value().matches("\\d+\\.\\d+"));
+		};
+		ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get(requestUri));
+		PathApiVersionResolver resolver = new PathApiVersionResolver(1, versionPathPredicate);
+		String actual = resolver.resolveVersion(exchange);
 		assertThat(actual).isEqualTo(expected);
 	}
 

spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ApiVersionConfigurer.java

@@ -110,16 +110,17 @@ public class ApiVersionConfigurer {
 	}
 
 	/**
-	 * Add a resolver that extracts the API version from a path segment
-	 * and that allows to include only certain paths based on the provided {@link Predicate}.
-	 * <p>Note that this resolver never returns {@code null}, and therefore
-	 * cannot yield to other resolvers, see {@link org.springframework.web.accept.PathApiVersionResolver}.
+	 * Variant of {@link #usePathSegment(int)} with a {@code Predicate<RequestPath>}
+	 * to determine whether a given path is versioned, providing additional
+	 * flexibility, and the option to resolve the version to {@code null}.
 	 * @param index the index of the path segment to check; e.g. for URL's like
 	 * {@code "/{version}/..."} use index 0, for {@code "/api/{version}/..."} index 1.
-	 * @param includePath a {@link Predicate} that allows to include a certain path
+	 * @param versionPathPredicate used to decide if a path is versioned (true)
+	 * or not (false).
+	 * @since 7.0.6
 	 */
-	public ApiVersionConfigurer usePathSegment(int index, Predicate<RequestPath> includePath) {
-		this.versionResolvers.add(new PathApiVersionResolver(index, includePath));
+	public ApiVersionConfigurer usePathSegment(int index, Predicate<RequestPath> versionPathPredicate) {
+		this.versionResolvers.add(new PathApiVersionResolver(index, versionPathPredicate));
 		return this;
 	}