Migrate transforms to use Values (#1205)

Author: schmidt-sebastian

firebase-firestore/ktx/src/test/kotlin/com/google/firebase/firestore/ktx/FirestoreTests.kt

@@ -22,8 +22,8 @@ import com.google.firebase.firestore.FieldPath
 import com.google.firebase.firestore.FirebaseFirestore
 import com.google.firebase.firestore.FirebaseFirestoreSettings
 import com.google.firebase.firestore.TestUtil
-import com.google.firebase.firestore.model.value.IntegerValue
 import com.google.firebase.firestore.model.value.ObjectValue
+import com.google.firebase.firestore.testutil.TestUtil.wrap
 import com.google.firebase.ktx.Firebase
 import com.google.firebase.ktx.app
 import com.google.firebase.ktx.initialize
@@ -171,7 +171,7 @@ class QuerySnapshotTests {
         val qs = TestUtil.querySnapshot(
                 "rooms",
                 mapOf(),
-                mapOf("id" to ObjectValue.fromMap(mapOf("a" to IntegerValue.valueOf(1).proto, "b" to IntegerValue.valueOf(2).proto))),
+                mapOf("id" to ObjectValue.fromMap(mapOf("a" to wrap(1).proto, "b" to wrap(2).proto))),
                 false,
                 false)
 

firebase-firestore/src/main/java/com/google/firebase/firestore/Query.java

@@ -45,6 +45,7 @@
 import com.google.firebase.firestore.model.value.ServerTimestampValue;
 import com.google.firebase.firestore.util.Executors;
 import com.google.firebase.firestore.util.Util;
+import com.google.firestore.v1.Value;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -336,9 +337,9 @@ private Query whereHelper(@NonNull FieldPath fieldPath, Operator op, Object valu
                 + "' queries on FieldPath.documentId().");
       } else if (op == Operator.IN) {
         validateDisjunctiveFilterElements(value, op);
-        List<FieldValue> referenceList = new ArrayList<>();
+        List<Value> referenceList = new ArrayList<>();
         for (Object arrayValue : (List) value) {
-          referenceList.add(parseDocumentIdValue(arrayValue));
+          referenceList.add(parseDocumentIdValue(arrayValue).getProto());
         }
         fieldValue = ArrayValue.fromList(referenceList);
       } else {

firebase-firestore/src/main/java/com/google/firebase/firestore/UserDataReader.java

@@ -357,13 +357,13 @@ private void parseSentinelFieldValue(
       context.addToFieldTransforms(context.getPath(), ServerTimestampOperation.getInstance());
 
     } else if (value instanceof ArrayUnionFieldValue) {
-      List<FieldValue> parsedElements =
+      List<Value> parsedElements =
           parseArrayTransformElements(((ArrayUnionFieldValue) value).getElements());
       ArrayTransformOperation arrayUnion = new ArrayTransformOperation.Union(parsedElements);
       context.addToFieldTransforms(context.getPath(), arrayUnion);
 
     } else if (value instanceof ArrayRemoveFieldValue) {
-      List<FieldValue> parsedElements =
+      List<Value> parsedElements =
           parseArrayTransformElements(((ArrayRemoveFieldValue) value).getElements());
       ArrayTransformOperation arrayRemove = new ArrayTransformOperation.Remove(parsedElements);
       context.addToFieldTransforms(context.getPath(), arrayRemove);
@@ -464,17 +464,17 @@ private Value parseTimestamp(Timestamp timestamp) {
         .build();
   }
 
-  private List<FieldValue> parseArrayTransformElements(List<Object> elements) {
+  private List<Value> parseArrayTransformElements(List<Object> elements) {
     ParseAccumulator accumulator = new ParseAccumulator(UserData.Source.Argument);
 
-    ArrayList<FieldValue> result = new ArrayList<>(elements.size());
+    List<Value> result = new ArrayList<>(elements.size());
     for (int i = 0; i < elements.size(); i++) {
       Object element = elements.get(i);
       // Although array transforms are used with writes, the actual elements
       // being unioned or removed are not considered writes since they cannot
       // contain any FieldValue sentinels, etc.
       ParseContext context = accumulator.rootContext();
-      result.add(FieldValue.valueOf(convertAndParseFieldData(element, context.childContext(i))));
+      result.add(convertAndParseFieldData(element, context.childContext(i)));
     }
     return result;
   }

firebase-firestore/src/main/java/com/google/firebase/firestore/model/Document.java

@@ -18,6 +18,7 @@
 import androidx.annotation.Nullable;
 import com.google.firebase.firestore.model.value.FieldValue;
 import com.google.firebase.firestore.model.value.ObjectValue;
+import com.google.firestore.v1.Value;
 import java.util.Comparator;
 
 /**
@@ -66,6 +67,11 @@ public ObjectValue getData() {
     return objectValue.get(path);
   }
 
+  public @Nullable Value getFieldProto(FieldPath path) {
+    FieldValue fieldValue = getField(path);
+    return fieldValue != null ? fieldValue.getProto() : null;
+  }
+
   public boolean hasLocalMutations() {
     return documentState.equals(DocumentState.LOCAL_MUTATIONS);
   }

firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/ArrayTransformOperation.java

@@ -16,9 +16,9 @@
 
 import androidx.annotation.Nullable;
 import com.google.firebase.Timestamp;
-import com.google.firebase.firestore.model.value.ArrayValue;
-import com.google.firebase.firestore.model.value.FieldValue;
-import java.util.ArrayList;
+import com.google.firebase.firestore.model.value.ProtoValues;
+import com.google.firestore.v1.ArrayValue;
+import com.google.firestore.v1.Value;
 import java.util.Collections;
 import java.util.List;
 
@@ -28,32 +28,31 @@
  * <p>Implementations are: ArrayTransformOperation.Union and ArrayTransformOperation.Remove
  */
 public abstract class ArrayTransformOperation implements TransformOperation {
-  private final List<FieldValue> elements;
+  private final List<Value> elements;
 
-  ArrayTransformOperation(List<FieldValue> elements) {
+  ArrayTransformOperation(List<Value> elements) {
     this.elements = Collections.unmodifiableList(elements);
   }
 
-  public List<FieldValue> getElements() {
+  public List<Value> getElements() {
     return elements;
   }
 
   @Override
-  public FieldValue applyToLocalView(@Nullable FieldValue previousValue, Timestamp localWriteTime) {
+  public Value applyToLocalView(@Nullable Value previousValue, Timestamp localWriteTime) {
     return apply(previousValue);
   }
 
   @Override
-  public FieldValue applyToRemoteDocument(
-      @Nullable FieldValue previousValue, FieldValue transformResult) {
+  public Value applyToRemoteDocument(@Nullable Value previousValue, Value transformResult) {
     // The server just sends null as the transform result for array operations, so we have to
     // calculate a result the same as we do for local applications.
     return apply(previousValue);
   }
 
   @Override
   @Nullable
-  public FieldValue computeBaseValue(@Nullable FieldValue currentValue) {
+  public Value computeBaseValue(@Nullable Value currentValue) {
     return null; // Array transforms are idempotent and don't require a base value.
   }
 
@@ -80,53 +79,58 @@ public int hashCode() {
   }
 
   /** Applies this ArrayTransformOperation against the specified previousValue. */
-  protected abstract ArrayValue apply(@Nullable FieldValue previousValue);
+  protected abstract Value apply(@Nullable Value previousValue);
 
   /**
-   * Inspects the provided value, returning an ArrayList copy of the internal array if it's an
-   * ArrayValue and an empty ArrayList if it's null or any other type of FSTFieldValue.
+   * Inspects the provided value, returning an ArrayValue.Builder containing the existing array
+   * elements or an empty builder if `value` is not an array.
    */
-  static List<FieldValue> coercedFieldValuesArray(@Nullable FieldValue value) {
-    if (value instanceof ArrayValue) {
-      return ((ArrayValue) value).getValues();
+  static ArrayValue.Builder coercedFieldValuesArray(@Nullable Value value) {
+    if (ProtoValues.isArray(value)) {
+      return value.getArrayValue().toBuilder();
     } else {
       // coerce to empty array.
-      return new ArrayList<>();
+      return ArrayValue.newBuilder();
     }
   }
 
   /** An array union transform operation. */
   public static class Union extends ArrayTransformOperation {
-    // TODO(mrschmidt): Migrate to list of Value protos
-    public Union(List<FieldValue> elements) {
+    public Union(List<Value> elements) {
       super(elements);
     }
 
     @Override
-    protected ArrayValue apply(@Nullable FieldValue previousValue) {
-      List<FieldValue> result = coercedFieldValuesArray(previousValue);
-      for (FieldValue element : getElements()) {
-        if (!result.contains(element)) {
-          result.add(element);
+    protected Value apply(@Nullable Value previousValue) {
+      ArrayValue.Builder result = coercedFieldValuesArray(previousValue);
+      for (Value unionElement : getElements()) {
+        if (!ProtoValues.contains(result.getValuesList(), unionElement)) {
+          result.addValues(unionElement);
         }
       }
-      return ArrayValue.fromList(result);
+      return Value.newBuilder().setArrayValue(result).build();
     }
   }
 
   /** An array remove transform operation. */
   public static class Remove extends ArrayTransformOperation {
-    public Remove(List<FieldValue> elements) {
+    public Remove(List<Value> elements) {
       super(elements);
     }
 
     @Override
-    protected ArrayValue apply(@Nullable FieldValue previousValue) {
-      List<FieldValue> result = coercedFieldValuesArray(previousValue);
-      for (FieldValue element : getElements()) {
-        result.removeAll(Collections.singleton(element));
+    protected Value apply(@Nullable Value previousValue) {
+      ArrayValue.Builder result = coercedFieldValuesArray(previousValue);
+      for (Value removeElement : getElements()) {
+        for (int i = 0; i < result.getValuesCount(); ) {
+          if (ProtoValues.equals(result.getValues(i), removeElement)) {
+            result.removeValues(i);
+          } else {
+            ++i;
+          }
+        }
       }
-      return ArrayValue.fromList(result);
+      return Value.newBuilder().setArrayValue(result).build();
     }
   }
 }

firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationResult.java

@@ -18,7 +18,7 @@
 
 import androidx.annotation.Nullable;
 import com.google.firebase.firestore.model.SnapshotVersion;
-import com.google.firebase.firestore.model.value.FieldValue;
+import com.google.firestore.v1.Value;
 import java.util.List;
 
 /**
@@ -31,9 +31,9 @@
  */
 public final class MutationResult {
   private final SnapshotVersion version;
-  @Nullable private final List<FieldValue> transformResults;
+  @Nullable private final List<Value> transformResults;
 
-  public MutationResult(SnapshotVersion version, @Nullable List<FieldValue> transformResults) {
+  public MutationResult(SnapshotVersion version, @Nullable List<Value> transformResults) {
     this.version = checkNotNull(version);
     this.transformResults = transformResults;
   }
@@ -56,12 +56,12 @@ public SnapshotVersion getVersion() {
 
   /**
    * The resulting fields returned from the backend after a TransformMutation has been committed.
-   * Contains one FieldValue for each FieldTransform that was in the mutation.
+   * Contains one Value for each FieldTransform that was in the mutation.
    *
    * <p>Will be null if the mutation was not a TransformMutation.
    */
   @Nullable
-  public List<FieldValue> getTransformResults() {
+  public List<Value> getTransformResults() {
     return transformResults;
   }
 }

firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/NumericIncrementTransformOperation.java

@@ -14,6 +14,8 @@
 
 package com.google.firebase.firestore.model.mutation;
 
+import static com.google.firebase.firestore.model.value.ProtoValues.isDouble;
+import static com.google.firebase.firestore.model.value.ProtoValues.isInteger;
 import static com.google.firebase.firestore.util.Assert.fail;
 import static com.google.firebase.firestore.util.Assert.hardAssert;
 
@@ -23,6 +25,8 @@
 import com.google.firebase.firestore.model.value.FieldValue;
 import com.google.firebase.firestore.model.value.IntegerValue;
 import com.google.firebase.firestore.model.value.NumberValue;
+import com.google.firebase.firestore.model.value.ProtoValues;
+import com.google.firestore.v1.Value;
 
 /**
  * Implements the backend semantics for locally computed NUMERIC_ADD (increment) transforms.
@@ -37,29 +41,28 @@ public NumericIncrementTransformOperation(NumberValue operand) {
   }
 
   @Override
-  public FieldValue applyToLocalView(@Nullable FieldValue previousValue, Timestamp localWriteTime) {
-    NumberValue baseValue = computeBaseValue(previousValue);
+  public Value applyToLocalView(@Nullable Value previousValue, Timestamp localWriteTime) {
+    Value baseValue = computeBaseValue(previousValue);
 
     // Return an integer value only if the previous value and the operand is an integer.
-    if (baseValue instanceof IntegerValue && operand instanceof IntegerValue) {
-      long sum = safeIncrement(((IntegerValue) baseValue).getIntegerValue(), operandAsLong());
-      return IntegerValue.valueOf(sum);
-    } else if (baseValue instanceof IntegerValue) {
-      double sum = ((IntegerValue) baseValue).getIntegerValue() + operandAsDouble();
-      return DoubleValue.valueOf(sum);
+    if (isInteger(baseValue) && operand instanceof IntegerValue) {
+      long sum = safeIncrement(baseValue.getIntegerValue(), operandAsLong());
+      return Value.newBuilder().setIntegerValue(sum).build();
+    } else if (isInteger(baseValue)) {
+      double sum = baseValue.getIntegerValue() + operandAsDouble();
+      return Value.newBuilder().setDoubleValue(sum).build();
     } else {
       hardAssert(
-          baseValue instanceof DoubleValue,
+          isDouble(baseValue),
           "Expected NumberValue to be of type DoubleValue, but was ",
           previousValue.getClass().getCanonicalName());
-      double sum = ((DoubleValue) baseValue).getDoubleValue() + operandAsDouble();
-      return DoubleValue.valueOf(sum);
+      double sum = baseValue.getDoubleValue() + operandAsDouble();
+      return Value.newBuilder().setDoubleValue(sum).build();
     }
   }
 
   @Override
-  public FieldValue applyToRemoteDocument(
-      @Nullable FieldValue previousValue, FieldValue transformResult) {
+  public Value applyToRemoteDocument(@Nullable Value previousValue, Value transformResult) {
     return transformResult;
   }
 
@@ -72,10 +75,10 @@ public FieldValue getOperand() {
    * otherwise returning a coerced IntegerValue of 0.
    */
   @Override
-  public NumberValue computeBaseValue(@Nullable FieldValue previousValue) {
-    return previousValue instanceof NumberValue
-        ? (NumberValue) previousValue
-        : IntegerValue.valueOf(0L);
+  public Value computeBaseValue(@Nullable Value previousValue) {
+    return ProtoValues.isNumber(previousValue)
+        ? previousValue
+        : Value.newBuilder().setIntegerValue(0).build();
   }
 
   /**

firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/ServerTimestampOperation.java

@@ -16,8 +16,8 @@
 
 import androidx.annotation.Nullable;
 import com.google.firebase.Timestamp;
-import com.google.firebase.firestore.model.value.FieldValue;
 import com.google.firebase.firestore.model.value.ServerTimestampValue;
+import com.google.firestore.v1.Value;
 
 /** Transforms a value into a server-generated timestamp. */
 public class ServerTimestampOperation implements TransformOperation {
@@ -30,19 +30,18 @@ public static ServerTimestampOperation getInstance() {
   }
 
   @Override
-  public FieldValue applyToLocalView(@Nullable FieldValue previousValue, Timestamp localWriteTime) {
-    return ServerTimestampValue.valueOf(localWriteTime, previousValue);
+  public Value applyToLocalView(@Nullable Value previousValue, Timestamp localWriteTime) {
+    return ServerTimestampValue.valueOf(localWriteTime, previousValue).getProto();
   }
 
   @Override
-  public FieldValue applyToRemoteDocument(
-      @Nullable FieldValue previousValue, FieldValue transformResult) {
+  public Value applyToRemoteDocument(@Nullable Value previousValue, Value transformResult) {
     return transformResult;
   }
 
   @Nullable
   @Override
-  public FieldValue computeBaseValue(@Nullable FieldValue currentValue) {
+  public Value computeBaseValue(@Nullable Value currentValue) {
     return null; // Server timestamps are idempotent and don't require a base value.
   }