chore(spanner): review comments

Author: harshachinta

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Mutation.java

@@ -404,13 +404,17 @@ private boolean isFloat32NaN(Value value) {
     return value.getType().equals(Type.float32()) && Float.isNaN(value.getFloat32());
   }
 
-  // Converts Mutation and returns a random Mutation from the available mutation list based on the
-  // following heuristics:
-  // 1. Prefer mutations other than INSERT, since INSERT mutations may contain autogenerated columns
-  //    whose information is unavailable on the client.
-  // 2. If the list only contains INSERT mutations, select the one with the highest number of
-  // values.
-  static com.google.spanner.v1.Mutation toProtoGetRandomMutation(
+  /**
+   * Converts the list of mutations to the corresponding protobuf mutations and returns a random
+   * mutation from the available list based on the following heuristics:
+   *
+   * <p>1. Prefer mutations other than INSERT, as INSERT mutations may contain autogenerated columns
+   * whose information is unavailable on the client.
+   *
+   * <p>2. If the list only contains INSERT mutations, select the one with the highest number of
+   * values.
+   */
+  static com.google.spanner.v1.Mutation toProtoAndReturnRandomMutation(
       Iterable<Mutation> mutations, List<com.google.spanner.v1.Mutation> out) {
     Mutation last = null;
     // The mutation currently being built.
@@ -419,9 +423,9 @@ static com.google.spanner.v1.Mutation toProtoGetRandomMutation(
     com.google.spanner.v1.Mutation.Write.Builder write = null;
     com.google.spanner.v1.KeySet.Builder keySet = null;
 
-    // Store all the mutations excluding INSERT.
+    // Stores all the mutations excluding INSERT mutations.
     List<com.google.spanner.v1.Mutation> allMutationsExcludingInsert = new ArrayList<>();
-    // Stores INSERT mutation with large number of values.
+    // Stores the INSERT mutation with largest number of values.
     com.google.spanner.v1.Mutation largeInsertMutation =
         com.google.spanner.v1.Mutation.getDefaultInstance();
 
@@ -505,9 +509,9 @@ static com.google.spanner.v1.Mutation toProtoGetRandomMutation(
     }
 
     // Select a random mutation based on the heuristic.
-    if (allMutationsExcludingInsert.size() > 0) {
-      int randomIndex = ThreadLocalRandom.current().nextInt(allMutationsExcludingInsert.size());
-      return allMutationsExcludingInsert.get(randomIndex);
+    if (!allMutationsExcludingInsert.isEmpty()) {
+      return allMutationsExcludingInsert.get(
+          ThreadLocalRandom.current().nextInt(allMutationsExcludingInsert.size()));
     } else {
       return largeInsertMutation;
     }

google-cloud-spanner/src/main/java/com/google/cloud/spanner/MutationGroup.java

@@ -48,7 +48,7 @@ public ImmutableList<Mutation> getMutations() {
 
   static BatchWriteRequest.MutationGroup toProto(final MutationGroup mutationGroup) {
     List<com.google.spanner.v1.Mutation> mutationsProto = new ArrayList<>();
-    Mutation.toProtoGetRandomMutation(mutationGroup.getMutations(), mutationsProto);
+    Mutation.toProtoAndReturnRandomMutation(mutationGroup.getMutations(), mutationsProto);
     return BatchWriteRequest.MutationGroup.newBuilder().addAllMutations(mutationsProto).build();
   }
 

google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java

@@ -238,7 +238,7 @@ public CommitResponse writeAtLeastOnceWithOptions(
       throws SpannerException {
     setActive(null);
     List<com.google.spanner.v1.Mutation> mutationsProto = new ArrayList<>();
-    Mutation.toProtoGetRandomMutation(mutations, mutationsProto);
+    Mutation.toProtoAndReturnRandomMutation(mutations, mutationsProto);
     Options options = Options.fromTransactionOptions(transactionOptions);
     final CommitRequest.Builder requestBuilder =
         CommitRequest.newBuilder()

google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java

@@ -373,7 +373,7 @@ ApiFuture<CommitResponse> commitAsync() {
         }
         committing = true;
         if (!mutations.isEmpty()) {
-          randomMutation = Mutation.toProtoGetRandomMutation(mutations, mutationsProto);
+          randomMutation = Mutation.toProtoAndReturnRandomMutation(mutations, mutationsProto);
         }
       }
       final SettableApiFuture<CommitResponse> res = SettableApiFuture.create();

google-cloud-spanner/src/test/java/com/google/cloud/spanner/MutationGroupTest.java

@@ -44,7 +44,7 @@ private Mutation getRandomMutation() {
 
   private BatchWriteRequest.MutationGroup getMutationGroupProto(ImmutableList<Mutation> mutations) {
     List<com.google.spanner.v1.Mutation> mutationsProto = new ArrayList<>();
-    Mutation.toProtoGetRandomMutation(mutations, mutationsProto);
+    Mutation.toProtoAndReturnRandomMutation(mutations, mutationsProto);
     return BatchWriteRequest.MutationGroup.newBuilder().addAllMutations(mutationsProto).build();
   }
 

google-cloud-spanner/src/test/java/com/google/cloud/spanner/MutationTest.java

@@ -322,7 +322,7 @@ public void serializationBasic() {
         com.google.spanner.v1.Mutation.getDefaultInstance();
     proto.add(existingProto);
 
-    Mutation.toProtoGetRandomMutation(mutations, proto);
+    Mutation.toProtoAndReturnRandomMutation(mutations, proto);
 
     assertThat(proto.size()).isAtLeast(1);
     assertThat(proto.get(0)).isSameInstanceAs(existingProto);
@@ -360,7 +360,8 @@ public void toProtoCoalescingChangeOfTable() {
             Mutation.newInsertBuilder("T2").set("C").to("V5").build());
 
     List<com.google.spanner.v1.Mutation> proto = new ArrayList<>();
-    com.google.spanner.v1.Mutation mutation = Mutation.toProtoGetRandomMutation(mutations, proto);
+    com.google.spanner.v1.Mutation mutation =
+        Mutation.toProtoAndReturnRandomMutation(mutations, proto);
     // Random mutation returned should be INSERT with large number of values
     MatcherAssert.assertThat(
         mutation,
@@ -394,7 +395,8 @@ public void toProtoCoalescingChangeOfOperation() {
             Mutation.newUpdateBuilder("T").set("C").to("V5").build());
 
     List<com.google.spanner.v1.Mutation> proto = new ArrayList<>();
-    com.google.spanner.v1.Mutation mutation = Mutation.toProtoGetRandomMutation(mutations, proto);
+    com.google.spanner.v1.Mutation mutation =
+        Mutation.toProtoAndReturnRandomMutation(mutations, proto);
     // Random mutation returned should be of UPDATE operation
     MatcherAssert.assertThat(
         mutation,
@@ -427,7 +429,8 @@ public void toProtoCoalescingChangeOfColumn() {
             Mutation.newInsertBuilder("T").set("C2").to("V5").build());
 
     List<com.google.spanner.v1.Mutation> proto = new ArrayList<>();
-    com.google.spanner.v1.Mutation mutation = Mutation.toProtoGetRandomMutation(mutations, proto);
+    com.google.spanner.v1.Mutation mutation =
+        Mutation.toProtoAndReturnRandomMutation(mutations, proto);
     MatcherAssert.assertThat(
         mutation,
         matchesProto(
@@ -459,7 +462,8 @@ public void toProtoCoalescingDelete() {
             Mutation.delete("T", KeySet.range(KeyRange.closedClosed(Key.of("kc"), Key.of("kd")))));
 
     List<com.google.spanner.v1.Mutation> proto = new ArrayList<>();
-    com.google.spanner.v1.Mutation mutation = Mutation.toProtoGetRandomMutation(mutations, proto);
+    com.google.spanner.v1.Mutation mutation =
+        Mutation.toProtoAndReturnRandomMutation(mutations, proto);
     // Random mutation returned should be of DELETE operation
     assertTrue(mutation.hasDelete());
 
@@ -492,7 +496,8 @@ public void toProtoCoalescingDeleteChanges() {
             Mutation.newInsertBuilder("T2").set("C").to("V1").build());
 
     List<com.google.spanner.v1.Mutation> proto = new ArrayList<>();
-    com.google.spanner.v1.Mutation mutation = Mutation.toProtoGetRandomMutation(mutations, proto);
+    com.google.spanner.v1.Mutation mutation =
+        Mutation.toProtoAndReturnRandomMutation(mutations, proto);
     assertTrue(mutation.hasDelete());
 
     assertThat(proto.size()).isEqualTo(4);

google-cloud-spanner/src/test/java/com/google/cloud/spanner/PgNumericTest.java

@@ -319,7 +319,7 @@ public void testMutation() {
                 .toStringArray(null)
                 .build());
     final List<com.google.spanner.v1.Mutation> expectedMutations = new ArrayList<>();
-    Mutation.toProtoGetRandomMutation(mutations, expectedMutations);
+    Mutation.toProtoAndReturnRandomMutation(mutations, expectedMutations);
 
     databaseClient
         .readWriteTransaction()