Skip to content

Conversation

@RasankRam
Copy link

@RasankRam RasankRam commented Oct 25, 2025

I have added the implementation of code generation for discriminators used with allOf schemas. OpenAPI Spec

Feature for #666

Some notes

  • Added support discriminator defined at schema level and inline allOf element level
  • Prevented merging schemas with multiple discriminators
  • Preserved discriminator types during pruning optimization
  • Added codegen tmpl
  • Added tests and examples
  • Added server-side code generation support for handling polymorphic types

Code generation

  • Add allof-discriminator.tmpl template for discriminator-based type generation
  • Generate methods for polymorphic types:
    • Discriminator() - returns discriminator property value
    • Is<Type>() - type checking helpers (e.g., IsCat(), IsDog())
    • As<Type>() - type casting methods with validation (e.g., AsCat(), AsDog())
    • ValueByDiscriminator() - type resolution based on discriminator value
    • UnmarshalJSON() - unmarshaling to preserve raw JSON for type conversion
  • Server-Side Support
    • Generate server handler interfaces that accept discriminated union types as parameters
    • Automatic type discrimination in request body parsing for polymorphic endpoints
    • Support for strict server mode with discriminator validation

@RasankRam RasankRam requested a review from a team as a code owner October 25, 2025 05:54
@vernak2539
Copy link

Hey @jamietanna, we meet again! How's it going??

Our teams have just run into this issue as well. Seems like this has been unsupported since 2022 (first issue). Totally get there's likely a reason as well.

Any chance that this PR could get looked at sooner than later? Totally cool if not, but figured I'd shoot my shot.

@mromaszewicz
Copy link
Member

mromaszewicz commented Oct 29, 2025

This is a nice change, however, it's also a bit scary, and I do believe it is breaking, so that it would have to be put behind a default-disabled flag. allOf/anyOf/oneOf have been very difficult to support and I tried all kinds of things until we got where we are today as a least broken approach.

The reason it's breaking is that you can no longer generate allOf models for two models which have discriminators, since in the past, we ignored them. You've added a new failure mode, but this should be very rare. I've found that rare things happen all the time with this project :)

It's a bit tricky to wrap your head around the meaning of recursive allOf's with discriminators, however I really like what you did, nice work!

Now, let's chat about discriminators, and merging schemas with contain them.

Say we have a top level type, Pet, with the petType discriminator as appears everywhere. Let's add another top level type Pest with the pestType discriminator.

As I understand your code, if I was to make something like this pseudocode:

schema:
   Mouse:
      allOf:
        - Pet
        - Pest

It would fail because of discriminator conflict.

Now, is this a valid example? I don't know.

If it worked, a Mouse would need to have petType and pestType properties, but they aren't in conflict with each other. A Mouse is still a Pet and it is also a Pest and anything which expects either of them should be able to accept a Mouse. I'm not sure if we should reject it.

I think the only real problem happens if both of the constituent schemas share a discriminator by the same name.

Should the top level merged type even have a discriminator? I'm not sure if it needs one to work, but it does need to have one given how OpenAPI specifies that properties are merged in allOf

Anyhow, edge cases like these are why I avoided solving as much of this problem as possible, and left it to application logic to figure it out.

@RasankRam RasankRam force-pushed the feature/codegen-discriminator-allof branch from ced092f to 4e63505 Compare November 6, 2025 00:56
@RasankRam
Copy link
Author

Hi, @mromaszewicz . Thanks for your feedback!

Regarding the breaking change - indeed, my changes were introducing a breaking change. I reconsidered the discriminator logic - I had code that inherited discriminators when merging allOf. Now, the discriminator is only extracted from the original schema.

I agree with the Mouse example. I implemented support for this functionality and wrote tests. So Mouse with both petType and pestType properties is now valid, since they're different properties. This aligns with OpenAPI spec's property merging behavior.

The only case that still errors out is when both schemas have discriminators with the same property name - I don't know what to do with this case, so I'm rejecting it.

I pushed the changes, rebased, and squashed the commits.

Also, I would like to ask a question about the necessity of the Discriminator() method (allof-discriminator.tmpl). I'm wondering if this is necessary, given that:

  • We generate type-checking methods like IsCat(), IsDog() for runtime type identification
  • Users can access the property directly (e.g., pet.PetType)

I added it for compatibility with unionElements (union.tmpl). The only use case I can think of is for debugging.
Since I don't have extensive experience in open source development, I can ignore all possible use cases.

@Boris-creator
Copy link

Hello @jamietanna, any updates on this issue? We’re also missing support for allOf + discriminator.

@lenta-platform
Copy link

Hi, we're also really looking forward to this feature

@RasankRam RasankRam force-pushed the feature/codegen-discriminator-allof branch from 4e63505 to f8829bd Compare November 17, 2025 02:27
@RasankRam

This comment was marked as off-topic.

@github-actions
Copy link

Thanks for your use of oapi-codegen!
We are very appreciative of your interest in improving the project, and that you're engaged with us.
However, @-mentioning the maintainers will be very unlikely to help move your issue along, and we do not want to encourage behaviour that leads to the "noisiest" users getting the benefit over folks who are waiting their turn.
Please remember that oapi-codegen is for the most part run by volunteers, and we have a request out to companies who use the project to help make it more sustainable, with sponsorship.
It will only be that sustained financial support will be able to make it possible to work more efficiently towards user requests.
For more info on engaging with the maintainers, see our CONTRIBUTING guide.

@jamietanna
Copy link
Member

It's on our TODO list: #2170

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants