[go] support io.Reader and []byte response types in client decode#23789
Conversation
e58bee9 to
746935a
Compare
746935a to
a8c1c98
Compare
The Go generator maps binary types to *os.File by default. Callers can override that with --type-mappings to get io.Reader or []byte, and setBody already handles both for request bodies. The shared decode helper, however, only asserts *string, *os.File, and **os.File. When Execute declares localVarReturnValue io.Reader or []byte and calls decode(&localVarReturnValue, ...), no branch matches and the call returns 'undefined response type'. Add two branches: - *io.Reader wraps the already-buffered bytes in a bytes.Reader. - *[]byte assigns the bytes directly. This branch must stay before the JSON branch, since json.Unmarshal accepts *[]byte and base64-decodes into it, which is not what we want for raw binary responses. Both shapes are useful for different scenarios: []byte is more honest (the response is fully buffered in memory anyway), while io.Reader is forward-compatible (the return type stays valid if the response path is ever refactored to skip the eager buffering and stream the body). bytes is already imported in every generated client.go. The same additive patch is applied to the eleven Go client samples.
a8c1c98 to
86b0a52
Compare
|
@dschmidt thanks for the PR is it correct to say you've been using this fix in your production environment for a while to confirm it works for your use cases? |
|
Honest answer: not in production yet. We hit the gap while preparing an opencloud-eu/libre-graph-api change that flips the generator's binary types to We did verify both shapes end-to-end against a regenerated client with
|
|
thanks for sharing more. let's give it a try thank you for the contribution. |
The Go generator maps
type: string+format: binaryto*os.Fileby default. Consumers can override that with--type-mappingsto getio.Readeror[]byte.setBodyalready has branches for both, so request bodies work. The shareddecodehelper does not: it only asserts*string,*os.File, and**os.File. WhenExecutedeclaresvar localVarReturnValue io.Readeror[]byteand callsdecode(&localVarReturnValue, ...), no branch matches and the call returns"undefined response type".This PR adds two
decodebranches that close the symmetry withsetBody:bytesis already imported in every generatedclient.go. The same additive patch is applied to the eleven Go client samples.Notes on choice of type
Both mappings are useful but for different scenarios:
[]byteis honest: the payload is fully buffered in memory anyway (the caller pre-slurps the response body for shared error/success handling). Callers get the bytes directly with no wrapper.io.Readeris forward-compatible: if the response path is ever refactored to skip the eager buffering and stream the body, the same return type stays valid for consumers without changing call sites.Neither offers true streaming today; that would require changes to
api.mustacheand is out of scope here.Verification
Generated the libre-graph-api spec with the type mappings against this branch and ran
httptest-backed integration tests covering both shapes:io.Readerupload (plainio.Reader,*os.Filefor backward compat, 1 MiBbytes.Reader),io.Readerdownload, empty-body download.[]byteupload,[]bytedownload, empty-body download.All pass.
PR checklist
./mvnw clean packagewas not run locally (no JDK), but the change is purely additive and isolated to one block indecode. No other template, generator, or model output is affected.master.cc @antihax @grokify @kemokemo @jirikuncar @ph4r5h4d @lwj5