Introducing UserDataReader/UserDataWriter

firebase-firestore/ktx/src/test/java/com/google/firebase/firestore/TestAccessHelper.java

@@ -0,0 +1,29 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.firebase.firestore;
+
+import org.mockito.Mockito;
+
+public class TestAccessHelper {
+  /**
+   * Install mocks for `FirebaseFirestore.getFirestoreSettings()` and
+   * `FirebaseFirestore.getUserDataWriter()`, which are used in DocumentSnapshot tests.
+   */
+  public static void installDocumentSnapshotMocks(FirebaseFirestore firestore) {
+    Mockito.when(firestore.getFirestoreSettings())
+        .thenReturn(new FirebaseFirestoreSettings.Builder().build());
+    Mockito.when(firestore.getUserDataWriter()).thenReturn(new UserDataWriter(firestore));
+  }
+}

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,7 +33,7 @@ public class TestUtil {
 
   public static FieldValue wrap(Object value) {
     DatabaseId databaseId = DatabaseId.forProject("project");
-    UserDataConverter dataConverter = new UserDataConverter(databaseId);
+    UserDataReader dataConverter = 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);

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

@@ -20,7 +20,7 @@ import com.google.firebase.FirebaseOptions
 import com.google.firebase.firestore.DocumentSnapshot
 import com.google.firebase.firestore.FieldPath
 import com.google.firebase.firestore.FirebaseFirestore
-import com.google.firebase.firestore.FirebaseFirestoreSettings
+import com.google.firebase.firestore.TestAccessHelper
 import com.google.firebase.firestore.TestUtil
 import com.google.firebase.firestore.model.value.IntegerValue
 import com.google.firebase.firestore.model.value.ObjectValue
@@ -117,7 +117,7 @@ data class Room(var a: Int = 0, var b: Int = 0)
 class DocumentSnapshotTests {
     @Before
     fun setup() {
-        Mockito.`when`(TestUtil.firestore().firestoreSettings).thenReturn(FirebaseFirestoreSettings.Builder().build())
+        TestAccessHelper.installDocumentSnapshotMocks(TestUtil.firestore())
     }
 
     @After
@@ -158,7 +158,7 @@ class DocumentSnapshotTests {
 class QuerySnapshotTests {
     @Before
     fun setup() {
-        Mockito.`when`(TestUtil.firestore().firestoreSettings).thenReturn(FirebaseFirestoreSettings.Builder().build())
+        TestAccessHelper.installDocumentSnapshotMocks(TestUtil.firestore())
     }
 
     @After

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

@@ -15,25 +15,18 @@
 package com.google.firebase.firestore;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.firebase.firestore.util.Assert.hardAssert;
 
 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.UserDataWriter.UserDataWriterOptions;
 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,20 +71,10 @@ 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 UserDataWriter userDataWriter;
+
   private final DocumentKey key;
 
   /** Is {@code null} if the document doesn't exist */
@@ -109,6 +92,7 @@ private FieldValueOptions(
     this.key = checkNotNull(key);
     this.doc = doc;
     this.metadata = new SnapshotMetadata(hasPendingWrites, isFromCache);
+    this.userDataWriter = firestore.getUserDataWriter();
   }
 
   static DocumentSnapshot fromDocument(
@@ -170,7 +154,7 @@ public Map<String, Object> getData(@NonNull ServerTimestampBehavior serverTimest
         ? null
         : convertObject(
             doc.getData(),
-            new FieldValueOptions(
+            new UserDataWriterOptions(
                 serverTimestampBehavior,
                 firestore.getFirestoreSettings().areTimestampsInSnapshotsEnabled()));
   }
@@ -285,7 +269,7 @@ public Object get(
         serverTimestampBehavior, "Provided serverTimestampBehavior value must not be null.");
     return getInternal(
         fieldPath.getInternalPath(),
-        new FieldValueOptions(
+        new UserDataWriterOptions(
             serverTimestampBehavior,
             firestore.getFirestoreSettings().areTimestampsInSnapshotsEnabled()));
   }
@@ -438,7 +422,7 @@ public Date getDate(
     Object maybeDate =
         getInternal(
             FieldPath.fromDotSeparatedPath(field).getInternalPath(),
-            new FieldValueOptions(
+            new UserDataWriterOptions(
                 serverTimestampBehavior, /*timestampsInSnapshotsEnabled=*/ false));
     return castTypedValue(maybeDate, field, Date.class);
   }
@@ -479,7 +463,8 @@ public Timestamp getTimestamp(
     Object maybeTimestamp =
         getInternal(
             FieldPath.fromDotSeparatedPath(field).getInternalPath(),
-            new FieldValueOptions(serverTimestampBehavior, /*timestampsInSnapshotsEnabled=*/ true));
+            new UserDataWriterOptions(
+                serverTimestampBehavior, /*timestampsInSnapshotsEnabled=*/ true));
     return castTypedValue(maybeTimestamp, field, Timestamp.class);
   }
 
@@ -546,87 +531,21 @@ 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;
+  private Map<String, Object> convertObject(
+      ObjectValue objectValue, UserDataWriterOptions options) {
+    Object result = userDataWriter.convertValue(objectValue, options);
+    hardAssert(result instanceof Map, "Documens should be decoded into Maps");
+    return (Map<String, Object>) result;
   }
 
   @Nullable
   private Object getInternal(
       @NonNull com.google.firebase.firestore.model.FieldPath fieldPath,
-      @NonNull FieldValueOptions options) {
+      @NonNull UserDataWriterOptions options) {
     if (doc != null) {
       FieldValue val = doc.getField(fieldPath);
       if (val != null) {
-        return convertValue(val, options);
+        return userDataWriter.convertValue(val, options);
       }
     }
     return null;

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

@@ -74,7 +74,8 @@ public interface InstanceRegistry {
   private final CredentialsProvider credentialsProvider;
   private final AsyncQueue asyncQueue;
   private final FirebaseApp firebaseApp;
-  private final UserDataConverter dataConverter;
+  private final UserDataReader userDataReader;
+  private final UserDataWriter userDataWriter;
   // When user requests to terminate, use this to notify `FirestoreMultiDbComponent` to deregister
   // this instance.
   private final InstanceRegistry instanceRegistry;
@@ -160,7 +161,8 @@ 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.userDataWriter = new UserDataWriter(this);
     this.persistenceKey = checkNotNull(persistenceKey);
     this.credentialsProvider = checkNotNull(credentialsProvider);
     this.asyncQueue = checkNotNull(asyncQueue);
@@ -577,8 +579,12 @@ DatabaseId getDatabaseId() {
     return databaseId;
   }
 
-  UserDataConverter getDataConverter() {
-    return dataConverter;
+  UserDataReader getUserDataReader() {
+    return userDataReader;
+  }
+
+  UserDataWriter getUserDataWriter() {
+    return userDataWriter;
   }
 
   /**

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));