Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support registration of resource hints for classpath location patterns #31162

Open
1 task
sbrannen opened this issue Sep 3, 2023 · 3 comments
Open
1 task
Labels
in: core Issues in core modules (aop, beans, core, context, expression) in: test Issues in the test module theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement

Comments

@sbrannen
Copy link
Member

sbrannen commented Sep 3, 2023

Overview

Prior to 6.1 M4 we did not have a need for registering resource hints for GraalVM native image for classpath location patterns (i.e., locations containing wildcards (**, *, or ?) or using the classpath*: prefix). Rather, we only had a concrete need to support static classpath locations.

However, since the introduction of support for location patterns in @PropertySource and @TestPropertySource, we now need to be able to register resource hints for patterns such as classpath*:my/config/**/file?.properties.

The Challenge

Class-path and module-path scanning uses PathMatchingResourcePatternResolver behind the scenes. To resolve the actual resources for a location pattern, we have to use that ResourcePatternResolver which returns resource types such as FileSystemResource and UrlResource instead of ClassPathResource, and it is not possible to easily infer the "path within the classpath" for a Resource type other than ClassPathResource.

For example, if we supply classpath*:**/aot/samples/basic/test?.yaml as the location pattern to @TestPropertySource and use PathMatchingResourcePatternResolver to resolve the actual locations, we end up with filesystem-based paths such as the following.

  • /Users/example/spring-framework/spring-test/bin/test/org/springframework/test/context/aot/samples/basic/test1.yaml
  • /Users/example/spring-framework/spring-test/bin/test/org/springframework/test/context/aot/samples/basic/test2.yaml

Whereas, what we actually need (in order to register resource hints) are the following paths within the classpath.

  • org/springframework/test/context/aot/samples/basic/test1.yaml
  • org/springframework/test/context/aot/samples/basic/test2.yaml

Brainstorming

One option is to use PathMatchingResourcePatternResolver to resolve the actual locations and then infer the "path within the classpath" by implementing logic specific to each resource type (e.g., FileSystemResource and UrlResource). I spiked an implementation that supports FileSystemResource by iterating over all classpath roots for the current JVM to find a given classpath root that is the base directory for the given FileSystemResource and then extracting the "path within the classpath" by removing the base directory.

That approach could work (and does work in my local tests); however, it does not take into account JARs in the classpath, URLClassLoader inspection, JAR manifests, etc. -- features that PathMatchingResourcePatternResolver supports while resolving the resources. In addition, it requires duplication of much of the logic in PathMatchingResourcePatternResolver.

Another option would be to introduce first-class support for registering an Ant-style classpath pattern in org.springframework.aot.hint.ResourceHints that transparently converts Ant-style wildcards into an analogous regular expression.

If we don't implement first-class support for Ant-style patterns and don't want to go with the PathMatchingResourcePatternResolver option (which I am currently not in favor of due to the aforementioned drawbacks), we could introduce a utility that converts from Ant-style patterns to our simplistic "Spring resource hint patterns" (i.e., only supports *) as follows.

private String convertAntPatternToHintPattern(String pattern) {
    pattern = pattern.replace('?', '*');              // ?     --> *
    pattern = pattern.replaceAll("(\\*)+", "\\*");    // (*)+  --> *
    pattern = pattern.replaceAll("(/\\*)+", "/\\*");  // (/*)+ --> /*
    pattern = pattern.replaceAll("(/\\*/)+", "/\\*"); // /*/   --> /*
    return pattern;
}

Passing the result of that function to ResourceHints#registerPattern works in my local test. However, that results in a lossy conversion from an Ant-style pattern TO a "Spring resource hint pattern" TO a regular expression (which is what is actually used in the JSON resource configuration file for GraalVM native image). Consequently, I am not in favor of introducing this utility: I've only included it here for the sake of completeness.

Related Issues

⚠️ The following issues are currently blocked by this issue.

Deliverables

  • Introduce a mechanism to support the registration of resource hints for classpath location patterns.
@sbrannen sbrannen added in: test Issues in the test module in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement theme: aot An issue related to Ahead-of-time processing labels Sep 3, 2023
@sbrannen sbrannen added this to the 6.1.0-RC1 milestone Sep 3, 2023
@bclozel
Copy link
Member

bclozel commented Sep 5, 2023

A related topic came up during a meeting with the GraalVM team. Right now the resource configuration format in GraalVM accepts regexps, but they will restrict that in the future to only accept a subset - something like "org/springframework/test/context/aot/samples/basic/*" and "*.yaml". The actual behavior is not defined yet.

This will need to happen because the runtime will start enforcing missing reflection and resource entries as exceptions. If you didn't register for it, you will get an exception at runtime if check for that resource. Doing that with extensive regexps is not a good fit at runtime.

@sbrannen
Copy link
Member Author

sbrannen commented Sep 6, 2023

they will restrict that in the future to only accept a subset

Thanks for the heads-up, @bclozel. That could certainly influence our decision.

@sbrannen sbrannen modified the milestones: 6.1.0-M5, 6.1.0-RC1 Sep 11, 2023
@sbrannen sbrannen modified the milestones: 6.1.0-RC1, 6.1.0-RC2 Sep 26, 2023
@sbrannen sbrannen modified the milestones: 6.1.0-RC2, 6.1.x Oct 14, 2023
sbrannen added a commit that referenced this issue Oct 25, 2023
Since we do not yet have support for registering resource hints for
classpath location patterns, we have decided to explicitly skip such
resources and log a warning to inform users that they need to manually
supply resource hints for the exact resources needed by their
application.

This commit applies this change for @⁠PropertySource and
@⁠TestPropertySource.

See gh-31162
Closes gh-31429
@bclozel bclozel modified the milestones: 6.1.x, 6.2.x Dec 18, 2023
@sbrannen sbrannen modified the milestones: 6.2.x, 6.x Backlog Dec 22, 2023
@sbrannen
Copy link
Member Author

Update

The GraalVM team has merged support to "restrict the Expressivity of Patterns in resource-config.json" in PR oracle/graal#8715 for inclusion in GraalVM for JDK 23 (September 17, 2024).

@jhoeller jhoeller modified the milestones: 6.x Backlog, General Backlog Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) in: test Issues in the test module theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants