HDFS-9157. [OEV and OIV] : Unnecessary parsing for mandatory arguements if '-h' option is specified as the only option (Contributed by nijel)

hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt

@@ -2066,6 +2066,9 @@ Release 2.8.0 - UNRELEASED
     HDFS-9187. Fix null pointer error in Globber when FS was not constructed
     via FileSystem#createFileSystem (cmccabe)
 
+    HDFS-9157. [OEV and OIV] : Unnecessary parsing for mandatory arguements if
+    '-h' option is specified as the only option (nijel via vinayakumarb)
+
 Release 2.7.2 - UNRELEASED
 
   INCOMPATIBLE CHANGES

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineEditsViewer/OfflineEditsViewer.java

@@ -39,7 +39,8 @@
 @InterfaceAudience.Private
 @InterfaceStability.Unstable
 public class OfflineEditsViewer extends Configured implements Tool {
-
+  private static final String HELP_OPT = "-h";
+  private static final String HELP_LONGOPT = "--help";
   private final static String defaultProcessor = "xml";
 
   /**
@@ -192,7 +193,12 @@ public int run(String[] argv) throws Exception {
     Options options = buildOptions();
     if(argv.length == 0) {
       printHelp();
-      return -1;
+      return 0;
+    }
+    // print help and exit with zero exit code
+    if (argv.length == 1 && isHelpOption(argv[0])) {
+      printHelp();
+      return 0;
     }
     CommandLineParser parser = new PosixParser();
     CommandLine cmd;
@@ -205,7 +211,9 @@ public int run(String[] argv) throws Exception {
       return -1;
     }
     
-    if(cmd.hasOption("h")) { // print help and exit
+    if (cmd.hasOption("h")) {
+      // print help and exit with non zero exit code since
+      // it is not expected to give help and other options together.
       printHelp();
       return -1;
     }
@@ -237,4 +245,9 @@ public static void main(String[] argv) throws Exception {
     int res = ToolRunner.run(new OfflineEditsViewer(), argv);
     System.exit(res);
   }
+
+  private static boolean isHelpOption(String arg) {
+    return arg.equalsIgnoreCase(HELP_OPT) ||
+        arg.equalsIgnoreCase(HELP_LONGOPT);
+  }
 }

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

@@ -41,6 +41,8 @@
  */
 @InterfaceAudience.Private
 public class OfflineImageViewerPB {
+  private static final String HELP_OPT = "-h";
+  private static final String HELP_LONGOPT = "--help";
   public static final Log LOG = LogFactory.getLog(OfflineImageViewerPB.class);
 
   private final static String usage = "Usage: bin/hdfs oiv [OPTIONS] -i INPUTFILE -o OUTPUTFILE\n"
@@ -131,7 +133,11 @@ public static int run(String[] args) throws Exception {
       printUsage();
       return 0;
     }
-
+    // print help and exit with zero exit code
+    if (args.length == 1 && isHelpOption(args[0])) {
+      printUsage();
+      return 0;
+    }
     CommandLineParser parser = new PosixParser();
     CommandLine cmd;
 
@@ -143,9 +149,11 @@ public static int run(String[] args) throws Exception {
       return -1;
     }
 
-    if (cmd.hasOption("h")) { // print help and exit
+    if (cmd.hasOption("h")) {
+      // print help and exit with non zero exit code since
+      // it is not expected to give help and other options together.
       printUsage();
-      return 0;
+      return -1;
     }
 
     String inputFile = cmd.getOptionValue("i");
@@ -202,4 +210,9 @@ public static int run(String[] args) throws Exception {
   private static void printUsage() {
     System.out.println(usage);
   }
+
+  private static boolean isHelpOption(String arg) {
+    return arg.equalsIgnoreCase(HELP_OPT) ||
+        arg.equalsIgnoreCase(HELP_LONGOPT);
+  }
 }

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineEditsViewer/TestOfflineEditsViewer.java

@@ -21,9 +21,11 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
+import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.io.PrintStream;
 import java.nio.ByteBuffer;
 import java.nio.channels.FileChannel;
 
@@ -35,8 +37,10 @@
 import org.apache.hadoop.hdfs.server.namenode.NameNodeLayoutVersion;
 import org.apache.hadoop.hdfs.server.namenode.OfflineEditsViewerHelper;
 import org.apache.hadoop.hdfs.tools.offlineEditsViewer.OfflineEditsViewer.Flags;
+import org.apache.hadoop.io.IOUtils;
 import org.apache.hadoop.test.PathUtils;
 import org.junit.After;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -285,4 +289,23 @@ private boolean filesEqualIgnoreTrailingZeros(String filenameSmall,
 
     return true;
   }
+
+  @Test
+  public void testOfflineEditsViewerHelpMessage() throws Throwable {
+    final ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+    final PrintStream out = new PrintStream(bytes);
+    final PrintStream oldOut = System.out;
+    try {
+      System.setOut(out);
+      int status = new OfflineEditsViewer().run(new String[] { "-h" });
+      assertTrue("" + "Exit code returned for help option is incorrect",
+          status == 0);
+      Assert.assertFalse(
+          "Invalid Command error displayed when help option is passed.", bytes
+              .toString().contains("Error parsing command-line options"));
+    } finally {
+      System.setOut(oldOut);
+      IOUtils.closeStream(out);
+    }
+  }
 }

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

@@ -67,6 +67,7 @@
 import org.apache.hadoop.net.NetUtils;
 import org.apache.hadoop.security.token.Token;
 import org.junit.AfterClass;
+import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Rule;
 import org.junit.Test;
@@ -350,6 +351,30 @@ public void testInvalidProcessorOption() throws Exception {
         status != 0);
   }
 
+  @Test
+  public void testOfflineImageViewerHelpMessage() throws Throwable {
+    final ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+    final PrintStream out = new PrintStream(bytes);
+    final PrintStream oldOut = System.out;
+    try {
+      System.setOut(out);
+      int status = OfflineImageViewerPB.run(new String[] { "-h" });
+      assertTrue("Exit code returned for help option is incorrect", status == 0);
+      Assert.assertFalse(
+          "Invalid Command error displayed when help option is passed.", bytes
+              .toString().contains("Error parsing command-line options"));
+      status =
+          OfflineImageViewerPB.run(new String[] { "-h", "-i",
+              originalFsimage.getAbsolutePath(), "-o", "-", "-p",
+              "FileDistribution", "-maxSize", "512", "-step", "8" });
+      Assert.assertTrue(
+          "Exit code returned for help with other option is incorrect",
+          status == -1);
+    } finally {
+      System.setOut(oldOut);
+      IOUtils.closeStream(out);
+    }
+  }
   private void testPBDelimitedWriter(String db)
       throws IOException, InterruptedException {
     final String DELIMITER = "\t";