Consolidate iterator parsing code

core/src/main/java/org/apache/accumulo/core/clientImpl/IteratorSettingsUtil.java

@@ -0,0 +1,163 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.clientImpl;
+
+import static java.util.Objects.requireNonNull;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+
+import org.apache.accumulo.core.client.AccumuloException;
+import org.apache.accumulo.core.client.IteratorSetting;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
+
+final class IteratorSettingsUtil {
+
+  private static final String ITERATOR_PREFIX = Property.TABLE_ITERATOR_PREFIX.getKey();
+  private static final String OPT_PREFIX = ".opt.";
+
+  private IteratorSettingsUtil() {}
+
+  /**
+   * Validates iterator property keys for known scopes and expected formatting.
+   */
+  static void validateIteratorScopes(Map<String,String> properties) throws AccumuloException {
+    requireNonNull(properties, "properties is null");
+    for (String key : properties.keySet()) {
+      if (!key.startsWith(ITERATOR_PREFIX)) {
+        continue;
+      }
+      String suffix = key.substring(ITERATOR_PREFIX.length());
+      int dotIdx = suffix.indexOf('.');
+      if (dotIdx <= 0) {
+        throw new AccumuloException("Invalid iterator property: " + key);
+      }
+      String scopeStr = suffix.substring(0, dotIdx);
+      try {
+        IteratorScope.valueOf(scopeStr);
+      } catch (IllegalArgumentException e) {
+        throw new AccumuloException("Invalid iterator scope in property: " + key, e);
+      }
+    }
+  }
+
+  /**
+   * Parses iterator settings for a scope and validates scopes.
+   *
+   * @param properties properties to parse
+   * @param scope scope to validate for
+   */
+  static List<IteratorSetting> parseIteratorSettings(Map<String,String> properties,
+      IteratorScope scope) throws AccumuloException {
+    return parseIteratorSettings(properties, scope, true);
+  }
+
+  /**
+   * Parses iterator settings for a scope.
+   *
+   * @param properties properties to parse
+   * @param scope scope to validate for
+   * @param validateScopes whether to validate scopes or not
+   */
+  static List<IteratorSetting> parseIteratorSettings(Map<String,String> properties,
+      IteratorScope scope, boolean validateScopes) throws AccumuloException {
+    requireNonNull(properties, "properties is null");
+    requireNonNull(scope, "scope is null");
+    if (validateScopes) {
+      validateIteratorScopes(properties);
+    }
+
+    String scopePrefix = ITERATOR_PREFIX + scope.name().toLowerCase() + ".";
+    Map<String,ParsedIterator> parsed = new HashMap<>();
+
+    for (Entry<String,String> entry : properties.entrySet()) {
+      String key = entry.getKey();
+      if (!key.startsWith(scopePrefix)) {
+        continue;
+      }
+
+      String suffix = key.substring(scopePrefix.length());
+      if (suffix.isEmpty()) {
+        throw new AccumuloException("Invalid iterator format: " + key);
+      }
+
+      int optIndex = suffix.indexOf(OPT_PREFIX);
+      if (optIndex >= 0) {
+        String iterName = suffix.substring(0, optIndex);
+        String optName = suffix.substring(optIndex + OPT_PREFIX.length());
+        if (iterName.isEmpty() || optName.isEmpty() || iterName.indexOf('.') >= 0) {
+          throw new AccumuloException("Invalid iterator format: " + key);
+        }
+        parsed.computeIfAbsent(iterName, k -> new ParsedIterator()).options.put(optName,
+            entry.getValue());
+      } else {
+        if (suffix.indexOf('.') >= 0) {
+          throw new AccumuloException("Invalid iterator format: " + key);
+        }
+        ParsedIterator iter = parsed.computeIfAbsent(suffix, k -> new ParsedIterator());
+        parseIteratorValue(iter, key, entry.getValue());
+      }
+    }
+
+    List<IteratorSetting> settings = new ArrayList<>(parsed.size());
+    for (Entry<String,ParsedIterator> entry : parsed.entrySet()) {
+      ParsedIterator iter = entry.getValue();
+      if (iter.className == null) {
+        continue;
+      }
+      settings
+          .add(new IteratorSetting(iter.priority, entry.getKey(), iter.className, iter.options));
+    }
+    return settings;
+  }
+
+  private static void parseIteratorValue(ParsedIterator iter, String key, String value)
+      throws AccumuloException {
+    int firstComma = value.indexOf(',');
+    if (firstComma < 0 || firstComma != value.lastIndexOf(',')) {
+      throw new AccumuloException("Bad value for iterator setting: " + key + "=" + value);
+    }
+    String priorityString = value.substring(0, firstComma);
+    String className = value.substring(firstComma + 1);
+    if (className.isEmpty()) {
+      throw new AccumuloException("Bad value for iterator setting: " + key + "=" + value);
+    }
+    int priority;
+    try {
+      priority = Integer.parseInt(priorityString);
+    } catch (NumberFormatException e) {
+      throw new AccumuloException("Bad value for iterator setting: " + key + "=" + value, e);
+    }
+    if (priority <= 0) {
+      throw new AccumuloException("Bad value for iterator setting: " + key + "=" + value);
+    }
+    iter.priority = priority;
+    iter.className = className;
+  }
+
+  private static final class ParsedIterator {
+    int priority;
+    String className;
+    final Map<String,String> options = new HashMap<>();
+  }
+}

core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsHelper.java

@@ -19,7 +19,6 @@
 package org.apache.accumulo.core.clientImpl;
 
 import java.util.EnumSet;
-import java.util.HashMap;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.TreeMap;
@@ -91,29 +90,13 @@ public IteratorSetting getIteratorSetting(String namespace, String name, Iterato
     if (!exists(namespace)) {
       throw new NamespaceNotFoundException(null, namespace, null);
     }
-    int priority = -1;
-    String classname = null;
-    Map<String,String> settings = new HashMap<>();
-
-    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(namespace)) {
-      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());
+    Map<String,String> properties = Map.copyOf(this.getConfiguration(namespace));
+    for (IteratorSetting setting : IteratorSettingsUtil.parseIteratorSettings(properties, scope)) {
+      if (setting.getName().equals(name)) {
+        return setting;
       }
     }
-    if (priority <= 0 || classname == null) {
-      return null;
-    }
-    return new IteratorSetting(priority, name, classname, settings);
+    return null;
   }
 
   @Override
@@ -123,17 +106,13 @@ public Map<String,EnumSet<IteratorScope>> listIterators(String namespace)
       throw new NamespaceNotFoundException(null, namespace, null);
     }
     Map<String,EnumSet<IteratorScope>> result = new TreeMap<>();
-    for (Entry<String,String> property : this.getProperties(namespace)) {
-      String name = property.getKey();
-      String[] parts = name.split("\\.");
-      if (parts.length == 4) {
-        if (parts[0].equals("table") && parts[1].equals("iterator")) {
-          IteratorScope scope = IteratorScope.valueOf(parts[2]);
-          if (!result.containsKey(parts[3])) {
-            result.put(parts[3], EnumSet.noneOf(IteratorScope.class));
-          }
-          result.get(parts[3]).add(scope);
-        }
+    Map<String,String> properties = Map.copyOf(this.getConfiguration(namespace));
+    IteratorSettingsUtil.validateIteratorScopes(properties);
+    for (IteratorScope scope : IteratorScope.values()) {
+      for (IteratorSetting setting : IteratorSettingsUtil.parseIteratorSettings(properties, scope,
+          false)) {
+        result.computeIfAbsent(setting.getName(), k -> EnumSet.noneOf(IteratorScope.class))
+            .add(scope);
       }
     }
     return result;
@@ -146,38 +125,33 @@ public void checkIteratorConflicts(String namespace, IteratorSetting setting,
     if (!exists(namespace)) {
       throw new NamespaceNotFoundException(null, namespace, null);
     }
+    Map<String,String> props = Map.copyOf(this.getConfiguration(namespace));
+    IteratorSettingsUtil.validateIteratorScopes(props);
     for (IteratorScope scope : scopes) {
       String scopeStr =
           String.format("%s%s", Property.TABLE_ITERATOR_PREFIX, scope.name().toLowerCase());
       String nameStr = String.format("%s.%s", scopeStr, setting.getName());
       String optStr = String.format("%s.opt.", nameStr);
       Map<String,String> optionConflicts = new TreeMap<>();
-      for (Entry<String,String> property : this.getProperties(namespace)) {
-        if (property.getKey().startsWith(scopeStr)) {
-          if (property.getKey().equals(nameStr)) {
-            throw new AccumuloException(new IllegalArgumentException("iterator name conflict for "
-                + setting.getName() + ": " + property.getKey() + "=" + property.getValue()));
-          }
-          if (property.getKey().startsWith(optStr)) {
-            optionConflicts.put(property.getKey(), property.getValue());
-          }
-          if (property.getKey().contains(".opt.")) {
-            continue;
-          }
-          String[] parts = property.getValue().split(",");
-          if (parts.length != 2) {
-            throw new AccumuloException("Bad value for existing iterator setting: "
-                + property.getKey() + "=" + property.getValue());
-          }
-          try {
-            if (Integer.parseInt(parts[0]) == setting.getPriority()) {
-              throw new AccumuloException(new IllegalArgumentException(
-                  "iterator priority conflict: " + property.getKey() + "=" + property.getValue()));
-            }
-          } catch (NumberFormatException e) {
-            throw new AccumuloException("Bad value for existing iterator setting: "
-                + property.getKey() + "=" + property.getValue());
-          }
+
+      if (props.containsKey(nameStr)) {
+        throw new AccumuloException(new IllegalArgumentException("iterator name conflict for "
+            + setting.getName() + ": " + nameStr + "=" + props.get(nameStr)));
+      }
+
+      for (IteratorSetting existing : IteratorSettingsUtil.parseIteratorSettings(props, scope,
+          false)) {
+        if (existing.getPriority() == setting.getPriority()) {
+          String key = String.format("%s.%s", scopeStr, existing.getName());
+          String value = props.get(key);
+          throw new AccumuloException(
+              new IllegalArgumentException("iterator priority conflict: " + key + "=" + value));
+        }
+      }
+
+      for (Entry<String,String> property : props.entrySet()) {
+        if (property.getKey().startsWith(optStr)) {
+          optionConflicts.put(property.getKey(), property.getValue());
         }
       }
       if (!optionConflicts.isEmpty()) {