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

Bump kotlinx-serialization-json from 1.4.1 to 1.5.0 #31408

Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion bom/application/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@
<azure-functions-java-spi.version>1.0.0</azure-functions-java-spi.version>
<kotlin.version>1.8.10</kotlin.version>
<kotlin.coroutine.version>1.6.4</kotlin.coroutine.version>
<kotlin-serialization.version>1.4.1</kotlin-serialization.version>
<kotlin-serialization.version>1.5.0</kotlin-serialization.version>
<dekorate.version>3.4.1</dekorate.version> <!-- Please check with Java Operator SDK team before updating -->
<maven-invoker.version>3.2.0</maven-invoker.version>
<awaitility.version>4.2.0</awaitility.version>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
<groupId>org.jetbrains.kotlinx</groupId>
<artifactId>kotlinx-serialization-json</artifactId>
</dependency>
<dependency>
<groupId>org.jetbrains.kotlin</groupId>
<artifactId>kotlin-reflect</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-junit5-internal</artifactId>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.quarkus.resteasy.reactive.kotlin.serialization.common.runtime;

import java.util.Optional;
import java.util.StringJoiner;

import io.quarkus.runtime.annotations.ConfigGroup;
Expand Down Expand Up @@ -100,6 +101,28 @@ public class JsonConfig {
@ConfigItem(defaultValue = "false")
public boolean useArrayPolymorphism = false;

/**
* Specifies the {@code JsonNamingStrategy} that should be used for all properties in classes for serialization and
* deserialization.
* This strategy is applied for all entities that have {@code StructureKind.CLASS}.
* <p>
* <p>
* {@code null} by default.
* <p>
* <p>
* This element can be one of two things:
* <ol>
* <li>the fully qualified class name of a type implements the {@code NamingStrategy} interface and has a no-arg
* constructor</li>
* <li>a value in the form {@code NamingStrategy.SnakeCase} which refers to built-in values provided by the kotlin
* serialization
* library itself.
* </li>
* </ol>
*/
@ConfigItem(name = "naming-strategy")
public Optional<String> namingStrategy;
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't really need to be optional as we are supplying a default value.

Copy link
Member

Choose a reason for hiding this comment

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

Surprisingly this does work in native mode. I was surprised, too, because there were no @RegisterForReflection annotations. I thought maybe the when statement in the test directly referencing the custom JsonNamingStrategy might be tripping some wires but I changed that to a String instead and it still works. I can see printlns I added in both the JVM and native tests. My hypothesis at this point is that since JsonNamingStrategy shows up in the other parts of the API that graalvm makes a note and tracks all the subclasses it finds of that type. I honestly don't know otherwise.

As for the Optional, does null count as a default value? The impression I got looking at other configs was "no." It truly is an optional config element and kotlin serialization will happily chug along without it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting about native mode.

As for the Optional thing, we can do it like you have it, just saying that Optional is unnecessary as you have provided a default value


@Override
public String toString() {
return new StringJoiner(", ", JsonConfig.class.getSimpleName() + "[", "]")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,16 @@ import jakarta.enterprise.inject.Produces
import jakarta.inject.Singleton
import kotlinx.serialization.ExperimentalSerializationApi
import kotlinx.serialization.json.Json
import kotlinx.serialization.json.JsonBuilder
import kotlinx.serialization.json.JsonNamingStrategy
import kotlinx.serialization.json.JsonNamingStrategy.Builtins
import java.lang.Thread
import kotlin.reflect.KMutableProperty1
import kotlin.reflect.full.memberProperties
import kotlin.reflect.jvm.isAccessible

@Singleton
class JsonProducer {

@ExperimentalSerializationApi
@Singleton
@Produces
Expand All @@ -29,12 +35,65 @@ class JsonProducer {
useAlternativeNames = configuration.json.useAlternativeNames
useArrayPolymorphism = configuration.json.useArrayPolymorphism

configuration.json.namingStrategy.ifPresent { strategy ->
loadStrategy(this, strategy, this@JsonProducer)
}
val sortedCustomizers = sortCustomizersInDescendingPriorityOrder(customizers)
for (customizer in sortedCustomizers) {
customizer.customize(this)
}
}

@ExperimentalSerializationApi
private fun loadStrategy(
jsonBuilder: JsonBuilder,
strategy: String,
jsonProducer: JsonProducer
) {
val strategyProperty: KMutableProperty1<JsonBuilder, JsonNamingStrategy> = (
JsonBuilder::class.memberProperties
.find { member -> member.name == "namingStrategy" }
?: throw ReflectiveOperationException("Could not find the namingStrategy property on JsonBuilder")
) as KMutableProperty1<JsonBuilder, JsonNamingStrategy>
strategyProperty.isAccessible = true

strategyProperty.set(
jsonBuilder,
if (strategy.startsWith("JsonNamingStrategy")) {
jsonProducer.extractBuiltIn(strategy)
} else {
jsonProducer.loadStrategyClass(strategy)
}
)
}

@ExperimentalSerializationApi
private fun loadStrategyClass(
strategy: String
): JsonNamingStrategy {
try {
val strategyClass: Class<JsonNamingStrategy> = Thread.currentThread().contextClassLoader.loadClass(strategy) as Class<JsonNamingStrategy>
val constructor = strategyClass.constructors
.find { it.parameterCount == 0 }
?: throw ReflectiveOperationException("No no-arg constructor found on $strategy")
return constructor.newInstance() as JsonNamingStrategy
} catch (e: ReflectiveOperationException) {
throw IllegalArgumentException("Error loading naming strategy: ${strategy.substringAfter('.')}", e)
}
}

@ExperimentalSerializationApi
private fun extractBuiltIn(
strategy: String
): JsonNamingStrategy {
val kClass = Builtins::class
val property = kClass.memberProperties.find { property ->
property.name == strategy.substringAfter('.')
} ?: throw IllegalArgumentException("Unknown naming strategy provided: ${strategy.substringAfter('.')}")

return property.get(JsonNamingStrategy) as JsonNamingStrategy
}

private fun sortCustomizersInDescendingPriorityOrder(
customizers: Iterable<JsonBuilderCustomizer>
): List<JsonBuilderCustomizer> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class GreetingResource {
@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
fun marry(person: Person): Person {
return Person(person.name.substringBefore(" ") + " Halpert")
return Person(person.fullName.substringBefore(" ") + " Halpert")
}

@GET
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package io.quarkus.it.kotser

import kotlinx.serialization.ExperimentalSerializationApi
import kotlinx.serialization.descriptors.SerialDescriptor
import kotlinx.serialization.json.JsonNamingStrategy

@OptIn(ExperimentalSerializationApi::class)
class TitleCase : JsonNamingStrategy {
override fun serialNameForJson(descriptor: SerialDescriptor, elementIndex: Int, serialName: String): String {
return serialName[0].uppercase() + serialName.substring(1)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package io.quarkus.it.kotser.model
import kotlinx.serialization.Serializable

@Serializable
data class Person(var name: String, var defaulted: String = "hi there!") {
data class Person(var fullName: String, var defaulted: String = "hi there!") {
override fun toString(): String {
TODO("this shouldn't get called. a proper serialization should be invoked.")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package io.quarkus.it.kotser.model
import kotlinx.serialization.Serializable

@Serializable
data class Person2(var name: String, var defaulted: String = "hey") {
data class Person2(var fullName: String, var defaulted: String = "hey") {
override fun toString(): String {
TODO("this shouldn't get called. a proper serialization should be invoked.")
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
quarkus.kotlin-serialization.json.encode-defaults=true
quarkus.kotlin-serialization.json.encode-defaults=true
#quarkus.kotlin-serialization.json.naming-strategy=io.quarkus.it.kotser.TitleCase
#quarkus.kotlin-serialization.json.naming-strategy=JsonNamingStrategy.SnakeCase
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,36 @@ import jakarta.ws.rs.core.MediaType
import org.hamcrest.CoreMatchers
import org.hamcrest.CoreMatchers.`is`
import org.junit.jupiter.api.Test
import java.util.Properties

@QuarkusTest
open class ResourceTest {

val nameField: String
var defaulted = "defaulted"

init {
val properties = Properties()
properties.load(javaClass.getResourceAsStream("/application.properties"))
val strategy: String? = properties.get("quarkus.kotlin-serialization.json.naming-strategy") as String?
when (strategy) {
"JsonNamingStrategy.SnakeCase" -> nameField = "full_name"
TitleCase::class.qualifiedName -> {
nameField = "FullName"
defaulted = "Defaulted"
}
null -> nameField = "fullName"
else -> throw IllegalArgumentException("unknown strategy: $strategy")
}
}

@Test
fun testGetFlow() {
When {
get("/flow")
} Then {
statusCode(200)
body(`is`("""[{"name":"Jim Halpert","defaulted":"hi there!"}]"""))
body(`is`("""[{"$nameField":"Jim Halpert","$defaulted":"hi there!"}]"""))
}
}

Expand All @@ -28,7 +48,7 @@ open class ResourceTest {
get("/")
} Then {
statusCode(200)
body(`is`("""{"name":"Jim Halpert","defaulted":"hi there!"}"""))
body(`is`("""{"$nameField":"Jim Halpert","$defaulted":"hi there!"}"""))
}
}

Expand All @@ -38,7 +58,7 @@ open class ResourceTest {
get("/restResponse")
} Then {
statusCode(200)
body(`is`("""{"name":"Jim Halpert","defaulted":"hi there!"}"""))
body(`is`("""{"$nameField":"Jim Halpert","$defaulted":"hi there!"}"""))
}
}

Expand All @@ -48,7 +68,7 @@ open class ResourceTest {
get("/restResponseList")
} Then {
statusCode(200)
body(`is`("""[{"name":"Jim Halpert","defaulted":"hi there!"}]"""))
body(`is`("""[{"$nameField":"Jim Halpert","$defaulted":"hi there!"}]"""))
}
}

Expand All @@ -58,7 +78,7 @@ open class ResourceTest {
get("/unknownType")
} Then {
statusCode(200)
body(`is`("""{"name":"Foo Bar","defaulted":"hey"}"""))
body(`is`("""{"$nameField":"Foo Bar","$defaulted":"hey"}"""))
}
}

Expand All @@ -68,7 +88,7 @@ open class ResourceTest {
get("/suspend")
} Then {
statusCode(200)
body(`is`("""{"name":"Jim Halpert","defaulted":"hi there!"}"""))
body(`is`("""{"$nameField":"Jim Halpert","$defaulted":"hi there!"}"""))
}
}

Expand All @@ -78,20 +98,20 @@ open class ResourceTest {
get("/suspendList")
} Then {
statusCode(200)
body(`is`("""[{"name":"Jim Halpert","defaulted":"hi there!"}]"""))
body(`is`("""[{"$nameField":"Jim Halpert","$defaulted":"hi there!"}]"""))
}
}

@Test
fun testPost() {
Given {
body("""{ "name": "Pam Beasley" }""")
body("""{ "$nameField": "Pam Beasley" }""")
contentType(JSON)
} When {
post("/")
} Then {
statusCode(200)
body(`is`("""{"name":"Pam Halpert","defaulted":"hi there!"}"""))
body(`is`("""{"$nameField":"Pam Halpert","$defaulted":"hi there!"}"""))
}
}

Expand Down