diff options
author | Greg Soltis <gsoltis@google.com> | 2018-04-30 19:47:10 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-04-30 19:47:10 -0700 |
commit | c7034314893f8644165c5cccaf1fec63e4924f56 (patch) | |
tree | 61231caa27243f46832590bad28da1efd06c3ca4 /Firestore/Source | |
parent | d8e9113e67d9695aef605e89f3009db482a87fb2 (diff) |
Port the feedback from the Java port (#1199)
* Port the feedback from the port
* Fix comment grammar
Diffstat (limited to 'Firestore/Source')
-rw-r--r-- | Firestore/Source/Core/FSTSyncEngine.mm | 3 | ||||
-rw-r--r-- | Firestore/Source/Remote/FSTRemoteEvent.h | 16 | ||||
-rw-r--r-- | Firestore/Source/Remote/FSTRemoteEvent.mm | 31 |
3 files changed, 24 insertions, 26 deletions
diff --git a/Firestore/Source/Core/FSTSyncEngine.mm b/Firestore/Source/Core/FSTSyncEngine.mm index ca15e64..775f865 100644 --- a/Firestore/Source/Core/FSTSyncEngine.mm +++ b/Firestore/Source/Core/FSTSyncEngine.mm @@ -303,8 +303,7 @@ static const FSTListenSequenceNumber kIrrelevantSequenceNumber = -1; if (iter == self->_limboKeysByTarget.end()) { FSTQueryView *qv = self.queryViewsByTarget[targetID]; FSTAssert(qv, @"Missing queryview for non-limbo query: %i", [targetID intValue]); - [remoteEvent filterUpdatesFromTargetChange:targetChange - existingDocuments:qv.view.syncedDocuments]; + [targetChange.mapping filterUpdatesUsingExistingKeys:qv.view.syncedDocuments]; } else { [remoteEvent synthesizeDeleteForLimboTargetChange:targetChange key:iter->second]; } diff --git a/Firestore/Source/Remote/FSTRemoteEvent.h b/Firestore/Source/Remote/FSTRemoteEvent.h index 322cb63..06099c7 100644 --- a/Firestore/Source/Remote/FSTRemoteEvent.h +++ b/Firestore/Source/Remote/FSTRemoteEvent.h @@ -43,6 +43,14 @@ NS_ASSUME_NONNULL_BEGIN * base class. */ @interface FSTTargetMapping : NSObject + +/** + * Strips out mapping changes that aren't actually changes. That is, if the document already + * existed in the target, and is being added in the target, and this is not a reset, we can + * skip doing the work to associate the document with the target because it has already been done. + */ +- (void)filterUpdatesUsingExistingKeys:(FSTDocumentKeySet *)existingKeys; + @end #pragma mark - FSTResetMapping @@ -184,14 +192,6 @@ initWithSnapshotVersion:(firebase::firestore::model::SnapshotVersion)snapshotVer - (void)synthesizeDeleteForLimboTargetChange:(FSTTargetChange *)targetChange key:(const firebase::firestore::model::DocumentKey &)key; -/** - * Strips out mapping changes that aren't actually changes. That is, if the document already - * existed in the target, and is being added in the target, and this is not a reset, we can - * skip doing the work to associate the document with the target because it has already been done. - */ -- (void)filterUpdatesFromTargetChange:(FSTTargetChange *)targetChange - existingDocuments:(FSTDocumentKeySet *)existingDocuments; - @end #pragma mark - FSTWatchChangeAggregator diff --git a/Firestore/Source/Remote/FSTRemoteEvent.mm b/Firestore/Source/Remote/FSTRemoteEvent.mm index 26c32a2..4f7e344 100644 --- a/Firestore/Source/Remote/FSTRemoteEvent.mm +++ b/Firestore/Source/Remote/FSTRemoteEvent.mm @@ -104,6 +104,10 @@ NS_ASSUME_NONNULL_BEGIN self.documents = [self.documents setByRemovingObject:documentKey]; } +- (void)filterUpdatesUsingExistingKeys:(FSTDocumentKeySet *)existingKeys { + // No-op. Resets are not filtered. +} + @end #pragma mark - FSTUpdateMapping @@ -174,6 +178,16 @@ NS_ASSUME_NONNULL_BEGIN self.removedDocuments = [self.removedDocuments setByAddingObject:documentKey]; } +- (void)filterUpdatesUsingExistingKeys:(FSTDocumentKeySet *)existingKeys { + __block FSTDocumentKeySet *result = _addedDocuments; + [_addedDocuments enumerateObjectsUsingBlock:^(FSTDocumentKey *docKey, BOOL *stop) { + if ([existingKeys containsObject:docKey]) { + result = [result setByRemovingObject:docKey]; + } + }]; + _addedDocuments = result; +} + @end #pragma mark - FSTTargetChange @@ -287,21 +301,6 @@ initWithSnapshotVersion:(SnapshotVersion)snapshotVersion return self; } -- (void)filterUpdatesFromTargetChange:(FSTTargetChange *)targetChange - existingDocuments:(FSTDocumentKeySet *)existingDocuments { - if ([targetChange.mapping isKindOfClass:[FSTUpdateMapping class]]) { - FSTUpdateMapping *update = (FSTUpdateMapping *)targetChange.mapping; - FSTDocumentKeySet *added = update.addedDocuments; - __block FSTDocumentKeySet *result = added; - [added enumerateObjectsUsingBlock:^(FSTDocumentKey *docKey, BOOL *stop) { - if ([existingDocuments containsObject:docKey]) { - result = [result setByRemovingObject:docKey]; - } - }]; - update.addedDocuments = result; - } -} - - (void)synthesizeDeleteForLimboTargetChange:(FSTTargetChange *)targetChange key:(const DocumentKey &)key { if (targetChange.currentStatusUpdate == FSTCurrentStatusUpdateMarkCurrent && @@ -320,7 +319,7 @@ initWithSnapshotVersion:(SnapshotVersion)snapshotVersion // However, if the document doesn't exist and the current marker arrives, the document is // not present in the snapshot and our normal view handling would consider the document to // remain in limbo indefinitely because there are no updates to the document. To avoid this, - // we specially handle this just this case here: synthesizing a delete. + // we specially handle this case here: synthesizing a delete. // // TODO(dimond): Ideally we would have an explicit lookup query instead resulting in an // explicit delete message and we could remove this special logic. |