xinglin commented on code in PR #5071:
URL: https://github.com/apache/hadoop/pull/5071#discussion_r1006017718


##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterTrash.java:
##########
@@ -144,6 +145,95 @@ private boolean addMountTable(final MountTable entry) 
throws IOException {
     return addResponse.getStatus();
   }
 
+  /**
+   * We manually create the trash root for TEST_USER. Then, moveToTrash
+   * shall not cause the router to create user's home dir anymore.
+   *
+   * We compare the modification of user home dir before and after moveToTrash,
+   * to verify that we don't try to re-create user home dir once it already
+   * exists. We need to pre-create the trash root, instead of user home dir,
+   * because adding a subdir to a dir will change the dir's modification time.
+   */
+  @Test
+  public void testMoveToTrashAutoCreateUserHomeAlreadyExisted()
+      throws IOException, URISyntaxException, InterruptedException {
+    MountTable addEntry = MountTable.newInstance(MOUNT_POINT,
+        Collections.singletonMap(ns0, MOUNT_POINT));
+    assertTrue(addMountTable(addEntry));
+    String testUserHome = "/user/" + TEST_USER;
+    String testUserTrashRoot = testUserHome + "/.Trash";
+
+    // Set owner to TEST_USER for root dir
+    DFSClient superUserClient = nnContext.getClient();
+    superUserClient.setOwner("/", TEST_USER, TEST_USER);
+
+    // Create MOUNT_POINT and user's trash root
+    UserGroupInformation ugi = 
UserGroupInformation.createRemoteUser(TEST_USER);
+    DFSClient client = nnContext.getClient(ugi);
+    client.mkdirs(MOUNT_POINT, new FsPermission("777"), true);
+    assertTrue(client.exists(MOUNT_POINT));
+    client.mkdirs(testUserTrashRoot, new FsPermission("775"), true);
+    assertTrue(client.exists(testUserTrashRoot));
+
+    // Create test file
+    client.create(FILE, true);
+    Path filePath = new Path(FILE);
+
+    // Get the fileStatus before moveToTrash
+    HdfsFileStatus status = client.getFileInfo(testUserHome);
+
+    // Test moveToTrash by TEST_USER
+    String trashPath = "/user/" + TEST_USER + "/.Trash/Current" + FILE;
+    Configuration routerConf = routerContext.getConf();
+    FileSystem fs = DFSTestUtil.getFileSystemAs(ugi, routerConf);
+    Trash trash = new Trash(fs, routerConf);
+    assertTrue(trash.moveToTrash(filePath));
+    assertTrue(nnFs.exists(new Path(trashPath)));
+
+    HdfsFileStatus status2 = client.getFileInfo(testUserHome);
+    assertEquals(status.getModificationTime(), status2.getModificationTime());

Review Comment:
   modtime won't change when the dir already exists and its content does not 
get modified (add a subdir or remove a subdir). 
   
   We don't mock rpc client in TestRouterTrash. If we use mocked RPC client for 
testing, we should probably put it in a separate new class? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to