From 987c03a5b269444a1fb8e9ed1b806db015f86ff3 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Fri, 16 Aug 2024 12:29:28 -0400 Subject: [PATCH 1/2] Refactor transaction --- .../firestore/core/FirestoreClient.java | 22 ++++++++++--- .../firebase/firestore/core/SyncEngine.java | 33 ------------------- 2 files changed, 17 insertions(+), 38 deletions(-) diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/core/FirestoreClient.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/core/FirestoreClient.java index 6e2d9b87b84..b9a9c72b3ea 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/core/FirestoreClient.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/core/FirestoreClient.java @@ -225,13 +225,25 @@ public Task write(final List mutations) { return source.getTask(); } - /** Tries to execute the transaction in updateFunction. */ + /** + * Takes an updateFunction in which a set of reads and writes can be performed atomically. In the + * updateFunction, the client can read and write values using the supplied transaction object. + * After the updateFunction, all changes will be committed. If a retryable error occurs (ex: some + * other client has changed any of the data referenced), then the updateFunction will be called + * again after a backoff. If the updateFunction still fails after all retries, then the + * transaction will be rejected. + * + *

The transaction object passed to the updateFunction contains methods for accessing documents + * and collections. Unlike other datastore access, data accessed with the transaction will not + * reflect local changes that have not been committed. For this reason, it is required that all + * reads are performed before any writes. Transactions must be performed while online. + * + *

The Task returned is resolved when the transaction is fully committed. + */ public Task transaction( TransactionOptions options, Function> updateFunction) { this.verifyNotTerminated(); - return AsyncQueue.callTask( - asyncQueue.getExecutor(), - () -> syncEngine.transaction(asyncQueue, options, updateFunction)); + return new TransactionRunner<>(asyncQueue, remoteStore, options, updateFunction).run(); } // TODO(b/261013682): Use an explicit executor in continuations. @@ -242,7 +254,7 @@ public Task> runAggregateQuery( final TaskCompletionSource> result = new TaskCompletionSource<>(); asyncQueue.enqueueAndForget( () -> - syncEngine + remoteStore .runAggregateQuery(query, aggregateFields) .addOnSuccessListener(data -> result.setResult(data)) .addOnFailureListener(e -> result.setException(e))); diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/core/SyncEngine.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/core/SyncEngine.java index e9bb7bafea6..9c51418fae7 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/core/SyncEngine.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/core/SyncEngine.java @@ -19,15 +19,12 @@ import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; -import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.TaskCompletionSource; import com.google.firebase.database.collection.ImmutableSortedMap; import com.google.firebase.database.collection.ImmutableSortedSet; -import com.google.firebase.firestore.AggregateField; import com.google.firebase.firestore.FirebaseFirestoreException; import com.google.firebase.firestore.LoadBundleTask; import com.google.firebase.firestore.LoadBundleTaskProgress; -import com.google.firebase.firestore.TransactionOptions; import com.google.firebase.firestore.auth.User; import com.google.firebase.firestore.bundle.BundleElement; import com.google.firebase.firestore.bundle.BundleLoader; @@ -51,11 +48,8 @@ import com.google.firebase.firestore.remote.RemoteEvent; import com.google.firebase.firestore.remote.RemoteStore; import com.google.firebase.firestore.remote.TargetChange; -import com.google.firebase.firestore.util.AsyncQueue; -import com.google.firebase.firestore.util.Function; import com.google.firebase.firestore.util.Logger; import com.google.firebase.firestore.util.Util; -import com.google.firestore.v1.Value; import com.google.protobuf.ByteString; import io.grpc.Status; import java.io.IOException; @@ -337,33 +331,6 @@ private void addUserCallback(int batchId, TaskCompletionSource userTask) { userTasks.put(batchId, userTask); } - /** - * Takes an updateFunction in which a set of reads and writes can be performed atomically. In the - * updateFunction, the client can read and write values using the supplied transaction object. - * After the updateFunction, all changes will be committed. If a retryable error occurs (ex: some - * other client has changed any of the data referenced), then the updateFunction will be called - * again after a backoff. If the updateFunction still fails after all retries, then the - * transaction will be rejected. - * - *

The transaction object passed to the updateFunction contains methods for accessing documents - * and collections. Unlike other datastore access, data accessed with the transaction will not - * reflect local changes that have not been committed. For this reason, it is required that all - * reads are performed before any writes. Transactions must be performed while online. - * - *

The Task returned is resolved when the transaction is fully committed. - */ - public Task transaction( - AsyncQueue asyncQueue, - TransactionOptions options, - Function> updateFunction) { - return new TransactionRunner(asyncQueue, remoteStore, options, updateFunction).run(); - } - - public Task> runAggregateQuery( - Query query, List aggregateFields) { - return remoteStore.runAggregateQuery(query, aggregateFields); - } - /** Called by FirestoreClient to notify us of a new remote event. */ @Override public void handleRemoteEvent(RemoteEvent event) { From 71c4d53e8a43ec38cd3c5ff497f3457099ceaef1 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Fri, 16 Aug 2024 12:50:17 -0400 Subject: [PATCH 2/2] Fix --- .../com/google/firebase/firestore/core/FirestoreClient.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/core/FirestoreClient.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/core/FirestoreClient.java index b9a9c72b3ea..c5aba1989d2 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/core/FirestoreClient.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/core/FirestoreClient.java @@ -243,7 +243,9 @@ public Task write(final List mutations) { public Task transaction( TransactionOptions options, Function> updateFunction) { this.verifyNotTerminated(); - return new TransactionRunner<>(asyncQueue, remoteStore, options, updateFunction).run(); + return AsyncQueue.callTask( + asyncQueue.getExecutor(), + () -> new TransactionRunner<>(asyncQueue, remoteStore, options, updateFunction).run()); } // TODO(b/261013682): Use an explicit executor in continuations.