microsoft / typespec

https://typespec.io/
MIT License
3.93k stars 177 forks source link

oneOf should be allowed on a model with a discriminator #2500

Open DMaro28 opened 9 months ago

DMaro28 commented 9 months ago

Context: oneOf with union has proven to create issues with code generation from openapi spec generated from Typespec. As a result, we moved to use model with a discriminator, which solves the issue with code generation, but creates another one. Swagger UI doesn't visualize full data models of possible options (models B and C in example below), but only array of enums. This is unacceptable from API user perspective, as oneOf is where the complexity of the API request for the user is.

The below structure would allow the best of both worlds, where ideally swagger UI visualizes the full data models of B and C.

enum Kind {
    B,
    C
}

@discriminator("kind")
@oneOf
model A {
  kind: Kind 
}

model B extends A {
  kind: Kind.B 
}

model C extends A {
  kind: Kind.C
}
timotheeguerin commented 9 months ago

Believe this is a dup of https://github.com/microsoft/typespec/issues/826 where we'd just add the oneOf all the time

bterlson commented 9 months ago

Another approach here is to not use @discriminator on the base type and use an explicit union instead:

enum Kind {
    B,
    C
}

@discriminator("kind")
@oneOf
union A { B; C; }

model ABase {
  kind: Kind 
}

model B extends ABase {
  kind: Kind.B 
}

model C extends ABase {
  kind: Kind.C
}

It does require an additional declaration, though. Thoughts @DMaro28?

DMaro28 commented 9 months ago

This solutions seems to lose A link to ABase in schema. Also, enums have duplicated values. Our technical team says OpenAPI generator has issues handling openapi spec generated with this Typespec.

markcowl commented 9 months ago

@markcowl attach to #826