Skip to content

Commit c422806

Browse files
var-constvar-const
authored
C++ migration: port FSTTransaction (#2362)
1 parent 520ea85 commit c422806

19 files changed

+574
-514
lines changed

‎Firestore/Example/Tests/Integration/FSTDatastoreTests.mm

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
#include "Firestore/core/src/firebase/firestore/util/async_queue.h"
4545
#include "Firestore/core/src/firebase/firestore/util/executor_libdispatch.h"
4646
#include "Firestore/core/src/firebase/firestore/util/hard_assert.h"
47+
#include "Firestore/core/src/firebase/firestore/util/status.h"
4748
#include "Firestore/core/src/firebase/firestore/util/string_apple.h"
4849
#include "absl/memory/memory.h"
4950

@@ -63,6 +64,7 @@
6364
using firebase::firestore::remote::RemoteStore;
6465
using firebase::firestore::util::AsyncQueue;
6566
using firebase::firestore::util::ExecutorLibdispatch;
67+
using firebase::firestore::util::Status;
6668

6769
NS_ASSUME_NONNULL_BEGIN
6870

@@ -202,8 +204,8 @@ - (void)tearDown {
202204
- (void)testCommit {
203205
XCTestExpectation *expectation = [self expectationWithDescription:@"commitWithCompletion"];
204206

205-
_datastore->CommitMutations({}, ^(NSError *_Nullable error) {
206-
XCTAssertNil(error, @"Failed to commit");
207+
_datastore->CommitMutations({}, [self, expectation](const Status &status) {
208+
XCTAssertTrue(status.ok(), @"Failed to commit");
207209
[expectation fulfill];
208210
});
209211

‎Firestore/Example/Tests/Integration/FSTTransactionTests.mm

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

‎Firestore/Source/API/FIRFirestore.mm

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
#include "Firestore/core/src/firebase/firestore/auth/credentials_provider.h"
4141
#include "Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h"
4242
#include "Firestore/core/src/firebase/firestore/core/database_info.h"
43+
#include "Firestore/core/src/firebase/firestore/core/transaction.h"
4344
#include "Firestore/core/src/firebase/firestore/model/database_id.h"
4445
#include "Firestore/core/src/firebase/firestore/model/resource_path.h"
4546
#include "Firestore/core/src/firebase/firestore/util/async_queue.h"
@@ -53,6 +54,7 @@
5354
using firebase::firestore::auth::CredentialsProvider;
5455
using firebase::firestore::auth::FirebaseCredentialsProvider;
5556
using firebase::firestore::core::DatabaseInfo;
57+
using firebase::firestore::core::Transaction;
5658
using firebase::firestore::model::DatabaseId;
5759
using firebase::firestore::model::ResourcePath;
5860
using util::AsyncQueue;
@@ -275,18 +277,19 @@ - (void)runTransactionWithBlock:(id _Nullable (^)(FIRTransaction *, NSError **))
275277
completion:
276278
(void (^)(id _Nullable result, NSError *_Nullable error))completion {
277279
// We wrap the function they provide in order to use internal implementation classes for
278-
// FSTTransaction, and to run the user callback block on the proper queue.
280+
// transaction, and to run the user callback block on the proper queue.
279281
if (!updateBlock) {
280282
FSTThrowInvalidArgument(@"Transaction block cannot be nil.");
281283
} else if (!completion) {
282284
FSTThrowInvalidArgument(@"Transaction completion block cannot be nil.");
283285
}
284286

285287
FSTTransactionBlock wrappedUpdate =
286-
^(FSTTransaction *internalTransaction,
288+
^(std::shared_ptr<Transaction> internalTransaction,
287289
void (^internalCompletion)(id _Nullable, NSError *_Nullable)) {
288290
FIRTransaction *transaction =
289-
[FIRTransaction transactionWithFSTTransaction:internalTransaction firestore:self];
291+
[FIRTransaction transactionWithInternalTransaction:std::move(internalTransaction)
292+
firestore:self];
290293
dispatch_async(queue, ^{
291294
NSError *_Nullable error = nil;
292295
id _Nullable result = updateBlock(transaction, &error);

‎Firestore/Source/API/FIRTransaction+Internal.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,16 @@
1616

1717
#import "FIRTransaction.h"
1818

19+
#include <memory>
20+
21+
#include "Firestore/core/src/firebase/firestore/core/transaction.h"
22+
1923
@class FIRFirestore;
20-
@class FSTTransaction;
2124

2225
@interface FIRTransaction (Internal)
2326

24-
+ (instancetype)transactionWithFSTTransaction:(FSTTransaction *)transaction
25-
firestore:(FIRFirestore *)firestore;
27+
+ (instancetype)transactionWithInternalTransaction:
28+
(std::shared_ptr<firebase::firestore::core::Transaction>)transaction
29+
firestore:(FIRFirestore *)firestore;
2630

2731
@end

‎Firestore/Source/API/FIRTransaction.mm

Lines changed: 51 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -16,51 +16,59 @@
1616

1717
#import "FIRTransaction.h"
1818

19+
#include <memory>
1920
#include <utility>
21+
#include <vector>
2022

2123
#import "Firestore/Source/API/FIRDocumentReference+Internal.h"
2224
#import "Firestore/Source/API/FIRDocumentSnapshot+Internal.h"
2325
#import "Firestore/Source/API/FIRFirestore+Internal.h"
2426
#import "Firestore/Source/API/FIRTransaction+Internal.h"
2527
#import "Firestore/Source/API/FSTUserDataConverter.h"
26-
#import "Firestore/Source/Core/FSTTransaction.h"
2728
#import "Firestore/Source/Model/FSTDocument.h"
2829
#import "Firestore/Source/Util/FSTUsageValidation.h"
2930

31+
#include "Firestore/core/src/firebase/firestore/core/transaction.h"
32+
#include "Firestore/core/src/firebase/firestore/util/error_apple.h"
3033
#include "Firestore/core/src/firebase/firestore/util/hard_assert.h"
34+
#include "Firestore/core/src/firebase/firestore/util/status.h"
3135

3236
using firebase::firestore::core::ParsedSetData;
3337
using firebase::firestore::core::ParsedUpdateData;
38+
using firebase::firestore::core::Transaction;
39+
using firebase::firestore::util::MakeNSError;
40+
using firebase::firestore::util::Status;
3441

3542
NS_ASSUME_NONNULL_BEGIN
3643

3744
#pragma mark - FIRTransaction
3845

3946
@interface FIRTransaction ()
4047

41-
- (instancetype)initWithTransaction:(FSTTransaction *)transaction
48+
- (instancetype)initWithTransaction:(std::shared_ptr<Transaction>)transaction
4249
firestore:(FIRFirestore *)firestore NS_DESIGNATED_INITIALIZER;
4350

44-
@property(nonatomic, strong, readonly) FSTTransaction *internalTransaction;
4551
@property(nonatomic, strong, readonly) FIRFirestore *firestore;
4652
@end
4753

4854
@implementation FIRTransaction (Internal)
4955

50-
+ (instancetype)transactionWithFSTTransaction:(FSTTransaction *)transaction
51-
firestore:(FIRFirestore *)firestore {
52-
return [[FIRTransaction alloc] initWithTransaction:transaction firestore:firestore];
56+
+ (instancetype)transactionWithInternalTransaction:(std::shared_ptr<Transaction>)transaction
57+
firestore:(FIRFirestore *)firestore {
58+
return [[FIRTransaction alloc] initWithTransaction:std::move(transaction) firestore:firestore];
5359
}
5460

5561
@end
5662

57-
@implementation FIRTransaction
63+
@implementation FIRTransaction {
64+
std::shared_ptr<Transaction> _internalTransaction;
65+
}
5866

59-
- (instancetype)initWithTransaction:(FSTTransaction *)transaction
67+
- (instancetype)initWithTransaction:(std::shared_ptr<Transaction>)transaction
6068
firestore:(FIRFirestore *)firestore {
6169
self = [super init];
6270
if (self) {
63-
_internalTransaction = transaction;
71+
_internalTransaction = std::move(transaction);
6472
_firestore = firestore;
6573
}
6674
return self;
@@ -77,7 +85,7 @@ - (FIRTransaction *)setData:(NSDictionary<NSString *, id> *)data
7785
[self validateReference:document];
7886
ParsedSetData parsed = merge ? [self.firestore.dataConverter parsedMergeData:data fieldMask:nil]
7987
: [self.firestore.dataConverter parsedSetData:data];
80-
[self.internalTransaction setData:std::move(parsed) forDocument:document.key];
88+
_internalTransaction->Set(document.key, std::move(parsed));
8189
return self;
8290
}
8391

@@ -86,60 +94,58 @@ - (FIRTransaction *)setData:(NSDictionary<NSString *, id> *)data
8694
mergeFields:(NSArray<id> *)mergeFields {
8795
[self validateReference:document];
8896
ParsedSetData parsed = [self.firestore.dataConverter parsedMergeData:data fieldMask:mergeFields];
89-
[self.internalTransaction setData:std::move(parsed) forDocument:document.key];
97+
_internalTransaction->Set(document.key, std::move(parsed));
9098
return self;
9199
}
92100

93101
- (FIRTransaction *)updateData:(NSDictionary<id, id> *)fields
94102
forDocument:(FIRDocumentReference *)document {
95103
[self validateReference:document];
96104
ParsedUpdateData parsed = [self.firestore.dataConverter parsedUpdateData:fields];
97-
[self.internalTransaction updateData:std::move(parsed) forDocument:document.key];
105+
_internalTransaction->Update(document.key, std::move(parsed));
98106
return self;
99107
}
100108

101109
- (FIRTransaction *)deleteDocument:(FIRDocumentReference *)document {
102110
[self validateReference:document];
103-
[self.internalTransaction deleteDocument:document.key];
111+
_internalTransaction->Delete(document.key);
104112
return self;
105113
}
106114

107115
- (void)getDocument:(FIRDocumentReference *)document
108116
completion:(void (^)(FIRDocumentSnapshot *_Nullable document,
109117
NSError *_Nullable error))completion {
110118
[self validateReference:document];
111-
[self.internalTransaction
112-
lookupDocumentsForKeys:{document.key}
113-
completion:^(NSArray<FSTMaybeDocument *> *_Nullable documents,
114-
NSError *_Nullable error) {
115-
if (error) {
116-
completion(nil, error);
117-
return;
118-
}
119-
HARD_ASSERT(documents.count == 1,
120-
"Mismatch in docs returned from document lookup.");
121-
FSTMaybeDocument *internalDoc = documents.firstObject;
122-
if ([internalDoc isKindOfClass:[FSTDeletedDocument class]]) {
123-
FIRDocumentSnapshot *doc =
124-
[FIRDocumentSnapshot snapshotWithFirestore:self.firestore
125-
documentKey:document.key
126-
document:nil
127-
fromCache:NO
128-
hasPendingWrites:NO];
129-
completion(doc, nil);
130-
} else if ([internalDoc isKindOfClass:[FSTDocument class]]) {
131-
FIRDocumentSnapshot *doc =
132-
[FIRDocumentSnapshot snapshotWithFirestore:self.firestore
133-
documentKey:internalDoc.key
134-
document:(FSTDocument *)internalDoc
135-
fromCache:NO
136-
hasPendingWrites:NO];
137-
completion(doc, nil);
138-
} else {
139-
HARD_FAIL("BatchGetDocumentsRequest returned unexpected document type: %s",
140-
NSStringFromClass([internalDoc class]));
141-
}
142-
}];
119+
_internalTransaction->Lookup(
120+
{document.key}, [self, document, completion](const std::vector<FSTMaybeDocument *> &documents,
121+
const Status &status) {
122+
if (!status.ok()) {
123+
completion(nil, MakeNSError(status));
124+
return;
125+
}
126+
127+
HARD_ASSERT(documents.size() == 1, "Mismatch in docs returned from document lookup.");
128+
FSTMaybeDocument *internalDoc = documents.front();
129+
if ([internalDoc isKindOfClass:[FSTDeletedDocument class]]) {
130+
FIRDocumentSnapshot *doc = [FIRDocumentSnapshot snapshotWithFirestore:self.firestore
131+
documentKey:document.key
132+
document:nil
133+
fromCache:NO
134+
hasPendingWrites:NO];
135+
completion(doc, nil);
136+
} else if ([internalDoc isKindOfClass:[FSTDocument class]]) {
137+
FIRDocumentSnapshot *doc =
138+
[FIRDocumentSnapshot snapshotWithFirestore:self.firestore
139+
documentKey:internalDoc.key
140+
document:(FSTDocument *)internalDoc
141+
fromCache:NO
142+
hasPendingWrites:NO];
143+
completion(doc, nil);
144+
} else {
145+
HARD_FAIL("BatchGetDocumentsRequest returned unexpected document type: %s",
146+
NSStringFromClass([internalDoc class]));
147+
}
148+
});
143149
}
144150

145151
- (FIRDocumentSnapshot *_Nullable)getDocument:(FIRDocumentReference *)document

‎Firestore/Source/Core/FSTFirestoreClient.mm

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
#import "Firestore/Source/Core/FSTEventManager.h"
3232
#import "Firestore/Source/Core/FSTQuery.h"
3333
#import "Firestore/Source/Core/FSTSyncEngine.h"
34-
#import "Firestore/Source/Core/FSTTransaction.h"
3534
#import "Firestore/Source/Core/FSTView.h"
3635
#import "Firestore/Source/Local/FSTLRUGarbageCollector.h"
3736
#import "Firestore/Source/Local/FSTLevelDB.h"

‎Firestore/Source/Core/FSTSyncEngine.mm

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@
1717
#import "Firestore/Source/Core/FSTSyncEngine.h"
1818

1919
#include <map>
20+
#include <memory>
2021
#include <set>
2122
#include <unordered_map>
2223
#include <utility>
2324

2425
#import "FIRFirestoreErrors.h"
2526
#import "Firestore/Source/Core/FSTQuery.h"
26-
#import "Firestore/Source/Core/FSTTransaction.h"
2727
#import "Firestore/Source/Core/FSTView.h"
2828
#import "Firestore/Source/Core/FSTViewSnapshot.h"
2929
#import "Firestore/Source/Local/FSTLocalStore.h"
@@ -36,18 +36,22 @@
3636

3737
#include "Firestore/core/src/firebase/firestore/auth/user.h"
3838
#include "Firestore/core/src/firebase/firestore/core/target_id_generator.h"
39+
#include "Firestore/core/src/firebase/firestore/core/transaction.h"
3940
#include "Firestore/core/src/firebase/firestore/local/reference_set.h"
4041
#include "Firestore/core/src/firebase/firestore/model/document_key.h"
4142
#include "Firestore/core/src/firebase/firestore/model/document_map.h"
4243
#include "Firestore/core/src/firebase/firestore/model/snapshot_version.h"
4344
#include "Firestore/core/src/firebase/firestore/remote/remote_event.h"
45+
#include "Firestore/core/src/firebase/firestore/util/error_apple.h"
4446
#include "Firestore/core/src/firebase/firestore/util/hard_assert.h"
4547
#include "Firestore/core/src/firebase/firestore/util/log.h"
48+
#include "Firestore/core/src/firebase/firestore/util/status.h"
4649
#include "absl/types/optional.h"
4750

4851
using firebase::firestore::auth::HashUser;
4952
using firebase::firestore::auth::User;
5053
using firebase::firestore::core::TargetIdGenerator;
54+
using firebase::firestore::core::Transaction;
5155
using firebase::firestore::local::ReferenceSet;
5256
using firebase::firestore::model::BatchId;
5357
using firebase::firestore::model::DocumentKey;
@@ -62,6 +66,8 @@
6266
using firebase::firestore::remote::RemoteStore;
6367
using firebase::firestore::remote::TargetChange;
6468
using firebase::firestore::util::AsyncQueue;
69+
using firebase::firestore::util::MakeNSError;
70+
using firebase::firestore::util::Status;
6571

6672
NS_ASSUME_NONNULL_BEGIN
6773

@@ -287,27 +293,30 @@ - (void)transactionWithRetries:(int)retries
287293
completion:(FSTVoidIDErrorBlock)completion {
288294
workerQueue->VerifyIsCurrentQueue();
289295
HARD_ASSERT(retries >= 0, "Got negative number of retries for transaction");
290-
FSTTransaction *transaction = _remoteStore->CreateTransaction();
296+
297+
std::shared_ptr<Transaction> transaction = _remoteStore->CreateTransaction();
291298
updateBlock(transaction, ^(id _Nullable result, NSError *_Nullable error) {
292299
workerQueue->Enqueue(
293300
[self, retries, workerQueue, updateBlock, completion, transaction, result, error] {
294301
if (error) {
295302
completion(nil, error);
296303
return;
297304
}
298-
[transaction commitWithCompletion:^(NSError *_Nullable transactionError) {
299-
if (!transactionError) {
305+
transaction->Commit([self, retries, workerQueue, updateBlock, completion,
306+
result](const Status &status) {
307+
if (status.ok()) {
300308
completion(result, nil);
301309
return;
302310
}
311+
303312
// TODO(b/35201829): Only retry on real transaction failures.
304313
if (retries == 0) {
305314
NSError *wrappedError =
306315
[NSError errorWithDomain:FIRFirestoreErrorDomain
307316
code:FIRFirestoreErrorCodeFailedPrecondition
308317
userInfo:@{
309318
NSLocalizedDescriptionKey : @"Transaction failed all retries.",
310-
NSUnderlyingErrorKey : transactionError
319+
NSUnderlyingErrorKey : MakeNSError(status)
311320
}];
312321
completion(nil, wrappedError);
313322
return;
@@ -317,7 +326,7 @@ - (void)transactionWithRetries:(int)retries
317326
workerQueue:workerQueue
318327
updateBlock:updateBlock
319328
completion:completion];
320-
}];
329+
});
321330
});
322331
});
323332
}

0 commit comments

Comments
 (0)