diff --git a/configuration-schema.json b/configuration-schema.json index 521b251ad9..8f902ca708 100644 --- a/configuration-schema.json +++ b/configuration-schema.json @@ -76,6 +76,10 @@ "type": "boolean", "description": "In the past, we merged schemas for `allOf` by inlining each schema within the schema list. This approach, though, is incorrect because `allOf` merges at the schema definition level, not at the resulting model level. So, new behavior merges OpenAPI specs but generates different code than we have in the past. Set OldMergeSchemas to true for the old behavior. Please see https://github.com/oapi-codegen/oapi-codegen/issues/531" }, + "old-allof-sibling-merging": { + "type": "boolean", + "description": "In the past, when a schema combined `allOf` with sibling fields at the same level (`properties`, `required`, `additionalProperties`, `description`), those siblings were silently discarded and the schema was emitted as a Go type alias to its sole `allOf` target. New behavior merges the parent's siblings with the `allOf` members so the generated type carries every field declared in the spec. This is a more accurate translation of OpenAPI semantics, but it changes the shape of generated types: a schema that previously produced `type X = Y` may now produce a distinct struct embedding Y with extra fields, which is not interchangeable with Y in downstream Go code. Set OldAllOfSiblingMerging to true to restore the prior behavior. Please see https://github.com/oapi-codegen/oapi-codegen/issues/697" + }, "old-enum-conflicts": { "type": "boolean", "description": "Enum values can generate conflicting typenames, so we've updated the code for enum generation to avoid these conflicts, but it will result in some enum types being renamed in existing code. Set OldEnumConflicts to true to revert to old behavior. Please see: Please see https://github.com/oapi-codegen/oapi-codegen/issues/549" diff --git a/internal/test/all_of/openapi.yaml b/internal/test/all_of/openapi.yaml index f73a8e5f1b..e1c780752f 100644 --- a/internal/test/all_of/openapi.yaml +++ b/internal/test/all_of/openapi.yaml @@ -51,3 +51,14 @@ components: type: integer format: int64 required: [ ID ] + PersonWithMorePropertiesOutsideOfAllOf: + type: object + description: | + This is a person record as returned from a Create endpoint. It contains + all the fields of a Person, with an additional property outside of allOf directives. + properties: + additionalProperty: + type: string + required: [ additionalProperty ] + allOf: + - $ref: "#/components/schemas/Person" diff --git a/internal/test/all_of/v1/openapi.gen.go b/internal/test/all_of/v1/openapi.gen.go index 46571fcce3..2f16c10b84 100644 --- a/internal/test/all_of/v1/openapi.gen.go +++ b/internal/test/all_of/v1/openapi.gen.go @@ -15,7 +15,9 @@ import ( "github.com/getkin/kin-openapi/openapi3" ) -// Person defines model for Person. +// Person This is a person, with mandatory first and last name, but optional ID +// number. This would be returned by a `Get` style API. We merge the person +// properties with another Schema which only provides required fields. type Person struct { // Embedded struct due to allOf(#/components/schemas/PersonProperties) PersonProperties `yaml:",inline"` @@ -30,7 +32,8 @@ type PersonProperties struct { LastName *string `json:"LastName,omitempty"` } -// PersonWithID defines model for PersonWithID. +// PersonWithID This is a person record as returned from a Create endpoint. It contains +// all the fields of a Person, with an additional resource UUID. type PersonWithID struct { // Embedded struct due to allOf(#/components/schemas/Person) Person `yaml:",inline"` @@ -38,22 +41,32 @@ type PersonWithID struct { ID int64 `json:"ID"` } +// PersonWithMorePropertiesOutsideOfAllOf This is a person record as returned from a Create endpoint. It contains +// all the fields of a Person, with an additional property outside of allOf directives. +type PersonWithMorePropertiesOutsideOfAllOf struct { + // Embedded struct due to allOf(#/components/schemas/Person) + Person `yaml:",inline"` + // Embedded fields due to inline allOf schema + AdditionalProperty string `json:"additionalProperty"` +} + // Base64 encoded, compressed with deflate, json marshaled OpenAPI spec. // Stored as a slice of fixed-width chunks rather than one concatenated // const string: with thousands of chunks the chained `+` fold is several // times slower for the Go compiler than parsing a slice literal. var swaggerSpec = []string{ - "lJRPb9s4EMW/yoC7R0FOsIs96Bas20BAkRpo0hziABlLI4spNWTJUQzB8HcvSMn/4BZpfRrA5OO8N7/R", - "VlW2c5aJJahiq0LVUoepXJAPlmOFxnxuVPG0VX97alSh/podb82mK7Px/MJbR140BbXLtsrT9157qlXx", - "pD5qH+QOO1KZ+oRT+bx7zlRNofLaiY7vqftWB9ABEFySzGCjpYUOuUaxfoAmCgFyDQaDAGNHGax6AZsk", - "0EA5XzL33Yp8DkluY3tTw4rAk/SeqYbVAAgvtyQvEGQwBDeLModHgo78mkBamp5fsjt4GjtBttKShy/J", - "OWxaXbVg2QzgvH3TNQXY+4ZGk6lDvmSVKRkcqULZ1StVonaZuois2F5kQYEAPU1CIC0KBEeVboZDQtEk", - "DekYGnOIIYsZLfngvQ+Tb4aXD7U+dQ7EtbOaJYNNS56AsGrjEPZaowN31upxoMV2by6I17yO5m7tG3nu", - "iKWc36VZxGON9R2KKpRm+e/fYyiahdbk48UDG5equ1+G+KilLed/Smti9NzUKPJum7vsjO1y/jskg6fK", - "+howHDlsvO0A4X9PKHQYQw6lQGVZUHNYcpxqJHKCwDaAsDhdDmTAutYT/p6C7X1F8PBQzn/KXuxfc2NT", - "xlpM/O+eggS4ifFBSiwkPZWpN/JhdHSdX+VXMXXriNFpVah/8qv8OrKB0qYEZ85gRa019TjyNckl2F/R", - "6LTOATbIAihgKG6zZYIolUGwIDHADr9RBJ86aNG5YTQUZ4ZRrKxVoRYnT8bJBGc57Beqwd6kFmKgxKlE", - "54yuksDsdfrOjWzE6n1yJt5SkOfOTt3v0u9HAAAA//8=", + "zFXNjts2EH6VAdujIG/QogfdgroNBLRZA02aQ7zAjsWRxZQasuTIhmD43QtS8h+8bbq3+DSAOR/n++Ho", + "oBrXe8fEElV1ULHpqMdcrihEx6lCax9bVX0+qO8DtapS3y0uXYu5ZTGdXwXnKYihqI7FQQX6ezCBtKo+", + "q19NiPIee1KF+g3n8un4VChNsQnGi0n3qQ+diWAiIPgMWcDeSAc9skZxYYQ2AQGyBotRgLGnAjaDgMsQ", + "aKFerpmHfkOhhAy3d4PVsCEIJENg0rAZAeH5HckzRBktwdtVXcIngp7ClkA6mq9fsz9zmiZBdtJRgD8y", + "c9h3punAsR3BB7czmiKceENryOpYrlkVSkZPqlJu84UaUcdC3UlWHe60oEiAgWYgkA4FoqfGtONZoUSS", + "xnwMrT3LUCSN1nzmPsSZN8PzL9pcMwdi7Z1hKWDfUSAgbLpkwglrYuBvRr0YWh1O5KIEw9tE7p3bUeCe", + "WOrl++xFOta60KOoShmWn368iGJYaEshNZ6zcY96/FcRPxnp6uVr05ozektqAvnqmMfiJtv18v8kGQI1", + "LmjAeMlhG1wPCD8HQqGzDSXUAo1jQcNxzcnVlMg5BK4FhNX140AG1NrM8Q8U3RAago8f6+V/Zi/J9rsL", + "dMng4yDRaHps3046vlbQb0aE2dYR3MQoNyQqoE2gRsyO4kupvkDMoowvB/Ha/hd6nu40T02GW5fhjNj0", + "3weKEiFLDVnUmHFUoXYU4iTgm/KhfEiWOU+M3qhK/VA+lG/S5ChdHnrhLTbUOaunZ7YluV8mf6I1eYVG", + "2CMLoICltEEdEySoAqIDSX71+BelZUM9dOj9OAmVqGECq7Wq1OrqyiRH9I7jaYm1ONg8QvKPOJfovTVN", + "Blh8mb8tU3xS9fVwzW88C3nL7Jr9Mf/+CQAA//8=", } // decodeSpec returns the embedded OpenAPI spec as raw JSON bytes, diff --git a/internal/test/all_of/v2/openapi.gen.go b/internal/test/all_of/v2/openapi.gen.go index b25d5bb90a..965051eec5 100644 --- a/internal/test/all_of/v2/openapi.gen.go +++ b/internal/test/all_of/v2/openapi.gen.go @@ -15,7 +15,9 @@ import ( "github.com/getkin/kin-openapi/openapi3" ) -// Person defines model for Person. +// Person This is a person, with mandatory first and last name, but optional ID +// number. This would be returned by a `Get` style API. We merge the person +// properties with another Schema which only provides required fields. type Person struct { FirstName string `json:"FirstName"` GovernmentIDNumber *int64 `json:"GovernmentIDNumber,omitempty"` @@ -30,7 +32,8 @@ type PersonProperties struct { LastName *string `json:"LastName,omitempty"` } -// PersonWithID defines model for PersonWithID. +// PersonWithID This is a person record as returned from a Create endpoint. It contains +// all the fields of a Person, with an additional resource UUID. type PersonWithID struct { FirstName string `json:"FirstName"` GovernmentIDNumber *int64 `json:"GovernmentIDNumber,omitempty"` @@ -38,22 +41,32 @@ type PersonWithID struct { LastName string `json:"LastName"` } +// PersonWithMorePropertiesOutsideOfAllOf This is a person record as returned from a Create endpoint. It contains +// all the fields of a Person, with an additional property outside of allOf directives. +type PersonWithMorePropertiesOutsideOfAllOf struct { + FirstName string `json:"FirstName"` + GovernmentIDNumber *int64 `json:"GovernmentIDNumber,omitempty"` + LastName string `json:"LastName"` + AdditionalProperty string `json:"additionalProperty"` +} + // Base64 encoded, compressed with deflate, json marshaled OpenAPI spec. // Stored as a slice of fixed-width chunks rather than one concatenated // const string: with thousands of chunks the chained `+` fold is several // times slower for the Go compiler than parsing a slice literal. var swaggerSpec = []string{ - "lJRPb9s4EMW/yoC7R0FOsIs96Bas20BAkRpo0hziABlLI4spNWTJUQzB8HcvSMn/4BZpfRrA5OO8N7/R", - "VlW2c5aJJahiq0LVUoepXJAPlmOFxnxuVPG0VX97alSh/podb82mK7Px/MJbR140BbXLtsrT9157qlXx", - "pD5qH+QOO1KZ+oRT+bx7zlRNofLaiY7vqftWB9ABEFySzGCjpYUOuUaxfoAmCgFyDQaDAGNHGax6AZsk", - "0EA5XzL33Yp8DkluY3tTw4rAk/SeqYbVAAgvtyQvEGQwBDeLModHgo78mkBamp5fsjt4GjtBttKShy/J", - "OWxaXbVg2QzgvH3TNQXY+4ZGk6lDvmSVKRkcqULZ1StVonaZuois2F5kQYEAPU1CIC0KBEeVboZDQtEk", - "DekYGnOIIYsZLfngvQ+Tb4aXD7U+dQ7EtbOaJYNNS56AsGrjEPZaowN31upxoMV2by6I17yO5m7tG3nu", - "iKWc36VZxGON9R2KKpRm+e/fYyiahdbk48UDG5equ1+G+KilLed/Smti9NzUKPJum7vsjO1y/jskg6fK", - "+howHDlsvO0A4X9PKHQYQw6lQGVZUHNYcpxqJHKCwDaAsDhdDmTAutYT/p6C7X1F8PBQzn/KXuxfc2NT", - "xlpM/O+eggS4ifFBSiwkPZWpN/JhdHSdX+VXMXXriNFpVah/8qv8OrKB0qYEZ85gRa019TjyNckl2F/R", - "6LTOATbIAihgKG6zZYIolUGwIDHADr9RBJ86aNG5YTQUZ4ZRrKxVoRYnT8bJBGc57Beqwd6kFmKgxKlE", - "54yuksDsdfrOjWzE6n1yJt5SkOfOTt3v0u9HAAAA//8=", + "zFXNjts2EH6VAdujIG/QogfdgroNBLRZA02aQ7zAjsWRxZQasuTIhmD43QtS8h+8bbq3+DSAOR/n++Ho", + "oBrXe8fEElV1ULHpqMdcrihEx6lCax9bVX0+qO8DtapS3y0uXYu5ZTGdXwXnKYihqI7FQQX6ezCBtKo+", + "q19NiPIee1KF+g3n8un4VChNsQnGi0n3qQ+diWAiIPgMWcDeSAc9skZxYYQ2AQGyBotRgLGnAjaDgMsQ", + "aKFerpmHfkOhhAy3d4PVsCEIJENg0rAZAeH5HckzRBktwdtVXcIngp7ClkA6mq9fsz9zmiZBdtJRgD8y", + "c9h3punAsR3BB7czmiKceENryOpYrlkVSkZPqlJu84UaUcdC3UlWHe60oEiAgWYgkA4FoqfGtONZoUSS", + "xnwMrT3LUCSN1nzmPsSZN8PzL9pcMwdi7Z1hKWDfUSAgbLpkwglrYuBvRr0YWh1O5KIEw9tE7p3bUeCe", + "WOrl++xFOta60KOoShmWn368iGJYaEshNZ6zcY96/FcRPxnp6uVr05ozektqAvnqmMfiJtv18v8kGQI1", + "LmjAeMlhG1wPCD8HQqGzDSXUAo1jQcNxzcnVlMg5BK4FhNX140AG1NrM8Q8U3RAago8f6+V/Zi/J9rsL", + "dMng4yDRaHps3046vlbQb0aE2dYR3MQoNyQqoE2gRsyO4kupvkDMoowvB/Ha/hd6nu40T02GW5fhjNj0", + "3weKEiFLDVnUmHFUoXYU4iTgm/KhfEiWOU+M3qhK/VA+lG/S5ChdHnrhLTbUOaunZ7YluV8mf6I1eYVG", + "2CMLoICltEEdEySoAqIDSX71+BelZUM9dOj9OAmVqGECq7Wq1OrqyiRH9I7jaYm1ONg8QvKPOJfovTVN", + "Blh8mb8tU3xS9fVwzW88C3nL7Jr9Mf/+CQAA//8=", } // decodeSpec returns the embedded OpenAPI spec as raw JSON bytes, diff --git a/internal/test/components/components.gen.go b/internal/test/components/components.gen.go index c80b28dc4d..85680e232c 100644 --- a/internal/test/components/components.gen.go +++ b/internal/test/components/components.gen.go @@ -319,10 +319,10 @@ type Enum4 string // Enum5 Numerical enum type Enum5 int -// EnumUnion defines model for EnumUnion. +// EnumUnion Two enums of the same type combined with allOf. type EnumUnion string -// EnumUnion2 defines model for EnumUnion2. +// EnumUnion2 Two enums of the same type combined with allOf. type EnumUnion2 string // FunnyValues Edge cases for enum names @@ -371,7 +371,7 @@ type OneOfObject11_AdditionalProperties struct { union json.RawMessage } -// OneOfObject12 defines model for OneOfObject12. +// OneOfObject12 allOf of oneOfs type OneOfObject12 struct { union json.RawMessage } diff --git a/internal/test/issues/issue-1087/deps/deps.gen.go b/internal/test/issues/issue-1087/deps/deps.gen.go index 55a8d8e2f1..3661be9d81 100644 --- a/internal/test/issues/issue-1087/deps/deps.gen.go +++ b/internal/test/issues/issue-1087/deps/deps.gen.go @@ -31,7 +31,22 @@ type BaseError struct { } // Error defines model for Error. -type Error = BaseError +type Error struct { + // Code The underlying http status code + Code int32 `json:"code"` + + // Domain The domain where the error is originating from as defined by the service + Domain string `json:"domain"` + + // Message A simple message in english describing the error and can be returned to the consumer + Message string `json:"message"` + + // Metadata Any additional details to be conveyed as determined by the service. If present, will return map of key value pairs + Metadata *map[string]string `json:"metadata,omitempty"` + + // Reason A reason code specific to the service and can be used to identify the exact issue. Should be unique within a domain + Reason string `json:"reason"` +} // N401 defines model for 401. type N401 = Error diff --git a/pkg/codegen/configuration.go b/pkg/codegen/configuration.go index f0a549a897..f2cc4c0162 100644 --- a/pkg/codegen/configuration.go +++ b/pkg/codegen/configuration.go @@ -215,6 +215,19 @@ type CompatibilityOptions struct { // than we have in the past. Set OldMergeSchemas to true for the old behavior. // Please see https://github.com/oapi-codegen/oapi-codegen/issues/531 OldMergeSchemas bool `yaml:"old-merge-schemas,omitempty"` + // In the past, when a schema combined `allOf` with sibling fields at the + // same level (`properties`, `required`, `additionalProperties`, + // `description`), those siblings were silently discarded and the schema + // was emitted as a Go type alias to its sole `allOf` target. New behavior + // merges the parent's siblings with the `allOf` members so the generated + // type carries every field declared in the spec. This is a more accurate + // translation of OpenAPI semantics, but it changes the shape of generated + // types: a schema that previously produced `type X = Y` may now produce + // a distinct struct embedding Y with extra fields, which is not + // interchangeable with Y in downstream Go code. Set OldAllOfSiblingMerging + // to true to restore the prior behavior. + // Please see https://github.com/oapi-codegen/oapi-codegen/issues/697 + OldAllOfSiblingMerging bool `yaml:"old-allof-sibling-merging,omitempty"` // Enum values can generate conflicting typenames, so we've updated the // code for enum generation to avoid these conflicts, but it will result // in some enum types being renamed in existing code. Set OldEnumConflicts to true diff --git a/pkg/codegen/schema.go b/pkg/codegen/schema.go index d9941381a1..2e216537f0 100644 --- a/pkg/codegen/schema.go +++ b/pkg/codegen/schema.go @@ -403,11 +403,48 @@ func GenerateGoSchema(sref *openapi3.SchemaRef, path []string) (Schema, error) { // so that in a RESTful paradigm, the Create operation can return // (object, id), so that other operations can refer to (id) if schema.AllOf != nil { - mergedSchema, err := MergeSchemas(schema.AllOf, path) + var mergedSchema Schema + var err error + // Behavior is gated on Compatibility.OldAllOfSiblingMerging: + // when set, the parent's structural siblings and Description are + // silently discarded (the historical behavior). When unset + // (default), they are merged into the result. + mergeSiblings := !globalState.options.Compatibility.OldAllOfSiblingMerging + if mergeSiblings && hasStructuralSiblings(schema) { + // Inject the parent (with AllOf cleared) as the final allOf + // member so its structural siblings — Properties, Required, + // AdditionalProperties — are merged with the allOf members + // rather than discarded. Issues #697, #931, #1710, #2102. + // + // Allocate a fresh slice rather than appending to schema.AllOf + // directly: if kin-openapi gave us a slice with spare capacity, + // `append` would write the new element into the shared backing + // array, mutating any other view that has been extended past + // len(schema.AllOf). + s := *schema + s.AllOf = nil + allOfRefs := make([]*openapi3.SchemaRef, 0, len(schema.AllOf)+1) + allOfRefs = append(allOfRefs, schema.AllOf...) + allOfRefs = append(allOfRefs, &openapi3.SchemaRef{Value: &s}) + mergedSchema, err = MergeSchemas(allOfRefs, path) + } else { + // Either the user opted into legacy behavior, or the parent is + // a pure wrapper with no structural siblings. In the wrapper + // case, MergeSchemas' single-element fast path returns the + // referenced type unchanged, preserving named-type identity. + mergedSchema, err = MergeSchemas(schema.AllOf, path) + } if err != nil { return Schema{}, fmt.Errorf("error merging schemas: %w", err) } mergedSchema.OAPISchema = schema + // Description is metadata, not a structural constraint, so it + // doesn't go through the merge. Copy it from the parent when set. + // Issue #1960. Gated on the same compatibility flag as the + // sibling-merge above. + if mergeSiblings && schema.Description != "" { + mergedSchema.Description = schema.Description + } // x-go-type on the parent is handled by the early return above // (combined extensions). For x-go-type-skip-optional-pointer, only // override the merged value when the parent sets it explicitly — @@ -1012,3 +1049,22 @@ func combinedSchemaExtensions(r *openapi3.SchemaRef) map[string]any { return combined } + +// hasStructuralSiblings reports whether a schema with allOf also carries +// fields outside allOf that materially affect the generated Go type. +// Such fields must be merged with the allOf members rather than discarded. +// +// Description and Title are excluded — they are metadata, not structural, +// and the caller propagates them separately. Nullable/ReadOnly/WriteOnly +// are also excluded for now: their strict-equality check in +// mergeOpenapiSchemas conflates the bool zero value with "unset" and would +// regress simple wrappers like {allOf: [X-with-nullable:true]}. +func hasStructuralSiblings(s *openapi3.Schema) bool { + if s == nil { + return false + } + return len(s.Properties) > 0 || + len(s.Required) > 0 || + s.AdditionalProperties.Has != nil || + s.AdditionalProperties.Schema != nil +}