Changes:

- Changed ClientSideIteratorScanner.getConfig() to use the safer getPluginEnv()
- Added getPluginEnv() impl to all IteratorEnvironments (besides test envs)
- Improved RFile.ScannerOptions.withTableProperties() javadoc to better explain how iterators will have access to these properties
- Opted for RFileScanner.IterEnv.getTableId() to return null instead of a dummy id
- Removed incorrect impl of RFileScanner.IterEnv.get(Service/Plugin)Env().getTableName() to instead just throw UnsupOpExc since the method doesn't make sense anyways.

Co-authored-by: Christopher Tubbs <[email protected]>

Author: kevinrr888

core/src/main/java/org/apache/accumulo/core/client/ClientSideIteratorScanner.java

@@ -145,13 +145,7 @@ public SamplerConfiguration getSamplerConfiguration() {
     @Override
     @Deprecated(since = "2.0.0")
     public AccumuloConfiguration getConfig() {
-      var ctx = context.get();
-      try {
-        return new ConfigurationCopy(
-            ctx.tableOperations().getConfiguration(ctx.getTableName(tableId.get())));
-      } catch (AccumuloException | TableNotFoundException e) {
-        throw new RuntimeException("Error getting table configuration", e);
-      }
+      return new ConfigurationCopy(getPluginEnv().getConfiguration(getTableId()));
     }
 
     @Deprecated(since = "2.1.0")
@@ -160,6 +154,11 @@ public ServiceEnvironment getServiceEnv() {
       return serviceEnvironment.get();
     }
 
+    @Override
+    public PluginEnvironment getPluginEnv() {
+      return serviceEnvironment.get();
+    }
+
     @Override
     public TableId getTableId() {
       return tableId.get();

core/src/main/java/org/apache/accumulo/core/client/rfile/RFile.java

@@ -176,8 +176,10 @@ public interface ScannerOptions {
      * properties could be passed here.
      *
      * <p>
-     * Starting with {@code 2.1.4}, {@link PluginEnvironment#getConfiguration(TableId)} (obtained by
-     * {@link IteratorEnvironment#getPluginEnv()}) will return the properties passed in here.
+     * Configured iterators will have access to these properties via the
+     * {@link PluginEnvironment#getConfiguration(TableId)} (obtained by
+     * {@link IteratorEnvironment#getPluginEnv()}). The tableId used to get the configuration should
+     * be the one returned programmatically from {@link IteratorEnvironment#getTableId()}.
      *
      * @param props iterable over Accumulo table key value properties.
      * @return this
@@ -189,10 +191,6 @@ public interface ScannerOptions {
      * {@link Property#TABLE_PREFIX} may be accepted and used. For example, cache and crypto
      * properties could be passed here.
      *
-     * <p>
-     * Starting with {@code 2.1.4}, {@link PluginEnvironment#getConfiguration(TableId)} (obtained by
-     * {@link IteratorEnvironment#getPluginEnv()}) will return the properties passed in here.
-     *
      * @param props a map instead of an Iterable
      * @return this
      * @see #withTableProperties(Iterable)

core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java

@@ -31,6 +31,7 @@
 import java.util.function.Supplier;
 
 import org.apache.accumulo.core.client.IteratorSetting;
+import org.apache.accumulo.core.client.PluginEnvironment;
 import org.apache.accumulo.core.client.Scanner;
 import org.apache.accumulo.core.client.rfile.RFileScannerBuilder.InputArgs;
 import org.apache.accumulo.core.client.sample.SamplerConfiguration;
@@ -85,8 +86,6 @@ class RFileScanner extends ScannerOptions implements Scanner {
   private static final String errorMsg =
       "This scanner is unrelated to any table or accumulo instance;"
           + " it operates directly on files. Therefore, it can not support this operation.";
-  private static final TableId dummyTableId = TableId.of("RFileScannerFakeTableId");
-
   private Range range;
   private BlockCacheManager blockCacheManager = null;
   private BlockCache dataCache = null;
@@ -357,9 +356,16 @@ public SamplerConfiguration getSamplerConfiguration() {
       return RFileScanner.this.getSamplerConfiguration();
     }
 
+    /**
+     * This method only exists to be used as described in {@link IteratorEnvironment#getPluginEnv()}
+     * so the table config can be obtained. This simply returns null since a table id does not make
+     * sense in the context of scanning RFiles, but is needed to obtain the table configuration.
+     *
+     * @return null
+     */
     @Override
     public TableId getTableId() {
-      return dummyTableId;
+      return null;
     }
 
     @Override
@@ -374,6 +380,11 @@ public ServiceEnvironment getServiceEnv() {
       return serviceEnvironment.get();
     }
 
+    @Override
+    public PluginEnvironment getPluginEnv() {
+      return serviceEnvironment.get();
+    }
+
     private ServiceEnvironment createServiceEnv() {
       return new ServiceEnvironment() {
         @Override
@@ -391,15 +402,14 @@ public <T> T instantiate(String className, Class<T> base)
 
         @Override
         public String getTableName(TableId tableId) {
-          Preconditions.checkArgument(tableId.equals(getTableId()), "Expected " + getTableId()
-              + " but got " + tableId + " when requesting the table config");
-          return tableId.canonical();
+          throw new UnsupportedOperationException(errorMsg);
         }
 
         @Override
         public Configuration getConfiguration(TableId tableId) {
-          Preconditions.checkArgument(tableId.equals(getTableId()), "Expected " + getTableId()
-              + " but got " + tableId + " when requesting the table config");
+          Preconditions.checkArgument(tableId == getTableId(),
+              "Expected tableId obtained from IteratorEnvironment.getTableId() but got " + tableId
+                  + " when requesting the table config");
           return new ConfigurationImpl(tableConf);
         }
 

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

@@ -35,6 +35,7 @@
 
 import org.apache.accumulo.core.client.AccumuloException;
 import org.apache.accumulo.core.client.AccumuloSecurityException;
+import org.apache.accumulo.core.client.PluginEnvironment;
 import org.apache.accumulo.core.client.SampleNotPresentException;
 import org.apache.accumulo.core.client.TableNotFoundException;
 import org.apache.accumulo.core.client.sample.SamplerConfiguration;
@@ -169,6 +170,11 @@ public ServiceEnvironment getServiceEnv() {
       return serviceEnvironment.get();
     }
 
+    @Override
+    public PluginEnvironment getPluginEnv() {
+      return serviceEnvironment.get();
+    }
+
     @Override
     public TableId getTableId() {
       return tableId;

server/base/src/main/java/org/apache/accumulo/server/iterators/TabletIteratorEnvironment.java

@@ -23,6 +23,7 @@
 import java.util.Collections;
 import java.util.Map;
 
+import org.apache.accumulo.core.client.PluginEnvironment;
 import org.apache.accumulo.core.client.SampleNotPresentException;
 import org.apache.accumulo.core.client.sample.SamplerConfiguration;
 import org.apache.accumulo.core.conf.AccumuloConfiguration;
@@ -232,6 +233,11 @@ public ServiceEnvironment getServiceEnv() {
     return serviceEnvironment;
   }
 
+  @Override
+  public PluginEnvironment getPluginEnv() {
+    return serviceEnvironment;
+  }
+
   @Override
   public TableId getTableId() {
     return tableId;

test/src/main/java/org/apache/accumulo/test/IteratorEnvIT.java

@@ -185,7 +185,8 @@ public boolean accept(Key k, Value v) {
    */
   private static void testEnv(IteratorScope scope, Map<String,String> opts,
       IteratorEnvironment env) {
-    TableId expectedTableId = TableId.of(opts.get("expected.table.id"));
+    String expTableIdStr = opts.get("expected.table.id");
+    TableId expTableId = expTableIdStr == null ? null : TableId.of(expTableIdStr);
 
     // verify getServiceEnv() and getPluginEnv() are the same objects,
     // so further checks only need to use getPluginEnv()
@@ -227,7 +228,7 @@ private static void testEnv(IteratorScope scope, Map<String,String> opts,
     if (env.isSamplingEnabled()) {
       throw new RuntimeException("Test failed - isSamplingEnabled returned true, expected false");
     }
-    if (!expectedTableId.equals(env.getTableId())) {
+    if (expTableId != null && !expTableId.equals(env.getTableId())) {
       throw new RuntimeException("Test failed - Error getting Table ID");
     }
   }
@@ -248,7 +249,8 @@ public void finish() {
   public void test() throws Exception {
     String[] tables = getUniqueNames(5);
     testScan(tables[0], ScanIter.class);
-    testRFileScan("RFileScannerFakeTableId", ScanIter.class);
+    // No table id when scanning at file level
+    testRFileScan(ScanIter.class);
     testOfflineScan(tables[1], ScanIter.class);
     testClientSideScan(tables[2], ScanIter.class);
     testCompact(tables[3], MajcIter.class);
@@ -269,14 +271,13 @@ private void testScan(String tableName,
     }
   }
 
-  private void testRFileScan(String tableName,
-      Class<? extends SortedKeyValueIterator<Key,Value>> iteratorClass) throws Exception {
+  private void testRFileScan(Class<? extends SortedKeyValueIterator<Key,Value>> iteratorClass)
+      throws Exception {
     TreeMap<Key,Value> data = createTestData();
     LocalFileSystem fs = FileSystem.getLocal(new Configuration());
     String rFilePath = createRFile(fs, data);
 
     IteratorSetting cfg = new IteratorSetting(1, iteratorClass);
-    cfg.addOption("expected.table.id", tableName);
     cfg.addOption("bad.col.fam", "badcf");
 
     try (Scanner scan = RFile.newScanner().from(rFilePath).withFileSystem(fs)