From 691d7b7ada2a830cd4504ef8ab1df8ba990424ad Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Fri, 13 Nov 2026 21:58:57 +0000 Subject: [PATCH] fix(firestore): retry idempotent commits on DEADLINE_EXCEEDED --- .../cloud/firestore/DocumentTransform.java | 29 +- .../google/cloud/firestore/FieldValue.java | 38 ++ .../google/cloud/firestore/UpdateBuilder.java | 120 +++++- .../cloud/firestore/WriteBatchTest.java | 368 ++++++++++++++++++ 4 files changed, 542 insertions(+), 13 deletions(-) diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentTransform.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentTransform.java index f6343a7e2c49..4cebfc9fd3b7 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentTransform.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentTransform.java @@ -31,25 +31,31 @@ final class DocumentTransform { private final SortedMap transforms; // Sorted for testing. + private final boolean isIdempotent; - private DocumentTransform(SortedMap transforms) { + private DocumentTransform(SortedMap transforms, boolean isIdempotent) { this.transforms = transforms; + this.isIdempotent = isIdempotent; } static DocumentTransform fromFieldPathMap(Map values) { SortedMap transforms = new TreeMap<>(); + boolean isIdempotent = true; for (Map.Entry entry : values.entrySet()) { FieldPath path = entry.getKey(); Object value = entry.getValue(); if (value instanceof FieldValue) { FieldValue fieldValue = (FieldValue) value; + isIdempotent &= fieldValue.isIdempotent(); if (fieldValue.includeInDocumentTransform()) { transforms.put(path, fieldValue.toProto(path)); } } else if (value instanceof Map) { - transforms.putAll( - extractFromMap((Map) value, path, /* allowTransforms= */ true)); + DocumentTransform subTransform = + extractFromMap((Map) value, path, /* allowTransforms= */ true); + transforms.putAll(subTransform.transforms); + isIdempotent &= subTransform.isIdempotent; } else if (entry.getValue() instanceof List) { // DocumentTransforms cannot be contained in Lists. We still process the List for // validation. @@ -57,12 +63,13 @@ static DocumentTransform fromFieldPathMap(Map values) { } } - return new DocumentTransform(transforms); + return new DocumentTransform(transforms, isIdempotent); } - private static SortedMap extractFromMap( + private static DocumentTransform extractFromMap( Map values, FieldPath path, boolean allowTransforms) { SortedMap transforms = new TreeMap<>(); + boolean isIdempotent = true; for (Map.Entry entry : values.entrySet()) { Object value = entry.getValue(); @@ -70,6 +77,7 @@ private static SortedMap extractFromMap( if (value instanceof FieldValue) { FieldValue fieldValue = (FieldValue) value; if (allowTransforms) { + isIdempotent &= fieldValue.isIdempotent(); if (fieldValue.includeInDocumentTransform()) { transforms.put(path, fieldValue.toProto(path)); } @@ -78,14 +86,17 @@ private static SortedMap extractFromMap( fieldValue.getMethodName() + " is not supported inside of an array."); } } else if (value instanceof Map) { - transforms.putAll(extractFromMap((Map) value, path, allowTransforms)); + DocumentTransform subTransform = + extractFromMap((Map) value, path, allowTransforms); + transforms.putAll(subTransform.transforms); + isIdempotent &= subTransform.isIdempotent; } else if (entry.getValue() instanceof List) { validateArray((List) entry.getValue(), path); } path = path.getParent(); } - return transforms; + return new DocumentTransform(transforms, isIdempotent); } private static void validateArray(List values, FieldPath path) { @@ -108,6 +119,10 @@ boolean isEmpty() { return transforms.isEmpty(); } + boolean isIdempotent() { + return isIdempotent; + } + Set getFields() { return Collections.unmodifiableSet(transforms.keySet()); } diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FieldValue.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FieldValue.java index 3854e2f4ba86..702bf3be4d83 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FieldValue.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FieldValue.java @@ -40,6 +40,11 @@ boolean includeInDocumentTransform() { return true; } + @Override + boolean isIdempotent() { + return true; + } + @Override String getMethodName() { return "FieldValue.serverTimestamp()"; @@ -66,6 +71,11 @@ boolean includeInDocumentTransform() { return false; } + @Override + boolean isIdempotent() { + return true; + } + @Override String getMethodName() { return "FieldValue.delete()"; @@ -95,6 +105,11 @@ boolean includeInDocumentTransform() { return true; } + @Override + boolean isIdempotent() { + return false; + } + @Override String getMethodName() { return "FieldValue.increment()"; @@ -144,6 +159,11 @@ boolean includeInDocumentTransform() { return true; } + @Override + boolean isIdempotent() { + return true; + } + @Override String getMethodName() { return "FieldValue.minimum()"; @@ -193,6 +213,11 @@ boolean includeInDocumentTransform() { return true; } + @Override + boolean isIdempotent() { + return true; + } + @Override String getMethodName() { return "FieldValue.maximum()"; @@ -242,6 +267,11 @@ boolean includeInDocumentTransform() { return true; } + @Override + boolean isIdempotent() { + return true; + } + @Override String getMethodName() { return "FieldValue.arrayUnion()"; @@ -298,6 +328,11 @@ boolean includeInDocumentTransform() { return true; } + @Override + boolean isIdempotent() { + return true; + } + @Override String getMethodName() { return "FieldValue.arrayRemove()"; @@ -491,6 +526,9 @@ public static VectorValue vector(@Nonnull double[] values) { /** Whether this FieldTransform should be included in the list of document transforms. */ abstract boolean includeInDocumentTransform(); + /** Whether this FieldValue operation is idempotent and safe to retry on DEADLINE_EXCEEDED. */ + abstract boolean isIdempotent(); + /** The name of the method that returned this FieldValue instance. */ abstract String getMethodName(); diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UpdateBuilder.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UpdateBuilder.java index cfa852ce4cfc..5be04f19630b 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UpdateBuilder.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UpdateBuilder.java @@ -24,15 +24,20 @@ import com.google.api.core.ApiFuture; import com.google.api.core.ApiFutures; import com.google.api.core.InternalExtensionOnly; +import com.google.api.gax.grpc.GrpcCallContext; +import com.google.api.gax.rpc.StatusCode; +import com.google.api.gax.rpc.UnaryCallable; import com.google.cloud.firestore.UserDataConverter.EncodingOptions; import com.google.cloud.firestore.encoding.CustomClassMapper; import com.google.cloud.firestore.telemetry.TelemetryConstants; import com.google.cloud.firestore.telemetry.TraceUtil; import com.google.cloud.firestore.telemetry.TraceUtil.Scope; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.MoreExecutors; import com.google.firestore.v1.CommitRequest; import com.google.firestore.v1.CommitResponse; +import com.google.firestore.v1.DocumentTransform.FieldTransform; import com.google.firestore.v1.Write; import com.google.protobuf.ByteString; import com.google.protobuf.Timestamp; @@ -58,13 +63,25 @@ */ @InternalExtensionOnly public abstract class UpdateBuilder { + static final Set IDEMPOTENT_COMMIT_RETRYABLE_CODES = + ImmutableSet.of( + StatusCode.Code.RESOURCE_EXHAUSTED, + StatusCode.Code.UNAVAILABLE, + StatusCode.Code.DEADLINE_EXCEEDED); + static final class WriteOperation { final Write write; final DocumentReference documentReference; + final boolean isIdempotent; WriteOperation(DocumentReference documentReference, Write write) { + this(documentReference, write, isIdempotent(write)); + } + + WriteOperation(DocumentReference documentReference, Write write, boolean isIdempotent) { this.documentReference = documentReference; this.write = write; + this.isIdempotent = isIdempotent; } @Override @@ -165,7 +182,7 @@ private T performCreate( write.addAllUpdateTransforms(documentTransform.toPb()); } - return addWrite(documentReference, write); + return addWrite(documentReference, write, documentTransform.isIdempotent()); } /** @@ -289,11 +306,19 @@ private T performSet( write.setUpdateMask(documentMask.toPb()); } - return addWrite(documentReference, write); + return addWrite(documentReference, write, documentTransform.isIdempotent()); } private T addWrite(DocumentReference documentReference, Write.Builder write) { - WriteOperation operation = new WriteOperation(documentReference, write.build()); + return addWrite(documentReference, write, /* isTransformIdempotent= */ true); + } + + private T addWrite( + DocumentReference documentReference, Write.Builder write, boolean isTransformIdempotent) { + Write builtWrite = write.build(); + WriteOperation operation = + new WriteOperation( + documentReference, builtWrite, isTransformIdempotent && isIdempotent(builtWrite)); int writeIndex; synchronized (writes) { Preconditions.checkState( @@ -571,7 +596,7 @@ public boolean allowTransform() { write.addAllUpdateTransforms(documentTransform.toPb()); } - return addWrite(documentReference, write); + return addWrite(documentReference, write, documentTransform.isIdempotent()); } /** @@ -640,8 +665,19 @@ ApiFuture> commit(@Nullable ByteString transactionId) { committed = true; CommitRequest request = buildCommitRequest(transactionId); - ApiFuture response = - firestore.sendRequest(request, firestore.getClient().commitCallable()); + UnaryCallable callable = + firestore.getClient().commitCallable(); + if (callable != null + && transactionId == null + && isIdempotentBatch() + && isIdempotentCommit(request)) { + callable = + callable.withDefaultCallContext( + GrpcCallContext.createDefault() + .withRetryableCodes(IDEMPOTENT_COMMIT_RETRYABLE_CODES)); + } + + ApiFuture response = firestore.sendRequest(request, callable); ApiFuture> returnValue = ApiFutures.transform( @@ -671,6 +707,78 @@ private CommitRequest buildCommitRequest(ByteString transactionId) { return builder.build(); } + boolean isIdempotentBatch() { + synchronized (writes) { + for (WriteOperation writeOperation : writes) { + if (!writeOperation.isIdempotent) { + return false; + } + } + return true; + } + } + + static boolean isIdempotentCommit(CommitRequest request) { + if (!request.getTransaction().isEmpty()) { + return false; + } + for (Write write : request.getWritesList()) { + if (!isIdempotent(write)) { + return false; + } + } + return true; + } + + static boolean isIdempotent(Write write) { + if (write.hasCurrentDocument()) { + com.google.firestore.v1.Precondition currentDocument = write.getCurrentDocument(); + switch (currentDocument.getConditionTypeCase()) { + case UPDATE_TIME: + return false; + case EXISTS: + if (!currentDocument.getExists()) { + return false; + } + if (write.getOperationCase() == Write.OperationCase.DELETE) { + return false; + } + break; + case CONDITIONTYPE_NOT_SET: + default: + break; + } + } + for (FieldTransform transform : write.getUpdateTransformsList()) { + if (!isIdempotentTransform(transform)) { + return false; + } + } + if (write.hasTransform()) { + for (FieldTransform transform : write.getTransform().getFieldTransformsList()) { + if (!isIdempotentTransform(transform)) { + return false; + } + } + } + return true; + } + + private static boolean isIdempotentTransform(FieldTransform transform) { + switch (transform.getTransformTypeCase()) { + case SET_TO_SERVER_VALUE: + case APPEND_MISSING_ELEMENTS: + case REMOVE_ALL_FROM_ARRAY: + case MAXIMUM: + case MINIMUM: + return true; + case INCREMENT: + case TRANSFORMTYPE_NOT_SET: + default: + return false; + } + } + /** Checks whether any updates have been queued. */ boolean isEmpty() { synchronized (writes) { diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/WriteBatchTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/WriteBatchTest.java index 1fec528406ee..badfd609ab6f 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/WriteBatchTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/WriteBatchTest.java @@ -26,19 +26,33 @@ import static com.google.cloud.firestore.LocalFirestoreHelper.set; import static com.google.cloud.firestore.LocalFirestoreHelper.update; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.Mockito.doReturn; +import com.google.api.gax.retrying.RetrySettings; +import com.google.api.gax.rpc.DeadlineExceededException; import com.google.api.gax.rpc.UnaryCallable; import com.google.cloud.Timestamp; import com.google.cloud.firestore.spi.v1.FirestoreRpc; import com.google.firestore.v1.BatchWriteRequest; import com.google.firestore.v1.CommitRequest; import com.google.firestore.v1.CommitResponse; +import com.google.firestore.v1.FirestoreGrpc; import com.google.firestore.v1.Write; +import com.google.protobuf.ByteString; +import io.grpc.Server; +import io.grpc.ServerBuilder; +import io.grpc.Status; +import io.grpc.stub.StreamObserver; +import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -380,4 +394,358 @@ public void throwsWhenModifyingACommittedWriteBatch() throws Exception { }, expectedErrorMessage); } + + private CommitRequest captureCommitRequest(Runnable commitAction) { + doReturn(commitResponse(10, 0)) + .when(firestoreMock) + .sendRequest( + commitCapture.capture(), + ArgumentMatchers.>any()); + commitAction.run(); + return commitCapture.getValue(); + } + + @Test + public void isIdempotentClassification() { + // 0. Compile-time enforced FieldValue.isIdempotent() and DocumentTransform.isIdempotent() + assertTrue(FieldValue.delete().isIdempotent()); + assertTrue(FieldValue.serverTimestamp().isIdempotent()); + assertTrue(FieldValue.arrayUnion("a").isIdempotent()); + assertTrue(FieldValue.arrayRemove("a").isIdempotent()); + assertTrue(FieldValue.minimum(1).isIdempotent()); + assertTrue(FieldValue.minimum(1.0).isIdempotent()); + assertTrue(FieldValue.maximum(1).isIdempotent()); + assertTrue(FieldValue.maximum(1.0).isIdempotent()); + assertFalse(FieldValue.increment(1).isIdempotent()); + assertFalse(FieldValue.increment(1.0).isIdempotent()); + + assertTrue( + DocumentTransform.fromFieldPathMap( + map( + FieldPath.of("del"), + FieldValue.delete(), + FieldPath.of("ts"), + FieldValue.serverTimestamp(), + FieldPath.of("nested"), + map( + "union", + FieldValue.arrayUnion("a"), + "remove", + FieldValue.arrayRemove("b"), + "min", + FieldValue.minimum(1), + "max", + FieldValue.maximum(10)))) + .isIdempotent()); + assertFalse( + DocumentTransform.fromFieldPathMap(map(FieldPath.of("inc"), FieldValue.increment(1))) + .isIdempotent()); + assertFalse( + DocumentTransform.fromFieldPathMap( + map(FieldPath.of("nested"), map("inc", FieldValue.increment(1)))) + .isIdempotent()); + + // Fail-closed FieldTransform check when TransformTypeCase is unset/unknown + assertFalse( + UpdateBuilder.isIdempotent( + Write.newBuilder() + .addUpdateTransforms( + com.google.firestore.v1.DocumentTransform.FieldTransform.getDefaultInstance()) + .build())); + + // 1. Idempotent single-document and batch writes -> true + assertTrue( + UpdateBuilder.isIdempotentCommit( + captureCommitRequest( + () -> documentReference.set(LocalFirestoreHelper.SINGLE_FIELD_MAP)))); + + assertTrue( + UpdateBuilder.isIdempotentCommit( + captureCommitRequest( + () -> + documentReference.set( + LocalFirestoreHelper.SINGLE_FIELD_MAP, SetOptions.merge())))); + + assertTrue( + UpdateBuilder.isIdempotentCommit( + captureCommitRequest( + () -> + documentReference.update( + map( + "plain", + "value", + "deleted", + FieldValue.delete(), + "serverTime", + FieldValue.serverTimestamp(), + "union", + FieldValue.arrayUnion("a", "b"), + "remove", + FieldValue.arrayRemove("c"), + "min", + FieldValue.minimum(10), + "max", + FieldValue.maximum(20)))))); + + assertTrue( + UpdateBuilder.isIdempotentCommit(captureCommitRequest(() -> documentReference.delete()))); + + assertTrue( + UpdateBuilder.isIdempotentCommit( + captureCommitRequest( + () -> + firestoreMock + .batch() + .set(documentReference, LocalFirestoreHelper.SINGLE_FIELD_MAP) + .set( + firestoreMock.document("coll/doc2"), + LocalFirestoreHelper.SINGLE_FIELD_MAP, + SetOptions.merge()) + .update( + firestoreMock.document("coll/doc3"), + map("foo", "bar", "arr", FieldValue.arrayUnion("x"))) + .delete(firestoreMock.document("coll/doc4")) + .commit()))); + + // 2. Non-idempotent writes -> false + assertFalse( + UpdateBuilder.isIdempotentCommit( + captureCommitRequest( + () -> documentReference.create(LocalFirestoreHelper.SINGLE_FIELD_MAP)))); + + assertFalse( + UpdateBuilder.isIdempotentCommit( + captureCommitRequest( + () -> documentReference.update(map("count", FieldValue.increment(1)))))); + + assertFalse( + UpdateBuilder.isIdempotentCommit( + captureCommitRequest( + () -> + documentReference.update( + map("nested", map("count", FieldValue.increment(1))))))); + + assertFalse( + UpdateBuilder.isIdempotentCommit( + captureCommitRequest( + () -> documentReference.set(map("count", FieldValue.increment(1)))))); + + Precondition updatedAt = Precondition.updatedAt(Timestamp.ofTimeSecondsAndNanos(1, 2)); + assertFalse( + UpdateBuilder.isIdempotentCommit( + captureCommitRequest( + () -> documentReference.update(LocalFirestoreHelper.SINGLE_FIELD_MAP, updatedAt)))); + + assertFalse( + UpdateBuilder.isIdempotentCommit( + captureCommitRequest(() -> documentReference.delete(Precondition.exists(true))))); + + assertFalse( + UpdateBuilder.isIdempotentCommit( + captureCommitRequest(() -> documentReference.delete(updatedAt)))); + + assertFalse( + UpdateBuilder.isIdempotentCommit( + captureCommitRequest( + () -> + firestoreMock + .batch() + .set(documentReference, LocalFirestoreHelper.SINGLE_FIELD_MAP) + .create( + firestoreMock.document("coll/doc2"), + LocalFirestoreHelper.SINGLE_FIELD_MAP) + .commit()))); + + assertFalse( + UpdateBuilder.isIdempotentCommit( + captureCommitRequest( + () -> + firestoreMock + .batch() + .set(documentReference, LocalFirestoreHelper.SINGLE_FIELD_MAP) + .update( + firestoreMock.document("coll/doc2"), + map("count", FieldValue.increment(1))) + .commit()))); + + CommitRequest transactionalCommit = + captureCommitRequest(() -> documentReference.set(LocalFirestoreHelper.SINGLE_FIELD_MAP)) + .toBuilder() + .setTransaction(ByteString.copyFromUtf8("txn-1")) + .build(); + assertFalse(UpdateBuilder.isIdempotentCommit(transactionalCommit)); + + // 3. UpdateBuilder.isIdempotentBatch() tracks WriteOperation idempotency via DocumentTransform + WriteBatch idempotentBatch = + firestoreMock + .batch() + .set(documentReference, LocalFirestoreHelper.SINGLE_FIELD_MAP) + .update( + firestoreMock.document("coll/doc2"), + map( + "ts", + FieldValue.serverTimestamp(), + "union", + FieldValue.arrayUnion("a"), + "remove", + FieldValue.arrayRemove("b"), + "min", + FieldValue.minimum(1), + "max", + FieldValue.maximum(2))); + assertTrue(idempotentBatch.isIdempotentBatch()); + + WriteBatch nonIdempotentBatch = + firestoreMock + .batch() + .set(documentReference, LocalFirestoreHelper.SINGLE_FIELD_MAP) + .update(firestoreMock.document("coll/doc2"), map("inc", FieldValue.increment(1))); + assertFalse(nonIdempotentBatch.isIdempotentBatch()); + } + + @Test + public void retriesIdempotentAndDoesNotRetryNonIdempotentCommitsOnDeadlineExceeded() + throws Exception { + AtomicInteger commitAttempts = new AtomicInteger(); + FirestoreGrpc.FirestoreImplBase service = + new FirestoreGrpc.FirestoreImplBase() { + @Override + public void commit( + CommitRequest request, StreamObserver responseObserver) { + int attempt = commitAttempts.incrementAndGet(); + if (attempt == 1) { + responseObserver.onError( + Status.DEADLINE_EXCEEDED + .withDescription("Simulated channel blackhole timeout") + .asRuntimeException()); + return; + } + CommitResponse.Builder responseBuilder = + CommitResponse.newBuilder() + .setCommitTime( + com.google.protobuf.Timestamp.newBuilder().setSeconds(100).build()); + for (int i = 0; i < request.getWritesCount(); i++) { + responseBuilder.addWriteResults( + com.google.firestore.v1.WriteResult.newBuilder() + .setUpdateTime( + com.google.protobuf.Timestamp.newBuilder().setSeconds(100 + i).build()) + .build()); + } + responseObserver.onNext(responseBuilder.build()); + responseObserver.onCompleted(); + } + }; + + Server server = ServerBuilder.forPort(0).addService(service).build().start(); + Firestore firestore = + FirestoreOptions.newBuilder() + .setProjectId("test-project") + .setEmulatorHost("localhost:" + server.getPort()) + .setRetrySettings( + RetrySettings.newBuilder() + .setInitialRetryDelayDuration(Duration.ofMillis(1)) + .setRetryDelayMultiplier(1.0) + .setMaxRetryDelayDuration(Duration.ofMillis(5)) + .setInitialRpcTimeoutDuration(Duration.ofSeconds(5)) + .setMaxRpcTimeoutDuration(Duration.ofSeconds(5)) + .setTotalTimeoutDuration(Duration.ofSeconds(10)) + .setMaxAttempts(5) + .build()) + .build() + .getService(); + + try { + DocumentReference docRef = firestore.document("coll/doc"); + + // 1. Idempotent writes retry on DEADLINE_EXCEEDED and succeed on Attempt #2 + commitAttempts.set(0); + docRef.set(LocalFirestoreHelper.SINGLE_FIELD_MAP).get(); + assertEquals(2, commitAttempts.get()); + + commitAttempts.set(0); + docRef.set(LocalFirestoreHelper.SINGLE_FIELD_MAP, SetOptions.merge()).get(); + assertEquals(2, commitAttempts.get()); + + commitAttempts.set(0); + docRef + .update( + map( + "foo", + "bar", + "arr", + FieldValue.arrayUnion("a"), + "min", + FieldValue.minimum(1), + "max", + FieldValue.maximum(10))) + .get(); + assertEquals(2, commitAttempts.get()); + + commitAttempts.set(0); + docRef.delete().get(); + assertEquals(2, commitAttempts.get()); + + commitAttempts.set(0); + firestore + .batch() + .set(docRef, LocalFirestoreHelper.SINGLE_FIELD_MAP) + .update(firestore.document("coll/doc2"), map("foo", "bar")) + .delete(firestore.document("coll/doc3")) + .commit() + .get(); + assertEquals(2, commitAttempts.get()); + + // 2. Non-idempotent writes do NOT retry on DEADLINE_EXCEEDED and fail after Attempt #1 + assertFailsOnFirstDeadlineExceeded( + commitAttempts, () -> docRef.create(LocalFirestoreHelper.SINGLE_FIELD_MAP).get()); + + assertFailsOnFirstDeadlineExceeded( + commitAttempts, () -> docRef.update(map("count", FieldValue.increment(1))).get()); + + assertFailsOnFirstDeadlineExceeded( + commitAttempts, + () -> + docRef + .update( + LocalFirestoreHelper.SINGLE_FIELD_MAP, + Precondition.updatedAt(Timestamp.ofTimeSecondsAndNanos(1, 0))) + .get()); + + assertFailsOnFirstDeadlineExceeded( + commitAttempts, () -> docRef.delete(Precondition.exists(true)).get()); + + assertFailsOnFirstDeadlineExceeded( + commitAttempts, + () -> + firestore + .batch() + .set(docRef, LocalFirestoreHelper.SINGLE_FIELD_MAP) + .update(firestore.document("coll/doc2"), map("count", FieldValue.increment(1))) + .commit() + .get()); + } finally { + firestore.close(); + server.shutdownNow(); + } + } + + private interface ThrowingRunnable { + void run() throws Exception; + } + + private static void assertFailsOnFirstDeadlineExceeded( + AtomicInteger commitAttempts, ThrowingRunnable action) throws Exception { + commitAttempts.set(0); + try { + action.run(); + fail("Expected ExecutionException caused by DeadlineExceededException"); + } catch (ExecutionException expected) { + assertTrue( + "Expected DeadlineExceededException cause, got: " + expected.getCause(), + expected.getCause() instanceof DeadlineExceededException + || (expected.getCause() != null + && expected.getCause().getCause() instanceof DeadlineExceededException)); + assertEquals(1, commitAttempts.get()); + } + } }