Skip to content
This repository has been archived by the owner on Feb 23, 2023. It is now read-only.

Refine support for kotlinx.serialization when reflection is involved #1410

Closed
jamesward opened this issue Dec 28, 2021 · 9 comments
Closed
Assignees
Labels
type: compatibility Native image compatibility issue
Milestone

Comments

@jamesward
Copy link

Now that Spring Boot supports kotlinx.serialization it'd be great to have the native hints to support it. I have:

@TypeHint(types = [kotlinx.serialization.encoding.CompositeEncoder::class, kotlinx.serialization.descriptors.SerialDescriptor::class])
@TypeHint(types = [Object::class])

Not sure why the Object one is needed but it seems strange.

@sdeleuze sdeleuze added this to the 0.11.2 milestone Jan 3, 2022
@sdeleuze sdeleuze self-assigned this Jan 3, 2022
@sdeleuze sdeleuze added the type: compatibility Native image compatibility issue label Jan 3, 2022
@sdeleuze
Copy link
Contributor

sdeleuze commented Jan 3, 2022

I will add those few hints, but maybe worth to raise it on Kotlin side as well to see if kotlinx.serialization can work without any need for reflection since that one of its expected benefits.

@jamesward
Copy link
Author

Thanks! It seems that the reflection is necessary: Kotlin/kotlinx.serialization#1125 (comment)

@sdeleuze
Copy link
Contributor

sdeleuze commented Jan 4, 2022

Yeah, seems to require Kotlin/kotlinx.serialization#1348 which has not yet been implemented.

@sdeleuze
Copy link
Contributor

@jamesward I would like to reproduce the issue in one of our sample, but webmvc-kotlin is already using kotlinx.serialization without the need for additional configuration. Any chance you could share a reproducer or send a quick PR that modifies webmvc-kotlin to trigger the error you see?

@sdeleuze sdeleuze added the status: waiting-for-feedback We need additional information before we can continue label Jan 10, 2022
@jamesward
Copy link
Author

I put together a reproducer:
https://github.com/jamesward/spring-native-ktx-serialization/tree/db-no-common

Native reflection configuration for kotlinx.serialization.encoding.CompositeEncoder is missing.

One key thing that was exposed by the reproducer is that there is something with the use of CoroutineCrudRepository which introduces the problem. Which is strange because the serialization doesn't (shouldn't) get used there. But without that, things do work as expected:
https://github.com/jamesward/spring-native-ktx-serialization

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 15, 2022
@sdeleuze sdeleuze changed the title Support for kotlinx.serialization Spring Data support for kotlinx.serialization Jan 17, 2022
@sdeleuze
Copy link
Contributor

Thank you so much for the repro @jamesward, this is super useful.

When running repro with --debug we can see th following stacktrace:

java.lang.ClassNotFoundException: kotlinx.serialization.encoding.CompositeEncoder
	at jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:53) ~[na:na]
	at jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178) ~[na:na]
	at java.lang.ClassLoader.loadClass(ClassLoader.java:139) ~[na:na]
	at kotlin.reflect.jvm.internal.KDeclarationContainerImpl.parseType(KDeclarationContainerImpl.kt:265) ~[na:na]
	at kotlin.reflect.jvm.internal.KDeclarationContainerImpl.loadParameterTypes(KDeclarationContainerImpl.kt:256) ~[na:na]
	at kotlin.reflect.jvm.internal.KDeclarationContainerImpl.findMethodBySignature(KDeclarationContainerImpl.kt:197) ~[na:na]
	at kotlin.reflect.jvm.internal.KFunctionImpl$caller$2.invoke(KFunctionImpl.kt:68) ~[na:na]
	at kotlin.reflect.jvm.internal.KFunctionImpl$caller$2.invoke(KFunctionImpl.kt:61) ~[na:na]
	at kotlin.reflect.jvm.internal.ReflectProperties$LazyVal.invoke(ReflectProperties.java:63) ~[na:na]
	at kotlin.reflect.jvm.internal.ReflectProperties$Val.getValue(ReflectProperties.java:32) ~[na:na]
	at kotlin.reflect.jvm.internal.KFunctionImpl.getCaller(KFunctionImpl.kt:61) ~[na:na]
	at kotlin.reflect.jvm.ReflectJvmMapping.getJavaMethod(ReflectJvmMapping.kt:63) ~[na:na]
	at kotlin.reflect.jvm.ReflectJvmMapping.getKotlinFunction(ReflectJvmMapping.kt:136) ~[na:na]
	at org.springframework.core.MethodParameter$KotlinDelegate.getGenericReturnType(MethodParameter.java:915) ~[na:na]
	at org.springframework.core.MethodParameter$KotlinDelegate.access$000(MethodParameter.java:866) ~[na:na]
	at org.springframework.core.MethodParameter.getGenericParameterType(MethodParameter.java:510) ~[na:na]
	at org.springframework.core.SerializableTypeWrapper$MethodParameterTypeProvider.getType(SerializableTypeWrapper.java:291) ~[na:na]
	at org.springframework.core.SerializableTypeWrapper.forTypeProvider(SerializableTypeWrapper.java:107) ~[na:na]
	at org.springframework.core.ResolvableType.forType(ResolvableType.java:1421) ~[na:na]
	at org.springframework.core.ResolvableType.forMethodParameter(ResolvableType.java:1342) ~[na:na]
	at org.springframework.core.ResolvableType.forMethodParameter(ResolvableType.java:1324) ~[na:na]
	at org.springframework.core.ResolvableType.forMethodParameter(ResolvableType.java:1291) ~[na:na]
	at org.springframework.core.ResolvableType.forMethodReturnType(ResolvableType.java:1251) ~[na:na]
	at org.springframework.core.GenericTypeResolver.resolveReturnType(GenericTypeResolver.java:80) ~[na:na]
	at org.springframework.beans.GenericTypeAwarePropertyDescriptor.<init>(GenericTypeAwarePropertyDescriptor.java:110) ~[na:na]
	at org.springframework.beans.CachedIntrospectionResults.buildGenericTypeAwarePropertyDescriptor(CachedIntrospectionResults.java:409) ~[na:na]
	at org.springframework.beans.CachedIntrospectionResults.<init>(CachedIntrospectionResults.java:300) ~[na:na]
	at org.springframework.beans.CachedIntrospectionResults.forClass(CachedIntrospectionResults.java:181) ~[na:na]
	at org.springframework.beans.BeanUtils.getPropertyDescriptors(BeanUtils.java:490) ~[na:na]
	at org.springframework.data.mapping.context.AbstractMappingContext.doAddPersistentEntity(AbstractMappingContext.java:415) ~[na:na]

So when defining interface BarRepo : CoroutineCrudRepository<Bar, Long> Spring Data is using reflective accesses to Bar which in src/main/kotlin looks like this:

@Serializable
data class Bar(@Id val id: Long?, val name: String)

But on after being processed AOT by the Kotlin compiler, it looks like that:

@kotlinx.serialization.Serializable public final data class Bar public constructor(id: kotlin.Long?, name: kotlin.String) {
    public companion object {
    }
    @kotlin.Deprecated public constructor(seen1: kotlin.Int, id: kotlin.Long?, name: kotlin.String?, serializationConstructorMarker: kotlinx.serialization.internal.SerializationConstructorMarker?) { /* compiled code */ }
    @field:org.springframework.data.annotation.Id public final val id: kotlin.Long? /* compiled code */
    public final val name: kotlin.String /* compiled code */
    public final operator fun component1(): kotlin.Long? { /* compiled code */ }
    public final operator fun component2(): kotlin.String { /* compiled code */ }
    @kotlin.Deprecated public object `$serializer` : kotlinx.serialization.internal.GeneratedSerializer<kotlinbars.common.Bar> {
    }
}

When I debug RepositoryFactoryBeanPostProcessor when running ./gradlew clean generateAot -Dspring.aot.debug=true, it seems the class with the Kotlin compiler plugin customization is used so we seems to be fine here.

@christophstrobl @mp911de I am wondering if we should refine RepositoryFactoryBeanPostProcessor on AOT side, refine Spring Data reflection handling for this use case, or both to avoid to have to specify manually the hints below. Any thoughs?

@TypeHint(types = [kotlinx.serialization.encoding.CompositeEncoder::class, kotlinx.serialization.descriptors.SerialDescriptor::class])
@TypeHint(types = [Object::class])
@TypeHint(types = [Bar::class], access = [TypeAccess.DECLARED_FIELDS, TypeAccess.QUERY_DECLARED_METHODS, TypeAccess.QUERY_PUBLIC_METHODS, TypeAccess.QUERY_DECLARED_CONSTRUCTORS])
@TypeHint(typeNames = ["kotlinbars.common.Bar\$\$serializer"])
@TypeHint(typeNames = ["kotlinbars.common.Bar\$Companion"], methods = [MethodHint(name = "serializer", parameterTypes = [])])

@mp911de
Copy link

mp911de commented Jan 18, 2022

I'm not sure that's the right entry-point. Kotlin serialization is not a Spring Data concern but a cross-cutting one originating from ResolvableType/MethodParameter and bleeding into uses of ReflectJvmMapping.

Instead, it would make sense to come up with a dedicated component that adds such hints ideally on the Framework level. Otherwise, if every individual component starts adding the same serialization type hints we quickly end up with a ton of duplications and potentially code that behaves slightly different creating an unpleasant experience for users.

@sdeleuze
Copy link
Contributor

Good point, we will have a deeper look on Framework side if we can refine the Kotlin support for such use case. cc @poutsma

@sdeleuze sdeleuze modified the milestones: 0.11.2, 0.11.3 Jan 18, 2022
@sdeleuze sdeleuze changed the title Spring Data support for kotlinx.serialization Refine support for kotlinx.serialization when reflection is involved Feb 4, 2022
@sdeleuze sdeleuze removed the status: feedback-provided Feedback has been provided label Feb 4, 2022
@sdeleuze
Copy link
Contributor

sdeleuze commented Feb 4, 2022

It worked without the Object hint for me and I had to add SerializationConstructorMarker, so let see how it goes with that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: compatibility Native image compatibility issue
Development

No branches or pull requests

4 participants