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

GenericApplicationContext does not honor ProtocolResolver when a resource loader is set via setResourceLoader() #28703

Closed
arteymix opened this issue Jun 24, 2022 · 3 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Milestone

Comments

@arteymix
Copy link

I'm trying to intercept certain URL patterns to resolve into the correct UrlResource, but the application context does not honour custom protocol resolvers.

In pseudocode:

GenericApplicationContext applicationContext = new GenericApplicationContext();
applicationContext.setResourceLoader(someResourceLoader);
applicationContext.addProtocolResolver(new CustomUrlResolver());
applicationContext.getResource("http://some.custom.url/"); // never resolved via CustomUrlResolver

The CustomUrlResolver resolver is never invoked because someResourceLoader takes precedence in getResource. Unfortunately, there's no way to work around this issue by adding a delegating resource loader, since there's no accessor for the resource loader.

I believe that the setResourceLoader(...) call is done somewhere in Spring Boot.

I had to do setResourceLoader(null). This is likely going to break things.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 24, 2022
arteymix added a commit to PavlidisLab/rdp that referenced this issue Jun 24, 2022
…context (fix #187)

Unfortunately, a workaround is needed so that GenericApplicationContext
honours the custom resolvers.

Related: spring-projects/spring-framework#28703
@sbrannen sbrannen changed the title GenericApplicationContext does not honour protocol resolvers when a resource loader is set via setResourceLoader() GenericApplicationContext does not honor ProtocolResolver when a resource loader is set via setResourceLoader() Jun 24, 2022
@sbrannen sbrannen added type: bug A general bug in: core Issues in core modules (aop, beans, core, context, expression) labels Jun 24, 2022
@sbrannen
Copy link
Member

Thanks for bringing this to our attention.

At a glance, it appears that a solution might be as simple as the following in GenericApplicationContext (i.e., copying the for-loop from the superclass -- although the for-loop would get executed twice in some cases).

public Resource getResource(String location) {
	for (ProtocolResolver protocolResolver : getProtocolResolvers()) {
		Resource resource = protocolResolver.resolve(location, this);
		if (resource != null) {
			return resource;
		}
	}
	if (this.resourceLoader != null) {
		return this.resourceLoader.getResource(location);
	}
	return super.getResource(location);
}

We'll look into it.

@sbrannen sbrannen removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 24, 2022
@sbrannen sbrannen self-assigned this Jun 24, 2022
@sbrannen sbrannen added this to the 5.3.22 milestone Jun 24, 2022
@arteymix
Copy link
Author

In that case, you can put the for-loop within the this.resourceLoader != null condition.

@sbrannen
Copy link
Member

In that case, you can put the for-loop within the this.resourceLoader != null condition.

Yes. You are of course correct, and that's exactly how I implemented it. 👍

When I said "at a glance", I meant that literally. It was the last thing I glanced at before signing off for the day, and I hadn't even put any thought into how to optimize the code. 😇

This is in place now for 5.3.22 (available soon in snapshots).

sbrannen pushed a commit to sbrannen/spring-framework that referenced this issue Jul 3, 2022
The tests introduced in commit 9868c28 pass on Mac OS and Linux but
fail on Microsoft Windows.

This commit updates the tests so that they pass on MS Windows as well.

See spring-projectsgh-28703
Closes spring-projectsgh-28746
sbrannen added a commit to sbrannen/spring-framework that referenced this issue Jul 3, 2022
This commit updates the tests so that they test something meaningful on
MS Windows as well as on Linux/Mac.

See spring-projectsgh-28703, spring-projectsgh-28746
sbrannen added a commit to sbrannen/spring-framework that referenced this issue Jul 3, 2022
The previous change to the tests resulted in a failure on Windows when
using the DefaultResourceLoader by expecting an exception when no
exception is thrown.

This commit narrows the scope of the if-clause to expect an exception
only when using the FileSystemResourceLoader on Windows.

See spring-projectsgh-28703, spring-projectsgh-28746
arteymix added a commit to PavlidisLab/rdp that referenced this issue Jul 4, 2022
…context (fix #187)

Unfortunately, a workaround is needed so that GenericApplicationContext
honours the custom resolvers.

Related: spring-projects/spring-framework#28703
arteymix added a commit to PavlidisLab/rdp that referenced this issue Jul 4, 2022
…context (fix #187)

Unfortunately, a workaround is needed so that GenericApplicationContext
honours the custom resolvers.

Related: spring-projects/spring-framework#28703
arteymix added a commit to PavlidisLab/rdp that referenced this issue Jul 5, 2022
…context (fix #187)

Unfortunately, a workaround is needed so that GenericApplicationContext
honours the custom resolvers.

Related: spring-projects/spring-framework#28703
arteymix added a commit to PavlidisLab/rdp that referenced this issue Jul 10, 2022
…context (fix #187)

Unfortunately, a workaround is needed so that GenericApplicationContext
honours the custom resolvers.

Related: spring-projects/spring-framework#28703
arteymix added a commit to PavlidisLab/rdp that referenced this issue Jul 10, 2022
…context (fix #187)

Unfortunately, a workaround is needed so that GenericApplicationContext
honours the custom resolvers.

Related: spring-projects/spring-framework#28703
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) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants