Update external-refs to responses in strict-server to fix an old regression#2357
Conversation
…pi-codegen#2010) PR oapi-codegen#1387 added an `$isExternalRef` branch to the strict-{,fiber-,iris-} interface templates that strips the `<NameTag>Response` suffix when embedding an external response ref. That made external refs to a `components/responses/...` resolve to the bare schema name (`N400`) instead of the strict envelope (`N400JSONResponse`). The result: when spec A and spec B both generate strict-server and A $refs B's response component, A's local envelope embeds `N400JSONResponse` while A's external-ref envelope embeds `externalRef0.N400`. The two struct shapes are no longer identical, so cross-package response casts (the standard pattern for sharing error shapes across services) stop compiling — the regression filed as oapi-codegen#2010. Investigation showed there is no smarter alternative: non-strict server modes emit no top-level type for `components/responses/...`, only `models: true` (gives the bare alias) and `strict-server: true` (gives the `<Name>JSONResponse` envelope, which is also the only form that carries a `Headers` field in the with-headers case) do. Changes: - Drop the `$isExternalRef` carve-out from the three strict-interface templates so external refs use the same `<Name>JSONResponse` embedding as internal refs. - Update `internal/test/issues/issue-removed-external-ref` golden output to match. - Update `internal/test/issues/issue-2113`'s common-package config to also generate `strict-server: true`. The fixture was relying on the PR oapi-codegen#1387 behavior; under the new policy the destination of a strict-server external ref must also generate a strict server, so `StandardErrorJSONResponse` is in scope. - Add `internal/test/issues/issue-2010` regression fixture: two specs with strict-server, the second `$ref`s the first's `components/responses/400`, and the test exercises the cross-package cast that was broken. - README: note the cross-spec strict-server requirement under the strict-server section. The earlier two commits of oapi-codegen#1387 are kept: the `Schema.IsExternalRef` helper, and the alias-vs-defined-type fix for content-schema external refs (which is a genuinely independent bug fix — methods can't be attached to non-local aliases). BREAKING CHANGE: external `$ref` to a `components/responses/...` from a strict-server target now requires the destination spec to also generate `strict-server: true`. This restores cross-package response casting that worked in v2.0.0 and earlier. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR reverts the Confidence Score: 4/5Safe to merge; the template fix is minimal and correct, the regression test exercises the exact broken path, and all golden files are updated consistently. No logic bugs or security issues found. The behavior change is unconditional (per repo rule 3, changes that alter generated output should be opt-in), but the PR makes a strong case that opt-in is infeasible here and the new behavior restores v2.0.0 semantics. All fixture files and template changes are consistent across the three interface backends. No files require special attention; all generated golden files match the template change and the regression test is well-structured.
|
| Filename | Overview |
|---|---|
| pkg/codegen/templates/strict/strict-interface.tmpl | Removes the $isExternalRef carve-out branch for content-bearing responses so external refs use the same <Name>JSONResponse embedding as internal refs; the variable is still referenced in the no-content path (line 200) so no dead variable. |
| pkg/codegen/templates/strict/strict-fiber-interface.tmpl | Same two-line removal as strict-interface.tmpl, applied to the Fiber backend interface template. Consistent with the other interface templates. |
| pkg/codegen/templates/strict/strict-iris-interface.tmpl | Same two-line removal as strict-interface.tmpl, applied to the Iris backend interface template. Consistent with the other interface templates. |
| internal/test/issues/issue-2010/issue_test.go | New regression test that verifies cross-package response casts compile and execute correctly when both specs use strict-server; the var _ = func(...) pattern is an idiomatic compile-time check. |
| internal/test/issues/issue-2113/gen/common/common.gen.go | Adds type StandardErrorJSONResponse ProblemDetails (a defined type, not an alias) generated because strict-server: true was added to config.common.yaml; used as the embedded field in the api package's ListThings400JSONResponse. |
| internal/test/issues/issue-2113/gen/api/api.gen.go | Updates ListThings400JSONResponse to embed externalRef0.StandardErrorJSONResponse instead of externalRef0.StandardError; the VisitListThingsResponse method on this struct correctly handles JSON encoding of the embedded fields. |
| internal/test/issues/issue-removed-external-ref/gen/spec_base/issue.gen.go | Golden file updated: PostInvalidExtRefTrouble300JSONResponse now embeds externalRef0.PascalJSONResponse instead of externalRef0.Pascal, matching the template fix. |
| internal/test/issues/issue-2113/config.common.yaml | Adds strict-server: true so the common package generates StandardErrorJSONResponse, which is required by the api package after the template change. |
| internal/test/issues/issue-2010/gen/spec_other/issue.gen.go | New fixture: GetOtherExample400JSONResponse embeds externalRef0.N400JSONResponse (the strict envelope from spec_base), enabling the cross-package cast tested in issue_test.go. |
| README.md | Adds a clear IMPORTANT callout documenting the new requirement that destination specs must also set strict-server: true when referenced via external $ref. |
Reviews (1): Last reviewed commit: "revert external-ref carve-out in strict-..." | Re-trigger Greptile
Closes: #2010
PR #1387 added an
$isExternalRefbranch to the strict-{,fiber-,iris-} interface templates that strips the<NameTag>Responsesuffix when embedding an external response ref. That made external refs to acomponents/responses/...resolve to the bare schema name (N400) instead of the strict envelope (N400JSONResponse).The result: when spec A and spec B both generate strict-server and A $refs B's response component, A's local envelope embeds
N400JSONResponsewhile A's external-ref envelope embedsexternalRef0.N400. The two struct shapes are no longer identical, so cross-package response casts (the standard pattern for sharing error shapes across services) stop compiling — the regression filed as #2010.Investigation showed there is no smarter alternative: non-strict server modes emit no top-level type for
components/responses/..., onlymodels: true(gives the bare alias) andstrict-server: true(gives the<Name>JSONResponseenvelope, which is also the only form that carries aHeadersfield in the with-headers case) do.Changes:
$isExternalRefcarve-out from the three strict-interface templates so external refs use the same<Name>JSONResponseembedding as internal refs.internal/test/issues/issue-removed-external-refgolden output to match.internal/test/issues/issue-2113's common-package config to also generatestrict-server: true. The fixture was relying on the PR Fix: Refer to external refs correctly in strict interfaces #1387 behavior; under the new policy the destination of a strict-server external ref must also generate a strict server, soStandardErrorJSONResponseis in scope.internal/test/issues/issue-2010regression fixture: two specs with strict-server, the second$refs the first'scomponents/responses/400, and the test exercises the cross-package cast that was broken.The earlier two commits of #1387 are kept: the
Schema.IsExternalRefhelper, and the alias-vs-defined-type fix for content-schema external refs (which is a genuinely independent bug fix — methods can't be attached to non-local aliases).BREAKING CHANGE: external
$refto acomponents/responses/...from a strict-server target now requires the destination spec to also generatestrict-server: true. This restores cross-package response casting that worked in v2.0.0 and earlier.