Skip to content

Add kotlin.serialization implementation of Serializer #124

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
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
/*
* Copyright (c) 2010-2021. Axon Framework
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.axonframework.extensions.kotlin.serialization

import kotlinx.serialization.KSerializer
import kotlinx.serialization.SerializationException
import kotlinx.serialization.json.Json
import kotlinx.serialization.json.JsonElement
import kotlinx.serialization.json.JsonNull
import org.axonframework.common.ObjectUtils
import org.axonframework.serialization.AnnotationRevisionResolver
import org.axonframework.serialization.ChainingConverter
import org.axonframework.serialization.Converter
import org.axonframework.serialization.RevisionResolver
import org.axonframework.serialization.SerializedObject
import org.axonframework.serialization.SerializedType
import org.axonframework.serialization.Serializer
import org.axonframework.serialization.SimpleSerializedObject
import org.axonframework.serialization.SimpleSerializedType
import org.axonframework.serialization.UnknownSerializedType
import kotlin.reflect.full.companionObject
import kotlin.reflect.full.companionObjectInstance

/**
* Implementation of Axon Serializer that uses a kotlinx.serialization implementation.
* The serialized format is JSON.
*
* The DSL function kotlinSerializer can be used to easily configure the parameters
* for this serializer.
*
* @see kotlinx.serialization.Serializer
* @see org.axonframework.serialization.Serializer
* @see kotlinSerializer
*/
class KotlinSerializer(
private val revisionResolver: RevisionResolver,
private val converter: Converter,
private val json: Json
) : Serializer {

private val serializerCache: MutableMap<Class<*>, KSerializer<*>> = mutableMapOf()

override fun <T> serialize(value: Any?, expectedRepresentation: Class<T>): SerializedObject<T> {
try {
val type = ObjectUtils.nullSafeTypeOf(value)
return when {
expectedRepresentation.isAssignableFrom(String::class.java) ->
SimpleSerializedObject(
(if (value == null) "null" else json.encodeToString(type.serializer(), value)) as T,
expectedRepresentation,
typeForClass(type)
)

expectedRepresentation.isAssignableFrom(JsonElement::class.java) ->
SimpleSerializedObject(
(if (value == null) JsonNull else json.encodeToJsonElement(type.serializer(), value)) as T,
expectedRepresentation,
typeForClass(type)
)

else ->
Copy link
Member

@smcvb smcvb May 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would've assumed the Serializer could default to byte[] instead of throwing an exception.
Doing so will have this serializer follow the same paradigm as the JacksonSerializer and XStreamSerializer.

Several spots in the framework assume that the expectedRepresentation is byte[].
So enabling this option up would make this serializer more valuable throughout the framework.

Copy link
Member

@smcvb smcvb May 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty confident this will require introducing the registerConverters method (as seen on line 128 of the JacksonSerializer too).
This method ensures Json specific ContentTypeConverters are added to the set-up.

Furthermore, I assume this will warrant introducing content type converters going from JsonElement to byte[], and vice versa.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally forgot about the (extra) converters. I will add some defaults to convert between byte arrays <-> Strings and JSON Nodes 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the JsonElement type the exception, just like JsonNode in the Jackson serializer. All other target types are serialized to String, and then converted with the chaining converter.

By default String, Byte array, Json Element and InputStream already work through service discovery, so that is fine.

If any other converters are needed I can add them.

Copy link
Member

@smcvb smcvb Sep 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for validating @hiddewie 👍
Adding tests to validate this works (as in, the conversion between those types through this serializer) is a massive plus.

throw org.axonframework.serialization.SerializationException("Cannot serialize type $type to representation $expectedRepresentation. String and JsonElement are supported.")
}
} catch (ex: SerializationException) {
throw org.axonframework.serialization.SerializationException("Cannot serialize type ${value?.javaClass?.name} to representation $expectedRepresentation.", ex)
}
}

override fun <T> canSerializeTo(expectedRepresentation: Class<T>): Boolean =
expectedRepresentation == String::class.java ||
expectedRepresentation == JsonElement::class.java

override fun <S, T> deserialize(serializedObject: SerializedObject<S>?): T? {
try {
if (serializedObject == null) {
return null
}

if (serializedObject.type == SerializedType.emptyType()) {
return null
}

val serializer: KSerializer<T> = serializedObject.serializer()
return when {
serializedObject.contentType.isAssignableFrom(String::class.java) ->
json.decodeFromString(serializer, serializedObject.data as String)

serializedObject.contentType.isAssignableFrom(JsonElement::class.java) ->
json.decodeFromJsonElement(serializer, serializedObject.data as JsonElement)

else ->
throw org.axonframework.serialization.SerializationException("Cannot deserialize from content type ${serializedObject.contentType} to type ${serializedObject.type}. String and JsonElement are supported.")
}
} catch (ex: SerializationException) {
throw org.axonframework.serialization.SerializationException(
"Could not deserialize from content type ${serializedObject?.contentType} to type ${serializedObject?.type}",
ex
)
}
}

private fun <S, T> SerializedObject<S>.serializer(): KSerializer<T> =
classForType(type).serializer() as KSerializer<T>

/**
* When a type is compiled by the Kotlin compiler extension, a companion object
* is created which contains a method `serializer()`. This method should be called
* to get the serializer of the class.
*
* In a 'normal' serialization environment, you would call the MyClass.serializer()
* method directly. Here we are in a generic setting, and need reflection to call
* the method.
*
* This method caches the reflection mapping from class to serializer for efficiency.
*/
private fun <T> Class<T>.serializer(): KSerializer<T> =
serializerCache.computeIfAbsent(this) {
// Class<T>: T must be non-null
val kClass = (this as Class<Any>).kotlin

val companion = kClass.companionObject
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that the hard requirement on the @Serializable annotation makes it pretty tough to use this serializer for Axon objects, like the TrackingToken.
I've skimmed the Kotlin Serializer documentation somewhat but didn't spot a solution for this just yet.
Do you, perchance, have a solution in mind for this, @hiddewie?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the toughest question :)

As far as I can see, there are two solutions:

  1. Annotate the serializable types in Axon core with @Serializable. This adds the compile time requirement of having the Kotlin compiler plugin dependency in the build process. That may or may not be OK. Although, there are also Jackson annotations in the source code already. In runtime there is no Kotlin requirement.
  2. Create handwritten serializers for every class that needs to be serializable. This is quite some (cumbersome, error prone) work, depending on the number of serializable classes in the Axon core. The serializer code can be maintained in this repository.

To illustrate option 2, I added a small example and custom serializer with test.

The third option is abandoning this approach because the Kotlin serialization is not wanted in the core framework, and it is not worth the effort to maintain the custom serializers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(After thinking about option 1 a bit more: I have not tested adding @Serializable on a Java class and then running the Kotlin compiler plugin on it. Theoretically it should work but in practice I don't know)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the insights here.
If you'd be at leisure to validate whether it is even possible to add the @Serializable annotation to a Java class, that would be amazing (and save me some personal investigation. 😅).

In the meantime, I'll start an internal discussion about whether we want to add the @Serializable annotation to the core of Axon Framework.

Copy link
Contributor Author

@hiddewie hiddewie Sep 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried a few things to test how the interoperability of Java classes with Kotlin annotations, and the Kotlin annotation processors work together.

What I did:

  • Complie Axon framewok locally (4.6.0-SNAPHSOT)
  • Add the Maven Kotlin compiler plugin which would compile the Kotlin annotations into serializer functions, see instructions here https://github.com/Kotlin/kotlinx.serialization#maven
  • Add @kotlinx.serialization.Serializable to ConfigToken

What I saw:

  • The annotation gets added (just like any other annotation) in the classfile
  • No serializer static method is generated.

What I did:

  • Copy the Java class and copy it to this repository, with the .java extension.
  • Let the Kotlin compiler, with the Kotlin serialization compiler plugin comple the .java class as well

What I saw:

  • The annotation gets added in the classfile
  • Still no serializer static method is generated.

What I did:

  • Let IntelliJ convert the .java file to a .kt Kotlin file (very ugly)

What I saw:

  • The annotation gets added in the classfile
  • A serializer method is generated.

So my conclusion is that the Kotlin serialization compiler plugin only works for Kotlin source files, and that the .serializer() static method cannot be generated for Java source files.

That is very unfortunate, I might open an issue with the Kotlin Serialization team to ask if that is supposed to work (ref Kotlin/kotlinx.serialization#1687).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either way, we can not assume that every class that comes here will have a serializer attached to it (nor be a Kotlin class)

If there is no serializer defined, or just based on types, we can fall back on a registry that we can keep as a map of Type (Class, Class name, etc) and a Serializer. That map could be configurable so someone might add support for classes without @Serializable like Java Dates, and we can pre-populate it with AF serializers configuration. Or keep the AF serializers in a separate config, but that's a finer detail.

This and
this is what I mean, as we need to pass the serializer explicitly in the very broad generic setting anyway, we might as well keep it somewhere in a config map, similar as to how this cache is doing but is just caching all serializers.

?: throw SerializationException("Class $this has no companion object. Did you mark it as @Serializable?")

val serializerMethod = companion.java.getMethod("serializer")
?: throw SerializationException("Class $this has no serializer() method. Did you mark it as @Serializable?")

serializerMethod.invoke(kClass.companionObjectInstance) as KSerializer<*>
} as KSerializer<T>

override fun classForType(type: SerializedType): Class<*> =
if (SerializedType.emptyType() == type) {
Void.TYPE
} else {
try {
Class.forName(type.name)
} catch (e: ClassNotFoundException) {
UnknownSerializedType::class.java
}
}

override fun typeForClass(type: Class<*>?): SerializedType =
if (type == null || Void.TYPE == type || Void::class.java == type) {
SimpleSerializedType.emptyType()
} else {
SimpleSerializedType(type.name, revisionResolver.revisionOf(type))
}

private fun revisionOf(type: Class<*>): String? =
revisionResolver.revisionOf(type)

override fun getConverter(): Converter =
converter

}

/**
* Configuration which will be used to construct a KotlinSerializer.
* This class is used in the kotlinSerializer DSL function.
*
* @see KotlinSerializer
* @see kotlinSerializer
*/
class KotlinSerializerConfiguration {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the driving force to use this format instead of a constructor with default values?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These properties are all var, so mutable. This makes a clean DSL possible, without invoking the constructor directly. But the DSL is totally optional and I could remove it.

Copy link
Member

@smcvb smcvb Sep 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I wouldn't know what's more "Kotlin-like".

If I'd look at the other Axon components, the settings are done through the Builder of a piece of infrastructure.
We've quite recently allowed Builder configuration directly through Axon's overall Configurer (for the PooledStreamingEventProcessor, to be exact), which we feel might be the way forward.

I'd wager that such a separate config object does not align very well with that idea.
However, a stated earlier, I am not aware what would be considered "Kotlin-like".

@sandjelkovic, what's your opinion on this?

Copy link
Contributor Author

@hiddewie hiddewie Sep 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this Configuration class, and used the constructor with default values. It is no problem to add it back later if it is needed.

The Configuration class could be seen as a builder pattern for a Kotlin-style DSL. The Configuration class contains the same properties as the constructor, and all mutable. In the DSL builder function you could configure the Configuration class (or builder, however you want to call it), and that will then invoke the constructor of the actual (immutable) service.

var revisionResolver: RevisionResolver = AnnotationRevisionResolver()
var converter: Converter = ChainingConverter()
var json: Json = Json
}

/**
* DSL function to configure a new KotlinSerializer.
*
* Usage example:
* <code>
* val serializer: KotlinSerializer = kotlinSerializer {
* json = Json
* converter = ChainingConverter()
* revisionResolver = AnnotationRevisionResolver()
* }
* </code>
*
* @see KotlinSerializer
*/
fun kotlinSerializer(init: KotlinSerializerConfiguration.() -> Unit = {}): KotlinSerializer {
val configuration = KotlinSerializerConfiguration()
configuration.init()
return KotlinSerializer(
configuration.revisionResolver,
configuration.converter,
configuration.json,
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package org.axonframework.extensions.kotlin.serialization

import kotlinx.serialization.Serializable
import kotlinx.serialization.json.Json
import kotlinx.serialization.json.JsonElement
import org.axonframework.serialization.AnnotationRevisionResolver
import org.axonframework.serialization.ChainingConverter
import org.axonframework.serialization.SerializedType
import org.axonframework.serialization.SimpleSerializedObject
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Test
import kotlin.test.assertNotNull
import kotlin.test.assertNull

class KotlinSerializerTest {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am missing test cases that validate if the KotlinSerializer can be used for Axon objects, like the Message, TrackingToken, and SagaEntry.

Assuming that's because those objects aren't annotated with the @Serializable annotation.
However, the Serializer in Axon must de-/serialize those objects for it to be usable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. See https://github.com/AxonFramework/extension-kotlin/pull/124/files#r645048334 for the comment about serializing internal classes.

I added one test case for the ConfigToken, and every internal class that should be serializable can be added in that way, if there is a serializer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha. To make this Serializer implementation workable for Axon Framework entirely, these tests will be a requirement too.

Making these can wait until we've concluded the conversation on the Serializer implementation though.
On the subject of using the @Serializable annotation on Axon components, and if that even works for Java classes, that is.

/**
* This class will automatically become serializable through the Kotlin serialization compiler plugin.
*/
@Serializable
data class TestData(
val name: String,
val value: Float?
)

@Test
fun canSerializeTo() {
val serializer = kotlinSerializer()

assertTrue(serializer.canSerializeTo(String::class.java))
assertTrue(serializer.canSerializeTo(JsonElement::class.java))
}

@Test
fun `configuration options`() {
val serializer = kotlinSerializer {
json = Json
converter = ChainingConverter()
revisionResolver = AnnotationRevisionResolver()
}
assertNotNull(serializer)
}

@Test
fun serialize() {
val serializer = kotlinSerializer()

val emptySerialized = serializer.serialize(TestData("", null), String::class.java)
assertEquals("SimpleSerializedType[org.axonframework.extensions.kotlin.serialization.KotlinSerializerTest\$TestData] (revision null)", emptySerialized.type.toString())
assertEquals("""{"name":"","value":null}""", emptySerialized.data)
assertEquals(String::class.java, emptySerialized.contentType)

val filledSerialized = serializer.serialize(TestData("name", 1.23f), String::class.java)
assertEquals("SimpleSerializedType[org.axonframework.extensions.kotlin.serialization.KotlinSerializerTest\$TestData] (revision null)", filledSerialized.type.toString())
assertEquals("""{"name":"name","value":1.23}""", filledSerialized.data)
assertEquals(String::class.java, filledSerialized.contentType)

val nullSerialized = serializer.serialize(null, String::class.java)
assertEquals("null", nullSerialized.data)
assertEquals(String::class.java, nullSerialized.contentType)
}

@Test
fun deserialize() {
val serializer = kotlinSerializer()

val nullDeserialized: Any? = serializer.deserialize(SimpleSerializedObject(
"",
String::class.java,
SerializedType.emptyType()
))
assertNull(nullDeserialized)

val emptyDeserialized: Any? = serializer.deserialize(SimpleSerializedObject(
"""{"name":"","value":null}""",
String::class.java,
TestData::class.java.name,
null
))
assertNotNull(emptyDeserialized as TestData)
assertEquals(emptyDeserialized.name, "")
assertEquals(emptyDeserialized.value, null)

val filledDeserialized: Any? = serializer.deserialize(SimpleSerializedObject(
"""{"name":"name","value":1.23}""",
String::class.java,
TestData::class.java.name,
null
))
assertNotNull(filledDeserialized as TestData)
assertEquals(filledDeserialized.name, "name")
assertEquals(filledDeserialized.value, 1.23f)
}
}
16 changes: 16 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<kotlin.version>1.5.10</kotlin.version>
<kotlin-logging.version>2.0.6</kotlin-logging.version>
<kotlin-serialization.version>1.2.1</kotlin-serialization.version>
<jackson-kotlin.version>2.12.3</jackson-kotlin.version>
<mockk.version>1.11.0</mockk.version>
<junit.jupiter.version>5.7.2</junit.jupiter.version>
Expand Down Expand Up @@ -130,6 +131,14 @@
<scope>test</scope>
</dependency>

<!-- Serialization -->
<dependency>
<groupId>org.jetbrains.kotlinx</groupId>
<artifactId>kotlinx-serialization-json</artifactId>
<version>${kotlin-serialization.version}</version>
<optional>true</optional>
</dependency>

<!-- Test dependencies -->
<dependency>
<groupId>org.junit.jupiter</groupId>
Expand Down Expand Up @@ -434,6 +443,7 @@
<compilerPlugins>
<plugin>no-arg</plugin>
<plugin>all-open</plugin>
<plugin>kotlinx-serialization</plugin>
</compilerPlugins>
<pluginOptions>
<!-- Axon specific classes -->
Expand Down Expand Up @@ -477,6 +487,12 @@
<artifactId>kotlin-maven-noarg</artifactId>
<version>${kotlin.version}</version>
</dependency>
<dependency>
<groupId>org.jetbrains.kotlin</groupId>
<artifactId>kotlin-maven-serialization</artifactId>
<version>${kotlin.version}</version>
<optional>true</optional>
</dependency>
</dependencies>
</plugin>
</plugins>
Expand Down