diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 69188dd0bf5..0fcb10b56d1 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -676,6 +676,8 @@ Release 0.23.3 - UNRELEASED HADOOP-8613. AbstractDelegationTokenIdentifier#getUser() should set token auth type. (daryn) + HADOOP-8627. FS deleteOnExit may delete the wrong path (daryn via bobby) + Release 0.23.2 - UNRELEASED NEW FEATURES diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FilterFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FilterFileSystem.java index 6cbaf591e5a..38ddb6c5f58 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FilterFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FilterFileSystem.java @@ -191,23 +191,6 @@ public boolean delete(Path f, boolean recursive) throws IOException { return fs.delete(f, recursive); } - /** - * Mark a path to be deleted when FileSystem is closed. - * When the JVM shuts down, - * all FileSystem objects will be closed automatically. - * Then, - * the marked path will be deleted as a result of closing the FileSystem. - * - * The path has to exist in the file system. - * - * @param f the path to delete. - * @return true if deleteOnExit is successful, otherwise false. - * @throws IOException - */ - public boolean deleteOnExit(Path f) throws IOException { - return fs.deleteOnExit(f); - } - /** List files in a directory. */ public FileStatus[] listStatus(Path f) throws IOException { return fs.listStatus(f); diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFilterFileSystem.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFilterFileSystem.java index 647a583d103..a374c08a186 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFilterFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFilterFileSystem.java @@ -179,7 +179,9 @@ public void primitiveMkdir(Path f, FsPermission absolutePermission, public Token getDelegationToken(String renewer) throws IOException { return null; } - + public boolean deleteOnExit(Path f) throws IOException { + return false; + } public String getScheme() { return "dontcheck"; } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestChRootedFileSystem.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestChRootedFileSystem.java index 127866be1b5..44d7a4a7c13 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestChRootedFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestChRootedFileSystem.java @@ -26,6 +26,7 @@ import org.apache.hadoop.fs.ContentSummary; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.FileSystemTestHelper; +import org.apache.hadoop.fs.FilterFileSystem; import org.apache.hadoop.fs.FsConstants; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.viewfs.ChRootedFileSystem; @@ -33,6 +34,7 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import static org.mockito.Mockito.*; public class TestChRootedFileSystem { FileSystem fSys; // The ChRoootedFs @@ -314,4 +316,37 @@ public void testResolvePath() throws IOException { public void testResolvePathNonExisting() throws IOException { fSys.resolvePath(new Path("/nonExisting")); } -} + + @Test + public void testDeleteOnExitPathHandling() throws IOException { + Configuration conf = new Configuration(); + conf.setClass("fs.mockfs.impl", MockFileSystem.class, FileSystem.class); + + URI chrootUri = URI.create("mockfs://foo/a/b"); + ChRootedFileSystem chrootFs = new ChRootedFileSystem(chrootUri, conf); + FileSystem mockFs = ((FilterFileSystem)chrootFs.getRawFileSystem()) + .getRawFileSystem(); + + // ensure delete propagates the correct path + Path chrootPath = new Path("/c"); + Path rawPath = new Path("/a/b/c"); + chrootFs.delete(chrootPath, false); + verify(mockFs).delete(eq(rawPath), eq(false)); + reset(mockFs); + + // fake that the path exists for deleteOnExit + FileStatus stat = mock(FileStatus.class); + when(mockFs.getFileStatus(eq(rawPath))).thenReturn(stat); + // ensure deleteOnExit propagates the correct path + chrootFs.deleteOnExit(chrootPath); + chrootFs.close(); + verify(mockFs).delete(eq(rawPath), eq(true)); + } + + static class MockFileSystem extends FilterFileSystem { + MockFileSystem() { + super(mock(FileSystem.class)); + } + public void initialize(URI name, Configuration conf) throws IOException {} + } +} \ No newline at end of file