You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by el...@apache.org on 2014/01/04 23:32:21 UTC
[2/3] git commit: ACCUMULO-1403 Allow setiter to operate on
SortedKeyValueIterator instead of only OptionDescriber
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/master
Commit: 68b60234f60a61e9197fbea91381fb1e26937123
Parents: 4313ff8
Author: Josh Elser <el...@apache.org>
Authored: Sat Jan 4 17:25:07 2014 -0500
Committer: Josh Elser <el...@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 {