Skip to content

Change timestampsInSnapshots default to true and deprecate the setting. #190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,6 @@ private static Map<String, Object> objectWithTimestamp(Timestamp timestamp) {
return map("timestamp", timestamp, "nested", map("timestamp2", timestamp));
}

// Note: because timestampsInSnapshotsEnabled is set to true in default test settings, this test
// is unaffected by the current default value in FirebaseFirestoreSettings.
@Test
public void testTimestampsInSnapshots() {
Timestamp originalTimestamp = new Timestamp(100, 123456789);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ public static FirebaseFirestoreSettings newTestSettings() {
return newTestSettingsWithSnapshotTimestampsEnabled(true);
}

@SuppressWarnings("deprecation") // for setTimestampsInSnapshotsEnabled()
public static FirebaseFirestoreSettings newTestSettingsWithSnapshotTimestampsEnabled(
boolean enabled) {
return new FirebaseFirestoreSettings.Builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,36 +186,6 @@ private void ensureClientConfigured() {
if (client != null) {
return;
}
if (!settings.areTimestampsInSnapshotsEnabled()) {
Logger.warn(
"Firestore",
"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"
+ "FirebaseFirestore firestore = FirebaseFirestore.getInstance();\n"
+ "FirebaseFirestoreSettings settings = new FirebaseFirestoreSettings.Builder()\n"
+ " .setTimestampsInSnapshotsEnabled(true)\n"
+ " .build();\n"
+ "firestore.setFirestoreSettings(settings);\n"
+ "\n"
+ "With this change, timestamps stored in Cloud Firestore will be read back as "
+ "com.google.firebase.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 = snapshot.getDate(\"created_at\");\n"
+ "// New:\n"
+ "Timestamp timestamp = snapshot.getTimestamp(\"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.");
}
DatabaseInfo databaseInfo =
new DatabaseInfo(databaseId, persistenceKey, settings.getHost(), settings.isSslEnabled());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public final class FirebaseFirestoreSettings {
// we will switch the default to the above value, 100 MB.
private static final long DEFAULT_CACHE_SIZE_BYTES = CACHE_SIZE_UNLIMITED;
private static final String DEFAULT_HOST = "firestore.googleapis.com";
private static final boolean DEFAULT_TIMESTAMPS_IN_SNAPSHOTS_ENABLED = false;
private static final boolean DEFAULT_TIMESTAMPS_IN_SNAPSHOTS_ENABLED = true;

/** A Builder for creating {@link FirebaseFirestoreSettings}. */
@PublicApi
Expand Down Expand Up @@ -110,28 +110,27 @@ public Builder setPersistenceEnabled(boolean value) {
}

/**
* Enables the use of {@link com.google.firebase.Timestamp Timestamps} for timestamp fields in
* {@link DocumentSnapshot DocumentSnapshots}.
* Specifies whether to use {@link com.google.firebase.Timestamp Timestamps} for timestamp
* fields in {@link DocumentSnapshot DocumentSnapshots}. This is now enabled by default and
* should not be disabled.
*
* <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>Previously, Firestore returned timestamp fields as {@link java.util.Date} but {@link
* java.util.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.firebase.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.firebase.Timestamp Timestamp}
* instead.
* <p>So now Firestore returns {@link com.google.firebase.Timestamp Timestamp} values instead of
* {@link java.util.Date}, avoiding this kind of problem.
*
* <p>NOTE: in the future {@link FirebaseFirestoreSettings#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.
* <p>To opt into the old behavior of returning {@link java.util.Date Dates}, you can
* temporarily set {@link FirebaseFirestoreSettings#areTimestampsInSnapshotsEnabled} to false.
*
* @return A settings object on which the return type for timestamp fields is configured as
* specified by the given {@code value}.
* @deprecated This setting now defaults to true and will be removed in a future release. If you
* are already setting it to true, just remove the setting. If you are setting it to false,
* you should update your code to expect {@link com.google.firebase.Timestamp Timestamps}
* instead of {@link java.util.Date Dates} and then remove the setting.
*/
@NonNull
@Deprecated
@PublicApi
public Builder setTimestampsInSnapshotsEnabled(boolean value) {
this.timestampsInSnapshotsEnabled = value;
Expand Down