fix: responseCacheControl in Node.js (#919)

* fix: responseCacheControl in Node.js * fix: code specification
ali-sdk · Jan 7, 2021 · 7ca7055 · 7ca7055
1 parent 3b009af
commit 7ca7055
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 77 deletions.
diff --git a/lib/browser/object.js b/lib/browser/object.js
@@ -3,7 +3,6 @@ const fs = require('fs');
 const copy = require('copy-to');
 const path = require('path');
 const mime = require('mime');
-const is = require('is-type-of');
 const callback = require('../common/callback');
 const merge = require('merge-descriptors');
 const { isBlob } = require('../common/utils/isBlob');
@@ -371,63 +370,3 @@ proto._deleteFileSafe = function _deleteFileSafe(filepath) {
     });
   });
 };
-
-/**
- * get
- * @param {String} name - object name
- * @param {String | Stream} file
- * @param {Object} options
- * @param {{res}}
- */
-proto.get = async function get(name, file, options = {}) {
-  let writeStream = null;
-  let needDestroy = false;
-
-  if (is.writableStream(file)) {
-    writeStream = file;
-  } else if (is.string(file)) {
-    writeStream = fs.createWriteStream(file);
-    needDestroy = true;
-  } else {
-    // get(name, options)
-    options = file;
-  }
-
-  options = options || {};
-  const responseCacheControl = options.responseCacheControl === null ? '' : 'no-cache';
-  options.subres = Object.assign(
-    responseCacheControl ? { 'response-cache-control': responseCacheControl } : {},
-    options.subres
-  );
-  if (options.versionId) {
-    options.subres.versionId = options.versionId;
-  }
-  if (options.process) {
-    options.subres['x-oss-process'] = options.process;
-  }
-
-  let result;
-  try {
-    const params = this._objectRequestParams('GET', name, options);
-    params.writeStream = writeStream;
-    params.successStatuses = [200, 206, 304];
-
-    result = await this.request(params);
-
-    if (needDestroy) {
-      writeStream.destroy();
-    }
-  } catch (err) {
-    if (needDestroy) {
-      writeStream.destroy();
-      // should delete the exists file before throw error
-      await this._deleteFileSafe(file);
-    }
-    throw err;
-  }
-
-  return {
-    res: result.res,
-    content: result.data
-  };
-};
diff --git a/lib/common/object/get.js b/lib/common/object/get.js
@@ -24,8 +24,12 @@ proto.get = async function get(name, file, options = {}) {
   }
 
   options = options || {};
+  const isBrowserEnv = process && process.browser;
+  const responseCacheControl = options.responseCacheControl === null ? '' : 'no-cache';
+  const defaultSubresOptions =
+    isBrowserEnv && responseCacheControl ? { 'response-cache-control': responseCacheControl } : {};
+  options.subres = Object.assign(defaultSubresOptions, options.subres);
 
-  options.subres = Object.assign({ 'response-cache-control': 'no-cache' }, options.subres);
   if (options.versionId) {
     options.subres.versionId = options.versionId;
   }

diff --git a/lib/common/utils/deepCopy.js b/lib/common/utils/deepCopy.js
@@ -24,7 +24,7 @@ exports.deepCopyWith = (obj, customizer) => {
             return value;
         }
         if (isBuffer_1.isBuffer(value)) {
-            return obj.slice();
+            return value.slice();
         }
         const copy = Array.isArray(value) ? [] : {};
         Object.keys(value).forEach((k) => {

diff --git a/lib/common/utils/deepCopy.ts b/lib/common/utils/deepCopy.ts
@@ -27,7 +27,7 @@ export const deepCopyWith = (obj: any, customizer?: (v: any, k: string, o: any)
     }
 
     if (isBuffer(value)) {
-      return obj.slice();
+      return value.slice();
     }
 
     const copy = Array.isArray(value) ? [] : {};

diff --git a/test/browser/browser.test.js b/test/browser/browser.test.js
@@ -640,19 +640,20 @@ describe('browser', () => {
   describe('get()', () => {
     const name = `${prefix}ali-sdk/get/${Date.now()}-oss.jpg`
     const store = new OSS(ossConfig);
-    before(() => {
-      await store.put('name', Buffer.from('oss.jpg'));
-    });
-    it('should get with disableCache option', async () => {
-      const originRequest = store.urllib.request;
-      let requestUrl;
-      store.urllib.request = (url, params) => {
-        requestUrl = url;
-        return originRequest.call(store.urllib, url, params);
-      };
-      await store.get(name);
-      store.urllib.request = originRequest;
-      assert(requestUrl.includes('response-cache-control=no-cache'));
+    before(async () => {
+      await store.put(name, Buffer.from('oss.jpg'));
+    });
+    it('should get with default responseCacheControl option', async () => {
+      const {
+        res: { requestUrls }
+      } = await store.get(name);
+      assert(requestUrls[0].includes('response-cache-control=no-cache'));
+      const {
+        res: { requestUrls: requestUrls2 }
+      } = await store.get(name, {
+        responseCacheControl: null
+      });
+      assert(!requestUrls2[0].includes('response-cache-control=no-cache'));
     });
   });
 

diff --git a/test/node/object.test.js b/test/node/object.test.js
@@ -774,6 +774,7 @@ describe('test/object.test.js', () => {
       const savepath = path.join(tmpdir, name.replace(/\//g, '-'));
       const result = await store.get(name, savepath);
       assert.equal(result.res.status, 200);
+      assert(!result.res.requestUrls[0].includes('response-cache-control=no-cache'));
       assert.equal(fs.statSync(savepath).size, fs.statSync(__filename).size);
     });