-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Migrate Spring AI from Jackson 2 to Jackson 3 #5246
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
base: main
Are you sure you want to change the base?
Conversation
It requires for now installing locally: - modelcontextprotocol/java-sdk#742 - https://github.com/victools/jsonschema-generator main branch JsonMapper is used instead of ObjectMapper, following Jackson 3 best practices and the same pattern used by Spring Framework and Spring Boot. JacksonUtils#instantiateAvailableModules now leverages Jackson service loader based discovery of Jackson module. TODO: - Upgrade to com.github.victools:jsonschema-generator:5.0.0 - Upgrade to MCP Java SDK 0.18.0 Signed-off-by: Sébastien Deleuze <[email protected]>
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 for pinging me @sdeleuze.
I've left some comments around using JsonMapper.shared() instead of new JsonMapper() in production code. I think it's a better approach to use the shared, since it avoids creating new instances. I tried to comment in all the production code places I saw in case it is easier for you like that.
Additionally, I left some other comments like the fact that we can use Jackson 3 with the ElasticsearchVectorStore and a small note on the hugging face chat model and the schema creation.
.../springframework/ai/mcp/client/common/autoconfigure/properties/McpStdioClientProperties.java
Outdated
Show resolved
Hide resolved
.../mcp/client/httpclient/autoconfigure/StreamableHttpHttpClientTransportAutoConfiguration.java
Outdated
Show resolved
Hide resolved
...ramework/ai/mcp/client/httpclient/autoconfigure/SseHttpClientTransportAutoConfiguration.java
Show resolved
Hide resolved
|
|
||
| var webClientBuilderTemplate = webClientBuilderProvider.getIfAvailable(WebClient::builder); | ||
| var objectMapper = objectMapperProvider.getIfAvailable(ObjectMapper::new); | ||
| var jsonMapper = jsonMapperProvider.getIfAvailable(JsonMapper::new); |
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.
We could use JsonMapper::shared instead of creating a new instance.
...ork/ai/mcp/client/webflux/autoconfigure/StreamableHttpWebFluxTransportAutoConfiguration.java
Show resolved
Hide resolved
...pring-ai-chroma-store/src/main/java/org/springframework/ai/chroma/vectorstore/ChromaApi.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/springframework/ai/vectorstore/elasticsearch/ElasticsearchVectorStore.java
Outdated
Show resolved
Hide resolved
...one-store/src/main/java/org/springframework/ai/vectorstore/pinecone/PineconeVectorStore.java
Outdated
Show resolved
Hide resolved
...ate-store/src/main/java/org/springframework/ai/vectorstore/weaviate/WeaviateVectorStore.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Sébastien Deleuze <[email protected]>
This draft PR for Spring AI 2.0 requires for now installing locally:
JsonMapperis used instead ofObjectMapper, following Jackson 3 best practices and the same pattern used by Spring Framework and Spring Boot.JacksonUtils#instantiateAvailableModulesnow leverages Jackson service loader based discovery of Jackson module.TODO:
cc @filiphr