From 5ce004167600311ab9d46429c538fcf5660d4b39 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Wed, 26 Jun 2024 14:13:48 -0400 Subject: [PATCH 1/6] Update sync_engine_impl.ts --- packages/firestore/src/core/sync_engine_impl.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/firestore/src/core/sync_engine_impl.ts b/packages/firestore/src/core/sync_engine_impl.ts index bba07f4f4bc..fae8d6f2565 100644 --- a/packages/firestore/src/core/sync_engine_impl.ts +++ b/packages/firestore/src/core/sync_engine_impl.ts @@ -1095,7 +1095,13 @@ export async function syncEngineEmitNewSnapsAndNotifyLocalStore( // secondary clients to update query state. if (viewSnapshot || remoteEvent) { if (syncEngineImpl.isPrimaryClient) { - const isCurrent = viewSnapshot && !viewSnapshot.fromCache; + // Query state is set to `current` if: + // - The is a view change and it is up-to-date, or, + // - There are no changes and the Target is current + const isCurrent = viewSnapshot + ? !viewSnapshot.fromCache + : remoteEvent?.targetChanges.get(queryView.targetId)?.current; + syncEngineImpl.sharedClientState.updateQueryState( queryView.targetId, isCurrent ? 'current' : 'not-current' From 15ec65f70eb1769564c992547ceb1cda3e15a491 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 27 Jun 2024 13:04:14 -0400 Subject: [PATCH 2/6] add spec test --- .../firestore/src/core/sync_engine_impl.ts | 2 +- .../test/unit/specs/listen_spec.test.ts | 28 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/packages/firestore/src/core/sync_engine_impl.ts b/packages/firestore/src/core/sync_engine_impl.ts index fae8d6f2565..ed5284414ce 100644 --- a/packages/firestore/src/core/sync_engine_impl.ts +++ b/packages/firestore/src/core/sync_engine_impl.ts @@ -1097,7 +1097,7 @@ export async function syncEngineEmitNewSnapsAndNotifyLocalStore( if (syncEngineImpl.isPrimaryClient) { // Query state is set to `current` if: // - The is a view change and it is up-to-date, or, - // - There are no changes and the Target is current + // - We received a global snapshot and the Target is current const isCurrent = viewSnapshot ? !viewSnapshot.fromCache : remoteEvent?.targetChanges.get(queryView.targetId)?.current; diff --git a/packages/firestore/test/unit/specs/listen_spec.test.ts b/packages/firestore/test/unit/specs/listen_spec.test.ts index df006fb33ff..ef28ac31c16 100644 --- a/packages/firestore/test/unit/specs/listen_spec.test.ts +++ b/packages/firestore/test/unit/specs/listen_spec.test.ts @@ -1895,4 +1895,32 @@ describeSpec('Listens:', [], () => { .restoreListen(query1, 'resume-token-1000', /* expectedCount= */ 1); } ); + + specTest( + 'Global snapshots would not alter query state if there is no changes', + ['multi-client'], + () => { + const query1 = query('collection'); + const docA = doc('collection/a', 1000, { key: 'a' }); + return ( + client(0) + .becomeVisible() + // Listen to the first query in the primary client + .expectPrimaryState(true) + .userListens(query1) + .watchAcksFull(query1, 1000, docA) + .expectEvents(query1, { added: [docA] }) + // Reproduces: https://github.com/firebase/firebase-js-sdk/issues/8314 + // Watch could send existence filters along with a global snapshot. + // If existence filter matches, there will be no view changes, and query should not be + // marked as "not-current" as the Target is up to date. + .watchFilters([query1], [docA.key]) + .watchSnapshots(2000, [], 'resume-token-2000') + // Listening to the query in the secondary tab. The snapshot is up to date. + .client(1) + .userListens(query1) + .expectEvents(query1, { added: [docA] }) + ); + } + ); }); From 739a0d778f10a931a221b2ebaf94b9487db11dd7 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 27 Jun 2024 13:12:36 -0400 Subject: [PATCH 3/6] update comment --- packages/firestore/src/core/sync_engine_impl.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/firestore/src/core/sync_engine_impl.ts b/packages/firestore/src/core/sync_engine_impl.ts index ed5284414ce..1f93db4f8bc 100644 --- a/packages/firestore/src/core/sync_engine_impl.ts +++ b/packages/firestore/src/core/sync_engine_impl.ts @@ -1096,8 +1096,8 @@ export async function syncEngineEmitNewSnapsAndNotifyLocalStore( if (viewSnapshot || remoteEvent) { if (syncEngineImpl.isPrimaryClient) { // Query state is set to `current` if: - // - The is a view change and it is up-to-date, or, - // - We received a global snapshot and the Target is current + // - There is a view change and it is up-to-date, or, + // - There is a global snapshot, the Target is current, and no changes to be resolved const isCurrent = viewSnapshot ? !viewSnapshot.fromCache : remoteEvent?.targetChanges.get(queryView.targetId)?.current; From f1f47a7d3607032b5be68d1a6343d9403e2a4543 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 27 Jun 2024 13:13:44 -0400 Subject: [PATCH 4/6] typo --- packages/firestore/test/unit/specs/listen_spec.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/test/unit/specs/listen_spec.test.ts b/packages/firestore/test/unit/specs/listen_spec.test.ts index ef28ac31c16..aa335a61302 100644 --- a/packages/firestore/test/unit/specs/listen_spec.test.ts +++ b/packages/firestore/test/unit/specs/listen_spec.test.ts @@ -1916,7 +1916,7 @@ describeSpec('Listens:', [], () => { // marked as "not-current" as the Target is up to date. .watchFilters([query1], [docA.key]) .watchSnapshots(2000, [], 'resume-token-2000') - // Listening to the query in the secondary tab. The snapshot is up to date. + // Listen to the query in the secondary tab. The snapshot is up to date. .client(1) .userListens(query1) .expectEvents(query1, { added: [docA] }) From be89e6ef933ff7ddfda05c15b1d41317bbc815fa Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 27 Jun 2024 15:39:11 -0400 Subject: [PATCH 5/6] update spec test --- .../test/unit/specs/listen_spec.test.ts | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/packages/firestore/test/unit/specs/listen_spec.test.ts b/packages/firestore/test/unit/specs/listen_spec.test.ts index aa335a61302..3404c4b4472 100644 --- a/packages/firestore/test/unit/specs/listen_spec.test.ts +++ b/packages/firestore/test/unit/specs/listen_spec.test.ts @@ -1905,21 +1905,29 @@ describeSpec('Listens:', [], () => { return ( client(0) .becomeVisible() - // Listen to the first query in the primary client .expectPrimaryState(true) + // Populate the cache first .userListens(query1) .watchAcksFull(query1, 1000, docA) .expectEvents(query1, { added: [docA] }) + .userUnlistens(query1) + .watchRemoves(query1) + // Listen to the query in the primary client + .userListens(query1, { resumeToken: 'resume-token-1000' }) + .expectEvents(query1, { + added: [docA], + fromCache: true + }) + .watchAcksFull(query1, 2000, docA) + .expectEvents(query1, { fromCache: false }) // Reproduces: https://github.com/firebase/firebase-js-sdk/issues/8314 - // Watch could send existence filters along with a global snapshot. - // If existence filter matches, there will be no view changes, and query should not be - // marked as "not-current" as the Target is up to date. - .watchFilters([query1], [docA.key]) - .watchSnapshots(2000, [], 'resume-token-2000') + // Watch could send a global snapshot from time to time. If there are no view changes, + // the query should not be marked as "not-current" as the Target is up to date. + .watchSnapshots(3000, [], 'resume-token-3000') // Listen to the query in the secondary tab. The snapshot is up to date. .client(1) .userListens(query1) - .expectEvents(query1, { added: [docA] }) + .expectEvents(query1, { added: [docA], fromCache: false }) ); } ); From 8ed81a9c407c350d4db5f9bfb3aa863e9042bef9 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 28 Jun 2024 12:13:00 -0400 Subject: [PATCH 6/6] add changeset --- .changeset/flat-scissors-suffer.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/flat-scissors-suffer.md diff --git a/.changeset/flat-scissors-suffer.md b/.changeset/flat-scissors-suffer.md new file mode 100644 index 00000000000..65334be9720 --- /dev/null +++ b/.changeset/flat-scissors-suffer.md @@ -0,0 +1,6 @@ +--- +'@firebase/firestore': patch +'firebase': patch +--- + +Fix persistence multi-tab snapshot listener metadata sync issue.