HADOOP-6201. Change FileSystem::listStatus contract to throw

FileNotFoundException if the directory does not exist, rather than letting
this be implementation-specific. Contributed by Jakob Homan.


git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@806745 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -74,6 +74,10 @@ Trunk (unreleased changes)
     MAPREDUCE-711. Removed Distributed Cache from Common, to move it
     under Map/Reduce. (Vinod Kumar Vavilapalli via yhemanth)
 
+    HADOOP-6201. Change FileSystem::listStatus contract to throw
+    FileNotFoundException if the directory does not exist, rather than letting
+    this be implementation-specific. (Jakob Homan via cdouglas)
+
   NEW FEATURES
 
     HADOOP-4268. Change fsck to use ClientProtocol methods so that the

src/java/org/apache/hadoop/fs/FileSystem.java

@@ -731,25 +731,25 @@ public abstract class FileSystem extends Configured implements Closeable {
    * List the statuses of the files/directories in the given path if the path is
    * a directory.
    * 
-   * @param f
-   *          given path
+   * @param f given path
    * @return the statuses of the files/directories in the given patch
-   * @throws IOException
+   * @throws FileNotFoundException when the path does not exist;
+   *         IOException see specific implementation
    */
-  public abstract FileStatus[] listStatus(Path f) throws IOException;
+  public abstract FileStatus[] listStatus(Path f) throws FileNotFoundException, 
+                                                         IOException;
     
   /*
    * Filter files/directories in the given path using the user-supplied path
    * filter. Results are added to the given array <code>results</code>.
    */
   private void listStatus(ArrayList<FileStatus> results, Path f,
-      PathFilter filter) throws IOException {
+      PathFilter filter) throws FileNotFoundException, IOException {
     FileStatus listing[] = listStatus(f);
-    if (listing != null) {
-      for (int i = 0; i < listing.length; i++) {
-        if (filter.accept(listing[i].getPath())) {
-          results.add(listing[i]);
-        }
+
+    for (int i = 0; i < listing.length; i++) {
+      if (filter.accept(listing[i].getPath())) {
+        results.add(listing[i]);
       }
     }
   }
@@ -764,10 +764,11 @@ public abstract class FileSystem extends Configured implements Closeable {
    *          the user-supplied path filter
    * @return an array of FileStatus objects for the files under the given path
    *         after applying the filter
-   * @throws IOException
-   *           if encounter any problem while fetching the status
+   * @throws FileNotFoundException when the path does not exist;
+   *         IOException see specific implementation   
    */
-  public FileStatus[] listStatus(Path f, PathFilter filter) throws IOException {
+  public FileStatus[] listStatus(Path f, PathFilter filter) 
+                                   throws FileNotFoundException, IOException {
     ArrayList<FileStatus> results = new ArrayList<FileStatus>();
     listStatus(results, f, filter);
     return results.toArray(new FileStatus[results.size()]);
@@ -781,10 +782,11 @@ public abstract class FileSystem extends Configured implements Closeable {
    *          a list of paths
    * @return a list of statuses for the files under the given paths after
    *         applying the filter default Path filter
-   * @exception IOException
+   * @throws FileNotFoundException when the path does not exist;
+   *         IOException see specific implementation
    */
   public FileStatus[] listStatus(Path[] files)
-      throws IOException {
+      throws FileNotFoundException, IOException {
     return listStatus(files, DEFAULT_FILTER);
   }
 
@@ -798,10 +800,11 @@ public abstract class FileSystem extends Configured implements Closeable {
    *          the user-supplied path filter
    * @return a list of statuses for the files under the given paths after
    *         applying the filter
-   * @exception IOException
+   * @throws FileNotFoundException when the path does not exist;
+   *         IOException see specific implementation
    */
   public FileStatus[] listStatus(Path[] files, PathFilter filter)
-      throws IOException {
+      throws FileNotFoundException, IOException {
     ArrayList<FileStatus> results = new ArrayList<FileStatus>();
     for (int i = 0; i < files.length; i++) {
       listStatus(results, files[i], filter);

src/java/org/apache/hadoop/fs/FsShell.java

@@ -532,17 +532,18 @@ public class FsShell extends Configured implements Tool {
       return;
     }
     FileStatus items[] = srcFs.listStatus(src);
-    if (items == null) {
+    try {
+      items = srcFs.listStatus(src);
+    } catch (FileNotFoundException fnfe) {
       throw new IOException("Could not get listing for " + src);
-    } else {
+    }
 
-      for (int i = 0; i < items.length; i++) {
-        if (!items[i].isDir()) {
-          setFileReplication(items[i].getPath(), srcFs, newRep, waitingList);
-        } else if (recursive) {
-          setReplication(newRep, srcFs, items[i].getPath(), recursive, 
-                         waitingList);
-        }
+    for (int i = 0; i < items.length; i++) {
+      if (!items[i].isDir()) {
+        setFileReplication(items[i].getPath(), srcFs, newRep, waitingList);
+      } else if (recursive) {
+        setReplication(newRep, srcFs, items[i].getPath(), recursive, 
+                       waitingList);
       }
     }
   }
@@ -706,7 +707,11 @@ public class FsShell extends Configured implements Tool {
         statusToPrint = globStatus;
       } else {
         Path statPaths[] = FileUtil.stat2Paths(globStatus, srcPath);
-        statusToPrint = srcFs.listStatus(statPaths);
+        try {
+          statusToPrint = srcFs.listStatus(statPaths);
+        } catch(FileNotFoundException fnfe) {
+          statusToPrint = null;
+        }
       }
       if ((statusToPrint == null) || ((statusToPrint.length == 0) &&
                                       (!srcFs.exists(srcPath)))){
@@ -1234,11 +1239,10 @@ public class FsShell extends Configured implements Tool {
     Path path = src.getPath();
     try {
       FileStatus[] files = srcFs.listStatus(path);
-      if ( files == null ) {
-        System.err.println(cmd + 
-                           ": could not get listing for '" + path + "'");
-      }
+
       return files;
+    } catch(FileNotFoundException fnfe) {
+      System.err.println(cmd + ": could not get listing for '" + path + "'");
     } catch (IOException e) {
       System.err.println(cmd + 
                          ": could not get get listing for '" + path + "' : " +

src/java/org/apache/hadoop/fs/RawLocalFileSystem.java

@@ -285,7 +285,7 @@ public class RawLocalFileSystem extends FileSystem {
     FileStatus[] results;
 
     if (!localf.exists()) {
-      return null;
+      throw new FileNotFoundException("File " + f + " does not exist.");
     }
     if (localf.isFile()) {
       return new FileStatus[] {

src/java/org/apache/hadoop/fs/Trash.java

@@ -170,10 +170,14 @@ public class Trash extends Configured {
 
   /** Delete old checkpoints. */
   public void expunge() throws IOException {
-    FileStatus[] dirs = fs.listStatus(trash);            // scan trash sub-directories
-    if( dirs == null){
+    FileStatus[] dirs = null;
+    
+    try {
+      dirs = fs.listStatus(trash);            // scan trash sub-directories
+    } catch (FileNotFoundException fnfe) {
       return;
     }
+
     long now = System.currentTimeMillis();
     for (int i = 0; i < dirs.length; i++) {
       Path path = dirs[i].getPath();
@@ -253,9 +257,6 @@ public class Trash extends Configured {
               continue;
             }
 
-            if (homes == null)
-              continue;
-
             for (FileStatus home : homes) {         // dump each trash
               if (!home.isDir())
                 continue;

src/java/org/apache/hadoop/fs/kfs/KosmosFileSystem.java

@@ -142,6 +142,9 @@ public class KosmosFileSystem extends FileSystem {
         Path absolute = makeAbsolute(path);
         String srep = absolute.toUri().getPath();
 
+        if(!kfsImpl.exists(srep))
+          throw new FileNotFoundException("File " + path + " does not exist.");
+
         if (kfsImpl.isFile(srep))
                 return new FileStatus[] { getFileStatus(path) } ;
 
@@ -249,15 +252,13 @@ public class KosmosFileSystem extends FileSystem {
         return kfsImpl.remove(srep) == 0;
 
       FileStatus[] dirEntries = listStatus(absolute);
-      if ((!recursive) && (dirEntries != null) && 
-            (dirEntries.length != 0)) {
+      if (!recursive && (dirEntries.length != 0)) {
         throw new IOException("Directory " + path.toString() + 
         " is not empty.");
       }
-      if (dirEntries != null) {
-        for (int i = 0; i < dirEntries.length; i++) {
-          delete(new Path(absolute, dirEntries[i].getPath()), recursive);
-        }
+
+      for (int i = 0; i < dirEntries.length; i++) {
+        delete(new Path(absolute, dirEntries[i].getPath()), recursive);
       }
       return kfsImpl.rmdir(srep) == 0;
     }

src/java/org/apache/hadoop/fs/s3/S3FileSystem.java

@@ -178,7 +178,7 @@ public class S3FileSystem extends FileSystem {
     Path absolutePath = makeAbsolute(f);
     INode inode = store.retrieveINode(absolutePath);
     if (inode == null) {
-      return null;
+      throw new FileNotFoundException("File " + f + " does not exist.");
     }
     if (inode.isFile()) {
       return new FileStatus[] {
@@ -303,10 +303,13 @@ public class S3FileSystem extends FileSystem {
        store.deleteBlock(block);
      }
    } else {
-     FileStatus[] contents = listStatus(absolutePath);
-     if (contents == null) {
+     FileStatus[] contents = null; 
+     try {
+       contents = listStatus(absolutePath);
+     } catch(FileNotFoundException fnfe) {
        return false;
      }
+
      if ((contents.length !=0) && (!recursive)) {
        throw new IOException("Directory " + path.toString() 
            + " is not empty.");

src/java/org/apache/hadoop/fs/s3native/NativeS3FileSystem.java

@@ -456,7 +456,7 @@ public class NativeS3FileSystem extends FileSystem {
     
     if (status.isEmpty() &&
         store.retrieveMetadata(key + FOLDER_SUFFIX) == null) {
-      return null;
+      throw new FileNotFoundException("File " + f + " does not exist.");
     }
     
     return status.toArray(new FileStatus[status.size()]);

src/test/core/org/apache/hadoop/fs/FileSystemContractBaseTest.java

@@ -162,8 +162,13 @@ public abstract class FileSystemContractBaseTest extends TestCase {
     }
   }
   
-  public void testListStatusReturnsNullForNonExistentFile() throws Exception {
-    assertNull(fs.listStatus(path("/test/hadoop/file")));
+  public void testListStatusThrowsExceptionForNonExistentFile() throws Exception {
+    try {
+      fs.listStatus(path("/test/hadoop/file"));
+      fail("Should throw FileNotFoundException");
+    } catch (FileNotFoundException fnfe) {
+      // expected
+    }
   }
   
   public void testListStatus() throws Exception {

src/test/core/org/apache/hadoop/fs/kfs/KFSEmulationImpl.java

@@ -20,6 +20,7 @@
 
 package org.apache.hadoop.fs.kfs;
 
+import java.io.FileNotFoundException;
 import java.io.IOException;
 
 import org.apache.hadoop.conf.Configuration;
@@ -50,11 +51,12 @@ public class KFSEmulationImpl implements IFSImpl {
 
     public String[] readdir(String path) throws IOException {
         FileStatus[] p = localFS.listStatus(new Path(path));
-        String[] entries = null;
-
-        if (p == null) {
-            return null;
+        try {
+          p = localFS.listStatus(new Path(path));
+        } catch ( FileNotFoundException fnfe ) {
+          return null;
         }
+        String[] entries = null;
 
         entries = new String[p.length];
         for (int i = 0; i < p.length; i++)

src/test/core/org/apache/hadoop/fs/loadGenerator/LoadGenerator.java

@@ -545,16 +545,15 @@ public class LoadGenerator extends Configured implements Tool {
    */
   private void initFileDirTables(Path path) throws IOException {
     FileStatus[] stats = fs.listStatus(path);
-    if (stats != null) { 
-      for (FileStatus stat : stats) {
-        if (stat.isDir()) {
-          dirs.add(stat.getPath().toString());
-          initFileDirTables(stat.getPath());
-        } else {
-          Path filePath = stat.getPath();
-          if (filePath.getName().startsWith(StructureGenerator.FILE_NAME_PREFIX)) {
-            files.add(filePath.toString());
-          }
+
+    for (FileStatus stat : stats) {
+      if (stat.isDir()) {
+        dirs.add(stat.getPath().toString());
+        initFileDirTables(stat.getPath());
+      } else {
+        Path filePath = stat.getPath();
+        if (filePath.getName().startsWith(StructureGenerator.FILE_NAME_PREFIX)) {
+          files.add(filePath.toString());
         }
       }
     }