Skip to content

Commit 053f0f5

Browse files
tzolovtzolov
committed
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 <christian.tzolov@broadcom.com>
1 parent 1cbace3 commit 053f0f5

File tree

3 files changed

+144
-34
lines changed

3 files changed

+144
-34
lines changed

‎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;
@@ -674,15 +675,13 @@ public Mono<McpSchema.CallToolResult> callTool(McpSchema.CallToolRequest callToo
674675
* @return A Mono that emits the list of all tools result
675676
*/
676677
public Mono<McpSchema.ListToolsResult> listTools() {
677-
return this.listTools(McpSchema.FIRST_PAGE).expand(result -> {
678-
if (result.nextCursor() != null) {
679-
return this.listTools(result.nextCursor());
680-
}
681-
return Mono.empty();
682-
}).reduce(new McpSchema.ListToolsResult(new ArrayList<>(), null), (allToolsResult, result) -> {
683-
allToolsResult.tools().addAll(result.tools());
684-
return allToolsResult;
685-
});
678+
return this.listTools(McpSchema.FIRST_PAGE)
679+
.expand(result -> (result.nextCursor() != null) ? this.listTools(result.nextCursor()) : Mono.empty())
680+
.reduce(new McpSchema.ListToolsResult(new ArrayList<>(), null), (allToolsResult, result) -> {
681+
allToolsResult.tools().addAll(result.tools());
682+
return allToolsResult;
683+
})
684+
.map(result -> new McpSchema.ListToolsResult(Collections.unmodifiableList(result.tools()), null));
686685
}
687686

688687
/**
@@ -736,15 +735,13 @@ private NotificationHandler asyncToolsChangeNotificationHandler(
736735
* @see #readResource(McpSchema.Resource)
737736
*/
738737
public Mono<McpSchema.ListResourcesResult> listResources() {
739-
return this.listResources(McpSchema.FIRST_PAGE).expand(result -> {
740-
if (result.nextCursor() != null) {
741-
return this.listResources(result.nextCursor());
742-
}
743-
return Mono.empty();
744-
}).reduce(new McpSchema.ListResourcesResult(new ArrayList<>(), null), (allResourcesResult, result) -> {
745-
allResourcesResult.resources().addAll(result.resources());
746-
return allResourcesResult;
747-
});
738+
return this.listResources(McpSchema.FIRST_PAGE)
739+
.expand(result -> (result.nextCursor() != null) ? this.listResources(result.nextCursor()) : Mono.empty())
740+
.reduce(new McpSchema.ListResourcesResult(new ArrayList<>(), null), (allResourcesResult, result) -> {
741+
allResourcesResult.resources().addAll(result.resources());
742+
return allResourcesResult;
743+
})
744+
.map(result -> new McpSchema.ListResourcesResult(Collections.unmodifiableList(result.resources()), null));
748745
}
749746

750747
/**
@@ -806,17 +803,16 @@ public Mono<McpSchema.ReadResourceResult> readResource(McpSchema.ReadResourceReq
806803
* @see McpSchema.ListResourceTemplatesResult
807804
*/
808805
public Mono<McpSchema.ListResourceTemplatesResult> listResourceTemplates() {
809-
return this.listResourceTemplates(McpSchema.FIRST_PAGE).expand(result -> {
810-
if (result.nextCursor() != null) {
811-
return this.listResourceTemplates(result.nextCursor());
812-
}
813-
return Mono.empty();
814-
})
806+
return this.listResourceTemplates(McpSchema.FIRST_PAGE)
807+
.expand(result -> (result.nextCursor() != null) ? this.listResourceTemplates(result.nextCursor())
808+
: Mono.empty())
815809
.reduce(new McpSchema.ListResourceTemplatesResult(new ArrayList<>(), null),
816810
(allResourceTemplatesResult, result) -> {
817811
allResourceTemplatesResult.resourceTemplates().addAll(result.resourceTemplates());
818812
return allResourceTemplatesResult;
819-
});
813+
})
814+
.map(result -> new McpSchema.ListResourceTemplatesResult(
815+
Collections.unmodifiableList(result.resourceTemplates()), null));
820816
}
821817

822818
/**
@@ -911,15 +907,13 @@ private NotificationHandler asyncResourcesUpdatedNotificationHandler(
911907
* @see #getPrompt(GetPromptRequest)
912908
*/
913909
public Mono<ListPromptsResult> listPrompts() {
914-
return this.listPrompts(McpSchema.FIRST_PAGE).expand(result -> {
915-
if (result.nextCursor() != null) {
916-
return this.listPrompts(result.nextCursor());
917-
}
918-
return Mono.empty();
919-
}).reduce(new ListPromptsResult(new ArrayList<>(), null), (allPromptsResult, result) -> {
920-
allPromptsResult.prompts().addAll(result.prompts());
921-
return allPromptsResult;
922-
});
910+
return this.listPrompts(McpSchema.FIRST_PAGE)
911+
.expand(result -> (result.nextCursor() != null) ? this.listPrompts(result.nextCursor()) : Mono.empty())
912+
.reduce(new ListPromptsResult(new ArrayList<>(), null), (allPromptsResult, result) -> {
913+
allPromptsResult.prompts().addAll(result.prompts());
914+
return allPromptsResult;
915+
})
916+
.map(result -> new McpSchema.ListPromptsResult(Collections.unmodifiableList(result.prompts()), null));
923917
}
924918

925919
/**

‎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)