Skip to content

Commit 57a1c63

Browse files
wu-huiwu-hui
committed
Spec tests comments
1 parent 2f639ae commit 57a1c63

File tree

5 files changed

+69
-52
lines changed

5 files changed

+69
-52
lines changed

‎packages/firestore/src/core/sync_engine.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -848,8 +848,7 @@ class SyncEngineImpl implements SyncEngine {
848848

849849
async emitNewSnapsAndNotifyLocalStore(
850850
changes: MaybeDocumentMap,
851-
remoteEvent?: RemoteEvent,
852-
fromBundle: boolean = false
851+
remoteEvent?: RemoteEvent
853852
): Promise<void> {
854853
const newSnaps: ViewSnapshot[] = [];
855854
const docChangesInAllViews: LocalViewChanges[] = [];
@@ -881,8 +880,7 @@ class SyncEngineImpl implements SyncEngine {
881880
const viewChange = queryView.view.applyChanges(
882881
viewDocChanges,
883882
/* updateLimboDocuments= */ this.isPrimaryClient,
884-
targetChange,
885-
fromBundle
883+
targetChange
886884
);
887885
this.updateTrackedLimbos(
888886
queryView.targetId,
@@ -1435,10 +1433,12 @@ async function loadBundleImpl(
14351433

14361434
const result = await loader.complete();
14371435
if (result.changedDocs) {
1436+
// TODO(b/160876443): This currently raises snapshots with
1437+
// `fromCache=false` if users already listen to some queries and bundles
1438+
// has newer version.
14381439
await syncEngine.emitNewSnapsAndNotifyLocalStore(
14391440
result.changedDocs,
1440-
/* remoteEvent */ undefined,
1441-
/* fromBundle */ true
1441+
/* remoteEvent */ undefined
14421442
);
14431443
}
14441444

‎packages/firestore/src/core/view.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -280,15 +280,13 @@ export class View {
280280
* change.
281281
* @param targetChange A target change to apply for computing limbo docs and
282282
* sync state.
283-
* @param fromBundle Whether the changes are from applying a bundle file.
284283
* @return A new ViewChange with the given docs, changes, and sync state.
285284
*/
286285
// PORTING NOTE: The iOS/Android clients always compute limbo document changes.
287286
applyChanges(
288287
docChanges: ViewDocumentChanges,
289288
updateLimboDocuments: boolean,
290-
targetChange?: TargetChange,
291-
fromBundle: boolean = false
289+
targetChange?: TargetChange
292290
): ViewChange {
293291
debugAssert(
294292
!docChanges.needsRefill,
@@ -325,7 +323,7 @@ export class View {
325323
oldDocs,
326324
changes,
327325
docChanges.mutatedKeys,
328-
newSyncState === SyncState.Local || fromBundle,
326+
newSyncState === SyncState.Local,
329327
syncStateChanged,
330328
/* excludesMetadataChanges= */ false
331329
);

‎packages/firestore/src/util/byte_stream.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@
1818
import { debugAssert } from './assert';
1919

2020
/**
21-
* For the byte streams where we have control (like backed by a UInt8Array),
22-
* how many bytes to read each time when `ReadableStreamReader.read()` is
23-
* called.
21+
* How many bytes to read each time when `ReadableStreamReader.read()` is
22+
* called. Only applicable for byte streams that we control (e.g. those backed
23+
* by an UInt8Array).
2424
*/
2525
export const DEFAULT_BYTES_PER_READ = 10240;
2626

‎packages/firestore/test/integration/api_internal/bundle.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ apiDescribe('Bundles', (persistence: boolean) => {
216216
let builder = bundleWithTestDocs(db);
217217
return withAlternateTestDb(persistence, async otherDb => {
218218
// eslint-disable-next-line @typescript-eslint/no-floating-promises
219-
expect(
219+
await expect(
220220
otherDb.loadBundle(
221221
builder.build('test-bundle', { seconds: 1001, nanos: 9999 })
222222
)

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

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

1818
import { Query } from '../../../src/core/query';
19-
import { doc, path, TestSnapshotVersion, version } from '../../util/helpers';
19+
import {
20+
doc,
21+
path,
22+
TestSnapshotVersion,
23+
version,
24+
wrapObject
25+
} from '../../util/helpers';
2026

2127
import { describeSpec, specTest } from './describe_spec';
2228
import { client, spec } from './spec_builder';
@@ -26,17 +32,17 @@ import {
2632
TEST_DATABASE_ID
2733
} from '../local/persistence_test_helpers';
2834
import { DocumentKey } from '../../../src/model/document_key';
29-
import * as api from '../../../src/protos/firestore_proto_api';
30-
import { Value } from '../../../src/protos/firestore_proto_api';
3135
import { toVersion } from '../../../src/remote/serializer';
36+
import { JsonObject } from '../../../src/model/object_value';
3237

3338
interface TestBundleDocument {
3439
key: DocumentKey;
3540
readTime: TestSnapshotVersion;
3641
createTime?: TestSnapshotVersion;
3742
updateTime?: TestSnapshotVersion;
38-
content?: api.ApiClientObjectMap<Value>;
43+
content?: JsonObject<unknown>;
3944
}
45+
4046
function bundleWithDocument(testDoc: TestBundleDocument): string {
4147
const builder = new TestBundleBuilder(TEST_DATABASE_ID);
4248
builder.addDocumentMetadata(
@@ -49,7 +55,7 @@ function bundleWithDocument(testDoc: TestBundleDocument): string {
4955
testDoc.key,
5056
toVersion(JSON_SERIALIZER, version(testDoc.createTime)),
5157
toVersion(JSON_SERIALIZER, version(testDoc.updateTime!)),
52-
testDoc.content!
58+
wrapObject(testDoc.content!).proto.mapValue.fields!
5359
);
5460
}
5561
return builder.build(
@@ -69,33 +75,37 @@ describeSpec('Bundles:', ['no-ios', 'no-android'], () => {
6975
readTime: 3000,
7076
createTime: 1999,
7177
updateTime: 2999,
72-
content: { key: { stringValue: 'b' } }
78+
content: { key: 'b' }
7379
});
7480

7581
return spec()
7682
.userListens(query1)
7783
.watchAcksFull(query1, 1000, docA)
7884
.expectEvents(query1, { added: [docA] })
7985
.loadBundle(bundleString)
80-
.expectEvents(query1, { modified: [docAChanged], fromCache: true });
86+
.expectEvents(query1, { modified: [docAChanged] });
8187
});
8288

83-
specTest('Newer deleted docs from bundles should delete cache', [], () => {
84-
const query1 = Query.atPath(path('collection'));
85-
const docA = doc('collection/a', 1000, { key: 'a' });
89+
specTest(
90+
'Newer deleted docs from bundles should delete cache docs',
91+
[],
92+
() => {
93+
const query1 = Query.atPath(path('collection'));
94+
const docA = doc('collection/a', 1000, { key: 'a' });
8695

87-
const bundleString = bundleWithDocument({
88-
key: docA.key,
89-
readTime: 3000
90-
});
96+
const bundleString = bundleWithDocument({
97+
key: docA.key,
98+
readTime: 3000
99+
});
91100

92-
return spec()
93-
.userListens(query1)
94-
.watchAcksFull(query1, 1000, docA)
95-
.expectEvents(query1, { added: [docA] })
96-
.loadBundle(bundleString)
97-
.expectEvents(query1, { removed: [docA], fromCache: true });
98-
});
101+
return spec()
102+
.userListens(query1)
103+
.watchAcksFull(query1, 1000, docA)
104+
.expectEvents(query1, { added: [docA] })
105+
.loadBundle(bundleString)
106+
.expectEvents(query1, { removed: [docA] });
107+
}
108+
);
99109

100110
specTest('Older deleted docs from bundles should do nothing', [], () => {
101111
const query1 = Query.atPath(path('collection'));
@@ -117,7 +127,7 @@ describeSpec('Bundles:', ['no-ios', 'no-android'], () => {
117127
});
118128

119129
specTest(
120-
'Newer docs from bundles should raise snapshot only when watch catches up with acknowledged writes',
130+
'Newer docs from bundles should raise snapshot only when Watch catches up with acknowledged writes',
121131
[],
122132
() => {
123133
const query = Query.atPath(path('collection'));
@@ -128,18 +138,20 @@ describeSpec('Bundles:', ['no-ios', 'no-android'], () => {
128138
readTime: 500,
129139
createTime: 250,
130140
updateTime: 500,
131-
content: { key: { stringValue: 'b' } }
141+
content: { key: 'b' }
132142
});
133143

134144
const bundleAfterMutationAck = bundleWithDocument({
135145
key: docA.key,
136146
readTime: 1001,
137147
createTime: 250,
138148
updateTime: 1001,
139-
content: { key: { stringValue: 'fromBundle' } }
149+
content: { key: 'fromBundle' }
140150
});
141151
return (
142152
spec()
153+
// TODO(b/160878667): Figure out what happens when memory eager GC is on
154+
// a bundle is loaded.
143155
.withGCEnabled(false)
144156
.userListens(query)
145157
.watchAcksFull(query, 250, docA)
@@ -166,8 +178,7 @@ describeSpec('Bundles:', ['no-ios', 'no-android'], () => {
166178
// the acknowledged mutation.
167179
.loadBundle(bundleAfterMutationAck)
168180
.expectEvents(query, {
169-
modified: [doc('collection/a', 1001, { key: 'fromBundle' })],
170-
fromCache: true
181+
modified: [doc('collection/a', 1001, { key: 'fromBundle' })]
171182
})
172183
);
173184
}
@@ -185,7 +196,7 @@ describeSpec('Bundles:', ['no-ios', 'no-android'], () => {
185196
readTime: 1001,
186197
createTime: 250,
187198
updateTime: 1001,
188-
content: { key: { stringValue: 'fromBundle' } }
199+
content: { key: 'fromBundle' }
189200
});
190201

191202
return (
@@ -223,8 +234,9 @@ describeSpec('Bundles:', ['no-ios', 'no-android'], () => {
223234
readTime: 500,
224235
createTime: 250,
225236
updateTime: 500,
226-
content: { key: { stringValue: 'b' } }
237+
content: { key: 'b' }
227238
});
239+
const limboQuery = Query.atPath(docA.key.path);
228240

229241
return (
230242
spec()
@@ -235,11 +247,21 @@ describeSpec('Bundles:', ['no-ios', 'no-android'], () => {
235247
.expectEvents(query, {})
236248
// Bundle tells otherwise, leads to limbo.
237249
.loadBundle(bundleString1)
250+
.expectLimboDocs(docA.key)
238251
.expectEvents(query, {
239252
added: [doc('collection/a', 500, { key: 'b' })],
240253
fromCache: true
241254
})
242-
.expectLimboDocs(docA.key)
255+
// .watchAcksFull(limboQuery, 1002, docA1)
256+
.watchAcks(limboQuery)
257+
.watchSends({ affects: [limboQuery] })
258+
.watchCurrents(limboQuery, 'resume-token-1002')
259+
.watchSnapshots(1002)
260+
.expectLimboDocs()
261+
.expectEvents(query, {
262+
removed: [doc('collection/a', 500, { key: 'b' })],
263+
fromCache: false
264+
})
243265
);
244266
});
245267

@@ -254,7 +276,7 @@ describeSpec('Bundles:', ['no-ios', 'no-android'], () => {
254276
readTime: 500,
255277
createTime: 250,
256278
updateTime: 500,
257-
content: { key: { stringValue: 'b' } }
279+
content: { key: 'b' }
258280
});
259281

260282
return client(0)
@@ -286,7 +308,7 @@ describeSpec('Bundles:', ['no-ios', 'no-android'], () => {
286308
readTime: 500,
287309
createTime: 250,
288310
updateTime: 500,
289-
content: { key: { stringValue: 'b' } }
311+
content: { key: 'b' }
290312
});
291313

292314
return client(0)
@@ -302,8 +324,7 @@ describeSpec('Bundles:', ['no-ios', 'no-android'], () => {
302324
})
303325
.loadBundle(bundleString1)
304326
.expectEvents(query, {
305-
modified: [doc('collection/a', 500, { key: 'b' })],
306-
fromCache: true
327+
modified: [doc('collection/a', 500, { key: 'b' })]
307328
});
308329
}
309330
);
@@ -319,7 +340,7 @@ describeSpec('Bundles:', ['no-ios', 'no-android'], () => {
319340
readTime: 500,
320341
createTime: 250,
321342
updateTime: 500,
322-
content: { key: { stringValue: 'b' } }
343+
content: { key: 'b' }
323344
});
324345

325346
return client(0)
@@ -336,13 +357,11 @@ describeSpec('Bundles:', ['no-ios', 'no-android'], () => {
336357
.client(0)
337358
.loadBundle(bundleString1)
338359
.expectEvents(query, {
339-
modified: [doc('collection/a', 500, { key: 'b' })],
340-
fromCache: true
360+
modified: [doc('collection/a', 500, { key: 'b' })]
341361
})
342362
.client(1)
343363
.expectEvents(query, {
344-
modified: [doc('collection/a', 500, { key: 'b' })],
345-
fromCache: true
364+
modified: [doc('collection/a', 500, { key: 'b' })]
346365
});
347366
}
348367
);

0 commit comments

Comments
 (0)