HADOOP-6536. Fixes FileUtil.fullyDelete() not to delete the contents of the sym-linked directory. Contributed by Ravi Gummadi

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

CHANGES.txt

@@ -142,6 +142,9 @@ Trunk (unreleased changes)
     HADOOP-6670. Use the UserGroupInformation's Subject as the criteria for
     equals and hashCode. (Owen O'Malley and Kan Zhang via ddas)
 
+    HADOOP-6536. Fixes FileUtil.fullyDelete() not to delete the contents of
+    the sym-linked directory. (Ravi Gummadi via amareshwari)
+
 Release 0.21.0 - Unreleased
 
   INCOMPATIBLE CHANGES

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

@@ -79,8 +79,22 @@ public static Path[] stat2Paths(FileStatus[] stats, Path path) {
   /**
    * Delete a directory and all its contents.  If
    * we return false, the directory may be partially-deleted.
+   * (1) If dir is symlink to a file, the symlink is deleted. The file pointed
+   *     to by the symlink is not deleted.
+   * (2) If dir is symlink to a directory, symlink is deleted. The directory
+   *     pointed to by symlink is not deleted.
+   * (3) If dir is a normal file, it is deleted.
+   * (4) If dir is a normal directory, then dir and all its contents recursively
+   *     are deleted.
    */
   public static boolean fullyDelete(File dir) throws IOException {
+    if (dir.delete()) {
+      // dir is (a) normal file, (b) symlink to a file, (c) empty directory or
+      // (d) symlink to a directory
+      return true;
+    }
+
+    // handle nonempty directory deletion
     if (!fullyDeleteContents(dir)) {
       return false;
     }
@@ -90,6 +104,8 @@ public static boolean fullyDelete(File dir) throws IOException {
   /**
    * Delete the contents of a directory, not the directory itself.  If
    * we return false, the directory may be partially-deleted.
+   * If dir is a symlink to a directory, all the contents of the actual
+   * directory pointed to by dir will be deleted.
    */
   public static boolean fullyDeleteContents(File dir) throws IOException {
     boolean deletionSucceeded = true;
@@ -97,13 +113,13 @@ public static boolean fullyDeleteContents(File dir) throws IOException {
     if (contents != null) {
       for (int i = 0; i < contents.length; i++) {
         if (contents[i].isFile()) {
-          if (!contents[i].delete()) {
+          if (!contents[i].delete()) {// normal file or symlink to another file
             deletionSucceeded = false;
             continue; // continue deletion of other files/dirs under dir
           }
         } else {
-          //try deleting the directory
-          // this might be a symlink
+          // Either directory or symlink to another directory.
+          // Try deleting the directory as this might be a symlink
           boolean b = false;
           b = contents[i].delete();
           if (b){

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

@@ -95,6 +95,69 @@ public void testFullyDelete() throws IOException {
     validateTmpDir();
   }
 
+  /**
+   * Tests if fullyDelete deletes
+   * (a) symlink to file only and not the file pointed to by symlink.
+   * (b) symlink to dir only and not the dir pointed to by symlink.
+   * @throws IOException
+   */
+  @Test
+  public void testFullyDeleteSymlinks() throws IOException {
+    setupDirs();
+    
+    File link = new File(del, LINK);
+    Assert.assertEquals(5, del.list().length);
+    // Since tmpDir is symlink to tmp, fullyDelete(tmpDir) should not
+    // delete contents of tmp. See setupDirs for details.
+    boolean ret = FileUtil.fullyDelete(link);
+    Assert.assertTrue(ret);
+    Assert.assertFalse(link.exists());
+    Assert.assertEquals(4, del.list().length);
+    validateTmpDir();
+
+    File linkDir = new File(del, "tmpDir");
+    // Since tmpDir is symlink to tmp, fullyDelete(tmpDir) should not
+    // delete contents of tmp. See setupDirs for details.
+    ret = FileUtil.fullyDelete(linkDir);
+    Assert.assertTrue(ret);
+    Assert.assertFalse(linkDir.exists());
+    Assert.assertEquals(3, del.list().length);
+    validateTmpDir();
+  }
+
+  /**
+   * Tests if fullyDelete deletes
+   * (a) dangling symlink to file properly
+   * (b) dangling symlink to directory properly
+   * @throws IOException
+   */
+  @Test
+  public void testFullyDeleteDanglingSymlinks() throws IOException {
+    setupDirs();
+    // delete the directory tmp to make tmpDir a dangling link to dir tmp and
+    // to make y as a dangling link to file tmp/x
+    boolean ret = FileUtil.fullyDelete(tmp);
+    Assert.assertTrue(ret);
+    Assert.assertFalse(tmp.exists());
+
+    // dangling symlink to file
+    File link = new File(del, LINK);
+    Assert.assertEquals(5, del.list().length);
+    // Even though 'y' is dangling symlink to file tmp/x, fullyDelete(y)
+    // should delete 'y' properly.
+    ret = FileUtil.fullyDelete(link);
+    Assert.assertTrue(ret);
+    Assert.assertEquals(4, del.list().length);
+
+    // dangling symlink to directory
+    File linkDir = new File(del, "tmpDir");
+    // Even though tmpDir is dangling symlink to tmp, fullyDelete(tmpDir) should
+    // delete tmpDir properly.
+    ret = FileUtil.fullyDelete(linkDir);
+    Assert.assertTrue(ret);
+    Assert.assertEquals(3, del.list().length);
+  }
+
   @Test
   public void testFullyDeleteContents() throws IOException {
     setupDirs();