Skip to content

Conversation

@lianup
Copy link
Contributor

@lianup lianup commented Jan 12, 2022

  1. 增加uploadFile接口,支持自定义meta
  2. 去除comment_on_pr.yml

Copy link
Contributor

@EmmetZC EmmetZC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


public Builder(URI uri) {
if (uri == null) {
throw new IllegalArgumentException("缺少上传图片接口URL");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

用了File之后,不一定是图片了。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修改

@xy-peng xy-peng linked an issue Jan 13, 2022 that may be closed by this pull request
setFileContentType(fileName);
this.meta = String.format("{\"filename\":\"%s\",\"sha256\":\"%s\"}", fileName, fileSha256);
return this;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

为什么不实现成

string meta = String.format("{\"filename\":\"%s\",\"sha256\":\"%s\"}", fileName, fileSha256);
return withFile(fileName, meta, inputStream);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

从逻辑的角度,withImage调用wtihFile来实现更合理。已修改


if (uri == null) {
throw new IllegalArgumentException("缺少上传图片接口URL");
if (meta == null || meta.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这些检查还是需要吧,除了fileSha256之外。开发者要是忘记调用withFile()或者withImage()就调用build()是要报错的哦。请再参考标准的builder实现。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

现在虽然我们的设计meta就意味着有其他参数,但是未来可能放开了呢?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

有道理。之前没有考虑未来放开的场景,已修改。

@xy-peng xy-peng merged commit d8f2850 into wechatpay-apiv3:master Jan 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

上传文件接口-不支持自定义meta字段

4 participants