ctubbsii closed pull request #397: ACCUMULO-3389 Iterator Names can't contain
dots
URL: https://github.com/apache/accumulo/pull/397
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/core/src/main/java/org/apache/accumulo/core/client/IteratorSetting.java
b/core/src/main/java/org/apache/accumulo/core/client/IteratorSetting.java
index a6d9a0994d..a3c31bae91 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/IteratorSetting.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/IteratorSetting.java
@@ -85,10 +85,11 @@ public String getName() {
}
/**
- * Set the iterator's name. Must be a simple alphanumeric identifier.
+ * Set the iterator's name. Must be a simple alphanumeric identifier. The
iterator name also may not contain a dot/period.
*/
public void setName(String name) {
checkArgument(name != null, "name is null");
+ checkArgument(!name.contains("."), "Iterator name cannot contain a
dot/period");
this.name = name;
}
diff --git
a/core/src/main/java/org/apache/accumulo/core/iterators/IteratorUtil.java
b/core/src/main/java/org/apache/accumulo/core/iterators/IteratorUtil.java
index 586f2c746a..e10cf8057f 100644
--- a/core/src/main/java/org/apache/accumulo/core/iterators/IteratorUtil.java
+++ b/core/src/main/java/org/apache/accumulo/core/iterators/IteratorUtil.java
@@ -177,7 +177,7 @@ public static void parseIterConf(IteratorScope scope,
List<IterInfo> iters, Map<
options.put(optName, entry.getValue());
} else {
- log.warn("Unrecognizable option: " + entry.getKey());
+ throw new IllegalArgumentException("Invalid iterator format: " +
entry.getKey());
}
}
diff --git
a/core/src/test/java/org/apache/accumulo/core/client/IteratorSettingTest.java
b/core/src/test/java/org/apache/accumulo/core/client/IteratorSettingTest.java
index ddea0b352d..2bde67ab7b 100644
---
a/core/src/test/java/org/apache/accumulo/core/client/IteratorSettingTest.java
+++
b/core/src/test/java/org/apache/accumulo/core/client/IteratorSettingTest.java
@@ -110,4 +110,13 @@ public void testEquivalentConstructor() {
assertNotEquals(setting1, notEquals2);
}
+
+ /**
+ * Iterator names cannot contain dots. Throw IllegalArgumentException is
invalid name is used.
+ */
+ @Test(expected = IllegalArgumentException.class)
+ public void testIteratorNameCannotContainDot() {
+ new IteratorSetting(500, "iterator.name.with.dots",
Combiner.class.getName());
+ }
+
}
diff --git
a/core/src/test/java/org/apache/accumulo/core/iterators/IteratorUtilTest.java
b/core/src/test/java/org/apache/accumulo/core/iterators/IteratorUtilTest.java
index db308472cd..1077483972 100644
---
a/core/src/test/java/org/apache/accumulo/core/iterators/IteratorUtilTest.java
+++
b/core/src/test/java/org/apache/accumulo/core/iterators/IteratorUtilTest.java
@@ -42,9 +42,12 @@
import org.apache.accumulo.core.iterators.user.SummingCombiner;
import org.junit.Assert;
import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
public class IteratorUtilTest {
+ private static final Logger log =
LoggerFactory.getLogger(IteratorUtilTest.class);
private static final Collection<ByteSequence> EMPTY_COL_FAMS = new
ArrayList<>();
static class WrappedIter implements SortedKeyValueIterator<Key,Value> {
@@ -311,4 +314,71 @@ public void onlyReadsRelevantIteratorScopeConfigurations()
throws Exception {
IterInfo ii = iterators.get(0);
Assert.assertEquals(new IterInfo(50, SummingCombiner.class.getName(),
"foo"), ii);
}
+
+ /**
+ * Iterators should not contain dots in the name. Also, if the split size on
"." is greater than one, it should be 3, i.e., itername.opt.optname
+ */
+ @Test
+ public void testInvalidIteratorFormats() {
+
+ Map<String,String> data = new HashMap<>();
+ List<IterInfo> iterators = new ArrayList<>();
+ Map<String,Map<String,String>> options = new HashMap<>();
+ AccumuloConfiguration conf;
+
+ // create iterator with 'dot' in name
+ try {
+ data.put(Property.TABLE_ITERATOR_SCAN_PREFIX + "foo.bar", "50," +
SummingCombiner.class.getName());
+ conf = new ConfigurationCopy(data);
+ IteratorUtil.parseIterConf(IteratorScope.scan, iterators, options, conf);
+ } catch (IllegalArgumentException ex) {
+ log.debug("caught expected exception: " + ex.getMessage());
+ }
+ data.clear();
+ iterators.clear();
+ options.clear();
+
+ // create iterator with 'dot' in name and with split size of 3. If split
size of three, then
+ // second part must be 'opt'.
+ try {
+ data.put(Property.TABLE_ITERATOR_SCAN_PREFIX + "foo.bar.baz", "49," +
SummingCombiner.class.getName());
+ conf = new ConfigurationCopy(data);
+ IteratorUtil.parseIterConf(IteratorScope.scan, iterators, options, conf);
+ } catch (IllegalArgumentException ex) {
+ log.debug("caught expected exception: " + ex.getMessage());
+ }
+ data.clear();
+ iterators.clear();
+ options.clear();
+
+ // create iterator with invalid option format
+ try {
+ data.put(Property.TABLE_ITERATOR_SCAN_PREFIX + "foobar", "48," +
SummingCombiner.class.getName());
+ data.put(Property.TABLE_ITERATOR_SCAN_PREFIX + "foobar.opt",
"fakevalue");
+ conf = new ConfigurationCopy(data);
+ IteratorUtil.parseIterConf(IteratorScope.scan, iterators, options, conf);
+ Assert.assertEquals(1, iterators.size());
+ IterInfo ii = iterators.get(0);
+ Assert.assertEquals(new IterInfo(48, SummingCombiner.class.getName(),
"foobar"), ii);
+ } catch (IllegalArgumentException ex) {
+ log.debug("caught expected exception: " + ex.getMessage());
+ }
+ data.clear();
+ iterators.clear();
+ options.clear();
+
+ // create iterator with 'opt' in incorrect position
+ try {
+ data.put(Property.TABLE_ITERATOR_SCAN_PREFIX + "foobaz", "47," +
SummingCombiner.class.getName());
+ data.put(Property.TABLE_ITERATOR_SCAN_PREFIX + "foobaz.fake.opt",
"fakevalue");
+ conf = new ConfigurationCopy(data);
+ IteratorUtil.parseIterConf(IteratorScope.scan, iterators, options, conf);
+ Assert.assertEquals(1, iterators.size());
+ IterInfo ii = iterators.get(0);
+ Assert.assertEquals(new IterInfo(47, SummingCombiner.class.getName(),
"foobaz"), ii);
+ } catch (IllegalArgumentException ex) {
+ log.debug("caught expected exception: " + ex.getMessage());
+ }
+ }
+
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
