[ 
https://issues.apache.org/jira/browse/HDFS-16024?focusedWorklogId=601423&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-601423
 ]

ASF GitHub Bot logged work on HDFS-16024:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 24/May/21 21:46
            Start Date: 24/May/21 21:46
    Worklog Time Spent: 10m 
      Work Description: goiri commented on a change in pull request #3009:
URL: https://github.com/apache/hadoop/pull/3009#discussion_r638296038



##########
File path: 
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/RemoteLocation.java
##########
@@ -80,6 +94,14 @@ public String getSrc() {
     return this.srcPath;
   }
 
+  public String getNsId() {

Review comment:
       Not needed anymore.
   This and getNnId().

##########
File path: 
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterTrash.java
##########
@@ -0,0 +1,263 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hdfs.server.federation.router;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.Trash;
+import org.apache.hadoop.fs.permission.FsPermission;
+import org.apache.hadoop.hdfs.DFSClient;
+import org.apache.hadoop.hdfs.DFSTestUtil;
+import org.apache.hadoop.hdfs.server.federation.MiniRouterDFSCluster;
+import org.apache.hadoop.hdfs.server.federation.RouterConfigBuilder;
+import org.apache.hadoop.hdfs.server.federation.StateStoreDFSCluster;
+import org.apache.hadoop.hdfs.server.federation.resolver.MountTableManager;
+import org.apache.hadoop.hdfs.server.federation.resolver.MountTableResolver;
+import org.apache.hadoop.hdfs.server.federation.store.protocol.*;
+import org.apache.hadoop.hdfs.server.federation.store.records.MountTable;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.hadoop.util.Time;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.net.URISyntaxException;
+import java.util.Collections;
+
+import static 
org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_KEY;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+/**
+ * This is a test through the Router move data to the Trash.
+ */
+public class TestRouterTrash {
+
+  public static final Logger LOG =
+      LoggerFactory.getLogger(TestRouterTrash.class);
+
+  private static StateStoreDFSCluster cluster;
+  private static MiniRouterDFSCluster.RouterContext routerContext;
+  private static MountTableResolver mountTable;
+  private static FileSystem routerFs;
+  private static FileSystem nnFs;
+  private static final String TEST_USER = "test-trash";
+  private static MiniRouterDFSCluster.NamenodeContext nnContext;
+  private static String ns0;
+  private static String ns1;
+  private static final String MOUNT_POINT = "/home/data";
+  private static final String FILE = MOUNT_POINT + "/file1";
+  private static final String TRASH_ROOT = "/user/" + TEST_USER + "/.Trash";
+  private static final String CURRENT = "/Current";
+
+  @BeforeClass
+  public static void globalSetUp() throws Exception {
+    // Build and start a federated cluster
+    cluster = new StateStoreDFSCluster(false, 2);
+    Configuration conf = new RouterConfigBuilder()
+        .stateStore()
+        .admin()
+        .rpc()
+        .http()
+        .build();
+    conf.set(FS_TRASH_INTERVAL_KEY, "100");
+    cluster.addRouterOverrides(conf);
+    cluster.startCluster();
+    cluster.startRouters();
+    cluster.waitClusterUp();
+
+    ns0 = cluster.getNameservices().get(0);
+    ns1 = cluster.getNameservices().get(1);
+
+    routerContext = cluster.getRandomRouter();
+    routerFs = routerContext.getFileSystem();
+    nnContext = cluster.getNamenode(ns0, null);
+    nnFs = nnContext.getFileSystem();
+    Router router = routerContext.getRouter();
+    mountTable = (MountTableResolver) router.getSubclusterResolver();
+  }
+
+  @AfterClass
+  public static void tearDown() {
+    if (cluster != null) {
+      cluster.stopRouter(routerContext);
+      cluster.shutdown();
+      cluster = null;
+    }
+  }
+
+  @After
+  public void clearMountTable() throws IOException {
+    RouterClient client = routerContext.getAdminClient();
+    MountTableManager mountTableManager = client.getMountTableManager();
+    GetMountTableEntriesRequest req1 =
+        GetMountTableEntriesRequest.newInstance("/");
+    GetMountTableEntriesResponse response =
+        mountTableManager.getMountTableEntries(req1);
+    for (MountTable entry : response.getEntries()) {
+      RemoveMountTableEntryRequest req2 =
+          RemoveMountTableEntryRequest.newInstance(entry.getSourcePath());
+      mountTableManager.removeMountTableEntry(req2);
+    }
+  }
+
+  @After
+  public void clearFile() throws IOException {
+    FileStatus[] fileStatuses = nnFs.listStatus(new Path("/"));
+    for (FileStatus file : fileStatuses) {
+      nnFs.delete(file.getPath(), true);
+    }
+  }
+
+  private boolean addMountTable(final MountTable entry) throws IOException {
+    RouterClient client = routerContext.getAdminClient();
+    MountTableManager mountTableManager = client.getMountTableManager();
+    AddMountTableEntryRequest addRequest =
+        AddMountTableEntryRequest.newInstance(entry);
+    AddMountTableEntryResponse addResponse =
+        mountTableManager.addMountTableEntry(addRequest);
+    // Reload the Router cache
+    mountTable.loadCache(true);
+    return addResponse.getStatus();
+  }
+
+  @Test
+  public void testMoveToTrashNoMountPoint() throws IOException,
+      URISyntaxException, InterruptedException {
+    MountTable addEntry = MountTable.newInstance(MOUNT_POINT,
+        Collections.singletonMap(ns0, MOUNT_POINT));
+    assertTrue(addMountTable(addEntry));
+    // current user client
+    DFSClient client = nnContext.getClient();
+    client.setOwner("/", TEST_USER, TEST_USER);
+    UserGroupInformation ugi = UserGroupInformation.
+        createRemoteUser(TEST_USER);
+    // test user client
+    client = nnContext.getClient(ugi);
+    client.mkdirs(MOUNT_POINT, new FsPermission("777"), true);
+    assertTrue(client.exists(MOUNT_POINT));
+    // crete test file
+    client.create(FILE, true);
+    Path filePath = new Path(FILE);
+
+    FileStatus[] fileStatuses = routerFs.listStatus(filePath);
+    assertEquals(1, fileStatuses.length);
+    assertEquals(TEST_USER, fileStatuses[0].getOwner());
+    // move to Trash
+    Configuration routerConf = routerContext.getConf();
+    FileSystem fs =
+        DFSTestUtil.getFileSystemAs(ugi, routerConf);
+    Trash trash = new Trash(fs, routerConf);
+    assertTrue(trash.moveToTrash(filePath));
+    fileStatuses = nnFs.listStatus(
+        new Path(TRASH_ROOT + CURRENT + MOUNT_POINT));
+    assertEquals(1, fileStatuses.length);
+    assertTrue(nnFs.exists(new Path(TRASH_ROOT + CURRENT + FILE)));
+    assertTrue(nnFs.exists(new Path("/user/" +
+        TEST_USER + "/.Trash/Current" + FILE)));
+    // When the target path in Trash already exists.
+    client.create(FILE, true);
+    filePath = new Path(FILE);
+    fileStatuses = routerFs.listStatus(filePath);
+    assertEquals(1, fileStatuses.length);
+    assertTrue(trash.moveToTrash(filePath));
+    fileStatuses = routerFs.listStatus(
+        new Path(TRASH_ROOT + CURRENT + MOUNT_POINT));
+    assertEquals(2, fileStatuses.length);
+  }
+
+  @Test
+  public void testDeleteToTrashExistMountPoint() throws IOException,
+      URISyntaxException, InterruptedException {
+    MountTable addEntry = MountTable.newInstance(MOUNT_POINT,
+        Collections.singletonMap(ns0, MOUNT_POINT));
+    assertTrue(addMountTable(addEntry));
+    // add Trash mount point
+    addEntry = MountTable.newInstance(TRASH_ROOT,
+        Collections.singletonMap(ns1, TRASH_ROOT));
+    assertTrue(addMountTable(addEntry));
+    // current user client
+    DFSClient client = nnContext.getClient();
+    client.setOwner("/", TEST_USER, TEST_USER);
+    UserGroupInformation ugi = UserGroupInformation.
+        createRemoteUser(TEST_USER);
+    // test user client
+    client = nnContext.getClient(ugi);
+    client.mkdirs(MOUNT_POINT, new FsPermission("777"), true);
+    assertTrue(client.exists(MOUNT_POINT));
+    // crete test file
+    client.create(FILE, true);
+    Path filePath = new Path(FILE);
+
+    FileStatus[] fileStatuses = routerFs.listStatus(filePath);
+    assertEquals(1, fileStatuses.length);
+    assertEquals(TEST_USER, fileStatuses[0].getOwner());
+
+    // move to Trash
+    Configuration routerConf = routerContext.getConf();
+    FileSystem fs =
+        DFSTestUtil.getFileSystemAs(ugi, routerConf);
+    Trash trash = new Trash(fs, routerConf);
+    assertTrue(trash.moveToTrash(filePath));
+    fileStatuses = nnFs.listStatus(
+        new Path(TRASH_ROOT + CURRENT + MOUNT_POINT));
+    assertEquals(1, fileStatuses.length);
+    assertTrue(nnFs.exists(new Path(TRASH_ROOT + CURRENT + FILE)));
+    // When the target path in Trash already exists.
+    client.create(FILE, true);
+    filePath = new Path(FILE);
+
+    fileStatuses = nnFs.listStatus(filePath);
+    assertEquals(1, fileStatuses.length);
+    assertTrue(trash.moveToTrash(filePath));
+    fileStatuses = nnFs.listStatus(
+        new Path(TRASH_ROOT + CURRENT + MOUNT_POINT));
+    assertEquals(2, fileStatuses.length);
+  }
+
+  @Test
+  public void testIsTrashPath() throws IOException {
+    UserGroupInformation ugi = UserGroupInformation.getCurrentUser();
+    assertNotNull(ugi);
+    assertTrue(MountTableResolver.isTrashPath(
+        "/user/" + ugi.getUserName() + "/.Trash/Current" + MOUNT_POINT));
+    assertTrue(MountTableResolver.isTrashPath(
+        "/user/" + ugi.getUserName() +
+            "/.Trash/" + Time.now() + MOUNT_POINT));
+    assertFalse(MountTableResolver.isTrashPath(MOUNT_POINT));
+  }
+
+  @Test
+  public void testSubtractTrashCurrentPath() throws IOException {
+    UserGroupInformation ugi = UserGroupInformation.getCurrentUser();
+    assertNotNull(ugi);
+    assertEquals(MOUNT_POINT, MountTableResolver.subtractTrashCurrentPath(
+        "/user/" + ugi.getUserName() + "/.Trash/Current" + MOUNT_POINT));
+    assertEquals(MOUNT_POINT, MountTableResolver.subtractTrashCurrentPath(
+        "/user/" + ugi.getUserName() +
+            "/.Trash/" + Time.now() + MOUNT_POINT));
+  }

Review comment:
       Add corner cases of files that do not have .Trash and other.

##########
File path: 
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterTrash.java
##########
@@ -0,0 +1,263 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hdfs.server.federation.router;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.Trash;
+import org.apache.hadoop.fs.permission.FsPermission;
+import org.apache.hadoop.hdfs.DFSClient;
+import org.apache.hadoop.hdfs.DFSTestUtil;
+import org.apache.hadoop.hdfs.server.federation.MiniRouterDFSCluster;
+import org.apache.hadoop.hdfs.server.federation.RouterConfigBuilder;
+import org.apache.hadoop.hdfs.server.federation.StateStoreDFSCluster;
+import org.apache.hadoop.hdfs.server.federation.resolver.MountTableManager;
+import org.apache.hadoop.hdfs.server.federation.resolver.MountTableResolver;
+import org.apache.hadoop.hdfs.server.federation.store.protocol.*;
+import org.apache.hadoop.hdfs.server.federation.store.records.MountTable;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.hadoop.util.Time;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.net.URISyntaxException;
+import java.util.Collections;
+
+import static 
org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_KEY;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+/**
+ * This is a test through the Router move data to the Trash.
+ */
+public class TestRouterTrash {
+
+  public static final Logger LOG =
+      LoggerFactory.getLogger(TestRouterTrash.class);
+
+  private static StateStoreDFSCluster cluster;
+  private static MiniRouterDFSCluster.RouterContext routerContext;
+  private static MountTableResolver mountTable;
+  private static FileSystem routerFs;
+  private static FileSystem nnFs;
+  private static final String TEST_USER = "test-trash";
+  private static MiniRouterDFSCluster.NamenodeContext nnContext;
+  private static String ns0;
+  private static String ns1;
+  private static final String MOUNT_POINT = "/home/data";
+  private static final String FILE = MOUNT_POINT + "/file1";
+  private static final String TRASH_ROOT = "/user/" + TEST_USER + "/.Trash";
+  private static final String CURRENT = "/Current";
+
+  @BeforeClass
+  public static void globalSetUp() throws Exception {
+    // Build and start a federated cluster
+    cluster = new StateStoreDFSCluster(false, 2);
+    Configuration conf = new RouterConfigBuilder()
+        .stateStore()
+        .admin()
+        .rpc()
+        .http()
+        .build();
+    conf.set(FS_TRASH_INTERVAL_KEY, "100");
+    cluster.addRouterOverrides(conf);
+    cluster.startCluster();
+    cluster.startRouters();
+    cluster.waitClusterUp();
+
+    ns0 = cluster.getNameservices().get(0);
+    ns1 = cluster.getNameservices().get(1);
+
+    routerContext = cluster.getRandomRouter();
+    routerFs = routerContext.getFileSystem();
+    nnContext = cluster.getNamenode(ns0, null);
+    nnFs = nnContext.getFileSystem();
+    Router router = routerContext.getRouter();
+    mountTable = (MountTableResolver) router.getSubclusterResolver();
+  }
+
+  @AfterClass
+  public static void tearDown() {
+    if (cluster != null) {
+      cluster.stopRouter(routerContext);
+      cluster.shutdown();
+      cluster = null;
+    }
+  }
+
+  @After
+  public void clearMountTable() throws IOException {
+    RouterClient client = routerContext.getAdminClient();
+    MountTableManager mountTableManager = client.getMountTableManager();
+    GetMountTableEntriesRequest req1 =
+        GetMountTableEntriesRequest.newInstance("/");
+    GetMountTableEntriesResponse response =
+        mountTableManager.getMountTableEntries(req1);
+    for (MountTable entry : response.getEntries()) {
+      RemoveMountTableEntryRequest req2 =
+          RemoveMountTableEntryRequest.newInstance(entry.getSourcePath());
+      mountTableManager.removeMountTableEntry(req2);
+    }
+  }
+
+  @After
+  public void clearFile() throws IOException {
+    FileStatus[] fileStatuses = nnFs.listStatus(new Path("/"));
+    for (FileStatus file : fileStatuses) {
+      nnFs.delete(file.getPath(), true);
+    }
+  }
+
+  private boolean addMountTable(final MountTable entry) throws IOException {
+    RouterClient client = routerContext.getAdminClient();
+    MountTableManager mountTableManager = client.getMountTableManager();
+    AddMountTableEntryRequest addRequest =
+        AddMountTableEntryRequest.newInstance(entry);
+    AddMountTableEntryResponse addResponse =
+        mountTableManager.addMountTableEntry(addRequest);
+    // Reload the Router cache
+    mountTable.loadCache(true);
+    return addResponse.getStatus();
+  }
+
+  @Test
+  public void testMoveToTrashNoMountPoint() throws IOException,
+      URISyntaxException, InterruptedException {
+    MountTable addEntry = MountTable.newInstance(MOUNT_POINT,
+        Collections.singletonMap(ns0, MOUNT_POINT));
+    assertTrue(addMountTable(addEntry));
+    // current user client
+    DFSClient client = nnContext.getClient();
+    client.setOwner("/", TEST_USER, TEST_USER);
+    UserGroupInformation ugi = UserGroupInformation.
+        createRemoteUser(TEST_USER);
+    // test user client
+    client = nnContext.getClient(ugi);
+    client.mkdirs(MOUNT_POINT, new FsPermission("777"), true);
+    assertTrue(client.exists(MOUNT_POINT));
+    // crete test file
+    client.create(FILE, true);
+    Path filePath = new Path(FILE);
+
+    FileStatus[] fileStatuses = routerFs.listStatus(filePath);
+    assertEquals(1, fileStatuses.length);
+    assertEquals(TEST_USER, fileStatuses[0].getOwner());
+    // move to Trash
+    Configuration routerConf = routerContext.getConf();
+    FileSystem fs =
+        DFSTestUtil.getFileSystemAs(ugi, routerConf);
+    Trash trash = new Trash(fs, routerConf);
+    assertTrue(trash.moveToTrash(filePath));
+    fileStatuses = nnFs.listStatus(
+        new Path(TRASH_ROOT + CURRENT + MOUNT_POINT));
+    assertEquals(1, fileStatuses.length);
+    assertTrue(nnFs.exists(new Path(TRASH_ROOT + CURRENT + FILE)));
+    assertTrue(nnFs.exists(new Path("/user/" +
+        TEST_USER + "/.Trash/Current" + FILE)));
+    // When the target path in Trash already exists.
+    client.create(FILE, true);
+    filePath = new Path(FILE);
+    fileStatuses = routerFs.listStatus(filePath);
+    assertEquals(1, fileStatuses.length);
+    assertTrue(trash.moveToTrash(filePath));
+    fileStatuses = routerFs.listStatus(
+        new Path(TRASH_ROOT + CURRENT + MOUNT_POINT));
+    assertEquals(2, fileStatuses.length);
+  }
+
+  @Test
+  public void testDeleteToTrashExistMountPoint() throws IOException,
+      URISyntaxException, InterruptedException {
+    MountTable addEntry = MountTable.newInstance(MOUNT_POINT,
+        Collections.singletonMap(ns0, MOUNT_POINT));
+    assertTrue(addMountTable(addEntry));
+    // add Trash mount point
+    addEntry = MountTable.newInstance(TRASH_ROOT,
+        Collections.singletonMap(ns1, TRASH_ROOT));
+    assertTrue(addMountTable(addEntry));
+    // current user client
+    DFSClient client = nnContext.getClient();
+    client.setOwner("/", TEST_USER, TEST_USER);
+    UserGroupInformation ugi = UserGroupInformation.
+        createRemoteUser(TEST_USER);
+    // test user client
+    client = nnContext.getClient(ugi);
+    client.mkdirs(MOUNT_POINT, new FsPermission("777"), true);
+    assertTrue(client.exists(MOUNT_POINT));
+    // crete test file
+    client.create(FILE, true);
+    Path filePath = new Path(FILE);
+
+    FileStatus[] fileStatuses = routerFs.listStatus(filePath);
+    assertEquals(1, fileStatuses.length);
+    assertEquals(TEST_USER, fileStatuses[0].getOwner());
+
+    // move to Trash
+    Configuration routerConf = routerContext.getConf();
+    FileSystem fs =
+        DFSTestUtil.getFileSystemAs(ugi, routerConf);
+    Trash trash = new Trash(fs, routerConf);
+    assertTrue(trash.moveToTrash(filePath));
+    fileStatuses = nnFs.listStatus(
+        new Path(TRASH_ROOT + CURRENT + MOUNT_POINT));
+    assertEquals(1, fileStatuses.length);
+    assertTrue(nnFs.exists(new Path(TRASH_ROOT + CURRENT + FILE)));
+    // When the target path in Trash already exists.
+    client.create(FILE, true);
+    filePath = new Path(FILE);
+
+    fileStatuses = nnFs.listStatus(filePath);
+    assertEquals(1, fileStatuses.length);
+    assertTrue(trash.moveToTrash(filePath));
+    fileStatuses = nnFs.listStatus(
+        new Path(TRASH_ROOT + CURRENT + MOUNT_POINT));
+    assertEquals(2, fileStatuses.length);
+  }
+
+  @Test
+  public void testIsTrashPath() throws IOException {
+    UserGroupInformation ugi = UserGroupInformation.getCurrentUser();
+    assertNotNull(ugi);
+    assertTrue(MountTableResolver.isTrashPath(
+        "/user/" + ugi.getUserName() + "/.Trash/Current" + MOUNT_POINT));
+    assertTrue(MountTableResolver.isTrashPath(
+        "/user/" + ugi.getUserName() +
+            "/.Trash/" + Time.now() + MOUNT_POINT));
+    assertFalse(MountTableResolver.isTrashPath(MOUNT_POINT));

Review comment:
       null and empty would also be good.

##########
File path: 
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterTrash.java
##########
@@ -0,0 +1,263 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hdfs.server.federation.router;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.Trash;
+import org.apache.hadoop.fs.permission.FsPermission;
+import org.apache.hadoop.hdfs.DFSClient;
+import org.apache.hadoop.hdfs.DFSTestUtil;
+import org.apache.hadoop.hdfs.server.federation.MiniRouterDFSCluster;
+import org.apache.hadoop.hdfs.server.federation.RouterConfigBuilder;
+import org.apache.hadoop.hdfs.server.federation.StateStoreDFSCluster;
+import org.apache.hadoop.hdfs.server.federation.resolver.MountTableManager;
+import org.apache.hadoop.hdfs.server.federation.resolver.MountTableResolver;
+import org.apache.hadoop.hdfs.server.federation.store.protocol.*;
+import org.apache.hadoop.hdfs.server.federation.store.records.MountTable;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.hadoop.util.Time;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.net.URISyntaxException;
+import java.util.Collections;
+
+import static 
org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_KEY;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+/**
+ * This is a test through the Router move data to the Trash.
+ */
+public class TestRouterTrash {
+
+  public static final Logger LOG =
+      LoggerFactory.getLogger(TestRouterTrash.class);
+
+  private static StateStoreDFSCluster cluster;
+  private static MiniRouterDFSCluster.RouterContext routerContext;
+  private static MountTableResolver mountTable;
+  private static FileSystem routerFs;
+  private static FileSystem nnFs;
+  private static final String TEST_USER = "test-trash";
+  private static MiniRouterDFSCluster.NamenodeContext nnContext;
+  private static String ns0;
+  private static String ns1;
+  private static final String MOUNT_POINT = "/home/data";
+  private static final String FILE = MOUNT_POINT + "/file1";
+  private static final String TRASH_ROOT = "/user/" + TEST_USER + "/.Trash";
+  private static final String CURRENT = "/Current";
+
+  @BeforeClass
+  public static void globalSetUp() throws Exception {
+    // Build and start a federated cluster
+    cluster = new StateStoreDFSCluster(false, 2);
+    Configuration conf = new RouterConfigBuilder()
+        .stateStore()
+        .admin()
+        .rpc()
+        .http()
+        .build();
+    conf.set(FS_TRASH_INTERVAL_KEY, "100");
+    cluster.addRouterOverrides(conf);
+    cluster.startCluster();
+    cluster.startRouters();
+    cluster.waitClusterUp();
+
+    ns0 = cluster.getNameservices().get(0);
+    ns1 = cluster.getNameservices().get(1);
+
+    routerContext = cluster.getRandomRouter();
+    routerFs = routerContext.getFileSystem();
+    nnContext = cluster.getNamenode(ns0, null);
+    nnFs = nnContext.getFileSystem();
+    Router router = routerContext.getRouter();
+    mountTable = (MountTableResolver) router.getSubclusterResolver();
+  }
+
+  @AfterClass
+  public static void tearDown() {
+    if (cluster != null) {
+      cluster.stopRouter(routerContext);
+      cluster.shutdown();
+      cluster = null;
+    }
+  }
+
+  @After
+  public void clearMountTable() throws IOException {
+    RouterClient client = routerContext.getAdminClient();
+    MountTableManager mountTableManager = client.getMountTableManager();
+    GetMountTableEntriesRequest req1 =
+        GetMountTableEntriesRequest.newInstance("/");
+    GetMountTableEntriesResponse response =
+        mountTableManager.getMountTableEntries(req1);
+    for (MountTable entry : response.getEntries()) {
+      RemoveMountTableEntryRequest req2 =
+          RemoveMountTableEntryRequest.newInstance(entry.getSourcePath());
+      mountTableManager.removeMountTableEntry(req2);
+    }
+  }
+
+  @After
+  public void clearFile() throws IOException {
+    FileStatus[] fileStatuses = nnFs.listStatus(new Path("/"));
+    for (FileStatus file : fileStatuses) {
+      nnFs.delete(file.getPath(), true);
+    }
+  }
+
+  private boolean addMountTable(final MountTable entry) throws IOException {
+    RouterClient client = routerContext.getAdminClient();
+    MountTableManager mountTableManager = client.getMountTableManager();
+    AddMountTableEntryRequest addRequest =
+        AddMountTableEntryRequest.newInstance(entry);
+    AddMountTableEntryResponse addResponse =
+        mountTableManager.addMountTableEntry(addRequest);
+    // Reload the Router cache
+    mountTable.loadCache(true);
+    return addResponse.getStatus();
+  }
+
+  @Test
+  public void testMoveToTrashNoMountPoint() throws IOException,
+      URISyntaxException, InterruptedException {
+    MountTable addEntry = MountTable.newInstance(MOUNT_POINT,
+        Collections.singletonMap(ns0, MOUNT_POINT));
+    assertTrue(addMountTable(addEntry));
+    // current user client
+    DFSClient client = nnContext.getClient();
+    client.setOwner("/", TEST_USER, TEST_USER);
+    UserGroupInformation ugi = UserGroupInformation.
+        createRemoteUser(TEST_USER);
+    // test user client
+    client = nnContext.getClient(ugi);
+    client.mkdirs(MOUNT_POINT, new FsPermission("777"), true);
+    assertTrue(client.exists(MOUNT_POINT));
+    // crete test file
+    client.create(FILE, true);
+    Path filePath = new Path(FILE);
+
+    FileStatus[] fileStatuses = routerFs.listStatus(filePath);
+    assertEquals(1, fileStatuses.length);
+    assertEquals(TEST_USER, fileStatuses[0].getOwner());
+    // move to Trash
+    Configuration routerConf = routerContext.getConf();
+    FileSystem fs =
+        DFSTestUtil.getFileSystemAs(ugi, routerConf);
+    Trash trash = new Trash(fs, routerConf);
+    assertTrue(trash.moveToTrash(filePath));
+    fileStatuses = nnFs.listStatus(
+        new Path(TRASH_ROOT + CURRENT + MOUNT_POINT));
+    assertEquals(1, fileStatuses.length);
+    assertTrue(nnFs.exists(new Path(TRASH_ROOT + CURRENT + FILE)));
+    assertTrue(nnFs.exists(new Path("/user/" +
+        TEST_USER + "/.Trash/Current" + FILE)));
+    // When the target path in Trash already exists.
+    client.create(FILE, true);
+    filePath = new Path(FILE);
+    fileStatuses = routerFs.listStatus(filePath);
+    assertEquals(1, fileStatuses.length);
+    assertTrue(trash.moveToTrash(filePath));
+    fileStatuses = routerFs.listStatus(
+        new Path(TRASH_ROOT + CURRENT + MOUNT_POINT));
+    assertEquals(2, fileStatuses.length);
+  }
+
+  @Test
+  public void testDeleteToTrashExistMountPoint() throws IOException,
+      URISyntaxException, InterruptedException {
+    MountTable addEntry = MountTable.newInstance(MOUNT_POINT,
+        Collections.singletonMap(ns0, MOUNT_POINT));
+    assertTrue(addMountTable(addEntry));
+    // add Trash mount point
+    addEntry = MountTable.newInstance(TRASH_ROOT,
+        Collections.singletonMap(ns1, TRASH_ROOT));
+    assertTrue(addMountTable(addEntry));
+    // current user client
+    DFSClient client = nnContext.getClient();
+    client.setOwner("/", TEST_USER, TEST_USER);
+    UserGroupInformation ugi = UserGroupInformation.
+        createRemoteUser(TEST_USER);
+    // test user client
+    client = nnContext.getClient(ugi);
+    client.mkdirs(MOUNT_POINT, new FsPermission("777"), true);
+    assertTrue(client.exists(MOUNT_POINT));
+    // crete test file
+    client.create(FILE, true);
+    Path filePath = new Path(FILE);
+
+    FileStatus[] fileStatuses = routerFs.listStatus(filePath);
+    assertEquals(1, fileStatuses.length);
+    assertEquals(TEST_USER, fileStatuses[0].getOwner());
+
+    // move to Trash
+    Configuration routerConf = routerContext.getConf();
+    FileSystem fs =
+        DFSTestUtil.getFileSystemAs(ugi, routerConf);
+    Trash trash = new Trash(fs, routerConf);
+    assertTrue(trash.moveToTrash(filePath));
+    fileStatuses = nnFs.listStatus(
+        new Path(TRASH_ROOT + CURRENT + MOUNT_POINT));
+    assertEquals(1, fileStatuses.length);
+    assertTrue(nnFs.exists(new Path(TRASH_ROOT + CURRENT + FILE)));
+    // When the target path in Trash already exists.
+    client.create(FILE, true);
+    filePath = new Path(FILE);
+
+    fileStatuses = nnFs.listStatus(filePath);
+    assertEquals(1, fileStatuses.length);
+    assertTrue(trash.moveToTrash(filePath));
+    fileStatuses = nnFs.listStatus(
+        new Path(TRASH_ROOT + CURRENT + MOUNT_POINT));
+    assertEquals(2, fileStatuses.length);
+  }
+
+  @Test
+  public void testIsTrashPath() throws IOException {
+    UserGroupInformation ugi = UserGroupInformation.getCurrentUser();
+    assertNotNull(ugi);
+    assertTrue(MountTableResolver.isTrashPath(
+        "/user/" + ugi.getUserName() + "/.Trash/Current" + MOUNT_POINT));
+    assertTrue(MountTableResolver.isTrashPath(
+        "/user/" + ugi.getUserName() +
+            "/.Trash/" + Time.now() + MOUNT_POINT));
+    assertFalse(MountTableResolver.isTrashPath(MOUNT_POINT));

Review comment:
       Can we add a couple corner cases like:
   /home/user/empty.Trash/Current
   /.Trash/Current
   /home/user/.Trash
   /home/user/.TrashNot




-- 
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.

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


Issue Time Tracking
-------------------

    Worklog Id:     (was: 601423)
    Time Spent: 6h 10m  (was: 6h)

> RBF: Rename data to the Trash should be based on src locations
> --------------------------------------------------------------
>
>                 Key: HDFS-16024
>                 URL: https://issues.apache.org/jira/browse/HDFS-16024
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>          Components: rbf
>    Affects Versions: 3.4.0
>            Reporter: Xiangyi Zhu
>            Assignee: Xiangyi Zhu
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 6h 10m
>  Remaining Estimate: 0h
>
> 1.When deleting data to the Trash without configuring a mount point for the 
> Trash, the Router should recognize and move the data to the Trash
> 2.When the user’s trash can is configured with a mount point and is different 
> from the NS of the deleted directory, the router should identify and move the 
> data to the trash can of the current user of src
> The same is true for using ViewFs mount points, I think we should be 
> consistent with it



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

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

Reply via email to