Skip to content

Commit f5e4474

Browse files
committed
Address more feedback.
1 parent 782e76f commit f5e4474

File tree

2 files changed

+37
-30
lines changed

2 files changed

+37
-30
lines changed

packages/firestore/src/util/bundle_reader.ts

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ export class BundleReader {
9797
}
9898
this.reader = (this.bundleStream as ReadableStream).getReader();
9999

100+
// Read the metadata (which is the first element).
100101
this.nextElement().then(
101102
element => {
102103
if (element && element.isBundleMetadata()) {
@@ -109,9 +110,7 @@ export class BundleReader {
109110
);
110111
}
111112
},
112-
error => {
113-
this.metadata.reject(error);
114-
}
113+
error => this.metadata.reject(error)
115114
);
116115
}
117116

@@ -175,25 +174,29 @@ export class BundleReader {
175174
* If reached end of the stream, returns a null.
176175
*/
177176
private async readLength(): Promise<Uint8Array | null> {
178-
let position = this.indexOfOpenBracket();
179-
while (position < 0) {
177+
let position: number;
178+
while ((position = this.indexOfOpenBracket()) < 0) {
180179
const done = await this.pullMoreDataToBuffer();
181180
if (done) {
182-
if (this.buffer.length === 0) {
183-
return null;
184-
}
185-
position = this.indexOfOpenBracket();
186-
// Underlying stream is closed, and we still cannot find a '{'.
187-
if (position < 0) {
188-
this.raiseError(
189-
'Reached the end of bundle when a length string is expected.'
190-
);
191-
}
192-
} else {
193-
position = this.indexOfOpenBracket();
181+
break;
194182
}
195183
}
196184

185+
// Broke out of the loop because underlying stream is closed, and there
186+
// happens to be no more data to process.
187+
if (this.buffer.length === 0) {
188+
return null;
189+
}
190+
191+
position = this.indexOfOpenBracket();
192+
// Broke out of the loop because underlying stream is closed, but still
193+
// cannot find an open bracket.
194+
if (position < 0) {
195+
this.raiseError(
196+
'Reached the end of bundle when a length string is expected.'
197+
);
198+
}
199+
197200
const result = this.buffer.slice(0, position);
198201
// Update the internal buffer to drop the read length.
199202
this.buffer = this.buffer.slice(position);

packages/firestore/test/unit/util/bundle.test.ts

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ function genericBundleReadingTests(bytesPerRead: number): void {
166166
};
167167
const limitToLastQueryString = lengthPrefixedString(limitToLastQuery);
168168

169-
async function getAllElement(
169+
async function getAllElements(
170170
bundle: BundleReader
171171
): Promise<SizedBundleElement[]> {
172172
const result: SizedBundleElement[] = [];
@@ -194,7 +194,7 @@ function genericBundleReadingTests(bytesPerRead: number): void {
194194
);
195195
}
196196

197-
async function parseThroughBundle(
197+
async function generateBundleAndParse(
198198
bundleString: string,
199199
bytesPerRead: number,
200200
validMeta = false
@@ -208,7 +208,7 @@ function genericBundleReadingTests(bytesPerRead: number): void {
208208
expect(await bundle.getMetadata()).to.deep.equal(meta.metadata);
209209
}
210210

211-
await getAllElement(bundle);
211+
await getAllElements(bundle);
212212
}
213213

214214
it('reads with query and doc with bytesPerRead ' + bytesPerRead, async () => {
@@ -224,7 +224,7 @@ function genericBundleReadingTests(bytesPerRead: number): void {
224224

225225
expect(await bundle.getMetadata()).to.deep.equal(meta.metadata);
226226

227-
const actual = await getAllElement(bundle);
227+
const actual = await getAllElements(bundle);
228228
expect(actual.length).to.equal(4);
229229
verifySizedElement(actual[0], limitQuery, limitQueryString);
230230
verifySizedElement(actual[1], limitToLastQuery, limitToLastQueryString);
@@ -246,7 +246,7 @@ function genericBundleReadingTests(bytesPerRead: number): void {
246246
);
247247
const bundle = new BundleReader(bundleStream);
248248

249-
const actual = await getAllElement(bundle);
249+
const actual = await getAllElements(bundle);
250250
expect(actual.length).to.equal(5);
251251
verifySizedElement(actual[0], doc1Meta, doc1MetaString);
252252
verifySizedElement(actual[1], doc1, doc1String);
@@ -270,7 +270,7 @@ function genericBundleReadingTests(bytesPerRead: number): void {
270270

271271
expect(await bundle.getMetadata()).to.deep.equal(meta.metadata);
272272

273-
const actual = await getAllElement(bundle);
273+
const actual = await getAllElements(bundle);
274274
expect(actual.length).to.equal(2);
275275
verifySizedElement(actual[0], doc1Meta, doc1MetaString);
276276
verifySizedElement(actual[1], doc1, doc1String);
@@ -286,7 +286,7 @@ function genericBundleReadingTests(bytesPerRead: number): void {
286286

287287
expect(await bundle.getMetadata()).to.deep.equal(meta.metadata);
288288

289-
const actual = await getAllElement(bundle);
289+
const actual = await getAllElements(bundle);
290290
expect(actual.length).to.equal(3);
291291
verifySizedElement(actual[0], noDocMeta, noDocMetaString);
292292
verifySizedElement(actual[1], doc1Meta, doc1MetaString);
@@ -301,7 +301,7 @@ function genericBundleReadingTests(bytesPerRead: number): void {
301301

302302
expect(await bundle.getMetadata()).to.deep.equal(meta.metadata);
303303

304-
const actual = await getAllElement(bundle);
304+
const actual = await getAllElements(bundle);
305305
expect(actual.length).to.equal(0);
306306
}
307307
);
@@ -310,28 +310,32 @@ function genericBundleReadingTests(bytesPerRead: number): void {
310310
'throws with ill-formatted bundle with bytesPerRead ' + bytesPerRead,
311311
async () => {
312312
await expect(
313-
parseThroughBundle('metadata: "no length prefix"', bytesPerRead)
313+
generateBundleAndParse('metadata: "no length prefix"', bytesPerRead)
314314
).to.be.rejectedWith(
315315
'Reached the end of bundle when a length string is expected.'
316316
);
317317

318318
await expect(
319-
parseThroughBundle('{metadata: "no length prefix"}', bytesPerRead)
319+
generateBundleAndParse('{metadata: "no length prefix"}', bytesPerRead)
320320
).to.be.rejectedWith('Unexpected end of JSON input');
321321

322322
await expect(
323-
parseThroughBundle(metaString + 'invalid-string', bytesPerRead, true)
323+
generateBundleAndParse(
324+
metaString + 'invalid-string',
325+
bytesPerRead,
326+
true
327+
)
324328
).to.be.rejectedWith(
325329
'Reached the end of bundle when a length string is expected.'
326330
);
327331

328332
await expect(
329-
parseThroughBundle('1' + metaString, bytesPerRead)
333+
generateBundleAndParse('1' + metaString, bytesPerRead)
330334
).to.be.rejectedWith('Reached the end of bundle when more is expected.');
331335

332336
// First element is not BundleMetadata.
333337
await expect(
334-
parseThroughBundle(doc1MetaString + doc1String, bytesPerRead)
338+
generateBundleAndParse(doc1MetaString + doc1String, bytesPerRead)
335339
).to.be.rejectedWith('The first element of the bundle is not a metadata');
336340
}
337341
);

0 commit comments

Comments
 (0)