FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix: Refer to external refs correctly in strict interfaces by jamietanna · Pull Request #1387 · oapi-codegen/oapi-codegen · GitHub

Fix: Refer to external refs correctly in strict interfaces - #1387

Merged
jamietanna merged 3 commits into
masterfrom
feature/external-ref-strict
Jan 25, 2024
Merged

Fix: Refer to external refs correctly in strict interfaces#1387
jamietanna merged 3 commits into
masterfrom
feature/external-ref-strict

Conversation

jamietanna commented Dec 18, 2023
edited
Loading

Copy link
Copy Markdown
Member

The current approach doesn't often work when generating types that are
in external packages, unless that package is also a strict server's
definition, which isn't always the case.


Noticed while working on #1378

jamietanna marked this pull request as draft December 18, 2023 12:14
jamietanna force-pushed the feature/external-ref-strict branch from 875db16 to 71813ff Compare December 18, 2023 12:22
jamietanna marked this pull request as ready for review December 18, 2023 12:22
jamietanna force-pushed the feature/external-ref-strict branch from 71813ff to 520ad55 Compare December 18, 2023 12:33
The current approach doesn't often work when generating types that are
in external packages, unless that package is also a strict server's
definition, which isn't always the case.
As part of follow-up changes, we'll need it, and it's handy to keep this
consistent with other types that have this.
In the case that we're referencing a type defined in an external
package, using an alias here results in build errors i.e.:

  ./api.gen.go:93:7: cannot define new methods on non-local type common.TypeNameHere

Instead, we can make sure we only alias when there's not an external
reference in use, using the new `Schema.IsExternalRef` function.
jamietanna force-pushed the feature/external-ref-strict branch from 520ad55 to 1ef78fc Compare January 19, 2024 10:57
jamietanna added bug Something isn't working area:external-refs Anything related to external references (referencing other file(s) within a spec) labels Jan 25, 2024
jamietanna merged commit 390058d into master Jan 25, 2024
jamietanna deleted the feature/external-ref-strict branch May 5, 2025 16:24
mromaszewicz added a commit that referenced this pull request Apr 30, 2026
) (#2357)

PR #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
#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 #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 #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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:external-refs Anything related to external references (referencing other file(s) within a spec) bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant


Back | FazBrowse Home | New Git URL