Skip to content

Commit efa1b33

Browse files
committed
Simplify tool specification sanitization logic and improve test assertions for tool order and properties.
1 parent b0121f1 commit efa1b33

5 files changed

Lines changed: 23 additions & 24 deletions

File tree

mcp-core/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -409,11 +409,9 @@ public Mono<Void> addTools(List<McpServerFeatures.AsyncToolSpecification> toolSp
409409

410410
private List<McpServerFeatures.AsyncToolSpecification> sanitizeToolSpecifications(
411411
List<McpServerFeatures.AsyncToolSpecification> toolSpecifications) {
412-
List<McpServerFeatures.AsyncToolSpecification> copiedToolSpecifications = new ArrayList<>(toolSpecifications);
413412
LinkedHashMap<String, McpServerFeatures.AsyncToolSpecification> toolSpecificationsByName = new LinkedHashMap<>();
414413

415-
for (int i = copiedToolSpecifications.size() - 1; i >= 0; i--) {
416-
var toolSpecification = copiedToolSpecifications.get(i);
414+
for (var toolSpecification : toolSpecifications) {
417415
if (toolSpecification == null) {
418416
throw new IllegalArgumentException("Tool specification must not be null");
419417
}
@@ -424,13 +422,10 @@ private List<McpServerFeatures.AsyncToolSpecification> sanitizeToolSpecification
424422
throw new IllegalArgumentException("Tool call handler must not be null");
425423
}
426424
var wrappedToolSpecification = withStructuredOutputHandling(this.jsonSchemaValidator, toolSpecification);
427-
toolSpecificationsByName.putIfAbsent(wrappedToolSpecification.tool().name(), wrappedToolSpecification);
425+
toolSpecificationsByName.put(wrappedToolSpecification.tool().name(), wrappedToolSpecification);
428426
}
429427

430-
List<McpServerFeatures.AsyncToolSpecification> sanitizedToolSpecifications = new ArrayList<>(
431-
toolSpecificationsByName.values());
432-
Collections.reverse(sanitizedToolSpecifications);
433-
return sanitizedToolSpecifications;
428+
return new ArrayList<>(toolSpecificationsByName.values());
434429
}
435430

436431
private static class StructuredOutputCallToolHandler

mcp-core/src/main/java/io/modelcontextprotocol/server/McpStatelessAsyncServer.java

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030

3131
import java.time.Duration;
3232
import java.util.ArrayList;
33-
import java.util.Collections;
3433
import java.util.HashMap;
3534
import java.util.HashSet;
3635
import java.util.LinkedHashMap;
@@ -396,12 +395,9 @@ public Mono<Void> addTools(List<McpStatelessServerFeatures.AsyncToolSpecificatio
396395

397396
private List<McpStatelessServerFeatures.AsyncToolSpecification> sanitizeToolSpecifications(
398397
List<McpStatelessServerFeatures.AsyncToolSpecification> toolSpecifications) {
399-
List<McpStatelessServerFeatures.AsyncToolSpecification> copiedToolSpecifications = new ArrayList<>(
400-
toolSpecifications);
401398
LinkedHashMap<String, McpStatelessServerFeatures.AsyncToolSpecification> toolSpecificationsByName = new LinkedHashMap<>();
402399

403-
for (int i = copiedToolSpecifications.size() - 1; i >= 0; i--) {
404-
var toolSpecification = copiedToolSpecifications.get(i);
400+
for (var toolSpecification : toolSpecifications) {
405401
if (toolSpecification == null) {
406402
throw new IllegalArgumentException("Tool specification must not be null");
407403
}
@@ -412,13 +408,10 @@ private List<McpStatelessServerFeatures.AsyncToolSpecification> sanitizeToolSpec
412408
throw new IllegalArgumentException("Tool call handler must not be null");
413409
}
414410
var wrappedToolSpecification = withStructuredOutputHandling(this.jsonSchemaValidator, toolSpecification);
415-
toolSpecificationsByName.putIfAbsent(wrappedToolSpecification.tool().name(), wrappedToolSpecification);
411+
toolSpecificationsByName.put(wrappedToolSpecification.tool().name(), wrappedToolSpecification);
416412
}
417413

418-
List<McpStatelessServerFeatures.AsyncToolSpecification> sanitizedToolSpecifications = new ArrayList<>(
419-
toolSpecificationsByName.values());
420-
Collections.reverse(sanitizedToolSpecifications);
421-
return sanitizedToolSpecifications;
414+
return new ArrayList<>(toolSpecificationsByName.values());
422415
}
423416

424417
/**

mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,8 +275,12 @@ void testAddToolsWithDuplicateInputKeepsLastOccurrence() {
275275
.verifyComplete();
276276

277277
StepVerifier.create(mcpAsyncServer.listTools().collectList()).assertNext(tools -> {
278-
assertThat(tools).extracting(McpSchema.Tool::name).containsExactly("middle-tool", "duplicate-tool");
279-
assertThat(tools.get(1).title()).isEqualTo("Last tool");
278+
assertThat(tools).extracting(McpSchema.Tool::name)
279+
.containsExactlyInAnyOrder("middle-tool", "duplicate-tool");
280+
assertThat(tools).filteredOn(tool -> tool.name().equals("duplicate-tool"))
281+
.singleElement()
282+
.extracting(McpSchema.Tool::title)
283+
.isEqualTo("Last tool");
280284
}).verifyComplete();
281285

282286
assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException();

mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,8 +264,11 @@ void testAddToolsWithDuplicateInputKeepsLastOccurrence() {
264264
.doesNotThrowAnyException();
265265

266266
assertThat(mcpSyncServer.listTools()).extracting(McpSchema.Tool::name)
267-
.containsExactly("middle-tool", "duplicate-tool");
268-
assertThat(mcpSyncServer.listTools().get(1).title()).isEqualTo("Last tool");
267+
.containsExactlyInAnyOrder("middle-tool", "duplicate-tool");
268+
assertThat(mcpSyncServer.listTools()).filteredOn(tool -> tool.name().equals("duplicate-tool"))
269+
.singleElement()
270+
.extracting(McpSchema.Tool::title)
271+
.isEqualTo("Last tool");
269272

270273
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
271274
}

mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,12 @@ void testStatelessSyncBulkToolMutations(String clientType) {
129129

130130
await().atMost(Duration.ofSeconds(5)).untilAsserted(() -> {
131131
List<Tool> tools = mcpClient.listTools().tools();
132-
assertThat(tools).extracting(McpSchema.Tool::name).containsExactly("middle-tool", "duplicate-tool");
133-
assertThat(tools.get(1).title()).isEqualTo("Last tool");
132+
assertThat(tools).extracting(McpSchema.Tool::name)
133+
.containsExactlyInAnyOrder("middle-tool", "duplicate-tool");
134+
assertThat(tools).filteredOn(tool -> tool.name().equals("duplicate-tool"))
135+
.singleElement()
136+
.extracting(McpSchema.Tool::title)
137+
.isEqualTo("Last tool");
134138
});
135139

136140
mcpServer.addTools(List.of(syncToolSpecification("middle-tool", "Replacement tool"),

0 commit comments

Comments
 (0)