Allow ClassPathResources to be filtered by FilteredClassLoader #14774
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
philwebb
added
type: enhancement
There was a problem hiding this comment.
Thanks for the PR!
I am not sure that reusing Predicate<String> is a good idea considering that we apply them on both resources and classes. I'll give it some more thoughts and perhaps someone else on the team will have some idea in the meantime.
Feel free to chime in as well!
|
|
||
| @Override | ||
| public URL getResource(String name) { | ||
| for (Predicate<String> filter : this.filters) { |
There was a problem hiding this comment.
This may be overkill but classes and resources predicates are being applied to both use cases with no way for an implementation to know what type of "resource" needs to be filtered.
There was a problem hiding this comment.
No exactly, this is why I found the current design a bit unfortunate. If there's no objection to changing FilteredClassLoader a bit more (and adjusting the tests currently using it), I could take a stab at refactoring into a builder idea, similar to ApplicationContextRunner.
Is this something that is desired/acceptable?
...-boot-test/src/test/java/org/springframework/boot/test/context/FilteredClassLoaderTests.java
Outdated
Show resolved
Hide resolved
sagacity
force-pushed
the
filtered-resources
branch
from
1f3d63f to
9863657
Compare
* pr/14774: Polish "Allow ClassPathResources to be filtered by FilteredClassLoader" Allow ClassPathResources to be filtered by FilteredClassLoader
|
@RoyJacobs thank you for making your first contribution to Spring Boot. I've merged it with a polish commit. I've decided in the end to split the internals with two filter lists so that only the case where raw filters are specified are affected by that side effect we've discussed. |
|
Fair enough :) |
4 participants
Hi! Based on a brief discussion on Gitter with @snicoll I've added support for filtering resources in
FilteredClassLoader. This can be used when testing things like@ConditionalOnResource.The current class is a bit tricky to augment since it relies on varargs lists of
Predicate<String>. This is why I've chosen to follow the same approach and allow filteringClassPathResources, but this means it's not possible to filter both classes and resources simultaneously. If this is desired the loader should probably be refactored to use a builder or something, but I didn't want to make this a breaking change.I've based the pull request on the
2.0.xbranch because themasterbranch doesn't build right now.