Skip to content

Commit

Permalink
fix: add check to file of get(#1228)
Browse files Browse the repository at this point in the history
* fix: fix issue with get(name, null, options) call not meeting expectations

* test: add test case

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

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

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

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

---------

Co-authored-by: csg01123119 <csg01123119@alibaba-inc.com>
  • Loading branch information
shungang and csg01123119 committed Aug 13, 2024
1 parent 1470e7d commit c52bd5b
Show file tree
Hide file tree
Showing 7 changed files with 90 additions and 3 deletions.
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

0 comments on commit c52bd5b

Please sign in to comment.