microsoft / typespec

https://typespec.io/
MIT License
4.35k stars 201 forks source link

[Bug]: cannot describe a oneOf between a string and a model #4077

Closed attilah closed 2 months ago

attilah commented 2 months ago

Describe the bug

In OpenAPI it is valid to have oneOf between a primitive value like string and an object. In TypeSpec I did not find a valid way to apply oneOf as the compiler says oneOf can only be used on models.

Reproduction

No real repro here, only the desired outcome, which is a valid OpenAPI spec with a union where one member is a string the other member is an object.

Checklist

timotheeguerin commented 2 months ago

I assume you explicitly mean here you wanted oneOf instead of anyOf? You can create a named union and use the @oneOf decorator to change the anyOf to oneOf there too https://typespec.io/playground?c=aW1wb3J0ICJAdHlwZXNwZWMvb3BlbmFwaTMiOwoKQE9wZW5BUEkub25lT2YKdW5pb24gTXlVxQh7CiAgc3RyaW5nLAogIEFiYywKfQoKbW9kZWzEDsUgbmFtZTrHJjsKfQo%3D&e=%40typespec%2Fopenapi3&options=%7B%7D

import "@typespec/openapi3";

@OpenAPI.oneOf
union MyUnion {
  string,
  Abc,
}

model Abc {
  name: string;
}
attilah commented 2 months ago

@timotheeguerin ah, thanks...probably the only variation I haven't tried. My thoughts was around polymorphism and discriminated unions, but it's just a simple union.

How would a .NET type using MyUnion would look like would it be able to automatically deserialize it without using discriminated unions and polymorphism? Or I need to supply my JsonConverter for the type where I use this pattern?

timotheeguerin commented 2 months ago

I guess that depends on the csharp library emitter that you use and how smart they want to be. Thie above example doesn't prevent you from using discriminated types. Do you have an example of the openapi3 you would like to produce?

attilah commented 2 months ago

Sure @timotheeguerin, here is an OpenAPI snippet that can demonstrate 2 issues I'm trying to solve:

So these all should be valid (invite is type of invite_attribute:

invite: {
  friend_id: "friend01"
}

or

invite: {
  name: "John",
  nickname: "Johnybravo",
  city: "Redmond",
  state: "WA",
  zip: "98052"
}

or

invite: {
  group: "Johny's Group",
  nickname: "Johnybravo",
  city: "Redmond",
  state: "WA",
  zip: "98052"
}

OpenAPI snippet:

invite_attribute:
  oneOf:
    - $ref: '#/components/schemas/friend_id'
    - $ref: '#/components/schemas/friend'

friend:
  allOf:
    - $ref: '#/components/schemas/friend_editable_fields'
    - type: object
      required:
        - nickname
        - city
        - state
        - zip

friend_editable_fields:
  allOf:
    - $ref: '#/components/schemas/friend_fields'
    - type: object
      anyOf:
        - title: friend name
          required:
            - name
        - title: friend group
          required:
            - group
      properties:
        name:
          type: string
        group:
          $ref: '#/components/schemas/group'
        email:
          type: string

friend_fields:
  type: object
  required:
    - nickname
    - city
    - state
    - zip
  properties:
    nickname:
      type: string
    city:
      type: string
    state:
      type: string
    zip:
      type: string

friend_id:
  type: string

group:
  type: string

I really appreciate helping out with this!

timotheeguerin commented 2 months ago

Yeah the part about having conditionally required field like that is not something TypeSpec supports, This is also something I think is pretty much impossible to represent nicely in most language and falls more into the validation category.

You can take a look a this language feature proposal that we have(no eta on when it will make it if it does) https://github.com/microsoft/typespec/pull/2141 that will let you define some more complex validation rules like this

attilah commented 2 months ago

@timotheeguerin and what about the union related part:

"In invite_attributes either a friend_id must be specified or the fields of a friend but as sibling fields" ?

is that doable?

timotheeguerin commented 2 months ago

Yeah that would just be as above with a named union(+optionally @oneOf decorator)

https://typespec.io/playground?c=aW1wb3J0ICJAdHlwZXNwZWMvb3BlbmFwaTMiOwoKQE9wZW5BUEkub25lT2YKdW5pb24gaW52aXRlX2F0dHJpYnV0ZSB7CiAgZnJpZW5kX2lkLMkNLAp9Cm1vZGVsyh3OKzogc3RyaW5nO88pxSZncm91cMoiICBuaWNrbmFtZcwUY2l0ecwQc3RhdM0hemnLRH0K&e=%40typespec%2Fopenapi3&options=%7B%7D

but not clear how different emitters that don't support union would represent that

attilah commented 2 months ago

Thanks I try the suggested solution.

Luckily I'm hand writing the C# types for now, so I've full control, we will use native Roslyn based code gen based on the tsp model in the future.