Refactor of json + json schema validation api into mcp-core #682
Conversation
scottslewis
changed the title
for mcp-core (DefaultMcpJsonMapperSupplier,DefaultMcpJsonSchemaValidator) and mcp-json-jackson2 (JacksonMcpJsonMapperSupplier, JacksonJsonSchemaValidatorSupplier)
creating a single DefaultMcpJson class that provides access to both the DefaultMcpJsonMapper and the DefaultMcpJson Validator
access to default mapper and defaul validator services.
…cp-java-sdk into issue_612_refactor
|
@sdelamo please check this is ok |
scottslewis
changed the title
|
@scottslewis Could you please clarify if the fact that |
mcp-json, mcp-json-jackson2, and mcp-core are distributed separately, so in OSGi environments these are loaded as separate bundles, with their own classloaders (separate from other bundle's classloaders and the threadcontextclassloader). In OSGi the java ServiceLoader (which in non-osgi environments the ServiceLoader uses threadcontextclassloader.getResource to load the JacksonMcpJsonMapperSupplier, and JacksonJsonSchemaValidatorSupplier files from mcp-json-jackson2), cannot find the Jackson*Supplier files, and so the defaults json mapper and the default json validator can't be found/created/set (as per stack traces reported in issue #562). Moving json api back into mcp-core does not directly fix this problem wrt ServiceLoader. In OSGi, environments, however, it allows the service component runtime to be used instead of the ServiceLoader. In OSGi, instances of these JacksonProvider service can be added to the OSGi service registry on startup. No code changes are necessary, but it is necessary to add OSGI metadata to mcp-core and mcp-jackson2, so that scr can 'see' these JacksonProviders in the OSGi service registry and create defaults on startup. The upshot is that the ServiceLoader functionality can be usurped by the OSGI service component runtime in OSGI enivronments, since the scr is fully dynamic (not just start/load time as ServiceLoader is), and fully deals with the classloader-per-jar aspect of osgi. The OSGi service registry and scr are well-established, well-debugged, and lightweight. This metadata can be added at build time, and is unlikely to be changed in the future. Another issue that led to filing this issue: having mcp-core depend upon any other api bundle (mcp-json) really creates a lot of problems for the building/packaging and deployment (and maintenance) of the sdk. For example, it requires two jars (mcp-core and mcp-json) in the most minimal configuration...otherwise mcp-core doesn't work/start at all (needed classes can't be loaded) and so at start/runtime failure can be very hard for consumers to track down. Combine this with the cross-environment ServiceLoader difficulties described above, combined with the fact that mcp-json is a very small number of (api) files, with no dependencies, it seems to me that mcp-json should just be re-absorbed into mcp-core. |
3 participants
into mcp-core.
Moved classes from mcp-json jar into new packages io.modelcontextprotocol.json, io.modelcontextprotocol.json.schema, and io.modelcontextprotocol.json.internal. Updated dependencies on moved classes in mcp-core, mcp-jackson2 and other maven modules.
Motivation and Context
This provides a fix for #612 and removed mcp-core dependencies on mcp-json. After these refactoring, mcp-json project/jar is not needed and only mcp-core and mcp-jackson2 have to be deployed to have a functioning sdk.
How Has This Been Tested?
Have run test suite locally. Some of the mcp-core tests are failing...apparently because of the environment setup for docker...for example:
[ERROR] HttpSseMcpAsyncClientLostConnectionTests.testPingWithExactExceptionType ┬╗ ExceptionInInitializer
[ERROR] HttpSseMcpAsyncClientTests.startContainer:41 ┬╗ IllegalState Previous attempts to find a Docker environment failed. Will not retry. Please see logs and check configuration
Breaking Changes
As per #612, previously mcp-core had compile-time dependencies on mcp-json, meaning that mcp-core, mcp-json, and mcp-jackson2 had to be deployed together or nothing would work...even code that just used mcp-core.
Types of changes
Checklist