Skip to content

refactor: return immutable lists from list methods #345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 27, 2025
+150 −42
Merged

refactor: return immutable lists from list methods #345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
053f0f5
refactor: return immutable lists from list methods
tzolov Jun 27, 2025
ebc6d9e
better timeout handling
tzolov Jun 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions .../mcp-spring-webflux/src/test/java/io/modelcontextprotocol/WebFluxSseIntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,12 +432,6 @@ void testCreateElicitationWithRequestTimeoutSuccess(String clientType) {
Function<ElicitRequest, ElicitResult> 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()));
};

Expand Down Expand Up @@ -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<ElicitRequest, ElicitResult> 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);
Expand Down Expand Up @@ -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();

Expand Down
58 changes: 58 additions & 0 deletions mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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 -> {
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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 -> {
Expand Down
62 changes: 28 additions & 34 deletions mcp/src/main/java/io/modelcontextprotocol/client/McpAsyncClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -674,15 +675,13 @@ public Mono<McpSchema.CallToolResult> callTool(McpSchema.CallToolRequest callToo
* @return A Mono that emits the list of all tools result
*/
public Mono<McpSchema.ListToolsResult> 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));
}

/**
Expand Down Expand Up @@ -736,15 +735,13 @@ private NotificationHandler asyncToolsChangeNotificationHandler(
* @see #readResource(McpSchema.Resource)
*/
public Mono<McpSchema.ListResourcesResult> 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));
}

/**
Expand Down Expand Up @@ -806,17 +803,16 @@ public Mono<McpSchema.ReadResourceResult> readResource(McpSchema.ReadResourceReq
* @see McpSchema.ListResourceTemplatesResult
*/
public Mono<McpSchema.ListResourceTemplatesResult> 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));
}

/**
Expand Down Expand Up @@ -911,15 +907,13 @@ private NotificationHandler asyncResourcesUpdatedNotificationHandler(
* @see #getPrompt(GetPromptRequest)
*/
public Mono<ListPromptsResult> 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));
}

/**
Expand Down
58 changes: 58 additions & 0 deletions mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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 -> {
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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 -> {
Expand Down