[
https://issues.apache.org/jira/browse/HADOOP-10521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13983001#comment-13983001
]
Charles Lamb commented on HADOOP-10521:
---------------------------------------
Hi Yi,
In general this looks good. I have a few little nits below.
Charles
Index:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/XAttrCommands.java
===================================================================
---
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/XAttrCommands.java
(revision 0)
+++
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/XAttrCommands.java
(working copy)
@@ -0,0 +1,234 @@
[email protected]
[email protected]
+class XAttrCommands extends FsCommand {
+ private static String GET_FATTR = "getfattr";
+ private static String SET_FATTR = "setfattr";
Consider a final decl.
+
+ public static void registerCommands(CommandFactory factory) {
+ factory.addClass(GetfattrCommand.class, "-" + GET_FATTR);
+ factory.addClass(SetfattrCommand.class, "-" + SET_FATTR);
+ }
+
Consider a final decl.
+ private static enum ENCODE {
+ TEXT,
+ HEX,
+ BASE64;
+ }
+
+ private static final String ENCODE_HEX = "0x";
+ private static final String ENCODE_BASE64 = "0s";
+
+ private static String convert(String name, byte[] value, ENCODE encode)
+ throws IOException {
+ StringBuilder buffer = new StringBuilder();
Consider a final decl.
+ buffer.append(name);
+ if (value != null && value.length != 0) {
+ buffer.append("=");
+ if (encode == ENCODE.TEXT) {
+ buffer.append("\"").append(new String(value, "utf-8")).append("\"");
+ } else if (encode == ENCODE.HEX) {
+ buffer.append(ENCODE_HEX).append(Hex.encodeHexString(value));
+ } else if (encode == ENCODE.BASE64) {
+ buffer.append(ENCODE_BASE64).append(Base64.encodeBase64String(value));
+ }
+ }
+ return buffer.toString();
+ }
+
+ private static byte[] convert(String value) throws IOException {
+ byte[] result = null;
+ if (value != null) {
+ if (value.length() >= 2) {
+ String en = value.substring(0,2);
s/0,2/0, 2/
final?
+ if (value.startsWith("\"") && value.endsWith("\"")) {
+ value = value.substring(1, value.length()-1);
s/)-1/) - 1/
Eclipse will issue warnings for making assignments to method arguments
("value" in this case). Consider calling the formal arg "value_arg"
and then adding a "String value = value_arg;" decl above.
+ result = value.getBytes("utf-8");
+ } else if (en.equalsIgnoreCase(ENCODE_HEX)) {
+ value = value.substring(2, value.length());
+ try {
+ result = Hex.decodeHex(value.toCharArray());
+ } catch (DecoderException e) {
+ throw new IOException(e);
+ }
+ } else if (en.equalsIgnoreCase(ENCODE_BASE64)) {
+ value = value.substring(2, value.length());
+ result = Base64.decodeBase64(value);
+ }
+ }
+ if (result == null) {
+ result = value.getBytes("utf-8");
+ }
+ }
+ return result;
+ }
+
+ /**
+ * Implementing the '-getfattr' command for the the FsShell.
s/Implementing/Implements/
s/the the/the/
+ */
+ public static class GetfattrCommand extends FsCommand {
Consider "final" for the three decls below:
+ public static String NAME = GET_FATTR;
+ public static String USAGE = "[-R] {-n name | -d} [-e en] <path>";
+ public static String DESCRIPTION =
+ "Displays the file name, and the set of extended attribute names " +
+ "(and optionally values) which are associated with that file or " +
+ "directory.\n" +
+ "-R: List the attributes of all files and directories recursively.\n" +
+ "-n name: Dump the value of the named extended attribute.\n" +
+ "-d: Dump the values of all extended attributes associated with " +
+ "pathname.\n" +
+ "-e en: Encode values after retrieving them. Valid values of en " +
+ "are \"text\", \"hex\", and \"base64\". Values encoded as text " +
+ "strings are enclosed in double quotes (\"), while strings encoded" +
+ " as hexadecimal and base64 are prefixed with 0x and 0s,
respectively.\n" +
+ "<path>: File or directory to list.\n";
"Displays the extended attribute names and values (if any) for a " +
"file or directory.\n" +
"-R: Recurisively list the attributes for all files and directories.\n" +
"-n name: Dump the value of the named extended attribute.\n" +
"-d: Dump the values of all extended attributes associated with " +
"pathname.\n" +
"-e <encoding>: Encode values after retrieving them. Valid encodings " +
"are \"text\", \"hex\", and \"base64\". Values encoded as text " +
"strings are enclosed in double quotes (\"), while values encoded" +
" as hexadecimal and base64 are prefixed with 0x and 0s, respectively.\n" +
"<path>: The file or directory.\n";
+ private static final String NAME_OPT = "-n";
+ private static final String ENCODE_OPT = "-e";
+ CommandFormat cf = new CommandFormat(0, Integer.MAX_VALUE, "d", "R");
final?
+
+ private String name = null;
+ private boolean dump = false;
+ private ENCODE encode = ENCODE.TEXT;
+ @Override
+ protected void processOptions(LinkedList<String> args) throws IOException {
+ name = StringUtils.popOptionWithArgument(NAME_OPT, args);
+ String en = StringUtils.popOptionWithArgument(ENCODE_OPT, args);
+ if (en != null) {
+ encode = ENCODE.valueOf(en.toUpperCase());
+ }
+
+ cf.parse(args);
+ setRecursive(cf.getOpt("R"));
+ dump = cf.getOpt("d");
+
+ if (!dump && name == null) {
+ throw new HadoopIllegalArgumentException(
+ "Must specify '-n name' or '-d' option.");
+ }
+
+ if (args.isEmpty()) {
+ throw new HadoopIllegalArgumentException("<path> is missing");
s/missing/missing./
+ }
+ if (args.size() > 1) {
+ throw new HadoopIllegalArgumentException("Too many arguments");
s/arguments/arguments./
+ }
+ }
+
+ @Override
+ protected void processPath(PathData item) throws IOException {
+ out.println("# file: " + item);
+ if (dump) {
+ Map<String, byte[]> xattrs = item.fs.getXAttrs(item.path);
+ if (xattrs != null) {
+ Iterator<Entry<String, byte[]>> iter = xattrs.entrySet().iterator();
+ while(iter.hasNext()) {
+ Entry<String, byte[]> entry = iter.next();
+ out.println(convert(entry.getKey(), entry.getValue(), encode));
+ }
+ }
+ } else {
+ byte[] value = item.fs.getXAttr(item.path, name);
+ if (value != null) {
+ out.println(convert(name, value, encode));
+ }
+ }
+ }
+ }
+
+ /**
+ * Implementing the '-setfattr' command for the the FsShell.
+ */
+ public static class SetfattrCommand extends FsCommand {
Consider final for the 3 decls below.
+ public static String NAME = SET_FATTR;
+ public static String USAGE = "{-n name [-v value] | -x name} <path>";
+ public static String DESCRIPTION =
+ "Associates a new value with an extended attribute name for file or " +
+ "directory.\n" +
Sets an extended attribute name and value for a file or directory.\n
+ "-n name: Specifies the name of the extended attribute to set.\n" +
-n name: The extended attribute name.\n
+ "-v value: Specifies the new value of the extended attribute. There " +
+ "are three methods available for encoding the value. If the given " +
+ "string is enclosed in double quotes, the inner string is treated " +
+ "as text. If the given string begins with 0x or 0X, it expresses " +
+ "a hexadecimal number. If the given string begins with 0s or 0S, " +
+ "base64 encoding is expected.\n" +
-v value: The extended attribute value. There are three different
encoding methods for the value. If the argument is enclosed in double
quotes, then the value is the string inside the quotes. If the
argument is prefixed with 0x or 0X, then it is taken as a hexadecimal
number. If the argument begins with 0s or 0S, then it is taken as a
base64 encoding.\n
+ "-x name: Remove the named extended attribute entirely.\n" +
-x name: Remove the extended attribute.\n
+ "<path>: File or directory to list.\n";
<path>: The file or directory
+ private static final String NAME_OPT = "-n";
+ private static final String VALUE_OPT = "-v";
+ private static final String REMOVE_OPT = "-x";
+
+ private String name = null;
+ private byte[] value = null;
+ private String xname = null;
+
+ @Override
+ protected void processOptions(LinkedList<String> args) throws IOException {
+ name = StringUtils.popOptionWithArgument(NAME_OPT, args);
+ String v = StringUtils.popOptionWithArgument(VALUE_OPT, args);
+ if (v != null) {
+ value = convert(v);
+ }
+ xname = StringUtils.popOptionWithArgument(REMOVE_OPT, args);
+
+ if (name != null && xname != null) {
+ throw new HadoopIllegalArgumentException(
+ "Can not specify both '-n name' and '-x name' option.");
+ }
+ if (name == null && xname == null) {
+ throw new HadoopIllegalArgumentException(
+ "Must specify '-n name' or '-x name' option.");
+ }
+
+ if (args.isEmpty()) {
+ throw new HadoopIllegalArgumentException("<path> is missing");
s/missing/missing./
+ }
+ if (args.size() > 1) {
+ throw new HadoopIllegalArgumentException("Too many arguments");
s/arguments/arguments./
+ }
+ }
+
+ @Override
+ protected void processPath(PathData item) throws IOException {
+ if (name != null) {
+ item.fs.setXAttr(item.path, name, value);
+ } else if (xname != null) {
+ item.fs.removeXAttr(item.path, xname);
+ }
+ }
+ }
+}
Index:
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestXAttrCommands.java
===================================================================
---
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestXAttrCommands.java
(revision 0)
+++
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestXAttrCommands.java
(working copy)
+
+ private int runCommand(String[] commands) throws Exception {
+ return ToolRunner.run(conf, new FsShell(), commands);
+ }
+
+}
Remove the blank line above.
> FsShell commands for extended attributes.
> -----------------------------------------
>
> Key: HADOOP-10521
> URL: https://issues.apache.org/jira/browse/HADOOP-10521
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs
> Affects Versions: HDFS XAttrs (HDFS-2006)
> Reporter: Yi Liu
> Assignee: Yi Liu
> Attachments: HADOOP-10521.1.patch, HADOOP-10521.2.patch,
> HADOOP-10521.3.patch, HADOOP-10521.patch
>
>
> “setfattr” and “getfattr” commands are added to FsShell for XAttr, and these
> are the same as in Linux.
--
This message was sent by Atlassian JIRA
(v6.2#6252)