Browse Source

HADOOP-7352. FileSystem#listStatus should throw IOE upon access error. Contributed by John Zhuge.

Xiao Chen 8 years ago
parent
commit
efdf810cf9

+ 6 - 8
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java

@@ -1524,13 +1524,14 @@ public abstract class FileSystem extends Configured implements Closeable {
    * <p>
    * Does not guarantee to return the List of files/directories status in a
    * sorted order.
+   * <p>
+   * Will not return null. Expect IOException upon access error.
    * @param f given path
    * @return the statuses of the files/directories in the given patch
-   * @throws FileNotFoundException when the path does not exist;
-   *         IOException see specific implementation
+   * @throws FileNotFoundException when the path does not exist
+   * @throws IOException see specific implementation
    */
-  public abstract FileStatus[] listStatus(Path f) throws FileNotFoundException, 
-                                                         IOException;
+  public abstract FileStatus[] listStatus(Path f) throws IOException;
 
   /**
    * Represents a batch of directory entries when iteratively listing a
@@ -1600,10 +1601,7 @@ public abstract class FileSystem extends Configured implements Closeable {
   private void listStatus(ArrayList<FileStatus> results, Path f,
       PathFilter filter) throws FileNotFoundException, IOException {
     FileStatus listing[] = listStatus(f);
-    if (listing == null) {
-      throw new IOException("Error accessing " + f);
-    }
-
+    Preconditions.checkNotNull(listing, "listStatus should not return NULL");
     for (int i = 0; i < listing.length; i++) {
       if (filter.accept(listing[i].getPath())) {
         results.add(listing[i]);

+ 1 - 4
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java

@@ -466,10 +466,7 @@ public class RawLocalFileSystem extends FileSystem {
     }
 
     if (localf.isDirectory()) {
-      String[] names = localf.list();
-      if (names == null) {
-        return null;
-      }
+      String[] names = FileUtil.list(localf);
       results = new FileStatus[names.length];
       int j = 0;
       for (int i = 0; i < names.length; i++) {

+ 3 - 0
hadoop-common-project/hadoop-common/src/site/markdown/filesystem/filesystem.md

@@ -170,6 +170,9 @@ While HDFS currently returns an alphanumerically sorted list, neither the Posix
 nor Java's `File.listFiles()` API calls define any ordering of returned values. Applications
 which require a uniform sort order on the results must perform the sorting themselves.
 
+**Null return**: Local filesystems prior to 3.0.0 returned null upon access
+error. It is considered erroneous. Expect IOException upon access error.
+
 #### Atomicity and Consistency
 
 By the time the `listStatus()` operation returns to the caller, there

+ 21 - 3
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FSMainOperationsBaseTest.java

@@ -274,8 +274,7 @@ public abstract class FSMainOperationsBaseTest extends FileSystemTestHelper {
       // expected
     }
   }
-  
-  // TODO: update after fixing HADOOP-7352
+
   @Test
   public void testListStatusThrowsExceptionForUnreadableDir()
   throws Exception {
@@ -630,7 +629,26 @@ public abstract class FSMainOperationsBaseTest extends FileSystemTestHelper {
     Assert.assertTrue(containsTestRootPath(getTestRootPath(fSys, TEST_DIR_AXX),
         filteredPaths));
   }
-  
+
+  @Test
+  public void testGlobStatusThrowsExceptionForUnreadableDir()
+      throws Exception {
+    Path testRootDir = getTestRootPath(fSys, "test/hadoop/dir");
+    Path obscuredDir = new Path(testRootDir, "foo");
+    Path subDir = new Path(obscuredDir, "bar"); //so foo is non-empty
+    fSys.mkdirs(subDir);
+    fSys.setPermission(obscuredDir, new FsPermission((short)0)); //no access
+    try {
+      fSys.globStatus(getTestRootPath(fSys, "test/hadoop/dir/foo/*"));
+      Assert.fail("Should throw IOException");
+    } catch (IOException ioe) {
+      // expected
+    } finally {
+      // make sure the test directory can be deleted
+      fSys.setPermission(obscuredDir, new FsPermission((short)0755)); //default
+    }
+  }
+
   @Test
   public void testWriteReadAndDeleteEmptyFile() throws Exception {
     writeReadAndDelete(0);

+ 19 - 0
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestPathData.java

@@ -30,9 +30,11 @@ import java.util.Arrays;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.permission.FsPermission;
 import org.apache.hadoop.test.GenericTestUtils;
 import org.apache.hadoop.util.Shell;
 import org.junit.After;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -217,6 +219,23 @@ public class TestPathData {
     );
   }
 
+  @Test
+  public void testGlobThrowsExceptionForUnreadableDir() throws Exception {
+    Path obscuredDir = new Path("foo");
+    Path subDir = new Path(obscuredDir, "bar"); //so foo is non-empty
+    fs.mkdirs(subDir);
+    fs.setPermission(obscuredDir, new FsPermission((short)0)); //no access
+    try {
+      PathData.expandAsGlob("foo/*", conf);
+      Assert.fail("Should throw IOException");
+    } catch (IOException ioe) {
+      // expected
+    } finally {
+      // make sure the test directory can be deleted
+      fs.setPermission(obscuredDir, new FsPermission((short)0755)); //default
+    }
+  }
+
   @Test (timeout = 30000)
   public void testWithStringAndConfForBuggyPath() throws Exception {
     String dirString = "file:///tmp";

+ 2 - 4
hadoop-hdfs-project/hadoop-hdfs-client/src/test/java/org/apache/hadoop/hdfs/web/TestTokenAspect.java

@@ -34,7 +34,6 @@ import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
-import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
@@ -130,9 +129,8 @@ public class TestTokenAspect {
     }
 
     @Override
-    public FileStatus[] listStatus(Path f) throws FileNotFoundException,
-        IOException {
-      return null;
+    public FileStatus[] listStatus(Path f) throws IOException {
+      return new FileStatus[0];
     }
 
     @Override

+ 1 - 1
hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpWithAcls.java

@@ -196,7 +196,7 @@ public class TestDistCpWithAcls {
 
     @Override
     public FileStatus[] listStatus(Path f) throws IOException {
-      return null;
+      return new FileStatus[0];
     }
 
     @Override

+ 1 - 1
hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpWithXAttrs.java

@@ -226,7 +226,7 @@ public class TestDistCpWithXAttrs {
 
     @Override
     public FileStatus[] listStatus(Path f) throws IOException {
-      return null;
+      return new FileStatus[0];
     }
 
     @Override