From abce0789ada69872646f695a994f74f6eb63c7af Mon Sep 17 00:00:00 2001 From: Marcin Romaszewicz Date: Sun, 12 Jul 2026 10:19:09 -0700 Subject: [PATCH 1/3] Fix discriminator stamping for pointer-typed and named discriminator fields Closes: #2297 The From*/Merge* union helpers assigned the discriminator value as a raw string literal to the variant's field (v.Code = "resource_exists"), which does not compile when the discriminator property is optional and renders as a pointer to a named enum type. The assignment also assumed the Go field name matches the JSON property name, and that the variant declares the field at all. When the union struct does not itself declare the discriminator property, the value is now stamped into the marshaled JSON instead, via runtime.JSONMerge with the JSON property name. This is type-agnostic: it works for pointer fields, named enum types, x-go-name renames, absent fields, and import-mapped external variants. When the union struct does declare the property, the field assignment remains (it feeds the union's MarshalJSON), now taking an address through a typed local variable when the field is pointer-typed (new Property.IsPointer helper). Output is unchanged for unions without discriminators and for the existing non-pointer discriminator cases, except that From* on a discriminated union now emits JSON with the same alphabetical key order Merge* always produced. Co-Authored-By: Claude Fable 5 --- .../test/aggregates/oneof/components.gen.go | 88 +++-- .../aggregates/oneof/discriminator.gen.go | 307 ++++++++++++++++++ .../aggregates/oneof/discriminator_test.go | 49 +++ .../aggregates/oneof/spec_discriminator.yaml | 88 ++++- .../test/schemas/nullable/spec31/types.gen.go | 20 +- pkg/codegen/schema.go | 7 + pkg/codegen/templates/union.tmpl | 81 +++-- 7 files changed, 583 insertions(+), 57 deletions(-) diff --git a/internal/test/aggregates/oneof/components.gen.go b/internal/test/aggregates/oneof/components.gen.go index 0728185dd0..9b744ed830 100644 --- a/internal/test/aggregates/oneof/components.gen.go +++ b/internal/test/aggregates/oneof/components.gen.go @@ -679,7 +679,6 @@ func (t OneOfObject13) AsOneOfVariant1() (OneOfVariant1, error) { // FromOneOfVariant1 overwrites any union data inside the OneOfObject13 as the provided OneOfVariant1 func (t *OneOfObject13) FromOneOfVariant1(v OneOfVariant1) error { t.Type = "v1" - b, err := json.Marshal(v) t.union = b return err @@ -688,7 +687,6 @@ func (t *OneOfObject13) FromOneOfVariant1(v OneOfVariant1) error { // MergeOneOfVariant1 performs a merge with any union data inside the OneOfObject13, using the provided OneOfVariant1 func (t *OneOfObject13) MergeOneOfVariant1(v OneOfVariant1) error { t.Type = "v1" - b, err := json.Marshal(v) if err != nil { return err @@ -709,7 +707,6 @@ func (t OneOfObject13) AsOneOfVariant6() (OneOfVariant6, error) { // FromOneOfVariant6 overwrites any union data inside the OneOfObject13 as the provided OneOfVariant6 func (t *OneOfObject13) FromOneOfVariant6(v OneOfVariant6) error { t.Type = "v6" - b, err := json.Marshal(v) t.union = b return err @@ -718,7 +715,6 @@ func (t *OneOfObject13) FromOneOfVariant6(v OneOfVariant6) error { // MergeOneOfVariant6 performs a merge with any union data inside the OneOfObject13, using the provided OneOfVariant6 func (t *OneOfObject13) MergeOneOfVariant6(v OneOfVariant6) error { t.Type = "v6" - b, err := json.Marshal(v) if err != nil { return err @@ -1059,19 +1055,25 @@ func (t OneOfObject5) AsOneOfVariant4() (OneOfVariant4, error) { // FromOneOfVariant4 overwrites any union data inside the OneOfObject5 as the provided OneOfVariant4 func (t *OneOfObject5) FromOneOfVariant4(v OneOfVariant4) error { - v.Discriminator = "OneOfVariant4" b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"discriminator":"OneOfVariant4"}`)) t.union = b return err } // MergeOneOfVariant4 performs a merge with any union data inside the OneOfObject5, using the provided OneOfVariant4 func (t *OneOfObject5) MergeOneOfVariant4(v OneOfVariant4) error { - v.Discriminator = "OneOfVariant4" b, err := json.Marshal(v) if err != nil { return err } + b, err = runtime.JSONMerge(b, []byte(`{"discriminator":"OneOfVariant4"}`)) + if err != nil { + return err + } merged, err := runtime.JSONMerge(t.union, b) t.union = merged @@ -1087,19 +1089,25 @@ func (t OneOfObject5) AsOneOfVariant5() (OneOfVariant5, error) { // FromOneOfVariant5 overwrites any union data inside the OneOfObject5 as the provided OneOfVariant5 func (t *OneOfObject5) FromOneOfVariant5(v OneOfVariant5) error { - v.Discriminator = "OneOfVariant5" b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"discriminator":"OneOfVariant5"}`)) t.union = b return err } // MergeOneOfVariant5 performs a merge with any union data inside the OneOfObject5, using the provided OneOfVariant5 func (t *OneOfObject5) MergeOneOfVariant5(v OneOfVariant5) error { - v.Discriminator = "OneOfVariant5" b, err := json.Marshal(v) if err != nil { return err } + b, err = runtime.JSONMerge(b, []byte(`{"discriminator":"OneOfVariant5"}`)) + if err != nil { + return err + } merged, err := runtime.JSONMerge(t.union, b) t.union = merged @@ -1148,19 +1156,25 @@ func (t OneOfObject6) AsOneOfVariant4() (OneOfVariant4, error) { // FromOneOfVariant4 overwrites any union data inside the OneOfObject6 as the provided OneOfVariant4 func (t *OneOfObject6) FromOneOfVariant4(v OneOfVariant4) error { - v.Discriminator = "v4" b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"discriminator":"v4"}`)) t.union = b return err } // MergeOneOfVariant4 performs a merge with any union data inside the OneOfObject6, using the provided OneOfVariant4 func (t *OneOfObject6) MergeOneOfVariant4(v OneOfVariant4) error { - v.Discriminator = "v4" b, err := json.Marshal(v) if err != nil { return err } + b, err = runtime.JSONMerge(b, []byte(`{"discriminator":"v4"}`)) + if err != nil { + return err + } merged, err := runtime.JSONMerge(t.union, b) t.union = merged @@ -1176,19 +1190,25 @@ func (t OneOfObject6) AsOneOfVariant5() (OneOfVariant5, error) { // FromOneOfVariant5 overwrites any union data inside the OneOfObject6 as the provided OneOfVariant5 func (t *OneOfObject6) FromOneOfVariant5(v OneOfVariant5) error { - v.Discriminator = "v5" b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"discriminator":"v5"}`)) t.union = b return err } // MergeOneOfVariant5 performs a merge with any union data inside the OneOfObject6, using the provided OneOfVariant5 func (t *OneOfObject6) MergeOneOfVariant5(v OneOfVariant5) error { - v.Discriminator = "v5" b, err := json.Marshal(v) if err != nil { return err } + b, err = runtime.JSONMerge(b, []byte(`{"discriminator":"v5"}`)) + if err != nil { + return err + } merged, err := runtime.JSONMerge(t.union, b) t.union = merged @@ -1237,19 +1257,25 @@ func (t OneOfObject61) AsOneOfVariant4() (OneOfVariant4, error) { // FromOneOfVariant4 overwrites any union data inside the OneOfObject61 as the provided OneOfVariant4 func (t *OneOfObject61) FromOneOfVariant4(v OneOfVariant4) error { - v.Discriminator = "v4" b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"discriminator":"v4"}`)) t.union = b return err } // MergeOneOfVariant4 performs a merge with any union data inside the OneOfObject61, using the provided OneOfVariant4 func (t *OneOfObject61) MergeOneOfVariant4(v OneOfVariant4) error { - v.Discriminator = "v4" b, err := json.Marshal(v) if err != nil { return err } + b, err = runtime.JSONMerge(b, []byte(`{"discriminator":"v4"}`)) + if err != nil { + return err + } merged, err := runtime.JSONMerge(t.union, b) t.union = merged @@ -1265,19 +1291,25 @@ func (t OneOfObject61) AsOneOfVariant5() (OneOfVariant5, error) { // FromOneOfVariant5 overwrites any union data inside the OneOfObject61 as the provided OneOfVariant5 func (t *OneOfObject61) FromOneOfVariant5(v OneOfVariant5) error { - v.Discriminator = "OneOfVariant5" b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"discriminator":"OneOfVariant5"}`)) t.union = b return err } // MergeOneOfVariant5 performs a merge with any union data inside the OneOfObject61, using the provided OneOfVariant5 func (t *OneOfObject61) MergeOneOfVariant5(v OneOfVariant5) error { - v.Discriminator = "OneOfVariant5" b, err := json.Marshal(v) if err != nil { return err } + b, err = runtime.JSONMerge(b, []byte(`{"discriminator":"OneOfVariant5"}`)) + if err != nil { + return err + } merged, err := runtime.JSONMerge(t.union, b) t.union = merged @@ -1326,19 +1358,25 @@ func (t OneOfObject62) AsOneOfVariant4() (OneOfVariant4, error) { // FromOneOfVariant4 overwrites any union data inside the OneOfObject62 as the provided OneOfVariant4 func (t *OneOfObject62) FromOneOfVariant4(v OneOfVariant4) error { - v.Discriminator = "variant_four" b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"discriminator":"variant_four"}`)) t.union = b return err } // MergeOneOfVariant4 performs a merge with any union data inside the OneOfObject62, using the provided OneOfVariant4 func (t *OneOfObject62) MergeOneOfVariant4(v OneOfVariant4) error { - v.Discriminator = "variant_four" b, err := json.Marshal(v) if err != nil { return err } + b, err = runtime.JSONMerge(b, []byte(`{"discriminator":"variant_four"}`)) + if err != nil { + return err + } merged, err := runtime.JSONMerge(t.union, b) t.union = merged @@ -1354,19 +1392,25 @@ func (t OneOfObject62) AsOneOfVariant51() (OneOfVariant51, error) { // FromOneOfVariant51 overwrites any union data inside the OneOfObject62 as the provided OneOfVariant51 func (t *OneOfObject62) FromOneOfVariant51(v OneOfVariant51) error { - v.Discriminator = "one_of_variant51" b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"discriminator":"one_of_variant51"}`)) t.union = b return err } // MergeOneOfVariant51 performs a merge with any union data inside the OneOfObject62, using the provided OneOfVariant51 func (t *OneOfObject62) MergeOneOfVariant51(v OneOfVariant51) error { - v.Discriminator = "one_of_variant51" b, err := json.Marshal(v) if err != nil { return err } + b, err = runtime.JSONMerge(b, []byte(`{"discriminator":"one_of_variant51"}`)) + if err != nil { + return err + } merged, err := runtime.JSONMerge(t.union, b) t.union = merged @@ -1574,7 +1618,6 @@ func (t OneOfObject9) AsOneOfVariant1() (OneOfVariant1, error) { // FromOneOfVariant1 overwrites any union data inside the OneOfObject9 as the provided OneOfVariant1 func (t *OneOfObject9) FromOneOfVariant1(v OneOfVariant1) error { t.Type = "v1" - b, err := json.Marshal(v) t.union = b return err @@ -1583,7 +1626,6 @@ func (t *OneOfObject9) FromOneOfVariant1(v OneOfVariant1) error { // MergeOneOfVariant1 performs a merge with any union data inside the OneOfObject9, using the provided OneOfVariant1 func (t *OneOfObject9) MergeOneOfVariant1(v OneOfVariant1) error { t.Type = "v1" - b, err := json.Marshal(v) if err != nil { return err @@ -1604,7 +1646,6 @@ func (t OneOfObject9) AsOneOfVariant6() (OneOfVariant6, error) { // FromOneOfVariant6 overwrites any union data inside the OneOfObject9 as the provided OneOfVariant6 func (t *OneOfObject9) FromOneOfVariant6(v OneOfVariant6) error { t.Type = "v6" - b, err := json.Marshal(v) t.union = b return err @@ -1613,7 +1654,6 @@ func (t *OneOfObject9) FromOneOfVariant6(v OneOfVariant6) error { // MergeOneOfVariant6 performs a merge with any union data inside the OneOfObject9, using the provided OneOfVariant6 func (t *OneOfObject9) MergeOneOfVariant6(v OneOfVariant6) error { t.Type = "v6" - b, err := json.Marshal(v) if err != nil { return err diff --git a/internal/test/aggregates/oneof/discriminator.gen.go b/internal/test/aggregates/oneof/discriminator.gen.go index 54105a793a..bbde1024d1 100644 --- a/internal/test/aggregates/oneof/discriminator.gen.go +++ b/internal/test/aggregates/oneof/discriminator.gen.go @@ -6,10 +6,41 @@ package aggregatesoneof import ( "encoding/json" "errors" + "fmt" "github.com/oapi-codegen/runtime" ) +// Defines values for IdempotencyConflictErrorCode. +const ( + IdempotencyConflict IdempotencyConflictErrorCode = "idempotency_conflict" +) + +// Valid indicates whether the value is a known member of the IdempotencyConflictErrorCode enum. +func (e IdempotencyConflictErrorCode) Valid() bool { + switch e { + case IdempotencyConflict: + return true + default: + return false + } +} + +// Defines values for ResourceConflictErrorCode. +const ( + ResourceExists ResourceConflictErrorCode = "resource_exists" +) + +// Valid indicates whether the value is a known member of the ResourceConflictErrorCode enum. +func (e ResourceConflictErrorCode) Valid() bool { + switch e { + case ResourceExists: + return true + default: + return false + } +} + // ConfigHttp defines model for ConfigHttp. type ConfigHttp struct { ConfigType string `json:"config_type"` @@ -33,9 +64,57 @@ type ConfigSsh struct { User *string `json:"user,omitempty"` } +// ConflictError defines model for ConflictError. +type ConflictError struct { + union json.RawMessage +} + +// ErrorBase defines model for ErrorBase. +type ErrorBase struct { + Code *string `json:"code,omitempty"` + Error string `json:"error"` +} + +// IdempotencyConflictError defines model for IdempotencyConflictError. +type IdempotencyConflictError struct { + Code *IdempotencyConflictErrorCode `json:"code,omitempty"` + Error string `json:"error"` +} + +// IdempotencyConflictErrorCode defines model for IdempotencyConflictError.Code. +type IdempotencyConflictErrorCode string + +// KindCat defines model for KindCat. +type KindCat struct { + Meow *string `json:"meow,omitempty"` +} + +// KindDog defines model for KindDog. +type KindDog struct { + Bark *string `json:"bark,omitempty"` +} + +// PetByKind defines model for PetByKind. +type PetByKind struct { + Kind *string `json:"kind,omitempty"` + union json.RawMessage +} + +// ResourceConflictError defines model for ResourceConflictError. +type ResourceConflictError struct { + Code *ResourceConflictErrorCode `json:"code,omitempty"` + Error string `json:"error"` +} + +// ResourceConflictErrorCode defines model for ResourceConflictError.Code. +type ResourceConflictErrorCode string + // PostConfigJSONRequestBody defines body for PostConfig for application/json ContentType. type PostConfigJSONRequestBody = ConfigSaveReq +// PostPetJSONRequestBody defines body for PostPet for application/json ContentType. +type PostPetJSONRequestBody = PetByKind + // AsConfigHttp returns the union data inside the ConfigSaveReq as a ConfigHttp func (t ConfigSaveReq) AsConfigHttp() (ConfigHttp, error) { var body ConfigHttp @@ -124,3 +203,231 @@ func (t *ConfigSaveReq) UnmarshalJSON(b []byte) error { err := t.union.UnmarshalJSON(b) return err } + +// AsResourceConflictError returns the union data inside the ConflictError as a ResourceConflictError +func (t ConflictError) AsResourceConflictError() (ResourceConflictError, error) { + var body ResourceConflictError + err := json.Unmarshal(t.union, &body) + return body, err +} + +// FromResourceConflictError overwrites any union data inside the ConflictError as the provided ResourceConflictError +func (t *ConflictError) FromResourceConflictError(v ResourceConflictError) error { + b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"code":"resource_exists"}`)) + t.union = b + return err +} + +// MergeResourceConflictError performs a merge with any union data inside the ConflictError, using the provided ResourceConflictError +func (t *ConflictError) MergeResourceConflictError(v ResourceConflictError) error { + b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"code":"resource_exists"}`)) + if err != nil { + return err + } + + merged, err := runtime.JSONMerge(t.union, b) + t.union = merged + return err +} + +// AsIdempotencyConflictError returns the union data inside the ConflictError as a IdempotencyConflictError +func (t ConflictError) AsIdempotencyConflictError() (IdempotencyConflictError, error) { + var body IdempotencyConflictError + err := json.Unmarshal(t.union, &body) + return body, err +} + +// FromIdempotencyConflictError overwrites any union data inside the ConflictError as the provided IdempotencyConflictError +func (t *ConflictError) FromIdempotencyConflictError(v IdempotencyConflictError) error { + b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"code":"idempotency_conflict"}`)) + t.union = b + return err +} + +// MergeIdempotencyConflictError performs a merge with any union data inside the ConflictError, using the provided IdempotencyConflictError +func (t *ConflictError) MergeIdempotencyConflictError(v IdempotencyConflictError) error { + b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"code":"idempotency_conflict"}`)) + if err != nil { + return err + } + + merged, err := runtime.JSONMerge(t.union, b) + t.union = merged + return err +} + +func (t ConflictError) Discriminator() (string, error) { + var discriminator struct { + Discriminator string `json:"code"` + } + err := json.Unmarshal(t.union, &discriminator) + return discriminator.Discriminator, err +} + +func (t ConflictError) ValueByDiscriminator() (interface{}, error) { + discriminator, err := t.Discriminator() + if err != nil { + return nil, err + } + switch discriminator { + case "idempotency_conflict": + return t.AsIdempotencyConflictError() + case "resource_exists": + return t.AsResourceConflictError() + default: + return nil, errors.New("unknown discriminator value: " + discriminator) + } +} + +func (t ConflictError) MarshalJSON() ([]byte, error) { + b, err := t.union.MarshalJSON() + return b, err +} + +func (t *ConflictError) UnmarshalJSON(b []byte) error { + err := t.union.UnmarshalJSON(b) + return err +} + +// AsKindCat returns the union data inside the PetByKind as a KindCat +func (t PetByKind) AsKindCat() (KindCat, error) { + var body KindCat + err := json.Unmarshal(t.union, &body) + return body, err +} + +// FromKindCat overwrites any union data inside the PetByKind as the provided KindCat +func (t *PetByKind) FromKindCat(v KindCat) error { + var discriminator string = "cat" + t.Kind = &discriminator + b, err := json.Marshal(v) + t.union = b + return err +} + +// MergeKindCat performs a merge with any union data inside the PetByKind, using the provided KindCat +func (t *PetByKind) MergeKindCat(v KindCat) error { + var discriminator string = "cat" + t.Kind = &discriminator + b, err := json.Marshal(v) + if err != nil { + return err + } + + merged, err := runtime.JSONMerge(t.union, b) + t.union = merged + return err +} + +// AsKindDog returns the union data inside the PetByKind as a KindDog +func (t PetByKind) AsKindDog() (KindDog, error) { + var body KindDog + err := json.Unmarshal(t.union, &body) + return body, err +} + +// FromKindDog overwrites any union data inside the PetByKind as the provided KindDog +func (t *PetByKind) FromKindDog(v KindDog) error { + var discriminator string = "dog" + t.Kind = &discriminator + b, err := json.Marshal(v) + t.union = b + return err +} + +// MergeKindDog performs a merge with any union data inside the PetByKind, using the provided KindDog +func (t *PetByKind) MergeKindDog(v KindDog) error { + var discriminator string = "dog" + t.Kind = &discriminator + b, err := json.Marshal(v) + if err != nil { + return err + } + + merged, err := runtime.JSONMerge(t.union, b) + t.union = merged + return err +} + +func (t PetByKind) Discriminator() (string, error) { + var discriminator struct { + Discriminator string `json:"kind"` + } + err := json.Unmarshal(t.union, &discriminator) + return discriminator.Discriminator, err +} + +func (t PetByKind) ValueByDiscriminator() (interface{}, error) { + discriminator, err := t.Discriminator() + if err != nil { + return nil, err + } + switch discriminator { + case "cat": + return t.AsKindCat() + case "dog": + return t.AsKindDog() + default: + return nil, errors.New("unknown discriminator value: " + discriminator) + } +} + +func (t PetByKind) MarshalJSON() ([]byte, error) { + b, err := t.union.MarshalJSON() + if err != nil { + return nil, err + } + object := make(map[string]json.RawMessage) + if t.union != nil { + err = json.Unmarshal(b, &object) + if err != nil { + return nil, err + } + } + + if t.Kind != nil { + object["kind"], err = json.Marshal(t.Kind) + if err != nil { + return nil, fmt.Errorf("error marshaling 'kind': %w", err) + } + } + b, err = json.Marshal(object) + return b, err +} + +func (t *PetByKind) UnmarshalJSON(b []byte) error { + err := t.union.UnmarshalJSON(b) + if err != nil { + return err + } + object := make(map[string]json.RawMessage) + err = json.Unmarshal(b, &object) + if err != nil { + return err + } + + if raw, found := object["kind"]; found { + err = json.Unmarshal(raw, &t.Kind) + if err != nil { + return fmt.Errorf("error reading 'kind': %w", err) + } + } + + return err +} diff --git a/internal/test/aggregates/oneof/discriminator_test.go b/internal/test/aggregates/oneof/discriminator_test.go index 23928ebf23..423eb1951a 100644 --- a/internal/test/aggregates/oneof/discriminator_test.go +++ b/internal/test/aggregates/oneof/discriminator_test.go @@ -48,3 +48,52 @@ func TestIssue1530(t *testing.T) { require.Equal(t, cfg, cfgByDiscriminator) }) } + +// TestIssue2297PointerDiscriminatorOnVariant covers a discriminator property +// that is optional on the variants and narrowed to a single-value enum, so it +// renders as a pointer to a named enum type. From*/Merge* stamp the +// discriminator into the union JSON without touching the variant's field, so +// the caller doesn't need to populate it. +func TestIssue2297PointerDiscriminatorOnVariant(t *testing.T) { + var conflict ConflictError + require.NoError(t, conflict.FromResourceConflictError(ResourceConflictError{Error: "already there"})) + + d, err := conflict.Discriminator() + require.NoError(t, err) + require.Equal(t, "resource_exists", d) + + v, err := conflict.ValueByDiscriminator() + require.NoError(t, err) + rce, ok := v.(ResourceConflictError) + require.True(t, ok) + require.NotNil(t, rce.Code) + require.Equal(t, ResourceExists, *rce.Code) + require.Equal(t, "already there", rce.Error) + + // Merging the other variant re-stamps the discriminator. + require.NoError(t, conflict.MergeIdempotencyConflictError(IdempotencyConflictError{Error: "replayed"})) + d, err = conflict.Discriminator() + require.NoError(t, err) + require.Equal(t, "idempotency_conflict", d) +} + +// TestIssue2297PointerDiscriminatorOnUnion covers the union schema declaring +// the discriminator property itself, optional, so the field on the union +// struct is pointer-typed and is set through an addressable value. +func TestIssue2297PointerDiscriminatorOnUnion(t *testing.T) { + var pet PetByKind + meow := "prrr" + require.NoError(t, pet.FromKindCat(KindCat{Meow: &meow})) + + require.NotNil(t, pet.Kind) + require.Equal(t, "cat", *pet.Kind) + + b, err := pet.MarshalJSON() + require.NoError(t, err) + require.Contains(t, string(b), `"kind":"cat"`) + + cat, err := pet.AsKindCat() + require.NoError(t, err) + require.NotNil(t, cat.Meow) + require.Equal(t, meow, *cat.Meow) +} diff --git a/internal/test/aggregates/oneof/spec_discriminator.yaml b/internal/test/aggregates/oneof/spec_discriminator.yaml index 222d074b7b..772d28f0a2 100644 --- a/internal/test/aggregates/oneof/spec_discriminator.yaml +++ b/internal/test/aggregates/oneof/spec_discriminator.yaml @@ -1,4 +1,4 @@ -paths: +paths: /config: post: summary: Save configuration @@ -10,6 +10,28 @@ paths: responses: "200": description: Configuration saved successfully + # issue-2297: anchor the pointer-discriminator schemas against pruning. + /conflict: + get: + summary: Reproduce issue 2297 + responses: + "409": + description: Conflict + content: + application/json: + schema: + $ref: "#/components/schemas/ConflictError" + /pet: + post: + summary: Union carrying its own optional discriminator property + requestBody: + content: + application/json: + schema: + $ref: "#/components/schemas/PetByKind" + responses: + "200": + description: OK components: schemas: ConfigHttp: @@ -54,4 +76,66 @@ components: private_key: type: string required: - - config_type \ No newline at end of file + - config_type + # issue-2297: the discriminator property is optional on the variants and + # narrowed to a single-value enum via allOf, so it renders as a pointer to + # a named enum type (*ResourceConflictErrorCode). From*/Merge* must stamp + # the discriminator value without assigning a raw string literal to that + # field. + ErrorBase: + type: object + required: [error] + properties: + code: + type: string + error: + type: string + ResourceConflictError: + allOf: + - $ref: "#/components/schemas/ErrorBase" + - properties: + code: + type: string + enum: [resource_exists] + IdempotencyConflictError: + allOf: + - $ref: "#/components/schemas/ErrorBase" + - properties: + code: + type: string + enum: [idempotency_conflict] + ConflictError: + oneOf: + - $ref: "#/components/schemas/ResourceConflictError" + - $ref: "#/components/schemas/IdempotencyConflictError" + discriminator: + propertyName: code + mapping: + resource_exists: "#/components/schemas/ResourceConflictError" + idempotency_conflict: "#/components/schemas/IdempotencyConflictError" + # issue-2297 (union-side variant): the union schema itself declares the + # discriminator property, optional, so the field on the union struct is + # pointer-typed and must be set through an addressable value. + PetByKind: + type: object + properties: + kind: + type: string + oneOf: + - $ref: "#/components/schemas/KindCat" + - $ref: "#/components/schemas/KindDog" + discriminator: + propertyName: kind + mapping: + cat: "#/components/schemas/KindCat" + dog: "#/components/schemas/KindDog" + KindCat: + type: object + properties: + meow: + type: string + KindDog: + type: object + properties: + bark: + type: string \ No newline at end of file diff --git a/internal/test/schemas/nullable/spec31/types.gen.go b/internal/test/schemas/nullable/spec31/types.gen.go index 20ab279770..65b59e586c 100644 --- a/internal/test/schemas/nullable/spec31/types.gen.go +++ b/internal/test/schemas/nullable/spec31/types.gen.go @@ -101,19 +101,25 @@ func (t DiscriminatedPet) AsCat() (Cat, error) { // FromCat overwrites any union data inside the DiscriminatedPet as the provided Cat func (t *DiscriminatedPet) FromCat(v Cat) error { - v.Kind = "Cat" b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"kind":"Cat"}`)) t.union = b return err } // MergeCat performs a merge with any union data inside the DiscriminatedPet, using the provided Cat func (t *DiscriminatedPet) MergeCat(v Cat) error { - v.Kind = "Cat" b, err := json.Marshal(v) if err != nil { return err } + b, err = runtime.JSONMerge(b, []byte(`{"kind":"Cat"}`)) + if err != nil { + return err + } merged, err := runtime.JSONMerge(t.union, b) t.union = merged @@ -129,19 +135,25 @@ func (t DiscriminatedPet) AsDog() (Dog, error) { // FromDog overwrites any union data inside the DiscriminatedPet as the provided Dog func (t *DiscriminatedPet) FromDog(v Dog) error { - v.Kind = "Dog" b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"kind":"Dog"}`)) t.union = b return err } // MergeDog performs a merge with any union data inside the DiscriminatedPet, using the provided Dog func (t *DiscriminatedPet) MergeDog(v Dog) error { - v.Kind = "Dog" b, err := json.Marshal(v) if err != nil { return err } + b, err = runtime.JSONMerge(b, []byte(`{"kind":"Dog"}`)) + if err != nil { + return err + } merged, err := runtime.JSONMerge(t.union, b) t.union = merged diff --git a/pkg/codegen/schema.go b/pkg/codegen/schema.go index d6cb2e45f3..be9cfb6516 100644 --- a/pkg/codegen/schema.go +++ b/pkg/codegen/schema.go @@ -180,6 +180,13 @@ func (p Property) HasOptionalPointer() bool { return !p.Required && !p.Schema.SkipOptionalPointer } +// IsPointer reports whether the generated Go field for this property is +// pointer-typed, i.e. GoTypeDef renders with a leading `*`. Templates use it +// when assigning a value to the field requires taking an address first. +func (p Property) IsPointer() bool { + return strings.HasPrefix(p.GoTypeDef(), "*") +} + // ZeroValueIsNil is a helper function to determine if the given Go type used // for this property has `nil` as its Go zero value. Slices (OpenAPI `array`) // and maps (OpenAPI `object` with only `additionalProperties`, rendered as diff --git a/pkg/codegen/templates/union.tmpl b/pkg/codegen/templates/union.tmpl index 61c2bf37fd..90621165e0 100644 --- a/pkg/codegen/templates/union.tmpl +++ b/pkg/codegen/templates/union.tmpl @@ -5,6 +5,34 @@ {{$numberOfUnionTypes := len .Schema.UnionElements -}} {{range .Schema.UnionElements}} {{$element := . -}} + {{- /* Resolve the discriminator value this element must carry. + Stamping only happens for 1:1 value-to-type mappings: when the + mapping doesn't cover every element there is nothing reliable + to stamp. When the union struct itself declares the + discriminator property, the value is assigned to that field + (its JSON rendering overwrites the union data); otherwise the + value is stamped into the marshaled JSON, which stays correct + regardless of how the variant declares the property (pointer, + named enum type, renamed or absent field, external package). + See https://github.com/oapi-codegen/oapi-codegen/issues/2297 */ -}} + {{$discriminatorValue := "" -}} + {{$stampDiscriminator := false -}} + {{$unionProperty := "" -}} + {{if $discriminator -}} + {{if eq $numberOfUnionTypes (len $discriminator.Mapping) -}} + {{range $value, $type := $discriminator.Mapping -}} + {{if eq $type $element -}} + {{$discriminatorValue = $value -}} + {{$stampDiscriminator = true -}} + {{end -}} + {{end -}} + {{end -}} + {{range $properties -}} + {{if eq .GoFieldName $discriminator.PropertyName -}} + {{$unionProperty = . -}} + {{end -}} + {{end -}} + {{end -}} // As{{ .Method }} returns the union data inside the {{$typeName}} as a {{.}} func (t {{$typeName}}) As{{ .Method }}() ({{.}}, error) { var body {{.}} @@ -14,42 +42,36 @@ // From{{ .Method }} overwrites any union data inside the {{$typeName}} as the provided {{.}} func (t *{{$typeName}}) From{{ .Method }} (v {{.}}) error { - {{if $discriminator -}} - {{if eq $numberOfUnionTypes (len $discriminator.Mapping) -}} - {{range $value, $type := $discriminator.Mapping -}} - {{if eq $type $element -}} - {{$hasProperty := false -}} - {{range $properties -}} - {{if eq .GoFieldName $discriminator.PropertyName -}} - t.{{$discriminator.PropertyName}} = "{{$value}}" - {{$hasProperty = true -}} - {{end -}} - {{end -}} - {{if not $hasProperty}}v.{{$discriminator.PropertyName}} = "{{$value}}"{{end}} - {{end -}} + {{if $stampDiscriminator -}} + {{if $unionProperty -}} + {{if $unionProperty.IsPointer -}} + var discriminator {{$unionProperty.Schema.TypeDecl}} = "{{$discriminatorValue}}" + t.{{$unionProperty.GoFieldName}} = &discriminator + {{else -}} + t.{{$unionProperty.GoFieldName}} = "{{$discriminatorValue}}" {{end -}} {{end -}} {{end -}} b, err := json.Marshal(v) + {{if and $stampDiscriminator (not $unionProperty) -}} + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"{{$discriminator.Property}}":"{{$discriminatorValue}}"}`)) + {{end -}} t.union = b return err } // Merge{{ .Method }} performs a merge with any union data inside the {{$typeName}}, using the provided {{.}} func (t *{{$typeName}}) Merge{{ .Method }} (v {{.}}) error { - {{if $discriminator -}} - {{if eq $numberOfUnionTypes (len $discriminator.Mapping) -}} - {{range $value, $type := $discriminator.Mapping -}} - {{if eq $type $element -}} - {{$hasProperty := false -}} - {{range $properties -}} - {{if eq .GoFieldName $discriminator.PropertyName -}} - t.{{$discriminator.PropertyName}} = "{{$value}}" - {{$hasProperty = true -}} - {{end -}} - {{end -}} - {{if not $hasProperty}}v.{{$discriminator.PropertyName}} = "{{$value}}"{{end}} - {{end -}} + {{if $stampDiscriminator -}} + {{if $unionProperty -}} + {{if $unionProperty.IsPointer -}} + var discriminator {{$unionProperty.Schema.TypeDecl}} = "{{$discriminatorValue}}" + t.{{$unionProperty.GoFieldName}} = &discriminator + {{else -}} + t.{{$unionProperty.GoFieldName}} = "{{$discriminatorValue}}" {{end -}} {{end -}} {{end -}} @@ -57,7 +79,12 @@ if err != nil { return err } - + {{if and $stampDiscriminator (not $unionProperty) -}} + b, err = runtime.JSONMerge(b, []byte(`{"{{$discriminator.Property}}":"{{$discriminatorValue}}"}`)) + if err != nil { + return err + } + {{end}} merged, err := runtime.JSONMerge(t.union, b) t.union = merged return err From bbf131b78d83f1b620d543ab1346eb4b66ba89d8 Mon Sep 17 00:00:00 2001 From: Marcin Romaszewicz Date: Sun, 12 Jul 2026 10:26:29 -0700 Subject: [PATCH 2/3] Move discriminator stamp logic into Go code Move as much logic as possible into Go code, instead of complicating templates. Co-Authored-By: Claude Fable 5 --- pkg/codegen/schema.go | 54 +++++++++++++++++++++++++ pkg/codegen/templates/union.tmpl | 69 +++++++++----------------------- 2 files changed, 72 insertions(+), 51 deletions(-) diff --git a/pkg/codegen/schema.go b/pkg/codegen/schema.go index be9cfb6516..f19c66555f 100644 --- a/pkg/codegen/schema.go +++ b/pkg/codegen/schema.go @@ -375,6 +375,60 @@ func (d *Discriminator) PropertyName() string { return SchemaNameToTypeName(d.Property) } +// DiscriminatorStamp describes how the generated From*/Merge* union helpers +// record the discriminator value for one union element. +// +// When the union struct itself declares the discriminator property, the +// helpers assign Value to that field: its JSON rendering overwrites the +// union data, so the field is load-bearing. Otherwise the value is merged +// into the marshaled JSON via JSONPatch, which stays correct regardless of +// how the variant declares the property — pointer, named enum type, renamed +// or absent field, or a type in an imported package — none of which is +// knowable from the union's side (see issue #2297). +type DiscriminatorStamp struct { + // Value is the discriminator value mapped to this union element. + Value string + // Property is the union struct's own discriminator field, when it + // declares one; nil means the value is merged into the JSON instead. + Property *Property + // JSONPatch is the JSON object literal merged into the union data when + // Property is nil, e.g. {"code":"resource_exists"}. Safe to embed in a + // backtick string literal: spec validation rejects discriminator + // property names and mapping keys containing quotes, backticks or + // control characters. + JSONPatch string +} + +// DiscriminatorStampFor resolves the discriminator stamp for the given union +// element, or nil when nothing should be stamped: there is no discriminator, +// the mapping doesn't cover every element, or several mapping values share +// one element type, making the value ambiguous (the 1:1 gate keeps parity +// with issue #2071). +func (s Schema) DiscriminatorStampFor(element UnionElement) *DiscriminatorStamp { + d := s.Discriminator + if d == nil || len(d.Mapping) != len(s.UnionElements) { + return nil + } + stamp := DiscriminatorStamp{} + found := false + for _, value := range SortedMapKeys(d.Mapping) { + if d.Mapping[value] == element.String() { + stamp.Value = value + found = true + } + } + if !found { + return nil + } + stamp.JSONPatch = fmt.Sprintf(`{"%s":"%s"}`, d.Property, stamp.Value) + for i := range s.Properties { + if s.Properties[i].GoFieldName() == d.PropertyName() { + stamp.Property = &s.Properties[i] + } + } + return &stamp +} + // UnionElement describe union element, based on prefix externalRef\d+ and real ref name from external schema. type UnionElement string diff --git a/pkg/codegen/templates/union.tmpl b/pkg/codegen/templates/union.tmpl index 90621165e0..ce8fe2dad9 100644 --- a/pkg/codegen/templates/union.tmpl +++ b/pkg/codegen/templates/union.tmpl @@ -1,38 +1,9 @@ {{range .Types}} {{$typeName := .TypeName -}} {{$discriminator := .Schema.Discriminator}} - {{$properties := .Schema.Properties -}} - {{$numberOfUnionTypes := len .Schema.UnionElements -}} + {{$schema := .Schema -}} {{range .Schema.UnionElements}} - {{$element := . -}} - {{- /* Resolve the discriminator value this element must carry. - Stamping only happens for 1:1 value-to-type mappings: when the - mapping doesn't cover every element there is nothing reliable - to stamp. When the union struct itself declares the - discriminator property, the value is assigned to that field - (its JSON rendering overwrites the union data); otherwise the - value is stamped into the marshaled JSON, which stays correct - regardless of how the variant declares the property (pointer, - named enum type, renamed or absent field, external package). - See https://github.com/oapi-codegen/oapi-codegen/issues/2297 */ -}} - {{$discriminatorValue := "" -}} - {{$stampDiscriminator := false -}} - {{$unionProperty := "" -}} - {{if $discriminator -}} - {{if eq $numberOfUnionTypes (len $discriminator.Mapping) -}} - {{range $value, $type := $discriminator.Mapping -}} - {{if eq $type $element -}} - {{$discriminatorValue = $value -}} - {{$stampDiscriminator = true -}} - {{end -}} - {{end -}} - {{end -}} - {{range $properties -}} - {{if eq .GoFieldName $discriminator.PropertyName -}} - {{$unionProperty = . -}} - {{end -}} - {{end -}} - {{end -}} + {{$stamp := $schema.DiscriminatorStampFor . -}} // As{{ .Method }} returns the union data inside the {{$typeName}} as a {{.}} func (t {{$typeName}}) As{{ .Method }}() ({{.}}, error) { var body {{.}} @@ -42,22 +13,20 @@ // From{{ .Method }} overwrites any union data inside the {{$typeName}} as the provided {{.}} func (t *{{$typeName}}) From{{ .Method }} (v {{.}}) error { - {{if $stampDiscriminator -}} - {{if $unionProperty -}} - {{if $unionProperty.IsPointer -}} - var discriminator {{$unionProperty.Schema.TypeDecl}} = "{{$discriminatorValue}}" - t.{{$unionProperty.GoFieldName}} = &discriminator - {{else -}} - t.{{$unionProperty.GoFieldName}} = "{{$discriminatorValue}}" - {{end -}} + {{if and $stamp $stamp.Property -}} + {{if $stamp.Property.IsPointer -}} + var discriminator {{$stamp.Property.Schema.TypeDecl}} = "{{$stamp.Value}}" + t.{{$stamp.Property.GoFieldName}} = &discriminator + {{else -}} + t.{{$stamp.Property.GoFieldName}} = "{{$stamp.Value}}" {{end -}} {{end -}} b, err := json.Marshal(v) - {{if and $stampDiscriminator (not $unionProperty) -}} + {{if and $stamp (not $stamp.Property) -}} if err != nil { return err } - b, err = runtime.JSONMerge(b, []byte(`{"{{$discriminator.Property}}":"{{$discriminatorValue}}"}`)) + b, err = runtime.JSONMerge(b, []byte(`{{$stamp.JSONPatch}}`)) {{end -}} t.union = b return err @@ -65,22 +34,20 @@ // Merge{{ .Method }} performs a merge with any union data inside the {{$typeName}}, using the provided {{.}} func (t *{{$typeName}}) Merge{{ .Method }} (v {{.}}) error { - {{if $stampDiscriminator -}} - {{if $unionProperty -}} - {{if $unionProperty.IsPointer -}} - var discriminator {{$unionProperty.Schema.TypeDecl}} = "{{$discriminatorValue}}" - t.{{$unionProperty.GoFieldName}} = &discriminator - {{else -}} - t.{{$unionProperty.GoFieldName}} = "{{$discriminatorValue}}" - {{end -}} + {{if and $stamp $stamp.Property -}} + {{if $stamp.Property.IsPointer -}} + var discriminator {{$stamp.Property.Schema.TypeDecl}} = "{{$stamp.Value}}" + t.{{$stamp.Property.GoFieldName}} = &discriminator + {{else -}} + t.{{$stamp.Property.GoFieldName}} = "{{$stamp.Value}}" {{end -}} {{end -}} b, err := json.Marshal(v) if err != nil { return err } - {{if and $stampDiscriminator (not $unionProperty) -}} - b, err = runtime.JSONMerge(b, []byte(`{"{{$discriminator.Property}}":"{{$discriminatorValue}}"}`)) + {{if and $stamp (not $stamp.Property) -}} + b, err = runtime.JSONMerge(b, []byte(`{{$stamp.JSONPatch}}`)) if err != nil { return err } From e7c701212c14812e7e229d92f7e9a9314797b1d0 Mon Sep 17 00:00:00 2001 From: Marcin Romaszewicz Date: Sun, 12 Jul 2026 10:35:41 -0700 Subject: [PATCH 3/3] Address code review comments --- .../test/aggregates/oneof/components.gen.go | 32 ++++ .../aggregates/oneof/discriminator.gen.go | 168 ++++++++++++++++++ .../aggregates/oneof/discriminator_test.go | 36 +++- .../aggregates/oneof/spec_discriminator.yaml | 30 +++- pkg/codegen/schema.go | 33 ++-- pkg/codegen/templates/union.tmpl | 4 +- 6 files changed, 281 insertions(+), 22 deletions(-) diff --git a/internal/test/aggregates/oneof/components.gen.go b/internal/test/aggregates/oneof/components.gen.go index 9b744ed830..1b9ec75d8a 100644 --- a/internal/test/aggregates/oneof/components.gen.go +++ b/internal/test/aggregates/oneof/components.gen.go @@ -680,6 +680,10 @@ func (t OneOfObject13) AsOneOfVariant1() (OneOfVariant1, error) { func (t *OneOfObject13) FromOneOfVariant1(v OneOfVariant1) error { t.Type = "v1" b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"type":"v1"}`)) t.union = b return err } @@ -691,6 +695,10 @@ func (t *OneOfObject13) MergeOneOfVariant1(v OneOfVariant1) error { if err != nil { return err } + b, err = runtime.JSONMerge(b, []byte(`{"type":"v1"}`)) + if err != nil { + return err + } merged, err := runtime.JSONMerge(t.union, b) t.union = merged @@ -708,6 +716,10 @@ func (t OneOfObject13) AsOneOfVariant6() (OneOfVariant6, error) { func (t *OneOfObject13) FromOneOfVariant6(v OneOfVariant6) error { t.Type = "v6" b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"type":"v6"}`)) t.union = b return err } @@ -719,6 +731,10 @@ func (t *OneOfObject13) MergeOneOfVariant6(v OneOfVariant6) error { if err != nil { return err } + b, err = runtime.JSONMerge(b, []byte(`{"type":"v6"}`)) + if err != nil { + return err + } merged, err := runtime.JSONMerge(t.union, b) t.union = merged @@ -1619,6 +1635,10 @@ func (t OneOfObject9) AsOneOfVariant1() (OneOfVariant1, error) { func (t *OneOfObject9) FromOneOfVariant1(v OneOfVariant1) error { t.Type = "v1" b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"type":"v1"}`)) t.union = b return err } @@ -1630,6 +1650,10 @@ func (t *OneOfObject9) MergeOneOfVariant1(v OneOfVariant1) error { if err != nil { return err } + b, err = runtime.JSONMerge(b, []byte(`{"type":"v1"}`)) + if err != nil { + return err + } merged, err := runtime.JSONMerge(t.union, b) t.union = merged @@ -1647,6 +1671,10 @@ func (t OneOfObject9) AsOneOfVariant6() (OneOfVariant6, error) { func (t *OneOfObject9) FromOneOfVariant6(v OneOfVariant6) error { t.Type = "v6" b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"type":"v6"}`)) t.union = b return err } @@ -1658,6 +1686,10 @@ func (t *OneOfObject9) MergeOneOfVariant6(v OneOfVariant6) error { if err != nil { return err } + b, err = runtime.JSONMerge(b, []byte(`{"type":"v6"}`)) + if err != nil { + return err + } merged, err := runtime.JSONMerge(t.union, b) t.union = merged diff --git a/internal/test/aggregates/oneof/discriminator.gen.go b/internal/test/aggregates/oneof/discriminator.gen.go index bbde1024d1..f3d79e73a4 100644 --- a/internal/test/aggregates/oneof/discriminator.gen.go +++ b/internal/test/aggregates/oneof/discriminator.gen.go @@ -100,6 +100,12 @@ type PetByKind struct { union json.RawMessage } +// RenamedPetByKind defines model for RenamedPetByKind. +type RenamedPetByKind struct { + Species *string `json:"kind,omitempty"` + union json.RawMessage +} + // ResourceConflictError defines model for ResourceConflictError. type ResourceConflictError struct { Code *ResourceConflictErrorCode `json:"code,omitempty"` @@ -115,6 +121,9 @@ type PostConfigJSONRequestBody = ConfigSaveReq // PostPetJSONRequestBody defines body for PostPet for application/json ContentType. type PostPetJSONRequestBody = PetByKind +// PostRenamedPetJSONRequestBody defines body for PostRenamedPet for application/json ContentType. +type PostRenamedPetJSONRequestBody = RenamedPetByKind + // AsConfigHttp returns the union data inside the ConfigSaveReq as a ConfigHttp func (t ConfigSaveReq) AsConfigHttp() (ConfigHttp, error) { var body ConfigHttp @@ -317,6 +326,10 @@ func (t *PetByKind) FromKindCat(v KindCat) error { var discriminator string = "cat" t.Kind = &discriminator b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"kind":"cat"}`)) t.union = b return err } @@ -329,6 +342,10 @@ func (t *PetByKind) MergeKindCat(v KindCat) error { if err != nil { return err } + b, err = runtime.JSONMerge(b, []byte(`{"kind":"cat"}`)) + if err != nil { + return err + } merged, err := runtime.JSONMerge(t.union, b) t.union = merged @@ -347,6 +364,10 @@ func (t *PetByKind) FromKindDog(v KindDog) error { var discriminator string = "dog" t.Kind = &discriminator b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"kind":"dog"}`)) t.union = b return err } @@ -359,6 +380,10 @@ func (t *PetByKind) MergeKindDog(v KindDog) error { if err != nil { return err } + b, err = runtime.JSONMerge(b, []byte(`{"kind":"dog"}`)) + if err != nil { + return err + } merged, err := runtime.JSONMerge(t.union, b) t.union = merged @@ -431,3 +456,146 @@ func (t *PetByKind) UnmarshalJSON(b []byte) error { return err } + +// AsKindCat returns the union data inside the RenamedPetByKind as a KindCat +func (t RenamedPetByKind) AsKindCat() (KindCat, error) { + var body KindCat + err := json.Unmarshal(t.union, &body) + return body, err +} + +// FromKindCat overwrites any union data inside the RenamedPetByKind as the provided KindCat +func (t *RenamedPetByKind) FromKindCat(v KindCat) error { + var discriminator string = "cat" + t.Species = &discriminator + b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"kind":"cat"}`)) + t.union = b + return err +} + +// MergeKindCat performs a merge with any union data inside the RenamedPetByKind, using the provided KindCat +func (t *RenamedPetByKind) MergeKindCat(v KindCat) error { + var discriminator string = "cat" + t.Species = &discriminator + b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"kind":"cat"}`)) + if err != nil { + return err + } + + merged, err := runtime.JSONMerge(t.union, b) + t.union = merged + return err +} + +// AsKindDog returns the union data inside the RenamedPetByKind as a KindDog +func (t RenamedPetByKind) AsKindDog() (KindDog, error) { + var body KindDog + err := json.Unmarshal(t.union, &body) + return body, err +} + +// FromKindDog overwrites any union data inside the RenamedPetByKind as the provided KindDog +func (t *RenamedPetByKind) FromKindDog(v KindDog) error { + var discriminator string = "dog" + t.Species = &discriminator + b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"kind":"dog"}`)) + t.union = b + return err +} + +// MergeKindDog performs a merge with any union data inside the RenamedPetByKind, using the provided KindDog +func (t *RenamedPetByKind) MergeKindDog(v KindDog) error { + var discriminator string = "dog" + t.Species = &discriminator + b, err := json.Marshal(v) + if err != nil { + return err + } + b, err = runtime.JSONMerge(b, []byte(`{"kind":"dog"}`)) + if err != nil { + return err + } + + merged, err := runtime.JSONMerge(t.union, b) + t.union = merged + return err +} + +func (t RenamedPetByKind) Discriminator() (string, error) { + var discriminator struct { + Discriminator string `json:"kind"` + } + err := json.Unmarshal(t.union, &discriminator) + return discriminator.Discriminator, err +} + +func (t RenamedPetByKind) ValueByDiscriminator() (interface{}, error) { + discriminator, err := t.Discriminator() + if err != nil { + return nil, err + } + switch discriminator { + case "cat": + return t.AsKindCat() + case "dog": + return t.AsKindDog() + default: + return nil, errors.New("unknown discriminator value: " + discriminator) + } +} + +func (t RenamedPetByKind) MarshalJSON() ([]byte, error) { + b, err := t.union.MarshalJSON() + if err != nil { + return nil, err + } + object := make(map[string]json.RawMessage) + if t.union != nil { + err = json.Unmarshal(b, &object) + if err != nil { + return nil, err + } + } + + if t.Species != nil { + object["kind"], err = json.Marshal(t.Species) + if err != nil { + return nil, fmt.Errorf("error marshaling 'kind': %w", err) + } + } + b, err = json.Marshal(object) + return b, err +} + +func (t *RenamedPetByKind) UnmarshalJSON(b []byte) error { + err := t.union.UnmarshalJSON(b) + if err != nil { + return err + } + object := make(map[string]json.RawMessage) + err = json.Unmarshal(b, &object) + if err != nil { + return err + } + + if raw, found := object["kind"]; found { + err = json.Unmarshal(raw, &t.Species) + if err != nil { + return fmt.Errorf("error reading 'kind': %w", err) + } + } + + return err +} diff --git a/internal/test/aggregates/oneof/discriminator_test.go b/internal/test/aggregates/oneof/discriminator_test.go index 423eb1951a..eee89b5261 100644 --- a/internal/test/aggregates/oneof/discriminator_test.go +++ b/internal/test/aggregates/oneof/discriminator_test.go @@ -79,7 +79,9 @@ func TestIssue2297PointerDiscriminatorOnVariant(t *testing.T) { // TestIssue2297PointerDiscriminatorOnUnion covers the union schema declaring // the discriminator property itself, optional, so the field on the union -// struct is pointer-typed and is set through an addressable value. +// struct is pointer-typed and is set through an addressable value. The value +// is also stamped into the union data, so Discriminator() and +// ValueByDiscriminator() work immediately after From*. func TestIssue2297PointerDiscriminatorOnUnion(t *testing.T) { var pet PetByKind meow := "prrr" @@ -88,12 +90,38 @@ func TestIssue2297PointerDiscriminatorOnUnion(t *testing.T) { require.NotNil(t, pet.Kind) require.Equal(t, "cat", *pet.Kind) - b, err := pet.MarshalJSON() + d, err := pet.Discriminator() require.NoError(t, err) - require.Contains(t, string(b), `"kind":"cat"`) + require.Equal(t, "cat", d) - cat, err := pet.AsKindCat() + v, err := pet.ValueByDiscriminator() require.NoError(t, err) + cat, ok := v.(KindCat) + require.True(t, ok) require.NotNil(t, cat.Meow) require.Equal(t, meow, *cat.Meow) + + b, err := pet.MarshalJSON() + require.NoError(t, err) + require.Contains(t, string(b), `"kind":"cat"`) +} + +// TestIssue2297RenamedDiscriminatorField covers a union whose discriminator +// property is renamed via x-go-name: the field is matched by its JSON name, +// assigned through the renamed Go field, and stamped into the union data. +func TestIssue2297RenamedDiscriminatorField(t *testing.T) { + var pet RenamedPetByKind + bark := "woof" + require.NoError(t, pet.FromKindDog(KindDog{Bark: &bark})) + + require.NotNil(t, pet.Species) + require.Equal(t, "dog", *pet.Species) + + d, err := pet.Discriminator() + require.NoError(t, err) + require.Equal(t, "dog", d) + + b, err := pet.MarshalJSON() + require.NoError(t, err) + require.Contains(t, string(b), `"kind":"dog"`) } diff --git a/internal/test/aggregates/oneof/spec_discriminator.yaml b/internal/test/aggregates/oneof/spec_discriminator.yaml index 772d28f0a2..da1d532cc8 100644 --- a/internal/test/aggregates/oneof/spec_discriminator.yaml +++ b/internal/test/aggregates/oneof/spec_discriminator.yaml @@ -32,6 +32,17 @@ paths: responses: "200": description: OK + /renamed-pet: + post: + summary: Union whose discriminator property is renamed via x-go-name + requestBody: + content: + application/json: + schema: + $ref: "#/components/schemas/RenamedPetByKind" + responses: + "200": + description: OK components: schemas: ConfigHttp: @@ -138,4 +149,21 @@ components: type: object properties: bark: - type: string \ No newline at end of file + type: string + # The union's discriminator field is renamed with x-go-name: the field + # detection must match on the JSON property name, and the assignment + # must use the renamed Go field. + RenamedPetByKind: + type: object + properties: + kind: + type: string + x-go-name: Species + oneOf: + - $ref: "#/components/schemas/KindCat" + - $ref: "#/components/schemas/KindDog" + discriminator: + propertyName: kind + mapping: + cat: "#/components/schemas/KindCat" + dog: "#/components/schemas/KindDog" \ No newline at end of file diff --git a/pkg/codegen/schema.go b/pkg/codegen/schema.go index f19c66555f..5d517eb264 100644 --- a/pkg/codegen/schema.go +++ b/pkg/codegen/schema.go @@ -378,24 +378,25 @@ func (d *Discriminator) PropertyName() string { // DiscriminatorStamp describes how the generated From*/Merge* union helpers // record the discriminator value for one union element. // -// When the union struct itself declares the discriminator property, the -// helpers assign Value to that field: its JSON rendering overwrites the -// union data, so the field is load-bearing. Otherwise the value is merged -// into the marshaled JSON via JSONPatch, which stays correct regardless of -// how the variant declares the property — pointer, named enum type, renamed -// or absent field, or a type in an imported package — none of which is -// knowable from the union's side (see issue #2297). +// The value is always merged into the marshaled JSON via JSONPatch, so +// Discriminator() and ValueByDiscriminator() — which read the union data — +// see it immediately after From*/Merge*. Stamping at the JSON level stays +// correct regardless of how the variant declares the property — pointer, +// named enum type, renamed or absent field, or a type in an imported +// package — none of which is knowable from the union's side (see issue +// #2297). When the union struct itself declares the discriminator property, +// the helpers additionally assign Value to that field: its JSON rendering +// overwrites the union data in MarshalJSON, so the field is load-bearing. type DiscriminatorStamp struct { // Value is the discriminator value mapped to this union element. Value string - // Property is the union struct's own discriminator field, when it - // declares one; nil means the value is merged into the JSON instead. + // Property is the union struct's own discriminator field, matched by + // JSON property name, when it declares one; nil when it doesn't. Property *Property - // JSONPatch is the JSON object literal merged into the union data when - // Property is nil, e.g. {"code":"resource_exists"}. Safe to embed in a - // backtick string literal: spec validation rejects discriminator - // property names and mapping keys containing quotes, backticks or - // control characters. + // JSONPatch is the JSON object literal merged into the union data, + // e.g. {"code":"resource_exists"}. Safe to embed in a backtick string + // literal: spec validation rejects discriminator property names and + // mapping keys containing quotes, backticks or control characters. JSONPatch string } @@ -421,8 +422,10 @@ func (s Schema) DiscriminatorStampFor(element UnionElement) *DiscriminatorStamp return nil } stamp.JSONPatch = fmt.Sprintf(`{"%s":"%s"}`, d.Property, stamp.Value) + // Match by JSON property name: the discriminator is a JSON-level + // concept, and the Go field may be renamed via x-go-name. for i := range s.Properties { - if s.Properties[i].GoFieldName() == d.PropertyName() { + if s.Properties[i].JsonFieldName == d.Property { stamp.Property = &s.Properties[i] } } diff --git a/pkg/codegen/templates/union.tmpl b/pkg/codegen/templates/union.tmpl index ce8fe2dad9..a867c3b8e1 100644 --- a/pkg/codegen/templates/union.tmpl +++ b/pkg/codegen/templates/union.tmpl @@ -22,7 +22,7 @@ {{end -}} {{end -}} b, err := json.Marshal(v) - {{if and $stamp (not $stamp.Property) -}} + {{if $stamp -}} if err != nil { return err } @@ -46,7 +46,7 @@ if err != nil { return err } - {{if and $stamp (not $stamp.Property) -}} + {{if $stamp -}} b, err = runtime.JSONMerge(b, []byte(`{{$stamp.JSONPatch}}`)) if err != nil { return err