feat(mcp): Refactor MCP server API to use Specification pattern#2508
feat(mcp): Refactor MCP server API to use Specification pattern#2508tzolov wants to merge 7 commits intospring-projects:mainfrom
Conversation
|
Depends on modelcontextprotocol/java-sdk#31 |
chemicL
left a comment
There was a problem hiding this comment.
Thanks for all the hard work in adapting the changes :) I left a few cosmetic remarks, but overall it will be awesome!
There was a problem hiding this comment.
typo: specificaitons -> specifications
There was a problem hiding this comment.
specificaitons -> specifications
There was a problem hiding this comment.
another typo.. and more in this file
There was a problem hiding this comment.
Another typo for specification - perhaps worth running a spell checker on all the changes in this PR.
There was a problem hiding this comment.
Is there any reason for not using a lambda expression here?
There was a problem hiding this comment.
I can do it but it requires explicit casting that I don't like. Anyway this is a deprecated throw away
There was a problem hiding this comment.
This was correct before if MVC is meant as the line above indicates
There was a problem hiding this comment.
toolCallbacks2 -> toolCallbacks ?
There was a problem hiding this comment.
does the toolCallbacks name need to change to one suffixed by 2?
There was a problem hiding this comment.
This requires cleanup I suppose
There was a problem hiding this comment.
ah, this is a blunder. renamed the classes but forgot to fix the configuration.
|
Thanks for the review @chemicL . Last commit should address the review comments |
There was a problem hiding this comment.
Would this work?
.map(c -> (McpSyncServerExchange exchange, List<McpSchema.Root> roots) -> c.accept(roots))
There was a problem hiding this comment.
I'm afraid not. Tried it before but it couldn't infer the types (or something like this).
This code will be removed after spring-ai M7 milestone release, so it is not that important.
46acf04 to
fd1ae33
Compare
There was a problem hiding this comment.
| public List<McpServerFeatures.SyncToolSpecification> syncToolsRegistrationToSpecificaiton( | |
| public List<McpServerFeatures.SyncToolSpecification> syncToolsRegistrationToSpecification( |
Several method names contain a typo: "Specificaiton" instead of "Specification".
5a001b8 to
5320640
Compare
- Rename Registration classes to Specification (SyncToolRegistration → SyncToolSpecification) - Update transport classes to use Provider suffix (WebFluxSseServerTransport → WebFluxSseServerTransportProvider) - Add exchange parameter to handler methods for better context passing - Introduce McpBackwardCompatibility class to maintain backward compatibility - Update MCP Server documentation to reflect new API patterns - Add tests for backward compatibility The changes align with the MCP specification evolution while maintaining backward compatibility through deprecated APIs. Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
…configuration Extracts the MCP tool callback functionality from McpClientAutoConfiguration into a new dedicated McpToolCallbackAutoConfiguration that is disabled by default. - Created new McpToolCallbackAutoConfiguration class that handles tool callback registration - Made tool callbacks opt-in by requiring explicit configuration with spring.ai.mcp.client.toolcallback.enabled=true - Removed deprecated tool callback methods from McpClientAutoConfiguration - Updated ClientMcpTransport references to McpClientTransport to align with MCP library changes - Added tests for the new auto-configuration and its conditions Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
5320640 to
55a11f6
Compare
- Change separator in McpToolUtils.prefixedToolName from hyphen to underscore - Add conversion of any remaining hyphens to underscores in formatted tool names - Update affected tests to reflect the new naming convention - Add comprehensive tests for McpToolUtils.prefixedToolName method - Add integration test for payment transaction tools with Vertex AI Gemini Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
|
squashed and merged in fc955c7 |
The changes align with the MCP specification evolution while maintaining backward compatibility through deprecated APIs.