smallrye-open-api
smallrye-open-api copied to clipboard
Support kotlin nullable/non-nullable types as if explicitly set @Schema(required=true/false)
Referring to https://github.com/smallrye/smallrye-open-api/issues/714#issuecomment-789527358 :
Kotlin language has support for nullable types where you explicitly need to state whether they can be null or not. This should translate to @Schema(required=true/false)
.
To support this, it would be sufficient to check for the existence of org.jetbrains.annotations.{Nullable, NotNull}
annotations. As far as I understand, https://github.com/smallrye/smallrye-open-api/issues/131 adds support for javax.validation.constraints.NotNull
already, so this might be a minor addition?
Of course, I'd rather ike to have the kotlin compiler to support generating the javax.validation.constraints.NotNull
annotations, but this currently seems not easily possible for reasons beyond my knowledge.
Thanks for researching this @lenalebt . It seems like a reasonable addition to me. I'm thinking that in addition to required
, the nullable
attribute should also be affected by these annotations.
Yep, that seems to make sense to me as well wrt nullable
.
Here's what I'm thinking -
-
@Nullable
-->nullable: true
, similar to usingOptional
-
@NotNull
--> property added torequired
array. Do not setnullable
, the default value is alreadyfalse
We are setting nullable
to false
for other Bean Validation constraints, despite the default being false
. @phillip-kruger, what do you think about removing that? Maybe save that for the next MP version that supports OpenAPI 3.1.0 where it appears nullable
is no longer an attribute of the JSON Schema.
Sound good to me !
@lenalebt - it turns out that these annotations use @Retention(RetentionPolicy.CLASS)
(instead of RUNTIME
) which are unfortunately not visible to the scanner... It's possible that at some point it could be supported, but that would require changes to the upstream Jandex library, but there are likely good reasons for excluding them there.
Oh, too bad, that basically ruins that idea to just support those annotations.
Just out of curiosity - I don't know much about how this lib is structured in other areas. There is some special kotlin reflection that can return that information as well, whether a type is nullable or not. I have seen that there is a plugin system. How much work would it be to have an additional kotlin plugin to support this? I mean - that one could easily be implemented in kotlin then, using the native language features, right?
Since we've been talking about these and the Jackson annotation, I've been thinking about a way to make the BeanValidationScanner
less monolithic and more pluggable. I think something along those lines would work, provided the code you write in Kotlin to implement the plugin would be callable from the scanner, which it should be. We can leave this issue open to work through that and see if it's viable.
The code of course needs to be callable. Just had a quick look over there. I don't know Jandex, and it seems to abstract away the underlying reflect classes?
If I get a java java.lang.reflect.Field
or java.lang.reflect.Method
I should be able to find out whether it's nullable or not, like this:
fun markKotlinRequired(field: java.lang.reflect.Field): Boolean =
field.kotlinProperty?.returnType?.isMarkedNullable == false
fun markKotlinRequired(method: java.lang.reflect.Method): Boolean =
method.kotlinFunction?.returnType?.isMarkedNullable == false
Unsure though if I would get this?
@lenalebt , I'm not sure either if that would work either. I imagine that Kotlin is somehow wrapping those reflection type and looking at the invisible annotations (which are still in the class file). Jandex extracts the class and annotation information into an efficient structure so that reflection can be avoided at runtime.
Hi, is this still blocked? Do I understand correctly that you'd still have to annotate with @Schema(required = true) in Kotlin for now? For some reason, I'm unable to find, it doesn't have any effect for me...
@Schema(title = "foo") // <-- this has an effect
data class SomeDto(
@Schema(required = true) // <-- this has no effect for me
val name: String
)
@b-thiswatch , it's still blocked for now. Can you share the output that is generated for that model?
@MikeEdgar thanks for the update. I will prepare a demo later. For now, I have solved it with
@Schema(requiredProperties = ["name"])
data class SomeDto(
val name: String
)
which I don't like very much, but it works.
Unblocked now that we have Jandex 3. I'll start looking at this soon for the 3.1.0 and 2.3.1 releases.