kevinrr888 commented on code in PR #6040:
URL: https://github.com/apache/accumulo/pull/6040#discussion_r2687772253


##########
test/src/main/java/org/apache/accumulo/test/functional/IteratorConflictsIT.java:
##########
@@ -0,0 +1,622 @@
+/*
+ * 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.test.functional;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Consumer;
+
+import org.apache.accumulo.core.client.Accumulo;
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.client.AccumuloException;
+import org.apache.accumulo.core.client.IteratorSetting;
+import org.apache.accumulo.core.client.Scanner;
+import org.apache.accumulo.core.client.admin.CloneConfiguration;
+import org.apache.accumulo.core.client.admin.NamespaceOperations;
+import org.apache.accumulo.core.client.admin.NewTableConfiguration;
+import org.apache.accumulo.core.client.admin.TableOperations;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.iterators.IteratorUtil;
+import org.apache.accumulo.core.iteratorsImpl.IteratorConfigUtil;
+import org.apache.accumulo.harness.SharedMiniClusterBase;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.function.Executable;
+
+/**
+ * Tests that iterator conflicts are detected and cause exceptions. Iterators 
can be added multiple
+ * ways. This test ensures that:
+ * <p>
+ * - {@link Scanner#addScanIterator(IteratorSetting)}
+ * <p>
+ * - {@link TableOperations#setProperty(String, String, String)}
+ * <p>
+ * - {@link TableOperations#modifyProperties(String, Consumer)}
+ * <p>
+ * - {@link NewTableConfiguration#attachIterator(IteratorSetting, EnumSet)}
+ * <p>
+ * - {@link TableOperations#attachIterator(String, IteratorSetting, EnumSet)}
+ * <p>
+ * - {@link NamespaceOperations#attachIterator(String, IteratorSetting, 
EnumSet)}
+ * <p>
+ * - {@link NamespaceOperations#setProperty(String, String, String)}
+ * <p>
+ * - {@link NamespaceOperations#modifyProperties(String, Consumer)}
+ * <p>
+ * - {@link CloneConfiguration.Builder#setPropertiesToSet(Map)}
+ * <p>
+ * All fail when conflicts arise from:
+ * <p>
+ * - Iterators attached directly to a table
+ * <p>
+ * - Iterators attached to a namespace, inherited by a table
+ * <p>
+ * - Default table iterators, but should not fail if {@link 
NewTableConfiguration#withoutDefaults()}
+ * is specified
+ * <p>
+ * - Adding the exact iterator already present should not fail
+ */
+public class IteratorConflictsIT extends SharedMiniClusterBase {
+  private static TableOperations tops;
+  private static NamespaceOperations nops;
+  private static AccumuloClient client;
+  // doesn't matter what iterator is used here
+  private static final String iterClass = SlowIterator.class.getName();
+  private static final IteratorSetting iter1 = new IteratorSetting(99, 
"iter1name", iterClass);
+  private static final String iter1Key = Property.TABLE_ITERATOR_PREFIX
+      + IteratorUtil.IteratorScope.scan.name().toLowerCase() + "." + 
iter1.getName();
+  private static final String iter1Val = "99," + iterClass;
+  private static final IteratorSetting iter1PrioConflict =
+      new IteratorSetting(99, "othername", iterClass);
+  private static final IteratorSetting iter1NameConflict =
+      new IteratorSetting(101, iter1.getName(), iterClass);
+  private static final String iter1PrioConflictKey = 
Property.TABLE_ITERATOR_PREFIX
+      + IteratorUtil.IteratorScope.scan.name().toLowerCase() + ".othername";
+  private static final String iter1PrioConflictVal = "99," + iterClass;
+  private static final String iter1NameConflictKey = 
Property.TABLE_ITERATOR_PREFIX
+      + IteratorUtil.IteratorScope.scan.name().toLowerCase() + "." + 
iter1.getName();
+  private static final String iter1NameConflictVal = "101," + iterClass;
+  private static final IteratorSetting defaultIterPrioConflict =
+      new IteratorSetting(20, "bar", iterClass);
+  private static final IteratorSetting defaultIterNameConflict =
+      new IteratorSetting(101, "vers", iterClass);
+  private static final IteratorSetting defaultTableIter =
+      
IteratorConfigUtil.getInitialTableIteratorSettings().keySet().iterator().next();
+  private static final String defaultIterPrioConflictKey = 
Property.TABLE_ITERATOR_PREFIX
+      + IteratorUtil.IteratorScope.scan.name().toLowerCase() + ".foo";
+  private static final String defaultIterPrioConflictVal =
+      defaultTableIter.getPriority() + "," + iterClass;
+  private static final String defaultIterNameConflictKey = 
Property.TABLE_ITERATOR_PREFIX
+      + IteratorUtil.IteratorScope.scan.name().toLowerCase() + "." + 
defaultTableIter.getName();
+  private static final String defaultIterNameConflictVal = "99," + iterClass;
+  private static final String defaultIterKey = 
Property.TABLE_ITERATOR_PREFIX.getKey()
+      + IteratorUtil.IteratorScope.scan.name().toLowerCase() + "." + 
defaultTableIter.getName();
+  private static final String defaultIterVal =
+      defaultTableIter.getPriority() + "," + 
defaultTableIter.getIteratorClass();
+
+  @BeforeAll
+  public static void startup() throws Exception {
+    SharedMiniClusterBase.startMiniCluster();
+    client = Accumulo.newClient().from(getClientProps()).build();
+    tops = client.tableOperations();
+    nops = client.namespaceOperations();
+  }
+
+  @AfterAll
+  public static void shutdown() throws Exception {
+    client.close();
+    SharedMiniClusterBase.stopMiniCluster();
+  }
+
+  @Test
+  public void testTableIterConflict() throws Exception {
+    final String[] tableNames = getUniqueNames(12);
+    String table1 = tableNames[0];
+    String table2 = tableNames[1];
+    String table3 = tableNames[2];
+    String table4 = tableNames[3];
+    String ns5 = tableNames[4];
+    String table5 = ns5 + "." + tableNames[5];
+    String ns6 = tableNames[6];
+    String table6 = ns6 + "." + tableNames[7];
+    String ns7 = tableNames[8];
+    String table7 = ns7 + "." + tableNames[9];
+    String table8 = tableNames[10];
+    for (String ns : List.of(ns5, ns6, ns7)) {
+      nops.create(ns);
+    }
+    for (String table : List.of(table1, table2, table3, table4, table5, 
table6, table7, table8)) {
+      tops.create(table);
+    }
+
+    // testing Scanner.addScanIterator
+    try (var scanner1 = client.createScanner(table1); var scanner2 = 
client.createScanner(table1)) {
+      testTableIterConflict(table1, RuntimeException.class, () -> {
+        scanner1.addScanIterator(iter1PrioConflict);
+        scanner1.iterator().hasNext();
+      }, () -> {
+        scanner2.addScanIterator(iter1NameConflict);
+        scanner2.iterator().hasNext();
+      });
+    }
+
+    // testing TableOperations.setProperty
+    testTableIterConflict(table2, AccumuloException.class,
+        () -> tops.setProperty(table2, iter1PrioConflictKey, 
iter1PrioConflictVal),
+        () -> tops.setProperty(table2, iter1NameConflictKey, 
iter1NameConflictVal));
+
+    // testing TableOperations.modifyProperties
+    testTableIterConflict(table3, AccumuloException.class,
+        () -> tops.modifyProperties(table3,
+            props -> props.put(iter1PrioConflictKey, iter1PrioConflictVal)),
+        () -> tops.modifyProperties(table3,
+            props -> props.put(iter1NameConflictKey, iter1NameConflictVal)));
+
+    // NewTableConfiguration.attachIterator is not applicable for this test
+    // Attaching the iterator to the table requires the table to exist, but 
testing
+    // NewTableConfiguration.attachIterator requires that the table does not 
exist
+
+    // testing TableOperations.attachIterator
+    testTableIterConflict(table4, AccumuloException.class,
+        () -> tops.attachIterator(table4, iter1PrioConflict),
+        () -> tops.attachIterator(table4, iter1NameConflict));
+
+    // testing NamespaceOperations.attachIterator
+    testTableIterConflict(table5, AccumuloException.class,
+        () -> nops.attachIterator(ns5, iter1PrioConflict),
+        () -> nops.attachIterator(ns5, iter1NameConflict));
+
+    // testing NamespaceOperations.setProperty
+    testTableIterConflict(table6, AccumuloException.class,
+        () -> nops.setProperty(ns6, iter1PrioConflictKey, 
iter1PrioConflictVal),
+        () -> nops.setProperty(ns6, iter1NameConflictKey, 
iter1NameConflictVal));
+
+    // testing NamespaceOperations.modifyProperties
+    testTableIterConflict(table7, AccumuloException.class,
+        () -> nops.modifyProperties(ns7,
+            props -> props.put(iter1PrioConflictKey, iter1PrioConflictVal)),
+        () -> nops.modifyProperties(ns7,
+            props -> props.put(iter1NameConflictKey, iter1NameConflictVal)));
+
+    // testing CloneConfiguration.Builder.setPropertiesToSet
+    String table9 = tableNames[11];
+    testTableIterConflict(table8, AccumuloException.class,
+        () -> tops.clone(table8, table9,
+            CloneConfiguration.builder()
+                .setPropertiesToSet(Map.of(iter1PrioConflictKey, 
iter1PrioConflictVal)).build()),
+        () -> tops.clone(table8, table9, CloneConfiguration.builder()
+            .setPropertiesToSet(Map.of(iter1NameConflictKey, 
iter1NameConflictVal)).build()));
+  }
+
+  private <T extends Exception> void testTableIterConflict(String table, 
Class<T> exceptionClass,
+      Executable iterPrioConflictExec, Executable iterNameConflictExec) throws 
Exception {
+    tops.attachIterator(table, iter1);
+    assertThrows(exceptionClass, iterPrioConflictExec);
+    assertThrows(exceptionClass, iterNameConflictExec);

Review Comment:
   removed exception message check at least for now. Need #6048 for server to 
throw exception back to user for CREATE_TABLE and CLONE_TABLE. Similar issue 
for scanner exceptions.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to