Drop Protobuf-backed FieldValues

Author: schmidt-sebastian

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/ValidationTest.java

@@ -461,44 +461,49 @@ public void queriesCannotBeSortedByAnUncommittedServerTimestamp() {
     TaskCompletionSource<Void> offlineCallbackDone = new TaskCompletionSource<>();
     TaskCompletionSource<Void> onlineCallbackDone = new TaskCompletionSource<>();
 
-    collection.addSnapshotListener(
-        (snapshot, error) -> {
-          assertNotNull(snapshot);
-
-          // Skip the initial empty snapshot.
-          if (snapshot.isEmpty()) return;
-
-          assertThat(snapshot.getDocuments()).hasSize(1);
-          DocumentSnapshot docSnap = snapshot.getDocuments().get(0);
-
-          if (snapshot.getMetadata().hasPendingWrites()) {
-            // Offline snapshot. Since the server timestamp is uncommitted, we shouldn't be able to
-            // query by it.
-            assertThrows(
-                IllegalArgumentException.class,
-                () ->
-                    collection
-                        .orderBy("timestamp")
-                        .endAt(docSnap)
-                        .addSnapshotListener((snapshot2, error2) -> {}));
-            offlineCallbackDone.setResult(null);
-          } else {
-            // Online snapshot. Since the server timestamp is committed, we should be able to query
-            // by it.
-            collection
-                .orderBy("timestamp")
-                .endAt(docSnap)
-                .addSnapshotListener((snapshot2, error2) -> {});
-            onlineCallbackDone.setResult(null);
-          }
-        });
+    ListenerRegistration listenerRegistration =
+        collection.addSnapshotListener(
+            (snapshot, error) -> {
+              assertNotNull(snapshot);
+
+              // Skip the initial empty snapshot.
+              if (snapshot.isEmpty()) return;
+
+              assertThat(snapshot.getDocuments()).hasSize(1);
+              DocumentSnapshot docSnap = snapshot.getDocuments().get(0);
+
+              if (snapshot.getMetadata().hasPendingWrites()) {
+                // Offline snapshot. Since the server timestamp is uncommitted, we shouldn't be able
+                // to query by it.
+                assertThrows(
+                    IllegalArgumentException.class,
+                    () ->
+                        collection
+                            .orderBy("timestamp")
+                            .endAt(docSnap)
+                            .addSnapshotListener((snapshot2, error2) -> {}));
+                // Use `trySetResult` since the callbacks fires twice if the WatchStream
+                // acknowledges the Write before the WriteStream.
+                offlineCallbackDone.trySetResult(null);
+              } else {
+                // Online snapshot. Since the server timestamp is committed, we should be able to
+                // query by it.
+                collection
+                    .orderBy("timestamp")
+                    .endAt(docSnap)
+                    .addSnapshotListener((snapshot2, error2) -> {});
+                onlineCallbackDone.trySetResult(null);
+              }
+            });
 
     DocumentReference document = collection.document();
     document.set(map("timestamp", FieldValue.serverTimestamp()));
     waitFor(offlineCallbackDone.getTask());
 
     waitFor(collection.firestore.getClient().enableNetwork());
     waitFor(onlineCallbackDone.getTask());
+
+    listenerRegistration.remove();
   }
 
   @Test

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

@@ -149,7 +149,7 @@ public Map<String, Object> getData(@NonNull ServerTimestampBehavior serverTimest
             firestore,
             firestore.getFirestoreSettings().areTimestampsInSnapshotsEnabled(),
             serverTimestampBehavior);
-    return doc == null ? null : userDataWriter.convertObject(doc.getData());
+    return doc == null ? null : userDataWriter.convertObject(doc.getData().getFieldsMap());
   }
 
   /**
@@ -533,7 +533,7 @@ private Object getInternal(
       if (val != null) {
         UserDataWriter userDataWriter =
             new UserDataWriter(firestore, timestampsInSnapshots, serverTimestampBehavior);
-        return userDataWriter.convertValue(val);
+        return userDataWriter.convertValue(val.getProto());
       }
     }
     return null;

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

@@ -35,25 +35,19 @@
 import com.google.firebase.firestore.model.mutation.FieldMask;
 import com.google.firebase.firestore.model.mutation.NumericIncrementTransformOperation;
 import com.google.firebase.firestore.model.mutation.ServerTimestampOperation;
-import com.google.firebase.firestore.model.value.ArrayValue;
-import com.google.firebase.firestore.model.value.BlobValue;
-import com.google.firebase.firestore.model.value.BooleanValue;
-import com.google.firebase.firestore.model.value.DoubleValue;
 import com.google.firebase.firestore.model.value.FieldValue;
-import com.google.firebase.firestore.model.value.GeoPointValue;
-import com.google.firebase.firestore.model.value.IntegerValue;
-import com.google.firebase.firestore.model.value.NullValue;
 import com.google.firebase.firestore.model.value.NumberValue;
 import com.google.firebase.firestore.model.value.ObjectValue;
-import com.google.firebase.firestore.model.value.ReferenceValue;
-import com.google.firebase.firestore.model.value.StringValue;
-import com.google.firebase.firestore.model.value.TimestampValue;
 import com.google.firebase.firestore.util.Assert;
 import com.google.firebase.firestore.util.CustomClassMapper;
 import com.google.firebase.firestore.util.Util;
+import com.google.firestore.v1.ArrayValue;
+import com.google.firestore.v1.MapValue;
+import com.google.firestore.v1.Value;
+import com.google.protobuf.NullValue;
+import com.google.type.LatLng;
 import java.util.ArrayList;
 import java.util.Date;
-import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -130,8 +124,7 @@ public ParsedUpdateData parseUpdateData(Map<String, Object> data) {
         context.addToFieldMask(fieldPath);
       } else {
         @Nullable
-        FieldValue parsedValue =
-            convertAndParseFieldData(fieldValue, context.childContext(fieldPath));
+        Value parsedValue = convertAndParseFieldData(fieldValue, context.childContext(fieldPath));
         if (parsedValue != null) {
           context.addToFieldMask(fieldPath);
           updateData.set(fieldPath, parsedValue);
@@ -181,8 +174,7 @@ public ParsedUpdateData parseUpdateData(List<Object> fieldsAndValues) {
         // Add it to the field mask, but don't add anything to updateData.
         context.addToFieldMask(parsedField);
       } else {
-        FieldValue parsedValue =
-            convertAndParseFieldData(fieldValue, context.childContext(parsedField));
+        Value parsedValue = convertAndParseFieldData(fieldValue, context.childContext(parsedField));
         if (parsedValue != null) {
           context.addToFieldMask(parsedField);
           updateData.set(parsedField, parsedValue);
@@ -209,16 +201,16 @@ public FieldValue parseQueryValue(Object input, boolean allowArrays) {
         new ParseAccumulator(
             allowArrays ? UserData.Source.ArrayArgument : UserData.Source.Argument);
 
-    @Nullable FieldValue parsed = convertAndParseFieldData(input, accumulator.rootContext());
+    @Nullable Value parsed = convertAndParseFieldData(input, accumulator.rootContext());
     hardAssert(parsed != null, "Parsed data should not be null.");
     hardAssert(
         accumulator.getFieldTransforms().isEmpty(),
         "Field transforms should have been disallowed.");
-    return parsed;
+    return FieldValue.valueOf(parsed);
   }
 
   /** Converts a POJO to native types and then parses it into model types. */
-  private FieldValue convertAndParseFieldData(Object input, ParseContext context) {
+  private Value convertAndParseFieldData(Object input, ParseContext context) {
     Object converted = CustomClassMapper.convertToPlainJavaTypes(input);
     return parseData(converted, context);
   }
@@ -239,7 +231,7 @@ private ObjectValue convertAndParseDocumentData(Object input, ParseContext conte
     }
 
     Object converted = CustomClassMapper.convertToPlainJavaTypes(input);
-    FieldValue value = parseData(converted, context);
+    FieldValue value = FieldValue.valueOf(parseData(converted, context));
 
     if (!(value instanceof ObjectValue)) {
       throw new IllegalArgumentException(badDocReason + "of type: " + Util.typeName(input));
@@ -257,7 +249,7 @@ private ObjectValue convertAndParseDocumentData(Object input, ParseContext conte
    *     not be included in the resulting parsed data.
    */
   @Nullable
-  private FieldValue parseData(Object input, ParseContext context) {
+  private Value parseData(Object input, ParseContext context) {
     if (input instanceof Map) {
       return parseMap((Map<?, ?>) input, context);
 
@@ -291,43 +283,42 @@ private FieldValue parseData(Object input, ParseContext context) {
     }
   }
 
-  private <K, V> ObjectValue parseMap(Map<K, V> map, ParseContext context) {
-    Map<String, FieldValue> result = new HashMap<>();
-
+  private <K, V> Value parseMap(Map<K, V> map, ParseContext context) {
     if (map.isEmpty()) {
       if (context.getPath() != null && !context.getPath().isEmpty()) {
         context.addToFieldMask(context.getPath());
       }
-      return ObjectValue.emptyObject();
+      return Value.newBuilder().setMapValue(MapValue.getDefaultInstance()).build();
     } else {
+      MapValue.Builder mapBuilder = MapValue.newBuilder();
       for (Entry<K, V> entry : map.entrySet()) {
         if (!(entry.getKey() instanceof String)) {
           throw context.createError(
               String.format("Non-String Map key (%s) is not allowed", entry.getValue()));
         }
         String key = (String) entry.getKey();
-        @Nullable FieldValue parsedValue = parseData(entry.getValue(), context.childContext(key));
+        @Nullable Value parsedValue = parseData(entry.getValue(), context.childContext(key));
         if (parsedValue != null) {
-          result.put(key, parsedValue);
+          mapBuilder.putFields(key, parsedValue);
         }
       }
+      return Value.newBuilder().setMapValue(mapBuilder).build();
     }
-    return ObjectValue.fromMap(result);
   }
 
-  private <T> ArrayValue parseList(List<T> list, ParseContext context) {
-    List<FieldValue> result = new ArrayList<>(list.size());
+  private <T> Value parseList(List<T> list, ParseContext context) {
+    ArrayValue.Builder arrayBuilder = ArrayValue.newBuilder();
     int entryIndex = 0;
     for (T entry : list) {
-      @Nullable FieldValue parsedEntry = parseData(entry, context.childContext(entryIndex));
+      @Nullable Value parsedEntry = parseData(entry, context.childContext(entryIndex));
       if (parsedEntry == null) {
         // Just include nulls in the array for fields being replaced with a sentinel.
-        parsedEntry = NullValue.nullValue();
+        parsedEntry = Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build();
       }
-      result.add(parsedEntry);
+      arrayBuilder.addValues(parsedEntry);
       entryIndex++;
     }
-    return ArrayValue.fromList(result);
+    return Value.newBuilder().setArrayValue(arrayBuilder).build();
   }
 
   /**
@@ -398,35 +389,37 @@ private void parseSentinelFieldValue(
    * @return The parsed value, or {@code null} if the value was a FieldValue sentinel that should
    *     not be included in the resulting parsed data.
    */
-  private FieldValue parseScalarValue(Object input, ParseContext context) {
+  private Value parseScalarValue(Object input, ParseContext context) {
     if (input == null) {
-      return NullValue.nullValue();
+      return Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build();
     } else if (input instanceof Integer) {
-      return IntegerValue.valueOf(((Integer) input).longValue());
+      return Value.newBuilder().setIntegerValue((Integer) input).build();
     } else if (input instanceof Long) {
-      return IntegerValue.valueOf(((Long) input));
+      return Value.newBuilder().setIntegerValue((Long) input).build();
     } else if (input instanceof Float) {
-      return DoubleValue.valueOf(((Float) input).doubleValue());
+      return Value.newBuilder().setDoubleValue(((Float) input).doubleValue()).build();
     } else if (input instanceof Double) {
-      return DoubleValue.valueOf((Double) input);
+      return Value.newBuilder().setDoubleValue((Double) input).build();
     } else if (input instanceof Boolean) {
-      return BooleanValue.valueOf((Boolean) input);
+      return Value.newBuilder().setBooleanValue((Boolean) input).build();
     } else if (input instanceof String) {
-      return StringValue.valueOf((String) input);
+      return Value.newBuilder().setStringValue((String) input).build();
     } else if (input instanceof Date) {
-      return TimestampValue.valueOf(new Timestamp((Date) input));
+      Timestamp timestamp = new Timestamp((Date) input);
+      return parseTimestamp(timestamp);
     } else if (input instanceof Timestamp) {
       Timestamp timestamp = (Timestamp) input;
-      long seconds = timestamp.getSeconds();
-      // Firestore backend truncates precision down to microseconds. To ensure offline mode works
-      // the same with regards to truncation, perform the truncation immediately without waiting for
-      // the backend to do that.
-      int truncatedNanoseconds = timestamp.getNanoseconds() / 1000 * 1000;
-      return TimestampValue.valueOf(new Timestamp(seconds, truncatedNanoseconds));
+      return parseTimestamp(timestamp);
     } else if (input instanceof GeoPoint) {
-      return GeoPointValue.valueOf((GeoPoint) input);
+      GeoPoint geoPoint = (GeoPoint) input;
+      return Value.newBuilder()
+          .setGeoPointValue(
+              LatLng.newBuilder()
+                  .setLatitude(geoPoint.getLatitude())
+                  .setLongitude(geoPoint.getLongitude()))
+          .build();
     } else if (input instanceof Blob) {
-      return BlobValue.valueOf((Blob) input);
+      return Value.newBuilder().setBytesValue(((Blob) input).toByteString()).build();
     } else if (input instanceof DocumentReference) {
       DocumentReference ref = (DocumentReference) input;
       // TODO: Rework once pre-converter is ported to Android.
@@ -442,14 +435,35 @@ private FieldValue parseScalarValue(Object input, ParseContext context) {
                   databaseId.getDatabaseId()));
         }
       }
-      return ReferenceValue.valueOf(databaseId, ref.getKey());
+      return Value.newBuilder()
+          .setReferenceValue(
+              String.format(
+                  "projects/%s/databases/%s/documents/%s",
+                  databaseId.getProjectId(),
+                  databaseId.getDatabaseId(),
+                  ((DocumentReference) input).getPath()))
+          .build();
     } else if (input.getClass().isArray()) {
       throw context.createError("Arrays are not supported; use a List instead");
     } else {
       throw context.createError("Unsupported type: " + Util.typeName(input));
     }
   }
 
+  private Value parseTimestamp(Timestamp timestamp) {
+    // Firestore backend truncates precision down to microseconds. To ensure offline mode works
+    // the same with regards to truncation, perform the truncation immediately without waiting for
+    // the backend to do that.
+    int truncatedNanoseconds = timestamp.getNanoseconds() / 1000 * 1000;
+
+    return Value.newBuilder()
+        .setTimestampValue(
+            com.google.protobuf.Timestamp.newBuilder()
+                .setSeconds(timestamp.getSeconds())
+                .setNanos(truncatedNanoseconds))
+        .build();
+  }
+
   private List<FieldValue> parseArrayTransformElements(List<Object> elements) {
     ParseAccumulator accumulator = new ParseAccumulator(UserData.Source.Argument);
 
@@ -460,7 +474,7 @@ private List<FieldValue> parseArrayTransformElements(List<Object> elements) {
       // being unioned or removed are not considered writes since they cannot
       // contain any FieldValue sentinels, etc.
       ParseContext context = accumulator.rootContext();
-      result.add(convertAndParseFieldData(element, context.childContext(i)));
+      result.add(FieldValue.valueOf(convertAndParseFieldData(element, context.childContext(i))));
     }
     return result;
   }