Skip to content

Commit 392e763

Browse files
schmidt-sebastianschmidt-sebastian
authored
View changes for held write acks (#2004)
1 parent 6d8cb93 commit 392e763

18 files changed

+228
-85
lines changed

‎Firestore/Example/Tests/API/FIRQuerySnapshotTests.mm

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ - (void)testIncludeMetadataChanges {
9393
oldDocuments:oldDocuments
9494
documentChanges:documentChanges
9595
fromCache:NO
96-
hasPendingWrites:NO
96+
mutatedKeys:DocumentKeySet {}
9797
syncStateChanged:YES];
9898
FIRSnapshotMetadata *metadata =
9999
[FIRSnapshotMetadata snapshotMetadataWithPendingWrites:NO fromCache:NO];
@@ -105,11 +105,13 @@ - (void)testIncludeMetadataChanges {
105105
FIRQueryDocumentSnapshot *doc1Snap = [FIRQueryDocumentSnapshot snapshotWithFirestore:firestore
106106
documentKey:doc1New.key
107107
document:doc1New
108-
fromCache:NO];
108+
fromCache:NO
109+
hasPendingWrites:NO];
109110
FIRQueryDocumentSnapshot *doc2Snap = [FIRQueryDocumentSnapshot snapshotWithFirestore:firestore
110111
documentKey:doc2New.key
111112
document:doc2New
112-
fromCache:NO];
113+
fromCache:NO
114+
hasPendingWrites:NO];
113115

114116
NSArray<FIRDocumentChange *> *changesWithoutMetadata = @[
115117
[[FIRDocumentChange alloc] initWithType:FIRDocumentChangeTypeModified

‎Firestore/Example/Tests/API/FSTAPIHelpers.mm

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@
7070
return [FIRDocumentSnapshot snapshotWithFirestore:FSTTestFirestore()
7171
documentKey:testutil::Key(path)
7272
document:doc
73-
fromCache:fromCache];
73+
fromCache:fromCache
74+
hasPendingWrites:hasMutations];
7475
}
7576

7677
FIRCollectionReference *FSTTestCollectionRef(const absl::string_view path) {
@@ -93,12 +94,17 @@
9394
FIRSnapshotMetadata *metadata =
9495
[FIRSnapshotMetadata snapshotMetadataWithPendingWrites:hasPendingWrites fromCache:fromCache];
9596
FSTDocumentSet *oldDocuments = FSTTestDocSet(FSTDocumentComparatorByKey, @[]);
97+
DocumentKeySet mutatedKeys;
9698
for (NSString *key in oldDocs) {
9799
oldDocuments = [oldDocuments
98100
documentSetByAddingDocument:FSTTestDoc(util::StringFormat("%s/%s", path, key), 1,
99101
oldDocs[key],
100102
hasPendingWrites ? FSTDocumentStateLocalMutations
101103
: FSTDocumentStateSynced)];
104+
if (hasPendingWrites) {
105+
const absl::string_view documentKey = util::StringFormat("%s/%s", path, key);
106+
mutatedKeys = mutatedKeys.insert(testutil::Key(documentKey));
107+
}
102108
}
103109
FSTDocumentSet *newDocuments = oldDocuments;
104110
NSArray<FSTDocumentViewChange *> *documentChanges = [NSArray array];
@@ -111,13 +117,17 @@
111117
arrayByAddingObject:[FSTDocumentViewChange
112118
changeWithDocument:docToAdd
113119
type:FSTDocumentViewChangeTypeAdded]];
120+
if (hasPendingWrites) {
121+
const absl::string_view documentKey = util::StringFormat("%s/%s", path, key);
122+
mutatedKeys = mutatedKeys.insert(testutil::Key(documentKey));
123+
}
114124
}
115125
FSTViewSnapshot *viewSnapshot = [[FSTViewSnapshot alloc] initWithQuery:FSTTestQuery(path)
116126
documents:newDocuments
117127
oldDocuments:oldDocuments
118128
documentChanges:documentChanges
119129
fromCache:fromCache
120-
hasPendingWrites:hasPendingWrites
130+
mutatedKeys:mutatedKeys
121131
syncStateChanged:YES];
122132
return [FIRQuerySnapshot snapshotWithFirestore:FSTTestFirestore()
123133
originalQuery:FSTTestQuery(path)

‎Firestore/Example/Tests/Core/FSTQueryListenerTests.mm

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ - (void)testRaisesCollectionEvents {
9191
oldDocuments:[FSTDocumentSet documentSetWithComparator:snap2.query.comparator]
9292
documentChanges:@[ change1, change4 ]
9393
fromCache:snap2.fromCache
94-
hasPendingWrites:snap2.hasPendingWrites
94+
mutatedKeys:snap2.mutatedKeys
9595
syncStateChanged:YES];
9696
XCTAssertEqualObjects(otherAccum, (@[ expectedSnap2 ]));
9797
}
@@ -290,7 +290,7 @@ - (void)testRaisesQueryMetadataEventsOnlyWhenHasPendingWritesOnTheQueryChanges {
290290
oldDocuments:snap3.documents
291291
documentChanges:@[]
292292
fromCache:snap4.fromCache
293-
hasPendingWrites:NO
293+
mutatedKeys:snap4.mutatedKeys
294294
syncStateChanged:snap4.syncStateChanged];
295295
XCTAssertEqualObjects(fullAccum, (@[ snap1, snap3, expectedSnap4 ]));
296296
}
@@ -324,7 +324,7 @@ - (void)testMetadataOnlyDocumentChangesAreFilteredOutWhenIncludeDocumentMetadata
324324
oldDocuments:snap1.documents
325325
documentChanges:@[ change3 ]
326326
fromCache:snap2.isFromCache
327-
hasPendingWrites:snap2.hasPendingWrites
327+
mutatedKeys:snap2.mutatedKeys
328328
syncStateChanged:snap2.syncStateChanged];
329329
XCTAssertEqualObjects(filteredAccum, (@[ snap1, expectedSnap2 ]));
330330
}
@@ -365,7 +365,7 @@ - (void)testWillWaitForSyncIfOnline {
365365
oldDocuments:[FSTDocumentSet documentSetWithComparator:snap3.query.comparator]
366366
documentChanges:@[ change1, change2 ]
367367
fromCache:NO
368-
hasPendingWrites:NO
368+
mutatedKeys:snap3.mutatedKeys
369369
syncStateChanged:YES];
370370
XCTAssertEqualObjects(events, (@[ expectedSnap ]));
371371
}
@@ -404,14 +404,14 @@ - (void)testWillRaiseInitialEventWhenGoingOffline {
404404
oldDocuments:[FSTDocumentSet documentSetWithComparator:snap1.query.comparator]
405405
documentChanges:@[ change1 ]
406406
fromCache:YES
407-
hasPendingWrites:NO
407+
mutatedKeys:snap1.mutatedKeys
408408
syncStateChanged:YES];
409409
FSTViewSnapshot *expectedSnap2 = [[FSTViewSnapshot alloc] initWithQuery:query
410410
documents:snap2.documents
411411
oldDocuments:snap1.documents
412412
documentChanges:@[ change2 ]
413413
fromCache:YES
414-
hasPendingWrites:NO
414+
mutatedKeys:snap2.mutatedKeys
415415
syncStateChanged:NO];
416416
XCTAssertEqualObjects(events, (@[ expectedSnap1, expectedSnap2 ]));
417417
}
@@ -437,7 +437,7 @@ - (void)testWillRaiseInitialEventWhenGoingOfflineAndThereAreNoDocs {
437437
oldDocuments:[FSTDocumentSet documentSetWithComparator:snap1.query.comparator]
438438
documentChanges:@[]
439439
fromCache:YES
440-
hasPendingWrites:NO
440+
mutatedKeys:snap1.mutatedKeys
441441
syncStateChanged:YES];
442442
XCTAssertEqualObjects(events, (@[ expectedSnap ]));
443443
}
@@ -462,7 +462,7 @@ - (void)testWillRaiseInitialEventWhenStartingOfflineAndThereAreNoDocs {
462462
oldDocuments:[FSTDocumentSet documentSetWithComparator:snap1.query.comparator]
463463
documentChanges:@[]
464464
fromCache:YES
465-
hasPendingWrites:NO
465+
mutatedKeys:snap1.mutatedKeys
466466
syncStateChanged:YES];
467467
XCTAssertEqualObjects(events, (@[ expectedSnap ]));
468468
}

‎Firestore/Example/Tests/Core/FSTViewSnapshotTest.mm

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,23 +116,23 @@ - (void)testViewSnapshotConstructor {
116116
type:FSTDocumentViewChangeTypeAdded] ];
117117

118118
BOOL fromCache = YES;
119-
BOOL hasPendingWrites = NO;
119+
DocumentKeySet mutatedKeys;
120120
BOOL syncStateChanged = YES;
121121

122122
FSTViewSnapshot *snapshot = [[FSTViewSnapshot alloc] initWithQuery:query
123123
documents:documents
124124
oldDocuments:oldDocuments
125125
documentChanges:documentChanges
126126
fromCache:fromCache
127-
hasPendingWrites:hasPendingWrites
127+
mutatedKeys:mutatedKeys
128128
syncStateChanged:syncStateChanged];
129129

130130
XCTAssertEqual(snapshot.query, query);
131131
XCTAssertEqual(snapshot.documents, documents);
132132
XCTAssertEqual(snapshot.oldDocuments, oldDocuments);
133133
XCTAssertEqual(snapshot.documentChanges, documentChanges);
134134
XCTAssertEqual(snapshot.fromCache, fromCache);
135-
XCTAssertEqual(snapshot.hasPendingWrites, hasPendingWrites);
135+
XCTAssertEqual(snapshot.mutatedKeys, mutatedKeys);
136136
XCTAssertEqual(snapshot.syncStateChanged, syncStateChanged);
137137
}
138138

‎Firestore/Example/Tests/Core/FSTViewTests.mm

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -641,6 +641,76 @@ - (void)testRemembersLocalMutationsFromPreviousCallToComputeChangesWithDocuments
641641
XCTAssertEqual(changes.mutatedKeys, (DocumentKeySet{doc2.key}));
642642
}
643643

644+
- (void)testRaisesHasPendingWritesForPendingMutationsInInitialSnapshot {
645+
FSTQuery *query = [self queryForMessages];
646+
FSTDocument *doc1 = FSTTestDoc("rooms/eros/messages/1", 0, @{}, FSTDocumentStateLocalMutations);
647+
FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}];
648+
FSTViewDocumentChanges *changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1 ])];
649+
FSTViewChange *viewChange = [view applyChangesToDocuments:changes];
650+
XCTAssertTrue(viewChange.snapshot.hasPendingWrites);
651+
}
652+
653+
- (void)testDoesntRaiseHasPendingWritesForCommittedMutationsInInitialSnapshot {
654+
FSTQuery *query = [self queryForMessages];
655+
FSTDocument *doc1 =
656+
FSTTestDoc("rooms/eros/messages/1", 0, @{}, FSTDocumentStateCommittedMutations);
657+
FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}];
658+
FSTViewDocumentChanges *changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1 ])];
659+
FSTViewChange *viewChange = [view applyChangesToDocuments:changes];
660+
XCTAssertFalse(viewChange.snapshot.hasPendingWrites);
661+
}
662+
663+
- (void)testSuppressesWriteAcknowledgementIfWatchHasNotCaughtUp {
664+
// This test verifies that we don't get three events for an FSTServerTimestamp mutation. We
665+
// suppress the event generated by the write acknowledgement and instead wait for Watch to catch
666+
// up.
667+
668+
FSTQuery *query = [self queryForMessages];
669+
FSTDocument *doc1 = FSTTestDoc("rooms/eros/messages/1", 1,
670+
@{@"time" : @1}, FSTDocumentStateLocalMutations);
671+
FSTDocument *doc1Committed = FSTTestDoc("rooms/eros/messages/1", 2,
672+
@{@"time" : @2}, FSTDocumentStateCommittedMutations);
673+
FSTDocument *doc1Acknowledged = FSTTestDoc("rooms/eros/messages/1", 2,
674+
@{@"time" : @2}, FSTDocumentStateSynced);
675+
FSTDocument *doc2 = FSTTestDoc("rooms/eros/messages/2", 1,
676+
@{@"time" : @1}, FSTDocumentStateLocalMutations);
677+
FSTDocument *doc2Modified = FSTTestDoc("rooms/eros/messages/2", 2,
678+
@{@"time" : @3}, FSTDocumentStateLocalMutations);
679+
FSTDocument *doc2Acknowledged = FSTTestDoc("rooms/eros/messages/2", 2,
680+
@{@"time" : @3}, FSTDocumentStateSynced);
681+
FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}];
682+
FSTViewDocumentChanges *changes =
683+
[view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])];
684+
FSTViewChange *viewChange = [view applyChangesToDocuments:changes];
685+
686+
XCTAssertEqualObjects(
687+
(@[
688+
[FSTDocumentViewChange changeWithDocument:doc1 type:FSTDocumentViewChangeTypeAdded],
689+
[FSTDocumentViewChange changeWithDocument:doc2 type:FSTDocumentViewChangeTypeAdded]
690+
]),
691+
viewChange.snapshot.documentChanges);
692+
693+
changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1Committed, doc2Modified ])];
694+
viewChange = [view applyChangesToDocuments:changes];
695+
// The 'doc1Committed' update is suppressed
696+
XCTAssertEqualObjects(
697+
(@[ [FSTDocumentViewChange changeWithDocument:doc2Modified
698+
type:FSTDocumentViewChangeTypeModified] ]),
699+
viewChange.snapshot.documentChanges);
700+
701+
changes =
702+
[view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1Acknowledged, doc2Acknowledged ])];
703+
viewChange = [view applyChangesToDocuments:changes];
704+
XCTAssertEqualObjects(
705+
(@[
706+
[FSTDocumentViewChange changeWithDocument:doc1Acknowledged
707+
type:FSTDocumentViewChangeTypeModified],
708+
[FSTDocumentViewChange changeWithDocument:doc2Acknowledged
709+
type:FSTDocumentViewChangeTypeMetadata]
710+
]),
711+
viewChange.snapshot.documentChanges);
712+
}
713+
644714
@end
645715

646716
NS_ASSUME_NONNULL_END

‎Firestore/Source/API/FIRDocumentChange.mm

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,12 @@ + (FIRDocumentChangeType)documentChangeTypeForChange:(FSTDocumentViewChange *)ch
6060
NSUInteger index = 0;
6161
NSMutableArray<FIRDocumentChange *> *changes = [NSMutableArray array];
6262
for (FSTDocumentViewChange *change in snapshot.documentChanges) {
63-
FIRQueryDocumentSnapshot *document =
64-
[FIRQueryDocumentSnapshot snapshotWithFirestore:firestore
65-
documentKey:change.document.key
66-
document:change.document
67-
fromCache:snapshot.isFromCache];
63+
FIRQueryDocumentSnapshot *document = [FIRQueryDocumentSnapshot
64+
snapshotWithFirestore:firestore
65+
documentKey:change.document.key
66+
document:change.document
67+
fromCache:snapshot.isFromCache
68+
hasPendingWrites:snapshot.mutatedKeys.contains(change.document.key)];
6869
HARD_ASSERT(change.type == FSTDocumentViewChangeTypeAdded,
6970
"Invalid event type for first snapshot");
7071
HARD_ASSERT(!lastDocument || snapshot.query.comparator(lastDocument, change.document) ==
@@ -86,11 +87,12 @@ + (FIRDocumentChangeType)documentChangeTypeForChange:(FSTDocumentViewChange *)ch
8687
continue;
8788
}
8889

89-
FIRQueryDocumentSnapshot *document =
90-
[FIRQueryDocumentSnapshot snapshotWithFirestore:firestore
91-
documentKey:change.document.key
92-
document:change.document
93-
fromCache:snapshot.isFromCache];
90+
FIRQueryDocumentSnapshot *document = [FIRQueryDocumentSnapshot
91+
snapshotWithFirestore:firestore
92+
documentKey:change.document.key
93+
document:change.document
94+
fromCache:snapshot.isFromCache
95+
hasPendingWrites:snapshot.mutatedKeys.contains(change.document.key)];
9496

9597
NSUInteger oldIndex = NSNotFound;
9698
NSUInteger newIndex = NSNotFound;

‎Firestore/Source/API/FIRDocumentReference.mm

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,10 +274,15 @@ - (void)getDocumentWithSource:(FIRFirestoreSource)source
274274
HARD_ASSERT(snapshot.documents.count <= 1, "Too many document returned on a document query");
275275
FSTDocument *document = [snapshot.documents documentForKey:key];
276276

277+
BOOL hasPendingWrites = document
278+
? snapshot.mutatedKeys.contains(key)
279+
: NO; // We don't raise `hasPendingWrites` for deleted documents.
280+
277281
FIRDocumentSnapshot *result = [FIRDocumentSnapshot snapshotWithFirestore:firestore
278282
documentKey:key
279283
document:document
280-
fromCache:snapshot.fromCache];
284+
fromCache:snapshot.fromCache
285+
hasPendingWrites:hasPendingWrites];
281286
listener(result, nil);
282287
};
283288

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ NS_ASSUME_NONNULL_BEGIN
2929
+ (instancetype)snapshotWithFirestore:(FIRFirestore *)firestore
3030
documentKey:(firebase::firestore::model::DocumentKey)documentKey
3131
document:(nullable FSTDocument *)document
32-
fromCache:(BOOL)fromCache;
32+
fromCache:(BOOL)fromCache
33+
hasPendingWrites:(BOOL)pendingWrites;
3334

3435
@property(nonatomic, strong, readonly, nullable) FSTDocument *internalDocument;
3536

0 commit comments

Comments
 (0)