Skip to content

Multiple bug fixes with proper attribute inheritance#661

Merged
n0tl3ss merged 2 commits intomicronaut-projects:masterfrom
n0tl3ss:bug/659
Feb 23, 2022
Merged

Multiple bug fixes with proper attribute inheritance#661
n0tl3ss merged 2 commits intomicronaut-projects:masterfrom
n0tl3ss:bug/659

Conversation

@n0tl3ss
Copy link
Member

@n0tl3ss n0tl3ss commented Feb 10, 2022

This code change fixes issue with inheritance of "description" and "nullable" attributes. The fields annotated with @Schema should always consider their "description" and "nullable" arguments. The "required" argument should be only displayed in the list of the required properties of the DTO.

Fix #659

@n0tl3ss n0tl3ss changed the title test and fix for #659 Multiple bug fixes with proper attribute inheritance Feb 10, 2022
@swaechter
Copy link

swaechter commented Mar 13, 2022

@n0tl3ss I just tested your fix with Micronaut 3.3.4 and Micronaut OpenAPI 4.0.1 Snapshot. A lot of our issues were fixed. A big thank you!

I found one issue regarding the nullability of a type. The new generated code looks like this (Attributes like protocol can be null when email sending is disabled):

    ReadEmailOutputLocationDto:
      required:
      - protocol
      type: object
      properties:
        protocol:
          allOf:
          - $ref: '#/components/schemas/EmailSendProtocolDto'
          - description: Protocol used for the connection
      description: Schema that represents an existing email output location
    ReadEmailSettingsDto:
      required:
      - active
      - hostname
      - plaintextPassword
      - port
      - protocol
      - senderEmail
      - username
      type: object
      properties:
        active:
          type: boolean
          description: "Flag that indicates whether the email sending is active or\
            \ not. If set to false, all other values are null"
        hostname:
          type: string
          description: Hostname or IP of the email server or null if email sending
            is disabled
          nullable: true
        port:
          type: integer
          description: Port of the email server or null if email sending is disabled
          format: int32
          nullable: true
        protocol:
          allOf:
          - $ref: '#/components/schemas/EmailSendProtocolDto'
          - description: Protocol used for the connection or null if email sending
              is disabled
            nullable: true
        username:
          type: string
          description: Email username to login or null if email sending is disabled
          nullable: true
        plaintextPassword:
          type: string
          description: Plaintext password for the email user to login in or null if
            email sending is disabled
          nullable: true
        senderEmail:
          type: string
          description: Sender email address that is used to send emails or null if
            email sending is disabled
          nullable: true
      description: Schema that represents the current email settings

Problematic is the location of the nullable:

    ReadEmailSettingsDto:
      required:
        SNIPPED
      type: object
      properties:
        SNIPPED OTHER PROPERTIES
        protocol:
          allOf:
          - $ref: '#/components/schemas/EmailSendProtocolDto'
          - description: Protocol used for the connection or null if email sending
              is disabled
            nullable: true <------------------------------------- Should not be down here

According to https://stackoverflow.com/questions/40920441/how-to-specify-a-property-can-be-null-or-a-reference-with-swagger the null should be at before the allOf, so like this:

    ReadEmailSettingsDto:
      required:
        SNIPPED
      type: object
      properties:
        SNIPPED OTHER PROPERTIES
        protocol:
          nullable: true <------------------------------------- Moved up here
          allOf:
          - $ref: '#/components/schemas/EmailSendProtocolDto'
          - description: Protocol used for the connection or null if email sending
              is disabled

Moving all nullable=true used in combination with allOf in our code fixes the OpenAPI generation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Micronaut OpenAPI wrongly reuses schema description and default values like nullable (Leads to wrongly generated Swagger schema)

3 participants