This is an automated email from the ASF dual-hosted git repository. cshannon pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push: new dac09c3751 Use TableOperations.modifyProperties in more places (#2965) dac09c3751 is described below commit dac09c3751028d171c465c724b47bc6379b91235 Author: Dom G <domgargu...@apache.org> AuthorDate: Mon Oct 17 17:08:10 2022 -0400 Use TableOperations.modifyProperties in more places (#2965) Use TableOps.modifyProperties in more places Co-authored-by: Christopher L. Shannon <christopher.l.shan...@gmail.com> --- .../core/clientImpl/TableOperationsHelper.java | 18 +++++++------- .../core/clientImpl/TableOperationsImpl.java | 28 ++++++++-------------- .../core/clientImpl/TableOperationsHelperTest.java | 12 ++++------ .../shell/commands/CreateTableCommand.java | 18 +++++++------- .../test/functional/ServerSideErrorIT.java | 9 ++++--- 5 files changed, 38 insertions(+), 47 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsHelper.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsHelper.java index 1748592792..9fab4fc5e6 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsHelper.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsHelper.java @@ -56,10 +56,12 @@ public abstract class TableOperationsHelper implements TableOperations { for (IteratorScope scope : scopes) { String root = String.format("%s%s.%s", Property.TABLE_ITERATOR_PREFIX, scope.name().toLowerCase(), setting.getName()); - for (Entry<String,String> prop : setting.getOptions().entrySet()) { - this.setProperty(tableName, root + ".opt." + prop.getKey(), prop.getValue()); - } - this.setProperty(tableName, root, setting.getPriority() + "," + setting.getIteratorClass()); + this.modifyProperties(tableName, properties -> { + for (Entry<String,String> prop : setting.getOptions().entrySet()) { + properties.put(root + ".opt." + prop.getKey(), prop.getValue()); + } + properties.put(root, setting.getPriority() + "," + setting.getIteratorClass()); + }); } } @@ -72,10 +74,10 @@ public abstract class TableOperationsHelper implements TableOperations { 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()); - } + this.modifyProperties(tableName, + properties -> copy.keySet().stream() + .filter(prop -> prop.equals(root) || prop.startsWith(root + ".opt.")) + .forEach(properties::remove)); } } diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java index 1f66da5630..d8fda622ea 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java @@ -1822,11 +1822,9 @@ public class TableOperationsImpl extends TableOperationsHelper { EXISTING_TABLE_NAME.validate(tableName); clearSamplerOptions(tableName); - List<Pair<String,String>> props = - new SamplerConfigurationImpl(samplerConfiguration).toTableProperties(); - for (Pair<String,String> pair : props) { - setProperty(tableName, pair.getFirst(), pair.getSecond()); - } + Map<String,String> props = + new SamplerConfigurationImpl(samplerConfiguration).toTablePropertiesMap(); + modifyProperties(tableName, properties -> properties.putAll(props)); } @Override @@ -2068,11 +2066,8 @@ public class TableOperationsImpl extends TableOperationsHelper { } } - Set<Entry<String,String>> es = - SummarizerConfiguration.toTableProperties(newConfigSet).entrySet(); - for (Entry<String,String> entry : es) { - setProperty(tableName, entry.getKey(), entry.getValue()); - } + Map<String,String> props = SummarizerConfiguration.toTableProperties(newConfigSet); + modifyProperties(tableName, properties -> properties.putAll(props)); } @Override @@ -2083,14 +2078,11 @@ public class TableOperationsImpl extends TableOperationsHelper { Collection<SummarizerConfiguration> summarizerConfigs = SummarizerConfiguration.fromTableProperties(getProperties(tableName)); - for (SummarizerConfiguration sc : summarizerConfigs) { - if (predicate.test(sc)) { - Set<String> ks = sc.toTableProperties().keySet(); - for (String key : ks) { - removeProperty(tableName, key); - } - } - } + modifyProperties(tableName, + properties -> summarizerConfigs.stream().filter(predicate) + .map(sc -> sc.toTableProperties().keySet()) + .forEach(keySet -> keySet.forEach(properties::remove))); + } @Override diff --git a/core/src/test/java/org/apache/accumulo/core/clientImpl/TableOperationsHelperTest.java b/core/src/test/java/org/apache/accumulo/core/clientImpl/TableOperationsHelperTest.java index 02dfa04cb5..7587298b7f 100644 --- a/core/src/test/java/org/apache/accumulo/core/clientImpl/TableOperationsHelperTest.java +++ b/core/src/test/java/org/apache/accumulo/core/clientImpl/TableOperationsHelperTest.java @@ -133,8 +133,7 @@ public class TableOperationsHelperTest { @Override public void setProperty(String tableName, String property, String value) { - if (!settings.containsKey(tableName)) - settings.put(tableName, new TreeMap<>()); + settings.computeIfAbsent(tableName, k -> new TreeMap<>()); settings.get(tableName).put(property, value); } @@ -142,13 +141,10 @@ public class TableOperationsHelperTest { public Map<String,String> modifyProperties(String tableName, Consumer<Map<String,String>> mapMutator) throws IllegalArgumentException, ConcurrentModificationException { + settings.computeIfAbsent(tableName, k -> new TreeMap<>()); var map = settings.get(tableName); - if (map != null) { - mapMutator.accept(map); - return Map.copyOf(map); - } else { - throw new IllegalArgumentException("No such table " + tableName); - } + mapMutator.accept(map); + return Map.copyOf(map); } @Override diff --git a/shell/src/main/java/org/apache/accumulo/shell/commands/CreateTableCommand.java b/shell/src/main/java/org/apache/accumulo/shell/commands/CreateTableCommand.java index 2113fa1988..3abdf15d78 100644 --- a/shell/src/main/java/org/apache/accumulo/shell/commands/CreateTableCommand.java +++ b/shell/src/main/java/org/apache/accumulo/shell/commands/CreateTableCommand.java @@ -27,7 +27,6 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.Set; import java.util.TreeSet; @@ -134,9 +133,9 @@ public class CreateTableCommand extends Command { shellState.setTableName(tableName); // switch shell to new table context if (cl.hasOption(createTableNoDefaultIters.getOpt())) { - for (String key : IteratorConfigUtil.generateInitialTableProperties(true).keySet()) { - shellState.getAccumuloClient().tableOperations().removeProperty(tableName, key); - } + Set<String> initialProps = IteratorConfigUtil.generateInitialTableProperties(true).keySet(); + shellState.getAccumuloClient().tableOperations().modifyProperties(tableName, + properties -> initialProps.forEach(properties::remove)); } // Copy options if flag was set @@ -144,12 +143,11 @@ public class CreateTableCommand extends Command { if (shellState.getAccumuloClient().tableOperations().exists(tableName)) { final Map<String,String> configuration = shellState.getAccumuloClient().tableOperations() .getConfiguration(cl.getOptionValue(createTableOptCopyConfig.getOpt())); - for (Entry<String,String> entry : configuration.entrySet()) { - if (Property.isValidTablePropertyKey(entry.getKey())) { - shellState.getAccumuloClient().tableOperations().setProperty(tableName, entry.getKey(), - entry.getValue()); - } - } + + shellState.getAccumuloClient().tableOperations().modifyProperties(tableName, + properties -> configuration.entrySet().stream() + .filter(entry -> Property.isValidTablePropertyKey(entry.getKey())) + .forEach(entry -> properties.put(entry.getKey(), entry.getValue()))); } } diff --git a/test/src/main/java/org/apache/accumulo/test/functional/ServerSideErrorIT.java b/test/src/main/java/org/apache/accumulo/test/functional/ServerSideErrorIT.java index 8522205d99..2899a87db3 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/ServerSideErrorIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/ServerSideErrorIT.java @@ -81,9 +81,12 @@ public class ServerSideErrorIT extends AccumuloClusterHarness { // remove the bad agg so accumulo can shutdown TableOperations to = c.tableOperations(); - for (Entry<String,String> e : to.getProperties(tableName)) { - to.removeProperty(tableName, e.getKey()); - } + Iterable<Entry<String,String>> tableProps = to.getProperties(tableName); + to.modifyProperties(tableName, properties -> { + for (Entry<String,String> e : tableProps) { + properties.remove(e.getKey()); + } + }); sleepUninterruptibly(500, TimeUnit.MILLISECONDS);