HADOOP-5045. FileSystem.isDirectory() should not be deprecated. (Suresh Srinivas via szetszwo)

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

CHANGES.txt

@@ -68,6 +68,9 @@ Trunk (unreleased changes)
 
     HADOOP-5078. Remove invalid AMI kernel in EC2 scripts. (tomwhite)
 
+    HADOOP-5045. FileSystem.isDirectory() should not be deprecated.  (Suresh
+    Srinivas via szetszwo)
+
 Release 0.20.0 - Unreleased
 
   INCOMPATIBLE CHANGES

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

@@ -625,8 +625,10 @@ public abstract class FileSystem extends Configured implements Closeable {
     }
   }
 
-  /** True iff the named path is a directory. */
-  /** @deprecated Use getFileStatus() instead */ @Deprecated
+  /** True iff the named path is a directory.
+   * Note: Avoid using this method. Instead reuse the FileStatus 
+   * returned by getFileStatus() or listStatus() methods.
+   */
   public boolean isDirectory(Path f) throws IOException {
     try {
       return getFileStatus(f).isDir();
@@ -635,7 +637,10 @@ public abstract class FileSystem extends Configured implements Closeable {
     }
   }
 
-  /** True iff the named path is a regular file. */
+  /** True iff the named path is a regular file.
+   * Note: Avoid using this method. Instead reuse the FileStatus 
+   * returned by getFileStatus() or listStatus() methods.
+   */
   public boolean isFile(Path f) throws IOException {
     try {
       return !getFileStatus(f).isDir();

src/core/org/apache/hadoop/fs/FileUtil.java

@@ -187,20 +187,30 @@ public class FileUtil {
                              boolean deleteSource,
                              boolean overwrite,
                              Configuration conf) throws IOException {
-    dst = checkDest(src.getName(), dstFS, dst, overwrite);
+    FileStatus fileStatus = srcFS.getFileStatus(src);
+    return copy(srcFS, fileStatus, dstFS, dst, deleteSource, overwrite, conf);
+  }
 
-    if (srcFS.getFileStatus(src).isDir()) {
+  /** Copy files between FileSystems. */
+  private static boolean copy(FileSystem srcFS, FileStatus srcStatus,
+                              FileSystem dstFS, Path dst,
+                              boolean deleteSource,
+                              boolean overwrite,
+                              Configuration conf) throws IOException {
+    Path src = srcStatus.getPath();
+    dst = checkDest(src.getName(), dstFS, dst, overwrite);
+    if (srcStatus.isDir()) {
       checkDependencies(srcFS, src, dstFS, dst);
       if (!dstFS.mkdirs(dst)) {
         return false;
       }
       FileStatus contents[] = srcFS.listStatus(src);
       for (int i = 0; i < contents.length; i++) {
-        copy(srcFS, contents[i].getPath(), dstFS, 
+        copy(srcFS, contents[i], dstFS,
              new Path(dst, contents[i].getPath().getName()),
              deleteSource, overwrite, conf);
       }
-    } else if (srcFS.isFile(src)) {
+    } else {
       InputStream in=null;
       OutputStream out = null;
       try {
@@ -212,8 +222,6 @@ public class FileUtil {
         IOUtils.closeStream(in);
         throw e;
       }
-    } else {
-      throw new IOException(src.toString() + ": No such file or directory");
     }
     if (deleteSource) {
       return srcFS.delete(src, true);
@@ -305,22 +313,28 @@ public class FileUtil {
   public static boolean copy(FileSystem srcFS, Path src, 
                              File dst, boolean deleteSource,
                              Configuration conf) throws IOException {
-    if (srcFS.getFileStatus(src).isDir()) {
+    FileStatus filestatus = srcFS.getFileStatus(src);
+    return copy(srcFS, filestatus, dst, deleteSource, conf);
+  }
+
+  /** Copy FileSystem files to local files. */
+  private static boolean copy(FileSystem srcFS, FileStatus srcStatus,
+                              File dst, boolean deleteSource,
+                              Configuration conf) throws IOException {
+    Path src = srcStatus.getPath();
+    if (srcStatus.isDir()) {
       if (!dst.mkdirs()) {
         return false;
       }
       FileStatus contents[] = srcFS.listStatus(src);
       for (int i = 0; i < contents.length; i++) {
-        copy(srcFS, contents[i].getPath(), 
+        copy(srcFS, contents[i],
              new File(dst, contents[i].getPath().getName()),
              deleteSource, conf);
       }
-    } else if (srcFS.isFile(src)) {
+    } else {
       InputStream in = srcFS.open(src);
       IOUtils.copyBytes(in, new FileOutputStream(dst), conf);
-    } else {
-      throw new IOException(src.toString() + 
-                            ": No such file or directory");
     }
     if (deleteSource) {
       return srcFS.delete(src, true);

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

@@ -189,7 +189,7 @@ public class FsShell extends Configured implements Tool {
       for (FileStatus status : srcs) {
         Path p = status.getPath();
         File f = dstIsDir? new File(dst, p.getName()): dst;
-        copyToLocal(srcFS, p, f, copyCrc);
+        copyToLocal(srcFS, status, f, copyCrc);
       }
     }
   }
@@ -220,7 +220,7 @@ public class FsShell extends Configured implements Tool {
    * @param copyCrc copy CRC files?
    * @exception IOException If some IO failed
    */
-  private void copyToLocal(final FileSystem srcFS, final Path src,
+  private void copyToLocal(final FileSystem srcFS, final FileStatus srcStatus,
                            final File dst, final boolean copyCrc)
     throws IOException {
     /* Keep the structure similar to ChecksumFileSystem.copyToLocal(). 
@@ -229,7 +229,8 @@ public class FsShell extends Configured implements Tool {
      * copyCrc and useTmpFile (may be useTmpFile need not be an option).
      */
     
-    if (!srcFS.getFileStatus(src).isDir()) {
+    Path src = srcStatus.getPath();
+    if (!srcStatus.isDir()) {
       if (dst.exists()) {
         // match the error message in FileUtil.checkDest():
         throw new IOException("Target " + dst + " already exists");
@@ -251,15 +252,19 @@ public class FsShell extends Configured implements Tool {
         ChecksumFileSystem csfs = (ChecksumFileSystem) srcFS;
         File dstcs = FileSystem.getLocal(srcFS.getConf())
           .pathToFile(csfs.getChecksumFile(new Path(dst.getCanonicalPath())));
-        copyToLocal(csfs.getRawFileSystem(), csfs.getChecksumFile(src),
-                    dstcs, false);
+        FileSystem fs = csfs.getRawFileSystem();
+        FileStatus status = csfs.getFileStatus(csfs.getChecksumFile(src));
+        copyToLocal(fs, status, dstcs, false);
       } 
     } else {
       // once FileUtil.copy() supports tmp file, we don't need to mkdirs().
-      dst.mkdirs();
-      for(FileStatus path : srcFS.listStatus(src)) {
-        copyToLocal(srcFS, path.getPath(), 
-                    new File(dst, path.getPath().getName()), copyCrc);
+      if (!dst.mkdirs()) {
+        throw new IOException("Failed to create local destination \"" +
+                              dst + "\".");
+      }
+      for(FileStatus status : srcFS.listStatus(src)) {
+        copyToLocal(srcFS, status,
+                    new File(dst, status.getPath().getName()), copyCrc);
       }
     }
   }
@@ -823,31 +828,30 @@ public class FsShell extends Configured implements Tool {
   void rename(String srcf, String dstf) throws IOException {
     Path srcPath = new Path(srcf);
     Path dstPath = new Path(dstf);
-    FileSystem srcFs = srcPath.getFileSystem(getConf());
-    FileSystem dstFs = dstPath.getFileSystem(getConf());
-    URI srcURI = srcFs.getUri();
-    URI dstURI = dstFs.getUri();
+    FileSystem fs = srcPath.getFileSystem(getConf());
+    URI srcURI = fs.getUri();
+    URI dstURI = dstPath.getFileSystem(getConf()).getUri();
     if (srcURI.compareTo(dstURI) != 0) {
       throw new IOException("src and destination filesystems do not match.");
     }
-    Path[] srcs = FileUtil.stat2Paths(srcFs.globStatus(srcPath), srcPath);
+    Path[] srcs = FileUtil.stat2Paths(fs.globStatus(srcPath), srcPath);
     Path dst = new Path(dstf);
-    if (srcs.length > 1 && !srcFs.isDirectory(dst)) {
+    if (srcs.length > 1 && !fs.isDirectory(dst)) {
       throw new IOException("When moving multiple files, " 
                             + "destination should be a directory.");
     }
     for(int i=0; i<srcs.length; i++) {
-      if (!srcFs.rename(srcs[i], dst)) {
+      if (!fs.rename(srcs[i], dst)) {
         FileStatus srcFstatus = null;
         FileStatus dstFstatus = null;
         try {
-          srcFstatus = srcFs.getFileStatus(srcs[i]);
+          srcFstatus = fs.getFileStatus(srcs[i]);
         } catch(FileNotFoundException e) {
           throw new FileNotFoundException(srcs[i] + 
           ": No such file or directory");
         }
         try {
-          dstFstatus = dstFs.getFileStatus(dst);
+          dstFstatus = fs.getFileStatus(dst);
         } catch(IOException e) {
         }
         if((srcFstatus!= null) && (dstFstatus!= null)) {
@@ -1084,11 +1088,12 @@ public class FsShell extends Configured implements Tool {
     boolean foption = c.getOpt("f") ? true: false;
     path = new Path(src);
     FileSystem srcFs = path.getFileSystem(getConf());
-    if (srcFs.isDirectory(path)) {
+    FileStatus fileStatus = srcFs.getFileStatus(path);
+    if (fileStatus.isDir()) {
       throw new IOException("Source must be a file.");
     }
 
-    long fileSize = srcFs.getFileStatus(path).getLen();
+    long fileSize = fileStatus.getLen();
     long offset = (fileSize > 1024) ? fileSize - 1024: 0;
 
     while (true) {

src/test/org/apache/hadoop/fs/DistributedFSCheck.java

@@ -106,11 +106,19 @@ public class DistributedFSCheck extends TestCase {
   private void listSubtree(Path rootFile,
                            SequenceFile.Writer writer
                            ) throws IOException {
-    if (!fs.isDirectory(rootFile)) {
+    FileStatus rootStatus = fs.getFileStatus(rootFile);
+    listSubtree(rootStatus, writer);
+  }
+
+  private void listSubtree(FileStatus rootStatus,
+                           SequenceFile.Writer writer
+                           ) throws IOException {
+    Path rootFile = rootStatus.getPath();
+    if (!rootStatus.isDir()) {
       nrFiles++;
       // For a regular file generate <fName,offset> pairs
       long blockSize = fs.getDefaultBlockSize();
-      long fileLength = fs.getFileStatus(rootFile).getLen();
+      long fileLength = rootStatus.getLen();
       for(long offset = 0; offset < fileLength; offset += blockSize)
         writer.append(new UTF8(rootFile.toString()), new LongWritable(offset));
       return;
@@ -120,7 +128,7 @@ public class DistributedFSCheck extends TestCase {
     if (children == null)
       throw new IOException("Could not get listing for " + rootFile);
     for (int i = 0; i < children.length; i++)
-      listSubtree(children[i].getPath(), writer);
+      listSubtree(children[i], writer);
   }
 
   /**