Follow-up improvements for #289 and #333

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<String>) {
-        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<String, Any?>): Map<String, JsonElement> = 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())
-        }
-    }
 }

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<String, Any?>): Map<String, JsonElement> = 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())
+        }
+    }
+}
+

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")
         }
     }