Remove the notExistingPath check and use proper lazy streams (#50)

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.

Author: Matyrobbrt

src/main/java/cpw/mods/niofs/union/UnionFileSystem.java

@@ -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;
@@ -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;
@@ -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())
@@ -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);
@@ -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());
@@ -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 /

src/test/java/cpw/mods/niofs/union/TestUnionFS.java

@@ -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;
@@ -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"));
@@ -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