Skip to content

Commit d9006ec

Browse files
tzolovtzolov
authored
refactor: return immutable lists from list methods (#345)
 - Wrap the list responses in immutable lists. - Add test to verify the immutable responses. - Better timeout handling Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
1 parent 2f55dd0 commit d9006ec

File tree

4 files changed

+150
-42
lines changed

4 files changed

+150
-42
lines changed

‎mcp-spring/mcp-spring-webflux/src/test/java/io/modelcontextprotocol/WebFluxSseIntegrationTests.java‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -432,12 +432,6 @@ void testCreateElicitationWithRequestTimeoutSuccess(String clientType) {
432432
Function<ElicitRequest, ElicitResult> elicitationHandler = request -> {
433433
assertThat(request.message()).isNotEmpty();
434434
assertThat(request.requestedSchema()).isNotNull();
435-
try {
436-
TimeUnit.SECONDS.sleep(2);
437-
}
438-
catch (InterruptedException e) {
439-
throw new RuntimeException(e);
440-
}
441435
return new ElicitResult(ElicitResult.Action.ACCEPT, Map.of("message", request.message()));
442436
};
443437

@@ -491,14 +485,18 @@ void testCreateElicitationWithRequestTimeoutSuccess(String clientType) {
491485
@ValueSource(strings = { "httpclient", "webflux" })
492486
void testCreateElicitationWithRequestTimeoutFail(String clientType) {
493487

488+
var latch = new CountDownLatch(1);
494489
// Client
495490
var clientBuilder = clientBuilders.get(clientType);
496491

497492
Function<ElicitRequest, ElicitResult> elicitationHandler = request -> {
498493
assertThat(request.message()).isNotEmpty();
499494
assertThat(request.requestedSchema()).isNotNull();
495+
500496
try {
501-
TimeUnit.SECONDS.sleep(2);
497+
if (!latch.await(2, TimeUnit.SECONDS)) {
498+
throw new RuntimeException("Timeout waiting for elicitation processing");
499+
}
502500
}
503501
catch (InterruptedException e) {
504502
throw new RuntimeException(e);
@@ -536,7 +534,7 @@ void testCreateElicitationWithRequestTimeoutFail(String clientType) {
536534

537535
var mcpServer = McpServer.async(mcpServerTransportProvider)
538536
.serverInfo("test-server", "1.0.0")
539-
.requestTimeout(Duration.ofSeconds(1))
537+
.requestTimeout(Duration.ofSeconds(1)) // 1 second.
540538
.tools(tool)
541539
.build();
542540

‎mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,20 @@ void testListAllTools() {
182182
});
183183
}
184184

185+
@Test
186+
void testListAllToolsReturnsImmutableList() {
187+
withClient(createMcpTransport(), mcpAsyncClient -> {
188+
StepVerifier.create(mcpAsyncClient.initialize().then(mcpAsyncClient.listTools()))
189+
.consumeNextWith(result -> {
190+
assertThat(result.tools()).isNotNull();
191+
// Verify that the returned list is immutable
192+
assertThatThrownBy(() -> result.tools().add(new Tool("test", "test", "{\"type\":\"object\"}")))
193+
.isInstanceOf(UnsupportedOperationException.class);
194+
})
195+
.verifyComplete();
196+
});
197+
}
198+
185199
@Test
186200
void testPingWithoutInitialization() {
187201
verifyCallSucceedsWithImplicitInitialization(client -> client.ping(), "pinging the server");
@@ -333,6 +347,21 @@ void testListAllResources() {
333347
});
334348
}
335349

350+
@Test
351+
void testListAllResourcesReturnsImmutableList() {
352+
withClient(createMcpTransport(), mcpAsyncClient -> {
353+
StepVerifier.create(mcpAsyncClient.initialize().then(mcpAsyncClient.listResources()))
354+
.consumeNextWith(result -> {
355+
assertThat(result.resources()).isNotNull();
356+
// Verify that the returned list is immutable
357+
assertThatThrownBy(
358+
() -> result.resources().add(Resource.builder().uri("test://uri").name("test").build()))
359+
.isInstanceOf(UnsupportedOperationException.class);
360+
})
361+
.verifyComplete();
362+
});
363+
}
364+
336365
@Test
337366
void testMcpAsyncClientState() {
338367
withClient(createMcpTransport(), mcpAsyncClient -> {
@@ -384,6 +413,20 @@ void testListAllPrompts() {
384413
});
385414
}
386415

416+
@Test
417+
void testListAllPromptsReturnsImmutableList() {
418+
withClient(createMcpTransport(), mcpAsyncClient -> {
419+
StepVerifier.create(mcpAsyncClient.initialize().then(mcpAsyncClient.listPrompts()))
420+
.consumeNextWith(result -> {
421+
assertThat(result.prompts()).isNotNull();
422+
// Verify that the returned list is immutable
423+
assertThatThrownBy(() -> result.prompts().add(new Prompt("test", "test", null)))
424+
.isInstanceOf(UnsupportedOperationException.class);
425+
})
426+
.verifyComplete();
427+
});
428+
}
429+
387430
@Test
388431
void testGetPromptWithoutInitialization() {
389432
GetPromptRequest request = new GetPromptRequest("simple_prompt", Map.of());
@@ -553,6 +596,21 @@ void testListAllResourceTemplates() {
553596
});
554597
}
555598

599+
@Test
600+
void testListAllResourceTemplatesReturnsImmutableList() {
601+
withClient(createMcpTransport(), mcpAsyncClient -> {
602+
StepVerifier.create(mcpAsyncClient.initialize().then(mcpAsyncClient.listResourceTemplates()))
603+
.consumeNextWith(result -> {
604+
assertThat(result.resourceTemplates()).isNotNull();
605+
// Verify that the returned list is immutable
606+
assertThatThrownBy(() -> result.resourceTemplates()
607+
.add(new McpSchema.ResourceTemplate("test://template", "test", null, null, null)))
608+
.isInstanceOf(UnsupportedOperationException.class);
609+
})
610+
.verifyComplete();
611+
});
612+
}
613+
556614
// @Test
557615
void testResourceSubscription() {
558616
withClient(createMcpTransport(), mcpAsyncClient -> {

‎mcp/src/main/java/io/modelcontextprotocol/client/McpAsyncClient.java‎

Lines changed: 28 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import java.time.Duration;
77
import java.util.ArrayList;
8+
import java.util.Collections;
89
import java.util.HashMap;
910
import java.util.List;
1011
import java.util.Map;
@@ -677,15 +678,13 @@ public Mono<McpSchema.CallToolResult> callTool(McpSchema.CallToolRequest callToo
677678
* @return A Mono that emits the list of all tools result
678679
*/
679680
public Mono<McpSchema.ListToolsResult> listTools() {
680-
return this.listTools(McpSchema.FIRST_PAGE).expand(result -> {
681-
if (result.nextCursor() != null) {
682-
return this.listTools(result.nextCursor());
683-
}
684-
return Mono.empty();
685-
}).reduce(new McpSchema.ListToolsResult(new ArrayList<>(), null), (allToolsResult, result) -> {
686-
allToolsResult.tools().addAll(result.tools());
687-
return allToolsResult;
688-
});
681+
return this.listTools(McpSchema.FIRST_PAGE)
682+
.expand(result -> (result.nextCursor() != null) ? this.listTools(result.nextCursor()) : Mono.empty())
683+
.reduce(new McpSchema.ListToolsResult(new ArrayList<>(), null), (allToolsResult, result) -> {
684+
allToolsResult.tools().addAll(result.tools());
685+
return allToolsResult;
686+
})
687+
.map(result -> new McpSchema.ListToolsResult(Collections.unmodifiableList(result.tools()), null));
689688
}
690689

691690
/**
@@ -739,15 +738,13 @@ private NotificationHandler asyncToolsChangeNotificationHandler(
739738
* @see #readResource(McpSchema.Resource)
740739
*/
741740
public Mono<McpSchema.ListResourcesResult> listResources() {
742-
return this.listResources(McpSchema.FIRST_PAGE).expand(result -> {
743-
if (result.nextCursor() != null) {
744-
return this.listResources(result.nextCursor());
745-
}
746-
return Mono.empty();
747-
}).reduce(new McpSchema.ListResourcesResult(new ArrayList<>(), null), (allResourcesResult, result) -> {
748-
allResourcesResult.resources().addAll(result.resources());
749-
return allResourcesResult;
750-
});
741+
return this.listResources(McpSchema.FIRST_PAGE)
742+
.expand(result -> (result.nextCursor() != null) ? this.listResources(result.nextCursor()) : Mono.empty())
743+
.reduce(new McpSchema.ListResourcesResult(new ArrayList<>(), null), (allResourcesResult, result) -> {
744+
allResourcesResult.resources().addAll(result.resources());
745+
return allResourcesResult;
746+
})
747+
.map(result -> new McpSchema.ListResourcesResult(Collections.unmodifiableList(result.resources()), null));
751748
}
752749

753750
/**
@@ -809,17 +806,16 @@ public Mono<McpSchema.ReadResourceResult> readResource(McpSchema.ReadResourceReq
809806
* @see McpSchema.ListResourceTemplatesResult
810807
*/
811808
public Mono<McpSchema.ListResourceTemplatesResult> listResourceTemplates() {
812-
return this.listResourceTemplates(McpSchema.FIRST_PAGE).expand(result -> {
813-
if (result.nextCursor() != null) {
814-
return this.listResourceTemplates(result.nextCursor());
815-
}
816-
return Mono.empty();
817-
})
809+
return this.listResourceTemplates(McpSchema.FIRST_PAGE)
810+
.expand(result -> (result.nextCursor() != null) ? this.listResourceTemplates(result.nextCursor())
811+
: Mono.empty())
818812
.reduce(new McpSchema.ListResourceTemplatesResult(new ArrayList<>(), null),
819813
(allResourceTemplatesResult, result) -> {
820814
allResourceTemplatesResult.resourceTemplates().addAll(result.resourceTemplates());
821815
return allResourceTemplatesResult;
822-
});
816+
})
817+
.map(result -> new McpSchema.ListResourceTemplatesResult(
818+
Collections.unmodifiableList(result.resourceTemplates()), null));
823819
}
824820

825821
/**
@@ -914,15 +910,13 @@ private NotificationHandler asyncResourcesUpdatedNotificationHandler(
914910
* @see #getPrompt(GetPromptRequest)
915911
*/
916912
public Mono<ListPromptsResult> listPrompts() {
917-
return this.listPrompts(McpSchema.FIRST_PAGE).expand(result -> {
918-
if (result.nextCursor() != null) {
919-
return this.listPrompts(result.nextCursor());
920-
}
921-
return Mono.empty();
922-
}).reduce(new ListPromptsResult(new ArrayList<>(), null), (allPromptsResult, result) -> {
923-
allPromptsResult.prompts().addAll(result.prompts());
924-
return allPromptsResult;
925-
});
913+
return this.listPrompts(McpSchema.FIRST_PAGE)
914+
.expand(result -> (result.nextCursor() != null) ? this.listPrompts(result.nextCursor()) : Mono.empty())
915+
.reduce(new ListPromptsResult(new ArrayList<>(), null), (allPromptsResult, result) -> {
916+
allPromptsResult.prompts().addAll(result.prompts());
917+
return allPromptsResult;
918+
})
919+
.map(result -> new McpSchema.ListPromptsResult(Collections.unmodifiableList(result.prompts()), null));
926920
}
927921

928922
/**

‎mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,20 @@ void testListAllTools() {
183183
});
184184
}
185185

186+
@Test
187+
void testListAllToolsReturnsImmutableList() {
188+
withClient(createMcpTransport(), mcpAsyncClient -> {
189+
StepVerifier.create(mcpAsyncClient.initialize().then(mcpAsyncClient.listTools()))
190+
.consumeNextWith(result -> {
191+
assertThat(result.tools()).isNotNull();
192+
// Verify that the returned list is immutable
193+
assertThatThrownBy(() -> result.tools().add(new Tool("test", "test", "{\"type\":\"object\"}")))
194+
.isInstanceOf(UnsupportedOperationException.class);
195+
})
196+
.verifyComplete();
197+
});
198+
}
199+
186200
@Test
187201
void testPingWithoutInitialization() {
188202
verifyCallSucceedsWithImplicitInitialization(client -> client.ping(), "pinging the server");
@@ -334,6 +348,21 @@ void testListAllResources() {
334348
});
335349
}
336350

351+
@Test
352+
void testListAllResourcesReturnsImmutableList() {
353+
withClient(createMcpTransport(), mcpAsyncClient -> {
354+
StepVerifier.create(mcpAsyncClient.initialize().then(mcpAsyncClient.listResources()))
355+
.consumeNextWith(result -> {
356+
assertThat(result.resources()).isNotNull();
357+
// Verify that the returned list is immutable
358+
assertThatThrownBy(
359+
() -> result.resources().add(Resource.builder().uri("test://uri").name("test").build()))
360+
.isInstanceOf(UnsupportedOperationException.class);
361+
})
362+
.verifyComplete();
363+
});
364+
}
365+
337366
@Test
338367
void testMcpAsyncClientState() {
339368
withClient(createMcpTransport(), mcpAsyncClient -> {
@@ -385,6 +414,20 @@ void testListAllPrompts() {
385414
});
386415
}
387416

417+
@Test
418+
void testListAllPromptsReturnsImmutableList() {
419+
withClient(createMcpTransport(), mcpAsyncClient -> {
420+
StepVerifier.create(mcpAsyncClient.initialize().then(mcpAsyncClient.listPrompts()))
421+
.consumeNextWith(result -> {
422+
assertThat(result.prompts()).isNotNull();
423+
// Verify that the returned list is immutable
424+
assertThatThrownBy(() -> result.prompts().add(new Prompt("test", "test", null)))
425+
.isInstanceOf(UnsupportedOperationException.class);
426+
})
427+
.verifyComplete();
428+
});
429+
}
430+
388431
@Test
389432
void testGetPromptWithoutInitialization() {
390433
GetPromptRequest request = new GetPromptRequest("simple_prompt", Map.of());
@@ -554,6 +597,21 @@ void testListAllResourceTemplates() {
554597
});
555598
}
556599

600+
@Test
601+
void testListAllResourceTemplatesReturnsImmutableList() {
602+
withClient(createMcpTransport(), mcpAsyncClient -> {
603+
StepVerifier.create(mcpAsyncClient.initialize().then(mcpAsyncClient.listResourceTemplates()))
604+
.consumeNextWith(result -> {
605+
assertThat(result.resourceTemplates()).isNotNull();
606+
// Verify that the returned list is immutable
607+
assertThatThrownBy(() -> result.resourceTemplates()
608+
.add(new McpSchema.ResourceTemplate("test://template", "test", null, null, null)))
609+
.isInstanceOf(UnsupportedOperationException.class);
610+
})
611+
.verifyComplete();
612+
});
613+
}
614+
557615
// @Test
558616
void testResourceSubscription() {
559617
withClient(createMcpTransport(), mcpAsyncClient -> {

0 commit comments

Comments
 (0)