HDFS-13661. Ls command with e option fails when the filesystem is not HDFS.

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Ls.java

@@ -25,6 +25,7 @@
 import java.util.Date;
 import java.util.LinkedList;
 
+import com.google.common.annotations.VisibleForTesting;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.util.StringUtils;
 
@@ -155,7 +156,7 @@ protected void processOptions(LinkedList<String> args)
    * Should display only paths of files and directories.
    * @return true display paths only, false display all fields
    */
-  @InterfaceAudience.Private
+  @VisibleForTesting
   boolean isPathOnly() {
     return this.pathOnly;
   }
@@ -164,7 +165,7 @@ boolean isPathOnly() {
    * Should the contents of the directory be shown or just the directory?
    * @return true if directory contents, false if just directory
    */
-  @InterfaceAudience.Private
+  @VisibleForTesting
   boolean isDirRecurse() {
     return this.dirRecurse;
   }
@@ -173,12 +174,12 @@ boolean isDirRecurse() {
    * Should file sizes be returned in human readable format rather than bytes?
    * @return true is human readable, false if bytes
    */
-  @InterfaceAudience.Private
+  @VisibleForTesting
   boolean isHumanReadable() {
     return this.humanReadable;
   }
 
-  @InterfaceAudience.Private
+  @VisibleForTesting
   private boolean isHideNonPrintable() {
     return hideNonPrintable;
   }
@@ -187,7 +188,7 @@ private boolean isHideNonPrintable() {
    * Should directory contents be displayed in reverse order
    * @return true reverse order, false default order
    */
-  @InterfaceAudience.Private
+  @VisibleForTesting
   boolean isOrderReverse() {
     return this.orderReverse;
   }
@@ -196,7 +197,7 @@ boolean isOrderReverse() {
    * Should directory contents be displayed in mtime order.
    * @return true mtime order, false default order
    */
-  @InterfaceAudience.Private
+  @VisibleForTesting
   boolean isOrderTime() {
     return this.orderTime;
   }
@@ -205,7 +206,7 @@ boolean isOrderTime() {
    * Should directory contents be displayed in size order.
    * @return true size order, false default order
    */
-  @InterfaceAudience.Private
+  @VisibleForTesting
   boolean isOrderSize() {
     return this.orderSize;
   }
@@ -214,13 +215,28 @@ boolean isOrderSize() {
    * Should access time be used rather than modification time.
    * @return true use access time, false use modification time
    */
-  @InterfaceAudience.Private
+  @VisibleForTesting
   boolean isUseAtime() {
     return this.useAtime;
   }
 
+  /**
+   * Should EC policies be displayed.
+   * @return true display EC policies, false doesn't display EC policies
+   */
+  @VisibleForTesting
+  boolean isDisplayECPolicy() {
+    return this.displayECPolicy;
+  }
+
   @Override
   protected void processPathArgument(PathData item) throws IOException {
+    if (isDisplayECPolicy() && item.fs.getContentSummary(item.path)
+        .getErasureCodingPolicy() == null) {
+      throw new UnsupportedOperationException("FileSystem "
+          + item.fs.getUri() + " does not support Erasure Coding");
+    }
+
     // implicitly recurse once for cmdline directories
     if (dirRecurse && item.stat.isDirectory()) {
       recursePath(item);
@@ -320,19 +336,19 @@ private void adjustColumnWidths(PathData items[]) throws IOException {
     StringBuilder fmt = new StringBuilder();
     fmt.append("%s%s"); // permission string
     fmt.append("%"  + maxRepl  + "s ");
+    fmt.append((maxOwner > 0) ? "%-" + maxOwner + "s " : "%s");
+    fmt.append((maxGroup > 0) ? "%-" + maxGroup + "s " : "%s");
     // Do not use '%-0s' as a formatting conversion, since it will throw a
     // a MissingFormatWidthException if it is used in String.format().
     // http://docs.oracle.com/javase/1.5.0/docs/api/java/util/Formatter.html#intFlags
     if(displayECPolicy){
-      int maxEC=0;
+      int maxEC = 0;
       for (PathData item : items) {
           ContentSummary contentSummary = item.fs.getContentSummary(item.path);
           maxEC=maxLength(maxEC,contentSummary.getErasureCodingPolicy().length());
       }
-      fmt.append(" %"+maxEC+"s ");
+      fmt.append((maxEC > 0) ? "%-" + maxEC + "s " : "%s");
     }
-    fmt.append((maxOwner > 0) ? "%-" + maxOwner + "s " : "%s");
-    fmt.append((maxGroup > 0) ? "%-" + maxGroup + "s " : "%s");
     fmt.append("%"  + maxLen   + "s ");
     fmt.append("%s %s"); // mod time & path
     lineFormat = fmt.toString();

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestLs.java

@@ -17,6 +17,7 @@
  */
 package org.apache.hadoop.fs.shell;
 
+import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY;
 import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SHELL_MISSING_DEFAULT_FS_WARNING_KEY;
 import static org.junit.Assert.*;
 import static org.mockito.Matchers.eq;
@@ -26,6 +27,7 @@
 import java.io.IOException;
 import java.io.PrintStream;
 import java.net.URI;
+import java.net.URISyntaxException;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Date;
@@ -57,17 +59,18 @@ public class TestLs {
   @BeforeClass
   public static void setup() throws IOException {
     conf = new Configuration();
-    conf.set("fs.defaultFS", "mockfs:///");
+    conf.set(FS_DEFAULT_NAME_KEY, "mockfs:///");
     conf.setClass("fs.mockfs.impl", MockFileSystem.class, FileSystem.class);
     mockFs = mock(FileSystem.class);
   }
 
   @Before
-  public void resetMock() throws IOException {
+  public void resetMock() throws IOException, URISyntaxException {
     reset(mockFs);
     AclStatus mockAclStatus = mock(AclStatus.class);
     when(mockAclStatus.getEntries()).thenReturn(new ArrayList<AclEntry>());
     when(mockFs.getAclStatus(any(Path.class))).thenReturn(mockAclStatus);
+    when(mockFs.getUri()).thenReturn(new URI(conf.get(FS_DEFAULT_NAME_KEY)));
   }
 
   // check that default options are correct
@@ -84,6 +87,7 @@ public void processOptionsNone() throws IOException {
     assertFalse(ls.isOrderSize());
     assertFalse(ls.isOrderTime());
     assertFalse(ls.isUseAtime());
+    assertFalse(ls.isDisplayECPolicy());
   }
 
   // check the -C option is recognised
@@ -101,6 +105,7 @@ public void processOptionsPathOnly() throws IOException {
     assertFalse(ls.isOrderSize());
     assertFalse(ls.isOrderTime());
     assertFalse(ls.isUseAtime());
+    assertFalse(ls.isDisplayECPolicy());
   }
 
   // check the -d option is recognised
@@ -118,6 +123,7 @@ public void processOptionsDirectory() throws IOException {
     assertFalse(ls.isOrderSize());
     assertFalse(ls.isOrderTime());
     assertFalse(ls.isUseAtime());
+    assertFalse(ls.isDisplayECPolicy());
   }
 
   // check the -h option is recognised
@@ -135,6 +141,7 @@ public void processOptionsHuman() throws IOException {
     assertFalse(ls.isOrderSize());
     assertFalse(ls.isOrderTime());
     assertFalse(ls.isUseAtime());
+    assertFalse(ls.isDisplayECPolicy());
   }
 
   // check the -R option is recognised
@@ -152,6 +159,7 @@ public void processOptionsRecursive() throws IOException {
     assertFalse(ls.isOrderSize());
     assertFalse(ls.isOrderTime());
     assertFalse(ls.isUseAtime());
+    assertFalse(ls.isDisplayECPolicy());
   }
 
   // check the -r option is recognised
@@ -169,6 +177,7 @@ public void processOptionsReverse() throws IOException {
     assertFalse(ls.isOrderSize());
     assertFalse(ls.isOrderTime());
     assertFalse(ls.isUseAtime());
+    assertFalse(ls.isDisplayECPolicy());
   }
 
   // check the -S option is recognised
@@ -186,6 +195,7 @@ public void processOptionsSize() throws IOException {
     assertTrue(ls.isOrderSize());
     assertFalse(ls.isOrderTime());
     assertFalse(ls.isUseAtime());
+    assertFalse(ls.isDisplayECPolicy());
   }
 
   // check the -t option is recognised
@@ -203,6 +213,7 @@ public void processOptionsMtime() throws IOException {
     assertFalse(ls.isOrderSize());
     assertTrue(ls.isOrderTime());
     assertFalse(ls.isUseAtime());
+    assertFalse(ls.isDisplayECPolicy());
   }
 
   // check the precedence of the -t and -S options
@@ -221,6 +232,7 @@ public void processOptionsMtimeSize() throws IOException {
     assertFalse(ls.isOrderSize());
     assertTrue(ls.isOrderTime());
     assertFalse(ls.isUseAtime());
+    assertFalse(ls.isDisplayECPolicy());
   }
 
   // check the precedence of the -t, -S and -r options
@@ -240,6 +252,7 @@ public void processOptionsMtimeSizeReverse() throws IOException {
     assertFalse(ls.isOrderSize());
     assertTrue(ls.isOrderTime());
     assertFalse(ls.isUseAtime());
+    assertFalse(ls.isDisplayECPolicy());
   }
 
   // chheck the -u option is recognised
@@ -257,6 +270,25 @@ public void processOptionsAtime() throws IOException {
     assertFalse(ls.isOrderSize());
     assertFalse(ls.isOrderTime());
     assertTrue(ls.isUseAtime());
+    assertFalse(ls.isDisplayECPolicy());
+  }
+
+  // chheck the -e option is recognised
+  @Test
+  public void processOptionsDisplayECPolicy() throws IOException {
+    LinkedList<String> options = new LinkedList<String>();
+    options.add("-e");
+    Ls ls = new Ls();
+    ls.processOptions(options);
+    assertFalse(ls.isPathOnly());
+    assertTrue(ls.isDirRecurse());
+    assertFalse(ls.isHumanReadable());
+    assertFalse(ls.isRecursive());
+    assertFalse(ls.isOrderReverse());
+    assertFalse(ls.isOrderSize());
+    assertFalse(ls.isOrderTime());
+    assertFalse(ls.isUseAtime());
+    assertTrue(ls.isDisplayECPolicy());
   }
 
   // check all options is handled correctly
@@ -271,6 +303,7 @@ public void processOptionsAll() throws IOException {
     options.add("-t"); // time order
     options.add("-S"); // size order
     options.add("-u"); // show atime
+    options.add("-e"); // show EC policies
     Ls ls = new Ls();
     ls.processOptions(options);
     assertTrue(ls.isPathOnly());
@@ -281,6 +314,7 @@ public void processOptionsAll() throws IOException {
     assertFalse(ls.isOrderSize()); // -t overrules -S
     assertTrue(ls.isOrderTime());
     assertTrue(ls.isUseAtime());
+    assertTrue(ls.isDisplayECPolicy());
   }
 
   // check listing of a single file
@@ -1100,6 +1134,35 @@ public void getName() {
     assertEquals("Ls.getName", expected, actual);
   }
 
+  @Test(expected = UnsupportedOperationException.class)
+  public void processPathFileDisplayECPolicyWhenUnsupported()
+      throws IOException {
+    TestFile testFile = new TestFile("testDirectory", "testFile");
+    LinkedList<PathData> pathData = new LinkedList<PathData>();
+    pathData.add(testFile.getPathData());
+    Ls ls = new Ls();
+    LinkedList<String> options = new LinkedList<String>();
+    options.add("-e");
+    ls.processOptions(options);
+    ls.processArguments(pathData);
+  }
+
+  @Test(expected = UnsupportedOperationException.class)
+  public void processPathDirDisplayECPolicyWhenUnsupported()
+      throws IOException {
+    TestFile testFile = new TestFile("testDirectory", "testFile");
+    TestFile testDir = new TestFile("", "testDirectory");
+    testDir.setIsDir(true);
+    testDir.addContents(testFile);
+    LinkedList<PathData> pathData = new LinkedList<PathData>();
+    pathData.add(testDir.getPathData());
+    Ls ls = new Ls();
+    LinkedList<String> options = new LinkedList<String>();
+    options.add("-e");
+    ls.processOptions(options);
+    ls.processArguments(pathData);
+  }
+
   // test class representing a file to be listed
   static class TestFile {
     private static final SimpleDateFormat DATE_FORMAT = new SimpleDateFormat(