HDFS-8205. CommandFormat#parse() should not parse option as value of …

…option. (Contributed by Peter Shi and Xiaoyu Yao)
dianhu · Apr 27, 2015 · 0d5b014 · 0d5b014
1 parent 7f07c4d
commit 0d5b014
Show file tree

Hide file tree

Showing 5 changed files with 73 additions and 3 deletions.
diff --git a/...-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandFormat.java b/...-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandFormat.java
@@ -114,7 +114,8 @@ public void parse(List<String> args) {
         options.put(opt, Boolean.TRUE);
       } else if (optionsWithValue.containsKey(opt)) {
         args.remove(pos);
-        if (pos < args.size() && (args.size() > minPar)) {
+        if (pos < args.size() && (args.size() > minPar)
+                && !args.get(pos).startsWith("-")) {
           arg = args.get(pos);
           args.remove(pos);
         } else {

diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCount.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCount.java
@@ -315,6 +315,34 @@ public void processPathWithQuotasBySSDStorageTypesHeader() throws Exception {
     verifyNoMoreInteractions(out);
   }
 
+  @Test
+  public void processPathWithQuotasByQTVH() throws Exception {
+    Path path = new Path("mockfs:/test");
+
+    when(mockFs.getFileStatus(eq(path))).thenReturn(fileStat);
+
+    PrintStream out = mock(PrintStream.class);
+
+    Count count = new Count();
+    count.out = out;
+
+    LinkedList<String> options = new LinkedList<String>();
+    options.add("-q");
+    options.add("-t");
+    options.add("-v");
+    options.add("-h");
+    options.add("dummy");
+    count.processOptions(options);
+    String withStorageTypeHeader =
+        // <----13---> <-------17------>
+        "   DISK_QUOTA    REM_DISK_QUOTA " +
+        "    SSD_QUOTA     REM_SSD_QUOTA " +
+        "ARCHIVE_QUOTA REM_ARCHIVE_QUOTA " +
+        "PATHNAME";
+    verify(out).println(withStorageTypeHeader);
+    verifyNoMoreInteractions(out);
+  }
+
   @Test
   public void processPathWithQuotasByMultipleStorageTypesContent() throws Exception {
     Path path = new Path("mockfs:/test");

diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
@@ -618,6 +618,9 @@ Release 2.7.1 - UNRELEASED
     HDFS-8070. Pre-HDFS-7915 DFSClient cannot use short circuit on
     post-HDFS-7915 DataNode (cmccabe)
 
+    HDFS-8205. CommandFormat#parse() should not parse option as
+    value of option. (Peter Shi and Xiaoyu Yao via Arpit Agarwal)
+
 Release 2.7.0 - 2015-04-20
 
   INCOMPATIBLE CHANGES

diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSAdmin.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSAdmin.java
@@ -221,9 +221,9 @@ private static class ClearSpaceQuotaCommand extends DFSAdminCommand {
     ClearSpaceQuotaCommand(String[] args, int pos, FileSystem fs) {
       super(fs);
       CommandFormat c = new CommandFormat(1, Integer.MAX_VALUE);
+      c.addOptionWithValue("storageType");
       List<String> parameters = c.parse(args, pos);
-      String storageTypeString =
-          StringUtils.popOptionWithArgument("-storageType", parameters);
+      String storageTypeString = c.getOptValue("storageType");
       if (storageTypeString != null) {
         this.type = StorageType.parseStorageType(storageTypeString);
       }

diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testHDFSConf.xml b/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testHDFSConf.xml
@@ -7721,6 +7721,44 @@
       </comparators>
     </test>
 
+    <test> <!-- TESTED -->
+      <description>setSpaceQuota -storageType: directory with quota by storage type</description>
+      <test-commands>
+        <command>-fs NAMENODE -mkdir /ttt</command>
+        <dfs-admin-command>-fs NAMENODE -setSpaceQuota 1m -storageType DISK /ttt </dfs-admin-command>
+        <command>-fs NAMENODE -count -q -t DISK /ttt</command>
+      </test-commands>
+      <cleanup-commands>
+        <command>-fs NAMENODE -rm -r /ttt</command>
+      </cleanup-commands>
+      <comparators>
+        <comparator>
+          <type>RegexpComparator</type>
+          <expected-output>( |\t)*1048576( |\t)*1048576 /ttt</expected-output>
+        </comparator>
+      </comparators>
+    </test>
+
+    <test> <!-- TESTED -->
+      <description>clrSpaceQuota -storageType: directory quota by storage type</description>
+      <test-commands>
+        <command>-fs NAMENODE -mkdir /ttt</command>
+        <dfs-admin-command>-fs NAMENODE -setSpaceQuota 1m -storageType DISK /ttt </dfs-admin-command>
+        <command>-fs NAMENODE -count -q -t DISK /ttt</command>
+        <dfs-admin-command>-fs NAMENODE -clrSpaceQuota -storageType DISK /ttt </dfs-admin-command>
+        <command>-fs NAMENODE -count -q -t DISK /ttt</command>
+      </test-commands>
+      <cleanup-commands>
+        <command>-fs NAMENODE -rm -r /ttt</command>
+      </cleanup-commands>
+      <comparators>
+        <comparator>
+          <type>RegexpComparator</type>
+          <expected-output>( |\t)*none( |\t)*inf /ttt</expected-output>
+        </comparator>
+      </comparators>
+    </test>
+
     <test> <!-- TESTED -->
       <description>count: directory using relative path with -q option</description>
       <test-commands>