Introducing UserDataReader/UserDataWriter

firebase-firestore/ktx/src/test/java/com/google/firebase/firestore/testutil/TestUtil.java

@@ -15,7 +15,7 @@
 package com.google.firebase.firestore.testutil;
 
 import com.google.firebase.Timestamp;
-import com.google.firebase.firestore.UserDataConverter;
+import com.google.firebase.firestore.UserDataReader;
 import com.google.firebase.firestore.core.Query;
 import com.google.firebase.firestore.model.DatabaseId;
 import com.google.firebase.firestore.model.Document;
@@ -33,10 +33,10 @@ public class TestUtil {
 
   public static FieldValue wrap(Object value) {
     DatabaseId databaseId = DatabaseId.forProject("project");
-    UserDataConverter dataConverter = new UserDataConverter(databaseId);
+    UserDataReader dataReader = new UserDataReader(databaseId);
     // HACK: We use parseQueryValue() since it accepts scalars as well as arrays / objects, and
     // our tests currently use wrap() pretty generically so we don't know the intent.
-    return dataConverter.parseQueryValue(value);
+    return dataReader.parseQueryValue(value);
   }
 
   public static ObjectValue wrapObject(Map<String, Object> value) {

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

@@ -162,8 +162,8 @@ public Task<Void> set(@NonNull Object data, @NonNull SetOptions options) {
     checkNotNull(options, "Provided options must not be null.");
     ParsedSetData parsed =
         options.isMerge()
-            ? firestore.getDataConverter().parseMergeData(data, options.getFieldMask())
-            : firestore.getDataConverter().parseSetData(data);
+            ? firestore.getUserDataReader().parseMergeData(data, options.getFieldMask())
+            : firestore.getUserDataReader().parseSetData(data);
     return firestore
         .getClient()
         .write(parsed.toMutationList(key, Precondition.NONE))
@@ -180,7 +180,7 @@ public Task<Void> set(@NonNull Object data, @NonNull SetOptions options) {
    */
   @NonNull
   public Task<Void> update(@NonNull Map<String, Object> data) {
-    ParsedUpdateData parsedData = firestore.getDataConverter().parseUpdateData(data);
+    ParsedUpdateData parsedData = firestore.getUserDataReader().parseUpdateData(data);
     return update(parsedData);
   }
 
@@ -199,7 +199,7 @@ public Task<Void> update(
       @NonNull String field, @Nullable Object value, Object... moreFieldsAndValues) {
     ParsedUpdateData parsedData =
         firestore
-            .getDataConverter()
+            .getUserDataReader()
             .parseUpdateData(
                 Util.collectUpdateArguments(
                     /* fieldPathOffset= */ 1, field, value, moreFieldsAndValues));
@@ -220,7 +220,7 @@ public Task<Void> update(
       @NonNull FieldPath fieldPath, @Nullable Object value, Object... moreFieldsAndValues) {
     ParsedUpdateData parsedData =
         firestore
-            .getDataConverter()
+            .getUserDataReader()
             .parseUpdateData(
                 Util.collectUpdateArguments(
                     /* fieldPathOffset= */ 1, fieldPath, value, moreFieldsAndValues));

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

@@ -19,21 +19,11 @@
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import com.google.firebase.Timestamp;
-import com.google.firebase.firestore.model.DatabaseId;
 import com.google.firebase.firestore.model.Document;
 import com.google.firebase.firestore.model.DocumentKey;
-import com.google.firebase.firestore.model.value.ArrayValue;
 import com.google.firebase.firestore.model.value.FieldValue;
-import com.google.firebase.firestore.model.value.ObjectValue;
-import com.google.firebase.firestore.model.value.ReferenceValue;
-import com.google.firebase.firestore.model.value.ServerTimestampValue;
-import com.google.firebase.firestore.model.value.TimestampValue;
 import com.google.firebase.firestore.util.CustomClassMapper;
-import com.google.firebase.firestore.util.Logger;
-import java.util.ArrayList;
 import java.util.Date;
-import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 
 /**
@@ -78,18 +68,6 @@ public enum ServerTimestampBehavior {
     static final ServerTimestampBehavior DEFAULT = ServerTimestampBehavior.NONE;
   }
 
-  /** Holds settings that define field value deserialization options. */
-  static class FieldValueOptions {
-    final ServerTimestampBehavior serverTimestampBehavior;
-    final boolean timestampsInSnapshotsEnabled;
-
-    private FieldValueOptions(
-        ServerTimestampBehavior serverTimestampBehavior, boolean timestampsInSnapshotsEnabled) {
-      this.serverTimestampBehavior = serverTimestampBehavior;
-      this.timestampsInSnapshotsEnabled = timestampsInSnapshotsEnabled;
-    }
-  }
-
   private final FirebaseFirestore firestore;
 
   private final DocumentKey key;
@@ -166,13 +144,8 @@ public Map<String, Object> getData() {
   public Map<String, Object> getData(@NonNull ServerTimestampBehavior serverTimestampBehavior) {
     checkNotNull(
         serverTimestampBehavior, "Provided serverTimestampBehavior value must not be null.");
-    return doc == null
-        ? null
-        : convertObject(
-            doc.getData(),
-            new FieldValueOptions(
-                serverTimestampBehavior,
-                firestore.getFirestoreSettings().areTimestampsInSnapshotsEnabled()));
+    UserDataWriter userDataWriter = new UserDataWriter(firestore, serverTimestampBehavior);
+    return doc == null ? null : userDataWriter.convertObject(doc.getData());
   }
 
   /**
@@ -283,11 +256,7 @@ public Object get(
     checkNotNull(fieldPath, "Provided field path must not be null.");
     checkNotNull(
         serverTimestampBehavior, "Provided serverTimestampBehavior value must not be null.");
-    return getInternal(
-        fieldPath.getInternalPath(),
-        new FieldValueOptions(
-            serverTimestampBehavior,
-            firestore.getFirestoreSettings().areTimestampsInSnapshotsEnabled()));
+    return getInternal(fieldPath.getInternalPath(), serverTimestampBehavior);
   }
 
   /**
@@ -437,9 +406,7 @@ public Date getDate(
         serverTimestampBehavior, "Provided serverTimestampBehavior value must not be null.");
     Object maybeDate =
         getInternal(
-            FieldPath.fromDotSeparatedPath(field).getInternalPath(),
-            new FieldValueOptions(
-                serverTimestampBehavior, /*timestampsInSnapshotsEnabled=*/ false));
+            FieldPath.fromDotSeparatedPath(field).getInternalPath(), serverTimestampBehavior);
     return castTypedValue(maybeDate, field, Date.class);
   }
 
@@ -478,8 +445,7 @@ public Timestamp getTimestamp(
         serverTimestampBehavior, "Provided serverTimestampBehavior value must not be null.");
     Object maybeTimestamp =
         getInternal(
-            FieldPath.fromDotSeparatedPath(field).getInternalPath(),
-            new FieldValueOptions(serverTimestampBehavior, /*timestampsInSnapshotsEnabled=*/ true));
+            FieldPath.fromDotSeparatedPath(field).getInternalPath(), serverTimestampBehavior);
     return castTypedValue(maybeTimestamp, field, Timestamp.class);
   }
 
@@ -546,87 +512,15 @@ private <T> T castTypedValue(Object value, String field, Class<T> clazz) {
     return clazz.cast(value);
   }
 
-  @Nullable
-  private Object convertValue(FieldValue value, FieldValueOptions options) {
-    if (value instanceof ObjectValue) {
-      return convertObject((ObjectValue) value, options);
-    } else if (value instanceof ArrayValue) {
-      return convertArray((ArrayValue) value, options);
-    } else if (value instanceof ReferenceValue) {
-      return convertReference((ReferenceValue) value);
-    } else if (value instanceof TimestampValue) {
-      return convertTimestamp((TimestampValue) value, options);
-    } else if (value instanceof ServerTimestampValue) {
-      return convertServerTimestamp((ServerTimestampValue) value, options);
-    } else {
-      return value.value();
-    }
-  }
-
-  private Object convertServerTimestamp(ServerTimestampValue value, FieldValueOptions options) {
-    switch (options.serverTimestampBehavior) {
-      case PREVIOUS:
-        return value.getPreviousValue();
-      case ESTIMATE:
-        return value.getLocalWriteTime();
-      default:
-        return value.value();
-    }
-  }
-
-  private Object convertTimestamp(TimestampValue value, FieldValueOptions options) {
-    Timestamp timestamp = value.value();
-    if (options.timestampsInSnapshotsEnabled) {
-      return timestamp;
-    } else {
-      return timestamp.toDate();
-    }
-  }
-
-  private Object convertReference(ReferenceValue value) {
-    DocumentKey key = value.value();
-    DatabaseId refDatabase = value.getDatabaseId();
-    DatabaseId database = this.firestore.getDatabaseId();
-    if (!refDatabase.equals(database)) {
-      // TODO: Somehow support foreign references.
-      Logger.warn(
-          "DocumentSnapshot",
-          "Document %s contains a document reference within a different database "
-              + "(%s/%s) which is not supported. It will be treated as a reference in "
-              + "the current database (%s/%s) instead.",
-          key.getPath(),
-          refDatabase.getProjectId(),
-          refDatabase.getDatabaseId(),
-          database.getProjectId(),
-          database.getDatabaseId());
-    }
-    return new DocumentReference(key, firestore);
-  }
-
-  private Map<String, Object> convertObject(ObjectValue objectValue, FieldValueOptions options) {
-    Map<String, Object> result = new HashMap<>();
-    for (Map.Entry<String, FieldValue> entry : objectValue.getInternalValue()) {
-      result.put(entry.getKey(), convertValue(entry.getValue(), options));
-    }
-    return result;
-  }
-
-  private List<Object> convertArray(ArrayValue arrayValue, FieldValueOptions options) {
-    ArrayList<Object> result = new ArrayList<>(arrayValue.getInternalValue().size());
-    for (FieldValue v : arrayValue.getInternalValue()) {
-      result.add(convertValue(v, options));
-    }
-    return result;
-  }
-
   @Nullable
   private Object getInternal(
       @NonNull com.google.firebase.firestore.model.FieldPath fieldPath,
-      @NonNull FieldValueOptions options) {
+      @NonNull ServerTimestampBehavior serverTimestampBehavior) {
     if (doc != null) {
       FieldValue val = doc.getField(fieldPath);
       if (val != null) {
-        return convertValue(val, options);
+        UserDataWriter userDataWriter = new UserDataWriter(firestore, serverTimestampBehavior);
+        return userDataWriter.convertValue(val);
       }
     }
     return null;

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

@@ -74,7 +74,7 @@ public interface InstanceRegistry {
   private final CredentialsProvider credentialsProvider;
   private final AsyncQueue asyncQueue;
   private final FirebaseApp firebaseApp;
-  private final UserDataConverter dataConverter;
+  private final UserDataReader userDataReader;
   // When user requests to terminate, use this to notify `FirestoreMultiDbComponent` to deregister
   // this instance.
   private final InstanceRegistry instanceRegistry;
@@ -160,7 +160,7 @@ static FirebaseFirestore newInstance(
       @Nullable GrpcMetadataProvider metadataProvider) {
     this.context = checkNotNull(context);
     this.databaseId = checkNotNull(checkNotNull(databaseId));
-    this.dataConverter = new UserDataConverter(databaseId);
+    this.userDataReader = new UserDataReader(databaseId);
     this.persistenceKey = checkNotNull(persistenceKey);
     this.credentialsProvider = checkNotNull(credentialsProvider);
     this.asyncQueue = checkNotNull(asyncQueue);
@@ -577,8 +577,8 @@ DatabaseId getDatabaseId() {
     return databaseId;
   }
 
-  UserDataConverter getDataConverter() {
-    return dataConverter;
+  UserDataReader getUserDataReader() {
+    return userDataReader;
   }
 
   /**

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

@@ -348,7 +348,7 @@ private Query whereHelper(@NonNull FieldPath fieldPath, Operator op, Object valu
       if (op == Operator.IN || op == Operator.ARRAY_CONTAINS_ANY) {
         validateDisjunctiveFilterElements(value, op);
       }
-      fieldValue = firestore.getDataConverter().parseQueryValue(value, op == Operator.IN);
+      fieldValue = firestore.getUserDataReader().parseQueryValue(value, op == Operator.IN);
     }
     Filter filter = FieldFilter.create(fieldPath.getInternalPath(), op, fieldValue);
     validateNewFilter(filter);
@@ -821,7 +821,7 @@ private Bound boundFromFields(String methodName, Object[] values, boolean before
         DocumentKey key = DocumentKey.fromPath(path);
         components.add(ReferenceValue.valueOf(firestore.getDatabaseId(), key));
       } else {
-        FieldValue wrapped = firestore.getDataConverter().parseQueryValue(rawValue);
+        FieldValue wrapped = firestore.getUserDataReader().parseQueryValue(rawValue);
         components.add(wrapped);
       }
     }

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

@@ -86,8 +86,8 @@ public Transaction set(
     checkNotNull(options, "Provided options must not be null.");
     ParsedSetData parsed =
         options.isMerge()
-            ? firestore.getDataConverter().parseMergeData(data, options.getFieldMask())
-            : firestore.getDataConverter().parseSetData(data);
+            ? firestore.getUserDataReader().parseMergeData(data, options.getFieldMask())
+            : firestore.getUserDataReader().parseSetData(data);
     transaction.set(documentRef.getKey(), parsed);
     return this;
   }
@@ -104,7 +104,7 @@ public Transaction set(
   @NonNull
   public Transaction update(
       @NonNull DocumentReference documentRef, @NonNull Map<String, Object> data) {
-    ParsedUpdateData parsedData = firestore.getDataConverter().parseUpdateData(data);
+    ParsedUpdateData parsedData = firestore.getUserDataReader().parseUpdateData(data);
     return update(documentRef, parsedData);
   }
 
@@ -127,7 +127,7 @@ public Transaction update(
       Object... moreFieldsAndValues) {
     ParsedUpdateData parsedData =
         firestore
-            .getDataConverter()
+            .getUserDataReader()
             .parseUpdateData(
                 Util.collectUpdateArguments(
                     /* fieldPathOffset= */ 1, field, value, moreFieldsAndValues));
@@ -152,7 +152,7 @@ public Transaction update(
       Object... moreFieldsAndValues) {
     ParsedUpdateData parsedData =
         firestore
-            .getDataConverter()
+            .getUserDataReader()
             .parseUpdateData(
                 Util.collectUpdateArguments(
                     /* fieldPathOffset= */ 1, fieldPath, value, moreFieldsAndValues));

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

@@ -65,11 +65,11 @@
  * @hide
  */
 @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
-public final class UserDataConverter {
+public final class UserDataReader {
 
   private final DatabaseId databaseId;
 
-  public UserDataConverter(DatabaseId databaseId) {
+  public UserDataReader(DatabaseId databaseId) {
     this.databaseId = databaseId;
   }
 
@@ -398,7 +398,6 @@ 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.
    */
-  @Nullable
   private FieldValue parseScalarValue(Object input, ParseContext context) {
     if (input == null) {
       return NullValue.nullValue();