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

describing nullable field by anyOf leads to bad swagger-ui #2094

Open
ChMThiel opened this issue Dec 6, 2024 · 8 comments
Open

describing nullable field by anyOf leads to bad swagger-ui #2094

ChMThiel opened this issue Dec 6, 2024 · 8 comments
Labels
question Further information is requested

Comments

@ChMThiel
Copy link

ChMThiel commented Dec 6, 2024

Using OpenApi 3.1.0 nullable fields are described using anyOf:

---
openapi: 3.1.0
components:
  schemas:
    DTO:
      type: object
      properties:
        name:
          type: string
          examples:
          - Hello World
        localTime:
          $ref: "#/components/schemas/LocalTime"
          type: string
          examples:
          - 13:45:30.123456789
          - 13:45:30.123456789
        localTimeNullable:
          type:
          - string
          - "null"
          examples:
          - 13:45:30.123456789
          - 13:45:30.123456789
          anyOf:
          - $ref: "#/components/schemas/LocalTime"
          - type: "null"
    LocalTime:
      type: string
      format: local-time
      examples:
      - 13:45:30.123456789
      externalDocs:
        description: As defined by 'partial-time' in RFC3339
        url: https://www.rfc-editor.org/rfc/rfc3339.html#section-5.6
paths:
  /hello:
    get:
      responses:
        "200":
          description: OK
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/DTO"
      summary: Hello
      tags:
      - Greeting Resource
info:
  title: code-with-quarkus API
  version: 1.0.0-SNAPSHOT

In swagger-ui this yaml will be rendered as follows:
grafik

As you can see (especially if collapsed) the 'real' type of the nullable field is not easy to see.

IMHO instead of using anyOf to describe that the field is nullable, using $ref and a type-array would be better:

---
openapi: 3.1.0
components:
  schemas:
    DTO:
      type: object
      properties:
        name:
          type: string
          examples:
          - Hello World
        localTime:
          $ref: "#/components/schemas/LocalTime"
          type: string
          examples:
          - 13:45:30.123456789
          - 13:45:30.123456789
        localTimeNullable:
          $ref: "#/components/schemas/LocalTime"
          type:
          - string
          - "null"
          examples:
          - 13:45:30.123456789
          - 13:45:30.123456789
    LocalTime:
      type: string
      format: local-time
      examples:
      - 13:45:30.123456789
      externalDocs:
        description: As defined by 'partial-time' in RFC3339
        url: https://www.rfc-editor.org/rfc/rfc3339.html#section-5.6
paths:
  /hello:
    get:
      responses:
        "200":
          description: OK
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/DTO"
      summary: Hello
      tags:
      - Greeting Resource
info:
  title: code-with-quarkus API
  version: 1.0.0-SNAPSHOT

This would be rendered as follows
grafik

AFAIK the later kind is valid OpenApi 3.1.0, https://editor-next.swagger.io/ shows no warnings/errors

see https://github.com/ChMThiel/quarkus_demo/tree/OpenApi_generator_3.1.0_nullable for reproducer

@Felk
Copy link

Felk commented Dec 6, 2024

If I understand the spec correctly, directly using $ref without anyOf/allOf may violate the OpenAPI spec: swagger-api/swagger-parser#2036 (comment), so while using $ref may look clean, it might not work :(

@MikeEdgar
Copy link
Member

MikeEdgar commented Dec 8, 2024

I don't think the proposed solution is a valid schema. Combining the $ref to the LocalTime (which itself has type: string) with the type: [ "null", "string" ] in the property schema results in a schema that will not pass for null cases. This is because the LocalTime enforces that the value is non-null.

It may be possible only if type actually overrides the type from the $ref, but I'm not sure that's the case.

My reading of section 7.5 of the JSON Schema spec [1] is that they are treated separately, so the null would be rendered invalid without the anyOf wrapping the $ref and the type: null schemas.

[1] https://json-schema.org/draft/2020-12/draft-bhutton-json-schema-01#name-applicators

@ChMThiel
Copy link
Author

ChMThiel commented Dec 9, 2024

Thank you @Felk and @MikeEdgar for your quick responses.
Frankly - I don't know whats the correct solution:

  • Is there a better way to describe/define the case in the openApi.yml? ... or
  • should Swagger-UI handle the null-cases differently?

All i know is that in OpenApi 3.0.3 the description in swagger-ui was different for the reader (I got a ticket in our project, that the swagger-ui description changed a lot since we updated to Quarkus 3.17 so that it's much more difficult for the user to understand the APIs).

  • Prior to Quarkus 3.17 a nullable LocalTime-field was decribed in swagger-ui:
    grafik

  • With Quarkus 3.17.3 per default OpenApi 3.1.0 is generated leading to the screenshot at the top of the issue (with deeply nested anyOf)

  • Using Quarkus 3.17.3 and setting the property mp.openapi.extensions.smallrye.openapi=3.0.3 the resulting OpenApi/Swagger-UI is as follows:

---
openapi: 3.0.3
components:
  schemas:
    DTO:
      type: object
      properties:
        name:
          type: string
          example: Hello World
        localTime:
          type: string
          allOf:
          - $ref: "#/components/schemas/LocalTime"
          example: 13:45:30.123456789
        localTimeNullable:
          type: string
          example: 13:45:30.123456789
          anyOf:
          - $ref: "#/components/schemas/LocalTime"
          - enum:
            - null
          nullable: true
    LocalTime:
      format: local-time
      type: string
      externalDocs:
        description: As defined by 'partial-time' in RFC3339
        url: https://www.rfc-editor.org/rfc/rfc3339.html#section-5.6
      example: 13:45:30.123456789
paths:
  /hello:
    get:
      responses:
        "200":
          description: OK
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/DTO"
      summary: Hello
      tags:
      - Greeting Resource
info:
  title: code-with-quarkus API
  version: 1.0.0-SNAPSHOT

grafik

I know that seems to be a low prio issue. Using/displaying anyOf is technically correct.
But I have a huge OpenApi with a lot of big DTOs with a lot of nullable fields. Per default we do not expand (quarkus.swagger-ui.default-model-expand-depth=0) the big objects because thats to verbose. Up to Quarkus 3.16 that was ok, because even on first expand the type/null info was shown. Since 3.17 the user sees only string | null and no longer the 'real' datatype.

So i have no way to give the user what he wants. ;(

Maybe i should open a related ticket in swagger-ui.

p.s. there seems a problem with OpenAPi-Validator APIDevTools/swagger-parser#262 so that i can not validate if the OpenApi.yaml i suggested is valid...At least https://editor-next.swagger.io/ digested it without any complains

@ChMThiel
Copy link
Author

ChMThiel commented Dec 9, 2024

In my search for an existing issue in swagger-ui i found the following: swagger-api/swagger-ui#9056

type: […, "null"] is the preferable way to define nullable in v3.1, using the alternative anyOf: […,{type: "null"}] instead results in a additional/unnecessary level of indirection in models generated from description documents.

according to this

localTimeNullable:
  $ref: "#/components/schemas/LocalTime"
  type:
  - string
  - "null"

might be correct and the better solution...

@MikeEdgar
Copy link
Member

I don't think that representation would give the expected results with JSON Schema. That root type doesn't override the type in the schema being referenced, it is an additional constraint. See the first paragraph about applicators [1].

Basically, the schema would validated against a non-null instance since both the $ref and the types would match. However, a null instance would only pass the root schema's type and fail against the $ref.

[1] https://json-schema.org/draft/2020-12/json-schema-core#section-7.5

@ChMThiel
Copy link
Author

According to OAI/OpenAPI-Specification#3148 both ways are valid (even if the example there uses no $ref)....but i'm no JSON-Schema expert.
All i can say is that the 'simpler' openApi is much nicer rendered in swagger-ui and is valid according to https://editor-next.swagger.io/ (as mentioned obove)

@MikeEdgar
Copy link
Member

I think the presence of the $ref makes the difference though, because it introduces type being declared twice to be applied to any instance.

You can play around with variations of JSON schema with a validator [1].

Consider the following JSON schema that uses the anyOf:

{
  "$schema": "https://json-schema.org/draft/2020-12/schema",
  "$defs": {
    "localtime": {
      "type": "string",
      "format": "local-time",
      "pattern": "\\d{2}:\\d{2}:\\d{2}"
    },
  },
  "type": "object",
  "properties": {
    "value": {
      "type": [ "null", "string" ],
      "anyOf": [
        { "$ref": "#/$defs/localtime" },
        { "type": "null" }
      ]
    }
  }
}

Both of these json inputs are valid:

{
	"value": "12:45:50"
}

{
	"value": null
}

However, if we modify the schema to remove the anyOf and directly use the $ref, the null value is no longer valid. This is because the value must match both the root type (which it does) as well as the schema being referenced.

{
  "$schema": "https://json-schema.org/draft/2020-12/schema",
  "$defs": {
    "localtime": {
      "type": "string",
      "format": "local-time",
      "pattern": "\\d{2}:\\d{2}:\\d{2}"
    },
  },
  "type": "object",
  "properties": {
    "value": {
      "type": [ "null", "string" ],
      "$ref": "#/$defs/localtime"
    }
  }
}

image

[1] https://www.jsonschemavalidator.net/

@ChMThiel
Copy link
Author

Thank you @MikeEdgar for your clarification.
I wrote a comment swagger-api/swagger-ui#9056 (comment)
Maybe there is a better way to render the type/null-information in swagger-ui...

@MikeEdgar MikeEdgar added the question Further information is requested label Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants