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

feat!: automatically detect contentType if not provided #1190

Merged
merged 5 commits into from May 13, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 0 additions & 6 deletions src/bucket.ts
Expand Up @@ -3438,12 +3438,6 @@ class Bucket extends ServiceObject {
});
}

const contentType = mime.contentType(path.basename(pathString));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed as this is identically covered inside of createWriteStream() when this code makes its way there.


if (contentType && !options.metadata.contentType) {
options.metadata.contentType = contentType;
}

if (options.resumable !== null && typeof options.resumable === 'boolean') {
upload();
} else {
Expand Down
10 changes: 8 additions & 2 deletions src/file.ts
Expand Up @@ -1700,9 +1700,15 @@ class File extends ServiceObject<File> {

if (options.contentType) {
options.metadata.contentType = options.contentType;
}

if (options.metadata.contentType === 'auto') {
options.metadata.contentType = mime.getType(this.name);
if (
!options.metadata.contentType ||
options.metadata.contentType === 'auto'
) {
const detectedContentType = mime.getType(this.name);
if (detectedContentType) {
options.metadata.contentType = detectedContentType;
}
}

Expand Down
36 changes: 0 additions & 36 deletions test/bucket.ts
Expand Up @@ -2246,10 +2246,6 @@ describe('Bucket', () => {
describe('upload', () => {
const basename = 'testfile.json';
const filepath = path.join(__dirname, '../../test/testdata/' + basename);
const textFilepath = path.join(
__dirname,
'../../test/testdata/textfile.txt'
);
const metadata = {
metadata: {
a: 'b',
Expand Down Expand Up @@ -2360,38 +2356,6 @@ describe('Bucket', () => {
});
});

it('should guess at the content type', 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(() => {
const expectedContentType = 'application/json; charset=utf-8';
assert.strictEqual(options.metadata.contentType, expectedContentType);
done();
});
return ws;
};
bucket.upload(filepath, options, assert.ifError);
});

it('should guess at the charset', 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(() => {
const expectedContentType = 'text/plain; charset=utf-8';
assert.strictEqual(options.metadata.contentType, expectedContentType);
done();
});
return ws;
};
bucket.upload(textFilepath, options, assert.ifError);
});

describe('resumable uploads', () => {
beforeEach(() => {
fsStatOverride = (path: string, callback: Function) => {
Expand Down
23 changes: 23 additions & 0 deletions test/file.ts
Expand Up @@ -1896,6 +1896,29 @@ describe('File', () => {
writable.write('data');
});

it('should detect contentType if not defined', done => {
const writable = file.createWriteStream();
// eslint-disable-next-line @typescript-eslint/no-explicit-any
file.startResumableUpload_ = (stream: {}, options: any) => {
assert.strictEqual(options.metadata.contentType, 'image/png');
done();
};

writable.write('data');
});

it('should not set a contentType if mime lookup failed', done => {
const file = new File('file-without-ext');
const writable = file.createWriteStream();
// eslint-disable-next-line @typescript-eslint/no-explicit-any
file.startResumableUpload_ = (stream: {}, options: any) => {
assert.strictEqual(typeof options.metadata.contentType, 'undefined');
done();
};

writable.write('data');
});

it('should set encoding with gzip:true', done => {
const writable = file.createWriteStream({gzip: true});
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down