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 <els...@apache.org> Authored: Sat Jan 4 17:25:07 2014 -0500 Committer: Josh Elser <els...@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 {
