refactor: deprecate tool spec's arguments-only handler in favor of CallToolRequest (#375)
Tool handlers now receive CallToolRequest instead of raw arguments to enable access to additional metadata parameters like progressToken - Deprecate handlers that take only Map<String, Object> arguments - Introduce new handlers that receive the full CallToolRequest object - Maintain backward compatibility with deprecated call handlers - Enhance API to provide access to complete tool request context beyond just arguments - Add builder pattern for AsyncToolSpecification and SyncToolSpecification - Add duplicate tool name validation during server building and runtime registration - Update all test files to use new callTool handlers and builder pattern - Add test coverage for new builder functionality and CallToolRequest handling Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
This commit is contained in:
Christian Tzolov
parent
95df67ec05
commit
901175e14f
2 changed files with 1687 additions and 692 deletions
|
|
@ -34,7 +34,6 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
|||
*
|
||||
* @author Christian Tzolov
|
||||
*/
|
||||
// KEEP IN SYNC with the class in mcp-test module
|
||||
public abstract class AbstractMcpAsyncServerTests {
|
||||
|
||||
private static final String TEST_TOOL_NAME = "test-tool";
|
||||
|
|
@ -102,6 +101,7 @@ public abstract class AbstractMcpAsyncServerTests {
|
|||
""";
|
||||
|
||||
@Test
|
||||
@Deprecated
|
||||
void testAddTool() {
|
||||
Tool newTool = new McpSchema.Tool("new-tool", "New test tool", emptyJsonSchema);
|
||||
var mcpAsyncServer = McpServer.async(createMcpTransportProvider())
|
||||
|
|
@ -117,6 +117,23 @@ public abstract class AbstractMcpAsyncServerTests {
|
|||
}
|
||||
|
||||
@Test
|
||||
void testAddToolCall() {
|
||||
Tool newTool = new McpSchema.Tool("new-tool", "New test tool", emptyJsonSchema);
|
||||
var mcpAsyncServer = McpServer.async(createMcpTransportProvider())
|
||||
.serverInfo("test-server", "1.0.0")
|
||||
.capabilities(ServerCapabilities.builder().tools(true).build())
|
||||
.build();
|
||||
|
||||
StepVerifier.create(mcpAsyncServer.addTool(McpServerFeatures.AsyncToolSpecification.builder()
|
||||
.tool(newTool)
|
||||
.callHandler((exchange, request) -> Mono.just(new CallToolResult(List.of(), false)))
|
||||
.build())).verifyComplete();
|
||||
|
||||
assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException();
|
||||
}
|
||||
|
||||
@Test
|
||||
@Deprecated
|
||||
void testAddDuplicateTool() {
|
||||
Tool duplicateTool = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema);
|
||||
|
||||
|
|
@ -137,6 +154,83 @@ public abstract class AbstractMcpAsyncServerTests {
|
|||
assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException();
|
||||
}
|
||||
|
||||
@Test
|
||||
void testAddDuplicateToolCall() {
|
||||
Tool duplicateTool = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema);
|
||||
|
||||
var mcpAsyncServer = McpServer.async(createMcpTransportProvider())
|
||||
.serverInfo("test-server", "1.0.0")
|
||||
.capabilities(ServerCapabilities.builder().tools(true).build())
|
||||
.toolCall(duplicateTool, (exchange, request) -> Mono.just(new CallToolResult(List.of(), false)))
|
||||
.build();
|
||||
|
||||
StepVerifier.create(mcpAsyncServer.addTool(McpServerFeatures.AsyncToolSpecification.builder()
|
||||
.tool(duplicateTool)
|
||||
.callHandler((exchange, request) -> Mono.just(new CallToolResult(List.of(), false)))
|
||||
.build())).verifyErrorSatisfies(error -> {
|
||||
assertThat(error).isInstanceOf(McpError.class)
|
||||
.hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists");
|
||||
});
|
||||
|
||||
assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException();
|
||||
}
|
||||
|
||||
@Test
|
||||
void testDuplicateToolCallDuringBuilding() {
|
||||
Tool duplicateTool = new Tool("duplicate-build-toolcall", "Duplicate toolcall during building",
|
||||
emptyJsonSchema);
|
||||
|
||||
assertThatThrownBy(() -> McpServer.async(createMcpTransportProvider())
|
||||
.serverInfo("test-server", "1.0.0")
|
||||
.capabilities(ServerCapabilities.builder().tools(true).build())
|
||||
.toolCall(duplicateTool, (exchange, request) -> Mono.just(new CallToolResult(List.of(), false)))
|
||||
.toolCall(duplicateTool, (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) // Duplicate!
|
||||
.build()).isInstanceOf(IllegalArgumentException.class)
|
||||
.hasMessage("Tool with name 'duplicate-build-toolcall' is already registered.");
|
||||
}
|
||||
|
||||
@Test
|
||||
void testDuplicateToolsInBatchListRegistration() {
|
||||
Tool duplicateTool = new Tool("batch-list-tool", "Duplicate tool in batch list", emptyJsonSchema);
|
||||
List<McpServerFeatures.AsyncToolSpecification> specs = List.of(
|
||||
McpServerFeatures.AsyncToolSpecification.builder()
|
||||
.tool(duplicateTool)
|
||||
.callHandler((exchange, request) -> Mono.just(new CallToolResult(List.of(), false)))
|
||||
.build(),
|
||||
McpServerFeatures.AsyncToolSpecification.builder()
|
||||
.tool(duplicateTool)
|
||||
.callHandler((exchange, request) -> Mono.just(new CallToolResult(List.of(), false)))
|
||||
.build() // Duplicate!
|
||||
);
|
||||
|
||||
assertThatThrownBy(() -> McpServer.async(createMcpTransportProvider())
|
||||
.serverInfo("test-server", "1.0.0")
|
||||
.capabilities(ServerCapabilities.builder().tools(true).build())
|
||||
.tools(specs)
|
||||
.build()).isInstanceOf(IllegalArgumentException.class)
|
||||
.hasMessage("Tool with name 'batch-list-tool' is already registered.");
|
||||
}
|
||||
|
||||
@Test
|
||||
void testDuplicateToolsInBatchVarargsRegistration() {
|
||||
Tool duplicateTool = new Tool("batch-varargs-tool", "Duplicate tool in batch varargs", emptyJsonSchema);
|
||||
|
||||
assertThatThrownBy(() -> McpServer.async(createMcpTransportProvider())
|
||||
.serverInfo("test-server", "1.0.0")
|
||||
.capabilities(ServerCapabilities.builder().tools(true).build())
|
||||
.tools(McpServerFeatures.AsyncToolSpecification.builder()
|
||||
.tool(duplicateTool)
|
||||
.callHandler((exchange, request) -> Mono.just(new CallToolResult(List.of(), false)))
|
||||
.build(),
|
||||
McpServerFeatures.AsyncToolSpecification.builder()
|
||||
.tool(duplicateTool)
|
||||
.callHandler((exchange, request) -> Mono.just(new CallToolResult(List.of(), false)))
|
||||
.build() // Duplicate!
|
||||
)
|
||||
.build()).isInstanceOf(IllegalArgumentException.class)
|
||||
.hasMessage("Tool with name 'batch-varargs-tool' is already registered.");
|
||||
}
|
||||
|
||||
@Test
|
||||
void testRemoveTool() {
|
||||
Tool too = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema);
|
||||
|
|
@ -144,7 +238,7 @@ public abstract class AbstractMcpAsyncServerTests {
|
|||
var mcpAsyncServer = McpServer.async(createMcpTransportProvider())
|
||||
.serverInfo("test-server", "1.0.0")
|
||||
.capabilities(ServerCapabilities.builder().tools(true).build())
|
||||
.tool(too, (exchange, args) -> Mono.just(new CallToolResult(List.of(), false)))
|
||||
.toolCall(too, (exchange, request) -> Mono.just(new CallToolResult(List.of(), false)))
|
||||
.build();
|
||||
|
||||
StepVerifier.create(mcpAsyncServer.removeTool(TEST_TOOL_NAME)).verifyComplete();
|
||||
|
|
@ -173,7 +267,7 @@ public abstract class AbstractMcpAsyncServerTests {
|
|||
var mcpAsyncServer = McpServer.async(createMcpTransportProvider())
|
||||
.serverInfo("test-server", "1.0.0")
|
||||
.capabilities(ServerCapabilities.builder().tools(true).build())
|
||||
.tool(too, (exchange, args) -> Mono.just(new CallToolResult(List.of(), false)))
|
||||
.toolCall(too, (exchange, args) -> Mono.just(new CallToolResult(List.of(), false)))
|
||||
.build();
|
||||
|
||||
StepVerifier.create(mcpAsyncServer.notifyToolsListChanged()).verifyComplete();
|
||||
|
|
|
|||
|
|
@ -4,8 +4,16 @@
|
|||
|
||||
package io.modelcontextprotocol.server;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThatCode;
|
||||
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
||||
|
||||
import java.util.List;
|
||||
|
||||
import org.junit.jupiter.api.AfterEach;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import io.modelcontextprotocol.spec.McpError;
|
||||
import io.modelcontextprotocol.spec.McpSchema;
|
||||
import io.modelcontextprotocol.spec.McpSchema.CallToolResult;
|
||||
|
|
@ -17,21 +25,13 @@ import io.modelcontextprotocol.spec.McpSchema.Resource;
|
|||
import io.modelcontextprotocol.spec.McpSchema.ServerCapabilities;
|
||||
import io.modelcontextprotocol.spec.McpSchema.Tool;
|
||||
import io.modelcontextprotocol.spec.McpServerTransportProvider;
|
||||
import org.junit.jupiter.api.AfterEach;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThatCode;
|
||||
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
||||
|
||||
/**
|
||||
* Test suite for the {@link McpSyncServer} that can be used with different
|
||||
* {@link io.modelcontextprotocol.spec.McpServerTransportProvider} implementations.
|
||||
* {@link McpTransportProvider} implementations.
|
||||
*
|
||||
* @author Christian Tzolov
|
||||
*/
|
||||
// KEEP IN SYNC with the class in mcp-test module
|
||||
public abstract class AbstractMcpSyncServerTests {
|
||||
|
||||
private static final String TEST_TOOL_NAME = "test-tool";
|
||||
|
|
@ -109,6 +109,7 @@ public abstract class AbstractMcpSyncServerTests {
|
|||
""";
|
||||
|
||||
@Test
|
||||
@Deprecated
|
||||
void testAddTool() {
|
||||
var mcpSyncServer = McpServer.sync(createMcpTransportProvider())
|
||||
.serverInfo("test-server", "1.0.0")
|
||||
|
|
@ -124,6 +125,23 @@ public abstract class AbstractMcpSyncServerTests {
|
|||
}
|
||||
|
||||
@Test
|
||||
void testAddToolCall() {
|
||||
var mcpSyncServer = McpServer.sync(createMcpTransportProvider())
|
||||
.serverInfo("test-server", "1.0.0")
|
||||
.capabilities(ServerCapabilities.builder().tools(true).build())
|
||||
.build();
|
||||
|
||||
Tool newTool = new McpSchema.Tool("new-tool", "New test tool", emptyJsonSchema);
|
||||
assertThatCode(() -> mcpSyncServer.addTool(McpServerFeatures.SyncToolSpecification.builder()
|
||||
.tool(newTool)
|
||||
.callHandler((exchange, request) -> new CallToolResult(List.of(), false))
|
||||
.build())).doesNotThrowAnyException();
|
||||
|
||||
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
|
||||
}
|
||||
|
||||
@Test
|
||||
@Deprecated
|
||||
void testAddDuplicateTool() {
|
||||
Tool duplicateTool = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema);
|
||||
|
||||
|
|
@ -141,6 +159,81 @@ public abstract class AbstractMcpSyncServerTests {
|
|||
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
|
||||
}
|
||||
|
||||
@Test
|
||||
void testAddDuplicateToolCall() {
|
||||
Tool duplicateTool = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema);
|
||||
|
||||
var mcpSyncServer = McpServer.sync(createMcpTransportProvider())
|
||||
.serverInfo("test-server", "1.0.0")
|
||||
.capabilities(ServerCapabilities.builder().tools(true).build())
|
||||
.toolCall(duplicateTool, (exchange, request) -> new CallToolResult(List.of(), false))
|
||||
.build();
|
||||
|
||||
assertThatThrownBy(() -> mcpSyncServer.addTool(McpServerFeatures.SyncToolSpecification.builder()
|
||||
.tool(duplicateTool)
|
||||
.callHandler((exchange, request) -> new CallToolResult(List.of(), false))
|
||||
.build())).isInstanceOf(McpError.class)
|
||||
.hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists");
|
||||
|
||||
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
|
||||
}
|
||||
|
||||
@Test
|
||||
void testDuplicateToolCallDuringBuilding() {
|
||||
Tool duplicateTool = new Tool("duplicate-build-toolcall", "Duplicate toolcall during building",
|
||||
emptyJsonSchema);
|
||||
|
||||
assertThatThrownBy(() -> McpServer.sync(createMcpTransportProvider())
|
||||
.serverInfo("test-server", "1.0.0")
|
||||
.capabilities(ServerCapabilities.builder().tools(true).build())
|
||||
.toolCall(duplicateTool, (exchange, request) -> new CallToolResult(List.of(), false))
|
||||
.toolCall(duplicateTool, (exchange, request) -> new CallToolResult(List.of(), false)) // Duplicate!
|
||||
.build()).isInstanceOf(IllegalArgumentException.class)
|
||||
.hasMessage("Tool with name 'duplicate-build-toolcall' is already registered.");
|
||||
}
|
||||
|
||||
@Test
|
||||
void testDuplicateToolsInBatchListRegistration() {
|
||||
Tool duplicateTool = new Tool("batch-list-tool", "Duplicate tool in batch list", emptyJsonSchema);
|
||||
List<McpServerFeatures.SyncToolSpecification> specs = List.of(
|
||||
McpServerFeatures.SyncToolSpecification.builder()
|
||||
.tool(duplicateTool)
|
||||
.callHandler((exchange, request) -> new CallToolResult(List.of(), false))
|
||||
.build(),
|
||||
McpServerFeatures.SyncToolSpecification.builder()
|
||||
.tool(duplicateTool)
|
||||
.callHandler((exchange, request) -> new CallToolResult(List.of(), false))
|
||||
.build() // Duplicate!
|
||||
);
|
||||
|
||||
assertThatThrownBy(() -> McpServer.sync(createMcpTransportProvider())
|
||||
.serverInfo("test-server", "1.0.0")
|
||||
.capabilities(ServerCapabilities.builder().tools(true).build())
|
||||
.tools(specs)
|
||||
.build()).isInstanceOf(IllegalArgumentException.class)
|
||||
.hasMessage("Tool with name 'batch-list-tool' is already registered.");
|
||||
}
|
||||
|
||||
@Test
|
||||
void testDuplicateToolsInBatchVarargsRegistration() {
|
||||
Tool duplicateTool = new Tool("batch-varargs-tool", "Duplicate tool in batch varargs", emptyJsonSchema);
|
||||
|
||||
assertThatThrownBy(() -> McpServer.sync(createMcpTransportProvider())
|
||||
.serverInfo("test-server", "1.0.0")
|
||||
.capabilities(ServerCapabilities.builder().tools(true).build())
|
||||
.tools(McpServerFeatures.SyncToolSpecification.builder()
|
||||
.tool(duplicateTool)
|
||||
.callHandler((exchange, request) -> new CallToolResult(List.of(), false))
|
||||
.build(),
|
||||
McpServerFeatures.SyncToolSpecification.builder()
|
||||
.tool(duplicateTool)
|
||||
.callHandler((exchange, request) -> new CallToolResult(List.of(), false))
|
||||
.build() // Duplicate!
|
||||
)
|
||||
.build()).isInstanceOf(IllegalArgumentException.class)
|
||||
.hasMessage("Tool with name 'batch-varargs-tool' is already registered.");
|
||||
}
|
||||
|
||||
@Test
|
||||
void testRemoveTool() {
|
||||
Tool tool = new McpSchema.Tool(TEST_TOOL_NAME, "Test tool", emptyJsonSchema);
|
||||
|
|
@ -148,7 +241,7 @@ public abstract class AbstractMcpSyncServerTests {
|
|||
var mcpSyncServer = McpServer.sync(createMcpTransportProvider())
|
||||
.serverInfo("test-server", "1.0.0")
|
||||
.capabilities(ServerCapabilities.builder().tools(true).build())
|
||||
.tool(tool, (exchange, args) -> new CallToolResult(List.of(), false))
|
||||
.toolCall(tool, (exchange, args) -> new CallToolResult(List.of(), false))
|
||||
.build();
|
||||
|
||||
assertThatCode(() -> mcpSyncServer.removeTool(TEST_TOOL_NAME)).doesNotThrowAnyException();
|
||||
|
|
|
|||
Loading…
Reference in a new issue