keith-turner commented on code in PR #5525:
URL: https://github.com/apache/accumulo/pull/5525#discussion_r2073734660
##########
test/src/main/java/org/apache/accumulo/test/IteratorEnvIT.java:
##########
@@ -71,121 +92,123 @@ public void configureMiniCluster(MiniAccumuloConfigImpl
cfg, Configuration hadoo
/**
* Basic scan iterator to test IteratorEnvironment returns what is expected.
*/
- public static class ScanIter extends WrappingIterator {
+ public static class ScanIter extends Filter {
IteratorScope scope = IteratorScope.scan;
@Override
public void init(SortedKeyValueIterator<Key,Value> source,
Map<String,String> options,
IteratorEnvironment env) throws IOException {
super.init(source, options, env);
- testEnv(scope, options, env);
-
// Checking for compaction on a scan should throw an error.
- try {
- env.isUserCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when
checking compaction on a scan.");
- } catch (IllegalStateException e) {}
- try {
- env.isFullMajorCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when
checking compaction on a scan.");
- } catch (IllegalStateException e) {}
+ testEnv(scope, options, env, true);
+ }
+
+ @Override
+ public boolean accept(Key k, Value v) {
+ // The only reason for filtering out some data is as a way to verify
init() and testEnv()
+ // have been called
+ return !k.getColumnFamily().toString().equals(BAD_COL_FAM);
}
}
/**
* Basic compaction iterator to test IteratorEnvironment returns what is
expected.
*/
- public static class MajcIter extends WrappingIterator {
+ public static class MajcIter extends Filter {
IteratorScope scope = IteratorScope.majc;
@Override
public void init(SortedKeyValueIterator<Key,Value> source,
Map<String,String> options,
IteratorEnvironment env) throws IOException {
super.init(source, options, env);
- testEnv(scope, options, env);
- try {
- env.isUserCompaction();
- } catch (IllegalStateException e) {
- throw new RuntimeException("Test failed");
- }
- try {
- env.isFullMajorCompaction();
- } catch (IllegalStateException e) {
- throw new RuntimeException("Test failed");
- }
+ // Checking for compaction on majc should not throw an error.
+ testEnv(scope, options, env, false);
+ }
+
+ @Override
+ public boolean accept(Key k, Value v) {
+ // The only reason for filtering out some data is as a way to verify
init() and testEnv()
+ // have been called
+ return !k.getColumnFamily().toString().equals(BAD_COL_FAM);
}
}
/**
*
*/
- public static class MincIter extends WrappingIterator {
+ public static class MincIter extends Filter {
IteratorScope scope = IteratorScope.minc;
@Override
public void init(SortedKeyValueIterator<Key,Value> source,
Map<String,String> options,
IteratorEnvironment env) throws IOException {
super.init(source, options, env);
- testEnv(scope, options, env);
- try {
- env.isUserCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when
checking compaction on a scan.");
- } catch (IllegalStateException e) {}
- try {
- env.isFullMajorCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when
checking compaction on a scan.");
- } catch (IllegalStateException e) {}
+ // Checking for compaction on minc should throw an error.
+ testEnv(scope, options, env, true);
+ }
+
+ @Override
+ public boolean accept(Key k, Value v) {
+ // The only reason for filtering out some data is as a way to verify
init() and testEnv()
+ // have been called
+ return !k.getColumnFamily().toString().equals(BAD_COL_FAM);
}
}
/**
* Test the environment methods return what is expected.
*/
- private static void testEnv(IteratorScope scope, Map<String,String> opts,
- IteratorEnvironment env) {
- TableId expectedTableId = TableId.of(opts.get("expected.table.id"));
+ private static void testEnv(IteratorScope scope, Map<String,String> opts,
IteratorEnvironment env,
+ boolean checkCompactionShouldThrow) {
+ String expTableIdString = opts.get(EXPECTED_TABLE_ID_OPT);
+ TableId expectedTableId = expTableIdString == null ? null :
TableId.of(expTableIdString);
+
+ if (checkCompactionShouldThrow) {
+ assertThrows(IllegalStateException.class, env::isUserCompaction,
+ "Test failed - Expected to throw IllegalStateException when checking
compaction");
+ assertThrows(IllegalStateException.class, env::isFullMajorCompaction,
+ "Test failed - Expected to throw IllegalStateException when checking
compaction");
+ } else {
+ assertDoesNotThrow(env::isUserCompaction,
+ "Test failed - Expected not to throw exception when checking
compaction");
+ assertDoesNotThrow(env::isFullMajorCompaction,
+ "Test failed - Expected not to throw exception when checking
compaction");
+ }
// verify getServiceEnv() and getPluginEnv() are the same objects,
// so further checks only need to use getPluginEnv()
@SuppressWarnings("deprecation")
ServiceEnvironment serviceEnv = env.getServiceEnv();
PluginEnvironment pluginEnv = env.getPluginEnv();
- if (serviceEnv != pluginEnv) {
- throw new RuntimeException("Test failed -
assertSame(getServiceEnv(),getPluginEnv())");
- }
+ assertEquals(serviceEnv, pluginEnv, "Test failed -
assertSame(getServiceEnv(),getPluginEnv())");
// verify property exists on the table config (deprecated and new),
// with and without custom prefix, but not in the system config
@SuppressWarnings("deprecation")
- String accTableConf =
env.getConfig().get("table.custom.iterator.env.test");
- if (!"value1".equals(accTableConf)) {
- throw new RuntimeException("Test failed - Expected table property not
found in getConfig().");
- }
+ String accTableConf = env.getConfig().get(CUSTOM_PROP_KEY);
+ assertEquals(CUSTOM_PROP_VAL, accTableConf,
+ "Test failed - Expected table property not found in getConfig().");
var tableConf = pluginEnv.getConfiguration(env.getTableId());
- if (!"value1".equals(tableConf.get("table.custom.iterator.env.test"))) {
- throw new RuntimeException("Test failed - Expected table property not
found in table conf.");
- }
- if (!"value1".equals(tableConf.getTableCustom("iterator.env.test"))) {
- throw new RuntimeException("Test failed - Expected table property not
found in table conf.");
- }
+ assertEquals(CUSTOM_PROP_VAL, tableConf.get(CUSTOM_PROP_KEY),
+ "Test failed - Expected table property not found in table conf.");
+ assertEquals(CUSTOM_PROP_VAL,
tableConf.getTableCustom(CUSTOM_PROP_KEY_SUFFIX),
+ "Test failed - Expected table property not found in table conf.");
var systemConf = pluginEnv.getConfiguration();
- if (systemConf.get("table.custom.iterator.env.test") != null) {
- throw new RuntimeException("Test failed - Unexpected table property
found in system conf.");
- }
+ assertNull(systemConf.get(CUSTOM_PROP_KEY),
+ "Test failed - Unexpected table property found in system conf.");
// check other environment settings
- if (!scope.equals(env.getIteratorScope())) {
- throw new RuntimeException("Test failed - Error getting iterator scope");
- }
- if (env.isSamplingEnabled()) {
- throw new RuntimeException("Test failed - isSamplingEnabled returned
true, expected false");
- }
- if (!expectedTableId.equals(env.getTableId())) {
- throw new RuntimeException("Test failed - Error getting Table ID");
+ assertEquals(scope, env.getIteratorScope(), "Test failed - Error getting
iterator scope");
+ assertFalse(env.isSamplingEnabled(),
+ "Test failed - isSamplingEnabled returned true, expected false");
+ assertEquals(expectedTableId, env.getTableId(), "Test failed - Error
getting Table ID");
+ assertNull(env.getSamplerConfiguration());
+ assertThrows(Exception.class, env::cloneWithSamplingEnabled);
Review Comment:
I can look into generally improving test when I look at #5500. I was just
thinking of RFileScanner when I opened that issue, but I can look at making it
more general.
##########
test/src/main/java/org/apache/accumulo/test/IteratorEnvIT.java:
##########
@@ -71,121 +92,123 @@ public void configureMiniCluster(MiniAccumuloConfigImpl
cfg, Configuration hadoo
/**
* Basic scan iterator to test IteratorEnvironment returns what is expected.
*/
- public static class ScanIter extends WrappingIterator {
+ public static class ScanIter extends Filter {
IteratorScope scope = IteratorScope.scan;
@Override
public void init(SortedKeyValueIterator<Key,Value> source,
Map<String,String> options,
IteratorEnvironment env) throws IOException {
super.init(source, options, env);
- testEnv(scope, options, env);
-
// Checking for compaction on a scan should throw an error.
- try {
- env.isUserCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when
checking compaction on a scan.");
- } catch (IllegalStateException e) {}
- try {
- env.isFullMajorCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when
checking compaction on a scan.");
- } catch (IllegalStateException e) {}
+ testEnv(scope, options, env, true);
+ }
+
+ @Override
+ public boolean accept(Key k, Value v) {
+ // The only reason for filtering out some data is as a way to verify
init() and testEnv()
+ // have been called
+ return !k.getColumnFamily().toString().equals(BAD_COL_FAM);
}
}
/**
* Basic compaction iterator to test IteratorEnvironment returns what is
expected.
*/
- public static class MajcIter extends WrappingIterator {
+ public static class MajcIter extends Filter {
IteratorScope scope = IteratorScope.majc;
@Override
public void init(SortedKeyValueIterator<Key,Value> source,
Map<String,String> options,
IteratorEnvironment env) throws IOException {
super.init(source, options, env);
- testEnv(scope, options, env);
- try {
- env.isUserCompaction();
- } catch (IllegalStateException e) {
- throw new RuntimeException("Test failed");
- }
- try {
- env.isFullMajorCompaction();
- } catch (IllegalStateException e) {
- throw new RuntimeException("Test failed");
- }
+ // Checking for compaction on majc should not throw an error.
+ testEnv(scope, options, env, false);
+ }
+
+ @Override
+ public boolean accept(Key k, Value v) {
+ // The only reason for filtering out some data is as a way to verify
init() and testEnv()
+ // have been called
+ return !k.getColumnFamily().toString().equals(BAD_COL_FAM);
}
}
/**
*
*/
- public static class MincIter extends WrappingIterator {
+ public static class MincIter extends Filter {
IteratorScope scope = IteratorScope.minc;
@Override
public void init(SortedKeyValueIterator<Key,Value> source,
Map<String,String> options,
IteratorEnvironment env) throws IOException {
super.init(source, options, env);
- testEnv(scope, options, env);
- try {
- env.isUserCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when
checking compaction on a scan.");
- } catch (IllegalStateException e) {}
- try {
- env.isFullMajorCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when
checking compaction on a scan.");
- } catch (IllegalStateException e) {}
+ // Checking for compaction on minc should throw an error.
+ testEnv(scope, options, env, true);
+ }
+
+ @Override
+ public boolean accept(Key k, Value v) {
+ // The only reason for filtering out some data is as a way to verify
init() and testEnv()
+ // have been called
+ return !k.getColumnFamily().toString().equals(BAD_COL_FAM);
}
}
/**
* Test the environment methods return what is expected.
*/
- private static void testEnv(IteratorScope scope, Map<String,String> opts,
- IteratorEnvironment env) {
- TableId expectedTableId = TableId.of(opts.get("expected.table.id"));
+ private static void testEnv(IteratorScope scope, Map<String,String> opts,
IteratorEnvironment env,
+ boolean checkCompactionShouldThrow) {
+ String expTableIdString = opts.get(EXPECTED_TABLE_ID_OPT);
+ TableId expectedTableId = expTableIdString == null ? null :
TableId.of(expTableIdString);
+
+ if (checkCompactionShouldThrow) {
Review Comment:
Could use the scope instead of adding the new boolean.
```suggestion
if (scope != majc) {
```
##########
core/src/main/java/org/apache/accumulo/core/iteratorsImpl/ClientIteratorEnvironment.java:
##########
@@ -221,7 +221,7 @@ public PluginEnvironment getPluginEnv() {
@Override
public TableId getTableId() {
- return this.tableId.orElseThrow();
+ return this.tableId.orElse(null);
Review Comment:
Should probably change javadoc for this method in IteratorEnvironment to
document this method could return null in cases where there is no table
associated with the iterator.
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
