HADOOP-18257. test changes + review comments.

Author: mehakmeet

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/audit/AuditTool.java

@@ -22,7 +22,6 @@
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.net.URI;
-import java.net.URISyntaxException;
 import java.util.Arrays;
 import java.util.List;
 
@@ -76,7 +75,7 @@ public class AuditTool extends Configured implements Tool, Closeable {
   private static final int INVALID_ARGUMENT = EXIT_COMMAND_ARGUMENT_ERROR;
 
   private static final String USAGE =
-      "bin/hadoop " + AUDIT_TOOL +
+      "hadoop " + AUDIT_TOOL +
           " s3a://<bucket_name>/<destination_dir_path>/" +
           " s3a://<bucket_name>/<audit_logs_dir_path>/" + "\n";
 

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/audit/mapreduce/S3AAuditLogMergerAndParser.java

@@ -96,6 +96,7 @@ public HashMap<String, String> parseAuditLog(String singleAuditLog) {
         }
       }
     }
+    LOG.info("MMT audit map: {}", auditLogMap);
     return auditLogMap;
   }
 
@@ -123,7 +124,7 @@ public HashMap<String, String> parseReferrerHeader(String referrerHeader) {
 
     int lengthOfReferrer = httpReferrer.length();
     int start = 0;
-    LOG.debug("HttpReferrer headers string: {}", httpReferrer);
+    LOG.info("HttpReferrer headers string: {}", httpReferrer);
     while (start < lengthOfReferrer) {
       // splits "key" and "value" of each header
       int equals = httpReferrer.indexOf("=", start);
@@ -146,7 +147,7 @@ public HashMap<String, String> parseReferrerHeader(String referrerHeader) {
       start = end + 1;
     }
 
-    LOG.debug("HttpReferrer headers map:{}", referrerHeaderMap);
+    LOG.info("HttpReferrer headers map:{}", referrerHeaderMap);
     return referrerHeaderMap;
   }
 
@@ -221,7 +222,7 @@ public boolean mergeAndParseAuditLogFiles(FileSystem fileSystem,
 
               // Get the referrer header value from the audit logs.
               String referrerHeader = auditLogMap.get(REFERRER_HEADER_KEY);
-              LOG.debug("Parsed referrer header : {}", referrerHeader);
+              LOG.info("Parsed referrer header : {}", referrerHeader);
               // referrer header value isn't parse-able.
               if (StringUtils.isBlank(referrerHeader) || referrerHeader
                   .equals("-")) {
@@ -280,4 +281,8 @@ public boolean mergeAndParseAuditLogFiles(FileSystem fileSystem,
   public long getAuditLogsParsed() {
     return auditLogsParsed;
   }
+
+  public long getReferrerHeaderLogParsed() {
+    return referrerHeaderLogParsed;
+  }
 }

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/audit/TestS3AAuditLogMergerAndParser.java

@@ -24,14 +24,17 @@
 import java.nio.file.Files;
 import java.util.Map;
 
+import org.assertj.core.api.Assertions;
 import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.s3a.AbstractS3ATestBase;
 import org.apache.hadoop.fs.s3a.audit.mapreduce.S3AAuditLogMergerAndParser;
+import org.apache.hadoop.util.Preconditions;
 
 /**
  * This will implement different tests on S3AAuditLogMergerAndParser class.
@@ -170,12 +173,14 @@ public void testParseAuditLog() {
   public void testParseAuditLogEmptyAndNull() {
     Map<String, String> parseAuditLogResultEmpty =
         s3AAuditLogMergerAndParser.parseAuditLog("");
-    assertTrue("the returned list should be empty for this test",
-        parseAuditLogResultEmpty.isEmpty());
+    Assertions.assertThat(parseAuditLogResultEmpty)
+        .describedAs("Audit log map should be empty")
+        .isEmpty();
     Map<String, String> parseAuditLogResultNull =
         s3AAuditLogMergerAndParser.parseAuditLog(null);
-    assertTrue("the returned list should be empty for this test",
-        parseAuditLogResultNull.isEmpty());
+    Assertions.assertThat(parseAuditLogResultEmpty)
+        .describedAs("Audit log map should be empty")
+        .isEmpty();
   }
 
   /**
@@ -220,21 +225,30 @@ public void testParseReferrerHeaderEmptyAndNull() {
    */
   @Test
   public void testMergeAndParseAuditLogFiles() throws IOException {
-    sampleDir = Files.createTempDirectory("sampleDir").toFile();
-    sampleFile = File.createTempFile("sampleFile", ".txt", sampleDir);
-    try (FileWriter fw = new FileWriter(sampleFile)) {
-      fw.write(SAMPLE_LOG_ENTRY);
-      fw.write(SAMPLE_LOG_ENTRY_1);
-      fw.flush();
-    }
+    // Getting the audit dir and file path from test/resources/
+    String auditLogDir = this.getClass().getClassLoader().getResource(
+        "TestAuditLogs").toString();
+    String auditSingleFile = this.getClass().getClassLoader().getResource(
+        "TestAuditLogs/sampleLog1").toString();
+
+    Preconditions.checkArgument(!StringUtils.isAnyBlank(auditLogDir,
+        auditSingleFile), String.format("Audit path should not be empty. Check "
+        + "test/resources. auditLogDir : %s, auditLogSingleFile :%s",
+        auditLogDir, auditSingleFile));
+    Path auditDirPath = new Path(auditLogDir);
+    Path auditFilePath = new Path(auditSingleFile);
+
     sampleDestDir = Files.createTempDirectory("sampleDestDir").toFile();
-    Path logsPath = new Path(sampleDir.toURI());
     Path destPath = new Path(sampleDestDir.toURI());
-    FileSystem fileSystem = logsPath.getFileSystem(getConfiguration());
+    FileSystem fileSystem = auditFilePath.getFileSystem(getConfiguration());
     boolean mergeAndParseResult =
         s3AAuditLogMergerAndParser.mergeAndParseAuditLogFiles(fileSystem,
-            logsPath, destPath);
-    assertTrue("the result should be true", mergeAndParseResult);
+            auditDirPath, destPath);
+    assertTrue("The merge and parse failed for the audit log",
+        mergeAndParseResult);
+    // 35 audit logs with referrer in each of the 2 sample files.
+    assertEquals("", s3AAuditLogMergerAndParser.getAuditLogsParsed(), 36 + 36);
+    assertEquals("", s3AAuditLogMergerAndParser.getReferrerHeaderLogParsed(), 36 + 36);
   }
 
   /**