Skip to content

Commit f9ce7d3

Browse files
schmidt-sebastianpongad
schmidt-sebastian
authored and
pongad
committed
Using commitTime instead of updateTime for deletes (#2493)
1 parent 1dad07a commit f9ce7d3

7 files changed

Lines changed: 33 additions & 23 deletions

File tree

‎google-cloud-firestore/src/main/java/com/google/cloud/firestore/UpdateBuilder.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -472,12 +472,13 @@ ApiFuture<List<WriteResult>> commit(@Nullable ByteString transactionId) {
472472
new ApiFunction<CommitResponse, List<WriteResult>>() {
473473
@Override
474474
public List<WriteResult> apply(CommitResponse commitResponse) {
475-
List<com.google.firestore.v1beta1.WriteResult> protoWriteResultList =
475+
List<com.google.firestore.v1beta1.WriteResult> writeResults =
476476
commitResponse.getWriteResultsList();
477477

478478
List<WriteResult> writeResultList = new ArrayList<>();
479-
for (com.google.firestore.v1beta1.WriteResult protoWriteResult : protoWriteResultList) {
480-
writeResultList.add(WriteResult.fromProto(protoWriteResult));
479+
for (com.google.firestore.v1beta1.WriteResult writeResult : writeResults) {
480+
writeResultList.add(
481+
WriteResult.fromProto(writeResult, commitResponse.getCommitTime()));
481482
}
482483

483484
return writeResultList;

‎google-cloud-firestore/src/main/java/com/google/cloud/firestore/WriteResult.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,9 @@ public Instant getUpdateTime() {
3939
return this.updateTime;
4040
}
4141

42-
static WriteResult fromProto(com.google.firestore.v1beta1.WriteResult protoWriteResult) {
43-
Timestamp timestamp = protoWriteResult.getUpdateTime();
42+
static WriteResult fromProto(
43+
com.google.firestore.v1beta1.WriteResult writeResult, Timestamp commitTime) {
44+
Timestamp timestamp = writeResult.hasUpdateTime() ? writeResult.getUpdateTime() : commitTime;
4445
Instant instant = Instant.ofEpochSecond(timestamp.getSeconds(), timestamp.getNanos());
4546
return new WriteResult(instant);
4647
}

‎google-cloud-firestore/src/test/java/com/google/cloud/firestore/DocumentReferenceTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import static com.google.cloud.firestore.LocalFirestoreHelper.NESTED_CLASS_OBJECT;
2828
import static com.google.cloud.firestore.LocalFirestoreHelper.SERVER_TIMESTAMP_PROTO;
2929
import static com.google.cloud.firestore.LocalFirestoreHelper.SERVER_TIMESTAMP_TRANSFORM;
30+
import static com.google.cloud.firestore.LocalFirestoreHelper.SINGLE_DELETE_COMMIT_RESPONSE;
3031
import static com.google.cloud.firestore.LocalFirestoreHelper.SINGLE_FIELD_MAP;
3132
import static com.google.cloud.firestore.LocalFirestoreHelper.SINGLE_FIELD_OBJECT;
3233
import static com.google.cloud.firestore.LocalFirestoreHelper.SINGLE_FIELD_PROTO;
@@ -255,7 +256,7 @@ public void notFound() throws Exception {
255256

256257
@Test
257258
public void deleteDocument() throws Exception {
258-
doReturn(SINGLE_WRITE_COMMIT_RESPONSE)
259+
doReturn(SINGLE_DELETE_COMMIT_RESPONSE)
259260
.when(firestoreMock)
260261
.sendRequest(
261262
commitCapture.capture(), Matchers.<UnaryCallable<CommitRequest, CommitResponse>>any());

‎google-cloud-firestore/src/test/java/com/google/cloud/firestore/LocalFirestoreHelper.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ public final class LocalFirestoreHelper {
9292
public static final Map<String, Value> ALL_SUPPORTED_TYPES_PROTO;
9393

9494
public static final CommitRequest SINGLE_CREATE_COMMIT_REQUEST;
95+
public static final ApiFuture<CommitResponse> SINGLE_DELETE_COMMIT_RESPONSE;
9596
public static final ApiFuture<CommitResponse> SINGLE_WRITE_COMMIT_RESPONSE;
9697

9798
public static final Date DATE;
@@ -196,12 +197,15 @@ public T answer(InvocationOnMock invocation) {
196197
};
197198
}
198199

199-
public static ApiFuture<CommitResponse> commitResponse(int count) {
200+
public static ApiFuture<CommitResponse> commitResponse(int adds, int deletes) {
200201
CommitResponse.Builder commitResponse = CommitResponse.newBuilder();
201202
commitResponse.getCommitTimeBuilder().setSeconds(0).setNanos(0);
202-
for (int i = 0; i < count; ++i) {
203+
for (int i = 0; i < adds; ++i) {
203204
commitResponse.addWriteResultsBuilder().getUpdateTimeBuilder().setSeconds(i).setNanos(i);
204205
}
206+
for (int i = 0; i < deletes; ++i) {
207+
commitResponse.addWriteResultsBuilder();
208+
}
205209
return ApiFutures.immediateFuture(commitResponse.build());
206210
}
207211

@@ -628,7 +632,8 @@ public boolean equals(Object o) {
628632
.build();
629633
ALL_SUPPORTED_TYPES_OBJECT = new AllSupportedTypes();
630634

631-
SINGLE_WRITE_COMMIT_RESPONSE = commitResponse(1);
635+
SINGLE_WRITE_COMMIT_RESPONSE = commitResponse(/* adds= */ 1, /* deletes= */ 0);
636+
SINGLE_DELETE_COMMIT_RESPONSE = commitResponse(/* adds= */ 0, /* deletes= */ 1);
632637
SINGLE_CREATE_COMMIT_REQUEST = commit(create(SINGLE_FIELD_PROTO));
633638

634639
NESTED_CLASS_OBJECT = new NestedClass();

‎google-cloud-firestore/src/test/java/com/google/cloud/firestore/TransactionTest.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ public Thread newThread(Runnable runnable) {
104104
@Test
105105
public void returnsValue() throws Exception {
106106
doReturn(beginResponse())
107-
.doReturn(commitResponse(0))
107+
.doReturn(commitResponse(0, 0))
108108
.when(firestoreMock)
109109
.sendRequest(requestCapture.capture(), Matchers.<UnaryCallable<Message, Message>>any());
110110

@@ -133,7 +133,7 @@ public void canReturnNull() throws Exception {
133133
doReturn(beginResponse())
134134
.doReturn(ApiFutures.immediateFailedFuture(new Exception()))
135135
.doReturn(beginResponse(ByteString.copyFromUtf8("foo2")))
136-
.doReturn(commitResponse(0))
136+
.doReturn(commitResponse(0, 0))
137137
.when(firestoreMock)
138138
.sendRequest(requestCapture.capture(), Matchers.<UnaryCallable<Message, Message>>any());
139139

@@ -269,7 +269,7 @@ public void limitsRetriesWithSuccess() throws Exception {
269269
.doReturn(beginResponse(ByteString.copyFromUtf8("foo5")))
270270
.doReturn(ApiFutures.immediateFailedFuture(new Exception()))
271271
.doReturn(beginResponse(ByteString.copyFromUtf8("foo6")))
272-
.doReturn(commitResponse(0))
272+
.doReturn(commitResponse(0, 0))
273273
.when(firestoreMock)
274274
.sendRequest(requestCapture.capture(), Matchers.<UnaryCallable<Message, Message>>any());
275275

@@ -305,7 +305,7 @@ public String updateCallback(Transaction transaction) {
305305
@Test
306306
public void getDocument() throws Exception {
307307
doReturn(beginResponse())
308-
.doReturn(commitResponse(0))
308+
.doReturn(commitResponse(0, 0))
309309
.when(firestoreMock)
310310
.sendRequest(requestCapture.capture(), Matchers.<UnaryCallable<Message, Message>>any());
311311

@@ -340,7 +340,7 @@ public DocumentSnapshot updateCallback(Transaction transaction)
340340
@Test
341341
public void getQuery() throws Exception {
342342
doReturn(beginResponse())
343-
.doReturn(commitResponse(0))
343+
.doReturn(commitResponse(0, 0))
344344
.when(firestoreMock)
345345
.sendRequest(requestCapture.capture(), Matchers.<UnaryCallable<Message, Message>>any());
346346

@@ -375,7 +375,7 @@ public QuerySnapshot updateCallback(Transaction transaction)
375375
@Test
376376
public void updateDocument() throws Exception {
377377
doReturn(beginResponse())
378-
.doReturn(commitResponse(0))
378+
.doReturn(commitResponse(0, 0))
379379
.when(firestoreMock)
380380
.sendRequest(requestCapture.capture(), Matchers.<UnaryCallable<Message, Message>>any());
381381

@@ -414,7 +414,7 @@ public String updateCallback(Transaction transaction) {
414414
@Test
415415
public void setDocument() throws Exception {
416416
doReturn(beginResponse())
417-
.doReturn(commitResponse(0))
417+
.doReturn(commitResponse(0, 0))
418418
.when(firestoreMock)
419419
.sendRequest(requestCapture.capture(), Matchers.<UnaryCallable<Message, Message>>any());
420420

@@ -449,7 +449,7 @@ public String updateCallback(Transaction transaction) {
449449
@Test
450450
public void createDocument() throws Exception {
451451
doReturn(beginResponse())
452-
.doReturn(commitResponse(0))
452+
.doReturn(commitResponse(0, 0))
453453
.when(firestoreMock)
454454
.sendRequest(requestCapture.capture(), Matchers.<UnaryCallable<Message, Message>>any());
455455

@@ -484,7 +484,7 @@ public String updateCallback(Transaction transaction) {
484484
@Test
485485
public void deleteDocument() throws Exception {
486486
doReturn(beginResponse())
487-
.doReturn(commitResponse(0))
487+
.doReturn(commitResponse(0, 0))
488488
.when(firestoreMock)
489489
.sendRequest(requestCapture.capture(), Matchers.<UnaryCallable<Message, Message>>any());
490490

‎google-cloud-firestore/src/test/java/com/google/cloud/firestore/WriteBatchTest.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public void before() {
6767

6868
@Test
6969
public void updateDocument() throws Exception {
70-
doReturn(commitResponse(4))
70+
doReturn(commitResponse(4, 0))
7171
.when(firestoreMock)
7272
.sendRequest(
7373
commitCapture.capture(), Matchers.<UnaryCallable<CommitRequest, CommitResponse>>any());
@@ -92,7 +92,7 @@ public void updateDocument() throws Exception {
9292

9393
@Test
9494
public void setDocument() throws Exception {
95-
doReturn(commitResponse(2))
95+
doReturn(commitResponse(2, 0))
9696
.when(firestoreMock)
9797
.sendRequest(
9898
commitCapture.capture(), Matchers.<UnaryCallable<CommitRequest, CommitResponse>>any());
@@ -120,7 +120,7 @@ public void setDocument() throws Exception {
120120

121121
@Test
122122
public void createDocument() throws Exception {
123-
doReturn(commitResponse(2))
123+
doReturn(commitResponse(2, 0))
124124
.when(firestoreMock)
125125
.sendRequest(
126126
commitCapture.capture(), Matchers.<UnaryCallable<CommitRequest, CommitResponse>>any());
@@ -143,7 +143,7 @@ public void createDocument() throws Exception {
143143

144144
@Test
145145
public void deleteDocument() throws Exception {
146-
doReturn(commitResponse(2))
146+
doReturn(commitResponse(2, 0))
147147
.when(firestoreMock)
148148
.sendRequest(
149149
commitCapture.capture(), Matchers.<UnaryCallable<CommitRequest, CommitResponse>>any());

‎google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,9 +213,11 @@ public void deleteDocument() throws Exception {
213213
} catch (ExecutionException e) {
214214
assertTrue(e.getMessage().contains("FAILED_PRECONDITION"));
215215
}
216-
documentReference.delete(Precondition.updatedAt(writeResult.getUpdateTime())).get();
216+
writeResult =
217+
documentReference.delete(Precondition.updatedAt(writeResult.getUpdateTime())).get();
217218
DocumentSnapshot documentSnapshot = documentReference.get().get();
218219
assertFalse(documentSnapshot.exists());
220+
assertTrue(writeResult.getUpdateTime().getEpochSecond() > 0);
219221
}
220222

221223
@Test

0 commit comments

Comments
 (0)