ConnectionDetailsFactories should use the context class loader to load factories

[lengors]

Add flag to determine whether to use context class loader or ConnectionDetailsFactory class loader in ConnectionDetailsFactories.

Implements #45010.

I introduced a new constructor in ConnectionDetailsFactories that allows selecting between the thread’s context class loader and the ConnectionDetailsFactory class's class loader. retains the previous behavior to ensure backward compatibility.

Additionally, I extended DockerComposeProperties with a new flag (defaulting to false) that reflects this setting. The DockerComposeServiceConnectionsApplicationListener now uses this flag when creating the ConnectionDetailsFactories.

I chose this approach to maintain the old behavior by default. If there's consensus that always using the thread’s context class loader is sufficient, I’m happy to simplify the implementation accordingly.

[philwebb]

Thanks very much for the PR @lengors. I think it's possible to argue that this is a bug, however, it feels a little risky to fix in a patch release so I'm keeping the target to 3.5.

I've refined your contribution a little to remove the property support and always use the context classloader. I think this should be fine, but if we find it introduces problems we can consider reintroducing the property.

If you can, please give the 3.5.x SNAPSHOT a go when https://github.com/spring-projects/spring-boot/actions/runs/14342588861 has finished to check that it solves your problem. Thanks!

[lengors]

Thanks very much for the PR @lengors. I think it's possible to argue that this is a bug, however, it feels a little risky to fix in a patch release so I'm keeping the target to 3.5.

I've refined your contribution a little to remove the property support and always use the context classloader. I think this should be fine, but if we find it introduces problems we can consider reintroducing the property.

If you can, please give the 3.5.x SNAPSHOT a go when https://github.com/spring-projects/spring-boot/actions/runs/14342588861 has finished to check that it solves your problem. Thanks!

@philwebb Thanks for reviewing.

Correct me if I'm wrong but as it stands, on DockerComposeServiceConnectionsApplicationListener, the ConnectionDetailsFactories is being instantiated by passing null as the class loader to the constructor which then SpringFactoriesLoader replaces with its own class classloder, which isn't necessarily the same as the thread's context classloader, right?

This would match the test I did where the custom ConnectionDetails create with the docker-compose factory still does not get injected when using dev-tools.

That said, I think being able to specify the classloader to instantiate ConnectionDetailsFactories is already a good step in the right direction, so thank you.

[philwebb]

Correct me if I'm wrong but as it stands, on DockerComposeServiceConnectionsApplicationListener, the ConnectionDetailsFactories is being instantiated by passing null as the class loader to the constructor which then SpringFactoriesLoader replaces with its own class classloder, which isn't necessarily the same as the thread's context classloader, right?

I think it should be OK, the factories code ends up being used here which call this ClassUtils method. That method calls getDefaultClassLoader() which should attempt to use the thread context class loader.

That being said, this stuff is quite hard to test so please let me know if you find things still aren't working as expected.

[lengors]

I think it should be OK, the factories code ends up being used here which call this ClassUtils method. That method calls getDefaultClassLoader() which should attempt to use the thread context class loader.

That being said, this stuff is quite hard to test so please let me know if you find things still aren't working as expected.

I've create a repository to illustrate the issue: https://github.com/lengors/spring-connection-details-factory-issue

But, I also take a look in the meantime, and I think the issue is here where, if the classloader passed to forResourceLocation/forDefaultResourceLocation is null, it will then be replaced by SpringFactoriesLoader.class.getClassLoader(), so by the time the ClassUtils method is invoked the classloader is not null anymore, and it will use the one pass to it rather than calling getDefaultClassLoader().

So, I think it needs to be explicitly passed to the forDefaultResourceLocation function (maybe even by invoking the getDefaultClassLoader() itself on the ConnectionDetailsFactories constructor).

[philwebb]

Oh no, I totally missed that code and it behaves differently than I expected. I think it might be done that way for back-compatibility reasons. I think that classloader is used there to load the factories files, and the other code is used to create the class. I'll run the sample you've provided and check.

[philwebb]

It looks like we have a bug in Spring Framework causing cache pollution. I've submitted spring-projects/spring-framework#34732. I've also put a work around in commit 4af0ee2. With that in place, the sample application works.

Thanks for your persistence getting this fixed!

[lengors]

It looks like we have a bug in Spring Framework causing cache pollution. I've submitted spring-projects/spring-framework#34732. I've also put a work around in commit 4af0ee2. With that in place, the sample application works.

Thanks for your persistence getting this fixed!

I gave it a test on both the sample application I provided and on an actual application I'm developing and both seemingly work. I'll keep using the -SNAPSHOT build for now and will report back if I find any issues.

Thank you very much for the work you've put into this!