Skip to content

Commit 6a15468

Browse files
dconeybedconeybe
committed
Bundle the readTime and resumeToken into a data structure rather than passing them around individually
1 parent fa0a7c0 commit 6a15468

File tree

6 files changed

+44
-59
lines changed

6 files changed

+44
-59
lines changed

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ describeSpec('Existence Filters:', [], () => {
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

@@ -45,7 +45,7 @@ describeSpec('Existence Filters:', [], () => {
4545
.watchSnapshots(2000)
4646
.expectEvents(query1, {})
4747
.watchSends({ affects: [query1] }, doc1)
48-
.watchFilters([query1], doc1.key)
48+
.watchFilters([query1], [doc1.key])
4949
.watchSnapshots(2000)
5050
.expectEvents(query1, { added: [doc1] });
5151
});
@@ -59,7 +59,7 @@ describeSpec('Existence Filters:', [], () => {
5959
.watchCurrents(query1, 'resume-token-1000')
6060
.watchSnapshots(2000)
6161
.expectEvents(query1, {})
62-
.watchFilters([query1], doc1.key)
62+
.watchFilters([query1], [doc1.key])
6363
.watchSnapshots(2000)
6464
.expectEvents(query1, { fromCache: true });
6565
});
@@ -96,7 +96,7 @@ describeSpec('Existence Filters:', [], () => {
9696
.userListens(query1)
9797
.watchAcksFull(query1, 1000, doc1, doc2)
9898
.expectEvents(query1, { added: [doc1, doc2] })
99-
.watchFilters([query1], doc1.key) // in the next sync doc2 was deleted
99+
.watchFilters([query1], [doc1.key]) // in the next sync doc2 was deleted
100100
.watchSnapshots(2000)
101101
// query is now marked as "inconsistent" because of filter mismatch
102102
.expectEvents(query1, { fromCache: true })
@@ -130,7 +130,7 @@ describeSpec('Existence Filters:', [], () => {
130130
resumeToken: 'existence-filter-resume-token'
131131
})
132132
.watchAcks(query1)
133-
.watchFilters([query1], doc1.key) // in the next sync doc2 was deleted
133+
.watchFilters([query1], [doc1.key]) // in the next sync doc2 was deleted
134134
.watchSnapshots(2000)
135135
// query is now marked as "inconsistent" because of filter mismatch
136136
.expectEvents(query1, { fromCache: true })
@@ -159,7 +159,7 @@ describeSpec('Existence Filters:', [], () => {
159159
// Send a mismatching existence filter with two documents, but don't
160160
// send a new global snapshot. We should not see an event until we
161161
// receive the snapshot.
162-
.watchFilters([query1], doc1.key, doc2.key)
162+
.watchFilters([query1], [doc1.key, doc2.key])
163163
.watchSends({ affects: [query1] }, doc3)
164164
.watchSnapshots(2000)
165165
// The query result includes doc3, but is marked as "inconsistent"
@@ -193,7 +193,7 @@ describeSpec('Existence Filters:', [], () => {
193193
.userListens(query1)
194194
.watchAcksFull(query1, 1000, doc1, doc2)
195195
.expectEvents(query1, { added: [doc1, doc2] })
196-
.watchFilters([query1], doc1.key) // in the next sync doc2 was deleted
196+
.watchFilters([query1], [doc1.key]) // in the next sync doc2 was deleted
197197
.watchSnapshots(2000)
198198
// query is now marked as "inconsistent" because of filter mismatch
199199
.expectEvents(query1, { fromCache: true })
@@ -229,7 +229,7 @@ describeSpec('Existence Filters:', [], () => {
229229
.userListens(query1)
230230
.watchAcksFull(query1, 1000, doc1, doc2)
231231
.expectEvents(query1, { added: [doc1, doc2] })
232-
.watchFilters([query1], doc1.key) // doc2 was deleted
232+
.watchFilters([query1], [doc1.key]) // doc2 was deleted
233233
.watchSnapshots(2000)
234234
.expectEvents(query1, { fromCache: true })
235235
// 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: 29 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,11 @@ export interface ActiveTargetMap {
8181
[targetId: string]: ActiveTargetSpec;
8282
}
8383

84+
export interface ResumeSpec {
85+
resumeToken?: string;
86+
readTime?: TestSnapshotVersion;
87+
}
88+
8489
/**
8590
* Tracks the expected memory state of a client (e.g. the expected active watch
8691
* targets based on userListens(), userUnlistens(), and watchRemoves()
@@ -255,10 +260,7 @@ export class SpecBuilder {
255260
return this;
256261
}
257262

258-
userListens(
259-
query: Query,
260-
resume?: { resumeToken?: string; readTime?: TestSnapshotVersion }
261-
): this {
263+
userListens(query: Query, resume?: ResumeSpec): this {
262264
this.nextStep();
263265

264266
const target = queryToTarget(query);
@@ -277,12 +279,7 @@ export class SpecBuilder {
277279
}
278280

279281
this.queryMapping.set(target, targetId);
280-
this.addQueryToActiveTargets(
281-
targetId,
282-
query,
283-
resume?.resumeToken,
284-
resume?.readTime
285-
);
282+
this.addQueryToActiveTargets(targetId, query, resume);
286283
this.currentStep = {
287284
userListen: { targetId, query: SpecBuilder.queryToSpec(query) },
288285
expectedState: { activeTargets: { ...this.activeTargets } }
@@ -295,14 +292,17 @@ export class SpecBuilder {
295292
* Registers a previously active target with the test expectations after a
296293
* stream disconnect.
297294
*/
298-
restoreListen(query: Query, resumeToken: string): this {
295+
restoreListen(
296+
query: Query,
297+
resumeToken: string
298+
): this {
299299
const targetId = this.queryMapping.get(queryToTarget(query));
300300

301301
if (isNullOrUndefined(targetId)) {
302302
throw new Error("Can't restore an unknown query: " + query);
303303
}
304304

305-
this.addQueryToActiveTargets(targetId!, query, resumeToken);
305+
this.addQueryToActiveTargets(targetId!, query, { resumeToken });
306306

307307
const currentStep = this.currentStep!;
308308
currentStep.expectedState = currentStep.expectedState || {};
@@ -536,12 +536,10 @@ export class SpecBuilder {
536536
const currentStep = this.currentStep!;
537537
this.clientState.activeTargets = {};
538538
targets.forEach(({ query, resumeToken, readTime }) => {
539-
this.addQueryToActiveTargets(
540-
this.getTargetId(query),
541-
query,
539+
this.addQueryToActiveTargets(this.getTargetId(query), query, {
542540
resumeToken,
543541
readTime
544-
);
542+
});
545543
});
546544
currentStep.expectedState = currentStep.expectedState || {};
547545
currentStep.expectedState.activeTargets = { ...this.activeTargets };
@@ -572,7 +570,7 @@ export class SpecBuilder {
572570
this.addQueryToActiveTargets(
573571
this.limboMapping[path],
574572
newQueryForPath(key.path),
575-
''
573+
{ resumeToken: '' }
576574
);
577575
});
578576

@@ -769,18 +767,18 @@ export class SpecBuilder {
769767
return this;
770768
}
771769

772-
watchFilters(queries: Query[], ...docs: DocumentKey[]): this {
770+
watchFilters(
771+
queries: Query[],
772+
docs: DocumentKey[] = []
773+
): this {
773774
this.nextStep();
774775
const targetIds = queries.map(query => {
775776
return this.getTargetId(query);
776777
});
777778
const keys = docs.map(key => {
778779
return key.path.canonicalString();
779780
});
780-
const filter: SpecWatchFilter = [targetIds] as SpecWatchFilter;
781-
for (const key of keys) {
782-
filter.push(key);
783-
}
781+
const filter: SpecWatchFilter = { targetIds, keys } as SpecWatchFilter;
784782
this.currentStep = {
785783
watchFilter: filter
786784
};
@@ -910,22 +908,14 @@ export class SpecBuilder {
910908
}
911909

912910
/** Registers a query that is active in another tab. */
913-
expectListen(
914-
query: Query,
915-
resume?: { resumeToken?: string; readTime?: TestSnapshotVersion }
916-
): this {
911+
expectListen(query: Query, resume?: ResumeSpec): this {
917912
this.assertStep('Expectations require previous step');
918913

919914
const target = queryToTarget(query);
920915
const targetId = this.queryIdGenerator.cachedId(target);
921916
this.queryMapping.set(target, targetId);
922917

923-
this.addQueryToActiveTargets(
924-
targetId,
925-
query,
926-
resume?.resumeToken,
927-
resume?.readTime
928-
);
918+
this.addQueryToActiveTargets(targetId, query, resume);
929919

930920
const currentStep = this.currentStep!;
931921
currentStep.expectedState = currentStep.expectedState || {};
@@ -1093,8 +1083,7 @@ export class SpecBuilder {
10931083
private addQueryToActiveTargets(
10941084
targetId: number,
10951085
query: Query,
1096-
resumeToken?: string,
1097-
readTime?: TestSnapshotVersion
1086+
resume?: ResumeSpec
10981087
): void {
10991088
if (this.activeTargets[targetId]) {
11001089
const activeQueries = this.activeTargets[targetId].queries;
@@ -1106,21 +1095,21 @@ export class SpecBuilder {
11061095
// `query` is not added yet.
11071096
this.activeTargets[targetId] = {
11081097
queries: [SpecBuilder.queryToSpec(query), ...activeQueries],
1109-
resumeToken: resumeToken || '',
1110-
readTime
1098+
resumeToken: resume?.resumeToken || '',
1099+
readTime: resume?.readTime
11111100
};
11121101
} else {
11131102
this.activeTargets[targetId] = {
11141103
queries: activeQueries,
1115-
resumeToken: resumeToken || '',
1116-
readTime
1104+
resumeToken: resume?.resumeToken || '',
1105+
readTime: resume?.readTime
11171106
};
11181107
}
11191108
} else {
11201109
this.activeTargets[targetId] = {
11211110
queries: [SpecBuilder.queryToSpec(query)],
1122-
resumeToken: resumeToken || '',
1123-
readTime
1111+
resumeToken: resume?.resumeToken || '',
1112+
readTime: resume?.readTime
11241113
};
11251114
}
11261115
}

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -694,12 +694,11 @@ abstract class TestRunner {
694694
}
695695

696696
private doWatchFilter(watchFilter: SpecWatchFilter): Promise<void> {
697-
const targetIds: TargetId[] = watchFilter[0];
697+
const { targetIds, keys } = watchFilter;
698698
debugAssert(
699699
targetIds.length === 1,
700700
'ExistenceFilters currently support exactly one target only.'
701701
);
702-
const keys = watchFilter.slice(1);
703702
const filter = new ExistenceFilter(keys.length);
704703
const change = new ExistenceFilterChange(targetIds[0], filter);
705704
return this.doWatchEvent(change);
@@ -1583,14 +1582,11 @@ export interface SpecClientState {
15831582
}
15841583

15851584
/**
1586-
* [[<target-id>, ...], <key>, ...]
1587-
* Note that the last parameter is really of type ...string (spread operator)
15881585
* The filter is based of a list of keys to match in the existence filter
15891586
*/
1590-
export interface SpecWatchFilter
1591-
extends Array<TargetId[] | string | undefined> {
1592-
'0': TargetId[];
1593-
'1': string | undefined;
1587+
export interface SpecWatchFilter {
1588+
targetIds: TargetId[];
1589+
keys: string[];
15941590
}
15951591

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

‎packages/firestore/test/util/spec_test_helpers.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ 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
4949
}
5050
};
5151
}

0 commit comments

Comments
 (0)