Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add expectedCount to Target in listen request #6854

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/firestore/src/local/target_data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export class TargetData {
* The number of documents that last matched the query at the resume token or
* read time.
dconeybe marked this conversation as resolved.
Show resolved Hide resolved
*/
readonly expectedCount: number = 0
readonly expectedCount: number | null = null
) {}

/** Creates a new target data instance with an updated sequence number. */
Expand Down Expand Up @@ -104,7 +104,7 @@ export class TargetData {
snapshotVersion,
this.lastLimboFreeSnapshotVersion,
resumeToken,
dconeybe marked this conversation as resolved.
Show resolved Hide resolved
this.expectedCount
/** expectedCount= */ null
dconeybe marked this conversation as resolved.
Show resolved Hide resolved
);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/firestore/src/remote/serializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1012,7 +1012,7 @@ export function toTarget(

if (targetData.resumeToken.approximateByteSize() > 0) {
result.resumeToken = toBytes(serializer, targetData.resumeToken);
result.expectedCount = targetData.expectedCount;
result.expectedCount = targetData.expectedCount ?? undefined;
} else if (targetData.snapshotVersion.compareTo(SnapshotVersion.min()) > 0) {
// TODO(wuandy): Consider removing above check because it is most likely true.
// Right now, many tests depend on this behaviour though (leaving min() out
Expand All @@ -1021,7 +1021,7 @@ export function toTarget(
serializer,
targetData.snapshotVersion.toTimestamp()
);
result.expectedCount = targetData.expectedCount;
result.expectedCount = targetData.expectedCount ?? undefined;
}

return result;
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/test/unit/remote/serializer.helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1779,7 +1779,7 @@ export function serializerTest(
},
resumeToken: new Uint8Array([1, 2, 3]),
targetId: 1,
expectedCount: 0
expectedCount: undefined
};
expect(result).to.deep.equal(expected);
});
Expand Down
14 changes: 3 additions & 11 deletions packages/firestore/test/unit/specs/listen_spec.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1811,7 +1811,7 @@ describeSpec('Listens:', [], () => {
);

specTest(
'Query with resume target can pass in expectedCount to listen request',
'Resuming a query should specify expectedCount when adding the target',
[],
() => {
const query1 = query('collection');
Expand Down Expand Up @@ -1845,7 +1845,7 @@ describeSpec('Listens:', [], () => {
);

specTest(
'ExpectedCount should equal to the number of documents that last matched the query at the resume token',
'Resuming a query should specify expectedCount that does not include pending mutations',
[],
() => {
const query1 = query('collection');
Expand Down Expand Up @@ -1879,22 +1879,14 @@ describeSpec('Listens:', [], () => {
() => {
const query1 = query('collection');
const docA = doc('collection/a', 1000, { key: 'a' });
const docBLocal = doc('collection/b', 1000, {
key: 'b'
}).setHasLocalMutations();

return spec()
.withGCEnabled(false)
.userListens(query1)
.watchAcksFull(query1, 1000, docA)
.expectEvents(query1, { added: [docA] })
.userSets('collection/b', { key: 'b' })
.expectEvents(query1, {
hasPendingWrites: true,
added: [docBLocal]
})
.disableNetwork()
.expectEvents(query1, { hasPendingWrites: true, fromCache: true })
.expectEvents(query1, { fromCache: true })
.enableNetwork()
.restoreListen(query1, 'resume-token-1000', 1);
dconeybe marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down