C++ migration: port FSTTransaction

Firestore/Example/Tests/Integration/FSTDatastoreTests.mm

@@ -44,6 +44,7 @@
 #include "Firestore/core/src/firebase/firestore/util/async_queue.h"
 #include "Firestore/core/src/firebase/firestore/util/executor_libdispatch.h"
 #include "Firestore/core/src/firebase/firestore/util/hard_assert.h"
+#include "Firestore/core/src/firebase/firestore/util/status.h"
 #include "Firestore/core/src/firebase/firestore/util/string_apple.h"
 #include "absl/memory/memory.h"
 
@@ -63,6 +64,7 @@
 using firebase::firestore::remote::RemoteStore;
 using firebase::firestore::util::AsyncQueue;
 using firebase::firestore::util::ExecutorLibdispatch;
+using firebase::firestore::util::Status;
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -202,8 +204,8 @@ - (void)tearDown {
 - (void)testCommit {
   XCTestExpectation *expectation = [self expectationWithDescription:@"commitWithCompletion"];
 
-  _datastore->CommitMutations({}, ^(NSError *_Nullable error) {
-    XCTAssertNil(error, @"Failed to commit");
+  _datastore->CommitMutations({}, [self, expectation](const Status &status) {
+    XCTAssertTrue(status.ok(), @"Failed to commit");
     [expectation fulfill];
   });
 

Firestore/Example/Tests/Integration/FSTTransactionTests.mm

@@ -404,7 +404,7 @@ - (void)testReadingADocTwiceWithDifferentVersions {
         // The get itself will fail, because we already read an earlier version of this document.
         // TODO(klimt): Perhaps we shouldn't fail reads for this, but should wait and fail the
         // whole transaction? It's an edge-case anyway, as developers shouldn't be reading the same
-        // do multiple times. But they need to handle read errors anyway.
+        // doc multiple times. But they need to handle read errors anyway.
         XCTAssertNotNil(*error);
         return nil;
       }

Firestore/Source/API/FIRFirestore.mm

@@ -40,6 +40,7 @@
 #include "Firestore/core/src/firebase/firestore/auth/credentials_provider.h"
 #include "Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h"
 #include "Firestore/core/src/firebase/firestore/core/database_info.h"
+#include "Firestore/core/src/firebase/firestore/core/transaction.h"
 #include "Firestore/core/src/firebase/firestore/model/database_id.h"
 #include "Firestore/core/src/firebase/firestore/model/resource_path.h"
 #include "Firestore/core/src/firebase/firestore/util/async_queue.h"
@@ -53,6 +54,7 @@
 using firebase::firestore::auth::CredentialsProvider;
 using firebase::firestore::auth::FirebaseCredentialsProvider;
 using firebase::firestore::core::DatabaseInfo;
+using firebase::firestore::core::Transaction;
 using firebase::firestore::model::DatabaseId;
 using firebase::firestore::model::ResourcePath;
 using util::AsyncQueue;
@@ -275,18 +277,19 @@ - (void)runTransactionWithBlock:(id _Nullable (^)(FIRTransaction *, NSError **))
                      completion:
                          (void (^)(id _Nullable result, NSError *_Nullable error))completion {
   // We wrap the function they provide in order to use internal implementation classes for
-  // FSTTransaction, and to run the user callback block on the proper queue.
+  // transaction, and to run the user callback block on the proper queue.
   if (!updateBlock) {
     FSTThrowInvalidArgument(@"Transaction block cannot be nil.");
   } else if (!completion) {
     FSTThrowInvalidArgument(@"Transaction completion block cannot be nil.");
   }
 
   FSTTransactionBlock wrappedUpdate =
-      ^(FSTTransaction *internalTransaction,
+      ^(std::shared_ptr<Transaction> internalTransaction,
         void (^internalCompletion)(id _Nullable, NSError *_Nullable)) {
         FIRTransaction *transaction =
-            [FIRTransaction transactionWithFSTTransaction:internalTransaction firestore:self];
+            [FIRTransaction transactionWithInternalTransaction:std::move(internalTransaction)
+                                                     firestore:self];
         dispatch_async(queue, ^{
           NSError *_Nullable error = nil;
           id _Nullable result = updateBlock(transaction, &error);

Firestore/Source/API/FIRTransaction+Internal.h

@@ -16,12 +16,16 @@
 
 #import "FIRTransaction.h"
 
+#include <memory>
+
+#include "Firestore/core/src/firebase/firestore/core/transaction.h"
+
 @class FIRFirestore;
-@class FSTTransaction;
 
 @interface FIRTransaction (Internal)
 
-+ (instancetype)transactionWithFSTTransaction:(FSTTransaction *)transaction
-                                    firestore:(FIRFirestore *)firestore;
++ (instancetype)transactionWithInternalTransaction:
+                    (std::shared_ptr<firebase::firestore::core::Transaction>)transaction
+                                         firestore:(FIRFirestore *)firestore;
 
 @end

Firestore/Source/API/FIRTransaction.mm

@@ -16,51 +16,59 @@
 
 #import "FIRTransaction.h"
 
+#include <memory>
 #include <utility>
+#include <vector>
 
 #import "Firestore/Source/API/FIRDocumentReference+Internal.h"
 #import "Firestore/Source/API/FIRDocumentSnapshot+Internal.h"
 #import "Firestore/Source/API/FIRFirestore+Internal.h"
 #import "Firestore/Source/API/FIRTransaction+Internal.h"
 #import "Firestore/Source/API/FSTUserDataConverter.h"
-#import "Firestore/Source/Core/FSTTransaction.h"
 #import "Firestore/Source/Model/FSTDocument.h"
 #import "Firestore/Source/Util/FSTUsageValidation.h"
 
+#include "Firestore/core/src/firebase/firestore/core/transaction.h"
+#include "Firestore/core/src/firebase/firestore/util/error_apple.h"
 #include "Firestore/core/src/firebase/firestore/util/hard_assert.h"
+#include "Firestore/core/src/firebase/firestore/util/status.h"
 
 using firebase::firestore::core::ParsedSetData;
 using firebase::firestore::core::ParsedUpdateData;
+using firebase::firestore::core::Transaction;
+using firebase::firestore::util::MakeNSError;
+using firebase::firestore::util::Status;
 
 NS_ASSUME_NONNULL_BEGIN
 
 #pragma mark - FIRTransaction
 
 @interface FIRTransaction ()
 
-- (instancetype)initWithTransaction:(FSTTransaction *)transaction
+- (instancetype)initWithTransaction:(std::shared_ptr<Transaction>)transaction
                           firestore:(FIRFirestore *)firestore NS_DESIGNATED_INITIALIZER;
 
-@property(nonatomic, strong, readonly) FSTTransaction *internalTransaction;
 @property(nonatomic, strong, readonly) FIRFirestore *firestore;
 @end
 
 @implementation FIRTransaction (Internal)
 
-+ (instancetype)transactionWithFSTTransaction:(FSTTransaction *)transaction
-                                    firestore:(FIRFirestore *)firestore {
-  return [[FIRTransaction alloc] initWithTransaction:transaction firestore:firestore];
++ (instancetype)transactionWithInternalTransaction:(std::shared_ptr<Transaction>)transaction
+                                         firestore:(FIRFirestore *)firestore {
+  return [[FIRTransaction alloc] initWithTransaction:std::move(transaction) firestore:firestore];
 }
 
 @end
 
-@implementation FIRTransaction
+@implementation FIRTransaction {
+  std::shared_ptr<Transaction> _internalTransaction;
+}
 
-- (instancetype)initWithTransaction:(FSTTransaction *)transaction
+- (instancetype)initWithTransaction:(std::shared_ptr<Transaction>)transaction
                           firestore:(FIRFirestore *)firestore {
   self = [super init];
   if (self) {
-    _internalTransaction = transaction;
+    _internalTransaction = std::move(transaction);
     _firestore = firestore;
   }
   return self;
@@ -77,7 +85,7 @@ - (FIRTransaction *)setData:(NSDictionary<NSString *, id> *)data
   [self validateReference:document];
   ParsedSetData parsed = merge ? [self.firestore.dataConverter parsedMergeData:data fieldMask:nil]
                                : [self.firestore.dataConverter parsedSetData:data];
-  [self.internalTransaction setData:std::move(parsed) forDocument:document.key];
+  _internalTransaction->Set(document.key, std::move(parsed));
   return self;
 }
 
@@ -86,60 +94,58 @@ - (FIRTransaction *)setData:(NSDictionary<NSString *, id> *)data
                 mergeFields:(NSArray<id> *)mergeFields {
   [self validateReference:document];
   ParsedSetData parsed = [self.firestore.dataConverter parsedMergeData:data fieldMask:mergeFields];
-  [self.internalTransaction setData:std::move(parsed) forDocument:document.key];
+  _internalTransaction->Set(document.key, std::move(parsed));
   return self;
 }
 
 - (FIRTransaction *)updateData:(NSDictionary<id, id> *)fields
                    forDocument:(FIRDocumentReference *)document {
   [self validateReference:document];
   ParsedUpdateData parsed = [self.firestore.dataConverter parsedUpdateData:fields];
-  [self.internalTransaction updateData:std::move(parsed) forDocument:document.key];
+  _internalTransaction->Update(document.key, std::move(parsed));
   return self;
 }
 
 - (FIRTransaction *)deleteDocument:(FIRDocumentReference *)document {
   [self validateReference:document];
-  [self.internalTransaction deleteDocument:document.key];
+  _internalTransaction->Delete(document.key);
   return self;
 }
 
 - (void)getDocument:(FIRDocumentReference *)document
          completion:(void (^)(FIRDocumentSnapshot *_Nullable document,
                               NSError *_Nullable error))completion {
   [self validateReference:document];
-  [self.internalTransaction
-      lookupDocumentsForKeys:{document.key}
-                  completion:^(NSArray<FSTMaybeDocument *> *_Nullable documents,
-                               NSError *_Nullable error) {
-                    if (error) {
-                      completion(nil, error);
-                      return;
-                    }
-                    HARD_ASSERT(documents.count == 1,
-                                "Mismatch in docs returned from document lookup.");
-                    FSTMaybeDocument *internalDoc = documents.firstObject;
-                    if ([internalDoc isKindOfClass:[FSTDeletedDocument class]]) {
-                      FIRDocumentSnapshot *doc =
-                          [FIRDocumentSnapshot snapshotWithFirestore:self.firestore
-                                                         documentKey:document.key
-                                                            document:nil
-                                                           fromCache:NO
-                                                    hasPendingWrites:NO];
-                      completion(doc, nil);
-                    } else if ([internalDoc isKindOfClass:[FSTDocument class]]) {
-                      FIRDocumentSnapshot *doc =
-                          [FIRDocumentSnapshot snapshotWithFirestore:self.firestore
-                                                         documentKey:internalDoc.key
-                                                            document:(FSTDocument *)internalDoc
-                                                           fromCache:NO
-                                                    hasPendingWrites:NO];
-                      completion(doc, nil);
-                    } else {
-                      HARD_FAIL("BatchGetDocumentsRequest returned unexpected document type: %s",
-                                NSStringFromClass([internalDoc class]));
-                    }
-                  }];
+  _internalTransaction->Lookup(
+      {document.key}, [self, document, completion](const std::vector<FSTMaybeDocument *> &documents,
+                                                   const Status &status) {
+        if (!status.ok()) {
+          completion(nil, MakeNSError(status));
+          return;
+        }
+
+        HARD_ASSERT(documents.size() == 1, "Mismatch in docs returned from document lookup.");
+        FSTMaybeDocument *internalDoc = documents.front();
+        if ([internalDoc isKindOfClass:[FSTDeletedDocument class]]) {
+          FIRDocumentSnapshot *doc = [FIRDocumentSnapshot snapshotWithFirestore:self.firestore
+                                                                    documentKey:document.key
+                                                                       document:nil
+                                                                      fromCache:NO
+                                                               hasPendingWrites:NO];
+          completion(doc, nil);
+        } else if ([internalDoc isKindOfClass:[FSTDocument class]]) {
+          FIRDocumentSnapshot *doc =
+              [FIRDocumentSnapshot snapshotWithFirestore:self.firestore
+                                             documentKey:internalDoc.key
+                                                document:(FSTDocument *)internalDoc
+                                               fromCache:NO
+                                        hasPendingWrites:NO];
+          completion(doc, nil);
+        } else {
+          HARD_FAIL("BatchGetDocumentsRequest returned unexpected document type: %s",
+                    NSStringFromClass([internalDoc class]));
+        }
+      });
 }
 
 - (FIRDocumentSnapshot *_Nullable)getDocument:(FIRDocumentReference *)document

Firestore/Source/Core/FSTFirestoreClient.mm

@@ -31,7 +31,6 @@
 #import "Firestore/Source/Core/FSTEventManager.h"
 #import "Firestore/Source/Core/FSTQuery.h"
 #import "Firestore/Source/Core/FSTSyncEngine.h"
-#import "Firestore/Source/Core/FSTTransaction.h"
 #import "Firestore/Source/Core/FSTView.h"
 #import "Firestore/Source/Local/FSTLRUGarbageCollector.h"
 #import "Firestore/Source/Local/FSTLevelDB.h"

Firestore/Source/Core/FSTSyncEngine.mm

@@ -17,13 +17,13 @@
 #import "Firestore/Source/Core/FSTSyncEngine.h"
 
 #include <map>
+#include <memory>
 #include <set>
 #include <unordered_map>
 #include <utility>
 
 #import "FIRFirestoreErrors.h"
 #import "Firestore/Source/Core/FSTQuery.h"
-#import "Firestore/Source/Core/FSTTransaction.h"
 #import "Firestore/Source/Core/FSTView.h"
 #import "Firestore/Source/Core/FSTViewSnapshot.h"
 #import "Firestore/Source/Local/FSTLocalStore.h"
@@ -36,18 +36,22 @@
 
 #include "Firestore/core/src/firebase/firestore/auth/user.h"
 #include "Firestore/core/src/firebase/firestore/core/target_id_generator.h"
+#include "Firestore/core/src/firebase/firestore/core/transaction.h"
 #include "Firestore/core/src/firebase/firestore/local/reference_set.h"
 #include "Firestore/core/src/firebase/firestore/model/document_key.h"
 #include "Firestore/core/src/firebase/firestore/model/document_map.h"
 #include "Firestore/core/src/firebase/firestore/model/snapshot_version.h"
 #include "Firestore/core/src/firebase/firestore/remote/remote_event.h"
+#include "Firestore/core/src/firebase/firestore/util/error_apple.h"
 #include "Firestore/core/src/firebase/firestore/util/hard_assert.h"
 #include "Firestore/core/src/firebase/firestore/util/log.h"
+#include "Firestore/core/src/firebase/firestore/util/status.h"
 #include "absl/types/optional.h"
 
 using firebase::firestore::auth::HashUser;
 using firebase::firestore::auth::User;
 using firebase::firestore::core::TargetIdGenerator;
+using firebase::firestore::core::Transaction;
 using firebase::firestore::local::ReferenceSet;
 using firebase::firestore::model::BatchId;
 using firebase::firestore::model::DocumentKey;
@@ -62,6 +66,8 @@
 using firebase::firestore::remote::RemoteStore;
 using firebase::firestore::remote::TargetChange;
 using firebase::firestore::util::AsyncQueue;
+using firebase::firestore::util::MakeNSError;
+using firebase::firestore::util::Status;
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -287,27 +293,30 @@ - (void)transactionWithRetries:(int)retries
                     completion:(FSTVoidIDErrorBlock)completion {
   workerQueue->VerifyIsCurrentQueue();
   HARD_ASSERT(retries >= 0, "Got negative number of retries for transaction");
-  FSTTransaction *transaction = _remoteStore->CreateTransaction();
+
+  std::shared_ptr<Transaction> transaction = _remoteStore->CreateTransaction();
   updateBlock(transaction, ^(id _Nullable result, NSError *_Nullable error) {
     workerQueue->Enqueue(
         [self, retries, workerQueue, updateBlock, completion, transaction, result, error] {
           if (error) {
             completion(nil, error);
             return;
           }
-          [transaction commitWithCompletion:^(NSError *_Nullable transactionError) {
-            if (!transactionError) {
+          transaction->Commit([self, retries, workerQueue, updateBlock, completion,
+                               result](const Status &status) {
+            if (status.ok()) {
               completion(result, nil);
               return;
             }
+
             // TODO(b/35201829): Only retry on real transaction failures.
             if (retries == 0) {
               NSError *wrappedError =
                   [NSError errorWithDomain:FIRFirestoreErrorDomain
                                       code:FIRFirestoreErrorCodeFailedPrecondition
                                   userInfo:@{
                                     NSLocalizedDescriptionKey : @"Transaction failed all retries.",
-                                    NSUnderlyingErrorKey : transactionError
+                                    NSUnderlyingErrorKey : MakeNSError(status)
                                   }];
               completion(nil, wrappedError);
               return;
@@ -317,7 +326,7 @@ - (void)transactionWithRetries:(int)retries
                                     workerQueue:workerQueue
                                     updateBlock:updateBlock
                                      completion:completion];
-          }];
+          });
         });
   });
 }