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

chore!: remove deprecated FirestoreOptions#setTimestampsInSnapshotsEnabled #308

Merged
merged 1 commit into from Jul 23, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion google-cloud-firestore/clirr-ignored-differences.xml
Expand Up @@ -17,7 +17,6 @@

<!-- see http://www.mojohaus.org/clirr-maven-plugin/examples/ignored-differences.html -->
<differences>

<!-- v2.0.0 -->
<difference>
<differenceType>7002</differenceType>
Expand All @@ -29,6 +28,11 @@
<className>com/google/cloud/firestore/Firestore</className>
<method>java.lang.Iterable getCollections()</method>
</difference>
<difference>
<differenceType>7002</differenceType>
<className>com/google/cloud/firestore/FirestoreOptions$Builder</className>
<method>com.google.cloud.firestore.FirestoreOptions$Builder setTimestampsInSnapshotsEnabled(boolean)</method>
</difference>

<!--
createIndex -> createIndexAsync
Expand Down
Expand Up @@ -215,7 +215,6 @@ public Map<String, Object> getData() {
Map<String, Object> decodedFields = new HashMap<>();
for (Map.Entry<String, Value> entry : fields.entrySet()) {
Object decodedValue = decodeValue(entry.getValue());
decodedValue = convertToDateIfNecessary(decodedValue);
decodedFields.put(entry.getKey(), decodedValue);
}
return decodedFields;
Expand Down Expand Up @@ -294,8 +293,7 @@ public Object get(@Nonnull FieldPath fieldPath) {
return null;
}

Object decodedValue = decodeValue(value);
return convertToDateIfNecessary(decodedValue);
return decodeValue(value);
}

/**
Expand All @@ -312,15 +310,6 @@ public <T> T get(@Nonnull FieldPath fieldPath, Class<T> valueType) {
return data == null ? null : CustomClassMapper.convertToCustomClass(data, valueType, docRef);
}

private Object convertToDateIfNecessary(Object decodedValue) {
if (decodedValue instanceof Timestamp) {
if (!this.rpcContext.areTimestampsInSnapshotsEnabled()) {
decodedValue = ((Timestamp) decodedValue).toDate();
}
}
return decodedValue;
}

/** Returns the Value Proto at 'fieldPath'. Returns null if the field was not found. */
@Nullable
Value extractField(@Nonnull FieldPath fieldPath) {
Expand Down Expand Up @@ -394,9 +383,6 @@ 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.
Expand All @@ -409,9 +395,6 @@ public Date getDate(@Nonnull String field) {
/**
* 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.
Expand Down
Expand Up @@ -310,12 +310,6 @@ public <T> ApiFuture<T> runAsyncTransaction(
return transactionRunner.run();
}

/** Returns whether the user has opted into receiving dates as com.google.cloud.Timestamp. */
@Override
public boolean areTimestampsInSnapshotsEnabled() {
return this.firestoreOptions.areTimestampsInSnapshotsEnabled();
}

/** Returns the name of the Firestore project associated with this client. */
@Override
public String getDatabaseName() {
Expand Down
Expand Up @@ -189,33 +189,6 @@ public Builder setDatabaseId(@Nonnull String databaseId) {
return this;
}

/**
* 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>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>So now Firestore returns {@link com.google.cloud.Timestamp Timestamp} values instead of
* {@link java.util.Date}, avoiding this kind of problem.
*
* <p>To opt into the old behavior of returning {@link java.util.Date Dates}, you can
* temporarily set {@link FirestoreOptions#areTimestampsInSnapshotsEnabled} to false.
*
* @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;
return this;
}

@Override
@Nonnull
public FirestoreOptions build() {
Expand Down
Expand Up @@ -31,8 +31,6 @@ interface FirestoreRpcContext<FS extends Firestore> {

FS getFirestore();

boolean areTimestampsInSnapshotsEnabled();

String getDatabaseName();

ResourcePath getResourcePath();
Expand Down
Expand Up @@ -211,8 +211,6 @@ public void deserializeBasicTypes() throws Exception {
streamObserverCapture.capture(),
Matchers.<ServerStreamingCallable>any());

doReturn(true).when(firestoreMock).areTimestampsInSnapshotsEnabled();

DocumentSnapshot snapshot = documentReference.get().get();
assertEquals(snapshot.getData(), ALL_SUPPORTED_TYPES_MAP);

Expand Down Expand Up @@ -299,15 +297,6 @@ public void deserializesDates() throws Exception {

DocumentSnapshot snapshot = documentReference.get().get();

doReturn(false).when(firestoreMock).areTimestampsInSnapshotsEnabled();

assertEquals(DATE, snapshot.get("dateValue"));
assertEquals(TIMESTAMP.toDate(), snapshot.get("timestampValue"));
assertEquals(DATE, snapshot.getData().get("dateValue"));
assertEquals(TIMESTAMP.toDate(), snapshot.getData().get("timestampValue"));

doReturn(true).when(firestoreMock).areTimestampsInSnapshotsEnabled();

assertEquals(Timestamp.of(DATE), snapshot.get("dateValue"));
assertEquals(TIMESTAMP, snapshot.get("timestampValue"));
assertEquals(Timestamp.of(DATE), snapshot.getData().get("dateValue"));
Expand Down