From 75978ddf543926b69beeefdaf447527f2e919b0f Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 20 Jun 2023 12:15:53 -0400 Subject: [PATCH 1/2] Remove the no-ios tag from bloom filter spec tests, now that the feature has been merged into the ios sdk in https://github.com/firebase/firebase-ios-sdk/pull/11457 --- .../unit/specs/existence_filter_spec.test.ts | 30 +++++++------------ .../test/unit/specs/limbo_spec.test.ts | 3 +- .../test/unit/specs/listen_spec.test.ts | 9 ++---- 3 files changed, 14 insertions(+), 28 deletions(-) diff --git a/packages/firestore/test/unit/specs/existence_filter_spec.test.ts b/packages/firestore/test/unit/specs/existence_filter_spec.test.ts index 8d20ce58d1d..c58fe3d000a 100644 --- a/packages/firestore/test/unit/specs/existence_filter_spec.test.ts +++ b/packages/firestore/test/unit/specs/existence_filter_spec.test.ts @@ -277,8 +277,7 @@ describeSpec('Existence Filters:', [], () => { */ specTest( 'Full re-query is skipped when bloom filter can identify documents deleted', - // TODO(b/278759251) Remove 'no-ios' once bloom filter is merged. - ['no-ios'], + [], () => { const query1 = query('collection'); const docA = doc('collection/a', 1000, { v: 1 }); @@ -310,8 +309,7 @@ describeSpec('Existence Filters:', [], () => { specTest( 'Full re-query is triggered when bloom filter can not identify documents deleted', - // TODO(b/278759251) Remove 'no-ios' once bloom filter is merged. - ['no-ios'], + [], () => { const query1 = query('collection'); const docA = doc('collection/a', 1000, { v: 1 }); @@ -343,8 +341,7 @@ describeSpec('Existence Filters:', [], () => { specTest( 'Bloom filter can process special characters in document name', - // TODO(b/278759251) Remove 'no-ios' once bloom filter is merged. - ['no-ios'], + [], () => { const query1 = query('collection'); const docA = doc('collection/ÀÒ∑', 1000, { v: 1 }); @@ -372,8 +369,7 @@ describeSpec('Existence Filters:', [], () => { specTest( 'Bloom filter fills in default values for undefined padding and hashCount', - // TODO(b/278759251) Remove 'no-ios' once bloom filter is merged. - ['no-ios'], + [], () => { const query1 = query('collection'); const docA = doc('collection/a', 1000, { v: 1 }); @@ -445,8 +441,7 @@ describeSpec('Existence Filters:', [], () => { specTest( 'Full re-query is triggered when bloom filter hashCount is invalid', - // TODO(b/278759251) Remove 'no-ios' once bloom filter is merged. - ['no-ios'], + [], () => { const query1 = query('collection'); const docA = doc('collection/a', 1000, { v: 1 }); @@ -480,8 +475,7 @@ describeSpec('Existence Filters:', [], () => { specTest( 'Full re-query is triggered when bloom filter is empty', - // TODO(b/278759251) Remove 'no-ios' once bloom filter is merged. - ['no-ios'], + [], () => { const query1 = query('collection'); const docA = doc('collection/a', 1000, { v: 1 }); @@ -516,8 +510,7 @@ describeSpec('Existence Filters:', [], () => { specTest( 'Same documents can have different bloom filters', - // TODO(b/278759251) Remove 'no-ios' once bloom filter is merged. - ['no-ios'], + [], () => { const query1 = query('collection', filter('v', '<=', 2)); const query2 = query('collection', filter('v', '>=', 2)); @@ -567,8 +560,7 @@ describeSpec('Existence Filters:', [], () => { specTest( 'Bloom filter is handled at global snapshot', - // TODO(b/278759251) Remove 'no-ios' once bloom filter is merged. - ['no-ios'], + [], () => { const query1 = query('collection'); const docA = doc('collection/a', 1000, { v: 1 }); @@ -600,8 +592,7 @@ describeSpec('Existence Filters:', [], () => { specTest( 'Bloom filter limbo resolution is denied', - // TODO(b/278759251) Remove 'no-ios' once bloom filter is merged. - ['no-ios'], + [], () => { const query1 = query('collection'); const docA = doc('collection/a', 1000, { v: 1 }); @@ -631,8 +622,7 @@ describeSpec('Existence Filters:', [], () => { specTest( 'Bloom filter with large size works as expected', - // TODO(b/278759251) Remove 'no-ios' once bloom filter is merged. - ['no-ios'], + [], () => { const query1 = query('collection'); const docs = []; diff --git a/packages/firestore/test/unit/specs/limbo_spec.test.ts b/packages/firestore/test/unit/specs/limbo_spec.test.ts index 4da2ec9e659..9e3b3aa3c41 100644 --- a/packages/firestore/test/unit/specs/limbo_spec.test.ts +++ b/packages/firestore/test/unit/specs/limbo_spec.test.ts @@ -929,8 +929,7 @@ describeSpec('Limbo Documents:', [], () => { specTest( 'Limbo resolution throttling with bloom filter application', - // TODO(b/278759251) Remove 'no-ios' once bloom filter is merged. - ['no-ios'], + [], () => { const query1 = query('collection'); const docA1 = doc('collection/a1', 1000, { key: 'a1' }); diff --git a/packages/firestore/test/unit/specs/listen_spec.test.ts b/packages/firestore/test/unit/specs/listen_spec.test.ts index a236c6fcafb..ec98be1092d 100644 --- a/packages/firestore/test/unit/specs/listen_spec.test.ts +++ b/packages/firestore/test/unit/specs/listen_spec.test.ts @@ -1812,8 +1812,7 @@ describeSpec('Listens:', [], () => { specTest( 'Resuming a query should specify expectedCount when adding the target', - // TODO(b/278759251) Remove 'no-ios' once bloom filter is merged. - ['no-ios'], + [], () => { const query1 = query('collection'); const docA = doc('collection/a', 1000, { key: 'a' }); @@ -1847,8 +1846,7 @@ describeSpec('Listens:', [], () => { specTest( 'Resuming a query should specify expectedCount that does not include pending mutations', - // TODO(b/278759251) Remove 'no-ios' once bloom filter is merged. - ['no-ios'], + [], () => { const query1 = query('collection'); const docA = doc('collection/a', 1000, { key: 'a' }); @@ -1877,8 +1875,7 @@ describeSpec('Listens:', [], () => { specTest( 'ExpectedCount in listen request should work after coming back online', - // TODO(b/278759251) Remove 'no-ios' once bloom filter is merged. - ['no-ios'], + [], () => { const query1 = query('collection'); const docA = doc('collection/a', 1000, { key: 'a' }); From b4d81b0447d3008ab409b64e5fe2bab791b7ff46 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 20 Jun 2023 14:24:30 -0400 Subject: [PATCH 2/2] yarn prettier --- .../unit/specs/existence_filter_spec.test.ts | 304 ++++++++---------- 1 file changed, 142 insertions(+), 162 deletions(-) diff --git a/packages/firestore/test/unit/specs/existence_filter_spec.test.ts b/packages/firestore/test/unit/specs/existence_filter_spec.test.ts index c58fe3d000a..9786d156241 100644 --- a/packages/firestore/test/unit/specs/existence_filter_spec.test.ts +++ b/packages/firestore/test/unit/specs/existence_filter_spec.test.ts @@ -473,182 +473,162 @@ describeSpec('Existence Filters:', [], () => { } ); - specTest( - 'Full re-query is triggered when bloom filter is empty', - [], - () => { - const query1 = query('collection'); - const docA = doc('collection/a', 1000, { v: 1 }); - const docB = doc('collection/b', 1000, { v: 1 }); - - //Generate an empty bloom filter. - const bloomFilterProto = generateBloomFilterProto({ - contains: [], - notContains: [], - bitCount: 0, - hashCount: 0 - }); - - return ( - spec() - .userListens(query1) - .watchAcksFull(query1, 1000, docA, docB) - .expectEvents(query1, { added: [docA, docB] }) - // DocB is deleted in the next sync. - .watchFilters([query1], [docA.key], bloomFilterProto) - .watchSnapshots(2000) - // Re-run query is triggered. - .expectEvents(query1, { fromCache: true }) - .expectActiveTargets({ - query: query1, - resumeToken: '', - targetPurpose: TargetPurpose.ExistenceFilterMismatch - }) - ); - } - ); + specTest('Full re-query is triggered when bloom filter is empty', [], () => { + const query1 = query('collection'); + const docA = doc('collection/a', 1000, { v: 1 }); + const docB = doc('collection/b', 1000, { v: 1 }); - specTest( - 'Same documents can have different bloom filters', - [], - () => { - const query1 = query('collection', filter('v', '<=', 2)); - const query2 = query('collection', filter('v', '>=', 2)); + //Generate an empty bloom filter. + const bloomFilterProto = generateBloomFilterProto({ + contains: [], + notContains: [], + bitCount: 0, + hashCount: 0 + }); - const docA = doc('collection/a', 1000, { v: 1 }); - const docB = doc('collection/b', 1000, { v: 2 }); - const docC = doc('collection/c', 1000, { v: 3 }); + return ( + spec() + .userListens(query1) + .watchAcksFull(query1, 1000, docA, docB) + .expectEvents(query1, { added: [docA, docB] }) + // DocB is deleted in the next sync. + .watchFilters([query1], [docA.key], bloomFilterProto) + .watchSnapshots(2000) + // Re-run query is triggered. + .expectEvents(query1, { fromCache: true }) + .expectActiveTargets({ + query: query1, + resumeToken: '', + targetPurpose: TargetPurpose.ExistenceFilterMismatch + }) + ); + }); - const bloomFilterProto1 = generateBloomFilterProto({ - contains: [docB], - notContains: [docA, docC], - bitCount: 5, - hashCount: 2 - }); - const bloomFilterProto2 = generateBloomFilterProto({ - contains: [docB], - notContains: [docA, docC], - bitCount: 4, - hashCount: 1 - }); - return ( - spec() - .userListens(query1) - .watchAcksFull(query1, 1000, docA, docB) - .expectEvents(query1, { added: [docA, docB] }) - .userListens(query2) - .expectEvents(query2, { added: [docB], fromCache: true }) - .watchAcksFull(query2, 1001, docB, docC) - .expectEvents(query2, { added: [docC] }) + specTest('Same documents can have different bloom filters', [], () => { + const query1 = query('collection', filter('v', '<=', 2)); + const query2 = query('collection', filter('v', '>=', 2)); - // DocA is deleted in the next sync for query1. - .watchFilters([query1], [docB.key], bloomFilterProto1) - .watchSnapshots(2000) - // BloomFilter identify docA is deleted, skip full query. - .expectEvents(query1, { fromCache: true }) - .expectLimboDocs(docA.key) // DocA is now in limbo. - - // DocC is deleted in the next sync for query2. - .watchFilters([query2], [docB.key], bloomFilterProto2) - .watchSnapshots(3000) - // BloomFilter identify docC is deleted, skip full query. - .expectEvents(query2, { fromCache: true }) - .expectLimboDocs(docA.key, docC.key) // DocC is now in limbo. - ); - } - ); + const docA = doc('collection/a', 1000, { v: 1 }); + const docB = doc('collection/b', 1000, { v: 2 }); + const docC = doc('collection/c', 1000, { v: 3 }); + + const bloomFilterProto1 = generateBloomFilterProto({ + contains: [docB], + notContains: [docA, docC], + bitCount: 5, + hashCount: 2 + }); + const bloomFilterProto2 = generateBloomFilterProto({ + contains: [docB], + notContains: [docA, docC], + bitCount: 4, + hashCount: 1 + }); + return ( + spec() + .userListens(query1) + .watchAcksFull(query1, 1000, docA, docB) + .expectEvents(query1, { added: [docA, docB] }) + .userListens(query2) + .expectEvents(query2, { added: [docB], fromCache: true }) + .watchAcksFull(query2, 1001, docB, docC) + .expectEvents(query2, { added: [docC] }) - specTest( - 'Bloom filter is handled at global snapshot', - [], - () => { - const query1 = query('collection'); - const docA = doc('collection/a', 1000, { v: 1 }); - const docB = doc('collection/b', 2000, { v: 2 }); - const docC = doc('collection/c', 3000, { v: 3 }); + // DocA is deleted in the next sync for query1. + .watchFilters([query1], [docB.key], bloomFilterProto1) + .watchSnapshots(2000) + // BloomFilter identify docA is deleted, skip full query. + .expectEvents(query1, { fromCache: true }) + .expectLimboDocs(docA.key) // DocA is now in limbo. + + // DocC is deleted in the next sync for query2. + .watchFilters([query2], [docB.key], bloomFilterProto2) + .watchSnapshots(3000) + // BloomFilter identify docC is deleted, skip full query. + .expectEvents(query2, { fromCache: true }) + .expectLimboDocs(docA.key, docC.key) // DocC is now in limbo. + ); + }); - const bloomFilterProto = generateBloomFilterProto({ - contains: [docA], - notContains: [docB] - }); + specTest('Bloom filter is handled at global snapshot', [], () => { + const query1 = query('collection'); + const docA = doc('collection/a', 1000, { v: 1 }); + const docB = doc('collection/b', 2000, { v: 2 }); + const docC = doc('collection/c', 3000, { v: 3 }); - return ( - spec() - .userListens(query1) - .watchAcksFull(query1, 1000, docA, docB) - .expectEvents(query1, { added: [docA, docB] }) - // Send a mismatching existence filter with one document, but don't - // send a new global snapshot. We should not see an event until we - // receive the snapshot. - .watchFilters([query1], [docA.key], bloomFilterProto) - .watchSends({ affects: [query1] }, docC) - .watchSnapshots(2000) - .expectEvents(query1, { added: [docC], fromCache: true }) - // Re-run of the query1 is skipped, docB is in limbo. - .expectLimboDocs(docB.key) - ); - } - ); + const bloomFilterProto = generateBloomFilterProto({ + contains: [docA], + notContains: [docB] + }); - specTest( - 'Bloom filter limbo resolution is denied', - [], - () => { - const query1 = query('collection'); - const docA = doc('collection/a', 1000, { v: 1 }); - const docB = doc('collection/b', 1000, { v: 1 }); - const bloomFilterProto = generateBloomFilterProto({ - contains: [docA], - notContains: [docB] - }); - return spec() + return ( + spec() .userListens(query1) .watchAcksFull(query1, 1000, docA, docB) .expectEvents(query1, { added: [docA, docB] }) + // Send a mismatching existence filter with one document, but don't + // send a new global snapshot. We should not see an event until we + // receive the snapshot. .watchFilters([query1], [docA.key], bloomFilterProto) + .watchSends({ affects: [query1] }, docC) .watchSnapshots(2000) - .expectEvents(query1, { fromCache: true }) - .expectLimboDocs(docB.key) // DocB is now in limbo. - .watchRemoves( - newQueryForPath(docB.key.path), - new RpcError(Code.PERMISSION_DENIED, 'no') - ) - .expectLimboDocs() // DocB is no longer in limbo. - .expectEvents(query1, { - removed: [docB] - }); - } - ); - - specTest( - 'Bloom filter with large size works as expected', - [], - () => { - const query1 = query('collection'); - const docs = []; - for (let i = 0; i < 100; i++) { - docs.push(doc(`collection/doc${i}`, 1000, { v: 1 })); - } - const docKeys = docs.map(item => item.key); + .expectEvents(query1, { added: [docC], fromCache: true }) + // Re-run of the query1 is skipped, docB is in limbo. + .expectLimboDocs(docB.key) + ); + }); - const bloomFilterProto = generateBloomFilterProto({ - contains: docs.slice(0, 50), - notContains: docs.slice(50), - bitCount: 1000, - hashCount: 16 + specTest('Bloom filter limbo resolution is denied', [], () => { + const query1 = query('collection'); + const docA = doc('collection/a', 1000, { v: 1 }); + const docB = doc('collection/b', 1000, { v: 1 }); + const bloomFilterProto = generateBloomFilterProto({ + contains: [docA], + notContains: [docB] + }); + return spec() + .userListens(query1) + .watchAcksFull(query1, 1000, docA, docB) + .expectEvents(query1, { added: [docA, docB] }) + .watchFilters([query1], [docA.key], bloomFilterProto) + .watchSnapshots(2000) + .expectEvents(query1, { fromCache: true }) + .expectLimboDocs(docB.key) // DocB is now in limbo. + .watchRemoves( + newQueryForPath(docB.key.path), + new RpcError(Code.PERMISSION_DENIED, 'no') + ) + .expectLimboDocs() // DocB is no longer in limbo. + .expectEvents(query1, { + removed: [docB] }); - return ( - spec() - .userListens(query1) - .watchAcksFull(query1, 1000, ...docs) - .expectEvents(query1, { added: docs }) - // Doc0 to doc49 are deleted in the next sync. - .watchFilters([query1], docKeys.slice(0, 50), bloomFilterProto) - .watchSnapshots(2000) - // BloomFilter correctly identifies docs that deleted, skip full query. - .expectEvents(query1, { fromCache: true }) - .expectLimboDocs(...docKeys.slice(50)) - ); + }); + + specTest('Bloom filter with large size works as expected', [], () => { + const query1 = query('collection'); + const docs = []; + for (let i = 0; i < 100; i++) { + docs.push(doc(`collection/doc${i}`, 1000, { v: 1 })); } - ); + const docKeys = docs.map(item => item.key); + + const bloomFilterProto = generateBloomFilterProto({ + contains: docs.slice(0, 50), + notContains: docs.slice(50), + bitCount: 1000, + hashCount: 16 + }); + return ( + spec() + .userListens(query1) + .watchAcksFull(query1, 1000, ...docs) + .expectEvents(query1, { added: docs }) + // Doc0 to doc49 are deleted in the next sync. + .watchFilters([query1], docKeys.slice(0, 50), bloomFilterProto) + .watchSnapshots(2000) + // BloomFilter correctly identifies docs that deleted, skip full query. + .expectEvents(query1, { fromCache: true }) + .expectLimboDocs(...docKeys.slice(50)) + ); + }); });