Migrate ArrayTransforms to use Values

Author: schmidt-sebastian

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/mutation/ArrayTransformOperation.java

@@ -18,8 +18,11 @@
 import com.google.firebase.Timestamp;
 import com.google.firebase.firestore.model.value.ArrayValue;
 import com.google.firebase.firestore.model.value.FieldValue;
+import com.google.firebase.firestore.model.value.ProtoValues;
+import com.google.firestore.v1.Value;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Iterator;
 import java.util.List;
 
 /**
@@ -28,13 +31,13 @@
  * <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;
   }
 
@@ -86,9 +89,9 @@ public int hashCode() {
    * 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.
    */
-  static List<FieldValue> coercedFieldValuesArray(@Nullable FieldValue value) {
+  static List<Value> coercedFieldValuesArray(@Nullable FieldValue value) {
     if (value instanceof ArrayValue) {
-      return ((ArrayValue) value).getValues();
+      return value.getProto().getArrayValue().getValuesList();
     } else {
       // coerce to empty array.
       return new ArrayList<>();
@@ -97,17 +100,23 @@ static List<FieldValue> coercedFieldValuesArray(@Nullable FieldValue value) {
 
   /** 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);
+      List<Value> result = coercedFieldValuesArray(previousValue);
+      for (Value unionElement : getElements()) {
+        boolean contains = false;
+        for (Value existingElement : result) {
+          if (ProtoValues.equals(unionElement, existingElement)) {
+            contains = true;
+            break;
+          }
+        }
+        if (!contains) {
+          result.add(unionElement);
         }
       }
       return ArrayValue.fromList(result);
@@ -116,15 +125,21 @@ protected ArrayValue apply(@Nullable FieldValue previousValue) {
 
   /** 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));
+      List<Value> result = coercedFieldValuesArray(previousValue);
+      for (Value removeElement : getElements()) {
+        Iterator<Value> existingValuesIterator = result.iterator();
+        while (existingValuesIterator.hasNext()) {
+          Value existingElement = existingValuesIterator.next();
+          if (ProtoValues.equals(existingElement, removeElement)) {
+            existingValuesIterator.remove();
+          }
+        }
       }
       return ArrayValue.fromList(result);
     }

firebase-firestore/src/main/java/com/google/firebase/firestore/model/value/ArrayValue.java

@@ -24,11 +24,11 @@ public class ArrayValue extends FieldValue {
     super(value);
   }
 
-  public static ArrayValue fromList(List<FieldValue> list) {
+  public static ArrayValue fromList(List<Value> list) {
     com.google.firestore.v1.ArrayValue.Builder builder =
         com.google.firestore.v1.ArrayValue.newBuilder();
-    for (FieldValue value : list) {
-      builder.addValues(value.getProto());
+    for (Value value : list) {
+      builder.addValues(value);
     }
     return new ArrayValue(Value.newBuilder().setArrayValue(builder).build());
   }

firebase-firestore/src/test/java/com/google/firebase/firestore/UserDataWriterTest.java

@@ -17,6 +17,7 @@
 import static com.google.firebase.firestore.testutil.TestUtil.blob;
 import static com.google.firebase.firestore.testutil.TestUtil.map;
 import static com.google.firebase.firestore.testutil.TestUtil.ref;
+import static com.google.firebase.firestore.testutil.TestUtil.valueOf;
 import static com.google.firebase.firestore.testutil.TestUtil.wrap;
 import static com.google.firebase.firestore.testutil.TestUtil.wrapObject;
 import static java.util.Arrays.asList;
@@ -270,8 +271,7 @@ public void testConvertsNestedObjects() {
 
   @Test
   public void testConvertsLists() {
-    ArrayValue expected =
-        ArrayValue.fromList(asList(StringValue.valueOf("value"), BooleanValue.valueOf(true)));
+    ArrayValue expected = ArrayValue.fromList(asList(valueOf("value"), valueOf(true)));
     FieldValue actual = wrap(asList("value", true));
     assertEquals(expected, actual);
   }

firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java

@@ -21,6 +21,7 @@
 import static com.google.firebase.firestore.testutil.TestUtil.key;
 import static com.google.firebase.firestore.testutil.TestUtil.map;
 import static com.google.firebase.firestore.testutil.TestUtil.ref;
+import static com.google.firebase.firestore.testutil.TestUtil.valueOf;
 import static com.google.firebase.firestore.testutil.TestUtil.wrap;
 import static com.google.firebase.firestore.testutil.TestUtil.wrapObject;
 import static junit.framework.TestCase.assertTrue;
@@ -128,12 +129,7 @@ public void testAddsNewFields() {
   public void testAddsMultipleNewFields() {
     ObjectValue object = ObjectValue.emptyObject();
     object = setField(object, "a", wrap("a"));
-    object =
-        object
-            .toBuilder()
-            .set(field("b"), wrap("b").getProto())
-            .set(field("c"), wrap("c").getProto())
-            .build();
+    object = object.toBuilder().set(field("b"), valueOf("b")).set(field("c"), valueOf("c")).build();
 
     assertEquals(wrapObject("a", "a", "b", "b", "c", "c"), object);
   }

firebase-firestore/src/test/java/com/google/firebase/firestore/model/ObjectValueBuilderTest.java

@@ -16,7 +16,7 @@
 
 import static com.google.firebase.firestore.testutil.TestUtil.field;
 import static com.google.firebase.firestore.testutil.TestUtil.map;
-import static com.google.firebase.firestore.testutil.TestUtil.wrap;
+import static com.google.firebase.firestore.testutil.TestUtil.valueOf;
 import static com.google.firebase.firestore.testutil.TestUtil.wrapObject;
 import static junit.framework.TestCase.assertEquals;
 
@@ -31,9 +31,9 @@
 @Config(manifest = Config.NONE)
 public class ObjectValueBuilderTest {
   private String fooString = "foo";
-  private Value fooValue = wrap(fooString).getProto();
+  private Value fooValue = valueOf(fooString);
   private String barString = "bar";
-  private Value barValue = wrap(barString).getProto();
+  private Value barValue = valueOf(barString);
   private Value emptyObject = ObjectValue.emptyObject().getProto();
 
   @Test

firebase-firestore/src/test/java/com/google/firebase/firestore/remote/RemoteSerializerTest.java

@@ -27,6 +27,7 @@
 import static com.google.firebase.firestore.testutil.TestUtil.ref;
 import static com.google.firebase.firestore.testutil.TestUtil.setMutation;
 import static com.google.firebase.firestore.testutil.TestUtil.transformMutation;
+import static com.google.firebase.firestore.testutil.TestUtil.valueOf;
 import static com.google.firebase.firestore.testutil.TestUtil.verifyMutation;
 import static com.google.firebase.firestore.testutil.TestUtil.wrap;
 import static java.util.Arrays.asList;
@@ -435,8 +436,8 @@ public void testEncodesArrayTransformMutations() {
                             .setFieldPath("a")
                             .setAppendMissingElements(
                                 ArrayValue.newBuilder()
-                                    .addValues(wrap("a").getProto())
-                                    .addValues(wrap(2).getProto())))
+                                    .addValues(valueOf("a"))
+                                    .addValues(valueOf(2))))
                     .addFieldTransforms(
                         DocumentTransform.FieldTransform.newBuilder()
                             .setFieldPath("bar.baz")

firebase-firestore/src/testUtil/java/com/google/firebase/firestore/testutil/TestUtil.java

@@ -70,6 +70,7 @@
 import com.google.firebase.firestore.remote.WatchChange;
 import com.google.firebase.firestore.remote.WatchChange.DocumentChange;
 import com.google.firebase.firestore.remote.WatchChangeAggregator;
+import com.google.firestore.v1.Value;
 import com.google.protobuf.ByteString;
 import java.io.IOException;
 import java.util.ArrayList;
@@ -143,6 +144,10 @@ public static ObjectValue wrapObject(Object... entries) {
     return wrapObject(map(entries));
   }
 
+  public static Value valueOf(Object value) {
+    return wrap(value).getProto();
+  }
+
   public static DocumentKey key(String key) {
     return DocumentKey.fromPathString(key);
   }