diff --git a/docs/readme.md b/docs/readme.md index 4a462d48..493a999c 100644 --- a/docs/readme.md +++ b/docs/readme.md @@ -158,6 +158,9 @@ Release Notes ### 9.2.0 - Changes - Crashlytics: Fix requiring user code to reference Crashlytics when using il2cpp. + - Firestore: Added `TransactionOptions` to control how many times a + transaction will retry commits before failing + ([#318](https://github.com/firebase/firebase-unity-sdk/pull/318)). ### 9.1.0 - Changes diff --git a/firestore/CMakeLists.txt b/firestore/CMakeLists.txt index 170d4c35..bbd41830 100644 --- a/firestore/CMakeLists.txt +++ b/firestore/CMakeLists.txt @@ -78,6 +78,7 @@ set(firebase_firestore_src src/Source.cs src/Timestamp.cs src/Transaction.cs + src/TransactionOptions.cs src/TransactionManager.cs src/UnknownPropertyHandling.cs src/ValueDeserializer.cs diff --git a/firestore/src/FirebaseFirestore.cs b/firestore/src/FirebaseFirestore.cs index 95c23604..9e28611d 100644 --- a/firestore/src/FirebaseFirestore.cs +++ b/firestore/src/FirebaseFirestore.cs @@ -248,10 +248,7 @@ public WriteBatch StartBatch() { /// be invoked on the main thread. /// A task which completes when the transaction has committed. public Task RunTransactionAsync(Func callback) { - Preconditions.CheckNotNull(callback, nameof(callback)); - // Just pass through to the overload where the callback returns a Task. - return RunTransactionAsync(transaction => - Util.MapResult(callback(transaction), null)); + return RunTransactionAsync(new TransactionOptions(), callback); } /// @@ -276,8 +273,65 @@ public Task RunTransactionAsync(Func callback) { /// A task which completes when the transaction has committed. The result of the task /// then contains the result of the callback. public Task RunTransactionAsync(Func> callback) { + return RunTransactionAsync(new TransactionOptions(), callback); + } + + /// + /// Runs a transaction asynchronously, with an asynchronous callback that returns a value. + /// The specified callback is executed for a newly-created transaction. + /// + /// + /// RunTransactionAsync executes the given callback on the main thread and then + /// attempts to commit the changes applied within the transaction. If any document read within + /// the transaction has changed, the will be retried. If it fails to + /// commit after the maximum number of attempts specified in the given TransactionOptions + /// object, the transaction will fail. + /// + /// The maximum number of writes allowed in a single transaction is 500, but note that + /// each usage of , FieldValue.ArrayUnion, + /// FieldValue.ArrayRemove, or FieldValue.Increment inside a transaction counts as + /// an additional write. + /// + /// + /// The result type of the callback. + /// The transaction options for controlling execution. Must not be + /// null. + /// The callback to execute. Must not be null. The callback will + /// be invoked on the main thread. + /// A task which completes when the transaction has committed. The result of the task + /// then contains the result of the callback. + public Task RunTransactionAsync(TransactionOptions options, Func> callback) { + Preconditions.CheckNotNull(options, nameof(options)); + Preconditions.CheckNotNull(callback, nameof(callback)); + return WithFirestoreProxy(proxy => _transactionManager.RunTransactionAsync(options, callback)); + } + + /// + /// Runs a transaction asynchronously, with an asynchronous callback that doesn't return a + /// value. The specified callback is executed for a newly-created transaction. + /// + /// + /// RunTransactionAsync executes the given callback on the main thread and then + /// attempts to commit the changes applied within the transaction. If any document read within + /// the transaction has changed, the will be retried. If it fails to + /// commit after the maximum number of attempts specified in the given TransactionOptions + /// object, the transaction will fail. + /// + /// The maximum number of writes allowed in a single transaction is 500, but note that + /// each usage of , FieldValue.ArrayUnion, + /// FieldValue.ArrayRemove, or FieldValue.Increment inside a transaction counts as + /// an additional write. + /// + /// The transaction options for controlling execution. Must not be + /// null. + /// The callback to execute. Must not be null. The callback will + /// be invoked on the main thread. + /// A task which completes when the transaction has committed. + public Task RunTransactionAsync(TransactionOptions options, Func callback) { + Preconditions.CheckNotNull(options, nameof(options)); Preconditions.CheckNotNull(callback, nameof(callback)); - return WithFirestoreProxy(proxy => _transactionManager.RunTransactionAsync(callback)); + // Just pass through to the overload where the callback returns a Task. + return RunTransactionAsync(options, transaction => Util.MapResult(callback(transaction), null)); } private static SnapshotsInSyncCallbackMap snapshotsInSyncCallbacks = new SnapshotsInSyncCallbackMap(); diff --git a/firestore/src/TransactionManager.cs b/firestore/src/TransactionManager.cs index 6d8a8bd9..c95aabb3 100644 --- a/firestore/src/TransactionManager.cs +++ b/firestore/src/TransactionManager.cs @@ -67,9 +67,11 @@ public void Dispose() { /// /// Runs a transaction. /// + /// The transaction options to use.. /// The callback to run.. /// A task that completes when the transaction has completed. - internal Task RunTransactionAsync(Func> callback) { + internal Task RunTransactionAsync(TransactionOptions options, + Func> callback) { // Store the result of the most recent invocation of the user-supplied callback. bool callbackWrapperInvoked = false; Task lastCallbackTask = null; @@ -118,7 +120,7 @@ internal Task RunTransactionAsync(Func> callback) { } }; - return _transactionManagerProxy.RunTransactionAsync(callbackId, ExecuteCallback) + return _transactionManagerProxy.RunTransactionAsync(callbackId, options.Proxy, ExecuteCallback) .ContinueWith(overallCallback); } diff --git a/firestore/src/TransactionOptions.cs b/firestore/src/TransactionOptions.cs new file mode 100644 index 00000000..cba33d44 --- /dev/null +++ b/firestore/src/TransactionOptions.cs @@ -0,0 +1,82 @@ +// 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 +// +// https://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. + +using System; +using System.Threading; + +namespace Firebase.Firestore { + +/// +/// Options to customize transaction behavior for +/// . +/// +public sealed class TransactionOptions { + + // The lock that must be held during all accesses to _proxy. + private readonly ReaderWriterLock _proxyLock = new ReaderWriterLock(); + + // The underlying C++ TransactionOptions object. + private TransactionOptionsProxy _proxy = new TransactionOptionsProxy(); + + internal TransactionOptionsProxy Proxy { + get { + _proxyLock.AcquireReaderLock(Int32.MaxValue); + try { + return new TransactionOptionsProxy(_proxy); + } finally { + _proxyLock.ReleaseReaderLock(); + } + } + } + + /// + /// Creates the default TransactionOptions. + /// + public TransactionOptions() { + } + + /// + /// The maximum number of attempts to commit, after which the transaction fails. + /// + /// + /// + /// The default value is 5, and must be greater than zero. + /// + public Int32 MaxAttempts { + get { + _proxyLock.AcquireReaderLock(Int32.MaxValue); + try { + return _proxy.max_attempts(); + } finally { + _proxyLock.ReleaseReaderLock(); + } + } + set { + _proxyLock.AcquireWriterLock(Int32.MaxValue); + try { + _proxy.set_max_attempts(value); + } finally { + _proxyLock.ReleaseWriterLock(); + } + } + } + + /// + public override string ToString() { + return nameof(TransactionOptions) + "{" + nameof(MaxAttempts) + "=" + MaxAttempts + "}"; + } + +} + +} diff --git a/firestore/src/swig/firestore.i b/firestore/src/swig/firestore.i index 60e3b729..853a1091 100644 --- a/firestore/src/swig/firestore.i +++ b/firestore/src/swig/firestore.i @@ -411,6 +411,13 @@ SWIG_CREATE_PROXY(firebase::firestore::LoadBundleTaskProgress); %rename("%s") firebase::firestore::LoadBundleTaskProgress::state; %include "firestore/src/include/firebase/firestore/load_bundle_task_progress.h" +// Generate a C# wrapper for TransactionOptions. +SWIG_CREATE_PROXY(firebase::firestore::TransactionOptions); +%rename("%s") firebase::firestore::TransactionOptions::TransactionOptions; +%rename("%s") firebase::firestore::TransactionOptions::max_attempts; +%rename("%s") firebase::firestore::TransactionOptions::set_max_attempts; +%include "firestore/src/include/firebase/firestore/transaction_options.h" + // Generate a C# wrapper for Firestore. Comes last because it refers to multiple // other classes (e.g. `CollectionReference`). SWIG_CREATE_PROXY(firebase::firestore::Firestore); diff --git a/firestore/src/swig/transaction_manager.cc b/firestore/src/swig/transaction_manager.cc index 72c0391a..9d382b10 100644 --- a/firestore/src/swig/transaction_manager.cc +++ b/firestore/src/swig/transaction_manager.cc @@ -177,6 +177,7 @@ class TransactionManagerInternal } Future RunTransaction(int32_t callback_id, + TransactionOptions options, TransactionCallbackFn callback_fn) { std::lock_guard lock(mutex_); if (is_disposed_) { @@ -184,16 +185,17 @@ class TransactionManagerInternal } auto shared_this = shared_from_this(); - return firestore_->RunTransaction([shared_this, callback_id, callback_fn]( - Transaction& transaction, - std::string& error_message) { - if (shared_this->ExecuteCallback(callback_id, callback_fn, transaction)) { - return Error::kErrorOk; - } else { - // Return a non-retryable error code. - return Error::kErrorInvalidArgument; + return firestore_->RunTransaction( + options, + [shared_this, callback_id, callback_fn](Transaction& transaction, std::string& error_message) { + if (shared_this->ExecuteCallback(callback_id, callback_fn, transaction)) { + return Error::kErrorOk; + } else { + // Return a non-retryable error code. + return Error::kErrorInvalidArgument; + } } - }); + ); } private: @@ -271,14 +273,15 @@ void TransactionManager::Dispose() { } Future TransactionManager::RunTransaction( - int32_t callback_id, TransactionCallbackFn callback_fn) { + int32_t callback_id, TransactionOptions options, + TransactionCallbackFn callback_fn) { // Make a local copy of `internal_` since it could be reset asynchronously // by a call to `Dispose()`. std::shared_ptr internal_local = internal_; if (!internal_local) { return {}; } - return internal_local->RunTransaction(callback_id, callback_fn); + return internal_local->RunTransaction(callback_id, options, callback_fn); } void TransactionCallback::OnCompletion(bool callback_successful) { diff --git a/firestore/src/swig/transaction_manager.h b/firestore/src/swig/transaction_manager.h index 706adc49..9e5da963 100644 --- a/firestore/src/swig/transaction_manager.h +++ b/firestore/src/swig/transaction_manager.h @@ -198,6 +198,7 @@ class TransactionManager { // If `Dispose()` has been invoked, or the `Firestore` instance has been // destroyed, then this method will immediately return an invalid `Future`. Future RunTransaction(int32_t callback_id, + TransactionOptions options, TransactionCallbackFn callback); private: diff --git a/firestore/testapp/Assets/Firebase/Sample/Firestore/InvalidArgumentsTest.cs b/firestore/testapp/Assets/Firebase/Sample/Firestore/InvalidArgumentsTest.cs index c85ecacd..26ac166a 100644 --- a/firestore/testapp/Assets/Firebase/Sample/Firestore/InvalidArgumentsTest.cs +++ b/firestore/testapp/Assets/Firebase/Sample/Firestore/InvalidArgumentsTest.cs @@ -171,6 +171,14 @@ public static InvalidArgumentsTestCase[] TestCases { name = "FirebaseFirestore_RunTransactionAsync_WithTypeParameter_NullCallback", action = FirebaseFirestore_RunTransactionAsync_WithTypeParameter_NullCallback }, + new InvalidArgumentsTestCase { + name = "FirebaseFirestore_RunTransactionAsync_WithoutTypeParameter_WithOptions_NullCallback", + action = FirebaseFirestore_RunTransactionAsync_WithoutTypeParameter_WithOptions_NullCallback + }, + new InvalidArgumentsTestCase { + name = "FirebaseFirestore_RunTransactionAsync_WithTypeParameter_WithOptions_NullCallback", + action = FirebaseFirestore_RunTransactionAsync_WithTypeParameter_WithOptions_NullCallback + }, new InvalidArgumentsTestCase { name = "FirebaseFirestoreSettings_Host_Null", action = FirebaseFirestoreSettings_Host_Null }, new InvalidArgumentsTestCase { name = "FirebaseFirestoreSettings_Host_EmptyString", @@ -693,6 +701,36 @@ private static void FirebaseFirestore_RunTransactionAsync_WithTypeParameter_Null () => handler.db.RunTransactionAsync(null)); } + private static void FirebaseFirestore_RunTransactionAsync_WithoutTypeParameter_WithOptions_NullCallback( + UIHandlerAutomated handler) { + var options = new TransactionOptions(); + handler.AssertException(typeof(ArgumentNullException), + () => handler.db.RunTransactionAsync(options, null)); + } + + private static void FirebaseFirestore_RunTransactionAsync_WithTypeParameter_WithOptions_NullCallback( + UIHandlerAutomated handler) { + var options = new TransactionOptions(); + handler.AssertException(typeof(ArgumentNullException), + () => handler.db.RunTransactionAsync(options, null)); + } + + private static void FirebaseFirestore_RunTransactionAsync_WithoutTypeParameter_WithOptions_NullOptions( + UIHandlerAutomated handler) { + DocumentReference doc = handler.TestDocument(); + handler.AssertException(typeof(ArgumentNullException), + () => handler.db.RunTransactionAsync(null, tx => tx.GetSnapshotAsync(doc))); + } + + private static void FirebaseFirestore_RunTransactionAsync_WithTypeParameter_WithOptions_NullOptions( + UIHandlerAutomated handler) { + DocumentReference doc = handler.TestDocument(); + handler.AssertException(typeof(ArgumentNullException), + () => handler.db.RunTransactionAsync(null, tx => tx.GetSnapshotAsync(doc) + .ContinueWith(snapshot => new object())) + ); + } + private static void FirebaseFirestoreSettings_Host_Null(UIHandlerAutomated handler) { FirebaseFirestoreSettings settings = handler.db.Settings; handler.AssertException(typeof(ArgumentNullException), () => settings.Host = null); diff --git a/firestore/testapp/Assets/Firebase/Sample/Firestore/UIHandlerAutomated.cs b/firestore/testapp/Assets/Firebase/Sample/Firestore/UIHandlerAutomated.cs index 9ebd579d..c94a54b6 100644 --- a/firestore/testapp/Assets/Firebase/Sample/Firestore/UIHandlerAutomated.cs +++ b/firestore/testapp/Assets/Firebase/Sample/Firestore/UIHandlerAutomated.cs @@ -105,6 +105,8 @@ protected override void Start() { TestTransactionsInParallel, // Waiting for all retries is slow, so we usually leave this test disabled. // TestTransactionMaxRetryFailure, + TestTransactionOptions, + TestTransactionWithExplicitMaxAttempts, TestSetOptions, TestCanTraverseCollectionsAndDocuments, TestCanTraverseCollectionAndDocumentParents, @@ -1096,6 +1098,43 @@ Task TestTransaction() { }); } + Task TestTransactionOptions() { + return Async(() => { + // Verify the initial values of a newly-created TransactionOptions object. + { + var options = new TransactionOptions(); + AssertEq(options.MaxAttempts, 5); + } + + // Verify that setting TransactionOptions.MaxAttempts works. + { + var options = new TransactionOptions(); + options.MaxAttempts = 1; + AssertEq(options.MaxAttempts, 1); + options.MaxAttempts = 42; + AssertEq(options.MaxAttempts, 42); + options.MaxAttempts = Int32.MaxValue; + AssertEq(options.MaxAttempts, Int32.MaxValue); + } + + // Verify that setting TransactionOptions.MaxAttempts throws on invalid value. + { + var options = new TransactionOptions(); + AssertException(typeof(ArgumentException), () => { options.MaxAttempts = 0; }); + AssertException(typeof(ArgumentException), () => { options.MaxAttempts = -1; }); + AssertException(typeof(ArgumentException), () => { options.MaxAttempts = -42; }); + AssertException(typeof(ArgumentException), () => { options.MaxAttempts = Int32.MinValue; }); + } + + // Verify that TransactionOptions.ToString() returns the right value. + { + var options = new TransactionOptions(); + options.MaxAttempts = 42; + AssertEq(options.ToString(), "TransactionOptions{MaxAttempts=42}"); + } + }); + } + // Tests the overload of RunTransactionAsync() where the update function returns a non-generic // Task object. Task TestTransactionWithNonGenericTask() { @@ -1436,6 +1475,28 @@ Task TestTransactionsInParallel() { }); } + Task TestTransactionWithExplicitMaxAttempts() { + return Async(() => { + var options = new TransactionOptions(); + options.MaxAttempts = 3; + int numAttempts = 0; + DocumentReference doc = TestDocument(); + + Task txnTask = db.RunTransactionAsync(options, transaction => { + numAttempts++; + return transaction.GetSnapshotAsync(doc).ContinueWith(snapshot => { + // Queue a write via the transaction. + transaction.Set(doc, TestData(0)); + // But also write the document (out-of-band) so the transaction is retried. + return doc.SetAsync(TestData(numAttempts)); + }).Unwrap(); + }); + + AssertTaskFaults(txnTask, FirestoreError.FailedPrecondition); + AssertEq(numAttempts, 3); + }); + } + Task TestTransactionMaxRetryFailure() { return Async(() => { int retries = 0; @@ -1708,9 +1769,9 @@ Task TestSnapshotMetadataEqualsAndGetHashCode() { /* Anonymous authentication must be enabled for TestAuthIntegration to pass. - + Also, the following security rules are required for TestAuthIntegrationt to pass: - + rules_version='2' service cloud.firestore { match /databases/{database}/documents {