From 053f0f5cd3f4ae965f696466f68d2405a61a88ea Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Fri, 27 Jun 2025 11:11:16 +0200 Subject: [PATCH 1/2] refactor: return immutable lists from list methods - Wrap the list responses in immutable lists. - Add test to verify the immutable responses. Signed-off-by: Christian Tzolov --- .../client/AbstractMcpAsyncClientTests.java | 58 +++++++++++++++++ .../client/McpAsyncClient.java | 62 +++++++++---------- .../client/AbstractMcpAsyncClientTests.java | 58 +++++++++++++++++ 3 files changed, 144 insertions(+), 34 deletions(-) diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java index 4f7d5678..9cccd396 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java @@ -182,6 +182,20 @@ void testListAllTools() { }); } + @Test + void testListAllToolsReturnsImmutableList() { + withClient(createMcpTransport(), mcpAsyncClient -> { + StepVerifier.create(mcpAsyncClient.initialize().then(mcpAsyncClient.listTools())) + .consumeNextWith(result -> { + assertThat(result.tools()).isNotNull(); + // Verify that the returned list is immutable + assertThatThrownBy(() -> result.tools().add(new Tool("test", "test", "{\"type\":\"object\"}"))) + .isInstanceOf(UnsupportedOperationException.class); + }) + .verifyComplete(); + }); + } + @Test void testPingWithoutInitialization() { verifyCallSucceedsWithImplicitInitialization(client -> client.ping(), "pinging the server"); @@ -333,6 +347,21 @@ void testListAllResources() { }); } + @Test + void testListAllResourcesReturnsImmutableList() { + withClient(createMcpTransport(), mcpAsyncClient -> { + StepVerifier.create(mcpAsyncClient.initialize().then(mcpAsyncClient.listResources())) + .consumeNextWith(result -> { + assertThat(result.resources()).isNotNull(); + // Verify that the returned list is immutable + assertThatThrownBy( + () -> result.resources().add(Resource.builder().uri("test://uri").name("test").build())) + .isInstanceOf(UnsupportedOperationException.class); + }) + .verifyComplete(); + }); + } + @Test void testMcpAsyncClientState() { withClient(createMcpTransport(), mcpAsyncClient -> { @@ -384,6 +413,20 @@ void testListAllPrompts() { }); } + @Test + void testListAllPromptsReturnsImmutableList() { + withClient(createMcpTransport(), mcpAsyncClient -> { + StepVerifier.create(mcpAsyncClient.initialize().then(mcpAsyncClient.listPrompts())) + .consumeNextWith(result -> { + assertThat(result.prompts()).isNotNull(); + // Verify that the returned list is immutable + assertThatThrownBy(() -> result.prompts().add(new Prompt("test", "test", null))) + .isInstanceOf(UnsupportedOperationException.class); + }) + .verifyComplete(); + }); + } + @Test void testGetPromptWithoutInitialization() { GetPromptRequest request = new GetPromptRequest("simple_prompt", Map.of()); @@ -553,6 +596,21 @@ void testListAllResourceTemplates() { }); } + @Test + void testListAllResourceTemplatesReturnsImmutableList() { + withClient(createMcpTransport(), mcpAsyncClient -> { + StepVerifier.create(mcpAsyncClient.initialize().then(mcpAsyncClient.listResourceTemplates())) + .consumeNextWith(result -> { + assertThat(result.resourceTemplates()).isNotNull(); + // Verify that the returned list is immutable + assertThatThrownBy(() -> result.resourceTemplates() + .add(new McpSchema.ResourceTemplate("test://template", "test", null, null, null))) + .isInstanceOf(UnsupportedOperationException.class); + }) + .verifyComplete(); + }); + } + // @Test void testResourceSubscription() { withClient(createMcpTransport(), mcpAsyncClient -> { diff --git a/mcp/src/main/java/io/modelcontextprotocol/client/McpAsyncClient.java b/mcp/src/main/java/io/modelcontextprotocol/client/McpAsyncClient.java index 617cec17..49101112 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/client/McpAsyncClient.java +++ b/mcp/src/main/java/io/modelcontextprotocol/client/McpAsyncClient.java @@ -5,6 +5,7 @@ import java.time.Duration; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -674,15 +675,13 @@ public Mono callTool(McpSchema.CallToolRequest callToo * @return A Mono that emits the list of all tools result */ public Mono listTools() { - return this.listTools(McpSchema.FIRST_PAGE).expand(result -> { - if (result.nextCursor() != null) { - return this.listTools(result.nextCursor()); - } - return Mono.empty(); - }).reduce(new McpSchema.ListToolsResult(new ArrayList<>(), null), (allToolsResult, result) -> { - allToolsResult.tools().addAll(result.tools()); - return allToolsResult; - }); + return this.listTools(McpSchema.FIRST_PAGE) + .expand(result -> (result.nextCursor() != null) ? this.listTools(result.nextCursor()) : Mono.empty()) + .reduce(new McpSchema.ListToolsResult(new ArrayList<>(), null), (allToolsResult, result) -> { + allToolsResult.tools().addAll(result.tools()); + return allToolsResult; + }) + .map(result -> new McpSchema.ListToolsResult(Collections.unmodifiableList(result.tools()), null)); } /** @@ -736,15 +735,13 @@ private NotificationHandler asyncToolsChangeNotificationHandler( * @see #readResource(McpSchema.Resource) */ public Mono listResources() { - return this.listResources(McpSchema.FIRST_PAGE).expand(result -> { - if (result.nextCursor() != null) { - return this.listResources(result.nextCursor()); - } - return Mono.empty(); - }).reduce(new McpSchema.ListResourcesResult(new ArrayList<>(), null), (allResourcesResult, result) -> { - allResourcesResult.resources().addAll(result.resources()); - return allResourcesResult; - }); + return this.listResources(McpSchema.FIRST_PAGE) + .expand(result -> (result.nextCursor() != null) ? this.listResources(result.nextCursor()) : Mono.empty()) + .reduce(new McpSchema.ListResourcesResult(new ArrayList<>(), null), (allResourcesResult, result) -> { + allResourcesResult.resources().addAll(result.resources()); + return allResourcesResult; + }) + .map(result -> new McpSchema.ListResourcesResult(Collections.unmodifiableList(result.resources()), null)); } /** @@ -806,17 +803,16 @@ public Mono readResource(McpSchema.ReadResourceReq * @see McpSchema.ListResourceTemplatesResult */ public Mono listResourceTemplates() { - return this.listResourceTemplates(McpSchema.FIRST_PAGE).expand(result -> { - if (result.nextCursor() != null) { - return this.listResourceTemplates(result.nextCursor()); - } - return Mono.empty(); - }) + return this.listResourceTemplates(McpSchema.FIRST_PAGE) + .expand(result -> (result.nextCursor() != null) ? this.listResourceTemplates(result.nextCursor()) + : Mono.empty()) .reduce(new McpSchema.ListResourceTemplatesResult(new ArrayList<>(), null), (allResourceTemplatesResult, result) -> { allResourceTemplatesResult.resourceTemplates().addAll(result.resourceTemplates()); return allResourceTemplatesResult; - }); + }) + .map(result -> new McpSchema.ListResourceTemplatesResult( + Collections.unmodifiableList(result.resourceTemplates()), null)); } /** @@ -911,15 +907,13 @@ private NotificationHandler asyncResourcesUpdatedNotificationHandler( * @see #getPrompt(GetPromptRequest) */ public Mono listPrompts() { - return this.listPrompts(McpSchema.FIRST_PAGE).expand(result -> { - if (result.nextCursor() != null) { - return this.listPrompts(result.nextCursor()); - } - return Mono.empty(); - }).reduce(new ListPromptsResult(new ArrayList<>(), null), (allPromptsResult, result) -> { - allPromptsResult.prompts().addAll(result.prompts()); - return allPromptsResult; - }); + return this.listPrompts(McpSchema.FIRST_PAGE) + .expand(result -> (result.nextCursor() != null) ? this.listPrompts(result.nextCursor()) : Mono.empty()) + .reduce(new ListPromptsResult(new ArrayList<>(), null), (allPromptsResult, result) -> { + allPromptsResult.prompts().addAll(result.prompts()); + return allPromptsResult; + }) + .map(result -> new McpSchema.ListPromptsResult(Collections.unmodifiableList(result.prompts()), null)); } /** diff --git a/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java b/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java index 9be6e553..a3edda2b 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java @@ -183,6 +183,20 @@ void testListAllTools() { }); } + @Test + void testListAllToolsReturnsImmutableList() { + withClient(createMcpTransport(), mcpAsyncClient -> { + StepVerifier.create(mcpAsyncClient.initialize().then(mcpAsyncClient.listTools())) + .consumeNextWith(result -> { + assertThat(result.tools()).isNotNull(); + // Verify that the returned list is immutable + assertThatThrownBy(() -> result.tools().add(new Tool("test", "test", "{\"type\":\"object\"}"))) + .isInstanceOf(UnsupportedOperationException.class); + }) + .verifyComplete(); + }); + } + @Test void testPingWithoutInitialization() { verifyCallSucceedsWithImplicitInitialization(client -> client.ping(), "pinging the server"); @@ -334,6 +348,21 @@ void testListAllResources() { }); } + @Test + void testListAllResourcesReturnsImmutableList() { + withClient(createMcpTransport(), mcpAsyncClient -> { + StepVerifier.create(mcpAsyncClient.initialize().then(mcpAsyncClient.listResources())) + .consumeNextWith(result -> { + assertThat(result.resources()).isNotNull(); + // Verify that the returned list is immutable + assertThatThrownBy( + () -> result.resources().add(Resource.builder().uri("test://uri").name("test").build())) + .isInstanceOf(UnsupportedOperationException.class); + }) + .verifyComplete(); + }); + } + @Test void testMcpAsyncClientState() { withClient(createMcpTransport(), mcpAsyncClient -> { @@ -385,6 +414,20 @@ void testListAllPrompts() { }); } + @Test + void testListAllPromptsReturnsImmutableList() { + withClient(createMcpTransport(), mcpAsyncClient -> { + StepVerifier.create(mcpAsyncClient.initialize().then(mcpAsyncClient.listPrompts())) + .consumeNextWith(result -> { + assertThat(result.prompts()).isNotNull(); + // Verify that the returned list is immutable + assertThatThrownBy(() -> result.prompts().add(new Prompt("test", "test", null))) + .isInstanceOf(UnsupportedOperationException.class); + }) + .verifyComplete(); + }); + } + @Test void testGetPromptWithoutInitialization() { GetPromptRequest request = new GetPromptRequest("simple_prompt", Map.of()); @@ -554,6 +597,21 @@ void testListAllResourceTemplates() { }); } + @Test + void testListAllResourceTemplatesReturnsImmutableList() { + withClient(createMcpTransport(), mcpAsyncClient -> { + StepVerifier.create(mcpAsyncClient.initialize().then(mcpAsyncClient.listResourceTemplates())) + .consumeNextWith(result -> { + assertThat(result.resourceTemplates()).isNotNull(); + // Verify that the returned list is immutable + assertThatThrownBy(() -> result.resourceTemplates() + .add(new McpSchema.ResourceTemplate("test://template", "test", null, null, null))) + .isInstanceOf(UnsupportedOperationException.class); + }) + .verifyComplete(); + }); + } + // @Test void testResourceSubscription() { withClient(createMcpTransport(), mcpAsyncClient -> { From ebc6d9e5112c68376dd9ecd75a7d75d956e8d758 Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Fri, 27 Jun 2025 13:48:50 +0200 Subject: [PATCH 2/2] better timeout handling Signed-off-by: Christian Tzolov --- .../WebFluxSseIntegrationTests.java | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/mcp-spring/mcp-spring-webflux/src/test/java/io/modelcontextprotocol/WebFluxSseIntegrationTests.java b/mcp-spring/mcp-spring-webflux/src/test/java/io/modelcontextprotocol/WebFluxSseIntegrationTests.java index 2f85654e..cac0ffac 100644 --- a/mcp-spring/mcp-spring-webflux/src/test/java/io/modelcontextprotocol/WebFluxSseIntegrationTests.java +++ b/mcp-spring/mcp-spring-webflux/src/test/java/io/modelcontextprotocol/WebFluxSseIntegrationTests.java @@ -432,12 +432,6 @@ void testCreateElicitationWithRequestTimeoutSuccess(String clientType) { Function elicitationHandler = request -> { assertThat(request.message()).isNotEmpty(); assertThat(request.requestedSchema()).isNotNull(); - try { - TimeUnit.SECONDS.sleep(2); - } - catch (InterruptedException e) { - throw new RuntimeException(e); - } return new ElicitResult(ElicitResult.Action.ACCEPT, Map.of("message", request.message())); }; @@ -491,14 +485,18 @@ void testCreateElicitationWithRequestTimeoutSuccess(String clientType) { @ValueSource(strings = { "httpclient", "webflux" }) void testCreateElicitationWithRequestTimeoutFail(String clientType) { + var latch = new CountDownLatch(1); // Client var clientBuilder = clientBuilders.get(clientType); Function elicitationHandler = request -> { assertThat(request.message()).isNotEmpty(); assertThat(request.requestedSchema()).isNotNull(); + try { - TimeUnit.SECONDS.sleep(2); + if (!latch.await(2, TimeUnit.SECONDS)) { + throw new RuntimeException("Timeout waiting for elicitation processing"); + } } catch (InterruptedException e) { throw new RuntimeException(e); @@ -536,7 +534,7 @@ void testCreateElicitationWithRequestTimeoutFail(String clientType) { var mcpServer = McpServer.async(mcpServerTransportProvider) .serverInfo("test-server", "1.0.0") - .requestTimeout(Duration.ofSeconds(1)) + .requestTimeout(Duration.ofSeconds(1)) // 1 second. .tools(tool) .build();