Skip to content

Commit 5636faf

Browse files
kpavlovkpavlov
authored
Updated ServerPromptsTest, move AbstractTransport to it's own file (#376)
# Refactor and enhance prompt handling logic and tests, update Kotest version and modularize `AbstractTransport` - Added more post-conditions/scenarios to ServerPromptsTest - Extracted `AbstractTransport` to a dedicated file for modularity. - Improved Protocol connection state validation: When disconnected, the Protocol will throw `IllegalStateException` instead of `Error`. - Minor Kotlin fixes and suppressions to improve code quality. ## Motivation and Context Improve test scenarios in ServerPromptsTest ## How Has This Been Tested? Integration tests updated ## Breaking Changes When disconnected, the Protocol will throw `IllegalStateException` instead of `Error`. ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [x] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [x] Test update - [x] Refactoring ## Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. --> - [x] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [x] My code follows the repository's style guidelines - [x] New and existing tests pass locally - [ ] I have added appropriate error handling - [ ] I have added or updated documentation as needed ## Additional context <!-- Add any other context, implementation notes, or design decisions -->
1 parent 6afef10 commit 5636faf

File tree

7 files changed

+282
-67
lines changed

7 files changed

+282
-67
lines changed

‎kotlin-sdk-core/api/kotlin-sdk-core.api‎

Lines changed: 74 additions & 0 deletions
Large diffs are not rendered by default.

‎kotlin-sdk-core/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/shared/AbstractTransport.kt‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package io.modelcontextprotocol.kotlin.sdk.shared
2+
3+
import io.modelcontextprotocol.kotlin.sdk.JSONRPCMessage
4+
import kotlinx.coroutines.CompletableDeferred
5+
6+
/**
7+
* Implements [onClose], [onError] and [onMessage] functions of [Transport] providing
8+
* corresponding [_onClose], [_onError] and [_onMessage] properties to use for an implementation.
9+
*/
10+
@Suppress("PropertyName")
11+
public abstract class AbstractTransport : Transport {
12+
protected var _onClose: (() -> Unit) = {}
13+
private set
14+
protected var _onError: ((Throwable) -> Unit) = {}
15+
private set
16+
17+
// to not skip messages
18+
private val _onMessageInitialized = CompletableDeferred<Unit>()
19+
protected var _onMessage: (suspend ((JSONRPCMessage) -> Unit)) = {
20+
_onMessageInitialized.await()
21+
_onMessage.invoke(it)
22+
}
23+
private set
24+
25+
override fun onClose(block: () -> Unit) {
26+
val old = _onClose
27+
_onClose = {
28+
old()
29+
block()
30+
}
31+
}
32+
33+
override fun onError(block: (Throwable) -> Unit) {
34+
val old = _onError
35+
_onError = { e ->
36+
old(e)
37+
block(e)
38+
}
39+
}
40+
41+
override fun onMessage(block: suspend (JSONRPCMessage) -> Unit) {
42+
val old: suspend (JSONRPCMessage) -> Unit = when (_onMessageInitialized.isCompleted) {
43+
true -> _onMessage
44+
false -> { _ -> }
45+
}
46+
47+
_onMessage = { message ->
48+
old(message)
49+
block(message)
50+
}
51+
52+
_onMessageInitialized.complete(Unit)
53+
}
54+
}

‎kotlin-sdk-core/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/shared/Transport.kt‎

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package io.modelcontextprotocol.kotlin.sdk.shared
22

33
import io.modelcontextprotocol.kotlin.sdk.types.JSONRPCMessage
4-
import kotlinx.coroutines.CompletableDeferred
54

65
/**
76
* Describes the minimal contract for MCP transport that a client or server can communicate over.
@@ -47,53 +46,3 @@ public interface Transport {
4746
*/
4847
public fun onMessage(block: suspend (JSONRPCMessage) -> Unit)
4948
}
50-
51-
/**
52-
* Implements [onClose], [onError] and [onMessage] functions of [Transport] providing
53-
* corresponding [_onClose], [_onError] and [_onMessage] properties to use for an implementation.
54-
*/
55-
@Suppress("PropertyName")
56-
public abstract class AbstractTransport : Transport {
57-
protected var _onClose: (() -> Unit) = {}
58-
private set
59-
protected var _onError: ((Throwable) -> Unit) = {}
60-
private set
61-
62-
// to not skip messages
63-
private val _onMessageInitialized = CompletableDeferred<Unit>()
64-
protected var _onMessage: (suspend ((JSONRPCMessage) -> Unit)) = {
65-
_onMessageInitialized.await()
66-
_onMessage.invoke(it)
67-
}
68-
private set
69-
70-
override fun onClose(block: () -> Unit) {
71-
val old = _onClose
72-
_onClose = {
73-
old()
74-
block()
75-
}
76-
}
77-
78-
override fun onError(block: (Throwable) -> Unit) {
79-
val old = _onError
80-
_onError = { e ->
81-
old(e)
82-
block(e)
83-
}
84-
}
85-
86-
override fun onMessage(block: suspend (JSONRPCMessage) -> Unit) {
87-
val old: suspend (JSONRPCMessage) -> Unit = when (_onMessageInitialized.isCompleted) {
88-
true -> _onMessage
89-
false -> { _ -> }
90-
}
91-
92-
_onMessage = { message ->
93-
old(message)
94-
block(message)
95-
}
96-
97-
_onMessageInitialized.complete(Unit)
98-
}
99-
}

‎kotlin-sdk-core/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/types/common.kt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package io.modelcontextprotocol.kotlin.sdk.types
22

3+
import io.modelcontextprotocol.kotlin.sdk.types.Icon.Theme.Dark
4+
import io.modelcontextprotocol.kotlin.sdk.types.Icon.Theme.Light
35
import kotlinx.serialization.SerialName
46
import kotlinx.serialization.Serializable
57
import kotlinx.serialization.json.JsonObject
@@ -27,6 +29,14 @@ public val SUPPORTED_PROTOCOL_VERSIONS: List<String> = listOf(
2729
public sealed interface WithMeta {
2830
@SerialName("_meta")
2931
public val meta: JsonObject?
32+
33+
@Deprecated(
34+
message = "Use 'meta' instead.",
35+
replaceWith = ReplaceWith("meta"),
36+
)
37+
@Suppress("PropertyName", "VariableNaming")
38+
public val _meta: JsonObject
39+
get() = meta ?: EmptyJsonObject
3040
}
3141

3242
// ============================================================================

‎kotlin-sdk-core/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/types/prompts.kt‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,22 @@ public data class GetPromptRequest(override val params: GetPromptRequestParams)
113113
@EncodeDefault
114114
override val method: Method = Method.Defined.PromptsGet
115115

116+
@Deprecated(
117+
message = "Use constructor with GetPromptRequestParams instead",
118+
replaceWith = ReplaceWith("GetPromptRequest(GetPromptRequestParams(name, arguments, meta))"),
119+
)
120+
public constructor(
121+
name: String,
122+
arguments: Map<String, String>? = null,
123+
meta: RequestMeta? = null,
124+
) : this(
125+
GetPromptRequestParams(
126+
name = name,
127+
arguments = arguments,
128+
meta = meta,
129+
),
130+
)
131+
116132
/**
117133
* The name of the prompt or prompt template to retrieve.
118134
*/

‎kotlin-sdk-test/build.gradle.kts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ kotlin {
1414
implementation(dependencies.platform(libs.ktor.bom))
1515
implementation(project(":kotlin-sdk"))
1616
implementation(kotlin("test"))
17+
implementation(libs.kotest.assertions.core)
1718
implementation(libs.kotest.assertions.json)
1819
implementation(libs.kotlin.logging)
1920
implementation(libs.kotlinx.coroutines.test)

‎kotlin-sdk-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/OldSchemaServerPromptsTest.kt‎

Lines changed: 127 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,24 @@
11
package io.modelcontextprotocol.kotlin.sdk.server
22

3+
import io.kotest.assertions.throwables.shouldThrow
4+
import io.kotest.matchers.collections.shouldBeEmpty
5+
import io.kotest.matchers.collections.shouldContain
6+
import io.kotest.matchers.collections.shouldContainExactly
7+
import io.kotest.matchers.collections.shouldHaveSize
8+
import io.kotest.matchers.nulls.shouldNotBeNull
9+
import io.kotest.matchers.shouldBe
10+
import io.kotest.matchers.throwable.shouldHaveMessage
11+
import io.modelcontextprotocol.kotlin.sdk.EmptyJsonObject
12+
import io.modelcontextprotocol.kotlin.sdk.GetPromptRequest
313
import io.modelcontextprotocol.kotlin.sdk.GetPromptResult
414
import io.modelcontextprotocol.kotlin.sdk.Implementation
515
import io.modelcontextprotocol.kotlin.sdk.Method
616
import io.modelcontextprotocol.kotlin.sdk.Prompt
717
import io.modelcontextprotocol.kotlin.sdk.ServerCapabilities
18+
import io.modelcontextprotocol.kotlin.sdk.types.McpException
819
import kotlinx.coroutines.CompletableDeferred
920
import kotlinx.coroutines.test.runTest
21+
import org.junit.jupiter.api.Nested
1022
import org.junit.jupiter.api.Test
1123
import org.junit.jupiter.api.assertThrows
1224
import kotlin.test.assertEquals
@@ -21,25 +33,84 @@ class OldSchemaServerPromptsTest : OldSchemaAbstractServerFeaturesTest() {
2133
)
2234

2335
@Test
24-
fun `removePrompt should remove a prompt`() = runTest {
36+
fun `Should list no prompts by default`() = runTest {
37+
client.listPrompts() shouldNotBeNull {
38+
prompts.shouldBeEmpty()
39+
}
40+
}
41+
42+
@Test
43+
fun `Should add a prompt`() = runTest {
2544
// Add a prompt
26-
val testPrompt = Prompt("test-prompt", "Test Prompt", null)
45+
val testPrompt = Prompt(
46+
name = "test-prompt-with-custom-handler",
47+
description = "Test Prompt",
48+
arguments = null,
49+
)
50+
val expectedPromptResult = GetPromptResult(
51+
description = "Test prompt description",
52+
messages = listOf(),
53+
)
54+
55+
server.addPrompt(testPrompt) {
56+
expectedPromptResult
57+
}
58+
59+
client.getPrompt(
60+
GetPromptRequest(
61+
name = "test-prompt-with-custom-handler",
62+
arguments = null,
63+
),
64+
) shouldBe expectedPromptResult
65+
66+
client.listPrompts() shouldNotBeNull {
67+
prompts shouldContainExactly listOf(testPrompt)
68+
nextCursor shouldBe null
69+
_meta shouldBe EmptyJsonObject
70+
}
71+
}
72+
73+
@Test
74+
fun `Should remove a prompt`() = runTest {
75+
// given
76+
val testPrompt = Prompt(
77+
name = "test-prompt-to-remove",
78+
description = "Test Prompt",
79+
arguments = null,
80+
)
2781
server.addPrompt(testPrompt) {
2882
GetPromptResult(
2983
description = "Test prompt description",
3084
messages = listOf(),
3185
)
3286
}
3387

34-
// Remove the prompt
88+
client.listPrompts() shouldNotBeNull {
89+
prompts shouldContain testPrompt
90+
}
91+
92+
// when
3593
val result = server.removePrompt(testPrompt.name)
3694

37-
// Verify the prompt was removed
95+
// then
3896
assertTrue(result, "Prompt should be removed successfully")
97+
val mcpException = shouldThrow<McpException> {
98+
client.getPrompt(
99+
GetPromptRequest(
100+
name = testPrompt.name,
101+
arguments = null,
102+
),
103+
)
104+
}
105+
mcpException shouldHaveMessage "MCP error -32603: Prompt not found: ${testPrompt.name}"
106+
107+
client.listPrompts() shouldNotBeNull {
108+
prompts.firstOrNull { it.name == testPrompt.name } shouldBe null
109+
}
39110
}
40111

41112
@Test
42-
fun `removePrompts should remove multiple prompts and send notification`() = runTest {
113+
fun `Should remove multiple prompts and send notification`() = runTest {
43114
// Add prompts
44115
val testPrompt1 = Prompt("test-prompt-1", "Test Prompt 1", null)
45116
val testPrompt2 = Prompt("test-prompt-2", "Test Prompt 2", null)
@@ -56,11 +127,17 @@ class OldSchemaServerPromptsTest : OldSchemaAbstractServerFeaturesTest() {
56127
)
57128
}
58129

130+
client.listPrompts() shouldNotBeNull {
131+
prompts shouldHaveSize 2
132+
}
59133
// Remove the prompts
60134
val result = server.removePrompts(listOf(testPrompt1.name, testPrompt2.name))
61135

62136
// Verify the prompts were removed
63137
assertEquals(2, result, "Both prompts should be removed")
138+
client.listPrompts() shouldNotBeNull {
139+
prompts.shouldBeEmpty()
140+
}
64141
}
65142

66143
@Test
@@ -82,21 +159,55 @@ class OldSchemaServerPromptsTest : OldSchemaAbstractServerFeaturesTest() {
82159
assertFalse(promptListChangedNotificationReceived, "No notification should be sent when prompt doesn't exist")
83160
}
84161

85-
@Test
86-
fun `removePrompt should throw when prompts capability is not supported`() = runTest {
162+
@Nested
163+
inner class NoPromptsCapabilitiesTests {
87164
// Create server without prompts capability
88-
val serverOptions = ServerOptions(
89-
capabilities = ServerCapabilities(),
90-
)
91-
val server = Server(
165+
val serverWithoutPrompts = Server(
92166
Implementation(name = "test server", version = "1.0"),
93-
serverOptions,
167+
ServerOptions(
168+
capabilities = ServerCapabilities(),
169+
),
94170
)
95171

96-
// Verify that removing a prompt throws an exception
97-
val exception = assertThrows<IllegalStateException> {
98-
server.removePrompt("test-prompt")
172+
@Test
173+
fun `RemovePrompt should throw when prompts capability is not supported`() = runTest {
174+
// Verify that removing a prompt throws an exception
175+
val exception = assertThrows<IllegalStateException> {
176+
serverWithoutPrompts.removePrompt("test-prompt")
177+
}
178+
assertEquals("Server does not support prompts capability.", exception.message)
179+
}
180+
181+
@Test
182+
fun `Remove Prompts should throw when prompts capability is not supported`() = runTest {
183+
// Verify that removing a prompt throws an exception
184+
val exception = assertThrows<IllegalStateException> {
185+
serverWithoutPrompts.removePrompts(emptyList())
186+
}
187+
assertEquals("Server does not support prompts capability.", exception.message)
188+
}
189+
190+
@Test
191+
fun `Add Prompt should throw when prompts capability is not supported`() = runTest {
192+
// Verify that removing a prompt throws an exception
193+
val exception = assertThrows<IllegalStateException> {
194+
serverWithoutPrompts.addPrompt(name = "test-prompt") {
195+
GetPromptResult(
196+
description = "Test prompt description",
197+
messages = listOf(),
198+
)
199+
}
200+
}
201+
assertEquals("Server does not support prompts capability.", exception.message)
202+
}
203+
204+
@Test
205+
fun `Add Prompts should throw when prompts capability is not supported`() = runTest {
206+
// Verify that removing a prompt throws an exception
207+
val exception = assertThrows<IllegalStateException> {
208+
serverWithoutPrompts.addPrompts(emptyList())
209+
}
210+
assertEquals("Server does not support prompts capability.", exception.message)
99211
}
100-
assertEquals("Server does not support prompts capability.", exception.message)
101212
}
102213
}

0 commit comments

Comments
 (0)