Skip to content

fix: #1905 handle oneOf in nested allOf schemas #1904

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
matgr1 wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix: #1905 handle oneOf in nested allOf schemas #1904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
eb82ae9
fix: apply OneOf merging after AllOf merging; improve dupe typename l…
Feb 14, 2025
ec10948
fix: run tidy/test/generate/lint
Feb 14, 2025
ca3d45b
fix: oapi-codegen#1905 tag with ticket
Feb 14, 2025
0fd6016
fix: reference ticket in test
Feb 14, 2025
b01d869
Merge remote-tracking branch 'upstream/main' into fix-one-of-nesting
Feb 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions examples/authenticated-api/stdhttp/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ require (
github.com/dprotaso/go-yit v0.0.0-20220510233725-9ba8df137936 // indirect
github.com/go-openapi/jsonpointer v0.21.0 // indirect
github.com/go-openapi/swag v0.23.0 // indirect
github.com/go-test/deep v1.0.8 // indirect
github.com/goccy/go-json v0.10.2 // indirect
github.com/gorilla/mux v1.8.1 // indirect
github.com/invopop/yaml v0.3.1 // indirect
Expand Down
1 change: 1 addition & 0 deletions examples/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ require (
github.com/go-playground/locales v0.14.1 // indirect
github.com/go-playground/universal-translator v0.18.1 // indirect
github.com/go-playground/validator/v10 v10.20.0 // indirect
github.com/go-test/deep v1.0.8 // indirect
github.com/goccy/go-json v0.10.2 // indirect
github.com/golang-jwt/jwt v3.2.2+incompatible // indirect
github.com/golang/snappy v0.0.4 // indirect
Expand Down
1 change: 1 addition & 0 deletions examples/minimal-server/stdhttp/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ require (
github.com/getkin/kin-openapi v0.128.0 // indirect
github.com/go-openapi/jsonpointer v0.21.0 // indirect
github.com/go-openapi/swag v0.23.0 // indirect
github.com/go-test/deep v1.0.8 // indirect
github.com/invopop/yaml v0.3.1 // indirect
github.com/josharian/intern v1.0.0 // indirect
github.com/mailru/easyjson v0.7.7 // indirect
Expand Down
1 change: 1 addition & 0 deletions examples/petstore-expanded/stdhttp/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ require (
github.com/dprotaso/go-yit v0.0.0-20220510233725-9ba8df137936 // indirect
github.com/go-openapi/jsonpointer v0.21.0 // indirect
github.com/go-openapi/swag v0.23.0 // indirect
github.com/go-test/deep v1.0.8 // indirect
github.com/google/uuid v1.4.0 // indirect
github.com/gorilla/mux v1.8.1 // indirect
github.com/invopop/yaml v0.3.1 // indirect
Expand Down
1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ go 1.21.0

require (
github.com/getkin/kin-openapi v0.128.0
github.com/go-test/deep v1.0.8
github.com/speakeasy-api/openapi-overlay v0.9.0
github.com/stretchr/testify v1.10.0
golang.org/x/text v0.20.0
Expand Down
1 change: 1 addition & 0 deletions internal/test/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ require (
github.com/go-playground/locales v0.14.1 // indirect
github.com/go-playground/universal-translator v0.18.1 // indirect
github.com/go-playground/validator/v10 v10.14.1 // indirect
github.com/go-test/deep v1.0.8 // indirect
github.com/goccy/go-json v0.10.2 // indirect
github.com/golang-jwt/jwt v3.2.2+incompatible // indirect
github.com/golang/snappy v0.0.4 // indirect
Expand Down
1 change: 1 addition & 0 deletions internal/test/strict-server/stdhttp/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ require (
github.com/dprotaso/go-yit v0.0.0-20220510233725-9ba8df137936 // indirect
github.com/go-openapi/jsonpointer v0.21.0 // indirect
github.com/go-openapi/swag v0.23.0 // indirect
github.com/go-test/deep v1.0.8 // indirect
github.com/google/uuid v1.4.0 // indirect
github.com/invopop/yaml v0.3.1 // indirect
github.com/josharian/intern v1.0.0 // indirect
Expand Down
13 changes: 11 additions & 2 deletions pkg/codegen/codegen.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"time"

"github.com/getkin/kin-openapi/openapi3"
"github.com/go-test/deep"
"golang.org/x/tools/imports"

"github.com/oapi-codegen/oapi-codegen/v2/pkg/util"
Expand Down Expand Up @@ -719,8 +720,16 @@ func GenerateTypes(t *template.Template, types []TypeDefinition) (string, error)
continue
}
// We want to create an error when we try to define the same type twice.
return "", fmt.Errorf("duplicate typename '%s' detected, can't auto-rename, "+
"please use x-go-name to specify your own name for one of them", typ.TypeName)
diff := deep.Equal(prevType.Schema.OAPISchema, typ.Schema.OAPISchema)
return "", fmt.Errorf(
"duplicate typename '%s', can't auto-rename, "+
"please use x-go-name to specify your own name for one of them"+
"\n\tpath1:\n\t\t%s\n\tpath2:\n\t\t%s\n\tdiff:\n\t\t%s",
typ.TypeName,
prevType.JsonName,
typ.JsonName,
strings.Join(diff, "\n\t\t"),
)
}

m[typ.TypeName] = typ
Expand Down
29 changes: 19 additions & 10 deletions pkg/codegen/merge_schemas.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func mergeSchemas(allOf []*openapi3.SchemaRef, path []string) (Schema, error) {
if err != nil {
return Schema{}, err
}
schema, err = mergeOpenapiSchemas(schema, oneOfSchema, true)
schema, err = mergeOpenapiSchemas(schema, oneOfSchema)
if err != nil {
return Schema{}, fmt.Errorf("error merging schemas for AllOf: %w", err)
}
Expand Down Expand Up @@ -74,7 +74,7 @@ func mergeAllOf(allOf []*openapi3.SchemaRef) (openapi3.Schema, error) {
var schema openapi3.Schema
for _, schemaRef := range allOf {
var err error
schema, err = mergeOpenapiSchemas(schema, *schemaRef.Value, true)
schema, err = mergeOpenapiSchemas(schema, *schemaRef.Value)
if err != nil {
return openapi3.Schema{}, fmt.Errorf("error merging schemas for AllOf: %w", err)
}
Expand All @@ -84,7 +84,7 @@ func mergeAllOf(allOf []*openapi3.SchemaRef) (openapi3.Schema, error) {

// mergeOpenapiSchemas merges two openAPI schemas and returns the schema
// all of whose fields are composed.
func mergeOpenapiSchemas(s1, s2 openapi3.Schema, allOf bool) (openapi3.Schema, error) {
func mergeOpenapiSchemas(s1, s2 openapi3.Schema) (openapi3.Schema, error) {
var result openapi3.Schema

result.Extensions = make(map[string]interface{})
Expand All @@ -96,10 +96,13 @@ func mergeOpenapiSchemas(s1, s2 openapi3.Schema, allOf bool) (openapi3.Schema, e
result.Extensions[k] = v
}

result.OneOf = append(s1.OneOf, s2.OneOf...)

// We are going to make AllOf transitive, so that merging an AllOf that
// contains AllOf's will result in a flat object.
// TODO: this does not account for other schema fields at the same level as
// an AllOf - they will be lost, see also where MergeSchemas is used
// in GenerateGoSchema (all fields from the parent are disregarded
// and instead only the schema containing the merged AllOfs is
// returned)
var err error
if s1.AllOf != nil {
var merged openapi3.Schema
Expand All @@ -120,6 +123,17 @@ func mergeOpenapiSchemas(s1, s2 openapi3.Schema, allOf bool) (openapi3.Schema, e

result.AllOf = append(s1.AllOf, s2.AllOf...)

// NOTE: this must happen _after_ AllOf merging above or OneOfs can be lost, for example:
// SchemaFoo: {allOf: [{allOf: [{oneOf: [A, B]}], SchemaBar]}
// the resulting schema will only include SchemaBar if OneOf merging is done before
// AllOf recursion - this is because we won't have any deeply nested OneOfs flattened
// into s1/s2 yet at that point
// TODO: this does not accurately merge OneOf/AnyOf, for example if an AllOf contains 2
// OneOf schema children, the result should require one from each set, but this will
// require only one from the combined set
result.OneOf = append(s1.OneOf, s2.OneOf...)
result.AnyOf = append(s1.AnyOf, s2.AnyOf...)

if s1.Type.Slice() != nil && s2.Type.Slice() != nil && !equalTypes(s1.Type, s2.Type) {
return openapi3.Schema{}, fmt.Errorf("can not merge incompatible types: %v, %v", s1.Type.Slice(), s2.Type.Slice())
}
Expand Down Expand Up @@ -222,11 +236,6 @@ func mergeOpenapiSchemas(s1, s2 openapi3.Schema, allOf bool) (openapi3.Schema, e
}
}

// Allow discriminators for allOf merges, but disallow for one/anyOfs.
if !allOf && (s1.Discriminator != nil || s2.Discriminator != nil) {
return openapi3.Schema{}, errors.New("merging two schemas with discriminators is not supported")
}

return result, nil
}

Expand Down
Loading