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

fix: fix issue with get(name, null, options) call not meeting expectations #1228

Merged
merged 7 commits into from
Aug 13, 2024
2 changes: 2 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,7 @@ dist/
lib/common/utils/createRequest.js
lib/common/utils/encodeString.js
lib/common/utils/getStandardRegion.js
lib/common/utils/checkEnv.js
lib/common/utils/isDingTalk.js

lib/common/bucket/putBucketInventory.d.ts
2 changes: 2 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,7 @@ dist/
lib/common/utils/createRequest.js
lib/common/utils/encodeString.js
lib/common/utils/getStandardRegion.js
lib/common/utils/checkEnv.js
lib/common/utils/isDingTalk.js

lib/common/bucket/putBucketInventory.d.ts
10 changes: 9 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2078,8 +2078,9 @@ Get an object from the bucket.
parameters:

- name {String} object name store on OSS
- [file] {String|WriteStream} file path or WriteStream instance to store the content
- [file] {String|WriteStream|Object} file path or WriteStream instance to store the content
If `file` is null or ignore this parameter, function will return info contains `content` property.
If `file` is Object, it will be treated as options.
- [options] {Object} optional parameters
- [versionId] {String} the version id of history object
- [timeout] {Number} the operation timeout
Expand Down Expand Up @@ -2157,6 +2158,13 @@ await store.get('ossdemo/not-exists-demo.txt', filepath, {
});
```

- If `file` is Object, it will be treated as options.

```js
const versionId = 'versionId string';
await store.get('ossdemo/not-exists-demo.txt', { versionId });
```

### .getStream(name[, options])

Get an object read stream.
Expand Down
5 changes: 3 additions & 2 deletions lib/common/object/get.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
const fs = require('fs');
const is = require('is-type-of');
const { isObject } = require('../utils/isObject');

const proto = exports;
/**
* get
* @param {String} name - object name
* @param {String | Stream} file
* @param {String | Stream | Object} file - file path or file stream or options
* @param {Object} options
* @param {{res}}
*/
Expand All @@ -18,7 +19,7 @@ proto.get = async function get(name, file, options = {}) {
} else if (is.string(file)) {
writeStream = fs.createWriteStream(file);
needDestroy = true;
} else {
} else if (isObject(file)) {
// get(name, options)
options = file;
}
Expand Down
1 change: 1 addition & 0 deletions lib/common/object/head.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const { checkEnv } = require('../utils/checkEnv');

const proto = exports;
/**
* head
Expand Down
35 changes: 35 additions & 0 deletions test/browser/browser.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,41 @@ describe('browser', () => {
});
assert(!requestUrls2[0].includes('response-cache-control=no-cache'));
});

it('test file is not a stream or string', async () => {
let result = await store.get(name, null, {
headers: {
Range: 'bytes=0-3'
}
});
assert.equal(result.res.headers['content-length'], '4');
result = await store.get(name, 2, {
headers: {
Range: 'bytes=0-3'
}
});
assert.equal(result.res.headers['content-length'], '4');
result = await store.get(name, undefined, {
headers: {
Range: 'bytes=0-3'
}
});
assert.equal(result.res.headers['content-length'], '4');
result = await store.get(name, true, {
headers: {
Range: 'bytes=0-3'
}
});
assert.equal(result.res.headers['content-length'], '4');
});
it('test file is options', async () => {
const result = await store.get(name, {
headers: {
Range: 'bytes=0-3'
}
});
assert.equal(result.res.headers['content-length'], '4');
});
});

describe('put', () => {
Expand Down
38 changes: 38 additions & 0 deletions test/node/object.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -884,6 +884,44 @@ describe('test/object.test.js', () => {
);
});

it('test file is not a stream or string', async () => {
let result = await store.get(name, null, {
headers: {
Range: 'bytes=0-9'
}
});
assert.equal(result.res.headers['content-length'], '10');
assert(Buffer.isBuffer(result.content), 'content should be Buffer');
result = await store.get(name, undefined, {
headers: {
Range: 'bytes=0-9'
}
});
assert.equal(result.res.headers['content-length'], '10');
result = await store.get(name, 1, {
headers: {
Range: 'bytes=0-9'
}
});
assert.equal(result.res.headers['content-length'], '10');
result = await store.get(name, true, {
headers: {
Range: 'bytes=0-9'
}
});
assert.equal(result.res.headers['content-length'], '10');
});

it('test file is options', async () => {
const result = await store.get(name, {
headers: {
Range: 'bytes=0-9'
}
});
assert.equal(result.res.headers['content-length'], '10');
assert(Buffer.isBuffer(result.content), 'content should be Buffer');
});

describe('If-Modified-Since header', () => {
it('should 200 when If-Modified-Since < object modified time', async () => {
let lastYear = new Date(resHeaders.date);
Expand Down
Loading