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

Introduce registerResource(Resource) in ResourceHints #29083

Conversation

sbrannen
Copy link
Member

@sbrannen sbrannen commented Sep 5, 2022

This PR simplifies the registration of hints for classpath: resources.

@sbrannen sbrannen added 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 5, 2022
@sbrannen sbrannen marked this pull request as draft September 5, 2022 15:10
@sbrannen sbrannen requested a review from snicoll September 5, 2022 15:10
Copy link
Member

@snicoll snicoll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the proposal. I've added a few comments.

@sbrannen sbrannen changed the title Introduce RuntimeHintsUtils.registerResource Introduce RuntimeHintsUtils.registerResourceIfNecessary Sep 6, 2022
@sbrannen sbrannen closed this in 28c492c Sep 6, 2022
@sbrannen sbrannen self-assigned this Sep 6, 2022
@sbrannen sbrannen added this to the 6.0.0-M6 milestone Sep 6, 2022
@sbrannen sbrannen added in: test Issues in the test module in: core Issues in core modules (aop, beans, core, context, expression) and removed in: core Issues in core modules (aop, beans, core, context, expression) labels Sep 6, 2022
@@ -92,4 +97,25 @@ public static void registerAnnotationIfNecessary(RuntimeHints hints, MergedAnnot
}
}

/**
* Register that the supplied resource should be made available at runtime.
* <p>If the supplied resource is not a {@link ClassPathResource}, it will
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Or does not exist"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current Javadoc reads like this:

Determine if the supplied resource is a ClassPathResource that exists and register the resource for run-time availability accordingly.

sbrannen added a commit that referenced this pull request Sep 6, 2022
@sbrannen sbrannen changed the title Introduce RuntimeHintsUtils.registerResourceIfNecessary Introduce registerResourceIfNecessary(Resource) in ResourceHints Sep 6, 2022
sbrannen added a commit to sbrannen/spring-framework that referenced this pull request Sep 6, 2022
Since getPath() returns a relative path if the resource was created
using the ClassPathResource(String,Class) constructor, there was
previously no way to consistently obtain the absolute path to the
resource within the class path.

This commit addresses this shortcoming by introducing a new
getAbsolutePath() for consistently obtaining the absolute path to the
resource within the class path.

See spring-projectsgh-29083
Closes spring-projectsgh-29094
@sbrannen
Copy link
Member Author

Reopening to switch back to the original method name and semantics.

@sbrannen sbrannen reopened this Sep 10, 2022
@sbrannen sbrannen closed this in 6d83a95 Sep 10, 2022
@snicoll snicoll changed the title Introduce registerResourceIfNecessary(Resource) in ResourceHints Introduce registerResource(Resource) in ResourceHints Sep 10, 2022
sbrannen added a commit that referenced this pull request Sep 11, 2022
This commit throws an exception in registerResource() if the supplied
resource is not a ClassPathResource.

See gh-29083
sbrannen added a commit to sbrannen/spring-framework that referenced this pull request Sep 12, 2022
Prior to this commit, the Javadoc for the getPath() method in
ClassPathResource stated the following.

> Return the path for this resource (as resource path within the class path).

That implied the returned path was an "absolute path" within the class
path; however, that was not always true.

If the resource was created using ClassPathResource(String) or
ClassPathResource(String, ClassLoader), the returned path was a cleaned
version of the ABSOLUTE PATH supplied to the constructor, WITHOUT a
leading slash.

If the resource was created using ClassPathResource(String, Class) with
an absolute path, the returned path was a cleaned version of the
ABSOLUTE PATH supplied to the constructor, WITH a leading slash.

If the resource was created using ClassPathResource(String, Class) with
a relative path, the returned path was a cleaned version of the
RELATIVE PATH supplied to the constructor.

In addition, ClassPathResource does not provide public access the Class
passed to the ClassPathResource(String, Class) constructor.

Consequently, the path returned by getPath() could not be reliably used
with ClassLoader.getResource(String) or with the recently introduced
registerResource(Resource) method in ResourceHints.

This commit addresses this issue by ensuring that getPath()
consistently returns the absolute path within the class path without a
leading slash.

See spring-projectsgh-29083
Reverts spring-projectsgh-29094
Closes spring-projectsgh-29099
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

Successfully merging this pull request may close these issues.

2 participants