fix: delegate MarshalJSON on strict response types wrapping union refs#2332
Conversation
Greptile SummaryThis PR fixes a long-standing bug (#970, #1665) where strict-server response types that are named defined types wrapping a Confidence Score: 5/5Safe to merge; the fix is correct, well-scoped, and all findings are P2 suggestions. No P0 or P1 issues found. The generated code is correct (defined type + delegating methods), the fix is applied consistently across all backend templates, and the regression tests verify the critical path. Only P2 suggestions remain. No files require special attention; the P2 suggestions are in pkg/codegen/schema.go and internal/test/issues/issue-970/config.yaml.
|
| Filename | Overview |
|---|---|
| pkg/codegen/schema.go | Adds HasCustomMarshalJSON() helper; logic is correct but doesn't guard against the IsRef+alias edge case |
| pkg/codegen/templates/strict/strict-interface.tmpl | Adds delegating MarshalJSON/UnmarshalJSON on per-operation response types wrapping union refs; used by stdhttp/chi/gorilla/echo/gin/echo5 backends |
| pkg/codegen/templates/strict/strict-fiber-interface.tmpl | Same fix applied for Fiber backend; not directly tested by the new regression test |
| pkg/codegen/templates/strict/strict-iris-interface.tmpl | Same fix applied for Iris backend; not directly tested by the new regression test |
| pkg/codegen/templates/strict/strict-responses.tmpl | Same fix applied for generic components/responses entries; this path is not exercised by the new test (spec has no components/responses) |
| internal/test/issues/issue-970/issue970.gen.go | Generated file looks correct: GetEvent200JSONResponse is a defined type (not alias) with proper delegating MarshalJSON/UnmarshalJSON |
| internal/test/issues/issue-970/issue970_test.go | Well-structured regression tests covering marshal, Visit (end-to-end), and roundtrip; only covers std-http backend |
Reviews (2): Last reviewed commit: "fix: delegate MarshalJSON on strict resp..." | Re-trigger Greptile
When a strict-server response content schema is a $ref to a oneOf/anyOf
union, the generated response type (e.g. `type X200JSONResponse Event`)
is a named defined type and does not inherit Event's MarshalJSON, so
json.Encode emits {} for the union's unexported raw message.
Generate delegating MarshalJSON/UnmarshalJSON on the response type when
the underlying schema is a ref to a union. Inline unions continue to
work via the existing $hasUnionElements template branch.
Fixes oapi-codegen#970.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mromaszewicz
force-pushed
the
fix/issue-970
branch
from
b691f36 to
c807212
Compare
|
@mromaszewicz do you have an idea when the next release will be so we can start using this? |
|
This is going into the next release, v2.7.0. I'm hoping to get it out in a week or two. Can you temporarily pin your usage to the top SHA in the main branch? I'm going to merge a few more commits, then go through a big PR/Issue scrub and decide when 2.7.0 is good enough. |
2 participants
When a strict-server response content schema is a $ref to a oneOf/anyOf union, the generated response type (e.g.
type X200JSONResponse Event) is a named defined type and does not inherit Event's MarshalJSON, so json.Encode emits {} for the union's unexported raw message.Generate delegating MarshalJSON/UnmarshalJSON on the response type when the underlying schema is a ref to a union. Inline unions continue to work via the existing $hasUnionElements template branch.
Fixes #970.
Fixes #1665.