From 9f30916a1b81a2a4e3993aecda1fdd2cc1d34224 Mon Sep 17 00:00:00 2001 From: Takanobu Asanuma Date: Thu, 20 Dec 2018 10:03:33 +0900 Subject: [PATCH] HDFS-13661. Ls command with e option fails when the filesystem is not HDFS. (cherry picked from commit d9635759182b614a1dd5034c30978e7c4be8d0dd) --- .../java/org/apache/hadoop/fs/shell/Ls.java | 40 +++++++---- .../org/apache/hadoop/fs/shell/TestLs.java | 67 ++++++++++++++++++- 2 files changed, 93 insertions(+), 14 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Ls.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Ls.java index a2d5017f6fe..9f713d76578 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Ls.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Ls.java @@ -25,6 +25,7 @@ import java.util.Comparator; 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 @@ class Ls extends FsCommand { * 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 @@ class Ls extends FsCommand { * 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 @@ class Ls extends FsCommand { * 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 @@ class Ls extends FsCommand { * 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 @@ class Ls extends FsCommand { * 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 @@ class Ls extends FsCommand { * 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 @@ class Ls extends FsCommand { * 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); @@ -298,19 +314,19 @@ class Ls extends FsCommand { 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(); diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestLs.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestLs.java index 0df1a6a8d3a..2e5c3e3bca0 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestLs.java +++ b/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.ByteArrayOutputStream; 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()); 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 class TestLs { assertFalse(ls.isOrderSize()); assertFalse(ls.isOrderTime()); assertFalse(ls.isUseAtime()); + assertFalse(ls.isDisplayECPolicy()); } // check the -C option is recognised @@ -101,6 +105,7 @@ public class TestLs { assertFalse(ls.isOrderSize()); assertFalse(ls.isOrderTime()); assertFalse(ls.isUseAtime()); + assertFalse(ls.isDisplayECPolicy()); } // check the -d option is recognised @@ -118,6 +123,7 @@ public class TestLs { assertFalse(ls.isOrderSize()); assertFalse(ls.isOrderTime()); assertFalse(ls.isUseAtime()); + assertFalse(ls.isDisplayECPolicy()); } // check the -h option is recognised @@ -135,6 +141,7 @@ public class TestLs { assertFalse(ls.isOrderSize()); assertFalse(ls.isOrderTime()); assertFalse(ls.isUseAtime()); + assertFalse(ls.isDisplayECPolicy()); } // check the -R option is recognised @@ -152,6 +159,7 @@ public class TestLs { assertFalse(ls.isOrderSize()); assertFalse(ls.isOrderTime()); assertFalse(ls.isUseAtime()); + assertFalse(ls.isDisplayECPolicy()); } // check the -r option is recognised @@ -169,6 +177,7 @@ public class TestLs { assertFalse(ls.isOrderSize()); assertFalse(ls.isOrderTime()); assertFalse(ls.isUseAtime()); + assertFalse(ls.isDisplayECPolicy()); } // check the -S option is recognised @@ -186,6 +195,7 @@ public class TestLs { assertTrue(ls.isOrderSize()); assertFalse(ls.isOrderTime()); assertFalse(ls.isUseAtime()); + assertFalse(ls.isDisplayECPolicy()); } // check the -t option is recognised @@ -203,6 +213,7 @@ public class TestLs { 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 class TestLs { 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 class TestLs { assertFalse(ls.isOrderSize()); assertTrue(ls.isOrderTime()); assertFalse(ls.isUseAtime()); + assertFalse(ls.isDisplayECPolicy()); } // chheck the -u option is recognised @@ -257,6 +270,25 @@ public class TestLs { 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 options = new LinkedList(); + 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 class TestLs { 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 class TestLs { 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 class TestLs { assertEquals("Ls.getName", expected, actual); } + @Test(expected = UnsupportedOperationException.class) + public void processPathFileDisplayECPolicyWhenUnsupported() + throws IOException { + TestFile testFile = new TestFile("testDirectory", "testFile"); + LinkedList pathData = new LinkedList(); + pathData.add(testFile.getPathData()); + Ls ls = new Ls(); + LinkedList options = new LinkedList(); + 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 = new LinkedList(); + pathData.add(testDir.getPathData()); + Ls ls = new Ls(); + LinkedList options = new LinkedList(); + 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(