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

Support for inline classes #199

Closed
jnizet opened this issue Dec 14, 2018 · 38 comments
Closed

Support for inline classes #199

jnizet opened this issue Dec 14, 2018 · 38 comments

Comments

@jnizet
Copy link

jnizet commented Dec 14, 2018

Kotlin 1.3 has experimental inline classes.

When wrapping a primitive type, an inline class instance is just compiled to the primitive type, making the code very efficient, whie still keeping it typesafe and preventing for example to sum Watts with Volts.

When the inline class instance is nullable, a wrapper class is used (just like java.lang.Integer is used to represent a nullable Int).

Unfortunately, inline classes don't follow the same rules as standard wrapper classes. Example:

import com.fasterxml.jackson.databind.ObjectMapper
import com.fasterxml.jackson.module.kotlin.KotlinModule

inline class Watt(val value: Long)

data class Foo(val nonNullable: Watt, val nullable: Watt?, val otherNullable: Watt?)
data class Bar(val nonNullable: Long, val nullable: Long?, val otherNullable: Long?)

fun main() {
    val foo = Foo(Watt(1000), Watt(2000), null)
    val bar = Bar(1000, 2000, null)

    val objectMapper = ObjectMapper().registerModule(KotlinModule())

    println("With Watt: ${objectMapper.writeValueAsString(foo)}")
    println("With Long: ${objectMapper.writeValueAsString(bar)}")
}

The output of this program is:

With Watt: {"nonNullable":1000,"nullable":{"value":2000},"otherNullable":null}
With Long: {"nonNullable":1000,"nullable":2000,"otherNullable":null}

It would be really nice if the first output was identical to the second one, and of course if unmarshalling worked too.

@TjeuKayim
Copy link

TjeuKayim commented Dec 31, 2018

I use this workaround to read and write inline classes.
The box-impl method boxes the value, so it can be used as JsonCreator.
https://github.com/Kotlin/KEEP/blob/master/proposals/inline-classes.md#inline-classes-abi-jvm

inline class Watt(@JsonValue val value: Long)

@Test
fun `Read and write inline classes with Jackson`() {
  val mapper = ObjectMapper().registerModule(KotlinModule()).registerModule(InlineModule)
  val watt = Watt(1234)
  val json = mapper.writeValueAsString(watt)
  assertEquals("1234", json)
  val result = mapper.readValue<Watt>(json)
  assertEquals(watt, result)
}

object InlineModule : SimpleModule("Inline") {
  override fun setupModule(context: SetupContext) {
    super.setupModule(context)
    context.appendAnnotationIntrospector(KotlinNamesAnnotationIntrospector)
  }

  object InlineAnnotationIntrospector : NopAnnotationIntrospector() {
    override fun findCreatorAnnotation(config: MapperConfig<*>, a: Annotated): JsonCreator.Mode? {
      if (a is AnnotatedMethod && a.name == "box-impl") {
        return JsonCreator.Mode.DEFAULT
      }
      return null
    }
  }
}

@kostya05983
Copy link
Contributor

@TjeuKayim, is serialization working with inline classes now?

@TjeuKayim
Copy link

TjeuKayim commented Oct 24, 2019

I tried building the latest master branch, however the build fails.

image

Instead I used 7bfb771.
I ran these unit-tests, and they all failed.

image

So inline classes are still not working in the latest release.

@cowtowncoder
Copy link
Member

Please do not use master as that is for 3.0.0 (and still has couple of issues) -- code for 2.10 is in 2.10 branch (for patches to 2.10), and 2.11 is for developing next 2.x minor version.

@apatrida
Copy link
Member

This would have to be studied to see how an inline class would be detected safely and without fail, I'm not sure if just the naming convention of the static constructor is clear enough. Maybe if detecting is Kotlin class, plus that naming convention, and a pattern of other methods maybe.

@apatrida
Copy link
Member

I think this would be a pretty messy fix, but is worth looking into. @TjeuKayim did you abandon that PR completely?

@TjeuKayim
Copy link

TjeuKayim commented Oct 26, 2019

Unfortunately, I can't spend more time on this, and currently I'm not using Kotlin for any project.
There has to be changed a lot: TjeuKayim/jackson-module-kotlin@master...TjeuKayim:class-descriptor-reflection.
I just found out that reflection functionality has improved since Kotlin 1.3.20 (https://github.com/Kotlin/KEEP/blob/master/proposals/inline-classes.md#reflection).

@TjeuKayim
Copy link

This would have to be studied to see how an inline class would be detected safely @apatrida

I used kotlin.reflect.jvm.internal.impl.descriptorsClassDescriptor.isInline, but this is an internal API.

@StephenOTT
Copy link

StephenOTT commented May 11, 2020

Has there been any updates to this? Or workaround code samples?
@TjeuKayim, do you have any compartmentalized samples?

@TjeuKayim
Copy link

No, I don't have any updates. The last few months I have not used Kotlin. For a workaround I refer to my comment above #199 (comment).

@StephenOTT
Copy link

@TjeuKayim thanks. The greater issue i have found is: #187. Once using the inline within a class, then it seems to fall apart on deserialization.

@efenderbosch
Copy link

efenderbosch commented May 22, 2020

Found a decent workaround today. Use @JsonDeserialize(builder = BuilderForYourRealClass::class)

Where BuilderForYourRealClass has "with" functions, one for each argument. The key is that you need a "with" method for the wrapped class. For example if I have an inline class MyInlineClass that simply wraps a String then the builder needs a function that accepts a String and creates an instance of MyInlineClass in the build function. Like this:

@JsonDeserialize(builder = MyClassThatUsesAnInlineClassBuilder::class)
data class MyClassThatUsesAnInlineClass(val foo: String, val bar: MyInlineClass)

class MyClassThatUsesAnInlineClassBuilder {
    lateinit var foo: String
    lateinit var bar: String
    fun withFoo(foo: String) = apply { this.foo = foo }
    fun withBar(bar: String) = apply { this.bar = bar }
    fun build() = MyClassThatUsesAnInlineClass(foo, MyInlineClass(bar))
}

michael-simons added a commit to neo4j/neo4j-ogm that referenced this issue Aug 21, 2020
Kotlin provides so called „Inline classes“ (https://kotlinlang.org/docs/reference/inline-classes.html), wrapping standard types into a dedicated type. This makes them a nice fit for IDs etc.

The object mapping process works ootb, the parameter mapping needs some extra work: One has to provide custom serializers for all data classes being used (see `IdTypes`). Those serializers can be registered with a dedicated `SimpleModule`. This module needs to be added to the `ObjectMapper` instance used by OGM to convert parameters:

```
ObjectMapperFactory.objectMapper()
        .registerModule(KotlinModule())
        .registerModule(IdTypesModule())
```

The kotlin jackson module may detect them at some point in the future itself (See FasterXML/jackson-module-kotlin#199), but until than that work is necessary.

This closes #822.
michael-simons added a commit to neo4j/neo4j-ogm that referenced this issue Aug 21, 2020
Kotlin provides so called „Inline classes“ (https://kotlinlang.org/docs/reference/inline-classes.html), wrapping standard types into a dedicated type. This makes them a nice fit for IDs etc.

The object mapping process works ootb, the parameter mapping needs some extra work: One has to provide custom serializers for all data classes being used (see `IdTypes`). Those serializers can be registered with a dedicated `SimpleModule`. This module needs to be added to the `ObjectMapper` instance used by OGM to convert parameters:

```
ObjectMapperFactory.objectMapper()
        .registerModule(KotlinModule())
        .registerModule(IdTypesModule())
```

The kotlin jackson module may detect them at some point in the future itself (See FasterXML/jackson-module-kotlin#199), but until than that work is necessary.

This closes #822.
michael-simons added a commit to neo4j/neo4j-ogm that referenced this issue Aug 24, 2020
Kotlin provides so called „Inline classes“ (https://kotlinlang.org/docs/reference/inline-classes.html), wrapping standard types into a dedicated type. This makes them a nice fit for IDs etc.

The object mapping process works ootb, the parameter mapping needs some extra work: One has to provide custom serializers for all data classes being used (see `IdTypes`). Those serializers can be registered with a dedicated `SimpleModule`. This module needs to be added to the `ObjectMapper` instance used by OGM to convert parameters:

```
ObjectMapperFactory.objectMapper()
        .registerModule(KotlinModule())
        .registerModule(IdTypesModule())
```

The kotlin jackson module may detect them at some point in the future itself (See FasterXML/jackson-module-kotlin#199), but until than that work is necessary.

This closes #822.
michael-simons added a commit to neo4j/neo4j-ogm that referenced this issue Aug 24, 2020
Kotlin provides so called „Inline classes“ (https://kotlinlang.org/docs/reference/inline-classes.html), wrapping standard types into a dedicated type. This makes them a nice fit for IDs etc.

The object mapping process works ootb, the parameter mapping needs some extra work: One has to provide custom serializers for all data classes being used (see `IdTypes`). Those serializers can be registered with a dedicated `SimpleModule`. This module needs to be added to the `ObjectMapper` instance used by OGM to convert parameters:

```
ObjectMapperFactory.objectMapper()
        .registerModule(KotlinModule())
        .registerModule(IdTypesModule())
```

The kotlin jackson module may detect them at some point in the future itself (See FasterXML/jackson-module-kotlin#199), but until than that work is necessary.

This closes #822.
@wem
Copy link

wem commented Feb 19, 2021

Another possible way would be to use JsonCreator:

inline class InlineType(val value: String)

data class ContainsInlineType(val inlineType: InlineType) {
    companion object {
        @JsonCreator
        @JvmStatic
        fun create(inlineType: String) = ContainsInlineType(InlineType(inlineType))
    }
}

@mvonrenteln
Copy link

@TjeuKayim Info: Your InlineAnnotationIntrospector Workaround does not seem to work with Kotlin 1.5. "box-impl" is now synthetic and jackson does not call the Introspector for it...

JsonCreators or Builder would not be a workaround for me either because they have to be added to every class that uses the inline class... This clutters all DTOs...

@Quantum64
Copy link

@k163377 Do you have a plan for value class deserialization support? With the stabilization of more Kotlin standard library components that utilize value classes such as kotlin.time.Duration, this is becoming a pain point for us. Wanted to check with you since you worked on the serialization support. If you don't have plans to work on this soon I may prototype the feature myself for a potential PR.

@k163377
Copy link
Contributor

k163377 commented Dec 30, 2021

@Quantum64
As for the partial support, I was going to start with the answer to #514.

As far as I have researched, I think the following two points are relatively easy to achieve.

  1. deserialize value class
  2. deserialize with a factory function (with JsonCreator) that includes value class in arguments.

However, there are still some unresolved issues regarding the deserialization of constructor containing value class.
Also, support for JsonDeserialize annotations has not yet been considered, but I feel that there may be some issues that are difficult to resolve.

p.s.
As a personal request, I would like to merge #512 and subsequent refactors first to avoid conflicts and to simplify changes for value class support.

@bugflux
Copy link

bugflux commented Oct 19, 2022

I would like to try and take a stab at this, but I think I need a bit of guidance. @k163377 you mentioned

there are many parts [in the deserialization of the value class] that do not work

Do we have tests or examples that could give some coverage of those cases? Also those PRs to be merged before make it harder for a first time contributor to understand the scope of changes, would those be a requirement or nice to haves?

@k163377
Copy link
Contributor

k163377 commented Oct 19, 2022

kotlin-reflect 1.7 is almost mandatory to support value class deserialization.
On the other hand, the current jackson-module-kotlin policy does not allow upgrading to kotlin 1.7 until kotlin 1.6 is deprecated.
This means that you will have to wait a few more years.

Below is a summary of as much as I can remember of the work that needs to be done and the patterns that need to be covered, but I'm not sure if I've covered it all.
Please note that each task requires knowledge of how the value class is represented in Java.

Required work

  1. k163377@ca2120b
  2. implement deserialization process for value class (probably by adding it to KotlinDeserializers)
  3. modify KotlinValueInstantiator to properly deserialize the value class argument

Note that ideally, Jackson annotations should also be handled properly.

Patterns that need to be covered

All combinations of the following patterns must be covered.

If the value of value class is...

  • primitive type
  • non-null object type
  • nullable object type

If the parameter on the Creator is...

  • non-null
  • nullable

Also note that ideally, options such as nullToEmptyCollection, nullIsSameAsDefault, and strictNullChecks should be supported.


I am very busy these days and have no time to work on this problem.
If I had the time and money, I would like to work on a project to rewrite jackson-module-kotlin to my liking...

@k163377
Copy link
Contributor

k163377 commented Jan 15, 2023

I have achieved partial support for deserialization by functions containing value class in an experimental project I am creating.
It is assumed that deserialization of the value class will work, at least if it was registered a Deserializer with ObjectMapper.
ProjectMapK/jackson-module-kogera#40

Snapshots are available from JitPack.
https://jitpack.io/#ProjectMapK/jackson-module-kogera/8dc5c4abda
Please refer to the following for installation instructions.
https://github.com/ProjectMapK/jackson-module-kogera#installation

However, this project does not use kotlin-reflect, so it is not possible to apply it directly to jackson-module-kotlin.
Also, there are some known issues as follows

  • The JsonDeserialize annotation does not work
  • JsonCreator annotation given in value class does not work

I would appreciate a star to keep me motivated.
https://github.com/ProjectMapK/jackson-module-kogera

@k163377
Copy link
Contributor

k163377 commented Feb 13, 2023

I noticed that there is no explicit specification (at least not that I am aware of) for handling value class.

In jackson-module-kogera, the basic specification is to treat value class like a value, as follows.

@JvmInline
value class Value(val value: Int)

val mapper = jacksonObjectMapper()
val json = mapper.writeValueAsString(Value(1)) // -> 1
val value = mapper.readValue<Value>(json) // -> Value(value=1)

The reason for this specification is as follows.

Also, this specification seemed the easiest to implement due to the constraints of handling value class in Jackson.

I plan to use the same specification when support value class in jackson-module-kotlin in the future.
I would appreciate any comments or suggestions you may have.
I would especially like to hear from @dinomite .

@k163377
Copy link
Contributor

k163377 commented Mar 17, 2023

Issues related to deserialization support for value class related content will be summarized in #650.
This issue will not be closed for a while as it includes discussion of serialization.

@fzyzcjy
Copy link

fzyzcjy commented May 4, 2023

Hi, is there any updates? Thanks!

@k163377
Copy link
Contributor

k163377 commented May 4, 2023

Many use cases for serialization are already supported.

For deserialization, I hope to have some support by 2.16.
However, I consider this a difficult goal.

wplong11 added a commit to wplong11/jackson-module-kotlin that referenced this issue Aug 3, 2023
wplong11 added a commit to wplong11/jackson-module-kotlin that referenced this issue Aug 3, 2023
wplong11 added a commit to wplong11/jackson-module-kotlin that referenced this issue Aug 3, 2023
wplong11 added a commit to wplong11/jackson-module-kotlin that referenced this issue Aug 3, 2023
wplong11 added a commit to wplong11/jackson-module-kotlin that referenced this issue Aug 3, 2023
@k163377
Copy link
Contributor

k163377 commented Oct 8, 2023

For the topic of Multi field value class, please go to the following issue.
#714

@k163377
Copy link
Contributor

k163377 commented Oct 10, 2023

Issue submitted on improving serialization performance of value class.
#715

@k163377
Copy link
Contributor

k163377 commented Dec 2, 2023

crossposting
#650 (comment)

@k163377
Copy link
Contributor

k163377 commented Feb 18, 2024

Deserialization of value class is supported since 2.17.
I spent several years researching and prototyping Jackson and modifying kotlin-reflect, and I am happy to have accomplished it.

An RC version will be available soon, so please give it a try.
I have also prepared a document with notes on the differences in behavior from the normal class (I am not very good at English, so please PR me if you have any suggestions for improvement).
https://github.com/FasterXML/jackson-module-kotlin/blob/2.17/docs/value-class-support.md

This issue is closed, please submit a new issue if you have any problems.

@k163377 k163377 closed this as completed Feb 18, 2024
@cowtowncoder
Copy link
Member

@k163377 Whoa! This sounds really great -- thank you for pushing it through.

@geirsagberg
Copy link

I created an issue for supporting value classes as map keys: #777

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet