feat!: automatically detect contentType if not provided (#1190)

* Automatically change contentType to auto if none provided. * add tests Co-authored-by: Stephen <stephenplusplus@users.noreply.github.com> Co-authored-by: Jonathan Lui <jonathanlui@google.com>
googleapis · May 13, 2020 · b31ba4a · b31ba4a
1 parent 5a4044a
commit b31ba4a
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 44 deletions.
diff --git a/src/bucket.ts b/src/bucket.ts
@@ -3440,12 +3440,6 @@ class Bucket extends ServiceObject {
       });
     }
 
-    const contentType = mime.contentType(path.basename(pathString));
-
-    if (contentType && !options.metadata.contentType) {
-      options.metadata.contentType = contentType;
-    }
-
     if (options.resumable !== null && typeof options.resumable === 'boolean') {
       upload();
     } else {

diff --git a/src/file.ts b/src/file.ts
@@ -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;
       }
     }
 

diff --git a/test/bucket.ts b/test/bucket.ts
@@ -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',
@@ -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) => {

diff --git a/test/file.ts b/test/file.ts
@@ -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