hadoop-common-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From x...@apache.org
Subject hadoop git commit: HADOOP-7352. FileSystem#listStatus should throw IOE upon access error. Contributed by John Zhuge.
Date Wed, 19 Oct 2016 01:19:28 GMT
Repository: hadoop
Updated Branches:
  refs/heads/trunk 29caf6d7d -> efdf810cf


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


Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/efdf810c
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/efdf810c
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/efdf810c

Branch: refs/heads/trunk
Commit: efdf810cf9f72d78e97e860576c64a382ece437c
Parents: 29caf6d
Author: Xiao Chen <xiao@apache.org>
Authored: Tue Oct 18 18:18:43 2016 -0700
Committer: Xiao Chen <xiao@apache.org>
Committed: Tue Oct 18 18:18:43 2016 -0700

----------------------------------------------------------------------
 .../java/org/apache/hadoop/fs/FileSystem.java   | 14 +++++-------
 .../apache/hadoop/fs/RawLocalFileSystem.java    |  5 +---
 .../src/site/markdown/filesystem/filesystem.md  |  3 +++
 .../hadoop/fs/FSMainOperationsBaseTest.java     | 24 +++++++++++++++++---
 .../apache/hadoop/fs/shell/TestPathData.java    | 19 ++++++++++++++++
 .../apache/hadoop/hdfs/web/TestTokenAspect.java |  6 ++---
 .../apache/hadoop/tools/TestDistCpWithAcls.java |  2 +-
 .../hadoop/tools/TestDistCpWithXAttrs.java      |  2 +-
 8 files changed, 54 insertions(+), 21 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hadoop/blob/efdf810c/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
index cc062c4..39b5b95 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
+++ b/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]);

http://git-wip-us.apache.org/repos/asf/hadoop/blob/efdf810c/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java
index 0fcddcf..5e6cb05 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java
+++ b/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++) {

http://git-wip-us.apache.org/repos/asf/hadoop/blob/efdf810c/hadoop-common-project/hadoop-common/src/site/markdown/filesystem/filesystem.md
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/site/markdown/filesystem/filesystem.md
b/hadoop-common-project/hadoop-common/src/site/markdown/filesystem/filesystem.md
index d927b8b..063bd97 100644
--- a/hadoop-common-project/hadoop-common/src/site/markdown/filesystem/filesystem.md
+++ b/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

http://git-wip-us.apache.org/repos/asf/hadoop/blob/efdf810c/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FSMainOperationsBaseTest.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FSMainOperationsBaseTest.java
b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FSMainOperationsBaseTest.java
index e862db4..35fd9be 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FSMainOperationsBaseTest.java
+++ b/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);

http://git-wip-us.apache.org/repos/asf/hadoop/blob/efdf810c/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestPathData.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestPathData.java
b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestPathData.java
index f2656e6..921fc18 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestPathData.java
+++ b/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";

http://git-wip-us.apache.org/repos/asf/hadoop/blob/efdf810c/hadoop-hdfs-project/hadoop-hdfs-client/src/test/java/org/apache/hadoop/hdfs/web/TestTokenAspect.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/test/java/org/apache/hadoop/hdfs/web/TestTokenAspect.java
b/hadoop-hdfs-project/hadoop-hdfs-client/src/test/java/org/apache/hadoop/hdfs/web/TestTokenAspect.java
index 36fd821..d70484d 100644
--- a/hadoop-hdfs-project/hadoop-hdfs-client/src/test/java/org/apache/hadoop/hdfs/web/TestTokenAspect.java
+++ b/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

http://git-wip-us.apache.org/repos/asf/hadoop/blob/efdf810c/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpWithAcls.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpWithAcls.java
b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpWithAcls.java
index e839991..49613ba 100644
--- a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpWithAcls.java
+++ b/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

http://git-wip-us.apache.org/repos/asf/hadoop/blob/efdf810c/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpWithXAttrs.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpWithXAttrs.java
b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpWithXAttrs.java
index 6b0e2b22..96193e8 100644
--- a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpWithXAttrs.java
+++ b/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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org


Mime
View raw message