accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From els...@apache.org
Subject [1/3] git commit: ACCUMULO-1403 Allow setiter to operate on SortedKeyValueIterator instead of only OptionDescriber
Date Sat, 04 Jan 2014 22:32:20 GMT
Updated Branches:
  refs/heads/1.6.0-SNAPSHOT 4313ff8bb -> 68b60234f
  refs/heads/master b4aee0daa -> 799ce56ff


ACCUMULO-1403 Allow setiter to operate on SortedKeyValueIterator instead of only OptionDescriber

Prompt the user to enter the necessary information when the provided class
does not implement OptionDescriber, with a soft recommendation to implement
OptionDescriber. Does not change the functionality of setiter on OptionDescriber
implementing classes.


Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/68b60234
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/68b60234
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/68b60234

Branch: refs/heads/1.6.0-SNAPSHOT
Commit: 68b60234f60a61e9197fbea91381fb1e26937123
Parents: 4313ff8
Author: Josh Elser <elserj@apache.org>
Authored: Sat Jan 4 17:25:07 2014 -0500
Committer: Josh Elser <elserj@apache.org>
Committed: Sat Jan 4 17:25:07 2014 -0500

----------------------------------------------------------------------
 .../util/shell/commands/SetIterCommand.java     | 184 ++++++++++++-------
 .../accumulo/core/util/shell/ShellTest.java     |  37 +++-
 .../org/apache/accumulo/test/ShellServerIT.java |  86 +++++++++
 3 files changed, 240 insertions(+), 67 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/68b60234/core/src/main/java/org/apache/accumulo/core/util/shell/commands/SetIterCommand.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/util/shell/commands/SetIterCommand.java
b/core/src/main/java/org/apache/accumulo/core/util/shell/commands/SetIterCommand.java
index 2d618be..2e8d5a4 100644
--- a/core/src/main/java/org/apache/accumulo/core/util/shell/commands/SetIterCommand.java
+++ b/core/src/main/java/org/apache/accumulo/core/util/shell/commands/SetIterCommand.java
@@ -31,6 +31,8 @@ import org.apache.accumulo.core.client.IteratorSetting;
 import org.apache.accumulo.core.client.NamespaceNotFoundException;
 import org.apache.accumulo.core.client.TableNotFoundException;
 import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.Value;
 import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
 import org.apache.accumulo.core.iterators.OptionDescriber;
 import org.apache.accumulo.core.iterators.OptionDescriber.IteratorOptions;
@@ -49,6 +51,7 @@ import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Option;
 import org.apache.commons.cli.OptionGroup;
 import org.apache.commons.cli.Options;
+import org.apache.commons.lang.StringUtils;
 import org.apache.commons.vfs2.FileSystemException;
 
 public class SetIterCommand extends Command {
@@ -84,7 +87,19 @@ public class SetIterCommand extends Command {
 
     ClassLoader classloader = getClassLoader(cl, shellState);
 
-    final String name = cl.getOptionValue(nameOpt.getOpt(), setUpOptions(classloader, shellState.getReader(),
classname, options));
+    // Get the iterator options, with potentially a name provided by the OptionDescriber
impl or through user input
+    String configuredName = setUpOptions(classloader, shellState.getReader(), classname,
options);
+
+    // Try to get the name provided by the setiter command
+    String name = cl.getOptionValue(nameOpt.getOpt(), null);
+    
+    // Cannot continue if no name is provided
+    if (null == name && null == configuredName) {
+      throw new IllegalArgumentException("No provided or default name for iterator");
+    } else if (null == name) {
+      // Fall back to the name from OptionDescriber or user input if none is provided on
setiter option
+      name = configuredName;
+    }
 
     if (namespaces) {
       try {
@@ -240,11 +255,13 @@ public class SetIterCommand extends Command {
   private static String setUpOptions(ClassLoader classloader, final ConsoleReader reader,
final String className, final Map<String,String> options)
       throws IOException, ShellCommandException {
     String input;
-    OptionDescriber skvi;
-    Class<? extends OptionDescriber> clazz;
+    @SuppressWarnings("rawtypes")
+    SortedKeyValueIterator untypedInstance;
+    @SuppressWarnings("rawtypes")
+    Class<? extends SortedKeyValueIterator> clazz;
     try {
-      clazz = classloader.loadClass(className).asSubclass(OptionDescriber.class);
-      skvi = clazz.newInstance();
+      clazz = classloader.loadClass(className).asSubclass(SortedKeyValueIterator.class);
+      untypedInstance = clazz.newInstance();
     } catch (ClassNotFoundException e) {
       StringBuilder msg = new StringBuilder("Unable to load ").append(className);
       if (className.indexOf('.') < 0) {
@@ -258,57 +275,45 @@ public class SetIterCommand extends Command {
     } catch (IllegalAccessException e) {
       throw new IllegalArgumentException(e.getMessage());
     } catch (ClassCastException e) {
-      StringBuilder msg = new StringBuilder("Loaded ");
-      msg.append(className).append(" but it does not implement ");
-      msg.append(OptionDescriber.class.getSimpleName());
-      msg.append("; use 'config -s' instead.");
+      StringBuilder msg = new StringBuilder(50);
+      msg.append(className).append(" loaded successfully but does not implement SortedKeyValueIterator.");
+      msg.append(" This class cannot be used with this command.");
       throw new ShellCommandException(ErrorCode.INITIALIZATION_FAILURE, msg.toString());
     }
 
-    final IteratorOptions itopts = skvi.describeOptions();
-    if (itopts.getName() == null) {
-      throw new IllegalArgumentException(className + " described its default distinguishing
name as null");
-    }
-    String shortClassName = className;
-    if (className.contains(".")) {
-      shortClassName = className.substring(className.lastIndexOf('.') + 1);
+    @SuppressWarnings("unchecked")
+    SortedKeyValueIterator<Key,Value> skvi = (SortedKeyValueIterator<Key,Value>)
untypedInstance;
+    OptionDescriber iterOptions = null;
+    if (OptionDescriber.class.isAssignableFrom(skvi.getClass())) {
+      iterOptions = (OptionDescriber) skvi;
     }
-    final Map<String,String> localOptions = new HashMap<String,String>();
-    do {
-      // clean up the overall options that caused things to fail
-      for (String key : localOptions.keySet()) {
-        options.remove(key);
+
+    String iteratorName;
+    if (null != iterOptions) {
+      final IteratorOptions itopts = iterOptions.describeOptions();
+      iteratorName = itopts.getName();
+      
+      if (iteratorName == null) {
+        throw new IllegalArgumentException(className + " described its default distinguishing
name as null");
       }
-      localOptions.clear();
-
-      reader.println(itopts.getDescription());
-
-      String prompt;
-      if (itopts.getNamedOptions() != null) {
-        for (Entry<String,String> e : itopts.getNamedOptions().entrySet()) {
-          prompt = Shell.repeat("-", 10) + "> set " + shortClassName + " parameter " +
e.getKey() + ", " + e.getValue() + ": ";
-          reader.flush();
-          input = reader.readLine(prompt);
-          if (input == null) {
-            reader.println();
-            throw new IOException("Input stream closed");
-          } else {
-            input = new String(input);
-          }
-          // Places all Parameters and Values into the LocalOptions, even if the value is
"".
-          // This allows us to check for "" values when setting the iterators and allows
us to remove
-          // the parameter and value from the table property.
-          localOptions.put(e.getKey(), input);
-        }
+      String shortClassName = className;
+      if (className.contains(".")) {
+        shortClassName = className.substring(className.lastIndexOf('.') + 1);
       }
-
-      if (itopts.getUnnamedOptionDescriptions() != null) {
-        for (String desc : itopts.getUnnamedOptionDescriptions()) {
-          reader.println(Shell.repeat("-", 10) + "> entering options: " + desc);
-          input = "start";
-          while (true) {
-            prompt = Shell.repeat("-", 10) + "> set " + shortClassName + " option (<name>
<value>, hit enter to skip): ";
-
+      final Map<String,String> localOptions = new HashMap<String,String>();
+      do {
+        // clean up the overall options that caused things to fail
+        for (String key : localOptions.keySet()) {
+          options.remove(key);
+        }
+        localOptions.clear();
+  
+        reader.println(itopts.getDescription());
+  
+        String prompt;
+        if (itopts.getNamedOptions() != null) {
+          for (Entry<String,String> e : itopts.getNamedOptions().entrySet()) {
+            prompt = Shell.repeat("-", 10) + "> set " + shortClassName + " parameter "
+ e.getKey() + ", " + e.getValue() + ": ";
             reader.flush();
             input = reader.readLine(prompt);
             if (input == null) {
@@ -317,22 +322,77 @@ public class SetIterCommand extends Command {
             } else {
               input = new String(input);
             }
-
-            if (input.length() == 0)
-              break;
-
-            String[] sa = input.split(" ", 2);
-            localOptions.put(sa[0], sa[1]);
+            // Places all Parameters and Values into the LocalOptions, even if the value
is "".
+            // This allows us to check for "" values when setting the iterators and allows
us to remove
+            // the parameter and value from the table property.
+            localOptions.put(e.getKey(), input);
+          }
+        }
+  
+        if (itopts.getUnnamedOptionDescriptions() != null) {
+          for (String desc : itopts.getUnnamedOptionDescriptions()) {
+            reader.println(Shell.repeat("-", 10) + "> entering options: " + desc);
+            input = "start";
+            prompt = Shell.repeat("-", 10) + "> set " + shortClassName + " option (<name>
<value>, hit enter to skip): ";
+            while (true) {
+              reader.flush();
+              input = reader.readLine(prompt);
+              if (input == null) {
+                reader.println();
+                throw new IOException("Input stream closed");
+              } else {
+                input = new String(input);
+              }
+  
+              if (input.length() == 0)
+                break;
+  
+              String[] sa = input.split(" ", 2);
+              localOptions.put(sa[0], sa[1]);
+            }
           }
         }
+  
+        options.putAll(localOptions);
+        if (!iterOptions.validateOptions(options))
+          reader.println("invalid options for " + clazz.getName());
+  
+      } while (!iterOptions.validateOptions(options));
+    } else {
+      reader.flush();
+      reader.println("The iterator class does not implement OptionDescriber. Consider this
for better iterator configuration using this setiter command.");
+      iteratorName = reader.readLine("Name for iterator (enter to skip): ");
+      if (null == iteratorName) {
+        reader.println();
+        throw new IOException("Input stream closed");
+      } else if (StringUtils.isWhitespace(iteratorName)) {
+        // Treat whitespace or empty string as no name provided
+        iteratorName = null;
       }
-
+      
+      reader.flush();
+      reader.println("Optional, configure name-value options for iterator:");
+      String prompt = Shell.repeat("-", 10) + "> set option (<name> <value>,
hit enter to skip): ";
+      final HashMap<String,String> localOptions = new HashMap<String,String>();
+      
+      while (true) {
+        reader.flush();
+        input = reader.readLine(prompt);
+        if (input == null) {
+          reader.println();
+          throw new IOException("Input stream closed");
+        } else if (StringUtils.isWhitespace(input)) {
+          break;
+        } 
+
+        String[] sa = input.split(" ", 2);
+        localOptions.put(sa[0], sa[1]);
+      }
+      
       options.putAll(localOptions);
-      if (!skvi.validateOptions(options))
-        reader.println("invalid options for " + clazz.getName());
-
-    } while (!skvi.validateOptions(options));
-    return itopts.getName();
+    }
+    
+    return iteratorName;
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/accumulo/blob/68b60234/core/src/test/java/org/apache/accumulo/core/util/shell/ShellTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/accumulo/core/util/shell/ShellTest.java b/core/src/test/java/org/apache/accumulo/core/util/shell/ShellTest.java
index 97852ca..bf203f7 100644
--- a/core/src/test/java/org/apache/accumulo/core/util/shell/ShellTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/util/shell/ShellTest.java
@@ -19,9 +19,8 @@ package org.apache.accumulo.core.util.shell;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
-import java.io.FileDescriptor;
-import java.io.FileInputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStream;
 import java.text.DateFormat;
 import java.text.SimpleDateFormat;
@@ -52,6 +51,25 @@ public class ShellTest {
     }
   }
 
+  public static class StringInputStream extends InputStream {
+    private String source = "";
+    private int offset = 0;
+
+    @Override
+    public int read() throws IOException {
+      if (offset == source.length())
+        return '\n';
+      else
+        return source.charAt(offset++);
+    }
+
+    public void set(String other) {
+      source = other;
+      offset = 0;
+    }
+  }
+
+  private StringInputStream input;
   private TestOutputStream output;
   private Shell shell;
 
@@ -84,7 +102,8 @@ public class ShellTest {
   public void setup() throws IOException {
     Shell.log.setLevel(Level.OFF);
     output = new TestOutputStream();
-    shell = new Shell(new ConsoleReader(new FileInputStream(FileDescriptor.in), output));
+    input = new StringInputStream();
+    shell = new Shell(new ConsoleReader(input, output));
     shell.setLogErrorsToConsole();
     shell.config("--fake", "-u", "test", "-p", "secret");
   }
@@ -236,8 +255,16 @@ public class ShellTest {
     exec(cmdFullPackage, false, "class not found", true);
 
     String cmdNoOption = "setiter -class java.lang.String -p 1";
-    exec(cmdNoOption, false, "Loaded", true);
-
+    exec(cmdNoOption, false, "loaded successfully but does not implement SortedKeyValueIterator",
true);
+
+    input.set("\n\n");
+    exec("setiter -scan -class org.apache.accumulo.core.iterators.ColumnFamilyCounter -p
30 -name foo", true);
+    
+    input.set("bar\nname value\n");
+    exec("setiter -scan -class org.apache.accumulo.core.iterators.ColumnFamilyCounter -p
31", true);
+    
+    //TODO can't verify this as config -t fails, functionality verified in ShellServerIT
+    
     exec("deletetable t -f", true, "Table: [t] has been deleted");
   }
 }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/68b60234/test/src/test/java/org/apache/accumulo/test/ShellServerIT.java
----------------------------------------------------------------------
diff --git a/test/src/test/java/org/apache/accumulo/test/ShellServerIT.java b/test/src/test/java/org/apache/accumulo/test/ShellServerIT.java
index 7a126ee..90f0a19 100644
--- a/test/src/test/java/org/apache/accumulo/test/ShellServerIT.java
+++ b/test/src/test/java/org/apache/accumulo/test/ShellServerIT.java
@@ -19,6 +19,7 @@ package org.apache.accumulo.test;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.io.File;
 import java.io.IOException;
@@ -34,6 +35,7 @@ import org.apache.accumulo.core.Constants;
 import org.apache.accumulo.core.client.Connector;
 import org.apache.accumulo.core.client.Scanner;
 import org.apache.accumulo.core.client.TableNotFoundException;
+import org.apache.accumulo.core.client.admin.TableOperations;
 import org.apache.accumulo.core.client.impl.Namespaces;
 import org.apache.accumulo.core.conf.AccumuloConfiguration;
 import org.apache.accumulo.core.conf.Property;
@@ -370,6 +372,90 @@ public class ShellServerIT extends SimpleMacIT {
     exec("deletetable -f t");
 
   }
+  
+  @Test(timeout = 30 * 1000)
+  public void setIterOptionPrompt() throws Exception {
+    Connector conn = getConnector();
+    String tableName = "setIterOptionPrompt";
+    
+    exec("createtable " + tableName);
+    input.set("\n\n");
+    // Setting a non-optiondescriber with no name should fail
+    exec("setiter -scan -class org.apache.accumulo.core.iterators.ColumnFamilyCounter -p
30", false);
+    
+    // Name as option will work
+    exec("setiter -scan -class org.apache.accumulo.core.iterators.ColumnFamilyCounter -p
30 -name cfcounter", true);
+
+    String expectedKey = "table.iterator.scan.cfcounter";
+    String expectedValue = "30,org.apache.accumulo.core.iterators.ColumnFamilyCounter";
+    TableOperations tops = conn.tableOperations();
+    checkTableForProperty(tops, tableName, expectedKey, expectedValue);
+
+    exec("deletetable " + tableName, true);
+    tableName = tableName + "1";
+    
+    exec("createtable " + tableName, true);
+    
+    input.set("customcfcounter\n\n");
+    
+    // Name on the CLI should override OptionDescriber (or user input name, in this case)
+    exec("setiter -scan -class org.apache.accumulo.core.iterators.ColumnFamilyCounter -p
30", true);
+    expectedKey = "table.iterator.scan.customcfcounter";
+    expectedValue = "30,org.apache.accumulo.core.iterators.ColumnFamilyCounter";
+    checkTableForProperty(tops, tableName, expectedKey, expectedValue);
+
+    exec("deletetable " + tableName, true);
+    tableName = tableName + "1";
+    
+    exec("createtable " + tableName, true);
+    
+    input.set("customcfcounter\nname1 value1\nname2 value2\n\n");
+    
+    // Name on the CLI should override OptionDescriber (or user input name, in this case)
+    exec("setiter -scan -class org.apache.accumulo.core.iterators.ColumnFamilyCounter -p
30", true);
+    expectedKey = "table.iterator.scan.customcfcounter";
+    expectedValue = "30,org.apache.accumulo.core.iterators.ColumnFamilyCounter";
+    checkTableForProperty(tops, tableName, expectedKey, expectedValue);
+    expectedKey = "table.iterator.scan.customcfcounter.opt.name1";
+    expectedValue = "value1";
+    checkTableForProperty(tops, tableName, expectedKey, expectedValue);
+    expectedKey = "table.iterator.scan.customcfcounter.opt.name2";
+    expectedValue = "value2";
+    checkTableForProperty(tops, tableName, expectedKey, expectedValue);
+
+    exec("deletetable " + tableName, true);
+    tableName = tableName + "1";
+    
+    exec("createtable " + tableName, true);
+    
+    input.set("\nname1 value1.1,value1.2,value1.3\nname2 value2\n\n");
+    
+    // Name on the CLI should override OptionDescriber (or user input name, in this case)
+    exec("setiter -scan -class org.apache.accumulo.core.iterators.ColumnFamilyCounter -p
30 -name cfcounter", true);
+    expectedKey = "table.iterator.scan.cfcounter";
+    expectedValue = "30,org.apache.accumulo.core.iterators.ColumnFamilyCounter";
+    checkTableForProperty(tops, tableName, expectedKey, expectedValue);
+    expectedKey = "table.iterator.scan.cfcounter.opt.name1";
+    expectedValue = "value1.1,value1.2,value1.3";
+    checkTableForProperty(tops, tableName, expectedKey, expectedValue);
+    expectedKey = "table.iterator.scan.cfcounter.opt.name2";
+    expectedValue = "value2";
+    checkTableForProperty(tops, tableName, expectedKey, expectedValue);
+  }
+  
+  protected void checkTableForProperty(TableOperations tops, String tableName, String expectedKey,
String expectedValue) throws Exception {
+    for (int i = 0; i < 5; i++) {
+      for (Entry<String,String> entry : tops.getProperties(tableName)) {
+        if (expectedKey.equals(entry.getKey())) {
+          assertEquals(expectedValue, entry.getValue());
+          return;
+        }
+      }
+      Thread.sleep(500);
+    }
+    
+    fail("Failed to find expected property on " + tableName + ": " + expectedKey + "=" +
expectedValue);
+  }
 
   @Test(timeout = 30 * 1000)
   public void notable() throws Exception {


Mime
View raw message