Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Errors displaying schemas with oneOf #9790

Closed
glowcloud opened this issue Apr 8, 2024 · 4 comments
Closed

Errors displaying schemas with oneOf #9790

glowcloud opened this issue Apr 8, 2024 · 4 comments

Comments

@glowcloud
Copy link
Contributor

Q&A (please complete the following information)

  • OS: macOs
  • Browser: chrome
  • Version: 123.0.6312.107
  • Swagger-UI version: 5.14.0
  • Swagger/OpenAPI version: OpenAPI 3.0

Content & configuration

Example Swagger/OpenAPI definition:

openapi: 3.0.0
info:
  title: Test
  description: 'Test'
  license:
    name: 'Apache 2.0'
    url: 'https://www.apache.org/licenses/LICENSE-2.0.html'
  version: '1.0'
servers:
  -
    url: 'https://localhost:8000'
components:
  schemas:
    OneOfParent:
      title: OneOfParent
      properties:
        additionalData:
          oneOf:
            -
              $ref: '#/components/schemas/FirstOneOf'
            -
              $ref: '#/components/schemas/SecondOneOf'
            -
              $ref: '#/components/schemas/ThirdOneOf'
      type: object
    FirstOneOf:
      title: FirstOneOf
      type: object
      allOf:
        -
          $ref: '#/components/schemas/OneOfParent'
        -
          properties:
            numberProp:
              type: number
              example: '1'
          type: object
    SecondOneOf:
      title: SecondOneOf
      type: object
      allOf:
        -
          $ref: '#/components/schemas/OneOfParent'
        -
          properties:
            person:
              properties:
                id:
                  type: string
              type: object
          type: object
    ThirdOneOf:
      title: ThirdOneOf
      type: object
      allOf:
        -
          $ref: '#/components/schemas/OneOfParent'
        -
          properties:
            person:
              properties:
                id:
                  type: string
                name:
                  type: string
                surname:
                  type: string
              type: object
          type: object

Describe the bug you're encountering

There are issues displaying OneOf objects in Swagger UI. It works properly in Swagger Editor.

In Swagger UI:

Screenshot 2024-04-08 at 14 58 16

In Swagger Editor:

Screenshot 2024-04-08 at 14 57 25

To reproduce...

Steps to reproduce the behavior:

  1. Load the specification
  2. Expand OneOfParent
  3. See that the names of the schemas are different than Swagger Editor and only include the referenced schema properties

Expected behavior

The names should be displayed the same way as in Swagger Editor.

@glowcloud
Copy link
Contributor Author

glowcloud commented Apr 8, 2024

The issue on the surface looks like it was introduced by 7300e6c but it seems like it actually uncovered a different issue, which you can see in Swagger Editor.

The names there are displayed correctly:

Screenshot 2024-04-08 at 15 35 55

but if we actually expand the schemas:
Screenshot 2024-04-08 at 15 38 58

you can see that additionalData from FirstOneOf is missing and the references are also not resolved properly.

The resolved subtree for OneOfParent:

{
    "title": "OneOfParent",
    "properties": {
        "additionalData": {
            "oneOf": [
                {
                    "$ref": "http://localhost:3200/examples/regressionJson.json#/components/schemas/OneOfParent",
                    "properties": {
                        "numberProp": {
                            "type": "number",
                            "example": "1"
                        }
                    },
                    "type": "object",
                    "title": "FirstOneOf",
                    "$$ref": "http://localhost:3200/examples/regressionJson.json#/components/schemas/FirstOneOf"
                },
                {
                    "$ref": "http://localhost:3200/examples/regressionJson.json#/components/schemas/OneOfParent",
                    "properties": {
                        "person": {
                            "properties": {
                                "id": {
                                    "type": "string"
                                }
                            },
                            "type": "object"
                        }
                    },
                    "type": "object",
                    "title": "SecondOneOf",
                    "$$ref": "http://localhost:3200/examples/regressionJson.json#/components/schemas/SecondOneOf"
                },
                {
                    "$ref": "http://localhost:3200/examples/regressionJson.json#/components/schemas/OneOfParent",
                    "properties": {
                        "person": {
                            "properties": {
                                "id": {
                                    "type": "string"
                                },
                                "name": {
                                    "type": "string"
                                },
                                "surname": {
                                    "type": "string"
                                }
                            },
                            "type": "object"
                        }
                    },
                    "type": "object",
                    "title": "ThirdOneOf",
                    "$$ref": "http://localhost:3200/examples/regressionJson.json#/components/schemas/ThirdOneOf"
                }
            ]
        }
    },
    "type": "object"
}

We can see both the resolved $$ref and unresolved $ref - because of this the names are different and we only get the schema under $ref, as that was one of the changes in this commit 7300e6c

If we expand FirstOneOf first and then OneOfParent, we'll get this:

Screenshot 2024-04-08 at 15 40 48

This looks to me like an issue with the resolver - perhaps with allOf and circular / unresolved references.

@glowcloud
Copy link
Contributor Author

In theory, we could pass fn to our Model component and use mergeJsonSchema - in place of

if ($ref) {
name = this.getModelName($ref)
const refSchema = this.getRefSchema(name)
if (Map.isMap(refSchema)) {
schema = refSchema.set("$$ref", $ref)
$$ref = $ref
} else {
schema = null
name = $ref
}
}

have something like this:

if ($ref) {
   const refName = this.getModelName($ref)
   const refSchema = this.getRefSchema(refName)
   if (Map.isMap(refSchema)) {
     schema = fromJS(fn.mergeJsonSchema(schema.toJS(), refSchema.toJS()))
     name = name || refName
     if (!$$ref) {
       schema = schema.set("$$ref", $ref)
       $$ref = $ref
     }
   } else if (!$$ref) {
     schema = null
     name = $ref
   }
 }

this will work for the spec in the issue:
Screenshot 2024-04-09 at 09 59 53

But it will not solve it for cases where we have more than one circular reference, as the $ref property will get replaced by the next defined $ref. To illustrate it, we can have this spec:

{
  "openapi": "3.0.0",
  "info": {
    "title": "Test",
    "description": "Test",
    "license": {
      "name": "Apache 2.0",
      "url": "https://www.apache.org/licenses/LICENSE-2.0.html"
    },
    "version": "1.0"
  },
  "servers": [
    {
      "url": "https://localhost:8000"
    }
  ],
  "components": {
    "schemas": {
      "Parent": {
        "title": "Parent",
        "properties": {
          "parentData": {
            "$ref": "#/components/schemas/OneOfParent"
          }
        },
        "type": "object"
      },
      "OneOfParent": {
        "title": "OneOfParent",
        "properties": {
          "additionalData": {
            "oneOf": [
              {
                "$ref": "#/components/schemas/FirstOneOf"
              }
            ]
          }
        },
        "type": "object"
      },
      "FirstOneOf": {
        "title": "FirstOneOf",
        "type": "object",
        "allOf": [
          {
            "$ref": "#/components/schemas/Parent"
          },
          {
            "$ref": "#/components/schemas/OneOfParent"
          },
          {
            "properties": {
              "numberProp": {
                "type": "number",
                "example": "1"
              }
            },
            "type": "object"
          }
        ]
      }
    }
  }
}

and our rendered model will be missing parentData:
Screenshot 2024-04-09 at 09 56 44

If we swap the order of references in allOf in FirstOneOf:

"FirstOneOf": {
      "title": "FirstOneOf",
      "type": "object",
      "allOf": [
        {
          "$ref": "#/components/schemas/OneOfParent"
        },
        {
          "$ref": "#/components/schemas/Parent"
        },
        {
          "properties": {
            "numberProp": {
              "type": "number",
              "example": "1"
            }
          },
          "type": "object"
        }
      ]
    }

This time additionalData will be missing:
Screenshot 2024-04-09 at 09 57 25

It looks to me like we would somehow need to keep these unresolved $refs in an array (as $ref: [<unresolved_ref_0>, <unresolved_ref_1>, ...]) instead of only having a string there, that gets replaced when we get another $ref

@char0n
Copy link
Member

char0n commented Apr 9, 2024

  • we will assume that every schema with $ref field and one or more additional fields is a result of allOf keyword merge
  • given that Reference Object cannot have any additional fields than $ref, the assumption is almost correct
  • we will use Immutable.mergeDeep to merge the referenced schema with the referencing one (which makes it compatible with swagger-client allOf specmap plugin)
  • this can introduce possible circular references (cycles) again into the deferenced tree

@char0n
Copy link
Member

char0n commented Apr 10, 2024

Addressed in #9794

@char0n char0n closed this as completed Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants