Make FieldValue a test-only class

firebase-firestore/src/main/java/com/google/firebase/firestore/model/value/FieldValue.java → firebase-firestore/ktx/src/test/java/com/google/firebase/firestore/model/value/FieldValue.java

@@ -21,6 +21,7 @@
  * Represents a FieldValue that is backed by a single Firestore V1 Value proto and implements
  * Firestore's Value semantics for ordering and equality.
  */
+// TODO(mrschmidt): Drop this class
 public class FieldValue implements Comparable<FieldValue> {
   final Value internalValue;
 

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

@@ -53,7 +53,7 @@ Object convertValue(Value value) {
     switch (value.getValueTypeCase()) {
       case MAP_VALUE:
         if (ServerTimestampValue.isServerTimestamp(value)) {
-          return convertServerTimestamp(ServerTimestampValue.valueOf(value));
+          return convertServerTimestamp(new ServerTimestampValue(value));
         }
         return convertObject(value.getMapValue().getFieldsMap());
       case ARRAY_VALUE:

firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteCollectionIndex.java

@@ -19,7 +19,7 @@
 import com.google.firebase.firestore.model.DocumentKey;
 import com.google.firebase.firestore.model.FieldPath;
 import com.google.firebase.firestore.model.ResourcePath;
-import com.google.firebase.firestore.model.value.FieldValue;
+import com.google.firestore.v1.Value;
 
 /**
  * A persisted "collection index" of all documents in the local cache (with mutations overlaid on
@@ -38,12 +38,12 @@ public class SQLiteCollectionIndex {
   }
 
   /** Adds the specified entry to the index. */
-  public void addEntry(FieldPath fieldPath, FieldValue fieldValue, DocumentKey documentKey) {
+  public void addEntry(FieldPath fieldPath, Value fieldValue, DocumentKey documentKey) {
     throw new RuntimeException("Not yet implemented.");
   }
 
   /** Adds the specified entry to the index. */
-  public void removeEntry(FieldPath fieldPath, FieldValue fieldValue, DocumentKey documentKey) {
+  public void removeEntry(FieldPath fieldPath, Value fieldValue, DocumentKey documentKey) {
     throw new RuntimeException("Not yet implemented.");
   }
 

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

@@ -31,7 +31,7 @@ public static ServerTimestampOperation getInstance() {
 
   @Override
   public Value applyToLocalView(@Nullable Value previousValue, Timestamp localWriteTime) {
-    return ServerTimestampValue.valueOf(localWriteTime, previousValue).getProto();
+    return ServerTimestampValue.valueOf(localWriteTime, previousValue);
   }
 
   @Override

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

@@ -28,7 +28,9 @@
 
 /** A structured object value stored in Firestore. */
 // TODO(mrschmidt): Rename to DocumentValue
-public class ObjectValue extends FieldValue {
+public class ObjectValue {
+  private Value internalValue;
+
   private static final ObjectValue EMPTY_INSTANCE =
       new ObjectValue(Value.newBuilder().setMapValue(MapValue.getDefaultInstance()).build());
 
@@ -38,13 +40,13 @@ public static ObjectValue fromMap(Map<String, Value> value) {
   }
 
   public ObjectValue(Value value) {
-    super(value);
     hardAssert(
         value.getValueTypeCase() == Value.ValueTypeCase.MAP_VALUE,
         "ObjectValues should be backed by a MapValue");
     hardAssert(
         !ServerTimestampValue.isServerTimestamp(value),
-        "ServerTimestamps should be converted to ServerTimestampValue");
+        "ServerTimestamps should not be used as an ObjectValue");
+    this.internalValue = value;
   }
 
   public static ObjectValue emptyObject() {
@@ -109,6 +111,26 @@ private FieldMask extractFieldMask(MapValue value) {
     }
   }
 
+  /** Returns the Protobuf that backs this ObjectValue. */
+  public Value getProto() {
+    return internalValue;
+  }
+
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) {
+      return true;
+    } else if (o instanceof ObjectValue) {
+      return ProtoValues.equals(internalValue, ((ObjectValue) o).internalValue);
+    }
+    return false;
+  }
+
+  @Override
+  public int hashCode() {
+    return internalValue.hashCode();
+  }
+
   /** Creates a ObjectValue.Builder instance that is based on the current value. */
   public ObjectValue.Builder toBuilder() {
     return new ObjectValue.Builder(this);

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

@@ -37,7 +37,7 @@
  * <li>They sort after all TimestampValues. With respect to other ServerTimestampValues, they sort
  *     by their localWriteTime.
  */
-public final class ServerTimestampValue extends FieldValue {
+public final class ServerTimestampValue {
   private static final String SERVER_TIMESTAMP_SENTINEL = "server_timestamp";
   private static final String TYPE_KEY = "__type__";
   private static final String PREVIOUS_VALUE_KEY = "__previous_value__";
@@ -48,18 +48,16 @@ public static boolean isServerTimestamp(@Nullable Value value) {
     return type != null && SERVER_TIMESTAMP_SENTINEL.equals(type.getStringValue());
   }
 
-  ServerTimestampValue(Value value) {
-    super(value);
+  private final Value internalValue;
+
+  public ServerTimestampValue(Value value) {
     hardAssert(
         ServerTimestampValue.isServerTimestamp(value),
         "Backing value is not a ServerTimestampValue");
+    this.internalValue = value;
   }
 
-  public static ServerTimestampValue valueOf(Value value) {
-    return new ServerTimestampValue(value);
-  }
-
-  public static FieldValue valueOf(Timestamp localWriteTime, @Nullable Value previousValue) {
+  public static Value valueOf(Timestamp localWriteTime, @Nullable Value previousValue) {
     Value encodedType = Value.newBuilder().setStringValue(SERVER_TIMESTAMP_SENTINEL).build();
     Value encodeWriteTime =
         Value.newBuilder()
@@ -78,7 +76,7 @@ public static FieldValue valueOf(Timestamp localWriteTime, @Nullable Value previ
       mapRepresentation.putFields(PREVIOUS_VALUE_KEY, previousValue);
     }
 
-    return new ServerTimestampValue(Value.newBuilder().setMapValue(mapRepresentation).build());
+    return Value.newBuilder().setMapValue(mapRepresentation).build();
   }
 
   /**

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

@@ -86,8 +86,7 @@ public void testToObjects() {
         .thenReturn(new FirebaseFirestoreSettings.Builder().build());
 
     ObjectValue objectData =
-        ObjectValue.fromMap(
-            map("timestamp", ServerTimestampValue.valueOf(Timestamp.now(), null).getProto()));
+        ObjectValue.fromMap(map("timestamp", ServerTimestampValue.valueOf(Timestamp.now(), null)));
     QuerySnapshot foo = TestUtil.querySnapshot("foo", map(), map("a", objectData), true, false);
 
     List<POJO> docs = foo.toObjects(POJO.class);

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

@@ -39,7 +39,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.TreeMap;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.robolectric.RobolectricTestRunner;
@@ -226,7 +225,7 @@ public void testConvertsGeoPointValue() {
 
   @Test
   public void testConvertsEmptyObjects() {
-    assertEquals(wrap(new TreeMap<String, FieldValue>()), ObjectValue.emptyObject());
+    assertEquals(wrapObject(), ObjectValue.emptyObject());
   }
 
   @Test
@@ -242,7 +241,7 @@ public void testConvertsSimpleObjects() {
             "c", wrap(true),
             "d", wrap(null));
 
-    FieldValue wrappedActual = wrapObject(actual);
+    ObjectValue wrappedActual = wrapObject(actual);
     assertEquals(wrappedActual, wrappedExpected);
   }
 
@@ -256,7 +255,7 @@ private static ObjectValue fromMap(Object... entries) {
 
   @Test
   public void testConvertsNestedObjects() {
-    FieldValue actual = wrapObject("a", map("b", map("c", "foo"), "d", true));
+    ObjectValue actual = wrapObject("a", map("b", map("c", "foo"), "d", true));
     ObjectValue.Builder expected = ObjectValue.newBuilder();
     expected.set(field("a.b.c"), valueOf("foo"));
     expected.set(field("a.d"), valueOf(true));

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

@@ -80,16 +80,15 @@ public void testExtractsFields() {
 
   @Test
   public void testExtractsFieldMask() {
-    FieldValue val =
+    ObjectValue val =
         wrapObject(
             "a",
             "b",
             "map",
             map("a", 1, "b", true, "c", "string", "nested", map("d", "e")),
             "emptymap",
             map());
-    assertTrue(val instanceof ObjectValue);
-    FieldMask mask = ((ObjectValue) val).getFieldMask();
+    FieldMask mask = val.getFieldMask();
     assertEquals(fieldMask("a", "map.a", "map.b", "map.c", "map.nested.d", "emptymap"), mask);
   }
 
@@ -254,21 +253,22 @@ public void testValueEquality() {
         .addEqualityGroup(wrap(timestamp2))
         // NOTE: ServerTimestampValues can't be parsed via wrap().
         .addEqualityGroup(
-            ServerTimestampValue.valueOf(new Timestamp(date1), null),
-            ServerTimestampValue.valueOf(new Timestamp(date1), null))
-        .addEqualityGroup(ServerTimestampValue.valueOf(new Timestamp(date2), null))
+            new FieldValue(ServerTimestampValue.valueOf(new Timestamp(date1), null)),
+            new FieldValue(ServerTimestampValue.valueOf(new Timestamp(date1), null)))
+        .addEqualityGroup(new FieldValue(ServerTimestampValue.valueOf(new Timestamp(date2), null)))
         .addEqualityGroup(wrap(geoPoint1), wrap(new GeoPoint(1, 0)))
         .addEqualityGroup(wrap(geoPoint2))
-        .addEqualityGroup(wrap(ref("coll/doc1")), wrapRef(dbId("project"), key("coll/doc1")))
-        .addEqualityGroup(wrapRef(dbId("project", "bar"), key("coll/doc2")))
-        .addEqualityGroup(wrapRef(dbId("project", "baz"), key("coll/doc2")))
+        .addEqualityGroup(
+            wrap(ref("coll/doc1")), new FieldValue(wrapRef(dbId("project"), key("coll/doc1"))))
+        .addEqualityGroup(new FieldValue(wrapRef(dbId("project", "bar"), key("coll/doc2"))))
+        .addEqualityGroup(new FieldValue(wrapRef(dbId("project", "baz"), key("coll/doc2"))))
         .addEqualityGroup(wrap(Arrays.asList("foo", "bar")), wrap(Arrays.asList("foo", "bar")))
         .addEqualityGroup(wrap(Arrays.asList("foo", "bar", "baz")))
         .addEqualityGroup(wrap(Arrays.asList("foo")))
-        .addEqualityGroup(wrapObject(map("bar", 1, "foo", 2)), wrapObject(map("foo", 2, "bar", 1)))
-        .addEqualityGroup(wrapObject(map("bar", 2, "foo", 1)))
-        .addEqualityGroup(wrapObject(map("bar", 1)))
-        .addEqualityGroup(wrapObject(map("foo", 1)))
+        .addEqualityGroup(wrap(map("bar", 1, "foo", 2)), wrap(map("foo", 2, "bar", 1)))
+        .addEqualityGroup(wrap(map("bar", 2, "foo", 1)))
+        .addEqualityGroup(wrap(map("bar", 1)))
+        .addEqualityGroup(wrap(map("foo", 1)))
         .testEquals();
   }
 
@@ -312,8 +312,8 @@ public void testValueOrdering() {
 
         // server timestamps come after all concrete timestamps.
         // NOTE: server timestamps can't be parsed with wrap().
-        .addEqualityGroup(ServerTimestampValue.valueOf(new Timestamp(date1), null))
-        .addEqualityGroup(ServerTimestampValue.valueOf(new Timestamp(date2), null))
+        .addEqualityGroup(new FieldValue(ServerTimestampValue.valueOf(new Timestamp(date1), null)))
+        .addEqualityGroup(new FieldValue(ServerTimestampValue.valueOf(new Timestamp(date2), null)))
 
         // strings
         .addEqualityGroup(wrap(""))
@@ -335,12 +335,12 @@ public void testValueOrdering() {
         .addEqualityGroup(wrap(blob(255)))
 
         // resource names
-        .addEqualityGroup(wrapRef(dbId("p1", "d1"), key("c1/doc1")))
-        .addEqualityGroup(wrapRef(dbId("p1", "d1"), key("c1/doc2")))
-        .addEqualityGroup(wrapRef(dbId("p1", "d1"), key("c10/doc1")))
-        .addEqualityGroup(wrapRef(dbId("p1", "d1"), key("c2/doc1")))
-        .addEqualityGroup(wrapRef(dbId("p1", "d2"), key("c1/doc1")))
-        .addEqualityGroup(wrapRef(dbId("p2", "d1"), key("c1/doc1")))
+        .addEqualityGroup(new FieldValue(wrapRef(dbId("p1", "d1"), key("c1/doc1"))))
+        .addEqualityGroup(new FieldValue(wrapRef(dbId("p1", "d1"), key("c1/doc2"))))
+        .addEqualityGroup(new FieldValue(wrapRef(dbId("p1", "d1"), key("c10/doc1"))))
+        .addEqualityGroup(new FieldValue(wrapRef(dbId("p1", "d1"), key("c2/doc1"))))
+        .addEqualityGroup(new FieldValue(wrapRef(dbId("p1", "d2"), key("c1/doc1"))))
+        .addEqualityGroup(new FieldValue(wrapRef(dbId("p2", "d1"), key("c1/doc1"))))
 
         // geo points
         .addEqualityGroup(wrap(new GeoPoint(-90, -180)))
@@ -363,11 +363,11 @@ public void testValueOrdering() {
         .addEqualityGroup(wrap(Arrays.asList("foo", "0")))
 
         // objects
-        .addEqualityGroup(wrapObject(map("bar", 0)))
-        .addEqualityGroup(wrapObject(map("bar", 0, "foo", 1)))
-        .addEqualityGroup(wrapObject(map("foo", 1)))
-        .addEqualityGroup(wrapObject(map("foo", 2)))
-        .addEqualityGroup(wrapObject(map("foo", "0")))
+        .addEqualityGroup(wrap(map("bar", 0)))
+        .addEqualityGroup(wrap(map("bar", 0, "foo", 1)))
+        .addEqualityGroup(wrap(map("foo", 1)))
+        .addEqualityGroup(wrap(map("foo", 2)))
+        .addEqualityGroup(wrap(map("foo", "0")))
         .testCompare();
   }
 
@@ -381,7 +381,7 @@ public void testCanonicalIds() {
     assertCanonicalId(wrap(new Timestamp(30, 1000)), "time(30,1000)");
     assertCanonicalId(wrap("a"), "a");
     assertCanonicalId(wrap(blob(1, 2, 3)), "010203");
-    assertCanonicalId(wrapRef(dbId("p1", "d1"), key("c1/doc1")), "c1/doc1");
+    assertCanonicalId(new FieldValue(wrapRef(dbId("p1", "d1"), key("c1/doc1"))), "c1/doc1");
     assertCanonicalId(wrap(new GeoPoint(30, 60)), "geo(30.0,60.0)");
     assertCanonicalId(wrap(Arrays.asList(1, 2, 3)), "[1,2,3]");
     assertCanonicalId(wrap(map("a", 1, "b", 2, "c", "3")), "{a:1,b:2,c:3}");

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

@@ -32,6 +32,7 @@
 import static com.google.firebase.firestore.testutil.TestUtil.wrapObject;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 import com.google.firebase.Timestamp;
 import com.google.firebase.firestore.FieldValue;
@@ -44,6 +45,7 @@
 import com.google.firebase.firestore.model.mutation.Precondition;
 import com.google.firebase.firestore.model.mutation.TransformMutation;
 import com.google.firebase.firestore.model.value.ObjectValue;
+import com.google.firebase.firestore.model.value.ProtoValues;
 import com.google.firebase.firestore.model.value.ServerTimestampValue;
 import java.util.Arrays;
 import java.util.Collections;
@@ -156,7 +158,8 @@ public void testAppliesLocalServerTimestampTransformsToDocuments() {
     ObjectValue expectedData =
         wrapObject(map("foo", map("bar", "<server-timestamp>"), "baz", "baz-value"));
     com.google.firebase.firestore.model.value.FieldValue fieldValue =
-        ServerTimestampValue.valueOf(timestamp, valueOf("bar-value"));
+        new com.google.firebase.firestore.model.value.FieldValue(
+            ServerTimestampValue.valueOf(timestamp, valueOf("bar-value")));
     expectedData = expectedData.toBuilder().set(field("foo.bar"), fieldValue.getProto()).build();
 
     Document expectedDoc =
@@ -308,9 +311,10 @@ public void testCreateArrayUnionTransform() {
         transformMutation(
             "collection/key",
             map(
-                "a", FieldValue.arrayUnion("tag"),
+                "a",
+                FieldValue.arrayUnion("tag"),
                 "bar.baz",
-                    FieldValue.arrayUnion(true, map("nested", map("a", Arrays.asList(1, 2))))));
+                FieldValue.arrayUnion(true, map("nested", map("a", Arrays.asList(1, 2))))));
     assertEquals(2, transform.getFieldTransforms().size());
 
     FieldTransform first = transform.getFieldTransforms().get(0);
@@ -688,7 +692,7 @@ public void testNumericIncrementBaseValue() {
                 0,
                 "nested",
                 map("double", 42.0, "long", 42, "string", 0, "map", 0, "missing", 0)));
-    assertEquals(expected, baseValue);
+    assertTrue(ProtoValues.equals(expected.getProto(), baseValue.getProto()));
   }
 
   @Test

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

@@ -51,6 +51,7 @@
 import com.google.firebase.firestore.model.SnapshotVersion;
 import com.google.firebase.firestore.model.mutation.Mutation;
 import com.google.firebase.firestore.model.value.FieldValue;
+import com.google.firebase.firestore.model.value.ObjectValue;
 import com.google.firebase.firestore.model.value.ProtoValues;
 import com.google.firebase.firestore.remote.WatchChange.WatchTargetChange;
 import com.google.firebase.firestore.remote.WatchChange.WatchTargetChangeType;
@@ -121,7 +122,7 @@ private void assertRoundTrip(
     com.google.firestore.v1.Value actual = value.getProto();
     assertEquals(typeCase, actual.getValueTypeCase());
     assertEquals(proto, actual);
-    assertEquals(value, new FieldValue(proto));
+    assertTrue(ProtoValues.equals(value.getProto(), proto));
   }
 
   @Test
@@ -258,7 +259,7 @@ public void testEncodeArrays() {
 
   @Test
   public void testEncodesNestedObjects() {
-    FieldValue model =
+    ObjectValue model =
         TestUtil.wrapObject(
             map(
                 "b",
@@ -304,7 +305,7 @@ public void testEncodesNestedObjects() {
             .putFields("o", valueBuilder().setMapValue(middle).build());
 
     com.google.firestore.v1.Value proto = valueBuilder().setMapValue(obj).build();
-    assertRoundTrip(model, proto, ValueTypeCase.MAP_VALUE);
+    assertRoundTrip(new FieldValue(model.getProto()), proto, ValueTypeCase.MAP_VALUE);
   }
 
   @Test