NIO: walkFiles consistently relative or absolute (#3773)

* walkFiles consistently relative or absolute

The expected behavior from Files.walkFileTree is that if the starting path is absolute, then all the visited paths are absolute. If the starting path is relative, then all the visited paths are relative.

This PR adds a test for this behavior, and shows that our current code
fails for an absolute path: it returns a mix of relative and absolute
paths.

This PR also adds a code change to fix the problem, so the actual
behavior matches what is expected.

This should fix issue #3772

* address issues found by linter

* rename PostTraversalWalker

* apply reviewer feedback

Author: jean-philippe-martin

java-storage-nio/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java

@@ -102,15 +102,19 @@ private static class LazyPathIterator extends AbstractIterator<Path> {
     private final Filter<? super Path> filter;
     private final CloudStorageFileSystem fileSystem;
     private final String prefix;
+    // whether to make the paths absolute before returning them.
+    private final boolean absolutePaths;
 
     LazyPathIterator(CloudStorageFileSystem fileSystem,
                      String prefix,
                      Iterator<Blob> blobIterator,
-                     Filter<? super Path> filter) {
+                     Filter<? super Path> filter,
+                     boolean absolutePaths) {
       this.prefix = prefix;
       this.blobIterator = blobIterator;
       this.filter = filter;
       this.fileSystem = fileSystem;
+      this.absolutePaths = absolutePaths;
     }
 
     @Override
@@ -123,6 +127,9 @@ protected Path computeNext() {
             continue;
           }
           if (filter.accept(path)) {
+            if (absolutePaths) {
+              return path.toAbsolutePath();
+            }
             return path;
           }
         } catch (IOException ex) {
@@ -769,7 +776,7 @@ public void createDirectory(Path dir, FileAttribute<?>... attrs) {
   }
 
   @Override
-  public DirectoryStream<Path> newDirectoryStream(Path dir, final Filter<? super Path> filter) {
+  public DirectoryStream<Path> newDirectoryStream(final Path dir, final Filter<? super Path> filter) {
     final CloudStoragePath cloudPath = CloudStorageUtil.checkPath(dir);
     checkNotNull(filter);
     initStorage();
@@ -793,7 +800,7 @@ public DirectoryStream<Path> newDirectoryStream(Path dir, final Filter<? super P
         return new DirectoryStream<Path>() {
           @Override
           public Iterator<Path> iterator() {
-            return new LazyPathIterator(cloudPath.getFileSystem(), prefix, blobIterator, filter);
+            return new LazyPathIterator(cloudPath.getFileSystem(), prefix, blobIterator, filter, dir.isAbsolute());
           }
 
           @Override

java-storage-nio/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java

@@ -17,6 +17,7 @@
 package com.google.cloud.storage.contrib.nio.it;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.api.client.http.HttpResponseException;
@@ -538,6 +539,53 @@ public void testListFiles() throws IOException {
     }
   }
 
+  @Test
+  public void testRelativityOfResolve() throws IOException {
+    try (FileSystem fs = getTestBucket()) {
+      Path abs1 = fs.getPath("/dir");
+      Path abs2 = abs1.resolve("subdir/");
+      Path rel1 = fs.getPath("dir");
+      Path rel2 = rel1.resolve("subdir/");
+      // children of absolute paths are absolute,
+      // children of relative paths are relative.
+      assertThat(abs1.isAbsolute()).isTrue();
+      assertThat(abs2.isAbsolute()).isTrue();
+      assertThat(rel1.isAbsolute()).isFalse();
+      assertThat(rel2.isAbsolute()).isFalse();
+    }
+  }
+
+  @Test
+  public void testWalkFiles() throws IOException {
+    try (FileSystem fs = getTestBucket()) {
+      List<Path> goodPaths = new ArrayList<>();
+      List<Path> paths = new ArrayList<>();
+      goodPaths.add(fs.getPath("dir/angel"));
+      goodPaths.add(fs.getPath("dir/alone"));
+      paths.add(fs.getPath("dir/dir2/another_angel"));
+      paths.add(fs.getPath("atroot"));
+      paths.addAll(goodPaths);
+      for (Path path : paths) {
+        fillFile(storage, BUCKET, path.toString(), SML_SIZE);
+      }
+      // Given a relative path as starting point, walkFileTree must return only relative paths.
+      List<Path> relativePaths = PostTraversalWalker.walkFileTree(fs.getPath("dir/"));
+      for (Path p : relativePaths) {
+        assertWithMessage("Should have been relative: " + p.toString()).that(p.isAbsolute()).isFalse();
+      }
+      // The 5 paths are:
+      // dir/, dir/angel, dir/alone, dir/dir2/, dir/dir2/another_angel.
+      assertThat(relativePaths.size()).isEqualTo(5);
+
+      // Given an absolute path as starting point, walkFileTree must return only relative paths.
+      List<Path> absolutePaths = PostTraversalWalker.walkFileTree(fs.getPath("/dir/"));
+      for (Path p : absolutePaths) {
+        assertWithMessage("Should have been absolute: " + p.toString()).that(p.isAbsolute()).isTrue();
+      }
+      assertThat(absolutePaths.size()).isEqualTo(5);
+    }
+  }
+
 
   @Test
   public void testDeleteRecursive() throws IOException {
@@ -620,6 +668,32 @@ private String randomSuffix() {
     return "-" + rnd.nextInt(99999);
   }
 
+  private static class PostTraversalWalker extends SimpleFileVisitor<Path> {
+    private final List<Path> paths = new ArrayList<>();
+
+    // Traverse the tree, return the list of files and folders.
+    static public ImmutableList<Path> walkFileTree(Path start) throws IOException {
+      PostTraversalWalker walker = new PostTraversalWalker();
+      Files.walkFileTree(start, walker);
+      return walker.getPaths();
+    }
+
+    @Override
+    public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
+      paths.add(file);
+      return FileVisitResult.CONTINUE;
+    }
+
+    @Override
+    public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException {
+      paths.add(dir);
+      return FileVisitResult.CONTINUE;
+    }
+
+    public ImmutableList<Path> getPaths() {
+      return ImmutableList.copyOf(paths);
+    }
+  }
 
   private CloudStorageFileSystem getTestBucket() throws IOException {
     // in typical usage we use the single-argument version of forBucket