HDFS-14203. Refactor OIV Delimited output entry building mechanism. Contributed by Adam Antal.

Signed-off-by: Wei-Chiu Chuang <weichiu@apache.org>

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/PBImageDelimitedTextWriter.java

@@ -27,7 +27,6 @@
 import java.io.IOException;
 import java.io.PrintStream;
 import java.text.SimpleDateFormat;
-import java.util.Date;
 
 /**
  * A PBImageDelimitedTextWriter generates a text representation of the PB fsimage,
@@ -44,80 +43,108 @@
  * constructor.
  */
 public class PBImageDelimitedTextWriter extends PBImageTextWriter {
-  private static final String DATE_FORMAT="yyyy-MM-dd HH:mm";
+  private static final String DATE_FORMAT = "yyyy-MM-dd HH:mm";
-  private final SimpleDateFormat dateFormatter =
+
-      new SimpleDateFormat(DATE_FORMAT);
+  static class OutputEntryBuilder {
+    private final SimpleDateFormat dateFormatter =
+        new SimpleDateFormat(DATE_FORMAT);
+
+    private PBImageDelimitedTextWriter writer;
+    private Path path;
+    private int replication = 0;
+    private long modificationTime;
+    private long accessTime = 0;
+    private long preferredBlockSize = 0;
+    private int blocksCount = 0;
+    private long fileSize = 0;
+    private long nsQuota = 0;
+    private long dsQuota = 0;
+
+    private String dirPermission = "-";
+    private PermissionStatus permissionStatus;
+    private String aclPermission = "";
+
+    OutputEntryBuilder(PBImageDelimitedTextWriter writer, INode inode) {
+      this.writer = writer;
+      switch (inode.getType()) {
+      case FILE:
+        INodeFile file = inode.getFile();
+        replication = file.getReplication();
+        modificationTime = file.getModificationTime();
+        accessTime = file.getAccessTime();
+        preferredBlockSize = file.getPreferredBlockSize();
+        blocksCount = file.getBlocksCount();
+        fileSize = FSImageLoader.getFileSize(file);
+        permissionStatus = writer.getPermission(file.getPermission());
+        if (file.hasAcl() && file.getAcl().getEntriesCount() > 0){
+          aclPermission = "+";
+        }
+        break;
+      case DIRECTORY:
+        INodeDirectory dir = inode.getDirectory();
+        modificationTime = dir.getModificationTime();
+        nsQuota = dir.getNsQuota();
+        dsQuota = dir.getDsQuota();
+        dirPermission = "d";
+        permissionStatus = writer.getPermission(dir.getPermission());
+        if (dir.hasAcl() && dir.getAcl().getEntriesCount() > 0){
+          aclPermission = "+";
+        }
+        break;
+      case SYMLINK:
+        INodeSymlink s = inode.getSymlink();
+        modificationTime = s.getModificationTime();
+        accessTime = s.getAccessTime();
+        permissionStatus = writer.getPermission(s.getPermission());
+        break;
+      default:
+        break;
+      }
+    }
+
+    void setPath(Path path) {
+      this.path = path;
+    }
+
+    public String build() {
+      assert permissionStatus != null : "The PermissionStatus is null!";
+      assert permissionStatus.getUserName() != null : "User name is null!";
+      assert permissionStatus.getGroupName() != null : "Group name is null!";
+
+      StringBuffer buffer = new StringBuffer();
+      writer.append(buffer, path.toString());
+      writer.append(buffer, replication);
+      writer.append(buffer, dateFormatter.format(modificationTime));
+      writer.append(buffer, dateFormatter.format(accessTime));
+      writer.append(buffer, preferredBlockSize);
+      writer.append(buffer, blocksCount);
+      writer.append(buffer, fileSize);
+      writer.append(buffer, nsQuota);
+      writer.append(buffer, dsQuota);
+      writer.append(buffer, dirPermission +
+          permissionStatus.getPermission().toString() + aclPermission);
+      writer.append(buffer, permissionStatus.getUserName());
+      writer.append(buffer, permissionStatus.getGroupName());
+      return buffer.substring(1);
+    }
+  }
 
   PBImageDelimitedTextWriter(PrintStream out, String delimiter, String tempPath)
       throws IOException {
     super(out, delimiter, tempPath);
   }
 
-  private String formatDate(long date) {
-    return dateFormatter.format(new Date(date));
-  }
-
   @Override
   public String getEntry(String parent, INode inode) {
-    StringBuffer buffer = new StringBuffer();
+    OutputEntryBuilder entryBuilder =
+        new OutputEntryBuilder(this, inode);
+
     String inodeName = inode.getName().toStringUtf8();
     Path path = new Path(parent.isEmpty() ? "/" : parent,
       inodeName.isEmpty() ? "/" : inodeName);
-    append(buffer, path.toString());
+    entryBuilder.setPath(path);
-    PermissionStatus p = null;
-    boolean isDir = false;
-    boolean hasAcl = false;
 
-    switch (inode.getType()) {
+    return entryBuilder.build();
-    case FILE:
-      INodeFile file = inode.getFile();
-      p = getPermission(file.getPermission());
-      hasAcl = file.hasAcl() && file.getAcl().getEntriesCount() > 0;
-      append(buffer, file.getReplication());
-      append(buffer, formatDate(file.getModificationTime()));
-      append(buffer, formatDate(file.getAccessTime()));
-      append(buffer, file.getPreferredBlockSize());
-      append(buffer, file.getBlocksCount());
-      append(buffer, FSImageLoader.getFileSize(file));
-      append(buffer, 0);  // NS_QUOTA
-      append(buffer, 0);  // DS_QUOTA
-      break;
-    case DIRECTORY:
-      INodeDirectory dir = inode.getDirectory();
-      p = getPermission(dir.getPermission());
-      hasAcl = dir.hasAcl() && dir.getAcl().getEntriesCount() > 0;
-      append(buffer, 0);  // Replication
-      append(buffer, formatDate(dir.getModificationTime()));
-      append(buffer, formatDate(0));  // Access time.
-      append(buffer, 0);  // Block size.
-      append(buffer, 0);  // Num blocks.
-      append(buffer, 0);  // Num bytes.
-      append(buffer, dir.getNsQuota());
-      append(buffer, dir.getDsQuota());
-      isDir = true;
-      break;
-    case SYMLINK:
-      INodeSymlink s = inode.getSymlink();
-      p = getPermission(s.getPermission());
-      append(buffer, 0);  // Replication
-      append(buffer, formatDate(s.getModificationTime()));
-      append(buffer, formatDate(s.getAccessTime()));
-      append(buffer, 0);  // Block size.
-      append(buffer, 0);  // Num blocks.
-      append(buffer, 0);  // Num bytes.
-      append(buffer, 0);  // NS_QUOTA
-      append(buffer, 0);  // DS_QUOTA
-      break;
-    default:
-      break;
-    }
-    assert p != null;
-    String dirString = isDir ? "d" : "-";
-    String aclString = hasAcl ? "+" : "";
-    append(buffer, dirString + p.getPermission().toString() + aclString);
-    append(buffer, p.getUserName());
-    append(buffer, p.getGroupName());
-    return buffer.substring(1);
   }
 
   @Override

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java

@@ -40,6 +40,9 @@
 import static org.apache.hadoop.hdfs.tools.offlineImageViewer.PBImageXmlWriter.ERASURE_CODING_SECTION_SCHEMA;
 import static org.apache.hadoop.hdfs.tools.offlineImageViewer.PBImageXmlWriter.ERASURE_CODING_SECTION_SCHEMA_CODEC_NAME;
 import static org.apache.hadoop.hdfs.tools.offlineImageViewer.PBImageXmlWriter.ERASURE_CODING_SECTION_SCHEMA_OPTION;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
 import org.apache.hadoop.io.erasurecode.ECSchema;
 import org.apache.hadoop.io.erasurecode.ErasureCodeConstants;
 import static org.junit.Assert.assertEquals;
@@ -98,6 +101,7 @@
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.permission.FsAction;
 import org.apache.hadoop.fs.permission.FsPermission;
+import org.apache.hadoop.fs.permission.PermissionStatus;
 import org.apache.hadoop.hdfs.DFSConfigKeys;
 import org.apache.hadoop.hdfs.DFSTestUtil;
 import org.apache.hadoop.hdfs.DistributedFileSystem;
@@ -105,7 +109,9 @@
 import org.apache.hadoop.hdfs.protocol.BlockType;
 import org.apache.hadoop.hdfs.protocol.ErasureCodingPolicy;
 import org.apache.hadoop.hdfs.protocol.HdfsConstants.SafeModeAction;
+import org.apache.hadoop.hdfs.protocol.proto.HdfsProtos;
 import org.apache.hadoop.hdfs.protocol.SystemErasureCodingPolicies;
+import org.apache.hadoop.hdfs.server.namenode.FsImageProto;
 import org.apache.hadoop.hdfs.server.namenode.FSImageTestUtil;
 import org.apache.hadoop.hdfs.server.namenode.INodeFile;
 import org.apache.hadoop.hdfs.server.namenode.NameNodeLayoutVersion;
@@ -132,6 +138,7 @@
 
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
+import com.google.protobuf.ByteString;
 
 public class TestOfflineImageViewer {
   private static final Logger LOG =
@@ -146,6 +153,7 @@ public class TestOfflineImageViewer {
   private static final long FILE_NODE_ID_2 = 16389;
   private static final long FILE_NODE_ID_3 = 16394;
   private static final long DIR_NODE_ID = 16391;
+  private static final long SAMPLE_TIMESTAMP = 946684800000L;
 
   // namespace as written to dfs, to be compared with viewer's output
   final static HashMap<String, FileStatus> writtenFiles = Maps.newHashMap();
@@ -659,6 +667,109 @@ public void testWebImageViewerSecureMode() throws Exception {
     }
   }
 
+  private FsImageProto.INodeSection.INode createSampleFileInode() {
+    HdfsProtos.BlockProto.Builder block =
+        HdfsProtos.BlockProto.newBuilder()
+            .setNumBytes(1024)
+            .setBlockId(8)
+            .setGenStamp(SAMPLE_TIMESTAMP);
+    FsImageProto.INodeSection.AclFeatureProto.Builder acl =
+        FsImageProto.INodeSection.AclFeatureProto.newBuilder()
+            .addEntries(2);
+    FsImageProto.INodeSection.INodeFile.Builder file =
+        FsImageProto.INodeSection.INodeFile.newBuilder()
+            .setReplication(5)
+            .setModificationTime(SAMPLE_TIMESTAMP)
+            .setAccessTime(SAMPLE_TIMESTAMP)
+            .setPreferredBlockSize(1024)
+            .addBlocks(block)
+            .addBlocks(block)
+            .addBlocks(block)
+            .setAcl(acl);
+
+    return FsImageProto.INodeSection.INode.newBuilder()
+        .setType(FsImageProto.INodeSection.INode.Type.FILE)
+        .setFile(file)
+        .setName(ByteString.copyFromUtf8("file"))
+        .setId(3)
+        .build();
+  }
+
+  private FsImageProto.INodeSection.INode createSampleDirInode() {
+    FsImageProto.INodeSection.AclFeatureProto.Builder acl =
+        FsImageProto.INodeSection.AclFeatureProto.newBuilder()
+            .addEntries(2);
+    FsImageProto.INodeSection.INodeDirectory.Builder directory =
+        FsImageProto.INodeSection.INodeDirectory.newBuilder()
+            .setDsQuota(1000)
+            .setNsQuota(700)
+            .setModificationTime(SAMPLE_TIMESTAMP)
+            .setAcl(acl);
+
+    return FsImageProto.INodeSection.INode.newBuilder()
+        .setType(FsImageProto.INodeSection.INode.Type.DIRECTORY)
+        .setDirectory(directory)
+        .setName(ByteString.copyFromUtf8("dir"))
+        .setId(3)
+        .build();
+  }
+
+  private FsImageProto.INodeSection.INode createSampleSymlink() {
+    FsImageProto.INodeSection.INodeSymlink.Builder symlink =
+        FsImageProto.INodeSection.INodeSymlink.newBuilder()
+            .setModificationTime(SAMPLE_TIMESTAMP)
+            .setAccessTime(SAMPLE_TIMESTAMP);
+
+    return FsImageProto.INodeSection.INode.newBuilder()
+        .setType(FsImageProto.INodeSection.INode.Type.SYMLINK)
+        .setSymlink(symlink)
+        .setName(ByteString.copyFromUtf8("sym"))
+        .setId(5)
+        .build();
+  }
+
+  private PBImageDelimitedTextWriter createDelimitedWriterSpy()
+      throws IOException {
+    FsPermission fsPermission = new FsPermission(
+        FsAction.ALL,
+        FsAction.WRITE_EXECUTE,
+        FsAction.WRITE);
+    PermissionStatus permStatus = new PermissionStatus(
+        "user_1",
+        "group_1",
+        fsPermission);
+
+    PBImageDelimitedTextWriter writer = new
+        PBImageDelimitedTextWriter(null, ",", "");
+    PBImageDelimitedTextWriter writerSpy = spy(writer);
+    when(writerSpy.getPermission(anyLong())).thenReturn(permStatus);
+    return writerSpy;
+  }
+
+  @Test
+  public void testWriterOutputEntryBuilderForFile() throws IOException {
+    assertEquals("/path/file,5,2000-01-01 00:00,2000-01-01 00:00," +
+                "1024,3,3072,0,0,-rwx-wx-w-+,user_1,group_1",
+        createDelimitedWriterSpy().getEntry("/path/",
+            createSampleFileInode()));
+  }
+
+  @Test
+  public void testWriterOutputEntryBuilderForDirectory() throws IOException {
+    assertEquals("/path/dir,0,2000-01-01 00:00,1970-01-01 00:00" +
+                ",0,0,0,700,1000,drwx-wx-w-+,user_1,group_1",
+        createDelimitedWriterSpy().getEntry("/path/",
+            createSampleDirInode()));
+  }
+
+  @Test
+  public void testWriterOutputEntryBuilderForSymlink() throws IOException {
+    assertEquals("/path/sym,0,2000-01-01 00:00,2000-01-01 00:00" +
+                ",0,0,0,0,0,-rwx-wx-w-,user_1,group_1",
+        createDelimitedWriterSpy().getEntry("/path/",
+            createSampleSymlink()));
+  }
+
   @Test
   public void testPBDelimitedWriter() throws IOException, InterruptedException {
     testPBDelimitedWriter("");  // Test in memory db.
@@ -667,7 +778,7 @@ public void testPBDelimitedWriter() throws IOException, InterruptedException {
   }
 
   @Test
-  public void testOutputEntryBuilder() throws IOException {
+  public void testCorruptionOutputEntryBuilder() throws IOException {
     PBImageCorruptionDetector corrDetector =
         new PBImageCorruptionDetector(null, ",", "");
     PBImageCorruption c1 = new PBImageCorruption(342, true, false, 3);