Services now checks the contextClassLoader, Services.class.classLoader, and the system classloader #574
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…r, and the system classloader Fixes: #568
lhazlewood
reviewed
Mar 11, 2020
| /** | ||
| * Helper class for loading services from the classpath, using a {@link ServiceLoader}. Decouples loading logic for | ||
| * better separation of concerns and testability. | ||
| */ | ||
| public final class Services { | ||
|
|
||
| private static final List<ClassLoaderAccessor> CLASS_LOADER_ACCESSORS = arrayToList(new ClassLoaderAccessor[] { |
There was a problem hiding this comment.
I think this class would be much cleaner and reduce cyclomatic complexity / loops if we wrapped the list in our own delegating ClassLoader implementation. Thoughts?
There was a problem hiding this comment.
I was going to go down that path, but I figured this option was less complex. Mostly because I'm unaware of any gotchas (ClassLoader is a 3000+ abstract class). I'm guessing the SecurityManager could come into play 🤷♂
|
It probably goes without saying, but I'll ask anyway - were you able to verify this works w/ @timsazon's sample app? |
|
(didn't see your comment in the Issue until just now :-P ) |
lhazlewood
approved these changes
Mar 12, 2020
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Services are now loaded from the context class loader, Services.class.classLoader, and the system classloader, the first classloader with a service wins, and the others are ignored. This mimics how
Classes.forName()works, and how JJWT attempted to auto-discover various implementations in previous versions.Fixes: #568