Skip to content

Conversation

@vikstrous
Copy link

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_a was processed first and included (making type_b fail at runtime)
  • Sometimes type_b was processed first and included (making type_a fail 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 break that 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 $matched flag in From* and Merge* 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 to Discriminator struct
    • Fixed generateUnion() to sort keys and include all mappings
    • Improved discriminator validation logic
  • 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

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:

  1. ValueByDiscriminator() switch: Now includes ALL discriminator values as cases (previously missing random values)
  2. From* methods: Consistently use the first alphabetically sorted discriminator value
  3. 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_MultipleMappingsToSameSchema validates all discriminator values work
  • ✅ All existing tests pass
  • ✅ Generated code includes all discriminator cases in switch statements
  • ✅ Code generation is deterministic across multiple runs

@vikstrous vikstrous requested a review from a team as a code owner October 20, 2025 17:51
@tobio
Copy link
Contributor

tobio commented Oct 20, 2025

I believe this is a duplicate of #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

@vikstrous
Copy link
Author

The other one looks like a smaller diff. Feel free to close this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants