Skip to content

Commit 3d6d92b

Browse files
committed
add bloom filter to existence filter, and watchFilters spec builder
1 parent c17af51 commit 3d6d92b

File tree

10 files changed

+81
-31
lines changed

10 files changed

+81
-31
lines changed

packages/firestore/src/protos/firestore_proto_api.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,8 @@ export declare type BeginTransactionRequest =
456456
firestoreV1ApiClientInterfaces.BeginTransactionRequest;
457457
export declare type BeginTransactionResponse =
458458
firestoreV1ApiClientInterfaces.BeginTransactionResponse;
459+
export declare type BloomFilter =
460+
firestoreV1ApiClientInterfaces.BloomFilter;
459461
export declare type CollectionSelector =
460462
firestoreV1ApiClientInterfaces.CollectionSelector;
461463
export declare type CommitRequest =

packages/firestore/src/remote/existence_filter.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@
1515
* limitations under the License.
1616
*/
1717

18+
import { BloomFilter as ProtoBloomFilter } from '../protos/firestore_proto_api';
19+
1820
export class ExistenceFilter {
1921
// TODO(b/33078163): just use simplest form of existence filter for now
20-
constructor(public count: number) {}
22+
constructor(public count: number, public unchangedNames?: ProtoBloomFilter) {
23+
}
2124
}

packages/firestore/src/remote/serializer.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
* limitations under the License.
1616
*/
1717

18+
import { resumeTokenForSnapshot } from '../../test/util/helpers';
1819
import { Bound } from '../core/bound';
1920
import { DatabaseId } from '../core/database_info';
2021
import {
@@ -537,8 +538,8 @@ export function fromWatchChange(
537538
assertPresent(change.filter, 'filter');
538539
const filter = change.filter;
539540
assertPresent(filter.targetId, 'filter.targetId');
540-
const count = filter.count || 0;
541-
const existenceFilter = new ExistenceFilter(count);
541+
const { count = 0, unchangedNames } = filter;
542+
const existenceFilter = new ExistenceFilter(count, unchangedNames);
542543
const targetId = filter.targetId;
543544
watchChange = new ExistenceFilterChange(targetId, existenceFilter);
544545
} else {

packages/firestore/test/unit/specs/existence_filter_spec.test.ts

Lines changed: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,32 @@ import { describeSpec, specTest } from './describe_spec';
2323
import { spec } from './spec_builder';
2424
import { RpcError } from './spec_rpc_error';
2525

26-
describeSpec('Existence Filters:', [], () => {
26+
describeSpec('Existence Filters:', ['exclusive'], () => {
2727
specTest('Existence filter match', [], () => {
2828
const query1 = query('collection');
2929
const doc1 = doc('collection/1', 1000, { v: 1 });
3030
return spec()
3131
.userListens(query1)
3232
.watchAcksFull(query1, 1000, doc1)
3333
.expectEvents(query1, { added: [doc1] })
34-
.watchFilters([query1], doc1.key)
34+
.watchFilters([query1], [doc1.key])
3535
.watchSnapshots(2000);
3636
});
3737

38+
// TODO:(mila) update the tests when bloom filter is
39+
specTest('Existence filter with bloom filter match', ['exclusive'], () => {
40+
const query1 = query('collection');
41+
const doc1 = doc('collection/1', 1000, { v: 1 });
42+
return spec()
43+
.userListens(query1)
44+
.watchAcksFull(query1, 1000, doc1)
45+
.expectEvents(query1, { added: [doc1] })
46+
.watchFilters([query1], [doc1.key],{bits:{bitmap:"a",padding:1}, hashCount:1})
47+
.watchSnapshots(2000)
48+
.watchFilters([query1])
49+
.watchSnapshots(3000);
50+
});
51+
3852
specTest('Existence filter match after pending update', [], () => {
3953
const query1 = query('collection');
4054
const doc1 = doc('collection/1', 2000, { v: 2 });
@@ -45,7 +59,7 @@ describeSpec('Existence Filters:', [], () => {
4559
.watchSnapshots(2000)
4660
.expectEvents(query1, {})
4761
.watchSends({ affects: [query1] }, doc1)
48-
.watchFilters([query1], doc1.key)
62+
.watchFilters([query1], [doc1.key])
4963
.watchSnapshots(2000)
5064
.expectEvents(query1, { added: [doc1] });
5165
});
@@ -59,7 +73,7 @@ describeSpec('Existence Filters:', [], () => {
5973
.watchCurrents(query1, 'resume-token-1000')
6074
.watchSnapshots(2000)
6175
.expectEvents(query1, {})
62-
.watchFilters([query1], doc1.key)
76+
.watchFilters([query1], [doc1.key])
6377
.watchSnapshots(2000)
6478
.expectEvents(query1, { fromCache: true });
6579
});
@@ -96,7 +110,33 @@ describeSpec('Existence Filters:', [], () => {
96110
.userListens(query1)
97111
.watchAcksFull(query1, 1000, doc1, doc2)
98112
.expectEvents(query1, { added: [doc1, doc2] })
99-
.watchFilters([query1], doc1.key) // in the next sync doc2 was deleted
113+
.watchFilters([query1], [doc1.key]) // in the next sync doc2 was deleted
114+
.watchSnapshots(2000)
115+
// query is now marked as "inconsistent" because of filter mismatch
116+
.expectEvents(query1, { fromCache: true })
117+
.expectActiveTargets({ query: query1, resumeToken: '' })
118+
.watchRemoves(query1) // Acks removal of query
119+
.watchAcksFull(query1, 2000, doc1)
120+
.expectLimboDocs(doc2.key) // doc2 is now in limbo
121+
.ackLimbo(2000, deletedDoc('collection/2', 2000))
122+
.expectLimboDocs() // doc2 is no longer in limbo
123+
.expectEvents(query1, {
124+
removed: [doc2]
125+
})
126+
);
127+
});
128+
129+
// todo
130+
specTest('Existence filter mismatch triggers bloom filter', ['exclusive'], () => {
131+
const query1 = query('collection');
132+
const doc1 = doc('collection/1', 1000, { v: 1 });
133+
const doc2 = doc('collection/2', 1000, { v: 2 });
134+
return (
135+
spec()
136+
.userListens(query1)
137+
.watchAcksFull(query1, 1000, doc1, doc2)
138+
.expectEvents(query1, { added: [doc1, doc2] })
139+
.watchFilters([query1], [doc1.key],{bits:{bitmap:"a",padding:1},hashCount:3}) // in the next sync doc2 was deleted
100140
.watchSnapshots(2000)
101141
// query is now marked as "inconsistent" because of filter mismatch
102142
.expectEvents(query1, { fromCache: true })
@@ -130,7 +170,7 @@ describeSpec('Existence Filters:', [], () => {
130170
resumeToken: 'existence-filter-resume-token'
131171
})
132172
.watchAcks(query1)
133-
.watchFilters([query1], doc1.key) // in the next sync doc2 was deleted
173+
.watchFilters([query1], [doc1.key]) // in the next sync doc2 was deleted
134174
.watchSnapshots(2000)
135175
// query is now marked as "inconsistent" because of filter mismatch
136176
.expectEvents(query1, { fromCache: true })
@@ -159,7 +199,7 @@ describeSpec('Existence Filters:', [], () => {
159199
// Send a mismatching existence filter with two documents, but don't
160200
// send a new global snapshot. We should not see an event until we
161201
// receive the snapshot.
162-
.watchFilters([query1], doc1.key, doc2.key)
202+
.watchFilters([query1], [doc1.key, doc2.key])
163203
.watchSends({ affects: [query1] }, doc3)
164204
.watchSnapshots(2000)
165205
// The query result includes doc3, but is marked as "inconsistent"
@@ -193,7 +233,7 @@ describeSpec('Existence Filters:', [], () => {
193233
.userListens(query1)
194234
.watchAcksFull(query1, 1000, doc1, doc2)
195235
.expectEvents(query1, { added: [doc1, doc2] })
196-
.watchFilters([query1], doc1.key) // in the next sync doc2 was deleted
236+
.watchFilters([query1], [doc1.key]) // in the next sync doc2 was deleted
197237
.watchSnapshots(2000)
198238
// query is now marked as "inconsistent" because of filter mismatch
199239
.expectEvents(query1, { fromCache: true })
@@ -229,7 +269,7 @@ describeSpec('Existence Filters:', [], () => {
229269
.userListens(query1)
230270
.watchAcksFull(query1, 1000, doc1, doc2)
231271
.expectEvents(query1, { added: [doc1, doc2] })
232-
.watchFilters([query1], doc1.key) // doc2 was deleted
272+
.watchFilters([query1], [doc1.key]) // doc2 was deleted
233273
.watchSnapshots(2000)
234274
.expectEvents(query1, { fromCache: true })
235275
// The SDK is unable to re-run the query, and does not remove doc2

packages/firestore/test/unit/specs/limbo_spec.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -880,7 +880,7 @@ describeSpec('Limbo Documents:', [], () => {
880880
// documents that changed since the resume token. This will cause it
881881
// to just send the docBs with an existence filter with a count of 3.
882882
.watchSends({ affects: [query1] }, docB1, docB2, docB3)
883-
.watchFilters([query1], docB1.key, docB2.key, docB3.key)
883+
.watchFilters([query1], [docB1.key, docB2.key, docB3.key])
884884
.watchSnapshots(1001)
885885
.expectEvents(query1, {
886886
added: [docB1, docB2, docB3],

packages/firestore/test/unit/specs/limit_spec.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ describeSpec('Limits:', [], () => {
341341
// we receive an existence filter, which indicates that our view is
342342
// out of sync.
343343
.watchSends({ affects: [limitQuery] }, secondDocument)
344-
.watchFilters([limitQuery], secondDocument.key)
344+
.watchFilters([limitQuery], [secondDocument.key])
345345
.watchSnapshots(1004)
346346
.expectActiveTargets({ query: limitQuery, resumeToken: '' })
347347
.watchRemoves(limitQuery)

packages/firestore/test/unit/specs/spec_builder.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import { DocumentKey } from '../../../src/model/document_key';
3333
import { FieldIndex } from '../../../src/model/field_index';
3434
import { JsonObject } from '../../../src/model/object_value';
3535
import { ResourcePath } from '../../../src/model/path';
36+
import { BloomFilter as ProtoBloomFilter} from '../../../src/protos/firestore_proto_api';
3637
import {
3738
isPermanentWriteError,
3839
mapCodeFromRpcCode,
@@ -769,18 +770,19 @@ export class SpecBuilder {
769770
return this;
770771
}
771772

772-
watchFilters(queries: Query[], ...docs: DocumentKey[]): this {
773+
watchFilters(
774+
queries: Query[],
775+
docs: DocumentKey[] = [],
776+
bloomFilter?: ProtoBloomFilter
777+
): this {
773778
this.nextStep();
774779
const targetIds = queries.map(query => {
775780
return this.getTargetId(query);
776781
});
777782
const keys = docs.map(key => {
778783
return key.path.canonicalString();
779784
});
780-
const filter: SpecWatchFilter = [targetIds] as SpecWatchFilter;
781-
for (const key of keys) {
782-
filter.push(key);
783-
}
785+
const filter = { targetIds, keys, bloomFilter } as SpecWatchFilter;
784786
this.currentStep = {
785787
watchFilter: filter
786788
};

packages/firestore/test/unit/specs/spec_test_runner.ts

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -693,13 +693,12 @@ abstract class TestRunner {
693693
}
694694

695695
private doWatchFilter(watchFilter: SpecWatchFilter): Promise<void> {
696-
const targetIds: TargetId[] = watchFilter[0];
696+
const { targetIds, keys, bloomFilter } = watchFilter;
697697
debugAssert(
698698
targetIds.length === 1,
699699
'ExistenceFilters currently support exactly one target only.'
700700
);
701-
const keys = watchFilter.slice(1);
702-
const filter = new ExistenceFilter(keys.length);
701+
const filter = new ExistenceFilter(keys.length, bloomFilter);
703702
const change = new ExistenceFilterChange(targetIds[0], filter);
704703
return this.doWatchEvent(change);
705704
}
@@ -1577,14 +1576,12 @@ export interface SpecClientState {
15771576
}
15781577

15791578
/**
1580-
* [[<target-id>, ...], <key>, ...]
1581-
* Note that the last parameter is really of type ...string (spread operator)
15821579
* The filter is based of a list of keys to match in the existence filter
15831580
*/
1584-
export interface SpecWatchFilter
1585-
extends Array<TargetId[] | string | undefined> {
1586-
'0': TargetId[];
1587-
'1': string | undefined;
1581+
export interface SpecWatchFilter {
1582+
targetIds: TargetId[];
1583+
keys: string[];
1584+
bloomFilter?: api.BloomFilter;
15881585
}
15891586

15901587
export type SpecLimitType = 'LimitToFirst' | 'LimitToLast';

packages/firestore/test/util/helpers.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -429,15 +429,19 @@ export function existenceFilterEvent(
429429
targetId: number,
430430
syncedKeys: DocumentKeySet,
431431
remoteCount: number,
432-
snapshotVersion: number
432+
snapshotVersion: number,
433+
bloomFilter?: api.BloomFilter
433434
): RemoteEvent {
434435
const aggregator = new WatchChangeAggregator({
435436
getRemoteKeysForTarget: () => syncedKeys,
436437
getTargetDataForTarget: targetId =>
437438
targetData(targetId, TargetPurpose.Listen, 'foo')
438439
});
439440
aggregator.handleExistenceFilter(
440-
new ExistenceFilterChange(targetId, new ExistenceFilter(remoteCount))
441+
new ExistenceFilterChange(
442+
targetId,
443+
new ExistenceFilter(remoteCount, bloomFilter)
444+
)
441445
);
442446
return aggregator.createRemoteEvent(version(snapshotVersion));
443447
}

packages/firestore/test/util/spec_test_helpers.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,9 @@ export function encodeWatchChange(
4444
if (watchChange instanceof ExistenceFilterChange) {
4545
return {
4646
filter: {
47+
targetId: watchChange.targetId,
4748
count: watchChange.existenceFilter.count,
48-
targetId: watchChange.targetId
49+
unchangedNames:watchChange.existenceFilter.unchangedNames
4950
}
5051
};
5152
}

0 commit comments

Comments
 (0)