Merge 1091902 and 1091970 from trunk for HADOOP-7224.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/yahoo-merge@1126769 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -27,6 +27,8 @@ Trunk (unreleased changes)
 
     HADOOP-7202. Improve shell Command base class.  (Daryn Sharp via szetszwo)
 
+    HADOOP-7224. Add CommandFactory to shell.  (Daryn Sharp via szetszwo)
+
   OPTIMIZATIONS
 
   BUG FIXES

src/java/org/apache/hadoop/fs/FsShell.java

@@ -26,7 +26,6 @@ import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Date;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.TimeZone;
 import java.util.zip.GZIPInputStream;
@@ -34,8 +33,10 @@ import java.util.zip.GZIPInputStream;
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.conf.Configured;
+import org.apache.hadoop.fs.shell.Command;
+import org.apache.hadoop.fs.shell.CommandFactory;
 import org.apache.hadoop.fs.shell.CommandFormat;
-import org.apache.hadoop.fs.shell.Count;
+import org.apache.hadoop.fs.shell.FsCommand;
 import org.apache.hadoop.io.DataInputBuffer;
 import org.apache.hadoop.io.DataOutputBuffer;
 import org.apache.hadoop.io.IOUtils;
@@ -57,6 +58,8 @@ public class FsShell extends Configured implements Tool {
 
   protected FileSystem fs;
   private Trash trash;
+  protected CommandFactory commandFactory;
+
   public static final SimpleDateFormat dateForm = 
     new SimpleDateFormat("yyyy-MM-dd HH:mm");
   protected static final SimpleDateFormat modifFmt =
@@ -82,6 +85,7 @@ public class FsShell extends Configured implements Tool {
     super(conf);
     fs = null;
     trash = null;
+    commandFactory = new CommandFactory();
   }
   
   protected void init() throws IOException {
@@ -1385,9 +1389,7 @@ public class FsShell extends Configured implements Tool {
       "[-tail [-f] <path>] [-text <path>]\n\t" +
       "[" + FsShellPermissions.CHMOD_USAGE + "]\n\t" +
       "[" + FsShellPermissions.CHOWN_USAGE + "]\n\t" +
-      "[" + FsShellPermissions.CHGRP_USAGE + "]\n\t" +      
-      "[" + Count.USAGE + "]\n\t" +      
-      "[-help [cmd]]\n";
+      "[" + FsShellPermissions.CHGRP_USAGE + "]";
 
     String conf ="-conf <configuration file>:  Specify an application configuration file.";
  
@@ -1546,7 +1548,10 @@ public class FsShell extends Configured implements Tool {
     String help = "-help [cmd]: \tDisplays help for given command or all commands if none\n" +
       "\t\tis specified.\n";
 
-    if ("fs".equals(cmd)) {
+    Command instance = commandFactory.getInstance("-" + cmd);
+    if (instance != null) {
+      System.out.println(instance.getDescription());
+    } else if ("fs".equals(cmd)) {
       System.out.println(fs);
     } else if ("conf".equals(cmd)) {
       System.out.println(conf);
@@ -1610,12 +1615,16 @@ public class FsShell extends Configured implements Tool {
       System.out.println(chown);
     } else if ("chgrp".equals(cmd)) {
       System.out.println(chgrp);
-    } else if (Count.NAME.equals(cmd)) {
-      System.out.println(Count.DESCRIPTION);
     } else if ("help".equals(cmd)) {
       System.out.println(help);
     } else {
       System.out.println(summary);
+      for (String thisCmdName : commandFactory.getNames()) {
+        instance = commandFactory.getInstance(thisCmdName);
+        System.out.println(instance.getUsage());
+      }
+      System.out.println("\t[-help [cmd]]\n");
+      
       System.out.println(fs);
       System.out.println(ls);
       System.out.println(lsr);
@@ -1644,7 +1653,12 @@ public class FsShell extends Configured implements Tool {
       System.out.println(chmod);
       System.out.println(chown);      
       System.out.println(chgrp);
-      System.out.println(Count.DESCRIPTION);
+
+      for (String thisCmdName : commandFactory.getNames()) {
+        instance = commandFactory.getInstance(thisCmdName);
+        System.out.println(instance.getDescription());
+      }
+
       System.out.println(help);
     }        
   }
@@ -1727,9 +1741,13 @@ public class FsShell extends Configured implements Tool {
    * Displays format of commands.
    * 
    */
-  private static void printUsage(String cmd) {
+  private void printUsage(String cmd) {
     String prefix = "Usage: java " + FsShell.class.getSimpleName();
-    if ("-fs".equals(cmd)) {
+
+    Command instance = commandFactory.getInstance(cmd);
+    if (instance != null) {
+      System.err.println(prefix + " [" + instance.getUsage() + "]");
+    } else if ("-fs".equals(cmd)) {
       System.err.println("Usage: java FsShell" + 
                          " [-fs <local | file system URI>]");
     } else if ("-conf".equals(cmd)) {
@@ -1747,8 +1765,6 @@ public class FsShell extends Configured implements Tool {
     } else if ("-df".equals(cmd) ) {
       System.err.println("Usage: java FsShell" +
                          " [" + cmd + " [<path>]]");
-    } else if ("-count".equals(cmd)) {
-      System.err.println(prefix + " [" + Count.USAGE + "]");
     } else if ("-rm".equals(cmd) || "-rmr".equals(cmd)) {
       System.err.println("Usage: java FsShell [" + cmd + 
                            " [-skipTrash] <src>]");
@@ -1786,7 +1802,6 @@ public class FsShell extends Configured implements Tool {
       System.err.println("           [-df [<path>]]");
       System.err.println("           [-du [-s] [-h] <path>]");
       System.err.println("           [-dus <path>]");
-      System.err.println("           [" + Count.USAGE + "]");
       System.err.println("           [-mv <src> <dst>]");
       System.err.println("           [-cp <src> <dst>]");
       System.err.println("           [-rm [-skipTrash] <path>]");
@@ -1810,6 +1825,10 @@ public class FsShell extends Configured implements Tool {
       System.err.println("           [" + FsShellPermissions.CHMOD_USAGE + "]");      
       System.err.println("           [" + FsShellPermissions.CHOWN_USAGE + "]");
       System.err.println("           [" + FsShellPermissions.CHGRP_USAGE + "]");
+      for (String name : commandFactory.getNames()) {
+      	instance = commandFactory.getInstance(name);
+        System.err.println("           [" + instance.getUsage() + "]");
+      }
       System.err.println("           [-help [cmd]]");
       System.err.println();
       ToolRunner.printGenericCommandUsage(System.err);
@@ -1820,7 +1839,12 @@ public class FsShell extends Configured implements Tool {
    * run
    */
   public int run(String argv[]) throws Exception {
-
+    // TODO: This isn't the best place, but this class is being abused with
+    // subclasses which of course override this method.  There really needs
+    // to be a better base class for all commands
+    commandFactory.setConf(getConf());
+    commandFactory.registerCommands(FsCommand.class);
+    
     if (argv.length < 1) {
       printUsage(""); 
       return -1;
@@ -1873,7 +1897,19 @@ public class FsShell extends Configured implements Tool {
 
     exitCode = 0;
     try {
-      if ("-put".equals(cmd) || "-copyFromLocal".equals(cmd)) {
+      Command instance = commandFactory.getInstance(cmd);
+      if (instance != null) {
+        try {
+          exitCode = instance.run(Arrays.copyOfRange(argv, i, argv.length));
+        } catch (Exception e) {
+          exitCode = -1;
+          LOG.debug("Error", e);
+          instance.displayError(e);
+          if (e instanceof IllegalArgumentException) {
+            printUsage(cmd);
+          }
+        }
+      } else if ("-put".equals(cmd) || "-copyFromLocal".equals(cmd)) {
         Path[] srcs = new Path[argv.length-2];
         for (int j=0 ; i < argv.length-1 ;) 
           srcs[j++] = new Path(argv[i++]);
@@ -1934,12 +1970,6 @@ public class FsShell extends Configured implements Tool {
         du(argv, i);
       } else if ("-dus".equals(cmd)) {
         dus(argv, i);
-      } else if ("-count".equals(cmd)) {
-        // TODO: next two lines are a temporary crutch until this entire
-        // block is overhauled
-        Count runner = ReflectionUtils.newInstance(Count.class, getConf());
-        runner.setCommandName(cmd); // TODO: will change with factory
-        exitCode = runner.run(Arrays.copyOfRange(argv, 1, argv.length));
       } else if ("-mkdir".equals(cmd)) {
         exitCode = doall(cmd, argv, i);
       } else if ("-touchz".equals(cmd)) {

src/java/org/apache/hadoop/fs/shell/Command.java

@@ -365,4 +365,35 @@ abstract public class Command extends Configured {
   public void displayWarning(String message) {
     err.println(getCommandName() + ": " + message);
   }
+  
+  /**
+   * The short usage suitable for the synopsis
+   * @return "name options"
+   */
+  public String getUsage() {
+    return getCommandField("USAGE");
+  }
+
+  /**
+   * The long usage suitable for help output
+   * @return text of the usage
+   */
+  public String getDescription() {
+    return getCommandField("DESCRIPTION");
+  }
+
+  /**
+   * Get a public static class field
+   * @param field the field to retrieve
+   * @return String of the field
+   */
+  private String getCommandField(String field) {
+    String value;
+    try {
+      value = (String)this.getClass().getField(field).get(null);
+    } catch (Exception e) {
+      throw new RuntimeException(StringUtils.stringifyException(e));
+    }
+    return value;
+  }
 }

src/java/org/apache/hadoop/fs/shell/CommandFactory.java

@@ -0,0 +1,130 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.shell;
+
+import java.util.Arrays;
+import java.util.Hashtable;
+
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.conf.Configurable;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.conf.Configured;
+import org.apache.hadoop.util.ReflectionUtils;
+import org.apache.hadoop.util.StringUtils;
+
+/** class to search for and register commands */
+
+@InterfaceAudience.Private
+@InterfaceStability.Unstable
+
+public class CommandFactory extends Configured implements Configurable {
+  private Hashtable<String, Class<? extends Command>> classMap =
+    new Hashtable<String, Class<? extends Command>>();
+
+  /** Factory constructor for commands */
+  public CommandFactory() {
+    this(null);
+  }
+  
+  /**
+   * Factory constructor for commands
+   * @param conf the hadoop configuration
+   */
+  public CommandFactory(Configuration conf) {
+    super(conf);
+  }
+
+  /**
+   * Invokes "static void registerCommands(CommandFactory)" on the given class.
+   * This method abstracts the contract between the factory and the command
+   * class.  Do not assume that directly invoking registerCommands on the
+   * given class will have the same effect.
+   * @param registrarClass class to allow an opportunity to register
+   */
+  public void registerCommands(Class<?> registrarClass) {
+    try {
+      registrarClass.getMethod(
+          "registerCommands", CommandFactory.class
+      ).invoke(null, this);
+    } catch (Exception e) {
+      throw new RuntimeException(StringUtils.stringifyException(e));
+    }
+  }
+
+  /**
+   * Register the given class as handling the given list of command
+   * names.
+   * @param cmdClass the class implementing the command names
+   * @param names one or more command names that will invoke this class
+   */
+  public void addClass(Class<? extends Command> cmdClass, String ... names) {
+    for (String name : names) classMap.put(name, cmdClass);
+  }
+  
+  /**
+   * Returns the class implementing the given command.  The
+   * class must have been registered via
+   * {@link #addClass(Class, String...)}
+   * @param cmd name of the command
+   * @return instance of the requested command
+   */
+  protected Class<? extends Command> getClass(String cmd) {
+    return classMap.get(cmd);
+  }
+  
+  /**
+   * Returns an instance of the class implementing the given command.  The
+   * class must have been registered via
+   * {@link #addClass(Class, String...)}
+   * @param cmd name of the command
+   * @return instance of the requested command
+   */
+  public Command getInstance(String cmd) {
+    return getInstance(cmd, getConf());
+  }
+
+  /**
+   * Get an instance of the requested command
+   * @param cmdName name of the command to lookup
+   * @param conf the hadoop configuration
+   * @return the {@link Command} or null if the command is unknown
+   */
+  public Command getInstance(String cmdName, Configuration conf) {
+    if (conf == null) throw new NullPointerException("configuration is null");
+    
+    Command instance = null;
+    Class<? extends Command> cmdClass = getClass(cmdName);
+    if (cmdClass != null) {
+      instance = ReflectionUtils.newInstance(cmdClass, conf);
+      instance.setCommandName(cmdName);
+    }
+    return instance;
+  }
+  
+  /**
+   * Gets all of the registered commands
+   * @return a sorted list of command names
+   */
+  public String[] getNames() {
+    String[] names = classMap.keySet().toArray(new String[0]);
+    Arrays.sort(names);
+    return names;
+  }
+}

src/java/org/apache/hadoop/fs/shell/Count.java

@@ -34,6 +34,14 @@ import org.apache.hadoop.fs.FsShell;
 @InterfaceStability.Evolving
 
 public class Count extends FsCommand {
+  /**
+   * Register the names for the count command
+   * @param factory the command factory that will instantiate this class
+   */
+  public static void registerCommands(CommandFactory factory) {
+    factory.addClass(Count.class, "-count");
+  }
+
   public static final String NAME = "count";
   public static final String USAGE = "-" + NAME + "[-q] <path>";
   public static final String DESCRIPTION = CommandUtils.formatDescription(USAGE, 

src/java/org/apache/hadoop/fs/shell/FsCommand.java

@@ -37,6 +37,14 @@ import org.apache.hadoop.fs.Path;
 // used to implement unnecessary abstract methods in the base class
 
 abstract public class FsCommand extends Command {
+  /**
+   * Register the command classes used by the fs subcommand
+   * @param factory where to register the class
+   */
+  public static void registerCommands(CommandFactory factory) {
+    Count.registerCommands(factory);
+  }
+
   protected FsCommand() {}
   
   protected FsCommand(Configuration conf) {

src/test/core/org/apache/hadoop/fs/shell/TestCommandFactory.java

@@ -0,0 +1,87 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.shell;
+
+import static org.junit.Assert.*;
+
+import org.apache.hadoop.conf.Configuration;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TestCommandFactory {
+  static CommandFactory factory;
+  static Configuration conf = new Configuration();
+  
+  static void registerCommands(CommandFactory factory) {
+  }
+  
+  @Before
+  public void testSetup() {
+    factory = new CommandFactory(conf);
+    assertNotNull(factory);
+  }
+  
+  @Test
+  public void testRegistration() {
+    assertArrayEquals(new String []{}, factory.getNames());
+
+    factory.registerCommands(TestRegistrar.class);
+    String [] names = factory.getNames();
+    assertArrayEquals(new String []{"tc1", "tc2", "tc2.1"}, names);
+    
+    factory.addClass(TestCommand3.class, "tc3");
+    names = factory.getNames();
+    assertArrayEquals(new String []{"tc1", "tc2", "tc2.1", "tc3"}, names);
+  }
+  
+  @Test
+  public void testGetInstances() {
+    factory.registerCommands(TestRegistrar.class);
+
+    Command instance;
+    instance = factory.getInstance("blarg");
+    assertNull(instance);
+    
+    instance = factory.getInstance("tc1");
+    assertNotNull(instance);
+    assertEquals(TestCommand1.class, instance.getClass());
+    assertEquals("tc1", instance.getCommandName());
+    
+    instance = factory.getInstance("tc2");
+    assertNotNull(instance);
+    assertEquals(TestCommand2.class, instance.getClass());
+    assertEquals("tc2", instance.getCommandName());
+
+    instance = factory.getInstance("tc2.1");
+    assertNotNull(instance);
+    assertEquals(TestCommand2.class, instance.getClass());    
+    assertEquals("tc2.1", instance.getCommandName());
+  }
+  
+  static class TestRegistrar {
+    public static void registerCommands(CommandFactory factory) {
+      factory.addClass(TestCommand1.class, "tc1");
+      factory.addClass(TestCommand2.class, "tc2", "tc2.1");
+    }
+  }
+  
+  static class TestCommand1 extends FsCommand {}
+  static class TestCommand2 extends FsCommand {}
+  static class TestCommand3 extends FsCommand {}
+}