Skip to content

Commit

Permalink
Remove the notExistingPath check and use proper lazy streams (#50)
Browse files Browse the repository at this point in the history 
The `!= notExistingPath` doesn't do anything, and the `newDirStream`
method would accumulate the results in a list, which is not efficient
for dir streams that early-exit. Streams are meant to be lazy.
  • Loading branch information
@Matyrobbrt
Matyrobbrt authored Nov 17, 2023
1 parent dff4f69 commit 420e984
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 39 deletions.
83 changes: 46 additions & 37 deletions src/main/java/cpw/mods/niofs/union/UnionFileSystem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.nio.file.WatchService;
import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.UserPrincipalLookupService;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedHashSet;
Expand Down Expand Up @@ -98,7 +99,6 @@ public synchronized Throwable fillInStackTrace() {
}

private final UnionPath root = new UnionPath(this, "/");
private final UnionPath notExistingPath = new UnionPath(this, "/SNOWMAN");
private final UnionFileSystemProvider provider;
private final String key;
private final List<Path> basepaths;
Expand Down Expand Up @@ -245,14 +245,6 @@ private Optional<BasicFileAttributes> getFileAttributes(final Path path) {
}
}

private Optional<Path> findFirstPathAt(final UnionPath path) {
return this.basepaths.stream()
.map(p -> toRealPath(p, path))
.filter(p -> p != notExistingPath)
.filter(Files::exists)
.findFirst();
}

private static boolean zipFsExists(UnionFileSystem ufs, Path path) {
try {
if (Optional.ofNullable(ufs.embeddedFileSystems.get(path.getFileSystem())).filter(efs -> !efs.fsCh.isOpen()).isPresent())
Expand All @@ -277,7 +269,7 @@ private Optional<Path> findFirstFiltered(final UnionPath unionPath) {
final Path p = this.basepaths.get(i);
final Path realPath = toRealPath(p, unionPath);
// Test if the real path exists and matches the filter of this file system
if (realPath != notExistingPath && testFilter(realPath, p)) {
if (testFilter(realPath, p)) {
if (realPath.getFileSystem() == FileSystems.getDefault()) {
if (realPath.toFile().exists()) {
return Optional.of(realPath);
Expand All @@ -297,32 +289,24 @@ private Optional<Path> findFirstFiltered(final UnionPath unionPath) {
final Path last = basepaths.get(lastElementIndex);
final Path realPath = toRealPath(last, unionPath);
// We still care about the FS filter, but not about the existence of the real path
if (realPath != notExistingPath && testFilter(realPath, last)) {
if (testFilter(realPath, last)) {
return Optional.of(realPath);
}
}

return Optional.empty();
}

private <T> Stream<T> streamPathList(final Function<Path, Optional<T>> function) {
return this.basepaths.stream()
.map(function)
.flatMap(Optional::stream);
}

@SuppressWarnings("unchecked")
public <A extends BasicFileAttributes> A readAttributes(final UnionPath path, final Class<A> type, final LinkOption... options) throws IOException {
if (type == BasicFileAttributes.class) {
// We need to run the test on the actual path,
for (Path base : this.basepaths) {
// We need to know the full path for the filter
Path realPath = toRealPath(base, path);
if (realPath != notExistingPath) {
Optional<BasicFileAttributes> fileAttributes = this.getFileAttributes(realPath);
if (fileAttributes.isPresent() && testFilter(realPath, base)) {
return (A) fileAttributes.get();
}
final Path realPath = toRealPath(base, path);
final Optional<BasicFileAttributes> fileAttributes = this.getFileAttributes(realPath);
if (fileAttributes.isPresent() && testFilter(realPath, base)) {
return (A) fileAttributes.get();
}
}
throw new NoSuchFileException(path.toString());
Expand Down Expand Up @@ -389,41 +373,66 @@ private SeekableByteChannel byteChannel(final Path path) {
}

public DirectoryStream<Path> newDirStream(final UnionPath path, final DirectoryStream.Filter<? super Path> filter) throws IOException {
final var allpaths = new LinkedHashSet<Path>();
List<Closeable> closeables = new ArrayList<>(basepaths.size());
Stream<Path> stream = Stream.empty();
for (final var bp : basepaths) {
final var dir = toRealPath(bp, path);
if (dir == notExistingPath) {
continue;
} else if (dir.getFileSystem() == FileSystems.getDefault() && !dir.toFile().exists()) {
if (dir.getFileSystem() == FileSystems.getDefault() && !dir.toFile().exists()) {
continue;
} else if (dir.getFileSystem().provider().getScheme().equals("jar") && !zipFsExists(this, dir)) {
continue;
} else if (Files.notExists(dir)) {
continue;
}
final var isSimple = embeddedFileSystems.containsKey(bp);
try (final var ds = Files.newDirectoryStream(dir, filter)) {
StreamSupport.stream(ds.spliterator(), false)
.filter(p -> testFilter(p, bp))
.map(other -> StreamSupport.stream(Spliterators.spliteratorUnknownSize((isSimple ? other : bp.relativize(other)).iterator(), Spliterator.ORDERED), false)
.map(Path::getFileName).map(Path::toString).toArray(String[]::new))
.map(this::fastPath)
.forEachOrdered(allpaths::add);
}
final var ds = Files.newDirectoryStream(dir, filter);
closeables.add(ds);
final var currentPaths = StreamSupport.stream(ds.spliterator(), false)
.filter(p -> testFilter(p, bp))
.map(other -> fastPath(isSimple ? other : bp.relativize(other)));
stream = Stream.concat(stream, currentPaths);
}
final Stream<Path> realStream = stream.distinct();
return new DirectoryStream<>() {
@Override
public Iterator<Path> iterator() {
return allpaths.iterator();
return realStream.iterator();
}

@Override
public void close() throws IOException {
// noop
List<IOException> exceptions = new ArrayList<>();

for (Closeable closeable : closeables) {
try {
closeable.close();
} catch (IOException e) {
exceptions.add(e);
}
}

if (!exceptions.isEmpty()) {
IOException aggregate = new IOException("Failed to close some streams in UnionFileSystem.newDirStream");
exceptions.forEach(aggregate::addSuppressed);
throw aggregate;
}
}
};
}

/**
* Create a relative UnionPath from the path elements of the given {@link Path}.
*/
private Path fastPath(Path pathToConvert) {
String[] parts = new String[pathToConvert.getNameCount()];

for (int i = 0; i < parts.length; i++) {
parts[i] = pathToConvert.getName(i).toString();
}

return fastPath(parts);
}

/*
* Standardize paths:
* Path separators converted to /
Expand Down
40 changes: 38 additions & 2 deletions src/test/java/cpw/mods/niofs/union/TestUnionFS.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import org.junit.jupiter.api.Test;

import java.io.FileNotFoundException;
import java.io.IOException;
import java.nio.file.FileSystems;
import java.nio.file.Files;
Expand All @@ -11,13 +12,22 @@
import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.FileAttributeView;
import java.nio.file.spi.FileSystemProvider;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;

import static org.junit.jupiter.api.Assertions.*;
import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertIterableEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class TestUnionFS {
private static final UnionFileSystemProvider UFSP = (UnionFileSystemProvider) FileSystemProvider.installedProviders().stream().filter(fsp->fsp.getScheme().equals("union")).findFirst().orElseThrow(()->new IllegalStateException("Couldn't find UnionFileSystemProvider"));
Expand Down Expand Up @@ -249,9 +259,35 @@ public void testDirectoryVisitorJar() throws Exception {
final var fileSystem = UFSP.newFileSystem(jar1, Map.of("additional", List.of(jar2, jar3)));
var root = fileSystem.getPath("/");
try (var dirStream = Files.newDirectoryStream(root)) {
final List<String> foundFiles = new ArrayList<>();
assertAll(
StreamSupport.stream(dirStream.spliterator(), false).map(p->()->Files.exists(p))
StreamSupport.stream(dirStream.spliterator(), false)
.peek(path -> foundFiles.add(path.toString()))
.map(p-> ()-> {
if (!Files.exists(p)) {
throw new FileNotFoundException(p.toString());
}
})
);
assertEquals(foundFiles, List.of(
"log4j2.xml",
"module-info.class",
"cpw",
"META-INF",
"LICENSE.txt",
"CREDITS.txt",
"url.png",
"pack.mcmeta",
"mcplogo.png",
"forge_logo.png",
"forge.srg",
"forge.sas",
"forge.exc",
"data",
"coremods",
"assets",
"net"
));
}
}
@Test
Expand Down

0 comments on commit 420e984

Please sign in to comment.