Skip to content

Add Kotlin reflection support to ModelOptionsUtils #1667

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

Closed
sdeleuze opened this issue Nov 4, 2024 · 5 comments
Closed

Add Kotlin reflection support to ModelOptionsUtils #1667

sdeleuze opened this issue Nov 4, 2024 · 5 comments
Labels
enhancement New feature or request kotlin
Milestone

Comments

@sdeleuze
Copy link
Contributor

sdeleuze commented Nov 4, 2024

As documented via #1666, schema generation from Kotlin classes currently requires using non idiomatic
code like data class Foo(@get:JsonProperty(required = true, value = "output") val bar: String) while the required information can be inferred from Kotlin null-safety and the value inferred from Kotlin reflection.

A related com.github.victools.jsonschema.generator.Module instance could be implemented and created when KotlinDetector.isKotlinReflectPresent() == true to provide those information automatically here.

That would allow to perform schema generation with just data class Foo(val bar: String).

@devcrocod
Copy link
Contributor

Hi,

I have some question regarding this issue, please clarify follow points:

  • In a data class like data class Foo(val bar: String, val baz: String?), should the property baz be not marked as required in the generated JSON schema because it's nullable, even though it doesn't have a default value? Or should it only be considered not required if it has a default value, like in data class Foo(val bar: String, val baz: String? = null)?

  • Also about properties with default values: How should the module handle non-nullable properties with default values, such as in data class Foo(val bar: String, val baz: Int = 10)?

  • Should the same logic be handled for functions?

@sdeleuze
Copy link
Contributor Author

sdeleuze commented Nov 8, 2024

Hi, thanks for your feedback.

I think conceptually the goal is to infer if a property is required or not, so I think I would consider both nullable properties and properties with default value as non required, and the other ones as required. So for your 2 examples, only bar is required.

For functions, I think that's outside of the scope of this issue. It could make sense to use Kotlin reflection instead of Java one to support more advanced Kotlin constructs, but so far I don't think there is advanced need for optional parameters with default values as only Function1 and Function2 are supported. Could be useful for suspending functions potentially, but that's a different need so I would suggest to focus on schema generation initially, and explore more advanced function invocation later.

@tzolov
Copy link
Contributor

tzolov commented Dec 23, 2024

Resolved by 756a9dc

@tzolov tzolov closed this as completed Dec 23, 2024
@tzolov tzolov added this to the 1.0.0-M5 milestone Dec 23, 2024
@sdeleuze
Copy link
Contributor Author

Thanks, will send the documentation refinement PR tomorrow.

@sdeleuze
Copy link
Contributor Author

See #2006 related PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request kotlin
Projects
None yet
Development

No branches or pull requests

3 participants