accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ctubb...@apache.org
Subject [accumulo] branch 1.8 updated: ACCUMULO-3389 Iterator Names can't contain dots (#397)
Date Fri, 16 Mar 2018 22:05:18 GMT
This is an automated email from the ASF dual-hosted git repository.

ctubbsii pushed a commit to branch 1.8
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/1.8 by this push:
     new 6ef308a  ACCUMULO-3389 Iterator Names can't contain dots (#397)
6ef308a is described below

commit 6ef308a888d25fe73734dac600b8d14460d22a6c
Author: Mark Owens <jmarkowe@gmail.com>
AuthorDate: Fri Mar 16 18:05:16 2018 -0400

    ACCUMULO-3389 Iterator Names can't contain dots (#397)
    
    Updated code to check iterator names for the presence of dots. If name is found to contain
a
    dot then an IllegalArgumentException is thrown. Wrote unit/IT tests also.
---
 .../accumulo/core/client/IteratorSetting.java      |  3 +-
 .../accumulo/core/iterators/IteratorUtil.java      |  2 +-
 .../accumulo/core/client/IteratorSettingTest.java  |  9 +++
 .../accumulo/core/iterators/IteratorUtilTest.java  | 70 ++++++++++++++++++++++
 4 files changed, 82 insertions(+), 2 deletions(-)

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 a6d9a09..a3c31ba 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 class IteratorSetting implements Writable {
   }
 
   /**
-   * 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 586f2c7..e10cf80 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 class IteratorUtil {
         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 ddea0b3..2bde67a 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 class IteratorSettingTest {
 
     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 db30847..1077483 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.AgeOffFilter;
 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 class IteratorUtilTest {
     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());
+    }
+  }
+
 }

-- 
To stop receiving notification emails like this one, please contact
ctubbsii@apache.org.

Mime
View raw message