From f8a3c4464128331103c5755d88352bb6991630fa Mon Sep 17 00:00:00 2001 From: Matyrobbrt Date: Thu, 16 Nov 2023 09:38:50 +0200 Subject: [PATCH 1/5] Remove the `notExistingPath` check and use a proper lazy stream for dirstreams --- .../cpw/mods/niofs/union/UnionFileSystem.java | 59 +++++++------------ .../cpw/mods/niofs/union/TestUnionFS.java | 11 +++- 2 files changed, 32 insertions(+), 38 deletions(-) diff --git a/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java b/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java index 42f7aec..00c6a8e 100644 --- a/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java +++ b/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java @@ -22,6 +22,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; @@ -96,7 +97,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 basepaths; @@ -241,14 +241,6 @@ private Optional getFileAttributes(final Path path) { } } - private Optional 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()) @@ -273,7 +265,7 @@ private Optional 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); @@ -293,7 +285,7 @@ private Optional 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); } } @@ -301,24 +293,16 @@ private Optional findFirstFiltered(final UnionPath unionPath) { return Optional.empty(); } - private Stream streamPathList(final Function> function) { - return this.basepaths.stream() - .map(function) - .flatMap(Optional::stream); - } - @SuppressWarnings("unchecked") public A readAttributes(final UnionPath path, final Class 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 fileAttributes = this.getFileAttributes(realPath); - if (fileAttributes.isPresent() && testFilter(realPath, base)) { - return (A) fileAttributes.get(); - } + final Path realPath = toRealPath(base, path); + final Optional fileAttributes = this.getFileAttributes(realPath); + if (fileAttributes.isPresent() && testFilter(realPath, base)) { + return (A) fileAttributes.get(); } } throw new NoSuchFileException(path.toString()); @@ -385,12 +369,11 @@ private SeekableByteChannel byteChannel(final Path path) { } public DirectoryStream newDirStream(final UnionPath path, final DirectoryStream.Filter filter) throws IOException { - final var allpaths = new LinkedHashSet(); + List closeables = new ArrayList<>(basepaths.size()); + Stream 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; @@ -398,24 +381,26 @@ public DirectoryStream newDirStream(final UnionPath path, final DirectoryS 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); + stream = Stream.concat(stream, 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)); } + final Stream realStream = stream; return new DirectoryStream<>() { @Override public Iterator iterator() { - return allpaths.iterator(); + return realStream.iterator(); } @Override public void close() throws IOException { - // noop + for (Closeable closeable : closeables) { + closeable.close(); + } } }; } diff --git a/src/test/java/cpw/mods/niofs/union/TestUnionFS.java b/src/test/java/cpw/mods/niofs/union/TestUnionFS.java index 0d0b75a..16780a1 100644 --- a/src/test/java/cpw/mods/niofs/union/TestUnionFS.java +++ b/src/test/java/cpw/mods/niofs/union/TestUnionFS.java @@ -1,7 +1,9 @@ package cpw.mods.niofs.union; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; +import java.io.FileNotFoundException; import java.io.IOException; import java.nio.file.FileSystems; import java.nio.file.Files; @@ -14,6 +16,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; import java.util.stream.StreamSupport; @@ -249,9 +252,15 @@ 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 AtomicInteger amount = new AtomicInteger(); assertAll( - StreamSupport.stream(dirStream.spliterator(), false).map(p->()->Files.exists(p)) + StreamSupport.stream(dirStream.spliterator(), false).map(p-> ()-> { + if (!Files.exists(p)) { + throw new FileNotFoundException(p.toString()); + } + }).peek(it -> amount.incrementAndGet()) ); + assert amount.get() > 10; } } @Test From 3dcdae0460774ddcba2c00f733d076c65caaf0e7 Mon Sep 17 00:00:00 2001 From: Matyrobbrt Date: Thu, 16 Nov 2023 09:56:16 +0200 Subject: [PATCH 2/5] Stream needs to be distinct --- src/main/java/cpw/mods/niofs/union/UnionFileSystem.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java b/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java index 00c6a8e..ae006e4 100644 --- a/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java +++ b/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java @@ -389,7 +389,7 @@ public DirectoryStream newDirStream(final UnionPath path, final DirectoryS .map(Path::getFileName).map(Path::toString).toArray(String[]::new)) .map(this::fastPath)); } - final Stream realStream = stream; + final Stream realStream = stream.distinct(); return new DirectoryStream<>() { @Override public Iterator iterator() { From dbd56f0a04913600ba476347320ff5d2066cde09 Mon Sep 17 00:00:00 2001 From: Matyrobbrt Date: Thu, 16 Nov 2023 18:32:19 +0200 Subject: [PATCH 3/5] Changes --- .../cpw/mods/niofs/union/UnionFileSystem.java | 2 +- .../cpw/mods/niofs/union/TestUnionFS.java | 47 +++++++++++++++---- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java b/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java index ae006e4..a443254 100644 --- a/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java +++ b/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java @@ -385,7 +385,7 @@ public DirectoryStream newDirStream(final UnionPath path, final DirectoryS closeables.add(ds); stream = Stream.concat(stream, 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(other -> StreamSupport.stream((isSimple ? other : bp.relativize(other)).spliterator(), false) .map(Path::getFileName).map(Path::toString).toArray(String[]::new)) .map(this::fastPath)); } diff --git a/src/test/java/cpw/mods/niofs/union/TestUnionFS.java b/src/test/java/cpw/mods/niofs/union/TestUnionFS.java index 16780a1..57517f9 100644 --- a/src/test/java/cpw/mods/niofs/union/TestUnionFS.java +++ b/src/test/java/cpw/mods/niofs/union/TestUnionFS.java @@ -1,7 +1,6 @@ package cpw.mods.niofs.union; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.function.Executable; import java.io.FileNotFoundException; import java.io.IOException; @@ -13,14 +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.concurrent.atomic.AtomicInteger; 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")); @@ -252,15 +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 AtomicInteger amount = new AtomicInteger(); + final List foundFiles = new ArrayList<>(); assertAll( - StreamSupport.stream(dirStream.spliterator(), false).map(p-> ()-> { - if (!Files.exists(p)) { - throw new FileNotFoundException(p.toString()); - } - }).peek(it -> amount.incrementAndGet()) + StreamSupport.stream(dirStream.spliterator(), false) + .peek(path -> foundFiles.add(path.toString())) + .map(p-> ()-> { + if (!Files.exists(p)) { + throw new FileNotFoundException(p.toString()); + } + }) ); - assert amount.get() > 10; + 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 From ff422072ee05e91a6e4a4024570afbcb5c5d22bc Mon Sep 17 00:00:00 2001 From: Matyrobbrt Date: Fri, 17 Nov 2023 17:04:02 +0200 Subject: [PATCH 4/5] Update --- .../cpw/mods/niofs/union/UnionFileSystem.java | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java b/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java index a443254..0e156dc 100644 --- a/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java +++ b/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java @@ -383,11 +383,10 @@ public DirectoryStream newDirStream(final UnionPath path, final DirectoryS final var isSimple = embeddedFileSystems.containsKey(bp); final var ds = Files.newDirectoryStream(dir, filter); closeables.add(ds); - stream = Stream.concat(stream, StreamSupport.stream(ds.spliterator(), false) + final var currentPaths = StreamSupport.stream(ds.spliterator(), false) .filter(p -> testFilter(p, bp)) - .map(other -> StreamSupport.stream((isSimple ? other : bp.relativize(other)).spliterator(), false) - .map(Path::getFileName).map(Path::toString).toArray(String[]::new)) - .map(this::fastPath)); + .map(other -> fastPath(isSimple ? other : bp.relativize(other))); + stream = Stream.concat(stream, currentPaths); } final Stream realStream = stream.distinct(); return new DirectoryStream<>() { @@ -405,6 +404,19 @@ public void close() throws IOException { }; } + /** + * 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 / From d97b77a81d589478dc4c0870b7aaf3e085943274 Mon Sep 17 00:00:00 2001 From: Matyrobbrt Date: Fri, 17 Nov 2023 17:37:44 +0200 Subject: [PATCH 5/5] Aggregate exceptions --- .../java/cpw/mods/niofs/union/UnionFileSystem.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java b/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java index 0e156dc..682ab51 100644 --- a/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java +++ b/src/main/java/cpw/mods/niofs/union/UnionFileSystem.java @@ -397,8 +397,20 @@ public Iterator iterator() { @Override public void close() throws IOException { + List exceptions = new ArrayList<>(); + for (Closeable closeable : closeables) { - closeable.close(); + 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; } } };