mozillazg / go-cos

腾讯云对象存储服务 COS(Cloud Object Storage) Go SDK(XML API)
https://godoc.org/github.com/mozillazg/go-cos
MIT License
88 stars 26 forks source link

To wrap the closer as a nop for request body #7

Closed jojohappy closed 5 years ago

jojohappy commented 5 years ago

Signed-off-by: jojohappy sarahdj0917@gmail.com

变更: 由于 Golang 的 net/http 包在做 client.Do(req) 时默认会调用 req.Body.Close(), 如果我们上传的 Object 是文件时,文件句柄将会被关闭,这样就没有办法在外部程序复用这个文件句柄,同时会有 file already closed 的错误发生。

这里的修改尝试在 io.Reader 外面再封装一个 NopCloser ,默认 Close() 返回 nil。对于文件句柄的关闭操作应当由调用方来处理。

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.1%) to 92.795% when pulling 884aacb60e5bf6619dda2a16d31c25353cbf338b on jojohappy:make_reader_closer_as_a_nop into 7c1d2307dad7bfe68e88f24a5fc2950d87ed8f37 on mozillazg:master.

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.1%) to 92.795% when pulling 884aacb60e5bf6619dda2a16d31c25353cbf338b on jojohappy:make_reader_closer_as_a_nop into 7c1d2307dad7bfe68e88f24a5fc2950d87ed8f37 on mozillazg:master.

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.1%) to 92.795% when pulling 884aacb60e5bf6619dda2a16d31c25353cbf338b on jojohappy:make_reader_closer_as_a_nop into 7c1d2307dad7bfe68e88f24a5fc2950d87ed8f37 on mozillazg:master.

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.1%) to 92.795% when pulling 884aacb60e5bf6619dda2a16d31c25353cbf338b on jojohappy:make_reader_closer_as_a_nop into 7c1d2307dad7bfe68e88f24a5fc2950d87ed8f37 on mozillazg:master.

mozillazg commented 5 years ago

@jojohappy Thanks for your PR!

听起来似乎是可以在传入参数前用 ioutil.NopCloser 包装一下文件 object,不需要在 cos 代码中做这个操作? cos 这边可以在相关 api 文档中加一下这个场景的注意事项。

jojohappy commented 5 years ago

@mozillazg 好的,谢谢。不过这样做的初衷是希望对用户透明,让用户不用去关心接口以外的内容。

mozillazg commented 5 years ago

@jojohappy req.body 自动 close 是为了防止出现资源泄露,如果直接用 ioutil.NopCloser 包装的话,反而需要用户去关心资源回收的问题。毕竟需要用 ioutil.NopCloser 包装的场景比较少,大部分情况下都是可以安全 close 的,这应该也是 client.Do(req) 中自动调用 close 的考虑。如果我的说法不恰当的话,欢迎一起讨论。

jojohappy commented 5 years ago

@mozillazg 谢谢你的回复,我同意你的说法。这个 PR 的目的是针对类似 *os.File 为目标的 io.Reader, 由于接口入参是 interface, 所以对于用户来说,他们更清楚的了解需要上传的对象是什么。 对于*os.File, 大家都默认会调用defer file.Close(), 那是不是用户自己来处理对象会比较好呢?当然我会再去找找有没有类似的案例,希望有更好的解决方案。

mozillazg commented 5 years ago

@jojohappy 后来又想了一下,修改位置的 body 主要是用于上传文件的场景下,你的修改确实是比较合理的考虑。

jojohappy commented 5 years ago

@mozillazg 非常感谢~ 如果后续需要做改进,可以再联系我~