ogen
ogen copied to clipboard
feat: Response types that return binary strings should be generated as io.ReadCloser
Description
Currently, a response that returns a binary strings will generate the following type:
type FooOKSomeContentType struct {
Data io.Reader
}
This type seems to force us to convert our own io.ReadClosers to something like bytes.Reader before assigning it to the Data field, because we have to Close our readers before releasing control. This unnecessarily copies the data around, instead of sending it directly to the client through the response itself.
If the type was generated as an io.ReadCloser, we could directly assign our own readClosers, and the code would make sure to Close after invoking io.Copy between the ResponseWriter and the response itself.
References
https://swagger.io/docs/specification/data-models/data-types/#file
~To clarify the current situation, ogen allows response streaming by using io.Reader for reading the response body, but there is no way for the client to terminate the request early. Is that correct? This functionality would be useful for dealing with server-side events ("text/event-stream" responses).~
Update: Looking at the generated code, it seems that ogen calls io.ReadAll on the response body before exposing it as an io.Reader, which is really just a bytes.Reader. This means that you can't actually make streaming requests right now. It would be nice to have a way of getting direct access to the the underlying http.Response.Body.
It seems that ogen calls
io.ReadAllon the response body
This doesn't seem to be the case anymore?
BTW, I think this can be implemented in a backwards compatible way (not break user code).
func encodeSomethingResponse(response SomethingOK, w http.ResponseWriter) error {
w.Header().Set("Content-Type", "application/octet-stream")
w.WriteHeader(200)
writer := w
if _, err := io.Copy(writer, response); err != nil {
return errors.Wrap(err, "write")
}
return nil
}
If we add to that something along the lines of:
if c, ok := response.Data.(io.Closer); ok {
if err := c.Close(); err != nil {
return errors.Wrap(err, "close")
}
}
This will do the expected and handle closing as well.
Would a PR for that be accepted?
I believe what @rubenv suggested is what the latest version of ogen does already. So the issue could be closed.