jetty#11789 run list distinct on the generated resources list during startup #11790
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
gregw
requested changes
Jul 25, 2024
| @@ -485,7 +486,8 @@ public void resolve(WebAppContext context) | |||
| resources.add(null); //always apply annotations with no resource first | |||
| resources.addAll(_orderedContainerResources); //next all annotations from container path | |||
| resources.addAll(_webInfClasses); //next everything from web-inf classes | |||
| resources.addAll(getWebInfResources(isOrdered())); //finally annotations (in order) from webinf path | |||
| resources.addAll(getWebInfResources(isOrdered())); //finally annotations (in order) from webinf path | |||
| resources = resources.stream().distinct().collect(Collectors.toList()); // filter out possible duplicates | |||
There was a problem hiding this comment.
@janbartel @jackeri I'm not opposed to defensively removing duplicates, but I think we should warn. Somethine like:
Suggested change
| resources = resources.stream().distinct().collect(Collectors.toList()); // filter out possible duplicates | |
| List<Resource> nodups = resources.stream().distinct().collect(Collectors.toList()); // filter out possible duplicates | |
| if (nodups.length() < resources.length()) | |
| { | |
| LOG.warning("Duplicate jars: " + resources); | |
| resources = nodups; | |
| } |
There was a problem hiding this comment.
I'm not keen on this, as it is an error to have duplicates on the classpath, which is where this derives from.
|
@jackeri I hate not to accept a contribution, but I really think it's a mistake to try and filter out duplicates: this is an important indication that there is something wrong with the configuration of the webapp. The most that I think we should do is to simply log that there are duplicates - it is up to the user to fix the configuration of their webapp. So if you change your PR to just log which are duplications, then OK with that. |
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.
Fixes the duplicate resource issue described in the ticket #11789