Simplify FieldValueOptions (#541)

Author: schmidt-sebastian

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

@@ -24,9 +24,10 @@
 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.FieldValueOptions;
 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;
@@ -80,6 +81,18 @@ 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;
@@ -165,7 +178,7 @@ public Map<String, Object> getData(@NonNull ServerTimestampBehavior serverTimest
         ? null
         : convertObject(
             doc.getData(),
-            FieldValueOptions.create(
+            new FieldValueOptions(
                 serverTimestampBehavior,
                 firestore.getFirestoreSettings().areTimestampsInSnapshotsEnabled()));
   }
@@ -286,7 +299,7 @@ public Object get(
         serverTimestampBehavior, "Provided serverTimestampBehavior value must not be null.");
     return getInternal(
         fieldPath.getInternalPath(),
-        FieldValueOptions.create(
+        new FieldValueOptions(
             serverTimestampBehavior,
             firestore.getFirestoreSettings().areTimestampsInSnapshotsEnabled()));
   }
@@ -449,7 +462,7 @@ public Date getDate(
     Object maybeDate =
         getInternal(
             FieldPath.fromDotSeparatedPath(field).getInternalPath(),
-            FieldValueOptions.create(
+            new FieldValueOptions(
                 serverTimestampBehavior, /*timestampsInSnapshotsEnabled=*/ false));
     return castTypedValue(maybeDate, field, Date.class);
   }
@@ -492,8 +505,7 @@ public Timestamp getTimestamp(
     Object maybeTimestamp =
         getInternal(
             FieldPath.fromDotSeparatedPath(field).getInternalPath(),
-            FieldValueOptions.create(
-                serverTimestampBehavior, /*timestampsInSnapshotsEnabled=*/ true));
+            new FieldValueOptions(serverTimestampBehavior, /*timestampsInSnapshotsEnabled=*/ true));
     return castTypedValue(maybeTimestamp, field, Timestamp.class);
   }
 
@@ -571,29 +583,56 @@ private Object convertValue(FieldValue value, FieldValueOptions options) {
     } else if (value instanceof ArrayValue) {
       return convertArray((ArrayValue) value, options);
     } else if (value instanceof ReferenceValue) {
-      ReferenceValue referenceValue = (ReferenceValue) value;
-      DocumentKey key = (DocumentKey) referenceValue.value(options);
-      DatabaseId refDatabase = ((ReferenceValue) 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);
+      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 value.value(options);
+      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()) {

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

@@ -70,16 +70,6 @@ public List<Object> value() {
     return res;
   }
 
-  @Override
-  public List<Object> value(FieldValueOptions options) {
-    // Recursively convert the array into the value that users will see in document snapshots.
-    List<Object> res = new ArrayList<>(internalValue.size());
-    for (FieldValue v : internalValue) {
-      res.add(v.value(options));
-    }
-    return res;
-  }
-
   public List<FieldValue> getInternalValue() {
     return internalValue;
   }

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

@@ -63,15 +63,6 @@ public abstract class FieldValue implements Comparable<FieldValue> {
   @Nullable
   public abstract Object value();
 
-  /**
-   * Converts a FieldValue into the value that users will see in document snapshots using the
-   * provided deserialization options.
-   */
-  @Nullable
-  public Object value(FieldValueOptions options) {
-    return value();
-  }
-
   @Override
   public abstract boolean equals(Object o);
 

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

@@ -67,15 +67,6 @@ public Map<String, Object> value() {
     return res;
   }
 
-  @Override
-  public Map<String, Object> value(FieldValueOptions options) {
-    Map<String, Object> res = new HashMap<>();
-    for (Map.Entry<String, FieldValue> entry : internalValue) {
-      res.put(entry.getKey(), entry.getValue().value(options));
-    }
-    return res;
-  }
-
   public ImmutableSortedMap<String, FieldValue> getInternalValue() {
     return internalValue;
   }

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

@@ -15,7 +15,7 @@
 package com.google.firebase.firestore.model.value;
 
 import com.google.firebase.Timestamp;
-import com.google.firebase.firestore.util.Assert;
+import com.google.firebase.firestore.DocumentSnapshot;
 import javax.annotation.Nullable;
 
 /**
@@ -47,21 +47,23 @@ public Object value() {
     return null;
   }
 
-  @Override
+  /**
+   * Returns the value of the field before this ServerTimestamp was set.
+   *
+   * <p>Preserving the previous values allows the user to display the last resoled value until the
+   * backend responds with the timestamp {@link DocumentSnapshot.ServerTimestampBehavior}.
+   */
   @Nullable
-  public Object value(FieldValueOptions options) {
-    switch (options.getServerTimestampBehavior()) {
-      case PREVIOUS:
-        return previousValue != null ? previousValue.value(options) : null;
-      case ESTIMATE:
-        return new TimestampValue(localWriteTime).value(options);
-      case NONE:
-        return null;
-      default:
-        throw Assert.fail(
-            "Unexpected case for ServerTimestampBehavior: %s",
-            options.getServerTimestampBehavior().name());
+  public Object getPreviousValue() {
+    if (previousValue instanceof ServerTimestampValue) {
+      return ((ServerTimestampValue) previousValue).getPreviousValue();
     }
+
+    return previousValue != null ? previousValue.value() : null;
+  }
+
+  public Timestamp getLocalWriteTime() {
+    return localWriteTime;
   }
 
   @Override

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

@@ -32,19 +32,10 @@ public int typeOrder() {
 
   @Override
   @NonNull
-  public Object value() {
+  public Timestamp value() {
     return internalValue;
   }
 
-  @Override
-  public Object value(FieldValueOptions options) {
-    if (options.areTimestampsInSnapshotsEnabled()) {
-      return internalValue;
-    } else {
-      return internalValue.toDate();
-    }
-  }
-
   public Timestamp getInternalValue() {
     return internalValue;
   }