Skip to content

Commit

Permalink
fix(bucket): Only disable resumable uploads for bucket.upload (fixes #…
Browse files Browse the repository at this point in the history
…1133) (#1135)

* test(bucket): failing test for #1133

* fix(bucket): only disable resumable uploads for bucket.upload when inspecting the file size (fixes #1133)

* text(bucket): Applied patch from @stephenplusplus - see #1135 (comment)
  • Loading branch information
rhodgkins committed May 1, 2020
1 parent a92b7bb commit 2c20148
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 14 deletions.
5 changes: 4 additions & 1 deletion src/bucket.ts
Expand Up @@ -3454,7 +3454,10 @@ class Bucket extends ServiceObject {
return;
}

options.resumable = fd.size > RESUMABLE_THRESHOLD;
if (fd.size <= RESUMABLE_THRESHOLD) {
// Only disable resumable uploads so createWriteStream still attempts them and falls back to simple upload.
options.resumable = false;
}

upload();
});
Expand Down
79 changes: 66 additions & 13 deletions test/bucket.ts
Expand Up @@ -21,6 +21,8 @@ import {
} from '@google-cloud/common';
import arrify = require('arrify');
import * as assert from 'assert';
import * as extend from 'extend';
import * as fs from 'fs';
import {describe, it, before, beforeEach, after, afterEach} from 'mocha';
import * as mime from 'mime-types';
import pLimit from 'p-limit';
Expand Down Expand Up @@ -89,6 +91,13 @@ class FakeNotification {
}
}

let fsStatOverride: Function | null;
const fakeFs = extend(true, {}, fs, {
stat: (filePath: string, callback: Function) => {
return (fsStatOverride || fs.stat)(filePath, callback);
},
});

let pLimitOverride: Function | null;
const fakePLimit = (limit: number) => (pLimitOverride || pLimit)(limit);

Expand Down Expand Up @@ -172,6 +181,7 @@ describe('Bucket', () => {

before(() => {
Bucket = proxyquire('../src/bucket.js', {
fs: fakeFs,
'p-limit': {default: fakePLimit},
'@google-cloud/promisify': fakePromisify,
'@google-cloud/paginator': fakePaginator,
Expand All @@ -188,6 +198,7 @@ describe('Bucket', () => {
});

beforeEach(() => {
fsStatOverride = null;
pLimitOverride = null;
bucket = new Bucket(STORAGE, BUCKET_NAME);
});
Expand Down Expand Up @@ -2381,19 +2392,61 @@ describe('Bucket', () => {
bucket.upload(textFilepath, options, assert.ifError);
});

it('should force a resumable upload', done => {
const fakeFile = new FakeFile(bucket, 'file-name');
const options = {destination: fakeFile, resumable: true};
fakeFile.createWriteStream = (options_: CreateWriteStreamOptions) => {
const ws = new stream.Writable();
ws.write = () => true;
setImmediate(() => {
assert.strictEqual(options_.resumable, options.resumable);
done();
});
return ws;
};
bucket.upload(filepath, options, assert.ifError);
describe('resumable uploads', () => {
beforeEach(() => {
fsStatOverride = (path: string, callback: Function) => {
callback(null, {size: 1}); // Small size to guarantee simple upload
};
});

it('should force a resumable upload', done => {
const fakeFile = new FakeFile(bucket, 'file-name');
const options = {destination: fakeFile, resumable: true};
fakeFile.createWriteStream = (options_: CreateWriteStreamOptions) => {
const ws = new stream.Writable();
ws.write = () => true;
setImmediate(() => {
assert.strictEqual(options_.resumable, options.resumable);
done();
});
return ws;
};
bucket.upload(filepath, options, assert.ifError);
});

it('should not pass resumable option to createWriteStream when file size is greater than minimum resumable threshold', done => {
const fakeFile = new FakeFile(bucket, 'file-name');
const options = {destination: fakeFile};
fsStatOverride = (path: string, callback: Function) => {
// Set size greater than threshold
callback(null, {size: 5000001});
};
fakeFile.createWriteStream = (options_: CreateWriteStreamOptions) => {
const ws = new stream.Writable();
ws.write = () => true;
setImmediate(() => {
assert.strictEqual(typeof options_.resumable, 'undefined');
done();
});
return ws;
};
bucket.upload(filepath, options, assert.ifError);
});

it('should prevent resumable when file size is less than minimum resumable threshold', done => {
const fakeFile = new FakeFile(bucket, 'file-name');
const options = {destination: fakeFile};
fakeFile.createWriteStream = (options_: CreateWriteStreamOptions) => {
const ws = new stream.Writable();
ws.write = () => true;
setImmediate(() => {
assert.strictEqual(options_.resumable, false);
done();
});
return ws;
};
bucket.upload(filepath, options, assert.ifError);
});
});

it('should allow overriding content type', done => {
Expand Down

0 comments on commit 2c20148

Please sign in to comment.