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

Enable implicit conversion from kotlin.time.Duration to java.time.Duration #683

Closed
kkurczewski opened this issue Jul 12, 2023 · 4 comments
Closed

Comments

@kkurczewski
Copy link
Contributor

Use case
Usage of kotlin.time.Duration instead of java.time.Duration in Kotlin projects

Describe the solution you'd like
My proposal is to add additional serialization option like: KotlinSerializationFeature.SERIALIZE_KOTLIN_DURATION_AS_JAVA_DURATION.

It would call kotlin.time.Duration.toJavaDuration() under hood and allow to reuse all features already implemented for java.time.Duration. It could be disabled by default to keep backward compatibility.

Describe alternatives you've considered
I can implement custom de/serializers or write custom KotlinTimeModule etc. While it is not a big deal downside of this is that implementation would be very similar (identical?) for everyone and has to be copy-pasted over all projects therefore I think it is a good fit for library.

@k163377
Copy link
Contributor

k163377 commented Jul 13, 2023

First, kotlin.time.Duration is a value class, making it particularly difficult to support deserialization in a kotlin-module.
#199
#544

Personally, I agree that the default behavior should be to match the behavior to java.time.Duration.
If it can be overridden by a custom serializer, I think it could be a destructive change, as the scope of impact would be small.

A better way of implementation would be to use Converter.
https://github.com/FasterXML/jackson-module-kotlin/blob/2.16/src/main/kotlin/com/fasterxml/jackson/module/kotlin/Converters.kt

@kkurczewski
Copy link
Contributor Author

Hi @k163377, thanks for the tip with Converters. I played with code a little bit and managed to address some scenarios, see #689. Indeed, it becomes clunky when data class wraps value class and I had to resort to annotation voodo on POJO in order to make tests pass.

That being said, if this works with annotations then (I think) it is doable. Unfortunately at the moment I lack knowledge of Jackson internals to push it further. If I will have some spare time I will try to dig deeper.

Any further tips/ideas are warmly welcome.

@k163377
Copy link
Contributor

k163377 commented Jul 28, 2023

@kkurczewski
Thank you and sorry for the delay in replying.
Please allow me to discuss further in the PR.

@k163377
Copy link
Contributor

k163377 commented Aug 6, 2023

Resolved by merge #689.

@k163377 k163377 closed this as completed Aug 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants