From 5664f556864796dab56ab0566a654c00060f214c Mon Sep 17 00:00:00 2001 From: MAERYO Date: Fri, 24 Oct 2025 01:06:00 +0900 Subject: [PATCH] refactor: extract JSON converter and add meta assertions --- .../kotlin/sdk/client/Client.kt | 82 ++-------------- .../kotlin/sdk/client/JsonConverter.kt | 95 +++++++++++++++++++ .../sdk/client/ClientMetaParameterTest.kt | 56 ++++++++++- 3 files changed, 157 insertions(+), 76 deletions(-) create mode 100644 kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/JsonConverter.kt diff --git a/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/Client.kt b/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/Client.kt index 56fd1caf..a633644f 100644 --- a/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/Client.kt +++ b/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/Client.kt @@ -50,14 +50,7 @@ import kotlinx.atomicfu.update import kotlinx.collections.immutable.minus import kotlinx.collections.immutable.persistentMapOf import kotlinx.collections.immutable.toPersistentSet -import kotlinx.serialization.ExperimentalSerializationApi -import kotlinx.serialization.json.JsonElement -import kotlinx.serialization.json.JsonNull import kotlinx.serialization.json.JsonObject -import kotlinx.serialization.json.JsonPrimitive -import kotlinx.serialization.json.add -import kotlinx.serialization.json.buildJsonArray -import kotlinx.serialization.json.buildJsonObject import kotlin.coroutines.cancellation.CancellationException private val logger = KotlinLogging.logger {} @@ -87,6 +80,11 @@ public class ClientOptions( public open class Client(private val clientInfo: Implementation, options: ClientOptions = ClientOptions()) : Protocol(options) { + companion object { + private val labelPattern = Regex("[a-zA-Z]([a-zA-Z0-9-]*[a-zA-Z0-9])?") + private val namePattern = Regex("[a-zA-Z0-9]([a-zA-Z0-9._-]*[a-zA-Z0-9])?") + } + /** * Retrieves the server's reported capabilities after the initialization process completes. * @@ -427,8 +425,8 @@ public open class Client(private val clientInfo: Implementation, options: Client ): CallToolResultBase? { validateMetaKeys(meta.keys) - val jsonArguments = convertToJsonMap(arguments) - val jsonMeta = convertToJsonMap(meta) + val jsonArguments = JsonConverter.convertToJsonMap(arguments) + val jsonMeta = JsonConverter.convertToJsonMap(meta) val request = CallToolRequest( name = name, @@ -598,9 +596,6 @@ public open class Client(private val clientInfo: Implementation, options: Client * - Name: alphanumeric start/end, may contain hyphens, underscores, dots (empty allowed) */ private fun validateMetaKeys(keys: Set) { - val labelPattern = Regex("[a-zA-Z]([a-zA-Z0-9-]*[a-zA-Z0-9])?") - val namePattern = Regex("[a-zA-Z0-9]([a-zA-Z0-9._-]*[a-zA-Z0-9])?") - keys.forEach { key -> require(key.isNotEmpty()) { "Meta key cannot be empty" } @@ -637,67 +632,4 @@ public open class Client(private val clientInfo: Implementation, options: Client } } } - - private fun convertToJsonMap(map: Map): Map = map.mapValues { (key, value) -> - try { - convertToJsonElement(value) - } catch (e: Exception) { - logger.warn { "Failed to convert value for key '$key': ${e.message}. Using string representation." } - JsonPrimitive(value.toString()) - } - } - - @OptIn(ExperimentalUnsignedTypes::class, ExperimentalSerializationApi::class) - private fun convertToJsonElement(value: Any?): JsonElement = when (value) { - null -> JsonNull - - is JsonElement -> value - - is String -> JsonPrimitive(value) - - is Number -> JsonPrimitive(value) - - is Boolean -> JsonPrimitive(value) - - is Char -> JsonPrimitive(value.toString()) - - is Enum<*> -> JsonPrimitive(value.name) - - is Map<*, *> -> buildJsonObject { value.forEach { (k, v) -> put(k.toString(), convertToJsonElement(v)) } } - - is Collection<*> -> buildJsonArray { value.forEach { add(convertToJsonElement(it)) } } - - is Array<*> -> buildJsonArray { value.forEach { add(convertToJsonElement(it)) } } - - // Primitive arrays - is IntArray -> buildJsonArray { value.forEach { add(it) } } - - is LongArray -> buildJsonArray { value.forEach { add(it) } } - - is FloatArray -> buildJsonArray { value.forEach { add(it) } } - - is DoubleArray -> buildJsonArray { value.forEach { add(it) } } - - is BooleanArray -> buildJsonArray { value.forEach { add(it) } } - - is ShortArray -> buildJsonArray { value.forEach { add(it) } } - - is ByteArray -> buildJsonArray { value.forEach { add(it) } } - - is CharArray -> buildJsonArray { value.forEach { add(it.toString()) } } - - // Unsigned arrays - is UIntArray -> buildJsonArray { value.forEach { add(JsonPrimitive(it)) } } - - is ULongArray -> buildJsonArray { value.forEach { add(JsonPrimitive(it)) } } - - is UShortArray -> buildJsonArray { value.forEach { add(JsonPrimitive(it)) } } - - is UByteArray -> buildJsonArray { value.forEach { add(JsonPrimitive(it)) } } - - else -> { - logger.debug { "Converting unknown type ${value::class} to string: $value" } - JsonPrimitive(value.toString()) - } - } } diff --git a/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/JsonConverter.kt b/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/JsonConverter.kt new file mode 100644 index 00000000..6b0a823f --- /dev/null +++ b/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/JsonConverter.kt @@ -0,0 +1,95 @@ +package io.modelcontextprotocol.kotlin.sdk.client + +import io.github.oshai.kotlinlogging.KotlinLogging +import kotlinx.serialization.ExperimentalSerializationApi +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonNull +import kotlinx.serialization.json.JsonPrimitive +import kotlinx.serialization.json.add +import kotlinx.serialization.json.buildJsonArray +import kotlinx.serialization.json.buildJsonObject + +private val logger = KotlinLogging.logger {} + +/** + * Utility object for converting Kotlin values to JSON elements. + */ +internal object JsonConverter { + /** + * Converts a map of values to a map of JSON elements. + * + * @param map The map to convert. + * @return A map where each value has been converted to a JsonElement. + */ + fun convertToJsonMap(map: Map): Map = map.mapValues { (key, value) -> + try { + convertToJsonElement(value) + } catch (e: Exception) { + logger.warn { "Failed to convert value for key '$key': ${e.message}. Using string representation." } + JsonPrimitive(value.toString()) + } + } + + /** + * Converts a Kotlin value to a JSON element. + * + * Supports primitive types, collections, arrays, and nested structures. + * + * @param value The value to convert. + * @return The corresponding JsonElement. + */ + @OptIn(ExperimentalUnsignedTypes::class, ExperimentalSerializationApi::class) + fun convertToJsonElement(value: Any?): JsonElement = when (value) { + null -> JsonNull + + is JsonElement -> value + + is String -> JsonPrimitive(value) + + is Number -> JsonPrimitive(value) + + is Boolean -> JsonPrimitive(value) + + is Char -> JsonPrimitive(value.toString()) + + is Enum<*> -> JsonPrimitive(value.name) + + is Map<*, *> -> buildJsonObject { value.forEach { (k, v) -> put(k.toString(), convertToJsonElement(v)) } } + + is Collection<*> -> buildJsonArray { value.forEach { add(convertToJsonElement(it)) } } + + is Array<*> -> buildJsonArray { value.forEach { add(convertToJsonElement(it)) } } + + // Primitive arrays + is IntArray -> buildJsonArray { value.forEach { add(it) } } + + is LongArray -> buildJsonArray { value.forEach { add(it) } } + + is FloatArray -> buildJsonArray { value.forEach { add(it) } } + + is DoubleArray -> buildJsonArray { value.forEach { add(it) } } + + is BooleanArray -> buildJsonArray { value.forEach { add(it) } } + + is ShortArray -> buildJsonArray { value.forEach { add(it) } } + + is ByteArray -> buildJsonArray { value.forEach { add(it) } } + + is CharArray -> buildJsonArray { value.forEach { add(it.toString()) } } + + // Unsigned arrays + is UIntArray -> buildJsonArray { value.forEach { add(JsonPrimitive(it)) } } + + is ULongArray -> buildJsonArray { value.forEach { add(JsonPrimitive(it)) } } + + is UShortArray -> buildJsonArray { value.forEach { add(JsonPrimitive(it)) } } + + is UByteArray -> buildJsonArray { value.forEach { add(JsonPrimitive(it)) } } + + else -> { + logger.debug { "Converting unknown type ${value::class} to string: $value" } + JsonPrimitive(value.toString()) + } + } +} + diff --git a/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/ClientMetaParameterTest.kt b/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/ClientMetaParameterTest.kt index e7061073..af14272d 100644 --- a/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/ClientMetaParameterTest.kt +++ b/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/ClientMetaParameterTest.kt @@ -94,6 +94,19 @@ class ClientMetaParameterTest { } assertTrue(result.isSuccess, "Edge case valid meta keys should be accepted") + + mockTransport.lastJsonRpcRequest()?.let { request -> + val params = request.params as JsonObject + val metaField = params["_meta"] as JsonObject + + // Verify all edge case meta keys are present with correct values + assertEquals(edgeCaseValidMeta.size, metaField.size, "All edge case meta keys should be included") + assertEquals("single-char-prefix-empty-name", metaField["a/"]?.jsonPrimitive?.content) + assertEquals("alphanumeric-hyphen-prefix", metaField["a1-b2/test"]?.jsonPrimitive?.content) + assertEquals("long-prefix", metaField["long.domain.name.here/config"]?.jsonPrimitive?.content) + assertEquals("minimal-valid-key", metaField["x/a"]?.jsonPrimitive?.content) + assertEquals("alphanumeric-name-only", metaField["test123"]?.jsonPrimitive?.content) + } } @Test @@ -212,6 +225,34 @@ class ClientMetaParameterTest { assertEquals("tools/call", request.method) val params = request.params as JsonObject assertTrue(params.containsKey("_meta"), "Request should contain _meta field") + + val metaField = params["_meta"] as JsonObject + + // Verify string conversion + assertEquals("text", metaField["string"]?.jsonPrimitive?.content) + + // Verify number conversion + assertEquals(42, metaField["number"]?.jsonPrimitive?.int) + + // Verify boolean conversion + assertEquals(true, metaField["boolean"]?.jsonPrimitive?.boolean) + + // Verify null conversion + assertTrue(metaField.containsKey("null_value"), "Should contain null_value key") + + // Verify list conversion + assertTrue(metaField.containsKey("list"), "Should contain list") + + // Verify map conversion + assertTrue(metaField.containsKey("map"), "Should contain nested map") + val nestedMap = metaField["map"] as JsonObject + assertEquals("value", nestedMap["nested"]?.jsonPrimitive?.content) + + // Verify enum/string conversion + assertEquals("STRING", metaField["enum"]?.jsonPrimitive?.content) + + // Verify array conversion + assertTrue(metaField.containsKey("int_array"), "Should contain int_array") } } @@ -228,7 +269,20 @@ class ClientMetaParameterTest { mockTransport.lastJsonRpcRequest()?.let { request -> val params = request.params as JsonObject val metaField = params["_meta"] as JsonObject - assertTrue(metaField.containsKey("config")) + assertTrue(metaField.containsKey("config"), "Should contain config key") + + // Verify nested structure + val config = metaField["config"] as JsonObject + assertTrue(config.containsKey("database"), "Config should contain database") + assertTrue(config.containsKey("features"), "Config should contain features") + + // Verify database nested structure + val database = config["database"] as JsonObject + assertEquals("localhost", database["host"]?.jsonPrimitive?.content) + assertEquals(5432, database["port"]?.jsonPrimitive?.int) + + // Verify features list + assertTrue(config.containsKey("features"), "Config should contain features list") } }