Backport to branch(3) : Refactor operations and their builders (#2332)

Co-authored-by: Toshihiro Suzuki <[email protected]>

Author: feeblefakie

core/src/main/java/com/scalar/db/api/Delete.java

@@ -8,6 +8,7 @@
 import com.scalar.db.io.Key;
 import java.util.Optional;
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import javax.annotation.concurrent.NotThreadSafe;
 
 /**
@@ -18,6 +19,16 @@
 @NotThreadSafe
 public class Delete extends Mutation {
 
+  Delete(
+      @Nullable String namespace,
+      String tableName,
+      Key partitionKey,
+      @Nullable Key clusteringKey,
+      @Nullable Consistency consistency,
+      @Nullable MutationCondition condition) {
+    super(namespace, tableName, partitionKey, clusteringKey, consistency, condition);
+  }
+
   /**
    * Constructs a {@code Delete} with the specified partition {@code Key}.
    *
@@ -69,7 +80,7 @@ public static Namespace newBuilder() {
 
   /**
    * Build a {@code Delete} operation from an existing {@code Delete} object using a builder. The
-   * builder will be parametrized by default with all the existing {@code Delete} attributes
+   * builder will be parametrized by default with all the existing {@code Delete} parameters.
    *
    * @param delete an existing {@code Delete} operation
    * @return a {@code Delete} operation builder

core/src/main/java/com/scalar/db/api/DeleteBuilder.java

@@ -92,16 +92,8 @@ public Buildable consistency(com.scalar.db.api.Consistency consistency) {
 
     @Override
     public Delete build() {
-      Delete delete = new Delete(partitionKey, clusteringKey);
-      delete.forNamespace(namespaceName).forTable(tableName);
-      if (condition != null) {
-        delete.withCondition(condition);
-      }
-      if (consistency != null) {
-        delete.withConsistency(consistency);
-      }
-
-      return delete;
+      return new Delete(
+          namespaceName, tableName, partitionKey, clusteringKey, consistency, condition);
     }
   }
 

core/src/main/java/com/scalar/db/api/Get.java

@@ -3,10 +3,13 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.common.base.MoreObjects;
+import com.google.common.collect.ImmutableSet;
 import com.scalar.db.api.GetBuilder.BuildableGetOrGetWithIndexFromExisting;
 import com.scalar.db.api.GetBuilder.Namespace;
 import com.scalar.db.io.Key;
 import java.util.Collection;
+import java.util.List;
+import javax.annotation.Nullable;
 import javax.annotation.concurrent.NotThreadSafe;
 
 /**
@@ -17,6 +20,18 @@
 @NotThreadSafe
 public class Get extends Selection {
 
+  Get(
+      @Nullable String namespace,
+      String tableName,
+      Key partitionKey,
+      @Nullable Key clusteringKey,
+      @Nullable Consistency consistency,
+      List<String> projections,
+      ImmutableSet<Conjunction> conjunctions) {
+    super(
+        namespace, tableName, partitionKey, clusteringKey, consistency, projections, conjunctions);
+  }
+
   /**
    * Constructs a {@code Get} with the specified partition {@code Key}.
    *
@@ -68,7 +83,7 @@ public static Namespace newBuilder() {
 
   /**
    * Build a {@code Get} operation from an existing {@code Get} object using a builder. The builder
-   * will be parametrized by default with all the existing {@code Get} attributes
+   * will be parametrized by default with all the existing {@code Get} parameters.
    *
    * @param get an existing {@code Get} operation
    * @return a {@code Get} operation builder
@@ -133,11 +148,6 @@ public Get withProjections(Collection<String> projections) {
     return (Get) super.withProjections(projections);
   }
 
-  @Override
-  Get withConjunctions(Collection<Conjunction> conjunctions) {
-    return (Get) super.withConjunctions(conjunctions);
-  }
-
   /**
    * Indicates whether some other object is "equal to" this object. The other object is considered
    * equal if:

core/src/main/java/com/scalar/db/api/GetBuilder.java

@@ -2,6 +2,7 @@
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
+import com.google.common.collect.ImmutableSet;
 import com.scalar.db.api.OperationBuilder.And;
 import com.scalar.db.api.OperationBuilder.Buildable;
 import com.scalar.db.api.OperationBuilder.ClearClusteringKey;
@@ -135,15 +136,18 @@ public BuildableGet consistency(com.scalar.db.api.Consistency consistency) {
 
     @Override
     public Get build() {
-      Get get = new Get(partitionKey, clusteringKey);
-      get.forNamespace(namespaceName).forTable(tableName);
-      if (!projections.isEmpty()) {
-        get.withProjections(projections);
-      }
-      if (consistency != null) {
-        get.withConsistency(consistency);
-      }
-      return get;
+      return build(ImmutableSet.of());
+    }
+
+    private Get build(ImmutableSet<Conjunction> conjunctions) {
+      return new Get(
+          namespaceName,
+          tableName,
+          partitionKey,
+          clusteringKey,
+          consistency,
+          projections,
+          conjunctions);
     }
   }
 
@@ -341,7 +345,7 @@ private BuildableGetWithWhere(BuildableGetWithOngoingWhere buildable) {
 
     @Override
     public Get build() {
-      return (Get) addConjunctionsTo(super.build(), where);
+      return super.build(getConjunctions(where));
     }
   }
 
@@ -420,15 +424,12 @@ public BuildableGetWithIndexOngoingWhereOr whereOr(Set<AndConditionSet> andCondi
     }
 
     public Get build() {
-      GetWithIndex getWithIndex = new GetWithIndex(indexKey);
-      getWithIndex.forNamespace(namespaceName).forTable(tableName);
-      if (!projections.isEmpty()) {
-        getWithIndex.withProjections(projections);
-      }
-      if (consistency != null) {
-        getWithIndex.withConsistency(consistency);
-      }
-      return getWithIndex;
+      return build(ImmutableSet.of());
+    }
+
+    private Get build(ImmutableSet<Conjunction> conjunctions) {
+      return new GetWithIndex(
+          namespaceName, tableName, indexKey, consistency, projections, conjunctions);
     }
   }
 
@@ -583,7 +584,7 @@ public BuildableGetWithIndexWhere consistency(com.scalar.db.api.Consistency cons
     }
 
     public Get build() {
-      return (Get) addConjunctionsTo(buildableGetWithIndex.build(), where);
+      return buildableGetWithIndex.build(getConjunctions(where));
     }
   }
 
@@ -771,28 +772,24 @@ private void checkConditionsEmpty() {
 
     @Override
     public Get build() {
-      Get get;
+      return build(
+          conjunctions.stream().map(Conjunction::of).collect(ImmutableSet.toImmutableSet()));
+    }
 
+    private Get build(ImmutableSet<Conjunction> conjunctions) {
       if (isGetWithIndex) {
-        get = new GetWithIndex(indexKey);
+        return new GetWithIndex(
+            namespaceName, tableName, indexKey, consistency, projections, conjunctions);
       } else {
-        get = new Get(partitionKey, clusteringKey);
-      }
-
-      if (!conjunctions.isEmpty()) {
-        get.withConjunctions(
-            conjunctions.stream().map(Conjunction::of).collect(Collectors.toSet()));
+        return new Get(
+            namespaceName,
+            tableName,
+            partitionKey,
+            clusteringKey,
+            consistency,
+            projections,
+            conjunctions);
       }
-
-      get.forNamespace(namespaceName).forTable(tableName);
-      if (!projections.isEmpty()) {
-        get.withProjections(projections);
-      }
-      if (consistency != null) {
-        get.withConsistency(consistency);
-      }
-
-      return get;
     }
   }
 
@@ -893,7 +890,7 @@ public BuildableGetFromExistingWithWhere clearNamespace() {
     }
 
     public Get build() {
-      return (Get) addConjunctionsTo(BuildableGetFromExisting.build(), where);
+      return BuildableGetFromExisting.build(getConjunctions(where));
     }
   }
 

core/src/main/java/com/scalar/db/api/GetWithIndex.java

@@ -1,14 +1,27 @@
 package com.scalar.db.api;
 
+import com.google.common.collect.ImmutableSet;
 import com.scalar.db.io.Key;
 import java.util.Collection;
+import java.util.List;
 import java.util.Objects;
+import javax.annotation.Nullable;
 import javax.annotation.concurrent.NotThreadSafe;
 
 /** A command to retrieve an entry from the underlying storage by using an index. */
 @NotThreadSafe
 public class GetWithIndex extends Get {
 
+  GetWithIndex(
+      @Nullable String namespace,
+      String tableName,
+      Key indexKey,
+      @Nullable Consistency consistency,
+      List<String> projections,
+      ImmutableSet<Conjunction> conjunctions) {
+    super(namespace, tableName, indexKey, null, consistency, projections, conjunctions);
+  }
+
   /**
    * Constructs an {@code GetWithIndex} with the specified index {@code Key}.
    *

core/src/main/java/com/scalar/db/api/Insert.java

@@ -17,16 +17,16 @@
 @NotThreadSafe
 public class Insert extends Mutation {
 
-  private final Map<String, Column<?>> columns;
+  private final ImmutableMap<String, Column<?>> columns;
 
   Insert(
       @Nullable String namespace,
       String tableName,
       Key partitionKey,
       @Nullable Key clusteringKey,
-      Map<String, Column<?>> columns) {
-    super(namespace, tableName, partitionKey, clusteringKey, null);
-    this.columns = ImmutableMap.copyOf(columns);
+      ImmutableMap<String, Column<?>> columns) {
+    super(namespace, tableName, partitionKey, clusteringKey, null, null);
+    this.columns = columns;
   }
 
   public Map<String, Column<?>> getColumns() {
@@ -120,7 +120,7 @@ public static InsertBuilder.Namespace newBuilder() {
 
   /**
    * Build a {@code Insert} operation from an existing {@code Insert} object using a builder. The
-   * builder will be parametrized by default with all the existing {@code Insert} object attributes.
+   * builder will be parametrized by default with all the existing {@code Insert} parameters.
    *
    * @param insert an existing {@code Insert} operation
    * @return a {@code Insert} operation builder

core/src/main/java/com/scalar/db/api/InsertBuilder.java

@@ -2,6 +2,7 @@
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
+import com.google.common.collect.ImmutableMap;
 import com.scalar.db.api.OperationBuilder.ClearClusteringKey;
 import com.scalar.db.api.OperationBuilder.ClearNamespace;
 import com.scalar.db.api.OperationBuilder.ClearValues;
@@ -186,7 +187,8 @@ public Buildable value(Column<?> column) {
 
     @Override
     public Insert build() {
-      return new Insert(namespaceName, tableName, partitionKey, clusteringKey, columns);
+      return new Insert(
+          namespaceName, tableName, partitionKey, clusteringKey, ImmutableMap.copyOf(columns));
     }
   }
 

core/src/main/java/com/scalar/db/api/Mutation.java

@@ -18,7 +18,18 @@
 public abstract class Mutation extends Operation {
 
   /** @deprecated As of release 3.13.0. Will be removed in release 5.0.0. */
-  @Deprecated private Optional<MutationCondition> condition;
+  @Deprecated @Nullable private MutationCondition condition;
+
+  Mutation(
+      @Nullable String namespace,
+      String tableName,
+      Key partitionKey,
+      @Nullable Key clusteringKey,
+      @Nullable Consistency consistency,
+      @Nullable MutationCondition condition) {
+    super(namespace, tableName, partitionKey, clusteringKey, consistency);
+    this.condition = condition;
+  }
 
   /**
    * @param partitionKey a partition key
@@ -28,7 +39,7 @@ public abstract class Mutation extends Operation {
   @Deprecated
   public Mutation(Key partitionKey, Key clusteringKey) {
     super(partitionKey, clusteringKey);
-    condition = Optional.empty();
+    condition = null;
   }
 
   /**
@@ -48,7 +59,7 @@ public Mutation(Mutation mutation) {
       @Nullable Key clusteringKey,
       @Nullable MutationCondition condition) {
     super(namespace, tableName, partitionKey, clusteringKey);
-    this.condition = Optional.ofNullable(condition);
+    this.condition = condition;
   }
 
   /**
@@ -60,7 +71,7 @@ public Mutation(Mutation mutation) {
   @Deprecated
   @Nonnull
   public Optional<MutationCondition> getCondition() {
-    return condition;
+    return Optional.ofNullable(condition);
   }
 
   /**
@@ -72,7 +83,7 @@ public Optional<MutationCondition> getCondition() {
    */
   @Deprecated
   public Mutation withCondition(MutationCondition condition) {
-    this.condition = Optional.ofNullable(condition);
+    this.condition = condition;
     return this;
   }
 
@@ -101,7 +112,7 @@ public boolean equals(Object o) {
       return false;
     }
     Mutation other = (Mutation) o;
-    return condition.equals(other.condition);
+    return Objects.equals(condition, other.condition);
   }
 
   @Override