From 58ba16e85f56bc1899d862c9f56d7809d0ab67f5 Mon Sep 17 00:00:00 2001
From: Robert Joseph Evans <bobby@apache.org>
Date: Thu, 2 Feb 2012 17:12:44 +0000
Subject: [PATCH] HADOOP-8001  ChecksumFileSystem's rename doesn't correctly
 handle checksum files (Daryn Sharp via bobby)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1239727 13f79535-47bb-0310-9956-ffa450edef68
---
 .../hadoop-common/CHANGES.txt                 |  3 ++
 .../apache/hadoop/fs/ChecksumFileSystem.java  | 17 +++---
 .../hadoop/fs/TestChecksumFileSystem.java     | 54 +++++++++++++++++++
 3 files changed, 67 insertions(+), 7 deletions(-)

diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt
index 874f9545a08..16dfdbcbf56 100644
--- a/hadoop-common-project/hadoop-common/CHANGES.txt
+++ b/hadoop-common-project/hadoop-common/CHANGES.txt
@@ -81,6 +81,9 @@ Trunk (unreleased changes)
 
   BUG FIXES
 
+    HADOOP-8001  ChecksumFileSystem's rename doesn't correctly handle checksum
+                 files. (Daryn Sharp via bobby)
+
     HADOOP-8006  TestFSInputChecker is failing in trunk.
                  (Daryn Sharp via bobby)
 
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFileSystem.java
index 040f59dbb8c..e122f25dcdd 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFileSystem.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFileSystem.java
@@ -474,18 +474,21 @@ public abstract class ChecksumFileSystem extends FilterFileSystem {
     if (fs.isDirectory(src)) {
       return fs.rename(src, dst);
     } else {
+      if (fs.isDirectory(dst)) {
+        dst = new Path(dst, src.getName());
+      }
 
       boolean value = fs.rename(src, dst);
       if (!value)
         return false;
 
-      Path checkFile = getChecksumFile(src);
-      if (fs.exists(checkFile)) { //try to rename checksum
-        if (fs.isDirectory(dst)) {
-          value = fs.rename(checkFile, dst);
-        } else {
-          value = fs.rename(checkFile, getChecksumFile(dst));
-        }
+      Path srcCheckFile = getChecksumFile(src);
+      Path dstCheckFile = getChecksumFile(dst);
+      if (fs.exists(srcCheckFile)) { //try to rename checksum
+        value = fs.rename(srcCheckFile, dstCheckFile);
+      } else if (fs.exists(dstCheckFile)) {
+        // no src checksum, so remove dst checksum
+        value = fs.delete(dstCheckFile, true); 
       }
 
       return value;
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestChecksumFileSystem.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestChecksumFileSystem.java
index 80347a72b45..0c24ad59b69 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestChecksumFileSystem.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestChecksumFileSystem.java
@@ -203,4 +203,58 @@ public class TestChecksumFileSystem {
     String str = readFile(localFs, testPath, 1024);
     assertEquals("testing stale checksum", str);
   }
+  
+  @Test
+  public void testRenameFileToFile() throws Exception {
+    Path srcPath = new Path(TEST_ROOT_DIR, "testRenameSrc");
+    Path dstPath = new Path(TEST_ROOT_DIR, "testRenameDst");
+    verifyRename(srcPath, dstPath, false);
+  }
+
+  @Test
+  public void testRenameFileIntoDir() throws Exception {
+    Path srcPath = new Path(TEST_ROOT_DIR, "testRenameSrc");
+    Path dstPath = new Path(TEST_ROOT_DIR, "testRenameDir");
+    localFs.mkdirs(dstPath);
+    verifyRename(srcPath, dstPath, true);
+  }
+
+  @Test
+  public void testRenameFileIntoDirFile() throws Exception {
+    Path srcPath = new Path(TEST_ROOT_DIR, "testRenameSrc");
+    Path dstPath = new Path(TEST_ROOT_DIR, "testRenameDir/testRenameDst");
+    assertTrue(localFs.mkdirs(dstPath));
+    verifyRename(srcPath, dstPath, false);
+  }
+
+
+  void verifyRename(Path srcPath, Path dstPath, boolean dstIsDir)
+      throws Exception { 
+    localFs.delete(srcPath,true);
+    localFs.delete(dstPath,true);
+    
+    Path realDstPath = dstPath;
+    if (dstIsDir) {
+      localFs.mkdirs(dstPath);
+      realDstPath = new Path(dstPath, srcPath.getName());
+    }
+    
+    // ensure file + checksum are moved
+    writeFile(localFs, srcPath, 1);
+    assertTrue(localFs.exists(localFs.getChecksumFile(srcPath)));
+    assertTrue(localFs.rename(srcPath, dstPath));
+    assertTrue(localFs.exists(localFs.getChecksumFile(realDstPath)));
+
+    // create a file with no checksum, rename, ensure dst checksum is removed    
+    writeFile(localFs.getRawFileSystem(), srcPath, 1);
+    assertFalse(localFs.exists(localFs.getChecksumFile(srcPath)));
+    assertTrue(localFs.rename(srcPath, dstPath));
+    assertFalse(localFs.exists(localFs.getChecksumFile(realDstPath)));
+    
+    // create file with checksum, rename over prior dst with no checksum
+    writeFile(localFs, srcPath, 1);
+    assertTrue(localFs.exists(localFs.getChecksumFile(srcPath)));
+    assertTrue(localFs.rename(srcPath, dstPath));
+    assertTrue(localFs.exists(localFs.getChecksumFile(realDstPath)));
+  }
 }