HDFS-14529. SetTimes to throw FileNotFoundException if inode is not found (#3243)

Reviewed-by: Akira Ajisaka <[email protected]>
Reviewed-by: Viraj Jasani <[email protected]>
Reviewed-by: Ayush Saxena <[email protected]>

Author: jojochuang

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java

@@ -124,11 +124,6 @@ static FileStatus setTimes(
       if (fsd.isPermissionEnabled()) {
         fsd.checkPathAccess(pc, iip, FsAction.WRITE);
       }
-      final INode inode = iip.getLastINode();
-      if (inode == null) {
-        throw new FileNotFoundException("File/Directory " + iip.getPath() +
-                                            " does not exist.");
-      }
       boolean changed = unprotectedSetTimes(fsd, iip, mtime, atime, true);
       if (changed) {
         fsd.getEditLog().logTimes(iip.getPath(), mtime, atime);
@@ -305,7 +300,7 @@ static boolean unprotectedSetOwner(
 
   static boolean setTimes(
       FSDirectory fsd, INodesInPath iip, long mtime, long atime, boolean force)
-          throws QuotaExceededException {
+      throws FileNotFoundException {
     fsd.writeLock();
     try {
       return unprotectedSetTimes(fsd, iip, mtime, atime, force);
@@ -497,10 +492,14 @@ private static void setDirStoragePolicy(
 
   static boolean unprotectedSetTimes(
       FSDirectory fsd, INodesInPath iip, long mtime, long atime, boolean force)
-          throws QuotaExceededException {
+      throws FileNotFoundException {
     assert fsd.hasWriteLock();
     boolean status = false;
     INode inode = iip.getLastINode();
+    if (inode == null) {
+      throw new FileNotFoundException("File/Directory " + iip.getPath() +
+          " does not exist.");
+    }
     int latest = iip.getLatestSnapshotId();
     if (mtime >= 0) {
       inode = inode.setModificationTime(mtime, latest);

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirAttrOp.java

@@ -23,11 +23,12 @@
 import org.apache.hadoop.fs.permission.FsPermission;
 import org.apache.hadoop.fs.permission.PermissionStatus;
 import org.apache.hadoop.hdfs.DFSUtil;
-import org.apache.hadoop.hdfs.protocol.QuotaExceededException;
 import org.apache.hadoop.hdfs.server.namenode.snapshot.SnapshotManager;
 import org.junit.Test;
 import org.mockito.Mockito;
 
+import java.io.FileNotFoundException;
+
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.when;
@@ -40,7 +41,8 @@ public class TestFSDirAttrOp {
       LoggerFactory.getLogger(TestFSDirAttrOp.class);
 
   private boolean unprotectedSetTimes(long atime, long atime0, long precision,
-      long mtime, boolean force) throws QuotaExceededException {
+      long mtime, boolean force)
+      throws FileNotFoundException {
     FSNamesystem fsn = Mockito.mock(FSNamesystem.class);
     SnapshotManager ssMgr = Mockito.mock(SnapshotManager.class);
     FSDirectory fsd = Mockito.mock(FSDirectory.class);
@@ -131,4 +133,16 @@ public void testUnprotectedSetTimes() throws Exception {
     assertTrue("SetTimes should update access time",
         unprotectedSetTimes(100, 0, 1000, 1, false));
   }
+
+  @Test(expected = FileNotFoundException.class)
+  public void testUnprotectedSetTimesFNFE()
+      throws FileNotFoundException {
+    FSDirectory fsd = Mockito.mock(FSDirectory.class);
+    INodesInPath iip = Mockito.mock(INodesInPath.class);
+
+    when(fsd.hasWriteLock()).thenReturn(Boolean.TRUE);
+    when(iip.getLastINode()).thenReturn(null);
+
+    FSDirAttrOp.unprotectedSetTimes(fsd, iip, 0, 0, false);
+  }
 }