Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jetty#11789 run list distinct on the generated resources list during startup #11790

Open
wants to merge 2 commits into
base: jetty-12.0.x
Choose a base branch
from

Conversation

jackeri
Copy link

@jackeri jackeri commented May 14, 2024

Fixes the duplicate resource issue described in the ticket #11789

@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not keen on this, as it is an error to have duplicates on the classpath, which is where this derives from.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants