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/trunk@1507429 13f79535-47bb-0310-9956-ffa450edef68

hadoop-common-project/hadoop-common/CHANGES.txt

@@ -652,6 +652,9 @@ Release 2.1.0-beta - 2013-07-02
     HADOOP-9773. TestLightWeightCache should not set size limit to zero when
     testing it.  (szetszwo)
 
+    HADOOP-9507. LocalFileSystem rename() is broken in some cases when
+    destination exists. (cnauroth)
+
   BREAKDOWN OF HADOOP-8562 SUBTASKS AND RELATED JIRAS
 
     HADOOP-8924. Hadoop Common creating package-info.java must not depend on

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

@@ -319,9 +319,35 @@ public FSDataOutputStream createNonRecursive(Path f, FsPermission permission,
 
   @Override
   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;
     }
+
+    // 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.isDirectory() && 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());
   }
   

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFileSystem.java

@@ -422,6 +422,88 @@ public void testBufferedFSInputStream() throws IOException {
       stm.close();
     }
   }
+
+  /**
+   * Tests a simple rename of a directory.
+   */
+  @Test
+  public void testRenameDirectory() throws IOException {
+    Path src = new Path(TEST_ROOT_DIR, "dir1");
+    Path dst = new Path(TEST_ROOT_DIR, "dir2");
+    fileSys.delete(src, true);
+    fileSys.delete(dst, true);
+    assertTrue(fileSys.mkdirs(src));
+    assertTrue(fileSys.rename(src, dst));
+    assertTrue(fileSys.exists(dst));
+    assertFalse(fileSys.exists(src));
+  }
+
+  /**
+   * 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
+   */
+  @Test
+  public void testRenameReplaceExistingEmptyDirectory() throws IOException {
+    Path src = new Path(TEST_ROOT_DIR, "dir1");
+    Path dst = new Path(TEST_ROOT_DIR, "dir2");
+    fileSys.delete(src, true);
+    fileSys.delete(dst, true);
+    assertTrue(fileSys.mkdirs(src));
+    writeFile(fileSys, new Path(src, "file1"), 1);
+    writeFile(fileSys, new Path(src, "file2"), 1);
+    assertTrue(fileSys.mkdirs(dst));
+    assertTrue(fileSys.rename(src, dst));
+    assertTrue(fileSys.exists(dst));
+    assertTrue(fileSys.exists(new Path(dst, "file1")));
+    assertTrue(fileSys.exists(new Path(dst, "file2")));
+    assertFalse(fileSys.exists(src));
+  }
+
+  /**
+   * 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
+   */
+  @Test
+  public void testRenameMoveToExistingNonEmptyDirectory() throws IOException {
+    Path src = new Path(TEST_ROOT_DIR, "dir1/dir2/dir3");
+    Path dst = new Path(TEST_ROOT_DIR, "dir1");
+    fileSys.delete(src, true);
+    fileSys.delete(dst, true);
+    assertTrue(fileSys.mkdirs(src));
+    writeFile(fileSys, new Path(src, "file1"), 1);
+    writeFile(fileSys, new Path(src, "file2"), 1);
+    assertTrue(fileSys.exists(dst));
+    assertTrue(fileSys.rename(src, dst));
+    assertTrue(fileSys.exists(dst));
+    assertTrue(fileSys.exists(new Path(dst, "dir3")));
+    assertTrue(fileSys.exists(new Path(dst, "dir3/file1")));
+    assertTrue(fileSys.exists(new Path(dst, "dir3/file2")));
+    assertFalse(fileSys.exists(src));
+  }
   
   private void verifyRead(FSDataInputStream stm, byte[] fileContents,
        int seekOff, int toRead) throws IOException {