Generate unions from oneOf return type

[ftatzky]

According to Swagger: Inheritance and Polymorphism polymorphism can be described with the oneOf keyword in responses. Right now openapi-to-graphql doesn't support that.

This change enables the generation of unions for oneOf return types in responses.

SimpleTest.yaml

openapi: 3.0.2
info:
  version: 1.0.0
  title: Test polymorphism
servers:
  - url: /poly-api
paths:
  '/document/{id}':
    get:
      description: Reads a document.
      operationId: readDocument
      parameters:
        - name: id
          in: path
          description: Document ID
          required: true
          schema:
            type: string
      responses:
        '200':
          description: Successful
          content:
            application/json:
              schema:
                oneOf:
                  - $ref: '#/components/schemas/Article'
                  - $ref: '#/components/schemas/Video'
                discriminator:
                  propertyName: documentType
                  mapping:
                    article: '#/components/schemas/Article'
                    video: '#/components/schemas/Video'
components:
  schemas:
    Document:
      type: object
      properties:
        id:
          type: string
          description: Document ID
        documentType:
          type: string
          enum:
            - article
            - video
          description: Document type
    Article:
      description: Article
      allOf:
        - $ref: '#/components/schemas/Document'
        - type: object
          properties:
            author:
              type: string
              description: Article author
            headline:
              $ref: '#/components/schemas/RichText'
    Video:
      description: Video
      allOf:
        - $ref: '#/components/schemas/Document'
        - type: object
          properties:
            videoUrl:
              type: string
              description: Video source URL
    RichText:
      type: object
      description: Richtext type
      properties:
        data:
          type: object
          description: Raw (JSON)

Generated output before the change

"""
The `JSON` scalar type represents JSON values as specified by [ECMA-404](http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-404.pdf).
"""
scalar JSON

type Query {
  """
  Reads a document.
  
  Equivalent to GET /document/{id}
  """
  document(
    """Document ID"""
    id: String!
  ): JSON
}

Generated output after the change

"""Article"""
type Article {
  """Article author"""
  author: String

  """Document type"""
  documentType: DocumentType

  """Richtext type"""
  headline: RichText

  """Document ID"""
  id: String
}

"""No description available."""
union Document = Article | Video

enum DocumentType {
  ARTICLE
  VIDEO
}

"""
The `JSON` scalar type represents JSON values as specified by [ECMA-404](http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-404.pdf).
"""
scalar JSON

type Query {
  """
  Reads a document.
  
  Equivalent to GET /document/{id}
  """
  document(
    """Document ID"""
    id: String!
  ): Document
}

"""Richtext type"""
type RichText {
  """Raw (JSON)"""
  data: JSON
}

"""Video"""
type Video {
  """Document type"""
  documentType: DocumentType

  """Document ID"""
  id: String

  """Video source URL"""
  videoUrl: String
}

[Alan-Cha]

Unfortunately, I don't think we can do this as oneOf and allOf does not necessitate union or object.

For example, this is an example provided by json-schema:

{
   "type": "number",
   "oneOf": [
     { "multipleOf": 5 },
     { "multipleOf": 3 }
   ]
 }

---

Additionally, even if oneOf contains a number of schemas, we still need to make a number of checks to see if we can still create a union as JSON schema oneOf is a lot more flexible than GraphQL union type.

For example, according to the specification, GraphQL unions cannot be composed of other unions

The member types of a Union type must all be Object base types; Scalar, Interface and Union types must not be member types of a Union. Similarly, wrapping types must not be member types of a Union.

That is why we have this check, to see if we have cases of nested oneOf/allOf.

[Alan-Cha]

This is for sure a bug that needs to be fixed. However, I think we need to approach it differently.

I think the problem actually has to do with the fact that we do not properly identify the type of Article or Video and as a result, we fall back onto the json type for Document.

We're failing this if statement which ensures that all the member types (Article and Video) are object types.

The type data is coming from here and the definition for that is here.

We identify the types of the schema here and unfortunately, Article and Video are determined to be null instead of object because we do not deference the schemas inside.

---

I think what we need to do is to make getSchemaTargetGraphQLType() more robust by considering oneOf/allOf and dereferencing member schemas.

In essence, I think we should move some of the logic from processing the oneOf/allOf into getSchemaTargetGraphQLType().

---

Let me know if this all makes sense to you!

[ftatzky]

@Alan-Cha yeah that makes total sense. As I said, I somehow knew that my changes were probably just to naive and a bandaid and has some major flaws I just couldn't see by myself. I talked to my team an my PO, and it's a story in our current sprint now and I can dedicate actual work time to dive deeper into this problem (which helps a lot). Thanks so far, and I will get back to you as soon as I have something. Btw is there a chat or some plattform you use for this community? Something where interaction is more immediate? If not that's ok, just asking. Best regards

[Alan-Cha]

@ftatzky If you would like to take another look at this problem, that would be super cool! Otherwise, I will have to take a look at this in the upcoming weeks as I'm juggling some other tasks right now. I'm happy to answer whatever questions you have though!

I think it should be easy to medium difficulty, leaning more towards easy. I think you are on the right track. We should have getSchemaTargetGraphQLType() return union. Checking for null and object here when we are trying to create a union is confusing. We just need to follow move the existing logic for determining whether or not to create a union and put it into getSchemaTargetGraphQLType().

Yes! We have a Gitter chat room! Again, would be happy to answer any questions that you may have.

[Alan-Cha]

There is one consideration we should make.

Currently, the plan is to move the existing logic into getSchemaTargetGraphQLType(), we should still consolidate allOf and oneOf here, and basically check if the targetGraphQLType (from getSchemaTargetGraphQLType()) is union here, and only if this is true, will we use the oneOf to create the union.

---

basically check if the targetGraphQLType (from getSchemaTargetGraphQLType()) is union here

Actually on second thought, we should probably just utilize the switch statement

---

My one concern is that we might want to use oneOf to do other things besides creating a union and this kind of logic flow may impede that.

An example of how we can use oneOf for other things is, given the following schema

{
   "type": "number",
   "oneOf": [
     { "multipleOf": 5 },
     { "multipleOf": 3 }
   ]
 }

We could create a new type that would actively check if it fits the multipleOf requirement.

However, for now, I think we only use oneOf to create unions. I'm just saying it would be nice to refactor in such a way that we can still use oneOf for other purposes without making the logic too complicated. For now, we don't necessarily need to do that.

Edit: Actually, I think everything will be fine if we just utilize the switch statement. Don't worry about what I said.

[Alan-Cha]

Let me know if this makes any sense! I might just be rambling 😅