You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by bi...@apache.org on 2011/11/28 19:55:55 UTC

svn commit: r1207509 - in /incubator/accumulo/branches/1.4/src: core/src/main/java/org/apache/accumulo/core/client/ core/src/main/java/org/apache/accumulo/core/client/admin/ core/src/main/java/org/apache/accumulo/core/util/ core/src/main/java/org/apach...

Author: billie
Date: Mon Nov 28 18:55:53 2011
New Revision: 1207509

URL: http://svn.apache.org/viewvc?rev=1207509&view=rev
Log:
ACCUMULO-167 simplified IteratorSetting

Modified:
    incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/IteratorSetting.java
    incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java
    incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
    incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperationsHelper.java
    incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/ArgumentChecker.java
    incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteIterCommand.java
    incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/shell/commands/SetIterCommand.java
    incubator/accumulo/branches/1.4/src/core/src/test/java/org/apache/accumulo/core/client/admin/TableOperationsHelperTest.java
    incubator/accumulo/branches/1.4/src/server/src/main/java/org/apache/accumulo/server/test/functional/ServerSideErrorTest.java

Modified: incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/IteratorSetting.java
URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/IteratorSetting.java?rev=1207509&r1=1207508&r2=1207509&view=diff
==============================================================================
--- incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/IteratorSetting.java (original)
+++ incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/IteratorSetting.java Mon Nov 28 18:55:53 2011
@@ -16,8 +16,7 @@
  */
 package org.apache.accumulo.core.client;
 
-import java.util.Collections;
-import java.util.EnumMap;
+import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -25,30 +24,34 @@ import java.util.Set;
 
 import org.apache.accumulo.core.data.Key;
 import org.apache.accumulo.core.data.Value;
-import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
 import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
+import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
 import org.apache.accumulo.core.util.ArgumentChecker;
+import org.apache.accumulo.core.util.Pair;
+import org.apache.hadoop.io.Text;
 
 /**
- * Configure a scan-time iterator.
+ * Configure an iterator for minc, majc, and/or scan. By default, IteratorSetting will be configured for scan.
+ * 
+ * Every iterator has a priority, a name, a class, a set of scopes, and configuration parameters.
  * 
- * Every iterator has a priority, a name, a class and any number of named configuration parameters. The typical use case:
+ * A typical use case configured for scan:
  * 
  * <pre>
  * IteratorSetting cfg = new IteratorSetting(priority, &quot;myIter&quot;, MyIterator.class);
- * MyIterator.setOption(cfg, 42);
+ * MyIterator.addOption(cfg, 42);
  * scanner.addScanIterator(cfg);
  * </pre>
- * 
  */
 public class IteratorSetting {
   private int priority;
   private String name;
   private String iteratorClass;
-  private Map<IteratorScope,Map<String,String>> properties = new EnumMap<IteratorScope,Map<String,String>>(IteratorScope.class);
+  private EnumSet<IteratorScope> scopes;
+  private Map<String,String> properties;
   
   /**
-   * Set layer at which this iterator applies. See {@link #setPriority(int) for how the priority is used.}
+   * Get layer at which this iterator applies. See {@link #setPriority(int) for how the priority is used.}
    * 
    * @return the priority of this Iterator
    */
@@ -57,10 +60,14 @@ public class IteratorSetting {
   }
   
   /**
+   * Set layer at which this iterator applies.
+   * 
    * @param priority
-   *          determines the order in which iterators are applied (system iterators are always applied first, then per-table and scan-time, lowest first)
+   *          determines the order in which iterators are applied (system iterators are always applied first, then user-configured iterators, lowest priority
+   *          first)
    */
   public void setPriority(int priority) {
+    ArgumentChecker.strictlyPositive(priority);
     this.priority = priority;
   }
   
@@ -79,6 +86,7 @@ public class IteratorSetting {
    * @param name
    */
   public void setName(String name) {
+    ArgumentChecker.notNull(name);
     this.name = name;
   }
   
@@ -98,87 +106,162 @@ public class IteratorSetting {
    * @param iteratorClass
    */
   public void setIteratorClass(String iteratorClass) {
+    ArgumentChecker.notNull(iteratorClass);
     this.iteratorClass = iteratorClass;
   }
   
   /**
-   * Get any properties set on the iterator. Note that this will be a merged view of the settings for all scopes.
+   * Get the scopes under which this iterator will be configured.
+   * 
+   * @return the scopes
+   */
+  public EnumSet<IteratorScope> getScopes() {
+    return scopes;
+  }
+  
+  /**
+   * Set the scopes under which this iterator will be configured.
    * 
-   * @return a read-only copy of the named options (for all scopes)
+   * @param scopes
+   *          the scopes to set
+   */
+  public void setScopes(EnumSet<IteratorScope> scopes) {
+    ArgumentChecker.notNull(scopes);
+    if (scopes.isEmpty())
+      throw new IllegalArgumentException("empty scopes");
+    this.scopes = scopes;
+  }
+  
+  /**
+   * Get the configuration parameters for this iterator.
+   * 
+   * @return the properties
    */
   public Map<String,String> getProperties() {
-    Map<String,String> result = new HashMap<String,String>();
-    for (Map<String,String> props : properties.values()) {
-      result.putAll(props);
-    }
-    return result;
+    return properties;
+  }
+  
+  /**
+   * Set the configuration parameters for this iterator.
+   * 
+   * @param properties
+   *          the properties to set
+   */
+  public void setProperties(Map<String,String> properties) {
+    this.properties.clear();
+    addOptions(properties);
+  }
+
+  /**
+   * @return <tt>true</tt> if this iterator has configuration parameters.
+   */
+  public boolean hasProperties() {
+    return !properties.isEmpty();
   }
   
   /**
-   * Construct a basic iterator setting with all the required values.
+   * Constructs an iterator setting configured for the scan scope with no parameters. (Parameters can be added later.)
    * 
    * @param priority
    *          the priority for the iterator @see {@link #setPriority(int)}
    * @param name
-   *          the name for the iterator
+   *          the distinguishing name for the iterator
    * @param iteratorClass
-   *          the full class name for the iterator
+   *          the fully qualified class name for the iterator
    */
   public IteratorSetting(int priority, String name, String iteratorClass) {
-    ArgumentChecker.notNull(name, iteratorClass);
-    this.priority = priority;
-    this.name = name;
-    this.iteratorClass = iteratorClass;
+    this(priority, name, iteratorClass, EnumSet.of(IteratorScope.scan), new HashMap<String,String>());
   }
   
   /**
-   * Construct a basic iterator setting.
+   * Constructs an iterator setting configured for the specified scopes with the specified parameters.
    * 
    * @param priority
    *          the priority for the iterator @see {@link #setPriority(int)}
    * @param name
-   *          the name for the iterator
+   *          the distinguishing name for the iterator
    * @param iteratorClass
-   *          the full class name for the iterator
-   * @param scope
-   *          the scope of the iterator
+   *          the fully qualified class name for the iterator
+   * @param scopes
+   *          the scopes of the iterator
    * @param properties
    *          any properties for the iterator
    */
-  public IteratorSetting(int priority, String name, String iteratorClass, IteratorScope scope, Map<String,String> properties) {
-    ArgumentChecker.notNull(name, iteratorClass, scope);
-    this.priority = priority;
-    this.name = name;
-    this.iteratorClass = iteratorClass;
-    addOptions(scope, properties);
+  public IteratorSetting(int priority, String name, String iteratorClass, EnumSet<IteratorScope> scopes, Map<String,String> properties) {
+    setPriority(priority);
+    setName(name);
+    setIteratorClass(iteratorClass);
+    setScopes(scopes);
+    this.properties = new HashMap<String,String>();
+    setProperties(properties);
   }
   
   /**
+   * Constructs an iterator setting using the given class's SimpleName for the iterator name. The iterator setting will be configured for the scan scope with no
+   * parameters.
+   * 
+   * @param priority
+   *          the priority for the iterator @see {@link #setPriority(int)}
+   * @param iteratorClass
+   *          the class for the iterator
+   */
+  public IteratorSetting(int priority, Class<? extends SortedKeyValueIterator<Key,Value>> iteratorClass) {
+    this(priority, iteratorClass.getSimpleName(), iteratorClass.getName());
+  }
+  
+  /**
+   * Constructs an iterator setting using the given class's SimpleName for the iterator name and configured for the specified scopes with the specified
+   * parameters.
+   * 
+   * @param priority
+   *          the priority for the iterator @see {@link #setPriority(int)}
+   * @param iteratorClass
+   *          the class for the iterator
+   * @param scopes
+   *          the scopes of the iterator
+   * @param properties
+   *          any properties for the iterator
+   */
+  public IteratorSetting(int priority, Class<? extends SortedKeyValueIterator<Key,Value>> iteratorClass, EnumSet<IteratorScope> scopes,
+      Map<String,String> properties) {
+    this(priority, iteratorClass.getSimpleName(), iteratorClass.getName(), scopes, properties);
+  }
+  
+  /**
+   * Constructs an iterator setting configured for the scan scope with no parameters.
+   * 
    * @param priority
    *          the priority for the iterator @see {@link #setPriority(int)}
    * @param name
-   *          the name for the iterator
+   *          the distinguishing name for the iterator
    * @param iteratorClass
-   *          the class to use for the class name
+   *          the class for the iterator
    */
   public IteratorSetting(int priority, String name, Class<? extends SortedKeyValueIterator<Key,Value>> iteratorClass) {
     this(priority, name, iteratorClass.getName());
   }
   
   /**
-   * Constructs an ScanIterator using the given class's SimpleName for the iterator name
+   * Constructs an iterator setting configured for the specified scopes with the specified parameters.
    * 
    * @param priority
-   *          the priority for the iterator
+   *          the priority for the iterator @see {@link #setPriority(int)}
+   * @param name
+   *          the distinguishing name for the iterator
    * @param iteratorClass
-   *          the class to use for the iterator
+   *          the class for the iterator
+   * @param scopes
+   *          the scopes of the iterator
+   * @param properties
+   *          any properties for the iterator
    */
-  public IteratorSetting(int priority, Class<? extends SortedKeyValueIterator<Key,Value>> iteratorClass) {
-    this(priority, iteratorClass.getSimpleName(), iteratorClass.getName());
+  public IteratorSetting(int priority, String name, Class<? extends SortedKeyValueIterator<Key,Value>> iteratorClass, EnumSet<IteratorScope> scopes,
+      Map<String,String> properties) {
+    this(priority, name, iteratorClass.getName(), scopes, properties);
   }
   
   /**
-   * Add another option to the iterator
+   * Add another option to the iterator.
    * 
    * @param option
    *          the name of the option
@@ -187,61 +270,91 @@ public class IteratorSetting {
    */
   public void addOption(String option, String value) {
     ArgumentChecker.notNull(option, value);
-    for (IteratorScope scope : IteratorScope.values()) {
-      addOptions(scope, Collections.singletonMap(option, value));
-    }
+    properties.put(option, value);
   }
   
   /**
-   * Add many options to the iterator
+   * Remove an option from the iterator.
    * 
-   * @param keyValues
-   *          the values to add to the options
+   * @param option
+   *          the name of the option
+   * @return the value previously associated with the option, or null if no such option existed
    */
-  public void addOptions(Set<Entry<String,String>> keyValues) {
-    for (Entry<String,String> keyValue : keyValues) {
-      addOption(keyValue.getKey(), keyValue.getValue());
-    }
+  public String removeOption(String option) {
+    ArgumentChecker.notNull(option);
+    return properties.remove(option);
   }
   
   /**
-   * Get the properties on the iterator by scope
+   * Add many options to the iterator.
    * 
-   * @return a mapping from scope to key/value settings
+   * @param propertyEntries
+   *          a set of entries to add to the options
    */
-  public Map<IteratorScope,Map<String,String>> getOptionsByScope() {
-    Map<IteratorScope,Map<String,String>> result = new EnumMap<IteratorScope,Map<String,String>>(IteratorScope.class);
-    for (Entry<IteratorScope,Map<String,String>> property : properties.entrySet()) {
-      result.put(property.getKey(), Collections.unmodifiableMap(property.getValue()));
+  public void addOptions(Set<Entry<String,String>> propertyEntries) {
+    ArgumentChecker.notNull(propertyEntries);
+    for (Entry<String,String> keyValue : propertyEntries) {
+      addOption(keyValue.getKey(), keyValue.getValue());
     }
-    return Collections.unmodifiableMap(result);
   }
   
   /**
-   * Add many options to to the iterator for a scope
+   * Add many options to the iterator.
    * 
-   * @param scope
-   *          the scope to put the properties
    * @param properties
-   *          key/value pairs
+   *          a map of entries to add to the options
    */
-  public void addOptions(IteratorScope scope, Map<String,String> properties) {
-    if (properties == null)
-      return;
-    if (!this.properties.containsKey(scope))
-      this.properties.put(scope, new HashMap<String,String>());
-    for (Entry<String,String> entry : properties.entrySet()) {
-      this.properties.get(scope).put(entry.getKey(), entry.getValue());
-    }
+  public void addOptions(Map<String,String> properties) {
+    ArgumentChecker.notNull(properties);
+    addOptions(properties.entrySet());
   }
   
   /**
-   * Delete the properties per-scope
-   * 
-   * @param scope
-   *          the scope to delete
+   * Remove all options from the iterator.
    */
-  public void deleteOptions(IteratorScope scope) {
-    this.properties.remove(scope);
+  public void clearOptions() {
+    properties.clear();
+  }
+  
+  /**
+   * @see java.lang.Object#toString()
+   */
+  @Override
+  public String toString() {
+    StringBuilder sb = new StringBuilder();
+    sb.append("name:");
+    sb.append(name);
+    sb.append(", priority:");
+    sb.append(Integer.toString(priority));
+    sb.append(", class:");
+    sb.append(iteratorClass);
+    sb.append(", scopes:");
+    sb.append(scopes);
+    sb.append(", properties:");
+    sb.append(properties);
+    return sb.toString();
+  }
+
+  /**
+   * A convenience class for passing column family and column qualifiers to iterator configuration methods.
+   */
+  public static class Column extends Pair<Text,Text> {
+    
+    public Column(Text columnFamily, Text columnQualifier) {
+      super(columnFamily, columnQualifier);
+    }
+    
+    public Column(Text columnFamily) {
+      super(columnFamily, null);
+    }
+    
+    public Column(String columnFamily, String columnQualifier) {
+      super(new Text(columnFamily), new Text(columnQualifier));
+    }
+    
+    public Column(String columnFamily) {
+      super(new Text(columnFamily), null);
+    }
+    
   }
 }

Modified: incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java
URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java?rev=1207509&r1=1207508&r2=1207509&view=diff
==============================================================================
--- incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java (original)
+++ incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java Mon Nov 28 18:55:53 2011
@@ -63,15 +63,13 @@ public interface ScannerBase extends Ite
   
   /**
    * @deprecated since 1.4
-   * @see {@link org.apache.accumulo.core.client.ScannerBase#addScanIterator(int, IteratorSetting)}
-   * @see {@link org.apache.accumulo.core.iterators.user.RegExFilter}
+   * @see {@link org.apache.accumulo.core.client.ScannerBase#addScanIterator(IteratorSetting)}
    */
   public void setScanIterators(int priority, String iteratorClass, String iteratorName);
   
   /**
    * @deprecated since 1.4
-   * @see {@link org.apache.accumulo.core.client.ScannerBase#addScanIterator(int, IteratorSetting)}
-   * @see {@link org.apache.accumulo.core.iterators.user.RegExFilter}
+   * @see {@link org.apache.accumulo.core.client.ScannerBase#addScanIterator(IteratorSetting)}
    */
   public void setScanIteratorOption(String iteratorName, String key, String value);
   
@@ -86,7 +84,7 @@ public interface ScannerBase extends Ite
    *           if an exception occurs reading from the iterator stack
    * 
    * @deprecated since 1.4
-   * @see {@link org.apache.accumulo.core.client.ScannerBase#addScanIterator(int, IteratorSetting)}
+   * @see {@link org.apache.accumulo.core.client.ScannerBase#addScanIterator(IteratorSetting)}
    * @see {@link org.apache.accumulo.core.iterators.user.RegExFilter}
    */
   public void setupRegex(String iteratorName, int iteratorPriority) throws IOException;
@@ -99,7 +97,7 @@ public interface ScannerBase extends Ite
    *          java regular expression to match
    * 
    * @deprecated since 1.4
-   * @see {@link org.apache.accumulo.core.client.ScannerBase#addScanIterator(int, IteratorSetting)}
+   * @see {@link org.apache.accumulo.core.client.ScannerBase#addScanIterator(IteratorSetting)}
    * @see {@link org.apache.accumulo.core.iterators.user.RegExFilter#ROW_REGEX}
    * 
    *      <pre>
@@ -119,7 +117,7 @@ public interface ScannerBase extends Ite
    *          java regular expression to match
    * 
    * @deprecated since 1.4
-   * @see {@link org.apache.accumulo.core.client.ScannerBase#addScanIterator(int, IteratorSetting)}
+   * @see {@link org.apache.accumulo.core.client.ScannerBase#addScanIterator(IteratorSetting)}
    * @see {@link org.apache.accumulo.core.iterators.user.RegExFilter#COLF_REGEX}
    */
   public void setColumnFamilyRegex(String regex);
@@ -133,7 +131,7 @@ public interface ScannerBase extends Ite
    *          java regular expression to match
    * 
    * @deprecated since 1.4
-   * @see {@link org.apache.accumulo.core.client.ScannerBase#addScanIterator(int, IteratorSetting)}
+   * @see {@link org.apache.accumulo.core.client.ScannerBase#addScanIterator(IteratorSetting)}
    * @see {@link org.apache.accumulo.core.iterators.user.RegExFilter#COLQ_REGEX}.
    * 
    */
@@ -146,7 +144,7 @@ public interface ScannerBase extends Ite
    *          java regular expression to match
    * 
    * @deprecated since 1.4
-   * @see {@link org.apache.accumulo.core.client.ScannerBase#addScanIterator(int, IteratorSetting)}
+   * @see {@link org.apache.accumulo.core.client.ScannerBase#addScanIterator(IteratorSetting)}
    * @see {@link org.apache.accumulo.core.iterators.user.RegExFilter}
    */
   public void setValueRegex(String regex);

Modified: incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java?rev=1207509&r1=1207508&r2=1207509&view=diff
==============================================================================
--- incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java (original)
+++ incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java Mon Nov 28 18:55:53 2011
@@ -18,6 +18,7 @@ package org.apache.accumulo.core.client.
 
 import java.io.IOException;
 import java.util.Collection;
+import java.util.EnumSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -30,6 +31,7 @@ import org.apache.accumulo.core.client.I
 import org.apache.accumulo.core.client.TableExistsException;
 import org.apache.accumulo.core.client.TableNotFoundException;
 import org.apache.accumulo.core.data.Range;
+import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
 import org.apache.accumulo.core.iterators.conf.PerColumnIteratorConfig;
 import org.apache.accumulo.core.security.Authorizations;
 import org.apache.accumulo.core.util.BulkImportHelper.AssignmentStats;
@@ -479,21 +481,26 @@ public interface TableOperations {
    *          the name of the table
    * @param name
    *          the name of the iterator
+   * @param scopes
+   *          the scopes of the iterator
    * @throws AccumuloSecurityException
    *           thrown if the user does not have the ability to set properties on the table
    * @throws AccumuloException
    * @throws TableNotFoundException
    *           throw if the table no longer exists
    */
-  public void removeIterator(String tableName, String name) throws AccumuloSecurityException, AccumuloException, TableNotFoundException;
+  public void removeIterator(String tableName, String name, EnumSet<IteratorScope> scopes) throws AccumuloSecurityException, AccumuloException,
+      TableNotFoundException;
   
   /**
-   * Get the settings for an iterator
+   * Get the settings for an iterator.
    * 
    * @param tableName
    *          the name of the table
    * @param name
    *          the name of the iterator
+   * @param scope
+   *          the scope of the iterator
    * @return the settings for this iterator
    * @throws AccumuloSecurityException
    *           thrown if the user does not have the ability to set properties on the table
@@ -501,7 +508,8 @@ public interface TableOperations {
    * @throws TableNotFoundException
    *           throw if the table no longer exists
    */
-  public IteratorSetting getIterator(String tableName, String name) throws AccumuloSecurityException, AccumuloException, TableNotFoundException;
+  public IteratorSetting getIteratorSetting(String tableName, String name, IteratorScope scope) throws AccumuloSecurityException, AccumuloException,
+      TableNotFoundException;
   
   /**
    * Get a list of iterators for this table.
@@ -513,5 +521,5 @@ public interface TableOperations {
    * @throws AccumuloException
    * @throws TableNotFoundException
    */
-  public Set<String> getIterators(String tableName) throws AccumuloSecurityException, AccumuloException, TableNotFoundException;
+  public Set<String> listIterators(String tableName) throws AccumuloSecurityException, AccumuloException, TableNotFoundException;
 }

Modified: incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperationsHelper.java
URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperationsHelper.java?rev=1207509&r1=1207508&r2=1207509&view=diff
==============================================================================
--- incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperationsHelper.java (original)
+++ incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperationsHelper.java Mon Nov 28 18:55:53 2011
@@ -16,7 +16,7 @@
  */
 package org.apache.accumulo.core.client.admin;
 
-import java.util.EnumMap;
+import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
@@ -34,25 +34,26 @@ public abstract class TableOperationsHel
   
   @Override
   public void attachIterator(String tableName, IteratorSetting setting) throws AccumuloSecurityException, AccumuloException, TableNotFoundException {
-    removeIterator(tableName, setting.getName());
-    for (Entry<IteratorScope,Map<String,String>> entry : setting.getOptionsByScope().entrySet()) {
-      String root = String.format("%s%s.%s", Property.TABLE_ITERATOR_PREFIX, entry.getKey().name().toLowerCase(), setting.getName());
+    removeIterator(tableName, setting.getName(), setting.getScopes());
+    for (IteratorScope scope : setting.getScopes()) {
+      String root = String.format("%s%s.%s", Property.TABLE_ITERATOR_PREFIX, scope.name().toLowerCase(), setting.getName());
       this.setProperty(tableName, root, setting.getPriority() + "," + setting.getIteratorClass());
-      for (Entry<String,String> prop : entry.getValue().entrySet()) {
+      for (Entry<String,String> prop : setting.getProperties().entrySet()) {
         this.setProperty(tableName, root + ".opt." + prop.getKey(), prop.getValue());
       }
     }
   }
   
   @Override
-  public void removeIterator(String tableName, String name) throws AccumuloSecurityException, AccumuloException, TableNotFoundException {
+  public void removeIterator(String tableName, String name, EnumSet<IteratorScope> scopes) throws AccumuloSecurityException, AccumuloException,
+      TableNotFoundException {
     Map<String,String> copy = new HashMap<String,String>();
     for (Entry<String,String> property : this.getProperties(tableName)) {
       copy.put(property.getKey(), property.getValue());
     }
-    for (Entry<String,String> property : copy.entrySet()) {
-      for (IteratorScope scope : IteratorScope.values()) {
-        String root = String.format("%s%s.%s", Property.TABLE_ITERATOR_PREFIX, scope.name().toLowerCase(), name);
+    for (IteratorScope scope : scopes) {
+      String root = String.format("%s%s.%s", Property.TABLE_ITERATOR_PREFIX, scope.name().toLowerCase(), name);
+      for (Entry<String,String> property : copy.entrySet()) {
         if (property.getKey().equals(root) || property.getKey().startsWith(root + ".opt."))
           this.removeProperty(tableName, property.getKey());
       }
@@ -60,43 +61,34 @@ public abstract class TableOperationsHel
   }
   
   @Override
-  public IteratorSetting getIterator(String tableName, String name) throws AccumuloSecurityException, AccumuloException, TableNotFoundException {
+  public IteratorSetting getIteratorSetting(String tableName, String name, IteratorScope scope) throws AccumuloSecurityException, AccumuloException,
+      TableNotFoundException {
     int priority = -1;
     String classname = null;
-    EnumMap<IteratorScope,Map<String,String>> settings = new EnumMap<IteratorScope,Map<String,String>>(IteratorScope.class);
+    Map<String,String> settings = new HashMap<String,String>();
     
+    String root = String.format("%s%s.%s", Property.TABLE_ITERATOR_PREFIX, scope.name().toLowerCase(), name);
+    String opt = root + ".opt.";
     for (Entry<String,String> property : this.getProperties(tableName)) {
-      for (IteratorScope scope : IteratorScope.values()) {
-        String root = String.format("%s%s.%s", Property.TABLE_ITERATOR_PREFIX, scope.name().toLowerCase(), name);
-        String opt = root + ".opt.";
-        if (property.getKey().equals(root)) {
-          String parts[] = property.getValue().split(",");
-          if (parts.length != 2) {
-            throw new AccumuloException("Bad value for iterator setting: " + property.getValue());
-          }
-          priority = Integer.parseInt(parts[0]);
-          classname = parts[1];
-          if (!settings.containsKey(scope))
-            settings.put(scope, new HashMap<String,String>());
-        } else if (property.getKey().startsWith(opt)) {
-          if (!settings.containsKey(scope))
-            settings.put(scope, new HashMap<String,String>());
-          settings.get(scope).put(property.getKey().substring(opt.length()), property.getValue());
+      if (property.getKey().equals(root)) {
+        String parts[] = property.getValue().split(",");
+        if (parts.length != 2) {
+          throw new AccumuloException("Bad value for iterator setting: " + property.getValue());
         }
+        priority = Integer.parseInt(parts[0]);
+        classname = parts[1];
+      } else if (property.getKey().startsWith(opt)) {
+        settings.put(property.getKey().substring(opt.length()), property.getValue());
       }
     }
-    if (priority < 0 || classname == null) {
+    if (priority <= 0 || classname == null) {
       return null;
     }
-    IteratorSetting result = new IteratorSetting(priority, name, classname);
-    for (Entry<IteratorScope,Map<String,String>> entry : settings.entrySet()) {
-      result.addOptions(entry.getKey(), entry.getValue());
-    }
-    return result;
+    return new IteratorSetting(priority, name, classname, EnumSet.of(scope), settings);
   }
   
   @Override
-  public Set<String> getIterators(String tableName) throws AccumuloSecurityException, AccumuloException, TableNotFoundException {
+  public Set<String> listIterators(String tableName) throws AccumuloSecurityException, AccumuloException, TableNotFoundException {
     Set<String> result = new HashSet<String>();
     Set<String> lifecycles = new HashSet<String>();
     for (IteratorScope scope : IteratorScope.values())

Modified: incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/ArgumentChecker.java
URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/ArgumentChecker.java?rev=1207509&r1=1207508&r2=1207509&view=diff
==============================================================================
--- incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/ArgumentChecker.java (original)
+++ incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/ArgumentChecker.java Mon Nov 28 18:55:53 2011
@@ -55,4 +55,9 @@ public class ArgumentChecker {
       if (args[i] == null)
         throw new IllegalArgumentException(NULL_ARG_MSG + ":arg" + i + " is null");
   }
+  
+  public static final void strictlyPositive(final int i) {
+    if (i <= 0)
+      throw new IllegalArgumentException("integer should be > 0, was " + i);
+  }
 }

Modified: incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteIterCommand.java
URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteIterCommand.java?rev=1207509&r1=1207508&r2=1207509&view=diff
==============================================================================
--- incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteIterCommand.java (original)
+++ incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteIterCommand.java Mon Nov 28 18:55:53 2011
@@ -16,9 +16,10 @@
  */
 package org.apache.accumulo.core.util.shell.commands;
 
+import java.util.EnumSet;
+
 import org.apache.accumulo.core.client.AccumuloException;
 import org.apache.accumulo.core.client.AccumuloSecurityException;
-import org.apache.accumulo.core.client.IteratorSetting;
 import org.apache.accumulo.core.client.TableNotFoundException;
 import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
 import org.apache.accumulo.core.util.shell.Shell;
@@ -45,29 +46,19 @@ public class DeleteIterCommand extends C
     }
     
     String name = cl.getOptionValue(nameOpt.getOpt());
-    if (!shellState.getConnector().tableOperations().getIterators(tableName).contains(name)) {
+    if (!shellState.getConnector().tableOperations().listIterators(tableName).contains(name)) {
       Shell.log.warn("no iterators found that match your criteria");
       return 0;
     }
-    IteratorSetting iterator = shellState.getConnector().tableOperations().getIterator(tableName, name);
-    if (iterator == null)
-      return 0;
-    boolean deleteAll = true;
-    if (cl.hasOption(mincScopeOpt.getOpt())) {
-      iterator.deleteOptions(IteratorScope.minc);
-      deleteAll = false;
-    }
-    if (cl.hasOption(majcScopeOpt.getOpt())) {
-      iterator.deleteOptions(IteratorScope.majc);
-      deleteAll = false;
-    }
-    if (cl.hasOption(scanScopeOpt.getOpt())) {
-      iterator.deleteOptions(IteratorScope.scan);
-      deleteAll = false;
-    }
-    shellState.getConnector().tableOperations().removeIterator(tableName, name);
-    if (!iterator.getProperties().isEmpty() && !deleteAll)
-      shellState.getConnector().tableOperations().attachIterator(tableName, iterator);
+    
+    EnumSet<IteratorScope> scopes = EnumSet.noneOf(IteratorScope.class);
+    if (cl.hasOption(mincScopeOpt.getOpt()))
+      scopes.add(IteratorScope.minc);
+    if (cl.hasOption(majcScopeOpt.getOpt()))
+      scopes.add(IteratorScope.majc);
+    if (cl.hasOption(scanScopeOpt.getOpt()))
+      scopes.add(IteratorScope.scan);
+    shellState.getConnector().tableOperations().removeIterator(tableName, name, scopes);
     return 0;
   }
   

Modified: incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/shell/commands/SetIterCommand.java
URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/shell/commands/SetIterCommand.java?rev=1207509&r1=1207508&r2=1207509&view=diff
==============================================================================
--- incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/shell/commands/SetIterCommand.java (original)
+++ incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/shell/commands/SetIterCommand.java Mon Nov 28 18:55:53 2011
@@ -17,6 +17,7 @@
 package org.apache.accumulo.core.util.shell.commands;
 
 import java.io.IOException;
+import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
@@ -103,21 +104,23 @@ public class SetIterCommand extends Comm
   
   protected void setTableProperties(CommandLine cl, Shell shellState, String tableName, int priority, Map<String,String> options, String classname, String name)
       throws AccumuloException, AccumuloSecurityException, ShellCommandException, TableNotFoundException {
-    IteratorSetting setting = new IteratorSetting(priority, name, classname);
     // remove empty values
     for (Iterator<Entry<String,String>> i = options.entrySet().iterator(); i.hasNext();) {
       Entry<String,String> entry = i.next();
       if (entry.getValue() == null || entry.getValue().isEmpty())
         i.remove();
     }
+    EnumSet<IteratorScope> scopes = EnumSet.noneOf(IteratorScope.class);
     if (cl.hasOption(mincScopeOpt.getOpt()))
-      setting.addOptions(IteratorScope.minc, options);
+      scopes.add(IteratorScope.minc);
     if (cl.hasOption(majcScopeOpt.getOpt()))
-      setting.addOptions(IteratorScope.majc, options);
+      scopes.add(IteratorScope.majc);
     if (cl.hasOption(scanScopeOpt.getOpt()))
-      setting.addOptions(IteratorScope.scan, options);
-    if (setting.getProperties().isEmpty())
+      scopes.add(IteratorScope.scan);
+    if (scopes.isEmpty())
       throw new IllegalArgumentException("You must select at least one scope to configure");
+    
+    IteratorSetting setting = new IteratorSetting(priority, name, classname, scopes, options);
     shellState.getConnector().tableOperations().attachIterator(tableName, setting);
   }
   

Modified: incubator/accumulo/branches/1.4/src/core/src/test/java/org/apache/accumulo/core/client/admin/TableOperationsHelperTest.java
URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/core/src/test/java/org/apache/accumulo/core/client/admin/TableOperationsHelperTest.java?rev=1207509&r1=1207508&r2=1207509&view=diff
==============================================================================
--- incubator/accumulo/branches/1.4/src/core/src/test/java/org/apache/accumulo/core/client/admin/TableOperationsHelperTest.java (original)
+++ incubator/accumulo/branches/1.4/src/core/src/test/java/org/apache/accumulo/core/client/admin/TableOperationsHelperTest.java Mon Nov 28 18:55:53 2011
@@ -19,6 +19,7 @@ package org.apache.accumulo.core.client.
 import java.io.IOException;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -199,36 +200,47 @@ public class TableOperationsHelperTest {
   public void testAttachIterator() throws Exception {
     Tester t = new Tester();
     Map<String,String> empty = Collections.emptyMap();
-    t.attachIterator("table", new IteratorSetting(10, "someName", "foo.bar", IteratorScope.scan, empty));
+    t.attachIterator("table", new IteratorSetting(10, "someName", "foo.bar", EnumSet.of(IteratorScope.scan), empty));
     t.check("table", new String[] {"table.iterator.scan.someName=10,foo.bar",});
-    t.removeIterator("table", "someName");
+    t.removeIterator("table", "someName", EnumSet.of(IteratorScope.scan));
     t.check("table", new String[] {});
+
     IteratorSetting setting = new IteratorSetting(10, "someName", "foo.bar");
-    setting.addOptions(IteratorScope.majc, Collections.singletonMap("key", "value"));
-    setting.addOptions(IteratorScope.scan, empty);
+    setting.setScopes(EnumSet.of(IteratorScope.majc));
+    setting.addOptions(Collections.singletonMap("key", "value"));
+    t.attachIterator("table", setting);
+    setting = new IteratorSetting(10, "someName", "foo.bar");
     t.attachIterator("table", setting);
     t.check("table", new String[] {"table.iterator.majc.someName=10,foo.bar", "table.iterator.majc.someName.opt.key=value",
         "table.iterator.scan.someName=10,foo.bar",});
+
+    setting = new IteratorSetting(20, "otherName", "some.classname");
+    setting.setScopes(EnumSet.of(IteratorScope.majc));
+    setting.addOptions(Collections.singletonMap("key", "value"));
+    t.attachIterator("table", setting);
     setting = new IteratorSetting(20, "otherName", "some.classname");
-    setting.addOptions(IteratorScope.majc, Collections.singletonMap("key", "value"));
-    setting.addOptions(IteratorScope.scan, empty);
     t.attachIterator("table", setting);
-    Set<String> two = t.getIterators("table");
+    Set<String> two = t.listIterators("table");
     Assert.assertEquals(2, two.size());
     Assert.assertTrue(two.contains("otherName"));
     Assert.assertTrue(two.contains("someName"));
-    t.removeIterator("table", "someName");
+    t.removeIterator("table", "someName", EnumSet.allOf(IteratorScope.class));
     t.check("table", new String[] {"table.iterator.majc.otherName=20,some.classname", "table.iterator.majc.otherName.opt.key=value",
         "table.iterator.scan.otherName=20,some.classname",});
-    setting = t.getIterator("table", "otherName");
+
+    setting = t.getIteratorSetting("table", "otherName", IteratorScope.scan);
+    Assert.assertEquals(20, setting.getPriority());
+    Assert.assertEquals("some.classname", setting.getIteratorClass());
+    Assert.assertFalse(setting.hasProperties());
+    setting = t.getIteratorSetting("table", "otherName", IteratorScope.majc);
     Assert.assertEquals(20, setting.getPriority());
     Assert.assertEquals("some.classname", setting.getIteratorClass());
-    Assert.assertEquals(0, setting.getOptionsByScope().get(IteratorScope.scan).size());
-    Assert.assertEquals(Collections.singletonMap("key", "value"), setting.getOptionsByScope().get(IteratorScope.majc));
-    setting.addOptions(IteratorScope.minc, Collections.singletonMap("a", "b"));
-    t.attachIterator("foo", setting);
-    t.check("foo", new String[] {"table.iterator.majc.otherName=20,some.classname", "table.iterator.majc.otherName.opt.key=value",
-        "table.iterator.minc.otherName=20,some.classname", "table.iterator.minc.otherName.opt.a=b", "table.iterator.scan.otherName=20,some.classname",});
+    Assert.assertTrue(setting.hasProperties());
+    Assert.assertEquals(Collections.singletonMap("key", "value"), setting.getProperties());
+    setting.setScopes(EnumSet.of(IteratorScope.minc));
+    t.attachIterator("table", setting);
+    t.check("table", new String[] {"table.iterator.majc.otherName=20,some.classname", "table.iterator.majc.otherName.opt.key=value",
+        "table.iterator.minc.otherName=20,some.classname", "table.iterator.minc.otherName.opt.key=value", "table.iterator.scan.otherName=20,some.classname",});
   }
   
 }

Modified: incubator/accumulo/branches/1.4/src/server/src/main/java/org/apache/accumulo/server/test/functional/ServerSideErrorTest.java
URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/server/src/main/java/org/apache/accumulo/server/test/functional/ServerSideErrorTest.java?rev=1207509&r1=1207508&r2=1207509&view=diff
==============================================================================
--- incubator/accumulo/branches/1.4/src/server/src/main/java/org/apache/accumulo/server/test/functional/ServerSideErrorTest.java (original)
+++ incubator/accumulo/branches/1.4/src/server/src/main/java/org/apache/accumulo/server/test/functional/ServerSideErrorTest.java Mon Nov 28 18:55:53 2011
@@ -58,7 +58,7 @@ public class ServerSideErrorTest extends
     
     getConnector().tableOperations().create("tt");
     IteratorSetting is = new IteratorSetting(5, "Bad Aggregator", BadCombiner.class);
-    Combiner.setColumns(is, Collections.singletonList(new Combiner.Column("acf")));
+    Combiner.setColumns(is, Collections.singletonList(new IteratorSetting.Column("acf")));
     getConnector().tableOperations().attachIterator("tt", is);
     
     BatchWriter bw = getConnector().createBatchWriter("tt", 1000000, 60000l, 2);