feat(openapi3conv): canonicalization pass for 3.0 -> 3.x#1162
Conversation
Adds a new package that rewrites the schema-level constructs whose
representation changed between OpenAPI 3.0 and 3.1, in place. The
transformation is mechanical and lossless — every 3.0 construct has a
direct 3.1 form. Use this when a downstream consumer (diff tools,
validators, code generators) needs a single canonical representation
regardless of the source spec's declared version.
Transformations (per the OAI upgrade guide):
openapi: 3.0.x -> openapi: 3.1.1 (configurable)
nullable: true + type: T -> type: [T, "null"]
nullable: true + type: [T, U] -> type: [T, U, "null"] (deduped)
nullable: true (no type) -> drop nullable
minimum: x + exclusiveMinimum: true -> exclusiveMinimum: x; minimum cleared
maximum: x + exclusiveMaximum: true -> exclusiveMaximum: x; maximum cleared
exclusiveMinimum: false / exclusiveMaximum: false -> dropped (default)
Schema example: v -> examples: [..., v]
API:
func UpgradeTo31(doc *openapi3.T) error
func UpgradeTo31WithOptions(doc *openapi3.T, opts UpgradeOptions) error
func UpgradeSchema(s *openapi3.Schema) // sub-tree variant
type UpgradeOptions struct {
SkipVersionBump bool // leave doc.OpenAPI alone
TargetVersion string // defaults to "3.1.1"
Verbose io.Writer // log each rewrite
}
In-place mutation matches the use case (kin-openapi shares one
openapi3.T type between 3.0 and 3.1; returning a deep-copied tree
would be wasteful and error-prone for $refs and components). The walk
is cycle-safe via a visited-pointer set.
Tests cover all transformations, idempotence on already-3.1 docs,
operation-tree walking, cycle safety, verbose logging, and nil safety.
Out of scope for this PR (lower priority, can land later):
- File-upload normalization (format: byte / format: binary)
- $schema dialect declaration on Schema Objects
- Direct 2.0 -> 3.1 (compose openapi2conv.ToV3 then this package)
The CI's goimports-reviser pass requires kin-openapi's own imports to sit in their own block separated from third-party ones.
Renames UpgradeTo31 to Upgrade and lifts the target version into the options struct, so callers can target 3.1, 3.2, or any future 3.x where representational rewrites can land in this package. UpgradeTo31 stays as a convenience wrapper. OpenAPI 3.2 introduced no breaking changes over 3.1 (purely additive: structured tags, streaming media types, additionalOperations, OAuth device flow, defaultMapping in discriminator). So 3.1 -> 3.2 is just a version-string change with no rewrites; the existing 3.0 -> 3.1 canonicalization is sufficient to reach a clean 3.2 form. Cross-major upgrades (3 -> 4 if/when v4 ships) are explicitly rejected with an error pointing at the openapi2conv pattern as the right home for that work. Same for downgrades and unparseable version strings. New tests cover: - TestUpgrade_CustomTarget — non-default 3.1.0 - TestUpgrade_TargetIs32 — 3.0 input, 3.2 target, rewrites still applied - TestUpgrade_SkipVersionBump — leaves doc.OpenAPI alone - TestUpgrade_RejectsCrossMajor — 4.0.0 target errors clearly - TestUpgrade_RejectsDowngrade — 3.1 -> 3.0 errors - TestUpgrade_RejectsInvalidVersion — non-semver target errors Updates package doc to spell out the in-scope/out-of-scope boundary (3.x ↔ 3.x in scope; cross-major out of scope; downgrades out of scope).
|
Thanks for the careful review, Pierre. Quick summary of how I'm planning to handle each:
Will reply inline on each. |
Five of the six items from getkin#1162's review: - logf: use Fprintln(w) for the trailing newline instead of editing the format string (line 135). - walkDoc: drop the superfluous nil-before-Map() check (line 184) — the Map() helper already handles nil Paths internally. - rewriteNullable: replace the hand-rolled "already has null" loop with slices.Contains (line 316). - rewriteNullable: simplify the type-array append (line 320). The earlier copy-then-append guarded against caller-side aliasing of *s.Type, but s.Type is reassigned immediately after, so the simpler in-place append is fine. - Drop SkipVersionBump from UpgradeOptions. Callers that want the representations canonicalized while keeping the original version string can do `original := doc.OpenAPI; openapi3conv.Upgrade(...); doc.OpenAPI = original` themselves — cheap escape hatch, doesn't justify a public option. Removes the corresponding test. The sixth item (drop Target / drop error return / private options) is held pending the design discussion on PR thread.
|
Pushed five of your six items in 7c84a48 (the design ones still pending the Target discussion):
|
Per the design discussion on the PR: - Drop the Target option. The OAI commits to strict compatibility for 3.x going forward (3.2.1 and 3.3.0 milestones), so a tool that handles 3.1+ correctly handles all later 3.x versions correctly too. The 3.0 → 3.1 transition is the only break and is exactly what this package bridges. Always upgrade to the latest 3.x version the package knows about (3.2.0 today; bumped via the internal latestTargetVersion const when a new minor lands). - Drop the error return. Documents must be Validate()'d before calling Upgrade; with that invariant, the only failure modes the function had (cross-major upgrade, downgrade, malformed Target) are all eliminated. - Make options private. UpgradeOptions becomes the unexported upgradeOptions, accessed via functional Option-style accessors. WithWriter is the only initial option; future options are added by introducing a new WithX function rather than expanding a public struct. - Drop UpgradeTo31 (no longer adds anything over Upgrade) and DefaultTargetVersion (latestTargetVersion is the internal replacement). - Drop parseVersion (only used by the cross-major / downgrade checks that are gone) and the strconv/strings imports. - Update package docstring to reflect the new always-latest model and the OAI compatibility commitment. Tests rewritten to match the new API: the Target / error-path tests are removed (no longer reachable), TestUpgradeTo31_* renamed to TestUpgrade_*, the cycle test uses time.After for an actual timeout, and the verbose test uses WithWriter. Closes the design thread on PR getkin#1162.
|
Pushed the rest in 32160b0. New API: openapi3conv.Upgrade(doc, openapi3conv.WithWriter(w)) // optionalAlways upgrades to the latest 3.x version the package knows about ( Tests rewritten to match: dropped the All your line-93 asks are now in. Re-requesting review. |
fenollp
left a comment
There was a problem hiding this comment.
LGTM.
Small point: in your tests you may want to ensure each document verifies .Validate(..) before and after .Upgrade(..). Also, you could add that property to the TestV3ApisGuruOpenapiDirectory test if you'd like.
Per review feedback: most Upgrade tests now use upgradeAndAssertValid which calls .Validate() on both sides — input must validate as 3.0, output must validate as latest 3.x. Two tests opt out (and document why) because they intentionally feed a 3.0-stamped doc that contains 3.1-only constructs to exercise the rewrite logic.
|
Thanks for the approval! Pushed 56035f5 addressing your small point — most Upgrade tests now use a Skipped wiring this into Ready when you are. |
|
Cool! Ready now:)
Yes I'd like that. I'll try it myself. |
Motivation
kin-openapi parses both OpenAPI 3.0 and 3.1, sharing one
openapi3.Ttype. Schema-level constructs serialize differently between the two versions but represent the same semantics:nullable: true(3.0) ↔type: ["string", "null"](3.1)minimum: 5, exclusiveMinimum: true(3.0) ↔exclusiveMinimum: 5(3.1)example: v(3.0) ↔examples: [v](3.1)Downstream consumers that need a single canonical form (diff tools, code generators, validators) currently have to special-case both representations or accept inconsistent output. In the JS/Python ecosystems this is a solved problem with multiple tools — apiture/openapi-down-convert (3.1 → 3.0), openapi-format (Node, upgrades 3.0 → 3.2), Scalar's openapi-upgrader, openapi-downgrade (Python), openapi-spec-converter. In Go (kin-openapi), the equivalent slot — paralleling the existing
openapi2conv— is empty.This PR fills that slot.
Scope
additionalOperations, OAuth device flow,defaultMapping), so 3.1 → 3.2 is a version-string change with no further rewrites.openapi2conv. The package errors out explicitly with a message pointing at that pattern.What it does
The 3.0 → 3.1 direction is mechanical and lossless: every 3.0 construct has a direct 3.1 form per the OAI upgrade guide. The package walks the schema graph and rewrites in place.
openapi: 3.0.xopenapi: 3.1.1(or any 3.x viaTarget)nullable: true+type: Ttype: [T, "null"]nullable: true+type: [T, U]type: [T, U, "null"](deduped)nullable: true(notype)nullableminimum: x+exclusiveMinimum: trueexclusiveMinimum: x;minimumclearedmaximum: x+exclusiveMaximum: trueexclusiveMaximum: x;maximumclearedexclusiveMinimum: false/exclusiveMaximum: falseexample: vexamples: [..., v]API
In-place mutation (vs returning a new doc à la
openapi2conv.ToV3) is intentional: kin-openapi already shares oneopenapi3.Ttype between 3.0, 3.1, and 3.2, so no type conversion is happening — only field rewrites. Deep-copying the entire tree just to leave the input untouched would be wasteful and error-prone for refs and components.The walk is cycle-safe via a
map[*openapi3.Schema]struct{}visited set.Tests
24 tests covering all transformations, idempotence, custom targets including 3.2, the cross-major / downgrade / bad-version error paths, operation-tree walking (parameters, request bodies, responses, callbacks), cycle safety, verbose logging, and nil safety.
Full
go test ./...is green; no changes outside the new package.Open questions
Target— currently3.1.1(the OAI guide uses this). Open to3.1.0if you prefer the floor.openapi3convparallelsopenapi2conv. The package's purpose is upgrading within 3.x rather than between major versions, so the name is slightly off-key. Open to alternatives.Out of scope
Lower-priority items I deliberately did not include:
format: byte→contentEncoding: base64, multipartformat: binary→contentMediaType). Context-dependent on media type and rarely the bottleneck.$schemadialect declaration on Schema Objects. Opt-in enhancement, not a representational difference.openapi2conv.ToV3thenopenapi3conv.Upgrade.Happy to address any of these in follow-ups if there's interest.