refactor: deprecate tool pre and post processing hooks to align with cross-SDK architecture - #89
Open
stenalpjolly wants to merge 1 commit into
Conversation
…cross-SDK architecture
Contributor
Author
|
/gcbrun |
anubhav756
requested changes
Sep 16, 2026
Contributor
There was a problem hiding this comment.
Please add documentation for orchestration pre and post processing to match the other SDKs, here https://mcp-toolbox.dev/documentation/configuration/pre-post-processing/
| * @param headers Default HTTP headers. | ||
| * @param credentialsProvider Provider for credentials. | ||
| */ | ||
| @Deprecated |
Contributor
There was a problem hiding this comment.
Why are we removing this deprecation?
| * @param credentialsProvider Provider for credentials. | ||
| * @param preProcessors List of pre-processors. | ||
| * @param postProcessors List of post-processors. | ||
| * @deprecated Use {@link #McpToolboxClientImpl(Transport, Map, CredentialsProvider)} instead. |
Contributor
There was a problem hiding this comment.
Did we not remove the deprecation from this constructor? I am a bit confused about whether we want to deprecate this or not.
| * | ||
| * @param processor The pre-processor to add. | ||
| * @return The tool instance. | ||
| * @deprecated Use orchestration framework callbacks instead of tool-level pre-processors. |
Contributor
There was a problem hiding this comment.
Is this written the other way around?
| * | ||
| * @param processor The post-processor to add. | ||
| * @return A new tool instance with the post-processor added. | ||
| * @deprecated Use orchestration framework callbacks instead of tool-level post-processors. |
Contributor
There was a problem hiding this comment.
Same, is this written the other way around?
anubhav756
requested changes
Sep 16, 2026
| * A functional interface for pre-processing tool inputs before invocation. | ||
| * | ||
| * @deprecated Tool execution hooks are deprecated in the core SDK to align with cross-SDK | ||
| * architecture. Use orchestration framework callbacks (e.g., ADK, LangChain4j, or Spring AI) |
Contributor
There was a problem hiding this comment.
optional: Once we create the pre/post processing doc, can we link that here?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This pull request marks the tool pre- and post-processing execution hook interfaces (
ToolPreProcessor,ToolPostProcessor), builder methods (McpToolboxClient.Builder.preProcessor,postProcessor), tool registration methods (Tool.addPreProcessor,addPostProcessor), and the 5-argumentMcpToolboxClientImplconstructor as@Deprecatedto align the Java SDK with the cross-language MCP Toolbox SDK architecture (such as Python PR #539) while preserving fullv1.0.0semantic versioning backward compatibility.Expectation & Implementation
mcp-toolbox-sdk-javav1.0.0 was released with PR feat: add pre & post processing hooks for tools #58's classes and methods in the public API, hard deletion would break SemVer compatibility. Marking these APIs as@Deprecatedguides users toward framework-level callbacks while maintaining 100% binary and source compatibility for1.xcallers.ToolPreProcessorandToolPostProcessorwith@Deprecatedand@deprecatedJavadoc tags.McpToolboxClient.Builder.preProcessor(ToolPreProcessor)andpostProcessor(ToolPostProcessor)(and their implementations inMcpToolboxClientBuilder) with@Deprecated.Tool.addPreProcessor(ToolPreProcessor)andTool.addPostProcessor(ToolPostProcessor)with@Deprecated.McpToolboxClientImpl(Transport, Map, CredentialsProvider, List<ToolPreProcessor>, List<ToolPostProcessor>)with@Deprecatedand un-deprecated the 3-argument constructorMcpToolboxClientImpl(Transport, Map, CredentialsProvider)so callers instantiatingMcpToolboxClientImplwithout deprecated hooks have a clean, non-deprecated constructor.@SuppressWarnings("deprecation")to internal classes and unit test methods that exercise deprecated hook functionality so the build compiles with zero deprecation warnings.Test cases
ToolTest,McpToolboxClientBuilderTest, andMcpToolboxClientImplTestcontinue to pass and validate deprecated hook execution paths for backward compatibility.mvn com.spotify.fmt:fmt-maven-plugin:check javadoc:javadoc checkstyle:check testunder JDK 17.Tool,McpToolboxClientBuilder,McpToolboxClientImpl).Acceptance criteria
ToolPreProcessor,ToolPostProcessor, and corresponding builder/tool registration methods are annotated with@Deprecatedand documented with@deprecatedJavadocs.McpToolboxClientImpl(Transport, Map, CredentialsProvider)is un-deprecated as the clean constructor when pre/post processors are omitted.checkstyle:check) and zero formatting deviations (fmt-maven-plugin:check).Breaking changes
None. All deprecated methods and interfaces remain fully functional for
1.xbackward compatibility.TAG=agy
CONV=8a6d2e06-1e32-45cb-84cc-9ffe9dfd9e98