Skip to content

Commit c3b37c9

Browse files
committed
port waitForPendingWrites
1 parent e64663a commit c3b37c9

21 files changed

+277
-9
lines changed

Firestore/Example/Tests/Integration/API/FIRDatabaseTests.mm

+52
Original file line numberDiff line numberDiff line change
@@ -1402,4 +1402,56 @@ - (void)testCanRemoveListenerAfterShutdown {
14021402
[listenerRegistration remove];
14031403
}
14041404

1405+
- (void)testWaitForPendingWritesCompletes {
1406+
FIRDocumentReference *doc = [self documentRef];
1407+
FIRFirestore *firestore = doc.firestore;
1408+
1409+
[self disableNetwork];
1410+
1411+
[doc setData:@{@"foo" : @"bar"}];
1412+
[firestore waitForPendingWritesWithCompletion:
1413+
[self completionForExpectationWithName:@"Wait for pending writes"]];
1414+
1415+
[firestore enableNetworkWithCompletion:[self completionForExpectationWithName:@"Enable network"]];
1416+
[self awaitExpectations];
1417+
}
1418+
1419+
- (void)testWaitForPendingWritesFailsWhenUserChanges {
1420+
FIRApp *app = testutil::AppForUnitTesting(util::MakeString([FSTIntegrationTestCase projectID]));
1421+
FIRFirestore *firestore = [self firestoreWithApp:app];
1422+
1423+
[firestore
1424+
disableNetworkWithCompletion:[self completionForExpectationWithName:@"Disable network"]];
1425+
[self awaitExpectations];
1426+
1427+
// Writes to local to prevent immediate call to the completion of waitForPendingWrites.
1428+
NSDictionary<NSString *, id> *data =
1429+
@{@"owner" : @{@"name" : @"Andy", @"email" : @"abc@xyz.com"}};
1430+
[[firestore documentWithPath:@"abc/123"] setData:data];
1431+
1432+
XCTestExpectation *expectation = [self expectationWithDescription:@"waitForPendingWrites"];
1433+
[firestore waitForPendingWritesWithCompletion:^(NSError *_Nullable error) {
1434+
XCTAssertNotNil(error);
1435+
XCTAssertEqualObjects(error.domain, FIRFirestoreErrorDomain);
1436+
XCTAssertEqual(error.code, FIRFirestoreErrorCodeCancelled);
1437+
[expectation fulfill];
1438+
}];
1439+
1440+
[self triggerUserChangeWithUid:@"user-to-fail-pending-writes"];
1441+
[self awaitExpectations];
1442+
}
1443+
1444+
- (void)testWaitForPendingWritesCompletesWhenOfflineIfNoPending {
1445+
FIRDocumentReference *doc = [self documentRef];
1446+
FIRFirestore *firestore = doc.firestore;
1447+
1448+
[firestore
1449+
disableNetworkWithCompletion:[self completionForExpectationWithName:@"Disable network"]];
1450+
[self awaitExpectations];
1451+
1452+
[firestore waitForPendingWritesWithCompletion:
1453+
[self completionForExpectationWithName:@"Wait for pending writes"]];
1454+
[self awaitExpectations];
1455+
}
1456+
14051457
@end

Firestore/Example/Tests/Local/FSTLevelDBMutationQueueTests.mm

+2-2
Original file line numberDiff line numberDiff line change
@@ -87,14 +87,14 @@ - (void)setUp {
8787

8888
- (void)testLoadNextBatchID_zeroWhenTotallyEmpty {
8989
// Initial seek is invalid
90-
XCTAssertEqual(LoadNextBatchIdFromDb(_db.ptr), 0);
90+
XCTAssertEqual(LoadNextBatchIdFromDb(_db.ptr), 1);
9191
}
9292

9393
- (void)testLoadNextBatchID_zeroWhenNoMutations {
9494
// Initial seek finds no mutations
9595
[self setDummyValueForKey:MutationLikeKey("mutationr", "foo", 20)];
9696
[self setDummyValueForKey:MutationLikeKey("mutationsa", "foo", 10)];
97-
XCTAssertEqual(LoadNextBatchIdFromDb(_db.ptr), 0);
97+
XCTAssertEqual(LoadNextBatchIdFromDb(_db.ptr), 1);
9898
}
9999

100100
- (void)testLoadNextBatchID_findsSingleRow {

Firestore/Example/Tests/Local/FSTLocalStoreTests.mm

+18
Original file line numberDiff line numberDiff line change
@@ -1106,6 +1106,24 @@ - (void)testHandlesPatchMutationWithTransformThenRemoteEvent {
11061106
FSTAssertChanged(Doc("foo/bar", 1, Map("sum", 1), DocumentState::kLocalMutations));
11071107
}
11081108

1109+
- (void)testGetHighestUnacknowledgeBatchId {
1110+
if ([self isTestBaseClass]) return;
1111+
1112+
XCTAssertEqual(-1, [self.localStore getHighestUnacknowledgedBatchId]);
1113+
1114+
[self writeMutation:FSTTestSetMutation(@"foo/bar", @{@"abc" : @123})];
1115+
XCTAssertEqual(1, [self.localStore getHighestUnacknowledgedBatchId]);
1116+
1117+
[self writeMutation:FSTTestPatchMutation("foo/bar", @{@"abc" : @321}, {})];
1118+
XCTAssertEqual(2, [self.localStore getHighestUnacknowledgedBatchId]);
1119+
1120+
[self acknowledgeMutationWithVersion:1];
1121+
XCTAssertEqual(2, [self.localStore getHighestUnacknowledgedBatchId]);
1122+
1123+
[self rejectMutation];
1124+
XCTAssertEqual(-1, [self.localStore getHighestUnacknowledgedBatchId]);
1125+
}
1126+
11091127
@end
11101128

11111129
NS_ASSUME_NONNULL_END

Firestore/Example/Tests/Util/FSTIntegrationTestCase.h

+6-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,12 @@ extern "C" {
5555
/** Returns a new Firestore connected to the project with the given projectID. */
5656
- (FIRFirestore *)firestoreWithProjectID:(NSString *)projectID;
5757

58-
/** Returns a new Firestore connected to the project with the given app. */
58+
/** Triggers a user change with given user id. */
59+
- (void)triggerUserChangeWithUid:(NSString *)uid;
60+
61+
/**
62+
* Returns a new Firestore connected to the project with the given app.
63+
*/
5964
- (FIRFirestore *)firestoreWithApp:(FIRApp *)app;
6065

6166
/** Synchronously shuts down the given firestore. */

Firestore/Example/Tests/Util/FSTIntegrationTestCase.mm

+33-3
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@
3333
#include <string>
3434
#include <utility>
3535

36+
#include "Firestore/core/src/firebase/firestore/auth/credentials_provider.h"
3637
#include "Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h"
38+
#include "Firestore/core/src/firebase/firestore/auth/user.h"
3739
#include "Firestore/core/src/firebase/firestore/model/database_id.h"
3840
#include "Firestore/core/src/firebase/firestore/remote/grpc_connection.h"
3941
#include "Firestore/core/src/firebase/firestore/util/autoid.h"
@@ -55,8 +57,10 @@
5557
#include "Firestore/core/src/firebase/firestore/util/executor_libdispatch.h"
5658

5759
namespace util = firebase::firestore::util;
60+
using firebase::firestore::auth::CredentialChangeListener;
5861
using firebase::firestore::auth::CredentialsProvider;
5962
using firebase::firestore::auth::EmptyCredentialsProvider;
63+
using firebase::firestore::auth::User;
6064
using firebase::firestore::model::DatabaseId;
6165
using firebase::firestore::testutil::AppForUnitTesting;
6266
using firebase::firestore::remote::GrpcConnection;
@@ -79,13 +83,37 @@
7983

8084
static bool runningAgainstEmulator = false;
8185

86+
// Behaves the same as `EmptyCredentialsProvider` except it can also trigger a user
87+
// change.
88+
class MockCredentialsProvider : public EmptyCredentialsProvider {
89+
public:
90+
void SetCredentialChangeListener(CredentialChangeListener changeListener) override {
91+
if (changeListener) {
92+
listener_ = std::move(changeListener);
93+
listener_(User::Unauthenticated());
94+
}
95+
}
96+
97+
void ChangeUser(NSString *new_id) {
98+
if (listener_) {
99+
listener_(firebase::firestore::auth::User::FromUid(new_id));
100+
}
101+
}
102+
103+
private:
104+
CredentialChangeListener listener_;
105+
};
106+
82107
@implementation FSTIntegrationTestCase {
83108
NSMutableArray<FIRFirestore *> *_firestores;
109+
std::shared_ptr<MockCredentialsProvider> _mockCredProdiver;
84110
}
85111

86112
- (void)setUp {
87113
[super setUp];
88114

115+
_mockCredProdiver = std::make_shared<MockCredentialsProvider>();
116+
89117
[self clearPersistenceOnce];
90118
[self primeBackend];
91119

@@ -239,13 +267,11 @@ - (FIRFirestore *)firestoreWithApp:(FIRApp *)app {
239267

240268
FIRSetLoggerLevel(FIRLoggerLevelDebug);
241269

242-
std::unique_ptr<CredentialsProvider> credentials_provider =
243-
absl::make_unique<firebase::firestore::auth::EmptyCredentialsProvider>();
244270
std::string projectID = util::MakeString(app.options.projectID);
245271
FIRFirestore *firestore =
246272
[[FIRFirestore alloc] initWithDatabaseID:DatabaseId(projectID)
247273
persistenceKey:util::MakeString(persistenceKey)
248-
credentialsProvider:std::move(credentials_provider)
274+
credentialsProvider:_mockCredProdiver
249275
workerQueue:std::move(workerQueue)
250276
firebaseApp:app
251277
instanceRegistry:nil];
@@ -255,6 +281,10 @@ - (FIRFirestore *)firestoreWithApp:(FIRApp *)app {
255281
return firestore;
256282
}
257283

284+
- (void)triggerUserChangeWithUid:(NSString *)uid {
285+
_mockCredProdiver->ChangeUser(uid);
286+
}
287+
258288
- (void)primeBackend {
259289
static dispatch_once_t onceToken;
260290
dispatch_once(&onceToken, ^{

Firestore/Source/API/FIRFirestore.mm

+4
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,10 @@ - (FIRWriteBatch *)batch {
194194
writeBatch:_firestore->GetBatch()];
195195
}
196196

197+
- (void)waitForPendingWritesWithCompletion:(void (^)(NSError *_Nullable error))completion {
198+
_firestore->WaitForPendingWrites(util::MakeCallback(completion));
199+
}
200+
197201
- (void)runTransactionWithBlock:(id _Nullable (^)(FIRTransaction *, NSError **))updateBlock
198202
dispatchQueue:(dispatch_queue_t)queue
199203
completion:

Firestore/Source/Core/FSTFirestoreClient.h

+7
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,13 @@ NS_ASSUME_NONNULL_BEGIN
111111
- (void)writeMutations:(std::vector<model::Mutation> &&)mutations
112112
callback:(util::StatusCallback)callback;
113113

114+
/**
115+
* Passes a callback that is triggered when all the pending writes at the
116+
* time when this method is called received server acknowledgement.
117+
* An acknowledgement can be either acceptance or rejections.
118+
*/
119+
- (void)waitForPendingWritesWithCallback:(util::StatusCallback)callback;
120+
114121
/** Tries to execute the transaction in updateCallback up to retries times. */
115122
- (void)transactionWithRetries:(int)retries
116123
updateCallback:(core::TransactionUpdateCallback)updateCallback

Firestore/Source/Core/FSTFirestoreClient.mm

+14
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,20 @@ - (void)writeMutations:(std::vector<Mutation> &&)mutations callback:(util::Statu
442442
});
443443
};
444444

445+
- (void)waitForPendingWritesWithCallback:(util::StatusCallback)callback {
446+
[self verifyNotShutdown];
447+
// Dispatch the result back onto the user dispatch queue.
448+
auto async_callback = [self, callback](util::Status status) {
449+
if (callback) {
450+
self->_userExecutor->Execute([=] { callback(std::move(status)); });
451+
}
452+
};
453+
454+
_workerQueue->Enqueue([self, async_callback]() {
455+
[self.syncEngine registerPendingWritesCallback:std::move(async_callback)];
456+
});
457+
}
458+
445459
- (void)transactionWithRetries:(int)retries
446460
updateCallback:(core::TransactionUpdateCallback)update_callback
447461
resultCallback:(core::TransactionResultCallback)resultCallback {

Firestore/Source/Core/FSTSyncEngine.h

+6
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,12 @@ NS_ASSUME_NONNULL_BEGIN
8686
- (void)writeMutations:(std::vector<model::Mutation> &&)mutations
8787
completion:(FSTVoidErrorBlock)completion;
8888

89+
/**
90+
* Registers a user callback that is called when all pending mutations at the moment of calling
91+
* are acknowledged .
92+
*/
93+
- (void)registerPendingWritesCallback:(util::StatusCallback)callback;
94+
8995
/**
9096
* Runs the given transaction block up to retries times and then calls completion.
9197
*

Firestore/Source/Core/FSTSyncEngine.mm

+58
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
#include "Firestore/core/src/firebase/firestore/model/document_key.h"
4141
#include "Firestore/core/src/firebase/firestore/model/document_map.h"
4242
#include "Firestore/core/src/firebase/firestore/model/document_set.h"
43+
#include "Firestore/core/src/firebase/firestore/model/mutation_batch.h"
4344
#include "Firestore/core/src/firebase/firestore/model/no_document.h"
4445
#include "Firestore/core/src/firebase/firestore/model/snapshot_version.h"
4546
#include "Firestore/core/src/firebase/firestore/remote/remote_event.h"
@@ -64,6 +65,7 @@
6465
using firebase::firestore::model::DocumentKey;
6566
using firebase::firestore::model::DocumentKeySet;
6667
using firebase::firestore::model::DocumentMap;
68+
using firebase::firestore::model::kBatchIdUnknown;
6769
using firebase::firestore::model::ListenSequenceNumber;
6870
using firebase::firestore::model::MaybeDocumentMap;
6971
using firebase::firestore::model::Mutation;
@@ -78,6 +80,7 @@
7880
using firebase::firestore::util::AsyncQueue;
7981
using firebase::firestore::util::MakeNSError;
8082
using firebase::firestore::util::Status;
83+
using firebase::firestore::util::StatusCallback;
8184

8285
NS_ASSUME_NONNULL_BEGIN
8386

@@ -191,6 +194,9 @@ @implementation FSTSyncEngine {
191194
std::unordered_map<User, NSMutableDictionary<NSNumber *, FSTVoidErrorBlock> *, HashUser>
192195
_mutationCompletionBlocks;
193196

197+
/** Stores user callbacks waiting for pending writes to be acknowledged. */
198+
std::unordered_map<model::BatchId, std::vector<StatusCallback>> _pendingWritesCallbacks;
199+
194200
/** FSTQueryViews for all active queries, indexed by query. */
195201
std::unordered_map<Query, FSTQueryView *> _queryViewsByQuery;
196202

@@ -290,6 +296,51 @@ - (void)writeMutations:(std::vector<Mutation> &&)mutations
290296
_remoteStore->FillWritePipeline();
291297
}
292298

299+
- (void)registerPendingWritesCallback:(StatusCallback)callback {
300+
if (!_remoteStore->CanUseNetwork()) {
301+
LOG_DEBUG("The network is disabled. The task returned by 'awaitPendingWrites()' will not "
302+
"complete until the network is enabled.");
303+
}
304+
305+
int largestPendingBatchId = [self.localStore getHighestUnacknowledgedBatchId];
306+
307+
if (largestPendingBatchId == kBatchIdUnknown) {
308+
// Trigger the callback right away if there is no pending writes at the moment.
309+
callback(Status::OK());
310+
return;
311+
}
312+
313+
auto it = _pendingWritesCallbacks.find(largestPendingBatchId);
314+
if (it != _pendingWritesCallbacks.end()) {
315+
it->second.push_back(std::move(callback));
316+
} else {
317+
_pendingWritesCallbacks.emplace(largestPendingBatchId,
318+
std::vector<StatusCallback>({std::move(callback)}));
319+
}
320+
}
321+
322+
/** Triggers callbacks waiting for this batch id to get acknowledged by server, if there are any. */
323+
- (void)triggerPendingWriteCallbacksWithBatchId:(int)batchId {
324+
auto it = _pendingWritesCallbacks.find(batchId);
325+
if (it != _pendingWritesCallbacks.end()) {
326+
for (const auto &callback : it->second) {
327+
callback(Status::OK());
328+
}
329+
330+
_pendingWritesCallbacks.erase(it);
331+
}
332+
}
333+
334+
- (void)failOutstandingPendingWritesAwaitingCallbacks:(absl::string_view)errorMessage {
335+
for (const auto &entry : _pendingWritesCallbacks) {
336+
for (const auto &callback : entry.second) {
337+
callback(Status(Error::Cancelled, errorMessage));
338+
}
339+
}
340+
341+
_pendingWritesCallbacks.clear();
342+
}
343+
293344
- (void)addMutationCompletionBlock:(FSTVoidErrorBlock)completion batchID:(BatchId)batchID {
294345
NSMutableDictionary<NSNumber *, FSTVoidErrorBlock> *completionBlocks =
295346
_mutationCompletionBlocks[_currentUser];
@@ -454,6 +505,8 @@ - (void)applySuccessfulWriteWithResult:(FSTMutationBatchResult *)batchResult {
454505
// consistently happen before listen events.
455506
[self processUserCallbacksForBatchID:batchResult.batch.batchID error:nil];
456507

508+
[self triggerPendingWriteCallbacksWithBatchId:batchResult.batch.batchID];
509+
457510
MaybeDocumentMap changes = [self.localStore acknowledgeBatchWithResult:batchResult];
458511
[self emitNewSnapshotsAndNotifyLocalStoreWithChanges:changes remoteEvent:absl::nullopt];
459512
}
@@ -472,6 +525,8 @@ - (void)rejectFailedWriteWithBatchID:(BatchId)batchID error:(NSError *)error {
472525
// consistently happen before listen events.
473526
[self processUserCallbacksForBatchID:batchID error:error];
474527

528+
[self triggerPendingWriteCallbacksWithBatchId:batchID];
529+
475530
[self emitNewSnapshotsAndNotifyLocalStoreWithChanges:changes remoteEvent:absl::nullopt];
476531
}
477532

@@ -623,6 +678,9 @@ - (void)credentialDidChangeWithUser:(const firebase::firestore::auth::User &)use
623678
_currentUser = user;
624679

625680
if (userChanged) {
681+
// Fails callbacks waiting for pending writes requested by previous user.
682+
[self failOutstandingPendingWritesAwaitingCallbacks:
683+
"'waitForPendingWrites' callback is cancelled due to a user change."];
626684
// Notify local store and emit any resulting events from swapping out the mutation queue.
627685
MaybeDocumentMap changes = [self.localStore userDidChange:user];
628686
[self emitNewSnapshotsAndNotifyLocalStoreWithChanges:changes remoteEvent:absl::nullopt];

Firestore/Source/Local/FSTLocalStore.h

+6
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,12 @@ NS_ASSUME_NONNULL_BEGIN
192192
*/
193193
- (nullable FSTMutationBatch *)nextMutationBatchAfterBatchID:(model::BatchId)batchID;
194194

195+
/**
196+
* Returns the largest (latest) batch id in mutation queue that is pending server response.
197+
* Returns `kBatchIdUnknown` if the queue is empty.
198+
*/
199+
- (model::BatchId)getHighestUnacknowledgedBatchId;
200+
195201
- (local::LruResults)collectGarbage:(FSTLRUGarbageCollector *)garbageCollector;
196202

197203
@end

Firestore/Source/Local/FSTLocalStore.mm

+6
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,12 @@ - (nullable FSTMutationBatch *)nextMutationBatchAfterBatchID:(BatchId)batchID {
427427
return self.persistence.run("ReadDocument", [&] { return _localDocuments->GetDocument(key); });
428428
}
429429

430+
- (model::BatchId)getHighestUnacknowledgedBatchId {
431+
return self.persistence.run("getHighestUnacknowledgedBatchId", [&]() -> model::BatchId {
432+
return _mutationQueue->GetHighestUnacknowledgedBatchId();
433+
});
434+
}
435+
430436
- (FSTQueryData *)allocateQuery:(Query)query {
431437
FSTQueryData *queryData = self.persistence.run("Allocate query", [&] {
432438
FSTQueryData *cached = _queryCache->GetTarget(query);

0 commit comments

Comments
 (0)