From 8620b8d21501e39e544d12ca988b0b8875932bb4 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Thu, 1 Sep 2022 10:44:31 -0700 Subject: [PATCH 1/3] test: read entire file when range is `{start: 0}` --- system-test/storage.ts | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/system-test/storage.ts b/system-test/storage.ts index c7614368e..dc6c899fb 100644 --- a/system-test/storage.ts +++ b/system-test/storage.ts @@ -2099,6 +2099,27 @@ describe('storage', () => { }); }); + it('should read an entire file if range `start:0` is provided', done => { + bucket.upload(FILES.big.path, (err: Error | null, file?: File | null) => { + assert.ifError(err); + + const fileSize = file!.metadata.size; + const byteRange = {start: 0}; + + let sizeStreamed = 0; + file! + .createReadStream(byteRange) + .on('data', (chunk: Buffer) => { + sizeStreamed += chunk.byteLength; + }) + .on('error', done) + .on('end', () => { + assert.strictEqual(sizeStreamed, fileSize); + file!.delete(done); + }); + }); + }); + it('should support readable[Symbol.asyncIterator]()', async () => { const fileContents = fs.readFileSync(FILES.big.path); From d50bb5ef49ac06c5cdb7ca38f20c11a4a61e84f7 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Thu, 1 Sep 2022 10:53:59 -0700 Subject: [PATCH 2/3] fix: `ReturnType` regression --- system-test/storage.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system-test/storage.ts b/system-test/storage.ts index dc6c899fb..e2367d08f 100644 --- a/system-test/storage.ts +++ b/system-test/storage.ts @@ -1719,7 +1719,7 @@ describe('storage', () => { // Validate the desired functionality const results = await testFunction(USER_PROJECT_OPTIONS); - return results; + return results as ReturnType; } it('bucket#combine', async () => { From 577dac3e074ab764778043d38d9d40e4f34085af Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Thu, 1 Sep 2022 11:34:03 -0700 Subject: [PATCH 3/3] fix: Remove premature `.end()` calls --- src/file.ts | 15 +++++---------- system-test/storage.ts | 29 ++++++++--------------------- 2 files changed, 13 insertions(+), 31 deletions(-) diff --git a/src/file.ts b/src/file.ts index dbab2a246..e48cb507b 100644 --- a/src/file.ts +++ b/src/file.ts @@ -1417,9 +1417,7 @@ class File extends ServiceObject { // Authenticate the request, then pipe the remote API request to the stream // returned to the user. const makeRequest = () => { - const query = { - alt: 'media', - } as FileQuery; + const query: FileQuery = {alt: 'media'}; if (this.generation) { query.generation = this.generation; @@ -1460,8 +1458,7 @@ class File extends ServiceObject { }) .on('response', res => { throughStream.emit('response', res); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - util.handleResp(null, res, null, onResponse as any); + util.handleResp(null, res, null, onResponse); }) .resume(); @@ -1525,7 +1522,7 @@ class File extends ServiceObject { const handoffStream = new PassThrough({ final: async cb => { // Preserving `onComplete`'s ability to - // close `throughStream` before pipeline + // destroy `throughStream` before pipeline // attempts to. await onComplete(null); cb(); @@ -1558,7 +1555,6 @@ class File extends ServiceObject { } if (rangeRequest || !shouldRunValidation) { - throughStream.end(); return; } @@ -1576,7 +1572,6 @@ class File extends ServiceObject { return; } if (this.metadata.contentEncoding === 'gzip') { - throughStream.end(); return; } } @@ -1611,14 +1606,14 @@ class File extends ServiceObject { throughStream.destroy(mismatchError); } else { - throughStream.end(); + return; } }; }; throughStream.on('reading', makeRequest); - return throughStream as Readable; + return throughStream; } createResumableUpload( diff --git a/system-test/storage.ts b/system-test/storage.ts index e2367d08f..a74b4ec2a 100644 --- a/system-test/storage.ts +++ b/system-test/storage.ts @@ -2099,27 +2099,6 @@ describe('storage', () => { }); }); - it('should read an entire file if range `start:0` is provided', done => { - bucket.upload(FILES.big.path, (err: Error | null, file?: File | null) => { - assert.ifError(err); - - const fileSize = file!.metadata.size; - const byteRange = {start: 0}; - - let sizeStreamed = 0; - file! - .createReadStream(byteRange) - .on('data', (chunk: Buffer) => { - sizeStreamed += chunk.byteLength; - }) - .on('error', done) - .on('end', () => { - assert.strictEqual(sizeStreamed, fileSize); - file!.delete(done); - }); - }); - }); - it('should support readable[Symbol.asyncIterator]()', async () => { const fileContents = fs.readFileSync(FILES.big.path); @@ -2140,6 +2119,14 @@ describe('storage', () => { assert.strictEqual(String(fileContents), String(remoteContents)); }); + it('should download an entire file if range `start:0` is provided', async () => { + const fileContents = fs.readFileSync(FILES.big.path); + const [file] = await bucket.upload(FILES.big.path); + const [result] = await file.download({start: 0}); + + assert.strictEqual(result.toString(), fileContents.toString()); + }); + it('should download an empty file', async () => { const fileContents = fs.readFileSync(FILES.empty.path); const [file] = await bucket.upload(FILES.empty.path);