Merge 0159f99 into c1a33a0

Author: morganchen12

Firestore/CHANGELOG.md

@@ -1,6 +1,11 @@
 # Unreleased
 - [changed] Add more details to the assertion failure in Query::Comparator() to
   help with future debugging (#9258).
+- [fixed] **Breaking change:** Fixed an issue where returning `nil` from the
+  update closure when running a transaction caused a crash in Swift by removing
+  the auto-generated `async throw`ing method from the `FirebaseFirestore`
+  module. In order to use the `async throw`ing transaction method, add the
+  `FirebaseFirestoreSwift` module dependency to your build target (#9426).
 
 # v8.14.0
 - [fixed] Fixed compiler warnings in `local_serializer.cc` about "implicit

Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h

@@ -119,7 +119,7 @@ NS_SWIFT_NAME(Firestore)
  * `FieldValue.increment()` inside a transaction counts as an additional write.
  *
  * In the updateBlock, a set of reads and writes can be performed atomically using the
- * `FIRTransaction` object passed to the block. After the updateBlock is run, Firestore will attempt
+ * `Transaction` object passed to the block. After the updateBlock is run, Firestore will attempt
  * to apply the changes to the server. If any of the data read has been modified outside of this
  * transaction since being read, then the transaction will be retried by executing the updateBlock
  * again. If the transaction still fails after 5 retries, then the transaction will fail.
@@ -132,7 +132,7 @@ NS_SWIFT_NAME(Firestore)
  * parameter. If this is set, then the transaction will not attempt to commit, and the given error
  * will be passed to the completion block.
  *
- * The `FIRTransaction` object passed to the updateBlock contains methods for accessing documents
+ * The `Transaction` object passed to the updateBlock contains methods for accessing documents
  * and collections. Unlike other firestore 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. Otherwise,
@@ -143,7 +143,8 @@ NS_SWIFT_NAME(Firestore)
  *     block will run even if the client is offline, unless the process is killed.
  */
 - (void)runTransactionWithBlock:(id _Nullable (^)(FIRTransaction *, NSError **))updateBlock
-                     completion:(void (^)(id _Nullable result, NSError *_Nullable error))completion;
+                     completion:(void (^)(id _Nullable result, NSError *_Nullable error))completion
+    __attribute__((swift_async(none)));  // Disable async import due to #9426.
 
 /**
  * Creates a write batch, used for performing multiple writes as a single

Firestore/Swift/Source/AsyncAwait/Firestore+AsyncAwait.swift

@@ -53,5 +53,50 @@ import Foundation
         }
       }
     }
+
+    /// Executes the given updateBlock and then attempts to commit the changes applied within an atomic
+    /// transaction.
+    ///
+    /// The maximum number of writes allowed in a single transaction is 500, but note that each usage of
+    /// `FieldValue.serverTimestamp()`, `FieldValue.arrayUnion()`, `FieldValue.arrayRemove()`, or
+    /// `FieldValue.increment()` inside a transaction counts as an additional write.
+    ///
+    /// In the `updateBlock`, a set of reads and writes can be performed atomically using the
+    /// `Transaction` object passed to the block. After the `updateBlock` is run, Firestore will attempt
+    /// to apply the changes to the server. If any of the data read has been modified outside of this
+    /// transaction since being read, then the transaction will be retried by executing the `updateBlock`
+    /// again. If the transaction still fails after 5 retries, then the transaction will fail.
+    ///
+    /// Since the `updateBlock` may be executed multiple times, it should avoiding doing anything that
+    /// would cause side effects.
+    ///
+    /// Any value maybe be returned from the `updateBlock`. If the transaction is successfully committed,
+    /// then the completion block will be passed that value. The `updateBlock` also has an `NSError` out
+    /// parameter. If this is set, then the transaction will not attempt to commit, and the given error
+    /// will be returned.
+    ///
+    /// The `Transaction` object passed to the `updateBlock` contains methods for accessing documents
+    /// and collections. Unlike other firestore 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. Otherwise,
+    /// reads will fail, the final commit will fail, and this function will return an error.
+    ///
+    /// - Parameter updateBlock The block to execute within the transaction context.
+    /// - Throws Throws an error if the transaction could not be committed, or if an error was explicitly specified in the `updateBlock` parameter.
+    /// - Returns Returns the value returned in the `updateBlock` parameter if no errors occurred.
+    func runTransaction(_ updateBlock: @escaping (Transaction, NSErrorPointer)
+      -> Any?) async throws -> Any? {
+      // This needs to be wrapped in order to express a nullable return value upon success.
+      // See https://github.com/firebase/firebase-ios-sdk/issues/9426 for more details.
+      return try await withCheckedThrowingContinuation { continuation in
+        self.runTransaction(updateBlock) { anyValue, error in
+          if let err = error {
+            continuation.resume(throwing: err)
+          } else {
+            continuation.resume(returning: anyValue)
+          }
+        }
+      }
+    }
   }
 #endif

Firestore/Swift/Tests/Integration/AsyncAwaitIntegrationTests.swift

@@ -65,5 +65,13 @@ let emptyBundle = """
         .loadBundle(InputStream(data: bundle.data(using: String.Encoding.utf8)!))
       XCTAssertEqual(LoadBundleTaskState.success, bundleProgress.state)
     }
+
+    func testRunTransactionDoesNotCrashOnNilSuccess() async throws {
+      let value = try await db.runTransaction { transact, error in
+        nil // should not crash
+      }
+
+      XCTAssertNil(value, "value should be nil on success")
+    }
   }
 #endif