-
Notifications
You must be signed in to change notification settings - Fork 646
Introduce McpJsonMapper interface to decouple from Jackson #543
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.
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
Introduce McpJsonMapper interface to decouple from Jackson #543
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have created a PR on top of this graemerocher#1
this.jsonMapper = new JacksonMcpJsonMapper(objectMapper); | ||
return this; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.jsonMapper = new JacksonMcpJsonMapper(objectMapper); | |
return this; | |
return jsonMapper(new JacksonMcpJsonMapper(objectMapper)); |
Do you want to use Vert.x's json? |
We want to use Micronaut Serialization. But basically, the idea is to allow users to use whatever JSON Serialization they want. |
This pull request creates two modules, `mcp-json` and `mcp-json-jackson`. It removes the `com.fasterxml.jackson.core:jackson-databind` and `com.networknt:json-schema-validator` dependencies from the `mcp` module. The `mcp` module now only depends on `com.fasterxml.jackson.core:jackson-annotations`. To use Jackson, you have to add `mcp-jackson` to your dependencies in addition to `mcp`. I added the dependency `mcp-jackson` to both `mcp-spring-mvc` and `mcp-spring-webflux` to avoid a breaking change in those modules. It provides two [SPI](https://docs.oracle.com/javase/tutorial/sound/SPI-intro.html) `JsonSchemaValidatorSupplier` and `JacksonJsonSchemaValidatorSupplier` to allow easy replacement for consumers who don't want to use Jackson. This pull request also ensures no `McpJsonMapper` is instantiated if one is provided via a builder method. Only if the builders don't receive a `McpJsonMapper` mapper, one is instantiated in the `build` method of the builder. The logic behind this is to allow frameworks to provide a `McpJsonMapper` mapper singleton implementation and feed it to the builders without paying the price of instantiating `McpJsonMappers`, which will not be used. The goal is to be able to use the `ObjectMapper` singleton of an application also for the MCP code. ## Breaking changes - This pull request also removes the deprecated `Tool` constructors, and it updates every test to use the builder API to instantiate tools. - Several constructors taking an ObjectMapper or constructor which forced the instantiation of a generic `McpJsonMapper` have been removed.
I did not review everything in detail, but from a high level perspective, this PR looks great, thanks for contributing it @graemerocher @sdelamo. The main open question for me is related to Jackson 3 which is about to be released. If we want to support Jackson 2 and Jackson 3 (which continues to use the same If we just plan to upgrade from Jackson 2 to Jackson 3, current naming is fine. |
probably including the major version makes sense for Jackson. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @graemerocher, @sdelamo ! Nice work.
Few improvement requests:
- Add the
mcp-json
andmcp-json-jackson
modules to themcp-bom
- Add missing MIT license headers:
/* * Copyright 2025 - 2025 the original author or authors. */
- Update mcp java sdk reference documentation (remainder for a follow-up PR)
There's another conceptual change that I'm puzzled with. Currently the mcp
is self-contained. But with this PR it becomes a "core" component requiring an additional mcp-json-xxx (like mcp-json-jackson) dependency for the same bootstrap experience without breaking changes.
I wonder if we should rename the existing mcp module to mcp-core and create a small mcp module that includes both mcp-core and mcp-json-jackson by default?
This will preserver the existing single-click experience for the end users
mcp/pom.xml
Outdated
<version>0.13.0-SNAPSHOT</version> | ||
</dependency> | ||
|
||
<dependency> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to the test section of the pom
<artifactId>mcp-parent</artifactId> | ||
<version>0.13.0-SNAPSHOT</version> | ||
</parent> | ||
<artifactId>mcp-json</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add to the mcp-bom
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see: graemerocher@1b4271b
mcp-json-jackson/pom.xml
Outdated
<artifactId>mcp-parent</artifactId> | ||
<version>0.13.0-SNAPSHOT</version> | ||
</parent> | ||
<artifactId>mcp-json-jackson</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add to the mcp-bom
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see: graemerocher@1b4271b
You are right. To avoid breaking changes, I left in this PR mcp-json-jackson as a compile dependency. Once this PR gets merged, I will create a PR that moves the mcp classes to mcp-core. |
|
* add mcp-json and mcp-json-jackson to BOM * add license header * add mcp-json-jackon as a compile dependency * rename to jackson 2
This pull request creates two modules, `mcp-json` and `mcp-json-jackson`. It removes the `com.fasterxml.jackson.core:jackson-databind` and `com.networknt:json-schema-validator` dependencies from the `mcp` module. The `mcp` module now only depends on `com.fasterxml.jackson.core:jackson-annotations`. To use Jackson, you have to add `mcp-jackson` to your dependencies in addition to `mcp`. I added the dependency `mcp-jackson` to both `mcp-spring-mvc` and `mcp-spring-webflux` to avoid a breaking change in those modules. It provides two [SPI](https://docs.oracle.com/javase/tutorial/sound/SPI-intro.html) `JsonSchemaValidatorSupplier` and `JacksonJsonSchemaValidatorSupplier` to allow easy replacement for consumers who don't want to use Jackson. This pull request also ensures no `McpJsonMapper` is instantiated if one is provided via a builder method. Only if the builders don't receive a `McpJsonMapper` mapper, one is instantiated in the `build` method of the builder. The logic behind this is to allow frameworks to provide a `McpJsonMapper` mapper singleton implementation and feed it to the builders without paying the price of instantiating `McpJsonMappers`, which will not be used. The goal is to be able to use the `ObjectMapper` singleton of an application also for the MCP code. Signed-off-by: Christian Tzolov <[email protected]>
Thank you @graemerocher @sdelamo Rebased, conflicts resolved, squashed and merged at 80d0ad8 |
Motivation and Context
The current MCP SDK is coupled to Jackson which prevents the use of other JSON serialization libraries and techniques.
This PR is an initial go and abstracting away Jackson usage in order to generate discussion. Left in draft to gather feedback.
This pull request creates two modules,
mcp-json
andmcp-json-jackson
. It removes thecom.fasterxml.jackson.core:jackson-databind
andcom.networknt:json-schema-validator
dependencies from themcp
module. Themcp
module now only depends oncom.fasterxml.jackson.core:jackson-annotations
.To use Jackson, you have to add
mcp-jackson
to your dependencies in addition tomcp
. I added the dependencymcp-jackson
to bothmcp-spring-mvc
andmcp-spring-webflux
to avoid a breaking change in those modules.It provides two SPI
JsonSchemaValidatorSupplier
andJsonSchemaValidatorSupplier
to allow easy replacement for consumers who don't want to use Jackson.This pull request also ensures no
McpJsonMapper
is instantiated if one is provided via a builder method. Only if the builders don't receive aMcpJsonMapper
mapper, one is instantiated in thebuild
method of the builder.The logic behind this is to allow frameworks to provide a
McpJsonMapper
mapper singleton implementation and feed it to the builders without paying the price of instantiatingMcpJsonMappers
, which will not be used. The goal is to be able to use theObjectMapper
singleton of an application also for the MCP code.How Has This Been Tested?
Yes tests included.
Breaking Changes
Tool
constructors, and it updates every test to use the builder API to instantiate tools.McpJsonMapper
have been removed.Types of changes
Checklist
Additional context