From ba6c5977df4b1af2ce49744f92a3ef7a20e87474 Mon Sep 17 00:00:00 2001 From: Kotaro Inoue Date: Tue, 2 Jan 2024 23:49:44 +0900 Subject: [PATCH 1/5] Respect parent properties in merging allOf schemas Signed-off-by: Kotaro Inoue --- pkg/codegen/schema.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/codegen/schema.go b/pkg/codegen/schema.go index f0054a8671..cbd8af8b8f 100644 --- a/pkg/codegen/schema.go +++ b/pkg/codegen/schema.go @@ -281,7 +281,14 @@ 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) + // deepcopy and clear allOf to avoid circular references + s := *schema + s.AllOf = nil + + // merge parent schema with allOf schemas to include parent properties (Issue #697) + allOfRefs := append(schema.AllOf, &openapi3.SchemaRef{Value: &s}) + + mergedSchema, err := MergeSchemas(allOfRefs, path) if err != nil { return Schema{}, fmt.Errorf("error merging schemas: %w", err) } From cb9261297c0267c0e846aa0ab233f563b8f89f8a Mon Sep 17 00:00:00 2001 From: Kotaro Inoue Date: Wed, 3 Jan 2024 00:07:43 +0900 Subject: [PATCH 2/5] Add test case to check if parent propeties are included in the merged schema Signed-off-by: Kotaro Inoue --- internal/test/all_of/openapi.yaml | 11 +++++++++ internal/test/all_of/v1/openapi.gen.go | 33 +++++++++++++++++--------- internal/test/all_of/v2/openapi.gen.go | 31 +++++++++++++++--------- 3 files changed, 53 insertions(+), 22 deletions(-) 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 db5fd302cd..685ae48424 100644 --- a/internal/test/all_of/v1/openapi.gen.go +++ b/internal/test/all_of/v1/openapi.gen.go @@ -20,6 +20,7 @@ type Person struct { // Embedded struct due to allOf(#/components/schemas/PersonProperties) PersonProperties `yaml:",inline"` // Embedded fields due to inline allOf schema + // Embedded fields due to inline allOf schema } // PersonProperties These are fields that specify a person. They are all optional, and @@ -36,22 +37,32 @@ type PersonWithID struct { Person `yaml:",inline"` // Embedded fields due to inline allOf schema ID int64 `json:"ID"` + // Embedded fields due to inline allOf schema +} + +// PersonWithMorePropertiesOutsideOfAllOf defines model for PersonWithMorePropertiesOutsideOfAllOf. +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, gzipped, json marshaled Swagger object var swaggerSpec = []string{ - "H4sIAAAAAAAC/5SUT2/bOBDFv8qAu0dBTrCLPegWrNtAQJEaaNIc4gAZSyOLKTVkyVEMwfB3L0jJ/+AW", - "aX0awOTjvDe/0VZVtnOWiSWoYqtC1VKHqVyQD5ZjhcZ8blTxtFV/e2pUof6aHW/Npiuz8fzCW0deNAW1", - "y7bK0/dee6pV8aQ+ah/kDjtSmfqEU/m8e85UTaHy2omO76n7VgfQARBcksxgo6WFDrlGsX6AJgoBcg0G", - "gwBjRxmsegGbJNBAOV8y992KfA5JbmN7U8OKwJP0nqmG1QAIL7ckLxBkMAQ3izKHR4KO/JpAWpqeX7I7", - "eBo7QbbSkocvyTlsWl21YNkM4Lx90zUF2PuGRpOpQ75klSkZHKlC2dUrVaJ2mbqIrNheZEGBAD1NQiAt", - "CgRHlW6GQ0LRJA3pGBpziCGLGS354L0Pk2+Glw+1PnUOxLWzmiWDTUuegLBq4xD2WqMDd9bqcaDFdm8u", - "iNe8juZu7Rt57oilnN+lWcRjjfUdiiqUZvnv32MomoXW5OPFAxuXqrtfhviopS3nf0prYvTc1Cjybpu7", - "7Iztcv47JIOnyvoaMBw5bLztAOF/Tyh0GEMOpUBlWVBzWHKcaiRygsA2gLA4XQ5kwLrWE/6egu19RfDw", - "UM5/yl7sX3NjU8ZaTPzvnoIEuInxQUosJD2VqTfyYXR0nV/lVzF164jRaVWof/Kr/DqygdKmBGfOYEWt", - "NfU48jXJJdhf0ei0zgE2yAIoYChus2WCKJVBsCAxwA6/UQSfOmjRuWE0FGeGUaysVaEWJ0/GyQRnOewX", - "qsHepBZioMSpROeMrpLA7HX6zo1sxOp9cibeUpDnzk7d79LvRwAAAP//lzc18GUFAAA=", + "H4sIAAAAAAAC/8xVzY7bNhB+lQHboyBv0KIH3YK6DQS0WQNNmkO8wI7FkcWUGrLkyIZg+N0LUvIfvG26", + "t/g0gDkf5/vh6KAa13vHxBJVdVCx6ajHXK4oRMepQmsfW1V9PqjvA7WqUt8tLl2LuWUxnV8F5ymIoaiO", + "xUEF+nswgbSqPqtfTYjyHntShfoN5/Lp+FQoTbEJxotJ96kPnYlgIiD4DFnA3kgHPbJGcWGENgEBsgaL", + "UYCxpwI2g4DLEGihXq6Zh35DoYQMt3eD1bAhCCRDYNKwGQHh+R3JM0QZLcHbVV3CJ4KewpZAOpqvX7M/", + "c5omQXbSUYA/MnPYd6bpwLEdwQe3M5oinHhDa8jqWK5ZFUpGT6pSbvOFGlHHQt1JVh3utKBIgIFmIJAO", + "BaKnxrTjWaFEksZ8DK09y1AkjdZ85j7EmTfD8y/aXDMHYu2dYSlg31EgIGy6ZMIJa2Lgb0a9GFodTuSi", + "BMPbRO6d21Hgnljq5fvsRTrWutCjqEoZlp9+vIhiWGhLITWes3GPevxXET8Z6erla9OaM3pLagL56pjH", + "4ibb9fL/JBkCNS5owHjJYRtcDwg/B0Khsw0l1AKNY0HDcc3J1ZTIOQSuBYTV9eNABtTazPEPFN0QGoKP", + "H+vlf2Yvyfa7C3TJ4OMg0Wh6bN9OOr5W0G9GhNnWEdzEKDckKqBNoEbMjuJLqb5AzKKMLwfx2v4Xep7u", + "NE9NhluX4YzY9N8HihIhSw1Z1JhxVKF2FOIk4JvyoXxIljlPjN6oSv1QPpRv0uQoXR564S021Dmrp2e2", + "JblfJn+iNXmFRtgjC6CApbRBHRMkqAKiA0l+9fgXpWVDPXTo/TgJlahhAqu1qtTq6sokR/SO42mJtTjY", + "PELyjziX6L01TQZYfJm/LVN8UvX1cM1vPAt5y+ya/TH//gkAAP//d44HiNkGAAA=", } // GetSwagger returns the content of the embedded swagger specification file diff --git a/internal/test/all_of/v2/openapi.gen.go b/internal/test/all_of/v2/openapi.gen.go index ab430340d6..4f3f18fc09 100644 --- a/internal/test/all_of/v2/openapi.gen.go +++ b/internal/test/all_of/v2/openapi.gen.go @@ -38,20 +38,29 @@ type PersonWithID struct { LastName string `json:"LastName"` } +// PersonWithMorePropertiesOutsideOfAllOf defines model for PersonWithMorePropertiesOutsideOfAllOf. +type PersonWithMorePropertiesOutsideOfAllOf struct { + FirstName string `json:"FirstName"` + GovernmentIDNumber *int64 `json:"GovernmentIDNumber,omitempty"` + LastName string `json:"LastName"` + AdditionalProperty string `json:"additionalProperty"` +} + // Base64 encoded, gzipped, json marshaled Swagger object var swaggerSpec = []string{ - "H4sIAAAAAAAC/5SUT2/bOBDFv8qAu0dBTrCLPegWrNtAQJEaaNIc4gAZSyOLKTVkyVEMwfB3L0jJ/+AW", - "aX0awOTjvDe/0VZVtnOWiSWoYqtC1VKHqVyQD5ZjhcZ8blTxtFV/e2pUof6aHW/Npiuz8fzCW0deNAW1", - "y7bK0/dee6pV8aQ+ah/kDjtSmfqEU/m8e85UTaHy2omO76n7VgfQARBcksxgo6WFDrlGsX6AJgoBcg0G", - "gwBjRxmsegGbJNBAOV8y992KfA5JbmN7U8OKwJP0nqmG1QAIL7ckLxBkMAQ3izKHR4KO/JpAWpqeX7I7", - "eBo7QbbSkocvyTlsWl21YNkM4Lx90zUF2PuGRpOpQ75klSkZHKlC2dUrVaJ2mbqIrNheZEGBAD1NQiAt", - "CgRHlW6GQ0LRJA3pGBpziCGLGS354L0Pk2+Glw+1PnUOxLWzmiWDTUuegLBq4xD2WqMDd9bqcaDFdm8u", - "iNe8juZu7Rt57oilnN+lWcRjjfUdiiqUZvnv32MomoXW5OPFAxuXqrtfhviopS3nf0prYvTc1Cjybpu7", - "7Iztcv47JIOnyvoaMBw5bLztAOF/Tyh0GEMOpUBlWVBzWHKcaiRygsA2gLA4XQ5kwLrWE/6egu19RfDw", - "UM5/yl7sX3NjU8ZaTPzvnoIEuInxQUosJD2VqTfyYXR0nV/lVzF164jRaVWof/Kr/DqygdKmBGfOYEWt", - "NfU48jXJJdhf0ei0zgE2yAIoYChus2WCKJVBsCAxwA6/UQSfOmjRuWE0FGeGUaysVaEWJ0/GyQRnOewX", - "qsHepBZioMSpROeMrpLA7HX6zo1sxOp9cibeUpDnzk7d79LvRwAAAP//lzc18GUFAAA=", + "H4sIAAAAAAAC/8xVzY7bNhB+lQHboyBv0KIH3YK6DQS0WQNNmkO8wI7FkcWUGrLkyIZg+N0LUvIfvG26", + "t/g0gDkf5/vh6KAa13vHxBJVdVCx6ajHXK4oRMepQmsfW1V9PqjvA7WqUt8tLl2LuWUxnV8F5ymIoaiO", + "xUEF+nswgbSqPqtfTYjyHntShfoN5/Lp+FQoTbEJxotJ96kPnYlgIiD4DFnA3kgHPbJGcWGENgEBsgaL", + "UYCxpwI2g4DLEGihXq6Zh35DoYQMt3eD1bAhCCRDYNKwGQHh+R3JM0QZLcHbVV3CJ4KewpZAOpqvX7M/", + "c5omQXbSUYA/MnPYd6bpwLEdwQe3M5oinHhDa8jqWK5ZFUpGT6pSbvOFGlHHQt1JVh3utKBIgIFmIJAO", + "BaKnxrTjWaFEksZ8DK09y1AkjdZ85j7EmTfD8y/aXDMHYu2dYSlg31EgIGy6ZMIJa2Lgb0a9GFodTuSi", + "BMPbRO6d21Hgnljq5fvsRTrWutCjqEoZlp9+vIhiWGhLITWes3GPevxXET8Z6erla9OaM3pLagL56pjH", + "4ibb9fL/JBkCNS5owHjJYRtcDwg/B0Khsw0l1AKNY0HDcc3J1ZTIOQSuBYTV9eNABtTazPEPFN0QGoKP", + "H+vlf2Yvyfa7C3TJ4OMg0Wh6bN9OOr5W0G9GhNnWEdzEKDckKqBNoEbMjuJLqb5AzKKMLwfx2v4Xep7u", + "NE9NhluX4YzY9N8HihIhSw1Z1JhxVKF2FOIk4JvyoXxIljlPjN6oSv1QPpRv0uQoXR564S021Dmrp2e2", + "JblfJn+iNXmFRtgjC6CApbRBHRMkqAKiA0l+9fgXpWVDPXTo/TgJlahhAqu1qtTq6sokR/SO42mJtTjY", + "PELyjziX6L01TQZYfJm/LVN8UvX1cM1vPAt5y+ya/TH//gkAAP//d44HiNkGAAA=", } // GetSwagger returns the content of the embedded swagger specification file From 0d104fcba1e275795a9ddf02f3d038affa919216 Mon Sep 17 00:00:00 2001 From: Kotaro Inoue Date: Wed, 3 Jan 2024 00:07:55 +0900 Subject: [PATCH 3/5] Make generate Signed-off-by: Kotaro Inoue --- .../test/issues/issue-1087/deps/deps.gen.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/internal/test/issues/issue-1087/deps/deps.gen.go b/internal/test/issues/issue-1087/deps/deps.gen.go index 8066ab2b57..3dbd4b4694 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 From 511bb8387b13ce83a23b341b255b81536fdb34f3 Mon Sep 17 00:00:00 2001 From: Marcin Romaszewicz Date: Sun, 3 May 2026 00:40:23 -0700 Subject: [PATCH 4/5] fix(codegen): only inject parent siblings into allOf merge when structural MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The unconditional injection of the parent (with AllOf cleared) as a final allOf member correctly merges structural siblings like properties/required, but it also forces n>=2 in mergeSchemas for pure wrappers like `{allOf: [$ref: X]}`, which previously took the n==1 fast-path and returned the referenced type. Without the fast-path those wrappers regenerate as inlined anonymous structs and lose their named-type identity (regression visible in examples/output-options/preferskipoptionalpointer). Add hasStructuralSiblings() so injection runs only when the parent has Properties, Required, or AdditionalProperties. Pure wrappers keep the fast-path. Also copy the parent's Description into the merged result — the merge does not carry Description, so it was being silently dropped. Addresses #697, #931, #1710, #1960, #2102. Partial fix for #1139 (anonymous-schema hoisting requires further work). --- internal/test/all_of/v1/openapi.gen.go | 39 +++++++++++------ internal/test/all_of/v2/openapi.gen.go | 39 +++++++++++------ internal/test/components/components.gen.go | 6 +-- pkg/codegen/schema.go | 51 ++++++++++++++++++---- 4 files changed, 98 insertions(+), 37 deletions(-) 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/pkg/codegen/schema.go b/pkg/codegen/schema.go index e7c0b7906b..6c6a029fab 100644 --- a/pkg/codegen/schema.go +++ b/pkg/codegen/schema.go @@ -403,18 +403,34 @@ 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 { - // deepcopy and clear allOf to avoid circular references - s := *schema - s.AllOf = nil - - // merge parent schema with allOf schemas to include parent properties (Issue #697) - allOfRefs := append(schema.AllOf, &openapi3.SchemaRef{Value: &s}) - - mergedSchema, err := MergeSchemas(allOfRefs, path) + var mergedSchema Schema + var err error + if 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. + s := *schema + s.AllOf = nil + allOfRefs := append(schema.AllOf, &openapi3.SchemaRef{Value: &s}) + mergedSchema, err = MergeSchemas(allOfRefs, path) + } else { + // Pure wrapper around allOf with no structural siblings. + // Preserve named-type identity by skipping the injection: + // MergeSchemas' single-element fast path returns the + // referenced type unchanged. + 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. + if 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 — @@ -1019,3 +1035,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 +} From aa1feeeb643dda682663ded46e3db50ae677a890 Mon Sep 17 00:00:00 2001 From: Marcin Romaszewicz Date: Sun, 3 May 2026 01:02:43 -0700 Subject: [PATCH 5/5] fix(codegen): gate allOf sibling merge behind compatibility flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Greptile P1 review on the previous commit: 1. append(schema.AllOf, ...) could mutate kin-openapi's backing array if the parsed slice carried spare capacity. Allocate a fresh slice with make() before appending. 2. Merging the parent's structural siblings into an allOf result is a breaking change to generated Go: a schema like {allOf: [Base], properties: {extra}} previously produced `type X = Base` (interchangeable with Base) and now produces a distinct struct (not interchangeable). Per project practice (cf. OldMergeSchemas, OldEnumConflicts, OldAliasing) gate behavior changes behind a compatibility flag. Add Compatibility.OldAllOfSiblingMerging (yaml: old-allof-sibling-merging, default false). Update configuration-schema.json with a matching description so users hit by downstream breakage can find the knob. The Description copy is gated on the same flag for consistency. Greptile also suggested narrowing hasStructuralSiblings to drop the pure-required case. Keeping Required in the trigger because that is the exact scenario issue #931 reports: a parent that adds `required: [foo]` to an allOf chain currently produces a Go type whose `Foo` field is `*string` with omitempty, ignoring the spec's required constraint. Required is structural in OAS terms — it changes field type in the generated struct — and the compatibility flag is the safety net for users whose downstream code depended on the alias output. --- configuration-schema.json | 4 ++++ pkg/codegen/configuration.go | 13 +++++++++++++ pkg/codegen/schema.go | 30 ++++++++++++++++++++++-------- 3 files changed, 39 insertions(+), 8 deletions(-) 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/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 6c6a029fab..2e216537f0 100644 --- a/pkg/codegen/schema.go +++ b/pkg/codegen/schema.go @@ -405,20 +405,33 @@ func GenerateGoSchema(sref *openapi3.SchemaRef, path []string) (Schema, error) { if schema.AllOf != nil { var mergedSchema Schema var err error - if hasStructuralSiblings(schema) { + // 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 := append(schema.AllOf, &openapi3.SchemaRef{Value: &s}) + 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 { - // Pure wrapper around allOf with no structural siblings. - // Preserve named-type identity by skipping the injection: - // MergeSchemas' single-element fast path returns the - // referenced type unchanged. + // 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 { @@ -427,8 +440,9 @@ func GenerateGoSchema(sref *openapi3.SchemaRef, path []string) (Schema, error) { 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. - if schema.Description != "" { + // 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