From 47b1a2bc55220d6ce274b5ffc5d6a193afbfe838 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Fri, 18 Aug 2023 15:35:03 +0200 Subject: [PATCH] Clarify usage of FilePatternResourceHintsRegistrar This commit review the API using a builder to make it more clear what the registrar does. Closes gh-29161 --- .../FilePatternResourceHintsRegistrar.java | 166 +++++++++++++----- ...ilePatternResourceHintsRegistrarTests.java | 70 +++++--- 2 files changed, 166 insertions(+), 70 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/aot/hint/support/FilePatternResourceHintsRegistrar.java b/spring-core/src/main/java/org/springframework/aot/hint/support/FilePatternResourceHintsRegistrar.java index ca02b4a17e9d..50d9ace64e3c 100644 --- a/spring-core/src/main/java/org/springframework/aot/hint/support/FilePatternResourceHintsRegistrar.java +++ b/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.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. * - *

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. + *

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 names, List locations, List 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 validateNames(List names) { - for (String name : names) { - if (name.contains("*")) { - throw new IllegalArgumentException("File name '" + name + "' cannot contain '*'"); - } - } - return names; - } - - private static List validateLocations(List locations) { - Assert.notEmpty(locations, "At least one location should be specified"); - List 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 validateExtensions(List 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 includes = new ArrayList<>(); @@ -108,4 +88,106 @@ public void registerHints(ResourceHints hints, @Nullable ClassLoader classLoader hints.registerPattern(hint -> hint.includes(includes.toArray(String[]::new))); } } + + /** + * Builder for {@link FilePatternResourceHintsRegistrar}. + */ + public static final class Builder { + + private final List classpathLocations = new ArrayList<>(); + + private final List filePrefixes = new ArrayList<>(); + + private final List 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 validateClasspathLocations(String... locations) { + List 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 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 validateFileExtensions(String... fileExtensions) { + for (String fileExtension : fileExtensions) { + if (!fileExtension.startsWith(".")) { + throw new IllegalArgumentException("Extension '" + fileExtension + "' should start with '.'"); + } + } + return Arrays.asList(fileExtensions); + } + + } } diff --git a/spring-core/src/test/java/org/springframework/aot/hint/support/FilePatternResourceHintsRegistrarTests.java b/spring-core/src/test/java/org/springframework/aot/hint/support/FilePatternResourceHintsRegistrarTests.java index 15906820e88f..7d7c6f127a3c 100644 --- a/spring-core/src/test/java/org/springframework/aot/hint/support/FilePatternResourceHintsRegistrarTests.java +++ b/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.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 @@ void registerWithLocationUsingResourceClasspathPrefixAndTrailingSlash() { @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 includes(String... patterns) { return hint -> { assertThat(hint.getIncludes().stream().map(ResourcePatternHint::getPattern))