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

Kotlin vs. OpenApi Nullable Properties: Best Practices? #220

Closed
sauterl opened this issue May 28, 2024 · 4 comments
Closed

Kotlin vs. OpenApi Nullable Properties: Best Practices? #220

sauterl opened this issue May 28, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@sauterl
Copy link
Contributor

sauterl commented May 28, 2024

Disclaimer: I am not sure whether this is a bug report or a feature request, so I'm just going to ask!

As far as I am aware, nullable properties have to be specifically marked as such, with the OpenApiNullable annotation.
However, since Kotlin differentiates between nullable and non-nullable types, is there a way to have the plugin automatically inject this into the OpenApi Specification?

Or did I miss another approach?

Thanks in advance!

@dzikoysk dzikoysk added the question Further information is requested label May 28, 2024
@dzikoysk
Copy link
Member

dzikoysk commented May 28, 2024

This plugin is written for Java, not Kotlin - via annotation processor API, not KSP. It means that we only have access to metadata exposed by Java's compiler and it misses most of the Kotlin's metadata - such as built-in nullability support.

The good thing, in terms of this question, is that Kotlin by default annotates these properties with @NotNull/@Nullable annotations and we have support for it:

val isNotNull = when {
customType?.nullability == Nullability.NOT_NULL -> true
customType?.nullability == Nullability.NULLABLE -> false
property.hasAnnotation("NotNull") -> true
propertyType.isPrimitive() -> true
property.hasAnnotation("Nullable") -> false
else -> false
}

So it should work for you out of the box. Did you notice some issues with these properties? 🤔

@sauterl
Copy link
Contributor Author

sauterl commented May 29, 2024

Thanks for the quick reply and the clarification regarding the target language.

I will have to investigate this on a clean slate. In our project, the generated OpenApi Specification was missing the "nullable" : true, if we didn't specify it with the @OpenApiNullable annotation. Based on your answer, this is unexpected behaviour.
Unfortunately, I do not recall whether this worked as expected in a previous version of Javalin, the following affects 6.1.3 (which is a couple of minor versions behind latest, I am aware of this). version 6.1.6.

Code Example

Specifically, the following ApiClientAnswer Kotlin data class:

@Serializable
data class ApiClientAnswer( //TODO add optional relevance score field
    /** The text that is part of this [ApiClientAnswer]. */
    @get:OpenApiNullable
    val text: String? = null,

    /** The [MediaItemId] associated with the [ApiClientAnswer]. Is usually added as contextual information by the receiving endpoint. */
    @JsonIgnore
    @get:OpenApiIgnore
    val mediaItemId: MediaItemId? = null,

    /** The name of the media item that is part of the answer. */
    val mediaItemName: String?  = null,

    /** The name of the collection the media item belongs to. */
    val mediaItemCollectionName: String? = null,

    /** For temporal [ApiClientAnswer]s: Start of the segment in question in milliseconds. */
    val start: Long? = null,

    /** For temporal [ApiClientAnswer]s: End of the segment in question in milliseconds. */
    val end: Long? = null,
) {}

Produced this OpenApi schema:

"ApiClientAnswer" : {
        "type" : "object",
        "additionalProperties" : false,
        "properties" : {
          "text" : {
            "type" : "string"
          },
          "mediaItemName" : {
            "type" : "string"
          },
          "mediaItemCollectionName" : {
            "type" : "string"
          },
          "start" : {
            "type" : "integer",
            "format" : "int64"
          },
          "end" : {
            "type" : "integer",
            "format" : "int64"
          }
        }
      }

Whereas if annotated with @OpenApiNullable:

@Serializable
data class ApiClientAnswer( //TODO add optional relevance score field
    /** The text that is part of this [ApiClientAnswer]. */
    val text: String? = null,

    /** The [MediaItemId] associated with the [ApiClientAnswer]. Is usually added as contextual information by the receiving endpoint. */
    @JsonIgnore
    @get:OpenApiIgnore
    val mediaItemId: MediaItemId? = null,

    /** The name of the media item that is part of the answer. */
    @get:OpenApiNullable
    val mediaItemName: String?  = null,

    /** The name of the collection the media item belongs to. */
    @get:OpenApiNullable
    val mediaItemCollectionName: String? = null,

    /** For temporal [ApiClientAnswer]s: Start of the segment in question in milliseconds. */
    @get:OpenApiNullable
    val start: Long? = null,

    /** For temporal [ApiClientAnswer]s: End of the segment in question in milliseconds. */
    @get:OpenApiNullable
    val end: Long? = null,
) {}

the generated JSON schema is as expected:

"ApiClientAnswer" : {
        "type" : "object",
        "additionalProperties" : false,
        "properties" : {
          "text" : {
            "type" : "string",
            "nullable" : true
          },
          "mediaItemName" : {
            "type" : "string",
            "nullable" : true
          },
          "mediaItemCollectionName" : {
            "type" : "string",
            "nullable" : true
          },
          "start" : {
            "type" : "integer",
            "format" : "int64",
            "nullable" : true
          },
          "end" : {
            "type" : "integer",
            "format" : "int64",
            "nullable" : true
          }
        }
      }

EDIT: Updated the javalin dependencies in our project to 6.1.6 and reported investigation.

@dzikoysk
Copy link
Member

dzikoysk commented Jun 7, 2024

Well, I was wrong. The Nullable/NotNull properties only affects the required list, it's unrelated to nullable property.

@dzikoysk dzikoysk added enhancement New feature or request and removed question Further information is requested labels Jun 7, 2024
dzikoysk added a commit that referenced this issue Jun 7, 2024
@dzikoysk
Copy link
Member

dzikoysk commented Jun 7, 2024

It should work like that out of the box, you can already try it in 6.1.7-SNAPSHOT :)

@github-project-automation github-project-automation bot moved this to 🆕 New in Javalin OpenApi Jun 7, 2024
@dzikoysk dzikoysk moved this from 🆕 New to ✅ Done in Javalin OpenApi Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

No branches or pull requests

2 participants