Skip to content

Improve MchSchema JSON consistency #26

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

Closed
tzolov opened this issue Mar 1, 2025 · 1 comment · Fixed by #27 or pascalconfluent/mcp-java-sdk#1
Closed

Improve MchSchema JSON consistency #26

tzolov opened this issue Mar 1, 2025 · 1 comment · Fixed by #27 or pascalconfluent/mcp-java-sdk#1
Milestone

Comments

@tzolov
Copy link
Contributor

tzolov commented Mar 1, 2025

Bug description

Redundant type handling in McpSchema Content implementations causes unnecessary code complexity and potential maintenance issues.
The Content interface and its implementations (TextContent, ImageContent, EmbeddedResource) currently maintain explicit type fields and methods, despite Jackson already handling type information through @JsonSubTypes annotation.
This redundancy increases code complexity and creates potential for inconsistencies if the explicit type values don't match the expected polymorphic types.

Environment

Java SDK version: 17
MCP Java SDK version: 0.8.0-SNAPSHOT

Steps to reproduce

	@Test
	void testTextContent() throws Exception {
		McpSchema.TextContent test = new McpSchema.TextContent("XXX");
		String value = mapper.writeValueAsString(test);
		assertEquals("""
				{"type":"text","text":"XXX"}""", value);
	}

Solution:

Lets Jackson handle the type information entirely and removes the redundant field and method:

  • Remove the type() method from Content interface
  • Removing the type field, constructor initialization, and getter method from all implementations
  • Adjusting constructors to no longer require the type parameter
  • Ensuring tests pass, confirming that JSON serialization still includes the correct type information
@tzolov tzolov added this to the 0.8.0 milestone Mar 1, 2025
@tzolov
Copy link
Contributor Author

tzolov commented Mar 1, 2025

Related to the spring-projects/spring-ai#2350

tzolov added a commit that referenced this issue Mar 1, 2025
- The type field and associated methods were redundant in Content implementations (TextContent, ImageContent, EmbeddedResource)
  as the type information is already handled by Jackson's polymorphic type handling via @JsonSubTypes annotation.
- Added comprehensive unit tests for McpSchema to validate serialization/deserialization behavior of all schema components.

Resolves #26
Resolve spring-projects/spring-ai#2350

Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
@tzolov tzolov closed this as completed in #27 Mar 3, 2025
tzolov added a commit that referenced this issue Mar 3, 2025
…ns (#27)

- The type field and associated methods were redundant in Content implementations (TextContent, ImageContent, EmbeddedResource)
  as the type information is already handled by Jackson's polymorphic type handling via @JsonSubTypes annotation.
- Add default implementation that returns the appropriate type string based on the implementing class (text, image, or resource)
- Added comprehensive unit tests for McpSchema to validate serialization/deserialization behavior of all schema components.
- Add deserialization tests for all content types
- Add json-unit-assertj for flexible JSON testing -  ignores array order and extra array items, reducing test brittleness while
maintaining functional validation.

Resolves #26
Resolve spring-projects/spring-ai#2350

Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment