-
Notifications
You must be signed in to change notification settings - Fork 580
support cloudbox #1355
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
support cloudbox #1355
Conversation
lib/common/signUtils.js
Outdated
* @param {string} region | ||
* @return {string} | ||
*/ | ||
exports.getSignRegion = function getSignRegion(cloudBoxId, region) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
必填项在前,可选项在后
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
js没有必填概念吧,都要填的,只不过cloudBoxId为undefined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
遵守函数定义的规范,你的JS doc写的是{string} [cloudBoxId],可选项放在前面了,如果两个都是必填,cloudBoxId的类型要增加undefined,且不能是可选
lib/common/signUtils.js
Outdated
* @returns {string} | ||
*/ | ||
exports.getStringToSign = function getStringToSign(region, date, canonicalRequest) { | ||
exports.getStringToSign = function getStringToSign(region, date, canonicalRequest, product) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
product改成可选项,product = 'oss',下面就不用写product || 'oss'了
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
已修改
@@ -138,6 +137,9 @@ describe('test/bucket.test.js', () => { | |||
assert.equal(result.bucket.IntranetEndpoint, `${config.region}-internal.aliyuncs.com`); | |||
assert.equal(result.bucket.AccessControlList.Grant, 'private'); | |||
assert.equal(result.bucket.StorageClass, 'Standard'); | |||
|
|||
assert.equal(result.bucket.AccessControlList.Grant, 'private'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2行多余的断言,删一下
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
已删除
test/node/object.test.js
Outdated
@@ -1086,6 +1083,9 @@ describe('test/object.test.js', () => { | |||
}); | |||
|
|||
describe('signatureUrl(), asyncSignatureUrl() and signatureUrlV4()', () => { | |||
before(function () { | |||
if (store.options.cloudBoxId) this.skip(); // 云盒不支持url签名 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这条注释删一下
test/node/object.test.js
Outdated
@@ -2626,6 +2634,9 @@ describe('test/object.test.js', () => { | |||
}); | |||
|
|||
describe('calculatePostSignature()', () => { | |||
before(function () { | |||
if (store.options.cloudBoxId) this.skip(); // 云盒只支持head v4签名 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这条注释和2703行的都删一下
test/node/signature.test.js
Outdated
]); | ||
}); | ||
it(`should authorizationV4 support ${isCloudBox ? 'cloudBox' : 'publicCloud'}`, async () => { | ||
sinon.restore(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
82行可以删了
test/node/sts.test.js
Outdated
@@ -87,9 +81,10 @@ describe('test/sts.test.js', () => { | |||
const stsClient = sts(stsConfig); | |||
let result = await stsClient.assumeRole(stsConfig.roleArn); | |||
assert.strictEqual(result.res.status, 200); | |||
|
|||
console.log(stsConfig.bucket); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
console.log删一下
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
console.log这个没有配置eslint吗
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
你看下呢
No description provided.