GeezerVison

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.

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.

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.

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

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

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.

It's on our TODO list: #2170