HADOOP-9507. LocalFileSystem rename() is broken in some cases when destination exists. Contributed by Chris Nauroth.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-1@1507442 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -102,6 +102,9 @@ Release 1.3.0 - unreleased
     MAPREDUCE-5288. ResourceEstimator#getEstimatedTotalMapOutputSize suffers 
     from divide by zero issues. (kkambatl via tucu)
 
+    HADOOP-9507. LocalFileSystem rename() is broken in some cases when
+    destination exists. (cnauroth)
+
 Release 1.2.1 - 2013.07.06
 
   INCOMPATIBLE CHANGES

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

@@ -285,10 +285,35 @@ public class RawLocalFileSystem extends FileSystem {
   }
 
   public boolean rename(Path src, Path dst) throws IOException {
-    if (pathToFile(src).renameTo(pathToFile(dst))) {
+    // Attempt rename using Java API.
+    File srcFile = pathToFile(src);
+    File dstFile = pathToFile(dst);
+    if (srcFile.renameTo(dstFile)) {
       return true;
     }
-    LOG.debug("Falling through to a copy of " + src + " to " + dst);
+
+    // Enforce POSIX rename behavior that a source directory replaces an existing
+    // destination if the destination is an empty directory.  On most platforms,
+    // this is already handled by the Java API call above.  Some platforms
+    // (notably Windows) do not provide this behavior, so the Java API call above
+    // fails.  Delete destination and attempt rename again.
+    if (this.exists(dst)) {
+      FileStatus sdst = this.getFileStatus(dst);
+      if (sdst.isDir() && dstFile.list().length == 0) {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("Deleting empty destination and renaming " + src + " to " +
+            dst);
+        }
+        if (this.delete(dst, false) && srcFile.renameTo(dstFile)) {
+          return true;
+        }
+      }
+    }
+
+    // The fallback behavior accomplishes the rename by a full copy.
+    if (LOG.isDebugEnabled()) {
+      LOG.debug("Falling through to a copy of " + src + " to " + dst);
+    }
     return FileUtil.copy(this, src, this, dst, true, getConf());
   }
   

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

@@ -18,6 +18,7 @@
 package org.apache.hadoop.fs;
 
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.io.IOUtils;
 import org.apache.hadoop.util.StringUtils;
 import java.io.*;
 import java.util.Arrays;
@@ -259,6 +260,100 @@ public class TestLocalFileSystem extends TestCase {
       stm.close();
     }
   }
+
+  /**
+   * Tests a simple rename of a directory.
+   */
+  public void testRenameDirectory() throws IOException {
+    FileSystem fs = FileSystem.getLocal(new Configuration());
+    Path src = new Path(TEST_ROOT_DIR, "dir1");
+    Path dst = new Path(TEST_ROOT_DIR, "dir2");
+    try {
+      fs.delete(src, true);
+      fs.delete(dst, true);
+      assertTrue(fs.mkdirs(src));
+      assertTrue(fs.rename(src, dst));
+      assertTrue(fs.exists(dst));
+      assertFalse(fs.exists(src));
+    } finally {
+      cleanupFileSystem(fs, src, dst);
+    }
+  }
+
+  /**
+   * Tests that renaming a directory replaces the destination if the destination
+   * is an existing empty directory.
+   * 
+   * Before:
+   *   /dir1
+   *     /file1
+   *     /file2
+   *   /dir2
+   * 
+   * After rename("/dir1", "/dir2"):
+   *   /dir2
+   *     /file1
+   *     /file2
+   */
+  public void testRenameReplaceExistingEmptyDirectory() throws IOException {
+    FileSystem fs = FileSystem.getLocal(new Configuration());
+    Path src = new Path(TEST_ROOT_DIR, "dir1");
+    Path dst = new Path(TEST_ROOT_DIR, "dir2");
+    try {
+      fs.delete(src, true);
+      fs.delete(dst, true);
+      assertTrue(fs.mkdirs(src));
+      writeFile(fs, new Path(src, "file1"));
+      writeFile(fs, new Path(src, "file2"));
+      assertTrue(fs.mkdirs(dst));
+      assertTrue(fs.rename(src, dst));
+      assertTrue(fs.exists(dst));
+      assertTrue(fs.exists(new Path(dst, "file1")));
+      assertTrue(fs.exists(new Path(dst, "file2")));
+      assertFalse(fs.exists(src));
+    } finally {
+      cleanupFileSystem(fs, src, dst);
+    }
+  }
+
+  /**
+   * Tests that renaming a directory to an existing directory that is not empty
+   * results in a full copy of source to destination.
+   * 
+   * Before:
+   *   /dir1
+   *     /dir2
+   *       /dir3
+   *         /file1
+   *         /file2
+   * 
+   * After rename("/dir1/dir2/dir3", "/dir1"):
+   *   /dir1
+   *     /dir3
+   *       /file1
+   *       /file2
+   */
+  public void testRenameMoveToExistingNonEmptyDirectory() throws IOException {
+    FileSystem fs = FileSystem.getLocal(new Configuration());
+    Path src = new Path(TEST_ROOT_DIR, "dir1/dir2/dir3");
+    Path dst = new Path(TEST_ROOT_DIR, "dir1");
+    try {
+      fs.delete(src, true);
+      fs.delete(dst, true);
+      assertTrue(fs.mkdirs(src));
+      writeFile(fs, new Path(src, "file1"));
+      writeFile(fs, new Path(src, "file2"));
+      assertTrue(fs.exists(dst));
+      assertTrue(fs.rename(src, dst));
+      assertTrue(fs.exists(dst));
+      assertTrue(fs.exists(new Path(dst, "dir3")));
+      assertTrue(fs.exists(new Path(dst, "dir3/file1")));
+      assertTrue(fs.exists(new Path(dst, "dir3/file2")));
+      assertFalse(fs.exists(src));
+    } finally {
+      cleanupFileSystem(fs, src, dst);
+    }
+  }
   
   private void verifyRead(FSDataInputStream stm, byte[] fileContents,
        int seekOff, int toRead) throws IOException {
@@ -275,4 +370,31 @@ public class TestLocalFileSystem extends TestCase {
       fail(s);
     }
   }
+
+  /**
+   * Cleans up the file system by deleting the given paths and closing the file
+   * system.
+   * 
+   * @param fs FileSystem to clean up
+   * @param paths Path... any number of paths to delete
+   */
+  private static void cleanupFileSystem(FileSystem fs, Path... paths) {
+    for (Path path: paths) {
+      deleteQuietly(fs, path);
+    }
+    IOUtils.cleanup(null, fs);
+  }
+
+  /**
+   * Deletes the given path and silences any exceptions.
+   * 
+   * @param fs FileSystem to perform the delete
+   * @param path Path to delete
+   */
+  private static void deleteQuietly(FileSystem fs, Path path) {
+    try {
+      fs.delete(path, true);
+    } catch (IOException e) {
+    }
+  }
 }