Skip to content
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

Firestore: Change timestampsInSnapshots default to true. #4353

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 @@ -85,34 +85,6 @@ 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. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public final class FirestoreOptions extends ServiceOptions<Firestore, FirestoreO
.add("https://www.googleapis.com/auth/cloud-platform")
.add("https://www.googleapis.com/auth/datastore")
.build();
private static final boolean DEFAULT_TIMESTAMPS_IN_SNAPSHOTS_ENABLED = false;
private static final boolean DEFAULT_TIMESTAMPS_IN_SNAPSHOTS_ENABLED = true;

private static final long serialVersionUID = -5853552236134770090L;

Expand Down Expand Up @@ -179,26 +179,26 @@ public Builder setDatabaseId(@Nonnull String databaseId) {
}

/**
* Enables the use of {@link com.google.cloud.Timestamp Timestamps} for timestamp fields in
* {@link DocumentSnapshot DocumentSnapshots}.
* Specifies whether to use {@link com.google.cloud.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.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>So now Firestore returns {@link com.google.cloud.Timestamp Timestamp} values instead of
* {@link java.util.Date}, avoiding this kind of problem.
*
* <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.
* <p>To opt into the old behavior of returning {@link java.util.Date Dates}, you can
* temporarily set {@link FirestoreOptions#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.cloud.Timestamp Timestamps}
* instead of {@link java.util.Date Dates} and then remove the setting.
*/
@Deprecated
@Nonnull
public Builder setTimestampsInSnapshotsEnabled(boolean value) {
this.timestampsInSnapshotsEnabled = value;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,7 @@ public class CollectionReferenceTest {
@Spy
private FirestoreImpl firestoreMock =
new FirestoreImpl(
FirestoreOptions.newBuilder()
.setProjectId("test-project")
.setTimestampsInSnapshotsEnabled(true)
.build(),
FirestoreOptions.newBuilder().setProjectId("test-project").build(),
Mockito.mock(FirestoreRpc.class));

@Captor private ArgumentCaptor<CommitRequest> argCaptor;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,7 @@ public ConformanceTest() {
firestoreMock =
Mockito.spy(
new FirestoreImpl(
FirestoreOptions.newBuilder()
.setProjectId("projectID")
.setTimestampsInSnapshotsEnabled(true)
.build(),
firestoreRpc));
FirestoreOptions.newBuilder().setProjectId("projectID").build(), firestoreRpc));
watchQuery = collection("projects/projectID/databases/(default)/documents/C").orderBy("a");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,7 @@ public class DocumentReferenceTest {
@Spy
private FirestoreImpl firestoreMock =
new FirestoreImpl(
FirestoreOptions.newBuilder()
.setProjectId("test-project")
.setTimestampsInSnapshotsEnabled(true)
.build(),
FirestoreOptions.newBuilder().setProjectId("test-project").build(),
Mockito.mock(FirestoreRpc.class));

@Captor private ArgumentCaptor<CommitRequest> commitCapture;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,7 @@ public class FirestoreTest {
@Spy
private FirestoreImpl firestoreMock =
new FirestoreImpl(
FirestoreOptions.newBuilder()
.setProjectId("test-project")
.setTimestampsInSnapshotsEnabled(true)
.build(),
FirestoreOptions.newBuilder().setProjectId("test-project").build(),
Mockito.mock(FirestoreRpc.class));

@Captor private ArgumentCaptor<BatchGetDocumentsRequest> getAllCapture;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,7 @@ public class QueryTest {
@Spy
private FirestoreImpl firestoreMock =
new FirestoreImpl(
FirestoreOptions.newBuilder()
.setProjectId("test-project")
.setTimestampsInSnapshotsEnabled(true)
.build(),
FirestoreOptions.newBuilder().setProjectId("test-project").build(),
Mockito.mock(FirestoreRpc.class));

@Captor private ArgumentCaptor<RunQueryRequest> runQuery;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,7 @@ public class TransactionTest {
@Spy
private FirestoreImpl firestoreMock =
new FirestoreImpl(
FirestoreOptions.newBuilder()
.setProjectId("test-project")
.setTimestampsInSnapshotsEnabled(true)
.build(),
firestoreRpc);
FirestoreOptions.newBuilder().setProjectId("test-project").build(), firestoreRpc);

@Captor private ArgumentCaptor<Message> requestCapture;
@Captor private ArgumentCaptor<ApiStreamObserver<Message>> streamObserverCapture;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,7 @@ public class WatchTest {
@Spy
private FirestoreImpl firestoreMock =
new FirestoreImpl(
FirestoreOptions.newBuilder()
.setProjectId("test-project")
.setTimestampsInSnapshotsEnabled(true)
.build(),
firestoreRpc);
FirestoreOptions.newBuilder().setProjectId("test-project").build(), firestoreRpc);

@Captor private ArgumentCaptor<ApiStreamObserver<ListenResponse>> streamObserverCapture;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,7 @@ public class WriteBatchTest {
@Spy
private FirestoreImpl firestoreMock =
new FirestoreImpl(
FirestoreOptions.newBuilder()
.setProjectId("test-project")
.setTimestampsInSnapshotsEnabled(true)
.build(),
FirestoreOptions.newBuilder().setProjectId("test-project").build(),
Mockito.mock(FirestoreRpc.class));

@Captor private ArgumentCaptor<CommitRequest> commitCapture;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,7 @@ public class ITSystemTest {

@Before
public void before() {
FirestoreOptions firestoreOptions =
FirestoreOptions.newBuilder().setTimestampsInSnapshotsEnabled(true).build();
FirestoreOptions firestoreOptions = FirestoreOptions.newBuilder().build();
firestore = firestoreOptions.getService();
randomColl =
firestore.collection(
Expand Down