Clarify usage of FilePatternResourceHintsRegistrar

This commit review the API using a builder to make it more clear what
the registrar does.

Closes gh-29161

spring-core/src/main/java/org/springframework/aot/hint/support/FilePatternResourceHintsRegistrar.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2022 the original author or authors.
+ * Copyright 2002-2023 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.
@@ -17,6 +17,7 @@
 package org.springframework.aot.hint.support;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 import org.springframework.aot.hint.ResourceHints;
@@ -25,10 +26,12 @@ import org.springframework.util.Assert;
 import org.springframework.util.ResourceUtils;
 
 /**
- * Register the necessary resource hints for loading files from the classpath.
+ * Register the necessary resource hints for loading files from the classpath,
+ * based on a file name prefix and an extension with convenience to support
+ * multiple classpath locations.
  *
- * <p>Candidates are identified by a file name, a location, and an extension.
- * The location can be the empty string to refer to the root of the classpath.
+ * <p>Only register hints for matching classpath locations, which allows for
+ * several locations to be provided without contributing unnecessary hints.
  *
  * @author Stephane Nicoll
  * @since 6.0
@@ -47,51 +50,28 @@ public class FilePatternResourceHintsRegistrar {
 	 * @param names the file names
 	 * @param locations the classpath locations
 	 * @param extensions the file extensions (starts with a dot)
+	 * @deprecated as of 6.0.12 in favor of {@link #forClassPathLocations(String...) the builder}
 	 */
+	@Deprecated(since = "6.0.12", forRemoval = true)
 	public FilePatternResourceHintsRegistrar(List<String> names, List<String> locations,
 			List<String> extensions) {
-		this.names = validateNames(names);
-		this.locations = validateLocations(locations);
-		this.extensions = validateExtensions(extensions);
+		this.names = Builder.validateFilePrefixes(names.toArray(String[]::new));
+		this.locations = Builder.validateClasspathLocations(locations.toArray(String[]::new));
+		this.extensions = Builder.validateFileExtensions(extensions.toArray(String[]::new));
 	}
 
-	private static List<String> validateNames(List<String> names) {
-		for (String name : names) {
-			if (name.contains("*")) {
-				throw new IllegalArgumentException("File name '" + name + "' cannot contain '*'");
-			}
-		}
-		return names;
-	}
-
-	private static List<String> validateLocations(List<String> locations) {
-		Assert.notEmpty(locations, "At least one location should be specified");
-		List<String> parsedLocations = new ArrayList<>();
-		for (String location : locations) {
-			if (location.startsWith(ResourceUtils.CLASSPATH_URL_PREFIX)) {
-				location = location.substring(ResourceUtils.CLASSPATH_URL_PREFIX.length());
-			}
-			if (location.startsWith("/")) {
-				location = location.substring(1);
-			}
-			if (!location.isEmpty() && !location.endsWith("/")) {
-				location = location + "/";
-			}
-			parsedLocations.add(location);
-		}
-		return parsedLocations;
-
-	}
-
-	private static List<String> validateExtensions(List<String> extensions) {
-		for (String extension : extensions) {
-			if (!extension.startsWith(".")) {
-				throw new IllegalArgumentException("Extension '" + extension + "' should start with '.'");
-			}
-		}
-		return extensions;
+	/**
+	 * Configure the registrar with the specified
+	 * {@linkplain Builder#withClasspathLocations(String...) classpath locations}.
+	 * @param locations the classpath locations
+	 * @return a {@link Builder} to further configure the registrar
+	 */
+	public static Builder forClassPathLocations(String... locations) {
+		Assert.notEmpty(locations, "At least one classpath location should be specified");
+		return new Builder().withClasspathLocations(locations);
 	}
 
+	@Deprecated(since = "6.0.12", forRemoval = true)
 	public void registerHints(ResourceHints hints, @Nullable ClassLoader classLoader) {
 		ClassLoader classLoaderToUse = (classLoader != null) ? classLoader : getClass().getClassLoader();
 		List<String> includes = new ArrayList<>();
@@ -108,4 +88,106 @@ public class FilePatternResourceHintsRegistrar {
 			hints.registerPattern(hint -> hint.includes(includes.toArray(String[]::new)));
 		}
 	}
+
+	/**
+	 * Builder for {@link FilePatternResourceHintsRegistrar}.
+	 */
+	public static final class Builder {
+
+		private final List<String> classpathLocations = new ArrayList<>();
+
+		private final List<String> filePrefixes = new ArrayList<>();
+
+		private final List<String> fileExtensions = new ArrayList<>();
+
+
+		/**
+		 * Consider the specified classpath locations. A location can either be
+		 * a special "classpath" pseudo location or a standard location, such as
+		 * {@code com/example/resources}. An empty String represents the root of
+		 * the classpath.
+		 * @param classpathLocations the classpath locations to consider
+		 * @return this builder
+		 */
+		public Builder withClasspathLocations(String... classpathLocations) {
+			this.classpathLocations.addAll(validateClasspathLocations(classpathLocations));
+			return this;
+		}
+
+		/**
+		 * Consider the specified file prefixes. Any file whose name starts with one
+		 * of the specified prefix is considered. A prefix cannot contain the {@code *}
+		 * character.
+		 * @param filePrefixes the file prefixes to consider
+		 * @return this builder
+		 */
+		public Builder withFilePrefixes(String... filePrefixes) {
+			this.filePrefixes.addAll(validateFilePrefixes(filePrefixes));
+			return this;
+		}
+
+		/**
+		 * Consider the specified file extensions. A file extension must starts with a
+		 * {@code .} character..
+		 * @param fileExtensions the file extensions to consider
+		 * @return this builder
+		 */
+		public Builder withFileExtensions(String... fileExtensions) {
+			this.fileExtensions.addAll(validateFileExtensions(fileExtensions));
+			return this;
+		}
+
+		FilePatternResourceHintsRegistrar build() {
+			Assert.notEmpty(this.classpathLocations, "At least one location should be specified");
+			return new FilePatternResourceHintsRegistrar(this.filePrefixes,
+					this.classpathLocations, this.fileExtensions);
+		}
+
+		/**
+		 * Register resource hints for the current state of this builder. For each
+		 * classpath location that resolves against the {@code ClassLoader}, file
+		 * starting with the configured file prefixes and extensions are registered.
+		 * @param hints the hints contributed so far for the deployment unit
+		 * @param classLoader the classloader, or {@code null} if even the system ClassLoader isn't accessible
+		 */
+		public void registerHints(ResourceHints hints, @Nullable ClassLoader classLoader) {
+			build().registerHints(hints, classLoader);
+		}
+
+		private static List<String> validateClasspathLocations(String... locations) {
+			List<String> parsedLocations = new ArrayList<>();
+			for (String location : locations) {
+				if (location.startsWith(ResourceUtils.CLASSPATH_URL_PREFIX)) {
+					location = location.substring(ResourceUtils.CLASSPATH_URL_PREFIX.length());
+				}
+				if (location.startsWith("/")) {
+					location = location.substring(1);
+				}
+				if (!location.isEmpty() && !location.endsWith("/")) {
+					location = location + "/";
+				}
+				parsedLocations.add(location);
+			}
+			return parsedLocations;
+		}
+
+		private static List<String> validateFilePrefixes(String... fileNames) {
+			for (String name : fileNames) {
+				if (name.contains("*")) {
+					throw new IllegalArgumentException("File prefix '" + name + "' cannot contain '*'");
+				}
+			}
+			return Arrays.asList(fileNames);
+		}
+
+		private static List<String> validateFileExtensions(String... fileExtensions) {
+			for (String fileExtension : fileExtensions) {
+				if (!fileExtension.startsWith(".")) {
+					throw new IllegalArgumentException("Extension '" + fileExtension + "' should start with '.'");
+				}
+			}
+			return Arrays.asList(fileExtensions);
+		}
+
+	}
 }

spring-core/src/test/java/org/springframework/aot/hint/support/FilePatternResourceHintsRegistrarTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2022 the original author or authors.
+ * Copyright 2002-2023 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.
@@ -16,7 +16,6 @@
 
 package org.springframework.aot.hint.support;
 
-import java.util.List;
 import java.util.function.Consumer;
 
 import org.junit.jupiter.api.Test;
@@ -24,6 +23,7 @@ import org.junit.jupiter.api.Test;
 import org.springframework.aot.hint.ResourceHints;
 import org.springframework.aot.hint.ResourcePatternHint;
 import org.springframework.aot.hint.ResourcePatternHints;
+import org.springframework.aot.hint.support.FilePatternResourceHintsRegistrar.Builder;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
@@ -37,80 +37,94 @@ class FilePatternResourceHintsRegistrarTests {
 
 	private final ResourceHints hints = new ResourceHints();
 
+	@Test
+	void configureWithNoClasspathLocation() {
+		assertThatIllegalArgumentException().isThrownBy(FilePatternResourceHintsRegistrar::forClassPathLocations)
+				.withMessageContaining("At least one classpath location should be specified");
+	}
 
 	@Test
-	void createWithInvalidName() {
-		assertThatIllegalArgumentException().isThrownBy(() -> new FilePatternResourceHintsRegistrar(
-						List.of("test*"), List.of(""), List.of(".txt")))
+	void configureWithInvalidFilePrefix() {
+		Builder builder = FilePatternResourceHintsRegistrar.forClassPathLocations("");
+		assertThatIllegalArgumentException().isThrownBy(() -> builder.withFilePrefixes("test*"))
 				.withMessageContaining("cannot contain '*'");
 	}
 
 	@Test
-	void createWithInvalidExtension() {
-		assertThatIllegalArgumentException().isThrownBy(() -> new FilePatternResourceHintsRegistrar(
-						List.of("test"), List.of(""), List.of("txt")))
+	void configureWithInvalidFileExtension() {
+		Builder builder = FilePatternResourceHintsRegistrar.forClassPathLocations("");
+		assertThatIllegalArgumentException().isThrownBy(() -> builder.withFileExtensions("txt"))
 				.withMessageContaining("should start with '.'");
 	}
 
 	@Test
 	void registerWithSinglePattern() {
-		new FilePatternResourceHintsRegistrar(List.of("test"), List.of(""), List.of(".txt"))
+		FilePatternResourceHintsRegistrar.forClassPathLocations("")
+				.withFilePrefixes("test").withFileExtensions(".txt")
 				.registerHints(this.hints, null);
 		assertThat(this.hints.resourcePatternHints()).singleElement()
 				.satisfies(includes("/", "test*.txt"));
 	}
 
 	@Test
-	void registerWithMultipleNames() {
-		new FilePatternResourceHintsRegistrar(List.of("test", "another"), List.of(""), List.of(".txt"))
+	void registerWithMultipleFilePrefixes() {
+		FilePatternResourceHintsRegistrar.forClassPathLocations("")
+				.withFilePrefixes("test").withFilePrefixes("another")
+				.withFileExtensions(".txt")
 				.registerHints(this.hints, null);
 		assertThat(this.hints.resourcePatternHints()).singleElement()
-				.satisfies(includes("/" , "test*.txt", "another*.txt"));
+				.satisfies(includes("/", "test*.txt", "another*.txt"));
 	}
 
 	@Test
-	void registerWithMultipleLocations() {
-		new FilePatternResourceHintsRegistrar(List.of("test"), List.of("", "META-INF"), List.of(".txt"))
+	void registerWithMultipleClasspathLocations() {
+		FilePatternResourceHintsRegistrar.forClassPathLocations("").withClasspathLocations("META-INF")
+				.withFilePrefixes("test").withFileExtensions(".txt")
 				.registerHints(this.hints, null);
 		assertThat(this.hints.resourcePatternHints()).singleElement()
 				.satisfies(includes("/", "test*.txt", "META-INF", "META-INF/test*.txt"));
 	}
 
 	@Test
-	void registerWithMultipleExtensions() {
-		new FilePatternResourceHintsRegistrar(List.of("test"), List.of(""), List.of(".txt", ".conf"))
+	void registerWithMultipleFileExtensions() {
+		FilePatternResourceHintsRegistrar.forClassPathLocations("")
+				.withFilePrefixes("test").withFileExtensions(".txt").withFileExtensions(".conf")
 				.registerHints(this.hints, null);
 		assertThat(this.hints.resourcePatternHints()).singleElement()
 				.satisfies(includes("/", "test*.txt", "test*.conf"));
 	}
 
 	@Test
-	void registerWithLocationWithoutTrailingSlash() {
-		new FilePatternResourceHintsRegistrar(List.of("test"), List.of("META-INF"), List.of(".txt"))
+	void registerWithClasspathLocationWithoutTrailingSlash() {
+		FilePatternResourceHintsRegistrar.forClassPathLocations("META-INF")
+				.withFilePrefixes("test").withFileExtensions(".txt")
 				.registerHints(this.hints, null);
 		assertThat(this.hints.resourcePatternHints()).singleElement()
 				.satisfies(includes("/", "META-INF", "META-INF/test*.txt"));
 	}
 
 	@Test
-	void registerWithLocationWithLeadingSlash() {
-		new FilePatternResourceHintsRegistrar(List.of("test"), List.of("/"), List.of(".txt"))
+	void registerWithClasspathLocationWithLeadingSlash() {
+		FilePatternResourceHintsRegistrar.forClassPathLocations("/")
+				.withFilePrefixes("test").withFileExtensions(".txt")
 				.registerHints(this.hints, null);
 		assertThat(this.hints.resourcePatternHints()).singleElement()
 				.satisfies(includes("/", "test*.txt"));
 	}
 
 	@Test
-	void registerWithLocationUsingResourceClasspathPrefix() {
-		new FilePatternResourceHintsRegistrar(List.of("test"), List.of("classpath:META-INF"), List.of(".txt"))
+	void registerWithClasspathLocationUsingResourceClasspathPrefix() {
+		FilePatternResourceHintsRegistrar.forClassPathLocations("classpath:META-INF")
+				.withFilePrefixes("test").withFileExtensions(".txt")
 				.registerHints(this.hints, null);
 		assertThat(this.hints.resourcePatternHints()).singleElement()
 				.satisfies(includes("/", "META-INF", "META-INF/test*.txt"));
 	}
 
 	@Test
-	void registerWithLocationUsingResourceClasspathPrefixAndTrailingSlash() {
-		new FilePatternResourceHintsRegistrar(List.of("test"), List.of("classpath:/META-INF"), List.of(".txt"))
+	void registerWithClasspathLocationUsingResourceClasspathPrefixAndTrailingSlash() {
+		FilePatternResourceHintsRegistrar.forClassPathLocations("classpath:/META-INF")
+				.withFilePrefixes("test").withFileExtensions(".txt")
 				.registerHints(this.hints, null);
 		assertThat(this.hints.resourcePatternHints()).singleElement()
 				.satisfies(includes("/", "META-INF", "META-INF/test*.txt"));
@@ -118,13 +132,13 @@ class FilePatternResourceHintsRegistrarTests {
 
 	@Test
 	void registerWithNonExistingLocationDoesNotRegisterHint() {
-		new FilePatternResourceHintsRegistrar(List.of("test"),
-				List.of("does-not-exist/", "another-does-not-exist/"),
-				List.of(".txt")).registerHints(this.hints, null);
+		FilePatternResourceHintsRegistrar.forClassPathLocations("does-not-exist/")
+				.withClasspathLocations("another-does-not-exist/")
+				.withFilePrefixes("test").withFileExtensions(".txt")
+				.registerHints(this.hints, null);
 		assertThat(this.hints.resourcePatternHints()).isEmpty();
 	}
 
-
 	private Consumer<ResourcePatternHints> includes(String... patterns) {
 		return hint -> {
 			assertThat(hint.getIncludes().stream().map(ResourcePatternHint::getPattern))