Skip to content

feat!: automatically detect contentType if not provided #1190

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.

Sign up for GitHub

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

Merged
jkwlui merged 5 commits into from
May 13, 2020
Merged

feat!: automatically detect contentType if not provided #1190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c61fa48
Automatically change contentType to auto if none provided.
May 12, 2020
7fa18d3
Merge c61fa4890b6d07b875466454846f764fcead33b2 into 0bb19098013acf71c…
May 12, 2020
3c0a9a7
add tests
stephenplusplus May 12, 2020
7dd866d
Merge branch 'master' into master
stephenplusplus May 13, 2020
4e119d7
Merge branch 'master' into master
jkwlui May 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions src/bucket.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3440,12 +3440,6 @@ class Bucket extends ServiceObject {
});
}

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

@stephenplusplus stephenplusplus May 12, 2020

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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1694,9 +1694,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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1884,6 +1884,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