abashev / vfs-s3

Amazon S3 driver for Apache commons-vfs (Virtual File System) project
Apache License 2.0
92 stars 50 forks source link

getChildren needs ending slash but won't accept one #22

Closed mslinn closed 10 years ago

mslinn commented 10 years ago

AWS CLI will not list the contents of a directory unless the path ends in a slash:

$ aws s3 ls s3://mslinntest/testDir
                           PRE testDir/
$ aws s3 ls s3://mslinntest/testDir/
                           PRE /
2014-09-13 15:18:19          0 
2014-09-13 15:35:52          0 testFile.txt

Seems the Java library works the same way, but vfs-s3 removes the trailing slash so getChildren() does not return the expected value:

import com.intridea.io.vfs.provider.s3.S3FileProvider;
import org.apache.commons.vfs2.FileObject;
import org.apache.commons.vfs2.FileSystemOptions;
import org.apache.commons.vfs2.VFS;
import org.apache.commons.vfs2.auth.StaticUserAuthenticator;
import org.apache.commons.vfs2.impl.DefaultFileSystemConfigBuilder;

import java.io.FileInputStream;
import java.util.Properties;

public class Test {
    public static void main(String[] args) throws Exception {
        Properties config = new Properties();
        config.load(new FileInputStream(System.getProperty("user.home") + "/.aws/config")); // same authentication file used by aws-cli
        StaticUserAuthenticator auth = new StaticUserAuthenticator(null, config.getProperty("aws_access_key_id"), config.getProperty("aws_secret_access_key"));
        FileSystemOptions opts = S3FileProvider.getDefaultFileSystemOptions();
        DefaultFileSystemConfigBuilder.getInstance().setUserAuthenticator(opts, auth);

        FileObject[] files = VFS.getManager().resolveFile("s3://mslinntest/testDir").getChildren();
        for (FileObject file : files)
          System.out.println(file);
    }
}

Output is:

Sep 13, 2014 7:00:57 PM com.intridea.io.vfs.provider.s3.S3FileProvider doCreateFileSystem
INFO: Initialize Amazon S3 service client ...
Sep 13, 2014 7:00:57 PM com.intridea.io.vfs.provider.s3.S3FileProvider doCreateFileSystem
INFO: ... Ok
Sep 13, 2014 7:00:57 PM com.intridea.io.vfs.provider.s3.S3FileSystem <init>
INFO: Created new S3 FileSystem mslinntest
Sep 13, 2014 7:00:59 PM com.intridea.io.vfs.provider.s3.S3FileObject doAttach
INFO: Attach folder to S3 Object: S3Object [key=testDir/, bucket=mslinntest, lastModified=Sat Sep 13 15:18:19 PDT 2014, dataInputStream=null, Metadata={ETag="d41d8cd98f00b204e9800998ecf8427e", Date=Sat Sep 13 19:01:00 PDT 2014, Content-Length=0, id-2=n7/EaF3YOkx4/oFnw0lQQ6NMjXfn2MbTQxUAhrqsHJdhzG+gx2Z6YY9rlIYnTUnE, request-id=A1047A519D88CDC3, Last-Modified=Sat Sep 13 15:18:19 PDT 2014, Content-Type=application/octet-stream}]
Exception in thread "main" org.apache.commons.vfs2.FileSystemException: Invalid descendent file name "/".
    at org.apache.commons.vfs2.impl.DefaultFileSystemManager.resolveName(DefaultFileSystemManager.java:791)
    at org.apache.commons.vfs2.provider.AbstractFileObject.getChildren(AbstractFileObject.java:710)
    at Test.main(Unknown Source)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:606)
    at com.intellij.rt.execution.application.AppMain.main(AppMain.java:134)

Should I not use getChildren() because there is a better way of listing an S3 directory?

Here is my project with both Scala and Java code that demonstrates this problem.

cuzz22000 commented 10 years ago

The problem actually is caused by S3FileObject not determining if the parent is a child directory of ROOT or ROOT itself. In the case the parent is ROOT then the NameScope should be NameScope.FILE_SYSTEM otherwise it is NameScope.CHILD. I updated the S3FileObject and created a unit test for listChildrenRoot , see the patch below.

diff --git a/src/main/java/com/intridea/io/vfs/provider/s3/S3FileObject.java b/src/main/java/com/intridea/io/vfs/provider/s3/S3FileObject.java
index 4f20f53..a77bbff 100644
--- a/src/main/java/com/intridea/io/vfs/provider/s3/S3FileObject.java
+++ b/src/main/java/com/intridea/io/vfs/provider/s3/S3FileObject.java
@@ -293,8 +293,8 @@
         for (String commonPrefix : commonPrefixes) {
             // strip path from name (leave only base name)
             final String stripPath = commonPrefix.substring(path.length());
-            FileObject childObject = resolveFile(stripPath, NameScope.CHILD);
-            if (childObject instanceof S3FileObject) {
+            FileObject childObject = resolveFile(stripPath, stripPath.equals("/")?NameScope.FILE_SYSTEM:NameScope.CHILD);
+            if (childObject instanceof S3FileObject && !stripPath.equals("/")) {
                 S3FileObject s3FileObject = (S3FileObject) childObject;
                 resolvedChildren.add(s3FileObject);
             }
diff --git a/src/test/java/com/intridea/io/vfs/provider/s3/S3ProviderTest.java b/src/test/java/com/intridea/io/vfs/provider/s3/S3ProviderTest.java
index 7bc7c47..54aa4e0 100644
--- a/src/test/java/com/intridea/io/vfs/provider/s3/S3ProviderTest.java
+++ b/src/test/java/com/intridea/io/vfs/provider/s3/S3ProviderTest.java
@@ -325,6 +325,13 @@
         FileObject[] children = baseDir.getChildren();
         assertEquals(children.length, 5);
     }
+    
+    @Test(dependsOnMethods={"createFileOk", "createDirOk","uploadBigFile"})
+    public void listChildrenRoot() throws FileSystemException {
+       FileObject root = fsManager.resolveFile("s3://" + bucketName + "/");
+       FileObject[] children = root.getChildren();
+       assertEquals(children.length, 2);
+    }

     @Test(dependsOnMethods={"createDirOk"})
     public void findFiles() throws FileSystemException {
mslinn commented 10 years ago

I have a Scala/sbt test project at https://github.com/mslinn/vfs-s3Test. The tests show a few issues:

abashev commented 10 years ago

@mslinn to be fair I was not able to reproduce your issue. Maybe you took old code or 2.1.x branch, please update to latest version and let me know does it fix everything or not.

And I don't see any problems with including code from @cuzz22000 into list of tests. I did a little refactoring for it so here you go - https://github.com/abashev/vfs-s3/commit/133a4844a0662291bb084b8ca507b12354c2565a