This is an automated email from the ASF dual-hosted git repository.

mmiller 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 e4299fa5b8 Add new CloneConfiguration API method (#2874)
e4299fa5b8 is described below

commit e4299fa5b82f7fcd835b5e81669882d90aa99ebd
Author: Mike Miller <mmil...@apache.org>
AuthorDate: Fri Aug 12 12:43:31 2022 +0000

    Add new CloneConfiguration API method (#2874)
    
    * Add new convenience method for empty object
    * Create private methods for reuse in TableOperationsImpl
    * Some various code clean up
---
 .../core/client/admin/CloneConfiguration.java      | 30 +++++++++------
 .../core/clientImpl/CloneConfigurationImpl.java    |  7 ++--
 .../core/clientImpl/TableOperationsImpl.java       | 43 +++++++++++++---------
 .../accumulo/manager/FateServiceHandler.java       |  6 +--
 .../accumulo/test/functional/CloneTestIT.java      |  8 ++--
 5 files changed, 55 insertions(+), 39 deletions(-)

diff --git 
a/core/src/main/java/org/apache/accumulo/core/client/admin/CloneConfiguration.java
 
b/core/src/main/java/org/apache/accumulo/core/client/admin/CloneConfiguration.java
index fa2c104258..a47fa7a58a 100644
--- 
a/core/src/main/java/org/apache/accumulo/core/client/admin/CloneConfiguration.java
+++ 
b/core/src/main/java/org/apache/accumulo/core/client/admin/CloneConfiguration.java
@@ -35,14 +35,14 @@ public interface CloneConfiguration {
    *
    * @return true if memory is flushed in the source table before cloning.
    */
-  public boolean isFlush();
+  boolean isFlush();
 
   /**
    * The source table properties are copied. This allows overriding of some of 
those properties.
    *
    * @return The source table properties to override.
    */
-  public Map<String,String> getPropertiesToSet();
+  Map<String,String> getPropertiesToSet();
 
   /**
    * The source table properties are copied, this allows reverting to system 
defaults for some of
@@ -50,7 +50,7 @@ public interface CloneConfiguration {
    *
    * @return The properties that are to be reverted to system defaults.
    */
-  public Set<String> getPropertiesToExclude();
+  Set<String> getPropertiesToExclude();
 
   /**
    * The new table is normally brought online after the cloning process. This 
allows leaving the new
@@ -58,21 +58,21 @@ public interface CloneConfiguration {
    *
    * @return true if the new table is to be kept offline after cloning.
    */
-  public boolean isKeepOffline();
+  boolean isKeepOffline();
 
   /**
    * A CloneConfiguration builder
    *
    * @since 1.10 and 2.1
    */
-  public static interface Builder {
+  interface Builder {
     /**
      * Determines if memory is flushed in the source table before cloning.
      *
      * @param flush
      *          true if memory is flushed in the source table before cloning.
      */
-    public Builder setFlush(boolean flush);
+    Builder setFlush(boolean flush);
 
     /**
      * The source table properties are copied. This allows overriding of some 
of those properties.
@@ -80,7 +80,7 @@ public interface CloneConfiguration {
      * @param propertiesToSet
      *          The source table properties to override.
      */
-    public Builder setPropertiesToSet(Map<String,String> propertiesToSet);
+    Builder setPropertiesToSet(Map<String,String> propertiesToSet);
 
     /**
      * The source table properties are copied, this allows reverting to system 
defaults for some of
@@ -89,7 +89,7 @@ public interface CloneConfiguration {
      * @param propertiesToExclude
      *          The properties that are to be reverted to system defaults.
      */
-    public Builder setPropertiesToExclude(Set<String> propertiesToExclude);
+    Builder setPropertiesToExclude(Set<String> propertiesToExclude);
 
     /**
      * The new table is normally brought online after the cloning process. 
This allows leaving the
@@ -98,21 +98,29 @@ public interface CloneConfiguration {
      * @param keepOffline
      *          true if the new table is to be kept offline after cloning.
      */
-    public Builder setKeepOffline(boolean keepOffline);
+    Builder setKeepOffline(boolean keepOffline);
 
     /**
      * Build the clone configuration
      *
      * @return the built immutable clone configuration
      */
-    public CloneConfiguration build();
+    CloneConfiguration build();
   }
 
   /**
    * @return a {@link CloneConfiguration} builder
    */
-  public static CloneConfiguration.Builder builder() {
+  static CloneConfiguration.Builder builder() {
     return new CloneConfigurationImpl();
   }
 
+  /**
+   * @return an empty configuration object with the default settings.
+   * @since 2.1.0
+   */
+  static CloneConfiguration empty() {
+    return builder().build();
+  }
+
 }
diff --git 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/CloneConfigurationImpl.java
 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/CloneConfigurationImpl.java
index d380090dae..9549fd6c5d 100644
--- 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/CloneConfigurationImpl.java
+++ 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/CloneConfigurationImpl.java
@@ -33,8 +33,7 @@ import com.google.common.base.Preconditions;
  */
 public class CloneConfigurationImpl implements CloneConfiguration, 
CloneConfiguration.Builder {
 
-  // The purpose of this is to allow building an immutable CloneConfiguration 
object without
-  // creating
+  // This boolean allows building an immutable CloneConfiguration object 
without creating
   // separate Builder and CloneConfiguration objects. This is done to reduce 
object creation and
   // copying. This could easily be changed to two objects without changing the 
interfaces.
   private boolean built = false;
@@ -62,14 +61,14 @@ public class CloneConfigurationImpl implements 
CloneConfiguration, CloneConfigur
   @Override
   public Map<String,String> getPropertiesToSet() {
     Preconditions.checkState(built);
-    return (propertiesToSet == null ? Collections.<String,String>emptyMap()
+    return (propertiesToSet == null ? Collections.emptyMap()
         : Collections.unmodifiableMap(propertiesToSet));
   }
 
   @Override
   public Set<String> getPropertiesToExclude() {
     Preconditions.checkState(built);
-    return (propertiesToExclude == null ? Collections.<String>emptySet()
+    return (propertiesToExclude == null ? Collections.emptySet()
         : Collections.unmodifiableSet(propertiesToExclude));
   }
 
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 60868ba229..e2cfc4840c 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
@@ -156,7 +156,7 @@ public class TableOperationsImpl extends 
TableOperationsHelper {
 
   private static final SecureRandom random = new SecureRandom();
 
-  public static final String CLONE_EXCLUDE_PREFIX = "!";
+  public static final String PROPERTY_EXCLUDE_PREFIX = "!";
   public static final String COMPACTION_CANCELED_MSG = "Compaction canceled";
   public static final String TABLE_DELETED_MSG = "Table is being deleted";
 
@@ -772,33 +772,21 @@ public class TableOperationsImpl extends 
TableOperationsHelper {
       throws AccumuloSecurityException, TableNotFoundException, 
AccumuloException,
       TableExistsException {
     NEW_TABLE_NAME.validate(newTableName);
+    requireNonNull(config, "CloneConfiguration required.");
 
     TableId srcTableId = context.getTableId(srcTableName);
 
     if (config.isFlush())
       _flush(srcTableId, null, null, true);
 
-    Set<String> propertiesToExclude = config.getPropertiesToExclude();
-    if (propertiesToExclude == null)
-      propertiesToExclude = Collections.emptySet();
-
-    Map<String,String> propertiesToSet = config.getPropertiesToSet();
-    if (propertiesToSet == null)
-      propertiesToSet = Collections.emptyMap();
+    Map<String,String> opts = new HashMap<>();
+    validatePropertiesToSet(opts, config.getPropertiesToSet());
 
     List<ByteBuffer> args = 
Arrays.asList(ByteBuffer.wrap(srcTableId.canonical().getBytes(UTF_8)),
         ByteBuffer.wrap(newTableName.getBytes(UTF_8)),
         
ByteBuffer.wrap(Boolean.toString(config.isKeepOffline()).getBytes(UTF_8)));
-    Map<String,String> opts = new HashMap<>();
-    for (Entry<String,String> entry : propertiesToSet.entrySet()) {
-      if (entry.getKey().startsWith(CLONE_EXCLUDE_PREFIX))
-        throw new IllegalArgumentException("Property can not start with " + 
CLONE_EXCLUDE_PREFIX);
-      opts.put(entry.getKey(), entry.getValue());
-    }
 
-    for (String prop : propertiesToExclude) {
-      opts.put(CLONE_EXCLUDE_PREFIX + prop, "");
-    }
+    prependPropertiesToExclude(opts, config.getPropertiesToExclude());
 
     doTableFateOperation(newTableName, AccumuloException.class, 
FateOperation.TABLE_CLONE, args,
         opts);
@@ -2017,4 +2005,25 @@ public class TableOperationsImpl extends 
TableOperationsHelper {
   public ImportDestinationArguments importDirectory(String directory) {
     return new BulkImport(directory, context);
   }
+
+  private void prependPropertiesToExclude(Map<String,String> opts, Set<String> 
propsToExclude) {
+    if (propsToExclude == null)
+      return;
+
+    for (String prop : propsToExclude) {
+      opts.put(PROPERTY_EXCLUDE_PREFIX + prop, "");
+    }
+  }
+
+  private void validatePropertiesToSet(Map<String,String> opts, 
Map<String,String> propsToSet) {
+    if (propsToSet == null)
+      return;
+
+    propsToSet.forEach((k, v) -> {
+      if (k.startsWith(PROPERTY_EXCLUDE_PREFIX))
+        throw new IllegalArgumentException(
+            "Property can not start with " + PROPERTY_EXCLUDE_PREFIX);
+      opts.put(k, v);
+    });
+  }
 }
diff --git 
a/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java
 
b/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java
index efac03777a..41ce18c9ff 100644
--- 
a/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java
+++ 
b/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java
@@ -301,9 +301,9 @@ class FateServiceHandler implements FateService.Iface {
         Set<String> propertiesToExclude = new HashSet<>();
 
         for (Entry<String,String> entry : options.entrySet()) {
-          if 
(entry.getKey().startsWith(TableOperationsImpl.CLONE_EXCLUDE_PREFIX)) {
-            propertiesToExclude
-                
.add(entry.getKey().substring(TableOperationsImpl.CLONE_EXCLUDE_PREFIX.length()));
+          if 
(entry.getKey().startsWith(TableOperationsImpl.PROPERTY_EXCLUDE_PREFIX)) {
+            propertiesToExclude.add(
+                
entry.getKey().substring(TableOperationsImpl.PROPERTY_EXCLUDE_PREFIX.length()));
             continue;
           }
 
diff --git 
a/test/src/main/java/org/apache/accumulo/test/functional/CloneTestIT.java 
b/test/src/main/java/org/apache/accumulo/test/functional/CloneTestIT.java
index e7bd4a2aa1..a533ec3f9a 100644
--- a/test/src/main/java/org/apache/accumulo/test/functional/CloneTestIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/functional/CloneTestIT.java
@@ -318,7 +318,7 @@ public class CloneTestIT extends AccumuloClusterHarness {
         bw.addMutations(mutations);
       }
 
-      client.tableOperations().clone(tables[0], tables[1], true, null, null);
+      client.tableOperations().clone(tables[0], tables[1], 
CloneConfiguration.empty());
 
       client.tableOperations().deleteRows(tables[1], new Text("4"), new 
Text("8"));
 
@@ -338,15 +338,15 @@ public class CloneTestIT extends AccumuloClusterHarness {
   public void testCloneRootTable() {
     try (AccumuloClient client = 
Accumulo.newClient().from(getClientProps()).build()) {
       assertThrows(AccumuloException.class,
-          () -> client.tableOperations().clone(RootTable.NAME, "rc1", true, 
null, null));
+          () -> client.tableOperations().clone(RootTable.NAME, "rc1", 
CloneConfiguration.empty()));
     }
   }
 
   @Test
   public void testCloneMetadataTable() {
     try (AccumuloClient client = 
Accumulo.newClient().from(getClientProps()).build()) {
-      assertThrows(AccumuloException.class,
-          () -> client.tableOperations().clone(MetadataTable.NAME, "mc1", 
true, null, null));
+      assertThrows(AccumuloException.class, () -> 
client.tableOperations().clone(MetadataTable.NAME,
+          "mc1", CloneConfiguration.empty()));
     }
   }
 }

Reply via email to