Spring-boot: DevTools overrides AppContext.resourceLoader causing custom protocol resolvers to be discarded

[JoostK]

Introduction

In SPR-12857 a feature was implemented for registering custom resource protocol resolvers on DefaultResourceLoader, including a setter on ConfigurableApplicationContext to expose this feature to application initializers. When enabling DevTools however a custom ResourceLoader is injected into the application context, hiding the custom protocol resolvers.

Analysis

The issue arrises from code in org.springframework.boot.devtools.restart.ClassLoaderFilesResourcePatternResolver.

Non-web Applications

For non-web applications, a PathMatchingResourcePatternResolver is used with a reference to either the ApplicationContext's resourceLoader, or a new DefaultResourceLoader if that is null:

    private static class ResourcePatternResolverFactory {

        public ResourcePatternResolver getResourcePatternResolver(
                ApplicationContext applicationContext, ResourceLoader resourceLoader) {
            return new PathMatchingResourcePatternResolver(resourceLoader == null
                    ? new DefaultResourceLoader() : resourceLoader);
        }

    }

Hence the protocol resolvers that are registered onto the ApplicationContext itself are no longer considered. I prepared a PR to change this but unfortunately, using applicationContext itself instead of a new DefaultResourceLoader fails the tests, as circular calls occur when requesting the ApplicationContext's classloader.

Web Applications

For web applications, a custom code path is followed. Again, when a ResourceLoader has been explicitly set onto the ApplicationContext it is used, otherwise a WebApplicationContextResourceLoader is created instead:

    private static class WebResourcePatternResolverFactory
            extends ResourcePatternResolverFactory {

        @Override
        public ResourcePatternResolver getResourcePatternResolver(
                ApplicationContext applicationContext, ResourceLoader resourceLoader) {
            if (applicationContext instanceof WebApplicationContext) {
                return new ServletContextResourcePatternResolver(resourceLoader == null
                        ? new WebApplicationContextResourceLoader(
                                (WebApplicationContext) applicationContext)
                        : resourceLoader);
            }
            return super.getResourcePatternResolver(applicationContext, resourceLoader);
        }

    }

This may seem to work as applicationContext is used this time, however it is only used for configuring a ServletContextResource:

    private static class WebApplicationContextResourceLoader
            extends DefaultResourceLoader {

        private final WebApplicationContext applicationContext;

        WebApplicationContextResourceLoader(WebApplicationContext applicationContext) {
            this.applicationContext = applicationContext;
        }

        @Override
        protected Resource getResourceByPath(String path) {
            if (this.applicationContext.getServletContext() != null) {
                return new ServletContextResource(
                        this.applicationContext.getServletContext(), path);
            }
            return super.getResourceByPath(path);
        }

    }

As can be seen, the above implementation returns a ServletContextResource without ever consulting the ApplicationContext's custom protocol resolvers. Somehow that feels incorrect, as its parent implementation may have returned an UrlResource as well, for example.

Workaround

I realize now that I can circumvent the problem by setting a new DefaultResourceLoader with the GenericApplicationContext (not ConfigurableApplicationContext unfortunately) from within the initializer and register my custom protocol resolver onto that, as then this resource loader is supposedly kept in the delegation chain:

public class ResourceLoaderInitializer implements ApplicationContextInitializer<GenericApplicationContext> {

    @Override
    public void initialize(GenericApplicationContext context) {
        DefaultResourceLoader resourceLoader = new DefaultResourceLoader();
        resourceLoader.addProtocolResolver(...);
        context.setResourceLoader(resourceLoader);
    }

}

This potentially introduces another problem however, in that an AbstractApplicationContext registers itself as ResourceLoader in the main BeanFactory and this registration is not reset when a custom resourceLoader is assigned, therefore DI of ResourceLoader will continue to resolve the ApplicationContext itself. I need to file an issue with Spring Context for this, as this is not a Spring Boot problem.

_@snicoll I am indeed the one who asked you about this at Spring I/O 😃 Only now did I get around to investigate some more and file this report._

[snicoll]

ResourcePatternResolverFactory shouldn't take both the ApplicationContext and a ResourceLoader as the former is a facade for the latter. I suspect that changing that should fix your issue. I have a patch locally but if you could share a sample that reproduces the problem, that would be quite helpful.

[JoostK]

I pushed a failing testcase to my fork, see https://github.com/JoostK/spring-boot/commit/1d93f424a679f1e8bb1c3f2b626de92945de32f1 for commit.

[snicoll]

@JoostK those tests do not reproduce the problems as described. They are overriding the facade (the ApplicationContext) rather than customizing the ResourceLoader. When I change your test to create a custom ResourceLoader and set it to the context, the test pass, e.g.

@Test
public void contextIsUsedAsResourceLoaderInNonWebApplication() throws Exception {
    GenericApplicationContext context = new GenericApplicationContext();
    ResourceLoader resourceLoader = new ResourceLoader() {

        @Override
        public Resource getResource(String location) {
            return new DescriptiveResource("Resource " + location);
        }

        @Nullable
        @Override
        public ClassLoader getClassLoader() {
            return ClassLoaderFilesResourcePatternResolverTests.class.getClassLoader();
        }
    };
    context.setResourceLoader(resourceLoader);
    this.resolver = new ClassLoaderFilesResourcePatternResolver(context, this.files);
    Resource resource = this.resolver.getResource("foo.txt");
    assertThat(resource.getDescription()).isEqualTo("Resource foo.txt");
}

The problem with overriding getResourceLoader in the context directly is that you're not customizing the ResourceLoader the ApplicationContext exposes but rather customizing the context itself. When Devtools switches the ResourceLoader that customization is hardcoded in the context so it will always behaves wrongly.

Given the description above, I can only assume that's a glitch in the test. Can you please revisit it and update your fork?

[JoostK]

When Devtools switches the ResourceLoader that customization is hardcoded in the context so it will always behaves wrongly.

This is exactly what my issue is. I am indeed overriding the ApplicationContext itself because it only works as a facade when GenericApplicationContext.resourceLoader is null, otherwise it uses the resource loading capabilities of its superclass, since GenericApplicationContext derives from DefaultResourceLoader. Therefore, when custom protocol resolvers are registered with the ApplicationContext itself (actually, the DefaultResourceLoader it extends) they fail to be consulted once Devtools injects the custom ResourceLoader. Therefore, I chose to reimplement ApplicationContext.getResource to easily showcase the fact that this method is never called.

You are correct in your suggestion to change the testcase the way you did, as you then implement the workaround I illustrated. What my tests show is that if GenericApplicationContext.resourceLoader is not set, then the ApplicationContext's implementation is not used, therefore its behaviour is lost.

The main problem here is that ApplicationContext in some cases is not just a facade, but an actual implementor of ResourceLoader. If you ask me, a better solution would be to remove DefaultResourceLoader from the inheritance chain of GenericApplicationContext and always delegate to an instance property. This would however be a breaking change and I understand that this cannot be changed at the moment (neither is it in Spring Boot, but in Spring Context).

Is the problem more clear now? Do you need anything else from me to help?

[snicoll]

Sorry but your test does not demonstrate the issue that you are describing (I think @wilkinsona and I got it in the meantime).

Devtools needs to customize the ResourceLoader so any attempt to override the getResource method on the context will break. However we can see now that we're missing the ProtocolResolver that might have been set directly on the context. We're going to copy those over to our custom ResourceLoader which should fix your issue.

[snicoll]

This potentially introduces another problem however,

If you set a custom ResourceLoader to the application context and ask to get the ResourceLoader bean you'll get the ApplicationContext (as you describe) that will delegate to your custom ResourceLoader. So essentially this isn't going to be a problem. That's a bit ouf of scope of this issue and feel free to raise a Spring Jira issue if you believe you've found a bug.

[JoostK]

@snicoll Thanks for your response, in the meantime I also came to realize that my "potential problem" was not applicable, given the delegation behaviour of GenericApplicationContext.

Copying over the protocol resolvers seems like a fine solution to me! Thanks for the quick turnaround.

[justinrknowles]

This fix doesn't seem to work, I see the code to copyProtocolResolvers but I still have the issue. If I include devtools the ProtocolResolver is ignored. If I remove the devtools jar the ProtocolResolver is used as expected. I think this should be reopened.

[bclozel]

@justinrknowles Could you create a new issue with a complete, minimal example (something we can unzip or git clone) that we can run to reproduce the problem?
You can of course link to that issue from the new one.

[darioseidl]

This issue is causing a problem with the SimpleStorageProtocolResolver in Spring Cloud AWS, reported here: https://github.com/spring-cloud/spring-cloud-aws/issues/348 and https://github.com/spring-cloud/spring-cloud-aws/issues/384.

When declaring a SimpleStorageProtocolResolver bean, it is added the the set of protocol resolvers in the DefaultResourceLoader and the call to super.getResource in ApplicationContext.getResource works.

However, with Devtools on the classpath, a different loader is called than DefaultResourceLoader which doesn't know about the SimpleStorageProtocolResolver.

[snicoll]

@darioseidl thanks for the nudge. Would you be able to share a sample in a new issue as already requested in the comment above yours?

[darioseidl]

I setup a quick demo here: https://github.com/darioseidl/s3demo

When you run the app, it will throw

Expected a SimpleStorageResource, but got ServletContext resource [/s3://test/test.txt]

Then comment out devtools in build.gradle.kts and run gradle clean then run it again, it should correctly resolve the resource and output:

OK: Amazon s3 resource [bucket='test' and object='test.txt']

To read from and write to a bucket, the AWS credentials have to be configured, but that's not necessary to reproduce the issue.

[philwebb]

Thanks for the sample @darioseidl, rather than re-open this one we've opened #17214. Please subscribe to that issue for updates.