feat: add code generation for discriminator with allOf by RasankRam · Pull Request #2114 · oapi-codegen/oapi-codegen

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.