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.

Sign up for GitHub

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

Improve Resource use in MetaInfConfiguration #10889

Merged
merged 7 commits into from
Nov 29, 2023
Merged

Improve Resource use in MetaInfConfiguration #10889

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
473afb5
Reduce resources kept open by MetaInfConfiguration
janbartel Nov 13, 2023
2aa8f34
Use better ResourceFactory regime to reduce references
janbartel Nov 14, 2023
4feecd9
Fix
janbartel Nov 14, 2023
19ed557
Fix
janbartel Nov 14, 2023
1a51854
Update jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/e…
janbartel Nov 27, 2023
70afc89
Changes post review
janbartel Nov 27, 2023
c44c319
Change post review
janbartel Nov 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
238 changes: 111 additions & 127 deletions ...0/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/MetaInfConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
Expand All @@ -40,6 +39,7 @@
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.URIUtil;
import org.eclipse.jetty.util.UriPatternPredicate;
import org.eclipse.jetty.util.resource.MountedPathResource;
import org.eclipse.jetty.util.resource.Resource;
import org.eclipse.jetty.util.resource.ResourceCollators;
import org.eclipse.jetty.util.resource.ResourceFactory;
Expand Down Expand Up @@ -173,18 +173,18 @@ public void findAndFilterContainerPaths(final WebAppContext context) throws Exce
if (modulePath != null)
{
List<Path> matchingBasePaths =
Stream.of(modulePath.split(File.pathSeparator))
.map(URIUtil::toURI)
.filter(uriPatternPredicate)
.map(Paths::get)
.toList();
for (Path path: matchingBasePaths)
Stream.of(modulePath.split(File.pathSeparator))
.map(URIUtil::toURI)
.filter(uriPatternPredicate)
.map(Paths::get)
.toList();
for (Path path : matchingBasePaths)
{
if (Files.isDirectory(path))
{
try (Stream<Path> listing = Files.list(path))
{
for (Path listEntry: listing.toList())
for (Path listEntry : listing.toList())
{
Resource resource = resourceFactory.newResource(listEntry);
context.getMetaData().addContainerResource(resource);
Expand Down Expand Up @@ -354,82 +354,94 @@ public void scanJars(final WebAppContext context, Collection<Resource> jars, boo
//Scan jars for META-INF information
if (jars != null)
{
for (Resource r : jars)
try (ResourceFactory.Closeable scanResourceFactory = ResourceFactory.closeable())
{
if (scanTypes.contains(METAINF_RESOURCES))
scanForResources(context, r, metaInfResourceCache);
if (scanTypes.contains(METAINF_FRAGMENTS))
scanForFragment(context, r, metaInfFragmentCache);
if (scanTypes.contains(METAINF_TLDS))
scanForTlds(context, r, metaInfTldCache);
for (Resource r : jars)
{
Resource dir = r;

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for (Resource r : jars)
{
Resource dir = r;
for (Resource dir : jars)
{

try
{
//If not already a directory, convert by mounting as jar file
if (!dir.isDirectory())
dir = scanResourceFactory.newJarFileResource(dir.getURI());
}
catch (Exception e)
{
//not an appropriate uri, skip it
continue;
}

if (isEmptyResource(dir))
continue;

if (scanTypes.contains(METAINF_RESOURCES))
scanForResources(context, dir, metaInfResourceCache);
if (scanTypes.contains(METAINF_FRAGMENTS))
scanForFragment(context, dir, metaInfFragmentCache);
if (scanTypes.contains(METAINF_TLDS))
scanForTlds(context, dir, metaInfTldCache);
janbartel marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}

/**
* Scan for META-INF/resources dir in the given jar.
* Scan for META-INF/resources dir in the given directory.
*
* @param context the context for the scan
* @param target the target resource to scan for
* @param dir the target directory to scan
* @param cache the resource cache
*/
public void scanForResources(WebAppContext context, Resource target, ConcurrentHashMap<Resource, Resource> cache)
public void scanForResources(WebAppContext context, Resource dir, ConcurrentHashMap<Resource, Resource> cache)
{
// Resource target does not exist
if (Resources.missing(target))
if (isEmptyResource(dir))
return;
ResourceFactory resourceFactory = ResourceFactory.of(context);

Resource resourcesDir;
if (cache != null && cache.containsKey(target))
Resource resourcesDir = null;

if (cache != null && cache.containsKey(dir))
{
resourcesDir = cache.get(target);
resourcesDir = cache.get(dir);
if (isEmptyResource(resourcesDir))
{
if (LOG.isDebugEnabled())
LOG.debug("{} cached as containing no META-INF/resources", target);
LOG.debug("{} cached as containing no META-INF/resources", dir);
return;
}
else if (LOG.isDebugEnabled())
LOG.debug("{} META-INF/resources found in cache ", target);
LOG.debug("{} META-INF/resources found in cache ", dir);
}
else
{
//not using caches or not in the cache so check for the resources dir
if (LOG.isDebugEnabled())
LOG.debug("{} META-INF/resources checked", target);
if (target.isDirectory())
{
//TODO think how to handle an unpacked jar file (eg for osgi)
resourcesDir = target.resolve("/META-INF/resources");
}
else
{
// Resource represents a packed jar
URI uri = target.getURI();
resourcesDir = resourceFactory.newResource(URIUtil.uriJarPrefix(uri, "!/META-INF/resources"));
}
LOG.debug("{} META-INF/resources checked", dir);

if (Resources.isReadableDirectory(resourcesDir) && (cache != null))
resourcesDir = dir.resolve("/META-INF/resources");

if (isEmptyResource(resourcesDir))
return;

//convert from an ephemeral Resource to one that is associated with the context's lifecycle
resourcesDir = context.getResourceFactory().newResource(resourcesDir.getURI());

if (cache != null)
{
Resource old = cache.putIfAbsent(target, resourcesDir);
Resource old = cache.putIfAbsent(dir, resourcesDir);
if (old != null)
resourcesDir = old;
else if (LOG.isDebugEnabled())
LOG.debug("{} META-INF/resources cache updated", target);
}

if (isEmptyResource(resourcesDir))
{
return;
LOG.debug("{} META-INF/resources cache updated", dir);
}
}

//add it to the meta inf resources for this context
Set<Resource> dirs = (Set<Resource>)context.getAttribute(METAINF_RESOURCES);
if (dirs == null)
{
dirs = new HashSet<>();
dirs = new HashSet<Resource>();
context.setAttribute(METAINF_RESOURCES, dirs);
}
if (LOG.isDebugEnabled())
Expand All @@ -444,65 +456,69 @@ private static boolean isEmptyResource(Resource resourcesDir)
}

/**
* Scan for META-INF/web-fragment.xml file in the given jar.
* Scan for META-INF/web-fragment.xml file in the given directory.
*
* @param context the context for the scan
* @param jar the jar resource to scan for fragments in
* @param dir the dir to scan for fragments
* @param cache the resource cache
*/
public void scanForFragment(WebAppContext context, Resource jar, ConcurrentHashMap<Resource, Resource> cache)
public void scanForFragment(WebAppContext context, Resource dir, ConcurrentHashMap<Resource, Resource> cache)
{
ResourceFactory resourceFactory = ResourceFactory.of(context);

Resource webFrag;
if (cache != null && cache.containsKey(jar))
Resource webFrag = null;
if (cache != null && cache.containsKey(dir))
{
webFrag = cache.get(jar);
webFrag = cache.get(dir);
if (isEmptyFragment(webFrag))
{
if (LOG.isDebugEnabled())
LOG.debug("{} cached as containing no META-INF/web-fragment.xml", jar);
LOG.debug("{} cached as containing no META-INF/web-fragment.xml", dir);
return;
}
else if (LOG.isDebugEnabled())
LOG.debug("{} META-INF/web-fragment.xml found in cache ", jar);
LOG.debug("{} META-INF/web-fragment.xml found in cache ", dir);
}
else
{
//not using caches or not in the cache so check for the web-fragment.xml
if (LOG.isDebugEnabled())
LOG.debug("{} META-INF/web-fragment.xml checked", jar);
if (jar.isDirectory())
{
webFrag = resourceFactory.newResource(jar.getPath().resolve("META-INF/web-fragment.xml"));
}
else
{
URI uri = jar.getURI();
webFrag = resourceFactory.newResource(URIUtil.uriJarPrefix(uri, "!/META-INF/web-fragment.xml"));
}
LOG.debug("{} META-INF/web-fragment.xml checked", dir);

if (Resources.isReadable(webFrag) && (cache != null))
Resource metaInf = dir.resolve("META-INF");
if (isEmptyResource(metaInf))
return;

webFrag = metaInf.resolve("web-fragment.xml");

if (isEmptyFragment(webFrag))
return;

//convert ephemeral Resource to one associated with the context's lifecycle ResourceFactory
webFrag = context.getResourceFactory().newResource(webFrag.getURI());

if (cache != null)
{
//web-fragment.xml doesn't exist: put token in cache to signal we've seen the jar
Resource old = cache.putIfAbsent(jar, webFrag);
Resource old = cache.putIfAbsent(dir, webFrag);
if (old != null)
webFrag = old;
else if (LOG.isDebugEnabled())
LOG.debug("{} META-INF/web-fragment.xml cache updated", jar);
LOG.debug("{} META-INF/web-fragment.xml cache updated", dir);
}

if (isEmptyFragment(webFrag))
return;
}

Map<Resource, Resource> fragments = (Map<Resource, Resource>)context.getAttribute(METAINF_FRAGMENTS);
if (fragments == null)
{
fragments = new HashMap<>();
fragments = new HashMap<Resource, Resource>();
context.setAttribute(METAINF_FRAGMENTS, fragments);
}
fragments.put(jar, webFrag);

if (dir instanceof MountedPathResource)
{
//ensure we link from the original .jar rather than jar:file:
dir = context.getResourceFactory().newResource(((MountedPathResource)dir).getContainerPath());
}

fragments.put(dir, webFrag);
if (LOG.isDebugEnabled())
LOG.debug("{} added to context", webFrag);
}
Expand All @@ -513,53 +529,45 @@ private static boolean isEmptyFragment(Resource webFrag)
}

/**
* Discover META-INF/*.tld files in the given jar
* Discover META-INF/*.tld files in the given directory
*
* @param context the context for the scan
* @param jar the jar resources to scan tlds for
* @param dir the directory resource to scan for .tlds
* @param cache the resource cache
* @throws Exception if unable to scan for tlds
* @throws Exception if unable to scan for .tlds
*/
public void scanForTlds(WebAppContext context, Resource jar, ConcurrentHashMap<Resource, Collection<URL>> cache)
public void scanForTlds(WebAppContext context, Resource dir, ConcurrentHashMap<Resource, Collection<URL>> cache)
throws Exception
{
Collection<URL> tlds;

if (cache != null && cache.containsKey(jar))
if (cache != null && cache.containsKey(dir))
{
Collection<URL> tmp = cache.get(jar);
Collection<URL> tmp = cache.get(dir);
if (tmp.isEmpty())
{
if (LOG.isDebugEnabled())
LOG.debug("{} cached as containing no tlds", jar);
LOG.debug("{} cached as containing no tlds", dir);
return;
}
else
{
tlds = tmp;
if (LOG.isDebugEnabled())
LOG.debug("{} tlds found in cache ", jar);
LOG.debug("{} tlds found in cache ", dir);
}
}
else
{
//not using caches or not in the cache so find all tlds
tlds = new HashSet<>();
if (jar.isDirectory())
{
tlds.addAll(getTlds(jar.getPath()));
}
else
{
URI uri = jar.getURI();
tlds.addAll(getTlds(context, uri));
}
tlds.addAll(getTlds(context, dir));

if (cache != null)
{
if (LOG.isDebugEnabled())
LOG.debug("{} tld cache updated", jar);
Collection<URL> old = cache.putIfAbsent(jar, tlds);
LOG.debug("{} tld cache updated", dir);
Collection<URL> old = cache.putIfAbsent(dir, tlds);
if (old != null)
tlds = old;
}
Expand Down Expand Up @@ -590,45 +598,21 @@ public void postConfigure(WebAppContext context) throws Exception
}

/**
* Find all .tld files in all subdirs of the given dir.
* Find all .tld files in the given directory.
*
* @param dir the directory to scan
* @return the list of tlds found
* @throws IOException if unable to scan the directory
*/
private Collection<URL> getTlds(Path dir) throws IOException
{
if (dir == null || !Files.isDirectory(dir))
return Collections.emptySet();

Set<URL> tlds = new HashSet<>();

try (Stream<Path> entries = Files.walk(dir)
.filter(Files::isRegularFile)
.filter(FileID::isTld))
{
Iterator<Path> iter = entries.iterator();
while (iter.hasNext())
{
Path entry = iter.next();
tlds.add(entry.toUri().toURL());
}
}
return tlds;
}

/**
* Find all .tld files in the given jar.
*
* @param uri the uri to jar file
* @param dir the directory
* @return the collection of tlds as url references
* @throws IOException if unable to scan the jar file
*/
private Collection<URL> getTlds(WebAppContext context, URI uri) throws IOException
private Collection<URL> getTlds(WebAppContext context, Resource dir) throws IOException
{
HashSet<URL> tlds = new HashSet<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

This is strongly discouraged by Java now. (using URL in a Set or any kind of comparison or hashcode/equals logic)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, as we're passing this on to the Apache Jasper implementation we need to support what they want.

Copy link
Contributor

Choose a reason for hiding this comment

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

The Apache Jasper Implementation uses a collection of org.apache.tomcat.util.descriptor.tld.TldResourcePath.
Internally, Apache Jasper uses String and URI, not URL.
The Set<URL> is entirely on us, not Apache Jasper.
See org.eclipse.jetty.ee10.apache.jsp.JettyTldPreScanned which also does not use Set<URL> too.

Resource r = ResourceFactory.of(context).newResource(URIUtil.uriJarPrefix(uri, "!/"));
try (Stream<Path> stream = Files.walk(r.getPath()))

Resource metaInf = dir.resolve("META-INF");
if (isEmptyResource(metaInf))
return tlds; //no tlds

try (Stream<Path> stream = Files.walk(metaInf.getPath()))
{
Iterator<Path> it = stream
.filter(Files::isRegularFile)
Expand Down
Loading