Return Dates as com.google.cloud.Timestamps

google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentSnapshot.java

@@ -16,23 +16,21 @@
 
 package com.google.cloud.firestore;
 
+import com.google.cloud.Timestamp;
 import com.google.cloud.firestore.UserDataConverter.EncodingOptions;
 import com.google.common.base.Preconditions;
 import com.google.firestore.v1beta1.Document;
 import com.google.firestore.v1beta1.Value;
 import com.google.firestore.v1beta1.Write;
-import com.google.protobuf.Timestamp;
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.concurrent.TimeUnit;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
-import org.threeten.bp.Instant;
 
 /**
  * A DocumentSnapshot contains data read from a document in a Firestore database. The data can be
@@ -51,17 +49,17 @@ public class DocumentSnapshot {
   private final FirestoreImpl firestore;
   private final DocumentReference docRef;
   @Nullable private final Map<String, Value> fields;
-  @Nullable private Instant readTime;
-  @Nullable private Instant updateTime;
-  @Nullable private Instant createTime;
+  @Nullable private final Timestamp readTime;
+  @Nullable private final Timestamp updateTime;
+  @Nullable private final Timestamp createTime;
 
   DocumentSnapshot(
       FirestoreImpl firestore,
       DocumentReference docRef,
       @Nullable Map<String, Value> fields,
-      @Nullable Instant readTime,
-      @Nullable Instant updateTime,
-      @Nullable Instant createTime) {
+      @Nullable Timestamp readTime,
+      @Nullable Timestamp updateTime,
+      @Nullable Timestamp createTime) {
     this.firestore = firestore;
     this.docRef = docRef;
     this.fields = fields;
@@ -98,19 +96,17 @@ static DocumentSnapshot fromObject(
 
   static DocumentSnapshot fromDocument(
       FirestoreImpl firestore, Timestamp readTime, Document document) {
-    Timestamp updateTime = document.getUpdateTime();
-    Timestamp createTime = document.getCreateTime();
     return new DocumentSnapshot(
         firestore,
         new DocumentReference(firestore, ResourcePath.create(document.getName())),
         document.getFieldsMap(),
-        Instant.ofEpochSecond(readTime.getSeconds(), readTime.getNanos()),
-        Instant.ofEpochSecond(updateTime.getSeconds(), updateTime.getNanos()),
-        Instant.ofEpochSecond(createTime.getSeconds(), createTime.getNanos()));
+        readTime,
+        Timestamp.fromProto(document.getUpdateTime()),
+        Timestamp.fromProto(document.getCreateTime()));
   }
 
   static DocumentSnapshot fromMissing(
-      FirestoreImpl firestore, DocumentReference documentReference, Instant readTime) {
+      FirestoreImpl firestore, DocumentReference documentReference, Timestamp readTime) {
     return new DocumentSnapshot(firestore, documentReference, null, readTime, null, null);
   }
 
@@ -126,11 +122,7 @@ private Object decodeValue(Value v) {
       case DOUBLE_VALUE:
         return v.getDoubleValue();
       case TIMESTAMP_VALUE:
-        Timestamp timestamp = v.getTimestampValue();
-        long milliseconds =
-            TimeUnit.SECONDS.toMillis(timestamp.getSeconds())
-                + TimeUnit.NANOSECONDS.toMillis(timestamp.getNanos());
-        return new Date(milliseconds);
+        return Timestamp.fromProto(v.getTimestampValue());
       case STRING_VALUE:
         return v.getStringValue();
       case BYTES_VALUE:
@@ -166,7 +158,7 @@ private Object decodeValue(Value v) {
    * @return The read time of this snapshot.
    */
   @Nullable
-  public Instant getReadTime() {
+  public Timestamp getReadTime() {
     return readTime;
   }
 
@@ -178,7 +170,7 @@ public Instant getReadTime() {
    *     exist.
    */
   @Nullable
-  public Instant getUpdateTime() {
+  public Timestamp getUpdateTime() {
     return updateTime;
   }
 
@@ -189,7 +181,7 @@ public Instant getUpdateTime() {
    *     exist.
    */
   @Nullable
-  public Instant getCreateTime() {
+  public Timestamp getCreateTime() {
     return createTime;
   }
 
@@ -222,7 +214,9 @@ public Map<String, Object> getData() {
 
     Map<String, Object> decodedFields = new HashMap<>();
     for (Map.Entry<String, Value> entry : fields.entrySet()) {
-      decodedFields.put(entry.getKey(), decodeValue(entry.getValue()));
+      Object decodedValue = decodeValue(entry.getValue());
+      decodedValue = convertToDateIfNecessary(decodedValue);
+      decodedFields.put(entry.getKey(), decodedValue);
     }
     return decodedFields;
   }
@@ -287,7 +281,17 @@ public Object get(@Nonnull FieldPath fieldPath) {
       return null;
     }
 
-    return decodeValue(value);
+    Object decodedValue = decodeValue(value);
+    return convertToDateIfNecessary(decodedValue);
+  }
+
+  private Object convertToDateIfNecessary(Object decodedValue) {
+    if (decodedValue instanceof Timestamp) {
+      if (!this.firestore.areTimestampsInSnapshotsEnabled()) {
+        decodedValue = ((Timestamp) decodedValue).toDate();
+      }
+    }
+    return decodedValue;
   }
 
   /** Returns the Value Proto at 'fieldPath'. Returns null if the field was not found. */
@@ -363,13 +367,31 @@ public Long getLong(@Nonnull String field) {
   /**
    * Returns the value of the field as a Date.
    *
+   * <p>This method ignores the global setting {@link
+   * FirestoreOptions#areTimestampsInSnapshotsEnabled}.
+   *
    * @param field The path to the field.
    * @throws RuntimeException if the value is not a Date.
    * @return The value of the field.
    */
   @Nullable
   public Date getDate(@Nonnull String field) {
-    return (Date) get(field);
+    return ((Timestamp) get(field)).toDate();
+  }
+
+  /**
+   * Returns the value of the field as a {@link Timestamp}.
+   *
+   * <p>This method ignores the global setting {@link
+   * FirestoreOptions#areTimestampsInSnapshotsEnabled}.
+   *
+   * @param field The path to the field.
+   * @throws RuntimeException if the value is not a Date.
+   * @return The value of the field.
+   */
+  @Nullable
+  public Timestamp getTimestamp(@Nonnull String field) {
+    return (Timestamp) get(field);
   }
 
   /**

google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreImpl.java

@@ -25,6 +25,7 @@
 import com.google.api.gax.rpc.BidiStreamingCallable;
 import com.google.api.gax.rpc.ServerStreamingCallable;
 import com.google.api.gax.rpc.UnaryCallable;
+import com.google.cloud.Timestamp;
 import com.google.cloud.firestore.spi.v1beta1.FirestoreRpc;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
@@ -45,9 +46,9 @@
 import java.util.Map;
 import java.util.Random;
 import java.util.concurrent.Executor;
+import java.util.logging.Logger;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
-import org.threeten.bp.Instant;
 
 /**
  * Main implementation of the Firestore client. This is the entry point for all Firestore
@@ -60,6 +61,7 @@ class FirestoreImpl implements Firestore {
   private static final String AUTO_ID_ALPHABET =
       "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";
 
+  private static final Logger LOGGER = Logger.getLogger("Firestore");
   private static final Tracer tracer = Tracing.getTracer();
   private static final io.opencensus.trace.Status TOO_MANY_RETRIES_STATUS =
       io.opencensus.trace.Status.ABORTED.withDescription("too many retries");
@@ -83,6 +85,34 @@ class FirestoreImpl implements Firestore {
             + "Please explicitly set your Project ID in FirestoreOptions.");
     this.databasePath =
         ResourcePath.create(DatabaseRootName.of(options.getProjectId(), options.getDatabaseId()));
+
+    if (!options.areTimestampsInSnapshotsEnabled()) {
+      LOGGER.warning(
+          "The behavior for java.util.Date objects stored in Firestore is going to change "
+              + "AND YOUR APP MAY BREAK.\n"
+              + "To hide this warning and ensure your app does not break, you need to add "
+              + "the following code to your app before calling any other Cloud Firestore "
+              + "methods:\n"
+              + "\n"
+              + "FirestoreOptions options = \n"
+              + "  FirestoreOptions.newBuilder().setTimestampsInSnapshotsEnabled(true).build();\n"
+              + "Firestore firestore = options.getService();\n"
+              + "\n"
+              + "With this change, timestamps stored in Cloud Firestore will be read back as "
+              + "com.google.cloud.Timestamp objects instead of as system java.util.Date "
+              + "objects. So you will also need to update code expecting a java.util.Date to "
+              + "instead expect a Timestamp. For example:\n"
+              + "\n"
+              + "// Old:\n"
+              + "java.util.Date date = (java.util.Date) snapshot.get(\"created_at\");\n"
+              + "// New:\n"
+              + "Timestamp timestamp = (Timestamp) snapshot.get(\"created_at\");\n"
+              + "java.util.Date date = timestamp.toDate();\n"
+              + "\n"
+              + "Please audit all existing usages of java.util.Date when you enable the new "
+              + "behavior. In a future release, the behavior will be changed to the new "
+              + "behavior, so if you do not follow these steps, YOUR APP MAY BREAK.");
+    }
   }
 
   /** Creates a pseudo-random 20-character ID that can be used for Firestore documents. */
@@ -159,7 +189,9 @@ public void onNext(BatchGetDocumentsResponse response) {
                         FirestoreImpl.this, ResourcePath.create(response.getFound().getName()));
                 documentSnapshot =
                     DocumentSnapshot.fromDocument(
-                        FirestoreImpl.this, response.getReadTime(), response.getFound());
+                        FirestoreImpl.this,
+                        Timestamp.fromProto(response.getReadTime()),
+                        response.getFound());
                 break;
               case MISSING:
                 documentReference =
@@ -169,9 +201,7 @@ public void onNext(BatchGetDocumentsResponse response) {
                     DocumentSnapshot.fromMissing(
                         FirestoreImpl.this,
                         documentReference,
-                        Instant.ofEpochSecond(
-                            response.getReadTime().getSeconds(),
-                            response.getReadTime().getNanos()));
+                        Timestamp.fromProto(response.getReadTime()));
                 break;
               default:
                 return;
@@ -369,6 +399,11 @@ public void onSuccess(Void ignored) {
         });
   }
 
+  /** Returns whether the user has opted into receiving dates as com.google.cloud.Timestamp. */
+  boolean areTimestampsInSnapshotsEnabled() {
+    return this.firestoreOptions.areTimestampsInSnapshotsEnabled();
+  }
+
   /** Returns the name of the Firestore project associated with this client. */
   String getDatabaseName() {
     return databasePath.toString();

google-cloud-clients/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreOptions.java

@@ -40,9 +40,11 @@ public final class FirestoreOptions extends ServiceOptions<Firestore, FirestoreO
           .build();
   private static final String DEFAULT_HOST = FirestoreSettings.getDefaultEndpoint();
   private static final String DEFAULT_DATABASE_ID = "(default)";
+  private static final boolean DEFAULT_TIMESTAMPS_IN_SNAPSHOTS_ENABLED = false;
   private static final long serialVersionUID = -5853552236134770088L;
 
   private final String databaseId;
+  private final boolean timestampsInSnapshotsEnabled;
 
   public static class DefaultFirestoreFactory implements FirestoreFactory {
 
@@ -92,11 +94,13 @@ public String getDatabaseId() {
   public static class Builder extends ServiceOptions.Builder<Firestore, FirestoreOptions, Builder> {
 
     private String databaseId = DEFAULT_DATABASE_ID;
+    private boolean timestampsInSnapshotsEnabled = DEFAULT_TIMESTAMPS_IN_SNAPSHOTS_ENABLED;
 
     private Builder() {}
 
     private Builder(FirestoreOptions options) {
       super(options);
+      timestampsInSnapshotsEnabled = options.timestampsInSnapshotsEnabled;
     }
 
     @Nonnull
@@ -106,26 +110,56 @@ public Builder setTransportOptions(@Nonnull TransportOptions transportOptions) {
         throw new IllegalArgumentException(
             "Only grpc transport is allowed for " + API_SHORT_NAME + ".");
       }
-      return super.setTransportOptions(transportOptions);
+      super.setTransportOptions(transportOptions);
+      return this;
     }
 
-    @Override
+    /**
+     * Enables the use of {@link com.google.cloud.Timestamp Timestamps} for timestamp fields in
+     * {@link DocumentSnapshot DocumentSnapshots}.
+     *
+     * <p>Currently, Firestore returns timestamp fields as {@link java.util.Date} but {@link
+     * java.util.Date Date} only supports millisecond precision, which leads to truncation and
+     * causes unexpected behavior when using a timestamp from a snapshot as a part of a subsequent
+     * query.
+     *
+     * <p>Setting {@code setTimestampsInSnapshotsEnabled(true)} will cause Firestore to return
+     * {@link com.google.cloud.Timestamp Timestamp} values instead of {@link java.util.Date Date},
+     * avoiding this kind of problem. To make this work you must also change any code that uses
+     * {@link java.util.Date Date} to use {@link com.google.cloud.Timestamp Timestamp} instead.
+     *
+     * <p>NOTE: in the future {@link FirestoreOptions#areTimestampsInSnapshotsEnabled} will default
+     * to true and this option will be removed so you should change your code to use Timestamp now
+     * and opt-in to this new behavior as soon as you can.
+     *
+     * @return A settings object on which the return type for timestamp fields is configured as
+     *     specified by the given {@code value}.
+     */
     @Nonnull
-    public FirestoreOptions build() {
-      return new FirestoreOptions(this);
+    public Builder setTimestampsInSnapshotsEnabled(boolean value) {
+      this.timestampsInSnapshotsEnabled = value;
+      return this;
     }
 
-    public Builder setDatabaseId(String databaseId) {
+    @Nonnull
+    public Builder setDatabaseId(@Nonnull String databaseId) {
       this.databaseId = databaseId;
       return this;
     }
+
+    @Override
+    @Nonnull
+    public FirestoreOptions build() {
+      return new FirestoreOptions(this);
+    }
   }
 
   @InternalApi("This class should only be extended within google-cloud-java")
   protected FirestoreOptions(Builder builder) {
     super(FirestoreFactory.class, FirestoreRpcFactory.class, builder, new FirestoreDefaults());
 
     this.databaseId = builder.databaseId;
+    this.timestampsInSnapshotsEnabled = builder.timestampsInSnapshotsEnabled;
   }
 
   private static class FirestoreDefaults implements ServiceDefaults<Firestore, FirestoreOptions> {
@@ -154,6 +188,14 @@ public static GrpcTransportOptions getDefaultGrpcTransportOptions() {
     return GrpcTransportOptions.newBuilder().build();
   }
 
+  /**
+   * Returns whether or not {@link DocumentSnapshot DocumentSnapshots} return timestamp fields as
+   * {@link com.google.cloud.Timestamp Timestamps}.
+   */
+  public boolean areTimestampsInSnapshotsEnabled() {
+    return timestampsInSnapshotsEnabled;
+  }
+
   @Override
   protected Set<String> getScopes() {
     return SCOPES;