From 97b98c33785aa7d216419814b5e7705d403f90bb Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 6 Sep 2022 11:53:09 +0200 Subject: [PATCH] Support default package for TypeReference in ResourceHintsPredicates Prior to this commit, if the TypeReference supplied to ResourceHintsPredicates.forResource(TypeReference,String) was for a class declared in the default package (i.e., without a package), the resolveAbsoluteResourceName() method incorrectly prepended two leading slashes (//) to the absolute resource name, causing correct matches to fail. This commit fixes this by adding special handling for a TypeReference without a package name. In addition, this commit introduces lenient handling of resource names by consistently removing a leading slash in ResourceHintsPredicates.forResource(*) methods. The latter aligns with absolute resource path handling in other places in the framework, such as ClassPathResource. Closes gh-29086 --- .../aot/agent/InstrumentedMethodTests.java | 9 +++-- .../predicate/ResourceHintsPredicates.java | 24 +++++++++--- .../ResourceHintsPredicatesTests.java | 39 ++++++++++++------- 3 files changed, 49 insertions(+), 23 deletions(-) diff --git a/spring-core-test/src/test/java/org/springframework/aot/agent/InstrumentedMethodTests.java b/spring-core-test/src/test/java/org/springframework/aot/agent/InstrumentedMethodTests.java index 60f100ea8a..e2839c46ad 100644 --- a/spring-core-test/src/test/java/org/springframework/aot/agent/InstrumentedMethodTests.java +++ b/spring-core-test/src/test/java/org/springframework/aot/agent/InstrumentedMethodTests.java @@ -36,6 +36,7 @@ import static org.assertj.core.api.Assertions.assertThat; * Tests for {@link InstrumentedMethod}. * * @author Brian Clozel + * @author Sam Brannen */ class InstrumentedMethodTests { @@ -556,7 +557,7 @@ class InstrumentedMethodTests { void classGetResourceShouldMatchResourcePatternWhenAbsolute() { RecordedInvocation invocation = RecordedInvocation.of(InstrumentedMethod.CLASS_GETRESOURCE) .onInstance(InstrumentedMethodTests.class).withArgument("/some/path/resource.txt").build(); - hints.resources().registerPattern("/some/*"); + hints.resources().registerPattern("some/*"); assertThatInvocationMatches(InstrumentedMethod.CLASS_GETRESOURCE, invocation); } @@ -564,7 +565,7 @@ class InstrumentedMethodTests { void classGetResourceShouldMatchResourcePatternWhenRelative() { RecordedInvocation invocation = RecordedInvocation.of(InstrumentedMethod.CLASS_GETRESOURCE) .onInstance(InstrumentedMethodTests.class).withArgument("resource.txt").build(); - hints.resources().registerPattern("/org/springframework/aot/agent/*"); + hints.resources().registerPattern("org/springframework/aot/agent/*"); assertThatInvocationMatches(InstrumentedMethod.CLASS_GETRESOURCE, invocation); } @@ -572,7 +573,7 @@ class InstrumentedMethodTests { void classGetResourceShouldNotMatchResourcePatternWhenInvalid() { RecordedInvocation invocation = RecordedInvocation.of(InstrumentedMethod.CLASS_GETRESOURCE) .onInstance(InstrumentedMethodTests.class).withArgument("/some/path/resource.txt").build(); - hints.resources().registerPattern("/other/*"); + hints.resources().registerPattern("other/*"); assertThatInvocationDoesNotMatch(InstrumentedMethod.CLASS_GETRESOURCE, invocation); } @@ -580,7 +581,7 @@ class InstrumentedMethodTests { void classGetResourceShouldNotMatchResourcePatternWhenExcluded() { RecordedInvocation invocation = RecordedInvocation.of(InstrumentedMethod.CLASS_GETRESOURCE) .onInstance(InstrumentedMethodTests.class).withArgument("/some/path/resource.txt").build(); - hints.resources().registerPattern(resourceHint -> resourceHint.includes("/some/*").excludes("/some/path/*")); + hints.resources().registerPattern(resourceHint -> resourceHint.includes("some/*").excludes("some/path/*")); assertThatInvocationDoesNotMatch(InstrumentedMethod.CLASS_GETRESOURCE, invocation); } diff --git a/spring-core/src/main/java/org/springframework/aot/hint/predicate/ResourceHintsPredicates.java b/spring-core/src/main/java/org/springframework/aot/hint/predicate/ResourceHintsPredicates.java index df93b6a058..8aa0060662 100644 --- a/spring-core/src/main/java/org/springframework/aot/hint/predicate/ResourceHintsPredicates.java +++ b/spring-core/src/main/java/org/springframework/aot/hint/predicate/ResourceHintsPredicates.java @@ -34,6 +34,7 @@ import org.springframework.util.ConcurrentLruCache; * * @author Brian Clozel * @author Stephane Nicoll + * @author Sam Brannen * @since 6.0 */ public class ResourceHintsPredicates { @@ -58,7 +59,11 @@ public class ResourceHintsPredicates { * Return a predicate that checks whether a resource hint is registered for the given * resource name, located in the given type's package. *

For example, {@code forResource(org.example.MyClass, "myResource.txt")} - * will match for {@code "/org/example/myResource.txt"}. + * will match against {@code "org/example/myResource.txt"}. + *

If the given resource name is an absolute path (i.e., starts with a + * leading slash), the supplied type will be ignored. For example, + * {@code forResource(org.example.MyClass, "/myResource.txt")} will match against + * {@code "myResource.txt"}. * @param type the type's package where to look for the resource * @param resourceName the resource name * @return the {@link RuntimeHints} predicate @@ -69,32 +74,39 @@ public class ResourceHintsPredicates { } private String resolveAbsoluteResourceName(TypeReference type, String resourceName) { + // absolute path if (resourceName.startsWith("/")) { + return resourceName.substring(1); + } + // default package + else if (type.getPackageName().isEmpty()) { return resourceName; } + // relative path else { - return "/" + type.getPackageName().replace('.', '/') - + "/" + resourceName; + return type.getPackageName().replace('.', '/') + "/" + resourceName; } } /** * Return a predicate that checks whether a resource hint is registered for * the given resource name. - * @param resourceName the full resource name + *

A leading slash will be removed. + * @param resourceName the absolute resource name * @return the {@link RuntimeHints} predicate */ public Predicate forResource(String resourceName) { + String resourceNameToUse = (resourceName.startsWith("/") ? resourceName.substring(1) : resourceName); return hints -> { AggregatedResourcePatternHints aggregatedResourcePatternHints = AggregatedResourcePatternHints.of( hints.resources()); boolean isExcluded = aggregatedResourcePatternHints.excludes().stream().anyMatch(excluded -> - CACHED_RESOURCE_PATTERNS.get(excluded).matcher(resourceName).matches()); + CACHED_RESOURCE_PATTERNS.get(excluded).matcher(resourceNameToUse).matches()); if (isExcluded) { return false; } return aggregatedResourcePatternHints.includes().stream().anyMatch(included -> - CACHED_RESOURCE_PATTERNS.get(included).matcher(resourceName).matches()); + CACHED_RESOURCE_PATTERNS.get(included).matcher(resourceNameToUse).matches()); }; } diff --git a/spring-core/src/test/java/org/springframework/aot/hint/predicate/ResourceHintsPredicatesTests.java b/spring-core/src/test/java/org/springframework/aot/hint/predicate/ResourceHintsPredicatesTests.java index 20f98977ef..d7d5415d0f 100644 --- a/spring-core/src/test/java/org/springframework/aot/hint/predicate/ResourceHintsPredicatesTests.java +++ b/spring-core/src/test/java/org/springframework/aot/hint/predicate/ResourceHintsPredicatesTests.java @@ -18,7 +18,6 @@ package org.springframework.aot.hint.predicate; import java.util.function.Predicate; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.aot.hint.RuntimeHints; @@ -30,40 +29,54 @@ import static org.assertj.core.api.Assertions.assertThat; * Tests for {@link ReflectionHintsPredicates}. * * @author Brian Clozel + * @author Sam Brannen */ class ResourceHintsPredicatesTests { private final ResourceHintsPredicates resources = new ResourceHintsPredicates(); - private RuntimeHints runtimeHints; + private final RuntimeHints runtimeHints = new RuntimeHints(); - @BeforeEach - void setup() { - this.runtimeHints = new RuntimeHints(); - } - @Test void resourcePatternMatchesResourceName() { - this.runtimeHints.resources().registerPattern("/test/*"); + this.runtimeHints.resources().registerPattern("test/*"); assertPredicateMatches(resources.forResource("/test/spring.properties")); } @Test void resourcePatternDoesNotMatchResourceName() { - this.runtimeHints.resources().registerPattern("/test/spring.*"); + this.runtimeHints.resources().registerPattern("test/spring.*"); assertPredicateDoesNotMatch(resources.forResource("/test/other.properties")); } @Test void resourcePatternMatchesTypeAndResourceName() { - this.runtimeHints.resources().registerPattern("/org/springframework/aot/hint/predicate/spring.*"); + this.runtimeHints.resources().registerPattern("org/springframework/aot/hint/predicate/spring.*"); assertPredicateMatches(resources.forResource(TypeReference.of(getClass()), "spring.properties")); } + @Test + void resourcePatternMatchesTypeAndAbsoluteResourceName() { + this.runtimeHints.resources().registerPattern("spring.*"); + assertPredicateMatches(resources.forResource(TypeReference.of(getClass()), "/spring.properties")); + } + + @Test + void resourcePatternMatchesTypeInDefaultPackageAndResourceName() { + this.runtimeHints.resources().registerPattern("spring.*"); + assertPredicateMatches(resources.forResource(TypeReference.of("DummyClass"), "spring.properties")); + } + + @Test + void resourcePatternMatchesTypeInDefaultPackageAndAbsoluteResourceName() { + this.runtimeHints.resources().registerPattern("spring.*"); + assertPredicateMatches(resources.forResource(TypeReference.of("DummyClass"), "/spring.properties")); + } + @Test void resourcePatternDoesNotMatchTypeAndResourceName() { - this.runtimeHints.resources().registerPattern("/spring.*"); + this.runtimeHints.resources().registerPattern("spring.*"); assertPredicateDoesNotMatch(resources.forResource(TypeReference.of(getClass()), "spring.properties")); } @@ -81,11 +94,11 @@ class ResourceHintsPredicatesTests { private void assertPredicateMatches(Predicate predicate) { - assertThat(predicate.test(this.runtimeHints)).isTrue(); + assertThat(predicate).accepts(this.runtimeHints); } private void assertPredicateDoesNotMatch(Predicate predicate) { - assertThat(predicate.test(this.runtimeHints)).isFalse(); + assertThat(predicate).rejects(this.runtimeHints); } }