Skip to content

Commit 3e4d6b6

Browse files
milaGGLmilaGGL
committed
Validate target purpose in spec tests
1 parent c4276bd commit 3e4d6b6

File tree

7 files changed

+116
-28
lines changed

7 files changed

+116
-28
lines changed

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

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*/
1717

1818
import { newQueryForPath } from '../../../src/core/query';
19+
import { TargetPurpose } from '../../../src/local/target_data';
1920
import { Code } from '../../../src/util/error';
2021
import { deletedDoc, doc, query } from '../../util/helpers';
2122

@@ -61,7 +62,11 @@ describeSpec('Existence Filters:', [], () => {
6162
.expectEvents(query1, {})
6263
.watchFilters([query1], [doc1.key])
6364
.watchSnapshots(2000)
64-
.expectEvents(query1, { fromCache: true });
65+
.expectEvents(query1, { fromCache: true })
66+
.expectActiveTargets({
67+
query: query1,
68+
targetPurpose: TargetPurpose.ExistenceFilterMismatch
69+
});
6570
});
6671

6772
specTest('Existence filter ignored with pending target', [], () => {
@@ -100,7 +105,11 @@ describeSpec('Existence Filters:', [], () => {
100105
.watchSnapshots(2000)
101106
// query is now marked as "inconsistent" because of filter mismatch
102107
.expectEvents(query1, { fromCache: true })
103-
.expectActiveTargets({ query: query1, resumeToken: '' })
108+
.expectActiveTargets({
109+
query: query1,
110+
targetPurpose: TargetPurpose.ExistenceFilterMismatch,
111+
resumeToken: ''
112+
})
104113
.watchRemoves(query1) // Acks removal of query
105114
.watchAcksFull(query1, 2000, doc1)
106115
.expectLimboDocs(doc2.key) // doc2 is now in limbo
@@ -134,7 +143,11 @@ describeSpec('Existence Filters:', [], () => {
134143
.watchSnapshots(2000)
135144
// query is now marked as "inconsistent" because of filter mismatch
136145
.expectEvents(query1, { fromCache: true })
137-
.expectActiveTargets({ query: query1, resumeToken: '' })
146+
.expectActiveTargets({
147+
query: query1,
148+
targetPurpose: TargetPurpose.ExistenceFilterMismatch,
149+
resumeToken: ''
150+
})
138151
.watchRemoves(query1) // Acks removal of query
139152
.watchAcksFull(query1, 2000, doc1)
140153
.expectLimboDocs(doc2.key) // doc2 is now in limbo
@@ -165,7 +178,11 @@ describeSpec('Existence Filters:', [], () => {
165178
// The query result includes doc3, but is marked as "inconsistent"
166179
// due to the filter mismatch
167180
.expectEvents(query1, { added: [doc3], fromCache: true })
168-
.expectActiveTargets({ query: query1, resumeToken: '' })
181+
.expectActiveTargets({
182+
query: query1,
183+
targetPurpose: TargetPurpose.ExistenceFilterMismatch,
184+
resumeToken: ''
185+
})
169186
.watchRemoves(query1) // Acks removal of query
170187
.watchAcksFull(query1, 3000, doc1, doc2, doc3)
171188
.expectEvents(query1, { added: [doc2] })
@@ -197,7 +214,11 @@ describeSpec('Existence Filters:', [], () => {
197214
.watchSnapshots(2000)
198215
// query is now marked as "inconsistent" because of filter mismatch
199216
.expectEvents(query1, { fromCache: true })
200-
.expectActiveTargets({ query: query1, resumeToken: '' })
217+
.expectActiveTargets({
218+
query: query1,
219+
targetPurpose: TargetPurpose.ExistenceFilterMismatch,
220+
resumeToken: ''
221+
})
201222
.watchRemoves(query1) // Acks removal of query
202223
.watchAcksFull(query1, 2000, doc1)
203224
.expectLimboDocs(doc2.key) // doc2 is now in limbo

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
newQueryForPath,
2121
queryWithLimit
2222
} from '../../../src/core/query';
23+
import { TargetPurpose } from '../../../src/local/target_data';
2324
import { TimerId } from '../../../src/util/async_queue';
2425
import { Code } from '../../../src/util/error';
2526
import { deletedDoc, doc, filter, orderBy, query } from '../../util/helpers';
@@ -889,7 +890,11 @@ describeSpec('Limbo Documents:', [], () => {
889890
// The view now contains the docAs and the docBs (6 documents), but
890891
// the existence filter indicated only 3 should match. This causes
891892
// the client to re-listen without a resume token.
892-
.expectActiveTargets({ query: query1, resumeToken: '' })
893+
.expectActiveTargets({
894+
query: query1,
895+
targetPurpose: TargetPurpose.ExistenceFilterMismatch,
896+
resumeToken: ''
897+
})
893898
// When the existence filter mismatch was detected, the client removed
894899
// then re-added the target. Watch needs to acknowledge the removal.
895900
.watchRemoves(query1)

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*/
1717

1818
import { LimitType, queryWithLimit } from '../../../src/core/query';
19+
import { TargetPurpose } from '../../../src/local/target_data';
1920
import { deletedDoc, doc, filter, orderBy, query } from '../../util/helpers';
2021

2122
import { describeSpec, specTest } from './describe_spec';
@@ -343,7 +344,11 @@ describeSpec('Limits:', [], () => {
343344
.watchSends({ affects: [limitQuery] }, secondDocument)
344345
.watchFilters([limitQuery], [secondDocument.key])
345346
.watchSnapshots(1004)
346-
.expectActiveTargets({ query: limitQuery, resumeToken: '' })
347+
.expectActiveTargets({
348+
query: limitQuery,
349+
targetPurpose: TargetPurpose.ExistenceFilterMismatch,
350+
resumeToken: ''
351+
})
347352
.watchRemoves(limitQuery)
348353
.watchAcksFull(limitQuery, 1005, secondDocument)
349354
// The snapshot after the existence filter mismatch triggers limbo

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

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*/
1717

1818
import { newQueryForPath } from '../../../src/core/query';
19+
import { TargetPurpose } from '../../../src/local/target_data';
1920
import { TimerId } from '../../../src/util/async_queue';
2021
import { Code } from '../../../src/util/error';
2122
import { deletedDoc, doc, filter, query } from '../../util/helpers';
@@ -137,7 +138,10 @@ describeSpec('Persistence Recovery', ['no-ios', 'no-android'], () => {
137138
// The primary client 0 receives a notification that the query can
138139
// be released, but it can only process the change after we recover
139140
// the database.
140-
.expectActiveTargets({ query: query1 })
141+
.expectActiveTargets({
142+
query: query1,
143+
targetPurpose: TargetPurpose.ExistenceFilterMismatch
144+
})
141145
.recoverDatabase()
142146
.runTimer(TimerId.AsyncQueueRetry)
143147
.expectActiveTargets()
@@ -641,7 +645,10 @@ describeSpec('Persistence Recovery', ['no-ios', 'no-android'], () => {
641645
query: filteredQuery,
642646
resumeToken: 'resume-token-2000'
643647
},
644-
{ query: limboQuery }
648+
{
649+
query: limboQuery,
650+
targetPurpose: TargetPurpose.LimboResolution
651+
}
645652
)
646653
.watchAcksFull(filteredQuery, 4000)
647654
.watchAcksFull(limboQuery, 4000, doc1b)
@@ -682,7 +689,7 @@ describeSpec('Persistence Recovery', ['no-ios', 'no-android'], () => {
682689
.runTimer(TimerId.AsyncQueueRetry)
683690
.expectActiveTargets(
684691
{ query: filteredQuery, resumeToken: 'resume-token-2000' },
685-
{ query: limboQuery }
692+
{ query: limboQuery, targetPurpose: TargetPurpose.LimboResolution }
686693
)
687694
.watchAcksFull(filteredQuery, 3000)
688695
.watchRemoves(
@@ -719,15 +726,21 @@ describeSpec('Persistence Recovery', ['no-ios', 'no-android'], () => {
719726
.expectActiveTargets()
720727
.recoverDatabase()
721728
.runTimer(TimerId.AsyncQueueRetry)
722-
.expectActiveTargets({ query: query1 })
729+
.expectActiveTargets({
730+
query: query1,
731+
targetPurpose: TargetPurpose.ExistenceFilterMismatch
732+
})
723733
.expectEvents(query1, { removed: [doc1], fromCache: true })
724734
.failDatabaseTransactions('Handle user change')
725735
.changeUser('user1')
726736
// The network is offline due to the failed user change
727737
.expectActiveTargets()
728738
.recoverDatabase()
729739
.runTimer(TimerId.AsyncQueueRetry)
730-
.expectActiveTargets({ query: query1 })
740+
.expectActiveTargets({
741+
query: query1,
742+
targetPurpose: TargetPurpose.ExistenceFilterMismatch
743+
})
731744
.expectEvents(query1, {
732745
added: [doc1],
733746
fromCache: true,
@@ -763,7 +776,10 @@ describeSpec('Persistence Recovery', ['no-ios', 'no-android'], () => {
763776
.changeUser('user1')
764777
.recoverDatabase()
765778
.runTimer(TimerId.AsyncQueueRetry)
766-
.expectActiveTargets({ query: query1 })
779+
.expectActiveTargets({
780+
query: query1,
781+
targetPurpose: TargetPurpose.ExistenceFilterMismatch
782+
})
767783
// We are now user 2
768784
.expectEvents(query1, { removed: [doc1], fromCache: true })
769785
.runTimer(TimerId.AsyncQueueRetry)

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

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
import { canonifyTarget, Target, targetEquals } from '../../../src/core/target';
2929
import { TargetIdGenerator } from '../../../src/core/target_id_generator';
3030
import { TargetId } from '../../../src/core/types';
31+
import { TargetPurpose } from '../../../src/local/target_data';
3132
import { Document } from '../../../src/model/document';
3233
import { DocumentKey } from '../../../src/model/document_key';
3334
import { FieldIndex } from '../../../src/model/field_index';
@@ -73,6 +74,7 @@ export interface LimboMap {
7374

7475
export interface ActiveTargetSpec {
7576
queries: SpecQuery[];
77+
targetPurpose: TargetPurpose;
7678
resumeToken?: string;
7779
readTime?: TestSnapshotVersion;
7880
}
@@ -279,7 +281,12 @@ export class SpecBuilder {
279281
}
280282

281283
this.queryMapping.set(target, targetId);
282-
this.addQueryToActiveTargets(targetId, query, resume);
284+
this.addQueryToActiveTargets(
285+
targetId,
286+
query,
287+
TargetPurpose.Listen,
288+
resume
289+
);
283290
this.currentStep = {
284291
userListen: { targetId, query: SpecBuilder.queryToSpec(query) },
285292
expectedState: { activeTargets: { ...this.activeTargets } }
@@ -299,7 +306,9 @@ export class SpecBuilder {
299306
throw new Error("Can't restore an unknown query: " + query);
300307
}
301308

302-
this.addQueryToActiveTargets(targetId!, query, { resumeToken });
309+
this.addQueryToActiveTargets(targetId!, query, TargetPurpose.Listen, {
310+
resumeToken
311+
});
303312

304313
const currentStep = this.currentStep!;
305314
currentStep.expectedState = currentStep.expectedState || {};
@@ -525,19 +534,32 @@ export class SpecBuilder {
525534
expectActiveTargets(
526535
...targets: Array<{
527536
query: Query;
537+
targetPurpose?: TargetPurpose;
528538
resumeToken?: string;
529539
readTime?: TestSnapshotVersion;
530540
}>
531541
): this {
532542
this.assertStep('Active target expectation requires previous step');
533543
const currentStep = this.currentStep!;
534544
this.clientState.activeTargets = {};
535-
targets.forEach(({ query, resumeToken, readTime }) => {
536-
this.addQueryToActiveTargets(this.getTargetId(query), query, {
545+
targets.forEach(
546+
({
547+
query,
548+
targetPurpose = TargetPurpose.Listen,
537549
resumeToken,
538550
readTime
539-
});
540-
});
551+
}) => {
552+
this.addQueryToActiveTargets(
553+
this.getTargetId(query),
554+
query,
555+
targetPurpose,
556+
{
557+
resumeToken,
558+
readTime
559+
}
560+
);
561+
}
562+
);
541563
currentStep.expectedState = currentStep.expectedState || {};
542564
currentStep.expectedState.activeTargets = { ...this.activeTargets };
543565
return this;
@@ -567,6 +589,7 @@ export class SpecBuilder {
567589
this.addQueryToActiveTargets(
568590
this.limboMapping[path],
569591
newQueryForPath(key.path),
592+
TargetPurpose.LimboResolution,
570593
{ resumeToken: '' }
571594
);
572595
});
@@ -909,7 +932,7 @@ export class SpecBuilder {
909932
const targetId = this.queryIdGenerator.cachedId(target);
910933
this.queryMapping.set(target, targetId);
911934

912-
this.addQueryToActiveTargets(targetId, query, resume);
935+
this.addQueryToActiveTargets(targetId, query, TargetPurpose.Listen, resume);
913936

914937
const currentStep = this.currentStep!;
915938
currentStep.expectedState = currentStep.expectedState || {};
@@ -1077,6 +1100,7 @@ export class SpecBuilder {
10771100
private addQueryToActiveTargets(
10781101
targetId: number,
10791102
query: Query,
1103+
targetPurpose: TargetPurpose,
10801104
resume?: ResumeSpec
10811105
): void {
10821106
if (this.activeTargets[targetId]) {
@@ -1089,19 +1113,22 @@ export class SpecBuilder {
10891113
// `query` is not added yet.
10901114
this.activeTargets[targetId] = {
10911115
queries: [SpecBuilder.queryToSpec(query), ...activeQueries],
1116+
targetPurpose,
10921117
resumeToken: resume?.resumeToken || '',
10931118
readTime: resume?.readTime
10941119
};
10951120
} else {
10961121
this.activeTargets[targetId] = {
10971122
queries: activeQueries,
1123+
targetPurpose,
10981124
resumeToken: resume?.resumeToken || '',
10991125
readTime: resume?.readTime
11001126
};
11011127
}
11021128
} else {
11031129
this.activeTargets[targetId] = {
11041130
queries: [SpecBuilder.queryToSpec(query)],
1131+
targetPurpose,
11051132
resumeToken: resume?.resumeToken || '',
11061133
readTime: resume?.readTime
11071134
};
@@ -1115,6 +1142,7 @@ export class SpecBuilder {
11151142
if (queriesAfterRemoval.length > 0) {
11161143
this.activeTargets[targetId] = {
11171144
queries: queriesAfterRemoval,
1145+
targetPurpose: this.activeTargets[targetId].targetPurpose,
11181146
resumeToken: this.activeTargets[targetId].resumeToken
11191147
};
11201148
} else {

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ import { Mutation } from '../../../src/model/mutation';
5353
import { encodeBase64 } from '../../../src/platform/base64';
5454
import { newSerializer } from '../../../src/platform/serializer';
5555
import * as api from '../../../src/protos/firestore_proto_api';
56+
import { ApiClientObjectMap } from '../../../src/protos/firestore_proto_api';
5657
import { Connection, Stream } from '../../../src/remote/connection';
5758
import { Datastore, newDatastore } from '../../../src/remote/datastore';
5859
import { WriteRequest } from '../../../src/remote/persistent_stream';
@@ -259,7 +260,12 @@ export class MockConnection implements Connection {
259260
* Tracks the currently active watch targets as detected by the mock watch
260261
* stream, as a mapping from target ID to query Target.
261262
*/
262-
activeTargets: { [targetId: number]: api.Target } = {};
263+
activeTargets: {
264+
[targetId: number]: {
265+
target: api.Target;
266+
labels?: ApiClientObjectMap<string>;
267+
};
268+
} = {};
263269

264270
/** A Deferred that is resolved once watch opens. */
265271
watchOpen = new Deferred<void>();
@@ -398,7 +404,10 @@ export class MockConnection implements Connection {
398404
++this.watchStreamRequestCount;
399405
if (request.addTarget) {
400406
const targetId = request.addTarget.targetId!;
401-
this.activeTargets[targetId] = request.addTarget;
407+
this.activeTargets[targetId] = {
408+
target: request.addTarget,
409+
labels: request.labels
410+
};
402411
} else if (request.removeTarget) {
403412
delete this.activeTargets[request.removeTarget];
404413
} else {

0 commit comments

Comments
 (0)