Skip to content

Commit

Permalink
Improvements to CombinedResource for AnnotationParser
Browse files Browse the repository at this point in the history 
Added `contains` and `getPathTo` methods to the Resource API, so they can be used by the AnnotationParser
Fixed numerous bugs in CombinedResource list and getAllResources
Reduced fallback to URI string manipulation.
  • Loading branch information
@gregw
gregw committed Oct 25, 2023
1 parent 16dea21 commit 283e2af
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 116 deletions.
33 changes: 29 additions & 4 deletions jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/CombinedResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,12 @@

import java.io.IOException;
import java.net.URI;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
Expand Down Expand Up @@ -274,11 +277,30 @@ public List<Resource> list()
@Override
public void copyTo(Path destination) throws IOException
{
// Copy resources in reverse to get proper overlay behavior
List<Resource> entries = getResources();
for (int r = entries.size(); r-- > 0; )
// This method could be implemented with the simple:
// List<Resource> entries = getResources();
// for (int r = entries.size(); r-- > 0; )
// entries.get(r).copyTo(destination);
// However, that may copy large overlayed resources. The implementation below avoids that:

Collection<Resource> all = getAllResources();
for (Resource r : all)
{
entries.get(r).copyTo(destination);
Path relative = getPathTo(r);
Path pathTo = Objects.equals(relative.getFileSystem(), destination.getFileSystem())
? destination.resolve(relative)
: resolveDifferentFileSystem(destination, relative);

if (r.isDirectory())
{
ensureDirExists(pathTo);
}
else
{
Path pathFrom = r.getPath();
ensureDirExists(pathTo.getParent());
Files.copy(pathFrom, pathTo, StandardCopyOption.COPY_ATTRIBUTES, StandardCopyOption.REPLACE_EXISTING);
}
}
}

Expand Down Expand Up @@ -313,13 +335,16 @@ public String toString()
@Override
public boolean contains(Resource other)
{
// Every resource from the (possibly combined) other resource ...
loop: for (Resource o : other)
{
// Must be contained in at least one of this resources
for (Resource r : _resources)
{
if (r.contains(o))
continue loop;
}
// A resource of the other did not match any in this
return false;
}
return true;
Expand Down
103 changes: 16 additions & 87 deletions jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,16 @@

package org.eclipse.jetty.util.resource;

import java.io.File;
import java.io.IOException;
import java.net.URI;
import java.nio.file.DirectoryIteratorException;
import java.nio.file.Files;
import java.nio.file.LinkOption;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardCopyOption;
import java.nio.file.attribute.FileTime;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -203,6 +200,22 @@ public Path getPath()
return path;
}

@Override
public boolean contains(Resource other)
{
if (other == null)
return false;

Path thisPath = getPath();
if (thisPath == null)
throw new UnsupportedOperationException("Resources without a Path must implement contains");

Path otherPath = other.getPath();
return otherPath != null &&
otherPath.getFileSystem().equals(thisPath.getFileSystem()) &&
otherPath.startsWith(thisPath);
}

public Path getRealPath()
{
resolveAlias();
Expand Down Expand Up @@ -480,90 +493,6 @@ private void resolveAlias()
}
}

/**
* FileSystem neutral copyTo, so that even copyTo from different FileSystem types
* can be performed.
*
* @param destination the destination to copy to.
* @throws IOException if unable to copy to
*/
@Override
public void copyTo(Path destination) throws IOException
{
Path src = getPath();

// Do we have to copy a single file?
if (Files.isRegularFile(src))
{
// Is the destination a directory?
if (Files.isDirectory(destination))
{
// to a directory, preserve the filename
Path destFile = destination.resolve(src.getFileName().toString());
Files.copy(src, destFile, StandardCopyOption.COPY_ATTRIBUTES, StandardCopyOption.REPLACE_EXISTING);
}
else
{
// to a file, use destination as-is
Files.copy(src, destination, StandardCopyOption.COPY_ATTRIBUTES, StandardCopyOption.REPLACE_EXISTING);
}
return;
}

// At this point this PathResource is a directory.
URI srcURI = src.toUri();
String srcURIStr = srcURI.toASCIIString();
if (!srcURIStr.endsWith("/"))
srcURIStr += "/";
int srcURISubIndex = srcURI.toASCIIString().length();
URI destURI = destination.toUri();

try (Stream<Path> entriesStream = Files.walk(src))
{
Iterator<Path> pathIterator = entriesStream
.filter((path) -> !path.equals(src))
.iterator();
while (pathIterator.hasNext())
{
Path path = pathIterator.next();
// Since we might be copying across FileSystem Providers, we use URIs to
// calculate the output path we need to use.
URI entryURI = path.toUri();
String subURI = entryURI.toASCIIString().substring(srcURISubIndex);

if (Files.isDirectory(path))
subURI += File.separator;

URI outputPathURI = URIUtil.addPath(destURI, subURI);
Path outputPath = Path.of(outputPathURI);
if (LOG.isDebugEnabled())
LOG.debug("CopyTo: {} > {}", path, outputPath);
if (Files.isDirectory(path))
{
ensureDirExists(path);
}
else
{
ensureDirExists(outputPath.getParent());
Files.copy(path, outputPath, StandardCopyOption.COPY_ATTRIBUTES, StandardCopyOption.REPLACE_EXISTING);
}
}
}
}

private void ensureDirExists(Path dir) throws IOException
{
if (Files.exists(dir))
{
if (!Files.isDirectory(dir))
{
throw new IOException("Conflict, unable to create directory where file exists: " + dir);
}
return;
}
Files.createDirectories(dir);
}

/**
* Ensure Path to URI is sane when it returns a directory reference.
*
Expand Down
108 changes: 91 additions & 17 deletions jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,14 @@
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
import java.util.function.BiFunction;
import java.util.stream.Stream;

import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.URIUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* <p>
Expand All @@ -44,6 +48,7 @@
*/
public abstract class Resource implements Iterable<Resource>
{
private static final Logger LOG = LoggerFactory.getLogger(Resource.class);
private static final LinkOption[] NO_FOLLOW_LINKS = new LinkOption[]{LinkOption.NOFOLLOW_LINKS};

public static String dump(Resource resource)
Expand Down Expand Up @@ -88,8 +93,10 @@ public boolean contains(Resource other)
return false;

URI thisURI = getURI();
URI otherURI = other.getURI();
if (thisURI == null)
throw new UnsupportedOperationException("Resources without a URI must implement contains");

URI otherURI = other.getURI();
if (otherURI == null)
return false;

Expand All @@ -102,10 +109,11 @@ public boolean contains(Resource other)
return false;

// Ensure that if `file` scheme is used, it's using a consistent convention to allow for startsWith check
thisURI = URIUtil.correctFileURI(thisURI);
otherURI = URIUtil.correctFileURI(otherURI);
String thisURIString = URIUtil.correctFileURI(thisURI).toASCIIString();
String otherURIString = URIUtil.correctFileURI(otherURI).toASCIIString();

return otherURI.toASCIIString().startsWith(thisURI.toASCIIString());
return otherURIString.startsWith(thisURIString) &&
(thisURIString.length() == otherURIString.length() || otherURIString.charAt(thisURIString.length()) == '/');
}

/**
Expand All @@ -127,7 +135,6 @@ public Path getPathTo(Resource other)
if (otherPath == null)
return null;

// TODO: should probably not allow results like "../../path/to/other/location.txt"
return thisPath.relativize(otherPath);
}

Expand Down Expand Up @@ -288,31 +295,98 @@ public URI getRealURI()
}

/**
* Copy the Resource to the new destination file.
* Copy the Resource to the new destination file or directory
*
* @param destination the destination file to create
* @param destination the destination file to create or directory to use.
* @throws IOException if unable to copy the resource
*/
public void copyTo(Path destination)
throws IOException
{
if (Files.exists(destination))
throw new IllegalArgumentException(destination + " exists");

// attempt simple file copy
Path src = getPath();
if (src != null)
if (src == null)
{
if (!isDirectory())
{
// use old school stream based copy
try (InputStream in = newInputStream(); OutputStream out = Files.newOutputStream(destination))
{
IO.copy(in, out);
}
return;
}
throw new UnsupportedOperationException("Directory Resources without a Path must implement copyTo");
}

// Do we have to copy a single file?
if (Files.isRegularFile(src))
{
Files.copy(src, destination, StandardCopyOption.COPY_ATTRIBUTES, StandardCopyOption.REPLACE_EXISTING);
// Is the destination a directory?
if (Files.isDirectory(destination))
{
// to a directory, preserve the filename
Path destPath = destination.resolve(src.getFileName().toString());
Files.copy(src, destPath, StandardCopyOption.COPY_ATTRIBUTES, StandardCopyOption.REPLACE_EXISTING);
}
else
{
// to a file, use destination as-is
Files.copy(src, destination, StandardCopyOption.COPY_ATTRIBUTES, StandardCopyOption.REPLACE_EXISTING);
}
return;
}

// use old school stream based copy
try (InputStream in = newInputStream();
OutputStream out = Files.newOutputStream(destination))
// At this point this PathResource is a directory.
assert isDirectory();

BiFunction<Path, Path, Path> resolver = src.getFileSystem().equals(destination.getFileSystem())
? Path::resolve
: Resource::resolveDifferentFileSystem;

try (Stream<Path> entriesStream = Files.walk(src))
{
IO.copy(in, out);
for (Iterator<Path> pathIterator = entriesStream.iterator(); pathIterator.hasNext();)
{
Path path = pathIterator.next();
if (src.equals(path))
continue;

Path relative = src.relativize(path);
Path destPath = resolver.apply(destination, relative);

if (LOG.isDebugEnabled())
LOG.debug("CopyTo: {} > {}", path, destPath);
if (Files.isDirectory(path))
{
ensureDirExists(destPath);
}
else
{
ensureDirExists(destPath.getParent());
Files.copy(path, destPath, StandardCopyOption.COPY_ATTRIBUTES, StandardCopyOption.REPLACE_EXISTING);
}
}
}
}

static Path resolveDifferentFileSystem(Path path, Path relative)
{
for (Path segment : relative)
path = path.resolve(segment.toString());
return path;
}

void ensureDirExists(Path dir) throws IOException
{
if (Files.exists(dir))
{
if (!Files.isDirectory(dir))
{
throw new IOException("Conflict, unable to create directory where file exists: " + dir);
}
return;
}
Files.createDirectories(dir);
}

/**
Expand Down
7 changes: 3 additions & 4 deletions ...y-ee10-annotations/src/main/java/org/eclipse/jetty/ee10/annotations/AnnotationParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -595,11 +595,10 @@ protected void parseDir(Set<? extends Handler> handlers, Resource dirResource) t

// Get the path relative to the base resource
Path relative = dirResource.getPathTo(candidate);
if (relative == null)
continue; // skip if unable to get a relative path

// select only non-hidden class files that are not modules nor versions
if (FileID.isHidden(relative) ||
// select only relative non-hidden class files that are not modules nor versions
if (relative == null ||
FileID.isHidden(relative) ||
FileID.isMetaInfVersions(relative) ||
FileID.isModuleInfoClass(relative) ||
!FileID.isClassFile(relative))
Expand Down
7 changes: 3 additions & 4 deletions ...tty-ee9-annotations/src/main/java/org/eclipse/jetty/ee9/annotations/AnnotationParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -592,11 +592,10 @@ protected void parseDir(Set<? extends Handler> handlers, Resource dirResource) t

// Get the path relative to the base resource
Path relative = dirResource.getPathTo(candidate);
if (relative == null)
continue; // skip if unable to get a relative path

// select only non-hidden class files that are not modules nor versions
if (FileID.isHidden(relative) ||
// select only relative non-hidden class files that are not modules nor versions
if (relative == null ||
FileID.isHidden(relative) ||
FileID.isMetaInfVersions(relative) ||
FileID.isModuleInfoClass(relative) ||
!FileID.isClassFile(relative))
Expand Down

0 comments on commit 283e2af

Please sign in to comment.