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

@Schema(required=true/false) ignored when springdoc-openapi-kotlin is in classpath #1285

Closed
Clubfan22 opened this issue Oct 5, 2021 · 3 comments
Labels
wontfix This will not be worked on

Comments

@Clubfan22
Copy link

Describe the bug

When springdoc-openapi-kotlin is in the classpath, the type-derived nullability (e.g. String vs String?) takes precedence over the @Schema annotation for determining whether a field is required or not.
For example, @field:Schema(required=true) member: String? is not a required member in the generated schema.

To Reproduce
Steps to reproduce the behavior:

  • What version of spring-boot you are using?
    2.5.5
  • What modules and versions of springdoc-openapi are you using?
    org,springdoc:springdoc-openapi-ui:1.5.10 and org,springdoc:springdoc-openapi-kotlin:1.5.10
  • What is the actual and the expected result using OpenAPI Description (yml or json)?

I expect that nullable Kotlin members are non-required by default; non-nullable Kotlin member required.
However, the value for required in a @Schema annotation should override that.
This is not the case: When springdoc-openapi-kotlin is in the classpath, the annotation's value is completely ignored.

  • Provide with a sample code (HelloController) or Test that reproduces the problem
import io.swagger.v3.oas.annotations.media.Schema

data class ExampleDto(
        @field:Schema(description = "Should be required")
        val nonNullable: String,

        @field:Schema(description = "Should not be required")
        val nullable: String?,

        @field:Schema(required = true, description = "Should be required")
        val nonNullableAnnotatedRequired: String,

        @field:Schema(required = false, description = "Should not be required")
        val nonNullableAnnotatedNotRequired: String,

        @field:Schema(required = true, description = "Should be required")
        val nullableAnnotatedRequired: String?,

        @field:Schema(required = false, description = "Should not be required")
        val nullableAnnotatedNotRequired: String?
)
import org.springframework.web.bind.annotation.GetMapping
import org.springframework.web.bind.annotation.RestController

@RestController
class ExampleController {
    @GetMapping("/example")
    fun getExampleDto(): ExampleDto = ExampleDto("", "", "", "", "", "")
}

Expected behavior

  • A clear and concise description of what you expected to happen.
    I expect that nonNullable, nonNullableAnnotatedRequired, and nullableAnnotatedRequired are considered as "required" in the generated schema, while the remaining members are not required.

  • What is the expected result using OpenAPI Description (yml or json)?

Expected result:

{
   "openapi":"3.0.1",
   "info":{
      "title":"OpenAPI definition",
      "version":"v0"
   },
   "servers":[
      {
         "url":"http://localhost:8080",
         "description":"Generated server url"
      }
   ],
   "paths":{
      "/example":{
         "get":{
            "tags":[
               "example-controller"
            ],
            "operationId":"getExampleDto",
            "responses":{
               "200":{
                  "description":"OK",
                  "content":{
                     "*/*":{
                        "schema":{
                           "$ref":"#/components/schemas/ExampleDto"
                        }
                     }
                  }
               }
            }
         }
      }
   },
   "components":{
      "schemas":{
         "ExampleDto":{
            "required":[
               "nonNullable",
               "nullableAnnotatedRequired",
               "nonNullableAnnotatedRequired"
            ],
            "type":"object",
            "properties":{
               "nonNullable":{
                  "type":"string",
                  "description":"Should be required"
               },
               "nullable":{
                  "type":"string",
                  "description":"Should not be required"
               },
               "nonNullableAnnotatedRequired":{
                  "type":"string",
                  "description":"Should be required"
               },
               "nonNullableAnnotatedNotRequired":{
                  "type":"string",
                  "description":"Should not be required"
               },
               "nullableAnnotatedRequired":{
                  "type":"string",
                  "description":"Should be required"
               },
               "nullableAnnotatedNotRequired":{
                  "type":"string",
                  "description":"Should not be required"
               }
            }
         }
      }
   }
}

Result with springdoc-openapi-kotlin in the classpath:

{
   "openapi":"3.0.1",
   "info":{
      "title":"OpenAPI definition",
      "version":"v0"
   },
   "servers":[
      {
         "url":"http://localhost:8080",
         "description":"Generated server url"
      }
   ],
   "paths":{
      "/example":{
         "get":{
            "tags":[
               "example-controller"
            ],
            "operationId":"getExampleDto",
            "responses":{
               "200":{
                  "description":"OK",
                  "content":{
                     "*/*":{
                        "schema":{
                           "$ref":"#/components/schemas/ExampleDto"
                        }
                     }
                  }
               }
            }
         }
      }
   },
   "components":{
      "schemas":{
         "ExampleDto":{
            "required":[
               "nonNullable",
               "nonNullableAnnotatedNotRequired",
               "nonNullableAnnotatedRequired"
            ],
            "type":"object",
            "properties":{
               "nonNullable":{
                  "type":"string",
                  "description":"Should be required"
               },
               "nullable":{
                  "type":"string",
                  "description":"Should not be required"
               },
               "nonNullableAnnotatedRequired":{
                  "type":"string",
                  "description":"Should be required"
               },
               "nonNullableAnnotatedNotRequired":{
                  "type":"string",
                  "description":"Should not be required"
               },
               "nullableAnnotatedRequired":{
                  "type":"string",
                  "description":"Should be required"
               },
               "nullableAnnotatedNotRequired":{
                  "type":"string",
                  "description":"Should not be required"
               }
            }
         }
      }
   }
}

Result without springdoc-openapi-kotlin in the classpath:

{
   "openapi":"3.0.1",
   "info":{
      "title":"OpenAPI definition",
      "version":"v0"
   },
   "servers":[
      {
         "url":"http://localhost:8080",
         "description":"Generated server url"
      }
   ],
   "paths":{
      "/example":{
         "get":{
            "tags":[
               "example-controller"
            ],
            "operationId":"getExampleDto",
            "responses":{
               "200":{
                  "description":"OK",
                  "content":{
                     "*/*":{
                        "schema":{
                           "$ref":"#/components/schemas/ExampleDto"
                        }
                     }
                  }
               }
            }
         }
      }
   },
   "components":{
      "schemas":{
         "ExampleDto":{
            "required":[
               "nonNullableAnnotatedRequired",
               "nullableAnnotatedRequired"
            ],
            "type":"object",
            "properties":{
               "nonNullable":{
                  "type":"string",
                  "description":"Should be required"
               },
               "nullable":{
                  "type":"string",
                  "description":"Should not be required"
               },
               "nonNullableAnnotatedRequired":{
                  "type":"string",
                  "description":"Should be required"
               },
               "nonNullableAnnotatedNotRequired":{
                  "type":"string",
                  "description":"Should not be required"
               },
               "nullableAnnotatedRequired":{
                  "type":"string",
                  "description":"Should be required"
               },
               "nullableAnnotatedNotRequired":{
                  "type":"string",
                  "description":"Should not be required"
               }
            }
         }
      }
   }
}

Additional context
Sample repository with springdoc-openapi-kotlin: https://github.com/Clubfan22/springdoc-openapi-kotlin-required-ignored/tree/with-springdoc-openapi-kotlin

Sample repository without springdoc-openapi-kotlin: https://github.com/Clubfan22/springdoc-openapi-kotlin-required-ignored/tree/without-springdoc-openapi-kotlin

SpringDoc-OpenApi's ModelResolver uses Jackson for determining some properties of classes,
i.e. members, methods and annotations.
Therefore, SpringDoc-OpenApi extends Swagger-core's AbstractModelConverter in the ModelResolver class.
and registers SwaggerAnnotationIntrospector with its object mapper.
SwaggerAnnotationIntrospector is used for reading the @Schema annotation's values.

However, if SpringDoc-OpenApi-Kotlin is on the classpath (!), its static code block is executed.
Within that code block, it registers the KotlinModule of Jackson with the ObjectMapper instance used by
ModelResolver. Because KotlinModule's setup method uses insertAnnotationIntrospector,
the KotlinAnnotationIntrospector becomes the new default.

Consequently, when the ModelResolver tries to determine whether a class member is required,
now the KotlinAnnotationIntrospector is always used. It returns true for non-nullable types, false otherwise.
Thus, the SwaggerAnnotationIntrospector is never called and the @Schema annotation is never checked.

By not loading SpringDoc-OpenApi-Kotlin, this problem is avoided and @Schema annotations are honored.
However, nullable kotlin types are not automatically marked as not required.

@bnasslahsen
Copy link
Contributor

@Clubfan22,

Your field: val nonNullableAnnotatedNotRequired: String, is from kotlin perspective is non-nullable.
While loading KotlinModule, all non-nullable kotlin fields are marked as required.

So this case, is not a coherent test case.

Note that that Loading KotlinAnnotationIntrospector in whatever order will lead to the same result for this field.

@Clubfan22
Copy link
Author

I agree that not all of the mentioned cases make sense, but I do have a use-case, which is currently broken.
For example in PATCH requests with JSON payload, I want to be differentiate between not changing a field and setting the field to null.
Consequently, I use JsonNullable properties on my DTOs. In the OpenApi definition, however, these should be optional, ergo required=false:

        @field:Schema(required = false, nullable = true, implementation = String::class,
                description = "Can have a value, be null, or be absent")
        val jsonNullable: JsonNullable<String>,

With springdoc-openapi-kotlin in the classpath, I cannot generate the OpenApi description I'd like to have.
I've updated the demo repo and added two branches showing the different behaviour: with-springdoc-openapi-kotlin-and-json-nullable and without-springdoc-openapi-kotlin-with-json-nullable

@peter-plochan
Copy link

@bnasslahsen and what about the not nullable fields with default values? E.g.:

        @field:Schema(required = false, description = "Should not be required")
        val nonNullableWithDefault: String = "a default value",

This does not work as well - the field is always required.

Also, I don't think that OpenAPI specification should be tightly coupled with the backend code itself (the default deserialisation behaviour). The "PATCH" use-case mentioned by @Clubfan22 is very nice example of the custom deserialisation which shows that the API can differ from the backend implementation.

@bnasslahsen bnasslahsen added the wontfix This will not be worked on label Jan 26, 2022
@springdoc springdoc locked as resolved and limited conversation to collaborators Jan 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants