diff --git a/packages/firestore/src/core/view.ts b/packages/firestore/src/core/view.ts index 225db0ee139..8da76d74faa 100644 --- a/packages/firestore/src/core/view.ts +++ b/packages/firestore/src/core/view.ts @@ -244,7 +244,8 @@ export class View { ); }); - const limboChanges = this.applyTargetChange(targetChange); + this.applyTargetChange(targetChange); + const limboChanges = this.updateLimboDocuments(); const synced = this.limboDocuments.size === 0 && this.current; const newSyncState = synced ? SyncState.Synced : SyncState.Local; const syncStateChanged = newSyncState !== this.syncState; @@ -320,9 +321,7 @@ export class View { * Updates syncedDocuments, current, and limbo docs based on the given change. * Returns the list of changes to which docs are in limbo. */ - private applyTargetChange( - targetChange?: TargetChange - ): LimboDocumentChange[] { + private applyTargetChange(targetChange?: TargetChange): void { if (targetChange) { const targetMapping = targetChange.mapping; if (targetMapping instanceof ResetMapping) { @@ -348,19 +347,23 @@ export class View { ); } } + } + + private updateLimboDocuments(): LimboDocumentChange[] { + // We can only determine limbo documents when we're in-sync with the server. + if (!this.current) { + return []; + } - // Recompute the set of limbo docs. // TODO(klimt): Do this incrementally so that it's not quadratic when // updating many documents. const oldLimboDocuments = this.limboDocuments; this.limboDocuments = documentKeySet(); - if (this.current) { - this.documentSet.forEach(doc => { - if (this.shouldBeInLimbo(doc.key)) { - this.limboDocuments = this.limboDocuments.add(doc.key); - } - }); - } + this.documentSet.forEach(doc => { + if (this.shouldBeInLimbo(doc.key)) { + this.limboDocuments = this.limboDocuments.add(doc.key); + } + }); // Diff the new limbo docs with the old limbo docs. const changes: LimboDocumentChange[] = []; diff --git a/packages/firestore/test/unit/specs/offline_spec.test.ts b/packages/firestore/test/unit/specs/offline_spec.test.ts index 00e34bb1f4f..d832400e034 100644 --- a/packages/firestore/test/unit/specs/offline_spec.test.ts +++ b/packages/firestore/test/unit/specs/offline_spec.test.ts @@ -115,4 +115,35 @@ describeSpec('Offline:', [], () => { .expectEvents(query, { fromCache: false }) ); }); + + specTest('Queries with limbo documents handle going offline.', [], () => { + const query = Query.atPath(path('collection')); + const docA = doc('collection/a', 1000, { key: 'a' }); + const docB = doc('collection/b', 1005, { key: 'b' }); + const limboQuery = Query.atPath(docA.key.path); + return ( + spec() + .userListens(query) + .watchAcksFull(query, 1000, docA) + .expectEvents(query, { added: [docA] }) + .watchResets(query) + // No more documents + .watchCurrents(query, 'resume-token-1001') + .watchSnapshots(1001) + // docA will now be in limbo (triggering fromCache=true) + .expectLimboDocs(docA.key) + .expectEvents(query, { fromCache: true }) + // first error triggers unknown state + .watchStreamCloses(Code.UNAVAILABLE) + .restoreListen(query, 'resume-token-1001') + // getting two more errors triggers offline state. + .watchStreamCloses(Code.UNAVAILABLE) + .watchStreamCloses(Code.UNAVAILABLE) + .watchAcksFull(query, 1001) + .watchAcksFull(limboQuery, 1001) + // Limbo document is resolved. No longer from cache. + .expectEvents(query, { removed: [docA], fromCache: false }) + .expectLimboDocs() + ); + }); });