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

Tomandersen/transaction options #3664

Merged
merged 29 commits into from
Apr 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
246a067
- Changes to protoc version to make it working for m1 macs.
eldhosembabu Feb 24, 2022
f7873ee
- Changes to protoc version to make it working for m1 macs.
eldhosembabu Feb 24, 2022
5a31b19
- Changes to dependency versions to make it working for m1 macs.
eldhosembabu Feb 26, 2022
1208d9d
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
eldhosembabu Feb 26, 2022
97518d5
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
eldhosembabu Feb 28, 2022
b8a85ef
- Reverting the robolectric version change
eldhosembabu Feb 28, 2022
80da16e
Revert "- Changes to dependency versions to make it working for m1 ma…
eldhosembabu Feb 28, 2022
cca600e
- Updating robolectric versions to 4.7+ to get support for m1 macs.
eldhosembabu Feb 28, 2022
090773a
Reverting some changes to check
eldhosembabu Mar 3, 2022
73dd784
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
eldhosembabu Mar 7, 2022
09e9664
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
eldhosembabu Mar 8, 2022
61c4819
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
eldhosembabu Mar 10, 2022
fcb9e05
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
eldhosembabu Mar 14, 2022
c3132b1
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
eldhosembabu Mar 16, 2022
3191148
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
eldhosembabu Mar 18, 2022
c64e321
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
eldhosembabu Mar 21, 2022
1fc153b
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
eldhosembabu Mar 22, 2022
e70532f
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
eldhosembabu Mar 29, 2022
b155fe7
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
eldhosembabu Apr 12, 2022
5119f6b
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
tom-andersen Apr 13, 2022
5c5dcdb
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
tom-andersen Apr 14, 2022
af391ae
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
tom-andersen Apr 19, 2022
a93bfce
Add TransactionOptions to allow control over number of attempts to co…
tom-andersen Apr 19, 2022
167632b
Add TransactionOptions to allow control over number of attempts to co…
tom-andersen Apr 19, 2022
c6d0361
Merge remote-tracking branch 'origin/tomandersen/transactionOptions' …
tom-andersen Apr 19, 2022
2857711
Add missing @NonNull annotation
tom-andersen Apr 19, 2022
bee89f6
Fix comments and add api.txt change.
tom-andersen Apr 21, 2022
bfe8641
Update CHANGELOG with TransactionOptions
tom-andersen Apr 21, 2022
1d62d46
Improve method comments for TransactionOptions.
tom-andersen Apr 26, 2022
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
2 changes: 2 additions & 0 deletions firebase-firestore/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ by opting into a release at
[go/firebase-android-release](http:go/firebase-android-release) (Googlers only).

# Unreleased
- [changed] Added `TransactionOptions` to control how many times a transaction
will retry commits before failing.
- [fixed] Fixed an issue where patching multiple fields shadows each other (#3528).

# 24.1.1
Expand Down
12 changes: 12 additions & 0 deletions firebase-firestore/api.txt
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ package com.google.firebase.firestore {
method @NonNull public com.google.firebase.firestore.LoadBundleTask loadBundle(@NonNull java.nio.ByteBuffer);
method @NonNull public com.google.android.gms.tasks.Task<java.lang.Void> runBatch(@NonNull com.google.firebase.firestore.WriteBatch.Function);
method @NonNull public <TResult> com.google.android.gms.tasks.Task<TResult> runTransaction(@NonNull com.google.firebase.firestore.Transaction.Function<TResult>);
method @NonNull public <TResult> com.google.android.gms.tasks.Task<TResult> runTransaction(@NonNull com.google.firebase.firestore.TransactionOptions, @NonNull com.google.firebase.firestore.Transaction.Function<TResult>);
method public void setFirestoreSettings(@NonNull com.google.firebase.firestore.FirebaseFirestoreSettings);
method @NonNull public com.google.android.gms.tasks.Task<java.lang.Void> setIndexConfiguration(@NonNull String);
method public static void setLoggingEnabled(boolean);
Expand Down Expand Up @@ -387,6 +388,17 @@ package com.google.firebase.firestore {
method @Nullable public TResult apply(@NonNull com.google.firebase.firestore.Transaction) throws com.google.firebase.firestore.FirebaseFirestoreException;
}

public final class TransactionOptions {
method public int getMaxAttempts();
}

public static final class TransactionOptions.Builder {
ctor public TransactionOptions.Builder();
ctor public TransactionOptions.Builder(@NonNull com.google.firebase.firestore.TransactionOptions);
method @NonNull public com.google.firebase.firestore.TransactionOptions build();
method @NonNull public com.google.firebase.firestore.TransactionOptions.Builder setMaxAttempts(int);
}

public class WriteBatch {
method @NonNull public com.google.android.gms.tasks.Task<java.lang.Void> commit();
method @NonNull public com.google.firebase.firestore.WriteBatch delete(@NonNull com.google.firebase.firestore.DocumentReference);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

Expand All @@ -29,7 +30,6 @@
import com.google.android.gms.tasks.TaskCompletionSource;
import com.google.android.gms.tasks.Tasks;
import com.google.firebase.firestore.FirebaseFirestoreException.Code;
import com.google.firebase.firestore.core.TransactionRunner;
import com.google.firebase.firestore.testutil.IntegrationTestUtil;
import com.google.firebase.firestore.util.AsyncQueue.TimerId;
import java.util.ArrayList;
Expand Down Expand Up @@ -651,7 +651,38 @@ public void testMakesDefaultMaxAttempts() {

Exception e = waitForException(transactionTask);
assertEquals(Code.FAILED_PRECONDITION, ((FirebaseFirestoreException) e).getCode());
assertEquals(TransactionRunner.DEFAULT_MAX_ATTEMPTS_COUNT, count.get());
assertEquals(TransactionOptions.DEFAULT_MAX_ATTEMPTS_COUNT, count.get());
}

@Test
public void testMakesOptionSpecifiedMaxAttempts() {
TransactionOptions options = new TransactionOptions.Builder().setMaxAttempts(1).build();

FirebaseFirestore firestore = testFirestore();
DocumentReference doc1 = firestore.collection("counters").document();
AtomicInteger count = new AtomicInteger(0);
waitFor(doc1.set(map("count", 15)));
Task<Void> transactionTask =
firestore.runTransaction(
options,
transaction -> {
// Get the first doc.
transaction.get(doc1);
// Do a write outside of the transaction to cause the transaction to fail.
waitFor(doc1.set(map("count", 1234 + count.incrementAndGet())));
return null;
});

Exception e = waitForException(transactionTask);
assertEquals(Code.FAILED_PRECONDITION, ((FirebaseFirestoreException) e).getCode());
assertEquals(options.getMaxAttempts(), count.get());
}

@Test
public void testTransactionOptionsZeroMaxAttempts_shouldThrowIllegalArgumentException() {
assertThrows(
IllegalArgumentException.class,
() -> new TransactionOptions.Builder().setMaxAttempts(0).build());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -405,9 +405,10 @@ public Query collectionGroup(@NonNull String collectionId) {
}

/**
* Executes the given updateFunction and then attempts to commit the changes applied within the
* transaction. If any document read within the transaction has changed, the updateFunction will
* be retried. If it fails to commit after 5 attempts, the transaction will fail.
* Executes the given {@code updateFunction} and then attempts to commit the changes applied
* within the transaction. If any document read within the transaction has changed, the
* updateFunction will be retried. If it fails to commit after 5 attempts (the default failure
* limit), the transaction will fail.
*
* <p>The maximum number of writes allowed in a single transaction is 500, but note that each
* usage of {@link FieldValue#serverTimestamp()}, {@link FieldValue#arrayUnion(Object...)}, {@link
Expand All @@ -419,7 +420,7 @@ public Query collectionGroup(@NonNull String collectionId) {
* @return The task returned from the updateFunction.
*/
private <ResultT> Task<ResultT> runTransaction(
Transaction.Function<ResultT> updateFunction, Executor executor) {
TransactionOptions options, Transaction.Function<ResultT> updateFunction, Executor executor) {
ensureClientConfigured();

// We wrap the function they provide in order to
Expand All @@ -434,23 +435,43 @@ private <ResultT> Task<ResultT> runTransaction(
updateFunction.apply(
new Transaction(internalTransaction, FirebaseFirestore.this)));

return client.transaction(wrappedUpdateFunction);
return client.transaction(options, wrappedUpdateFunction);
}

/**
* Executes the given updateFunction and then attempts to commit the changes applied within the
* transaction. If any document read within the transaction has changed, the updateFunction will
* be retried. If it fails to commit after 5 attempts, the transaction will fail.
* Executes the given {@code updateFunction} and then attempts to commit the changes applied
* within the transaction. If any document read within the transaction has changed, the
* updateFunction will be retried. If it fails to commit after 5 attempts (the default failure
* limit), the transaction will fail. To have a different number of retries, use the {@link
* FirebaseFirestore#runTransaction(TransactionOptions, Transaction.Function)} method instead.
*
* @param updateFunction The function to execute within the transaction context.
* @return The task returned from the updateFunction.
*/
@NonNull
public <TResult> Task<TResult> runTransaction(
@NonNull Transaction.Function<TResult> updateFunction) {
return runTransaction(TransactionOptions.DEFAULT, updateFunction);
}

/**
* Executes the given {@code updateFunction} and then attempts to commit the changes applied
* within the transaction. If any document read within the transaction has changed, the
* updateFunction will be retried. If it fails to commit after the maxmimum number of attempts
* specified in transactionOptions, the transaction will fail.
*
* @param options The transaction options for controlling execution.
* @param updateFunction The function to execute within the transaction context.
* @return The task returned from the updateFunction.
*/
@NonNull
public <TResult> Task<TResult> runTransaction(
@NonNull TransactionOptions options, @NonNull Transaction.Function<TResult> updateFunction) {
checkNotNull(updateFunction, "Provided transaction update function must not be null.");
return runTransaction(
updateFunction, com.google.firebase.firestore.core.Transaction.getDefaultExecutor());
options,
updateFunction,
com.google.firebase.firestore.core.Transaction.getDefaultExecutor());
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
// Copyright 2022 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package com.google.firebase.firestore;

import androidx.annotation.NonNull;

/**
* Options to customize transaction behavior for {@link
* FirebaseFirestore#runTransaction(TransactionOptions, Transaction.Function)}.
*/
public final class TransactionOptions {

static final TransactionOptions DEFAULT = new TransactionOptions.Builder().build();
static final int DEFAULT_MAX_ATTEMPTS_COUNT = 5;

private final int maxAttempts;

private TransactionOptions(int maxAttempts) {
this.maxAttempts = maxAttempts;
}

/** A Builder for creating {@code TransactionOptions}. */
public static final class Builder {
private int maxAttempts = DEFAULT_MAX_ATTEMPTS_COUNT;

/** Constructs a new {@code TransactionOptions} Builder object. */
public Builder() {}

/**
* Constructs a new {@code TransactionOptions} Builder based on an existing {@code
* TransactionOptions} object.
*/
public Builder(@NonNull TransactionOptions options) {
maxAttempts = options.maxAttempts;
}

/**
* Set maximum number of attempts to commit, after which transaction fails.
*
tom-andersen marked this conversation as resolved.
Show resolved Hide resolved
* <p>The default value is 5. Setting the value to less than 1 will result in an {@link
* IllegalArgumentException}.
*
* @return This builder
*/
@NonNull
public Builder setMaxAttempts(int maxAttempts) {
if (maxAttempts < 1) throw new IllegalArgumentException("Max attempts must be at least 1");
this.maxAttempts = maxAttempts;
return this;
}

/**
* Build the {@code TransactionOptions} object.
*
* @return The built {@code TransactionOptions} object
*/
@NonNull
public TransactionOptions build() {
return new TransactionOptions(maxAttempts);
}
}

/**
* Get maximum number of attempts to commit, after which transaction fails. Default is 5.
*
* @return The maximum number of attempts
*/
public int getMaxAttempts() {
return maxAttempts;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;

TransactionOptions that = (TransactionOptions) o;

return maxAttempts == that.maxAttempts;
}

@Override
public int hashCode() {
return maxAttempts;
}

@Override
public String toString() {
return "TransactionOptions{" + "maxAttempts=" + maxAttempts + '}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.firebase.firestore.FirebaseFirestoreException.Code;
import com.google.firebase.firestore.FirebaseFirestoreSettings;
import com.google.firebase.firestore.LoadBundleTask;
import com.google.firebase.firestore.TransactionOptions;
import com.google.firebase.firestore.auth.CredentialsProvider;
import com.google.firebase.firestore.auth.User;
import com.google.firebase.firestore.bundle.BundleReader;
Expand Down Expand Up @@ -228,10 +229,12 @@ public Task<Void> write(final List<Mutation> mutations) {
}

/** Tries to execute the transaction in updateFunction. */
public <TResult> Task<TResult> transaction(Function<Transaction, Task<TResult>> updateFunction) {
public <TResult> Task<TResult> transaction(
TransactionOptions options, Function<Transaction, Task<TResult>> updateFunction) {
this.verifyNotTerminated();
return AsyncQueue.callTask(
asyncQueue.getExecutor(), () -> syncEngine.transaction(asyncQueue, updateFunction));
asyncQueue.getExecutor(),
() -> syncEngine.transaction(asyncQueue, options, updateFunction));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
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;
Expand Down Expand Up @@ -307,8 +308,10 @@ private void addUserCallback(int batchId, TaskCompletionSource<Void> userTask) {
* <p>The Task returned is resolved when the transaction is fully committed.
*/
public <TResult> Task<TResult> transaction(
AsyncQueue asyncQueue, Function<Transaction, Task<TResult>> updateFunction) {
return new TransactionRunner<TResult>(asyncQueue, remoteStore, updateFunction).run();
AsyncQueue asyncQueue,
TransactionOptions options,
Function<Transaction, Task<TResult>> updateFunction) {
return new TransactionRunner<TResult>(asyncQueue, remoteStore, options, updateFunction).run();
}

/** Called by FirestoreClient to notify us of a new remote event. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.google.android.gms.tasks.Task;
import com.google.android.gms.tasks.TaskCompletionSource;
import com.google.firebase.firestore.FirebaseFirestoreException;
import com.google.firebase.firestore.TransactionOptions;
import com.google.firebase.firestore.remote.Datastore;
import com.google.firebase.firestore.remote.RemoteStore;
import com.google.firebase.firestore.util.AsyncQueue;
Expand All @@ -27,7 +28,6 @@

/** TransactionRunner encapsulates the logic needed to run and retry transactions with backoff. */
public class TransactionRunner<TResult> {
public static final int DEFAULT_MAX_ATTEMPTS_COUNT = 5;
private AsyncQueue asyncQueue;
private RemoteStore remoteStore;
private Function<Transaction, Task<TResult>> updateFunction;
Expand All @@ -39,12 +39,13 @@ public class TransactionRunner<TResult> {
public TransactionRunner(
AsyncQueue asyncQueue,
RemoteStore remoteStore,
TransactionOptions options,
Function<Transaction, Task<TResult>> updateFunction) {

this.asyncQueue = asyncQueue;
this.remoteStore = remoteStore;
this.updateFunction = updateFunction;
this.attemptsRemaining = DEFAULT_MAX_ATTEMPTS_COUNT;
this.attemptsRemaining = options.getMaxAttempts();

backoff = new ExponentialBackoff(asyncQueue, TimerId.RETRY_TRANSACTION);
}
Expand Down