extension-kotlin icon indicating copy to clipboard operation
extension-kotlin copied to clipboard

Add `kotlin.serialization` implementation of `Serializer`

Open hiddewie opened this issue 4 years ago • 6 comments

(Followup from #121 which had to be closed because I deleted my Github fork.)

This PR adds support for Kotlin serialization, see issue #13.

This PR includes #120 (this is required in order to use the stable version of kotlin.serialization).

In summary:

  • Adds a class KotlinSerializer which implements Axon Serializer.
  • Configurable RevisionResolver, Converter, and Json, with the same defaults as the Jackson serializer implementation.
  • Because Json is configurable, applications can define how JSON should be serialized based on business requirements.
  • Adds a DSL function kotlinSerializer to create a KotlinSerializer with configuration options.
  • Useful errors when the .serializer() method cannot be found on a companion object of a to-serialize object.
  • Class -> KSerializer is cached for performance and to avoid too much reflection.
  • Simple test to show that the implementation works.

Ref https://kotlinlang.org/docs/serialization.html with basic overview. Ref https://github.com/Kotlin/kotlinx.serialization/blob/master/docs/serialization-guide.md with serialization guide.

hiddewie avatar May 30 '21 14:05 hiddewie

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

sonarqubecloud[bot] avatar Jun 03 '21 19:06 sonarqubecloud[bot]

Thank you for the great comments @smcvb. The discussion in https://github.com/AxonFramework/extension-kotlin/pull/124/files#r645048334 remains unresolved which will require more changes whatever solution is chosen.

hiddewie avatar Jun 03 '21 19:06 hiddewie

No problem :)

I investigated the compilation of the @Serializable annotation in Axon Java source code. That does not look like it will work.

Then I found https://github.com/Kotlin/kotlinx.serialization/blob/f673bc2/docs/serializers.md#deriving-external-serializer-for-another-kotlin-class-experimental, which could mean that we create singleton objects marked as @Serializer(forClass = ...) that would auto-generate the needed serializers within the compilation process of this library. Unfortunately it doesn't work yet: the compilation crashes.

hiddewie avatar Sep 20 '21 18:09 hiddewie

FYI, @hiddewie, I've adjusted the milestone to 0.3.0 instead of 0.2.0.

The intent for this is the desire to release 0.2.0 of the Kotlin extension. We felt we couldn't wait much longer until this release.

This, by the way, doesn't mean a 0.3.0 release couldn't be around the corner, which might contain this serializer. ;-)

smcvb avatar Oct 12 '21 09:10 smcvb

Sorry for the long wait, I didn't forget the PR but it took some time until I could dive into it again!

I updated this branch to include all changes in master.

I updated the Kotlin serialization version to 1.3.2, but this makes no changes to everything I have observed so far.

In particular I experimented with a Surrogate serializer (https://github.com/AxonFramework/extension-kotlin/pull/124#discussion_r817801799). This works, but still requires a lot of maintenance work to 'wrap' each Axon internal class with a surrogate class in this library, while copying the public fields manually into the surrogate class for serialization. Performance-wise this is also not ideal.

https://github.com/AxonFramework/extension-kotlin/pull/124/files#diff-cc9e7866a7929cd55740daaa367352b89517dc56787fa6eb48f5c2fcb783b1bbR12-R31

I tested the automatically-generated serializer for external classes functionality again, but this still gives compiler errors:

@Serializer(forClass = ConfigToken::class)
object ConfigTokenSerializer 

If I manually type the simple class like ConfigToken using Kotlin, it does work. So there might again be a Kotlin <-> Java interop problem here, causing errors in the Kotlin compiler.

hiddewie avatar Apr 16 '22 12:04 hiddewie

Removing the milestone as this is still under discussion, while the release is intended to come soon. Will add this to the following release milestone for further discussions.

smcvb avatar Sep 12 '22 12:09 smcvb

Marking this pull request as obsolete through the introduction of #338.

smcvb avatar Jul 17 '24 12:07 smcvb