fix codegen for same type used with multiple discriminators
Fix Missing Discriminator Mappings When Multiple Values Map to Same Schema
Problem
When an OpenAPI spec has multiple discriminator values that map to the same schema type, the code generator would only include one of the values in the generated code, and which value was included would vary randomly between runs.
Example
Given an OpenAPI spec with discriminator mappings like:
discriminator:
propertyName: category
mapping:
type_a: '#/components/schemas/VariantA'
type_b: '#/components/schemas/VariantA'
type_x: '#/components/schemas/VariantB'
The generated ValueByDiscriminator() switch would be incomplete, randomly including either type_a OR type_b, but not both:
switch discriminator {
case "type_a": // Sometimes "type_b" instead - randomly varies!
return t.AsVariantA()
case "type_x":
return t.AsVariantB()
default:
return nil, errors.New("unknown discriminator value: " + discriminator)
}
This means valid discriminator values specified in the OpenAPI spec would fail at runtime with "unknown discriminator value" errors.
Root Cause
The discriminator mapping builder in generateUnion() iterated over the spec's discriminator map and broke after finding the first match:
for k, v := range discriminator.Mapping {
if v == element.Ref {
outSchema.Discriminator.Mapping[k] = elementSchema.GoType
mapped = true
break // ❌ Only includes ONE random value per schema
}
}
Since Go map iteration order is randomized, this meant:
- Sometimes
type_awas processed first and included (makingtype_bfail at runtime) - Sometimes
type_bwas processed first and included (makingtype_afail at runtime) - The code generation varied randomly between runs
Solution
1. Include All Discriminator Mappings
Updated generateUnion() in schema.go to:
- Process all discriminator keys in sorted order (for deterministic output)
- Include all keys that map to each schema type
- Remove the early
breakthat caused mappings to be silently dropped
Before:
for k, v := range discriminator.Mapping {
if v == element.Ref {
outSchema.Discriminator.Mapping[k] = elementSchema.GoType
mapped = true
break // ❌ Only adds ONE mapping per schema
}
}
After:
sortedKeys := make([]string, 0, len(discriminator.Mapping))
for k := range discriminator.Mapping {
sortedKeys = append(sortedKeys, k)
}
sort.Strings(sortedKeys)
for _, k := range sortedKeys {
v := discriminator.Mapping[k]
if v == element.Ref {
outSchema.Discriminator.Mapping[k] = elementSchema.GoType
mapped = true
// ✅ Continue to include ALL mappings
}
}
2. Added SortedMappingKeys() Helper Method
Added a helper method to the Discriminator struct that returns mapping keys in sorted order, ensuring deterministic code generation across runs:
func (d *Discriminator) SortedMappingKeys() []string {
keys := make([]string, 0, len(d.Mapping))
for k := range d.Mapping {
keys = append(keys, k)
}
sort.Strings(keys)
return keys
}
3. Updated Templates
Modified union.tmpl to use sorted keys:
- Use
SortedMappingKeys()instead of direct map iteration for deterministic output - Add
$matchedflag inFrom*andMerge*methods to use the first (alphabetically) discriminator value - Ensures
ValueByDiscriminator()includes ALL discriminator values as switch cases
{{$matched := false -}}
{{range $value := $discriminator.SortedMappingKeys -}}
{{$type := index $discriminator.Mapping $value -}}
{{if and (eq $type $element) (not $matched) -}}
v.{{$discriminator.PropertyName}} = "{{$value}}"
{{$matched = true -}}
{{end -}}
{{end -}}
4. Improved Validation
Updated validation logic to check that all union elements have at least one discriminator mapping, rather than requiring count equality between mappings and elements (which incorrectly fails when multiple discriminator values map to the same schema).
Changes Summary
Modified Files
-
pkg/codegen/schema.go:- Added
SortedMappingKeys()method toDiscriminatorstruct - Fixed
generateUnion()to sort keys and include all mappings - Improved discriminator validation logic
- Added
-
pkg/codegen/templates/union.tmpl:- Updated
From{{ .Method }}to use sorted keys with early-exit - Updated
Merge{{ .Method }}to use sorted keys with early-exit - Updated
ValueByDiscriminator()to use sorted keys
- Updated
Test Coverage
- Added test case with multiple discriminator values mapping to the same schema
- Test verifies code generation is deterministic across multiple runs
- Test validates all discriminator values appear in switch statements
Impact
Generated Code Changes
For schemas with multiple discriminator values mapping to the same type:
-
ValueByDiscriminator()switch: Now includes ALL discriminator values as cases (previously missing random values) -
From*methods: Consistently use the first alphabetically sorted discriminator value -
Merge*methods: Same deterministic behavior
Example:
// Before: INCOMPLETE - missing valid discriminator values!
switch discriminator {
case "type_a": // ❌ type_b is missing! Will fail at runtime
return t.AsVariantA()
case "type_x":
return t.AsVariantB()
default:
return nil, errors.New("unknown discriminator value: " + discriminator)
}
// After: COMPLETE - all discriminator values included
switch discriminator {
case "type_a": // ✅ Both values included
return t.AsVariantA()
case "type_b": // ✅ No longer missing
return t.AsVariantA()
case "type_x":
return t.AsVariantB()
case "type_y":
return t.AsVariantB()
default:
return nil, errors.New("unknown discriminator value: " + discriminator)
}
Backward Compatibility
This is a bug fix that corrects missing discriminator mappings. For projects with multiple discriminator values mapping to the same schema:
Before this fix:
- Runtime errors:
"unknown discriminator value: type_b"for valid discriminator values - Incomplete switch statements missing valid cases
- Random code generation (sometimes worked, sometimes didn't)
After this fix:
- All discriminator values work correctly
- Complete switch statements with all cases
- Deterministic code generation
- The discriminator value used in
From*/Merge*methods is the alphabetically first value
This change fixes a correctness bug where valid OpenAPI discriminator mappings were silently dropped, causing runtime failures.
Verification
- ✅ New test
TestOneOfWithDiscriminator_MultipleMappingsToSameSchemavalidates all discriminator values work - ✅ All existing tests pass
- ✅ Generated code includes all discriminator cases in switch statements
- ✅ Code generation is deterministic across multiple runs
I believe this is a duplicate of https://github.com/oapi-codegen/oapi-codegen/pull/2071. I don't actually care which one is merged, but it would be great to make some progress on one of these if possible @jamietanna
The other one looks like a smaller diff. Feel free to close this one.