From e1c9f67c1f23a3bce10492e86a6998da0fd8c786 Mon Sep 17 00:00:00 2001 From: Eric Hare Date: Thu, 5 Nov 2026 10:46:57 -0800 Subject: [PATCH] fix: describe accepted MCP update syntax in tool schemas --- .../command/clause/update/UpdateClause.java | 89 ++++++- .../command/clause/update/UpdateOperator.java | 42 +++- .../v1/mcp/McpOpenApiSchemaAdapterTest.java | 23 ++ .../api/v1/mcp/McpSchemaIntegrationTest.java | 49 ++++ .../v1/mcp/McpUpdateSchemaContractTest.java | 234 ++++++++++++++++++ 5 files changed, 419 insertions(+), 18 deletions(-) create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpUpdateSchemaContractTest.java diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateClause.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateClause.java index aa8c314757..f3d9bcabdf 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateClause.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateClause.java @@ -9,6 +9,7 @@ import java.util.stream.Collectors; import org.eclipse.microprofile.openapi.annotations.enums.SchemaType; import org.eclipse.microprofile.openapi.annotations.media.Schema; +import org.eclipse.microprofile.openapi.annotations.media.SchemaProperty; /** * Value type read from incoming JSON and used for resolving into actual {@link UpdateOperation}s @@ -16,16 +17,94 @@ */ @JsonDeserialize(using = UpdateClauseDeserializer.class) @Schema( + name = "UpdateClause", type = SchemaType.OBJECT, implementation = Object.class, - examples = - """ + additionalProperties = Schema.False.class, + description = + "A Data API update object mapping operators such as $set and $unset to objects of document paths and values. Operators may be combined on distinct paths. Tables support only $set, $unset, $push, and $pullAll; collections support every operator except $pullAll.", + properties = { + @SchemaProperty( + name = UpdateOperator.Names.SET, + type = SchemaType.OBJECT, + description = + "Sets paths to JSON values, including null, arrays, objects, and extended JSON."), + @SchemaProperty( + name = UpdateOperator.Names.SET_ON_INSERT, + type = SchemaType.OBJECT, + description = "Sets paths only when an upsert inserts a document."), + @SchemaProperty( + name = UpdateOperator.Names.UNSET, + type = SchemaType.OBJECT, + description = "Removes paths. The values supplied for them are ignored."), + @SchemaProperty( + name = UpdateOperator.Names.INC, + type = SchemaType.OBJECT, + description = "Maps paths to numeric increments, including fractions."), + @SchemaProperty( + name = UpdateOperator.Names.MUL, + type = SchemaType.OBJECT, + description = "Maps paths to numeric multipliers, including fractions."), + @SchemaProperty( + name = UpdateOperator.Names.MIN, + type = SchemaType.OBJECT, + description = "Updates paths when the supplied values are smaller."), + @SchemaProperty( + name = UpdateOperator.Names.MAX, + type = SchemaType.OBJECT, + description = "Updates paths when the supplied values are larger."), + @SchemaProperty( + name = UpdateOperator.Names.RENAME, + type = SchemaType.OBJECT, + description = "Maps existing paths to new path strings."), + @SchemaProperty( + name = UpdateOperator.Names.CURRENT_DATE, + type = SchemaType.OBJECT, + description = "Sets paths to the current date using true or {\"$type\": \"date\"}."), + @SchemaProperty( + name = UpdateOperator.Names.POP, + type = SchemaType.OBJECT, + description = "Removes the first (-1) or last (1) element of arrays."), + @SchemaProperty( + name = UpdateOperator.Names.PUSH, + type = SchemaType.OBJECT, + description = + "Appends values to arrays, or to list, set, and map columns. Use $each for multiple values; only collections accept $position."), + @SchemaProperty( + name = UpdateOperator.Names.ADD_TO_SET, + type = SchemaType.OBJECT, + description = "Adds unique array values. Use {\"$each\": [...]} for multiple values."), + @SchemaProperty( + name = UpdateOperator.Names.PULL_ALL, + type = SchemaType.OBJECT, + description = + "Tables only. Removes the listed values from list and set columns, and the listed keys from map columns.") + }, + examples = { + """ { "$set" : {"location": "New York"}, - "$unset" : {"new_data": 1} + "$unset" : {"obsolete": 1} } - """) -public record UpdateClause(EnumMap updateOperationDefs) { + """, + """ + {"$inc": {"score": 0.25}, "$mul": {"price": 1.5}, + "$min": {"lowest": 2.5}, "$max": {"highest": 9.5}} + """, + """ + {"$push": {"items": {"$each": [1, 2], "$position": 0}}, + "$addToSet": {"tags": {"$each": ["a", "b"]}}, + "$pop": {"history": -1}, "$rename": {"oldName": "newName"}} + """, + """ + {"$currentDate": {"updated": true}, + "$setOnInsert": {"created": {"$date": 123456}}, "$set": {"optional": null}} + """, + "{}" + }) +// Hide parsed operations from schemas without changing Jackson serialization. +public record UpdateClause( + @Schema(hidden = true) EnumMap updateOperationDefs) { /** * Method that will validate update operation definitions of the clause and construct an ordered * set of {@link UpdateOperation}s. diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperator.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperator.java index f9920aab0e..a05cefbef8 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperator.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperator.java @@ -7,83 +7,83 @@ /** List of update operator that's supported in update. */ public enum UpdateOperator { - ADD_TO_SET("$addToSet") { + ADD_TO_SET(Names.ADD_TO_SET) { @Override public UpdateOperation resolveOperation(ObjectNode arguments) { return AddToSetOperation.construct(arguments); } }, - CURRENT_DATE("$currentDate") { + CURRENT_DATE(Names.CURRENT_DATE) { @Override public UpdateOperation resolveOperation(ObjectNode arguments) { return CurrentDateOperation.construct(arguments); } }, - INC("$inc") { + INC(Names.INC) { @Override public UpdateOperation resolveOperation(ObjectNode arguments) { return IncOperation.construct(arguments); } }, - MAX("$max") { + MAX(Names.MAX) { @Override public UpdateOperation resolveOperation(ObjectNode arguments) { return MinMaxOperation.constructMax(arguments); } }, - MIN("$min") { + MIN(Names.MIN) { @Override public UpdateOperation resolveOperation(ObjectNode arguments) { return MinMaxOperation.constructMin(arguments); } }, - MUL("$mul") { + MUL(Names.MUL) { @Override public UpdateOperation resolveOperation(ObjectNode arguments) { return MulOperation.construct(arguments); } }, - POP("$pop") { + POP(Names.POP) { @Override public UpdateOperation resolveOperation(ObjectNode arguments) { return PopOperation.construct(arguments); } }, - PUSH("$push") { + PUSH(Names.PUSH) { @Override public UpdateOperation resolveOperation(ObjectNode arguments) { return PushOperation.construct(arguments); } }, - RENAME("$rename") { + RENAME(Names.RENAME) { @Override public UpdateOperation resolveOperation(ObjectNode arguments) { return RenameOperation.construct(arguments); } }, - SET("$set") { + SET(Names.SET) { @Override public UpdateOperation resolveOperation(ObjectNode arguments) { return SetOperation.constructSet(arguments); } }, - SET_ON_INSERT("$setOnInsert") { + SET_ON_INSERT(Names.SET_ON_INSERT) { @Override public UpdateOperation resolveOperation(ObjectNode arguments) { return SetOperation.constructSetOnInsert(arguments); } }, - UNSET("$unset") { + UNSET(Names.UNSET) { @Override public UpdateOperation resolveOperation(ObjectNode arguments) { return UnsetOperation.construct(arguments); @@ -92,7 +92,23 @@ public UpdateOperation resolveOperation(ObjectNode arguments) { // Then operators that we recognize but do not (yet) support - PULL_ALL("$pullAll"); + PULL_ALL(Names.PULL_ALL); + + public interface Names { + String ADD_TO_SET = "$addToSet"; + String CURRENT_DATE = "$currentDate"; + String INC = "$inc"; + String MAX = "$max"; + String MIN = "$min"; + String MUL = "$mul"; + String POP = "$pop"; + String PUSH = "$push"; + String RENAME = "$rename"; + String SET = "$set"; + String SET_ON_INSERT = "$setOnInsert"; + String UNSET = "$unset"; + String PULL_ALL = "$pullAll"; + } private String apiName; diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpOpenApiSchemaAdapterTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpOpenApiSchemaAdapterTest.java index b093b66e99..d29fd2df1a 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpOpenApiSchemaAdapterTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpOpenApiSchemaAdapterTest.java @@ -297,6 +297,29 @@ void shouldEscapeComponentNamesInReferences() { assertEquals("#/components/schemas/A~1B~0C", McpOpenApiSchemaAdapter.componentRef("A/B~C")); } + @Test + void shouldPreserveFractionalSchemaBoundsAndDefault() throws Exception { + var document = fixtureDocument(); + ((ObjectNode) document.path("components").path("schemas")) + .putObject("Fractional") + .put("type", "number") + .put("minimum", 0.25) + .put("maximum", 1.75) + .put("default", 0.5); + var schema = + bundle(MAPPER.readTree("{\"$ref\":\"#/components/schemas/Fractional\"}"), document); + var component = schema.at(schema.path("$ref").asText().substring(1)); + + assertEquals(0.25, component.path("minimum").doubleValue()); + assertEquals(1.75, component.path("maximum").doubleValue()); + assertEquals(0.5, component.path("default").doubleValue()); + assertAccepts(schema, "0.25"); + assertAccepts(schema, "1.75"); + assertAccepts(schema, component.path("default").toString()); + assertRejects(schema, "0.125"); + assertRejects(schema, "1.875"); + } + /** Bundles a copy of {@code input}, so tests can compare the result with the input. */ private static ObjectNode bundle(JsonNode input, JsonNode document) { ObjectNode schema = input.deepCopy(); diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpSchemaIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpSchemaIntegrationTest.java index 6ad33411b8..b6c2c243f5 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpSchemaIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpSchemaIntegrationTest.java @@ -12,7 +12,10 @@ import io.quarkus.test.junit.TestProfile; import io.stargate.sgv2.jsonapi.testresource.NoGlobalResourcesTestProfile; import io.vertx.core.json.JsonObject; +import java.util.HashSet; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; /** Checks the schemas clients receive from the packaged application's tools/list endpoint. */ @QuarkusIntegrationTest @@ -149,6 +152,52 @@ void findAndRerankPreservesNestedConstraints() { .thenAssertResults(); } + @ParameterizedTest + @ValueSource(strings = {"updateOne", "updateMany", "findOneAndUpdate"}) + void updateToolsAdvertiseAcceptedOperatorSyntax(String tool) { + mcpClient + .when() + .toolsList( + page -> { + var schema = schemaFor(page, tool); + var document = json(page.findByName(tool).inputSchema()); + var update = document.path("properties").path("update"); + var visited = new HashSet(); + while (update.has("$ref")) { + var reference = update.path("$ref").asText(); + assertTrue(reference.startsWith("#/"), reference); + assertTrue(visited.add(reference), "Circular update schema reference"); + update = document.at(reference.substring(1)); + assertFalse(update.isMissingNode(), reference); + } + var examples = update.path("examples"); + assertFalse(examples.isEmpty(), "Update tools must advertise usable examples"); + var arguments = + new JsonObject() + .put("keyspace", "default_keyspace") + .put("collection", "schema_test"); + for (var example : examples) { + arguments.put("update", new JsonObject(example.toString())); + assertAccepts(schema, arguments); + assertAccepts(schema, arguments.copy().putNull("filter")); + } + for (var malformed : + new String[] { + "{\"updateOperationDefs\":{}}", + "{\"$unknown\":{}}", + "{\"$set\":null}", + "{\"$set\":5}", + "{\"$set\":[]}" + }) { + assertRejects(schema, arguments.copy().put("update", new JsonObject(malformed))); + } + arguments.remove("update"); + assertRejects(schema, arguments); + assertRejects(schema, arguments.copy().putNull("update")); + }) + .thenAssertResults(); + } + private static JsonSchema schemaFor(ToolsPage page, String name) { var tool = page.findByName(name); assertNotNull(tool, name); diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpUpdateSchemaContractTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpUpdateSchemaContractTest.java new file mode 100644 index 0000000000..3735e8a286 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpUpdateSchemaContractTest.java @@ -0,0 +1,234 @@ +package io.stargate.sgv2.jsonapi.api.v1.mcp; + +import static io.restassured.RestAssured.given; +import static org.junit.jupiter.api.Assertions.*; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.networknt.schema.JsonSchemaFactory; +import com.networknt.schema.SpecVersion; +import io.quarkiverse.mcp.server.GlobalInputSchemaGenerator; +import io.quarkiverse.mcp.server.ToolManager; +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.junit.TestProfile; +import io.stargate.sgv2.jsonapi.api.model.command.Command; +import io.stargate.sgv2.jsonapi.api.model.command.Updatable; +import io.stargate.sgv2.jsonapi.api.model.command.clause.update.UpdateClause; +import io.stargate.sgv2.jsonapi.api.model.command.clause.update.UpdateOperator; +import io.stargate.sgv2.jsonapi.exception.UpdateException; +import io.stargate.sgv2.jsonapi.testresource.NoGlobalResourcesTestProfile; +import jakarta.inject.Inject; +import jakarta.validation.Validator; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.EnumSource; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; + +/** Checks update arguments advertised by the registered tools against the real command parser. */ +@QuarkusTest +@TestProfile(NoGlobalResourcesTestProfile.Impl.class) +class McpUpdateSchemaContractTest { + private static final List TOOLS = List.of("updateOne", "updateMany", "findOneAndUpdate"); + + @Inject ObjectMapper objectMapper; + @Inject GlobalInputSchemaGenerator schemaGenerator; + @Inject ToolManager toolManager; + @Inject Validator validator; + + @ParameterizedTest + @ValueSource(strings = {"updateOne", "updateMany", "findOneAndUpdate"}) + void advertisedExamplesValidateAndBuild(String tool) throws Exception { + var schema = inputSchema(tool); + var examples = updateSchema(schema).path("examples"); + assertFalse(examples.isEmpty(), "Update arguments must advertise usable examples"); + for (var update : examples) { + var payload = toolInput(update); + assertSchemaAccepts(schema, payload); + assertNotNull(parseCommand(tool, payload).updateClause().buildOperations()); + } + } + + @ParameterizedTest + @MethodSource("supportedUpdates") + void toolSchemasAcceptSupportedUpdates(String tool, String json) throws Exception { + var payload = toolInput(objectMapper.readTree(json)); + assertSchemaAccepts(inputSchema(tool), payload); + assertNotNull(parseCommand(tool, payload).updateClause().buildOperations()); + } + + static Stream supportedUpdates() { + // Nonempty arguments exercise each operation's parser, including supported array modifiers. + var updates = + List.of( + "{}", + "{\"$set\": {}}", + "{\"$set\": {\"profile.name\": \"Ada\", \"optional\": null}}", + "{\"$unset\": {\"obsolete\": null}}", + "{\"$setOnInsert\": {\"created\": true}}", + "{\"$inc\": {\"score\": 0.25}}", + "{\"$mul\": {\"score\": 1.5}}", + "{\"$min\": {\"score\": 2.5}}", + "{\"$max\": {\"score\": 9.5}}", + "{\"$rename\": {\"oldName\": \"newName\"}}", + "{\"$currentDate\": {\"updated\": true, \"reviewed\": {\"$type\": \"date\"}}}", + "{\"$pop\": {\"first\": -1, \"last\": 1}}", + "{\"$push\": {\"items\": {\"$each\": [1, 2], \"$position\": 0}}}", + "{\"$addToSet\": {\"tags\": {\"$each\": [\"a\", \"b\"]}}}", + """ + {"$set": {"created": {"$date": 123456}, + "owner": {"$uuid": "550e8400-e29b-41d4-a716-446655440000"}, + "nested": {"values": [null, true, 1.25, {"name": "Ada"}]}}, + "$unset": {"obsolete": 1}} + """); + return TOOLS.stream().flatMap(tool -> updates.stream().map(json -> Arguments.of(tool, json))); + } + + @ParameterizedTest + @MethodSource("invalidUpdates") + void toolSchemasRejectMalformedUpdates(String tool, String json) throws Exception { + var update = objectMapper.readTree(json); + assertSchemaRejects(inputSchema(tool), toolInput(update)); + assertThrows(UpdateException.class, () -> objectMapper.treeToValue(update, UpdateClause.class)); + } + + static Stream invalidUpdates() { + var updates = + List.of( + "[]", + "5", + "\"update\"", + "{\"name\": \"Ada\"}", + "{\"updateOperationDefs\": {}}", + "{\"$unknown\": {}}", + "{\"$set\": null}", + "{\"$set\": 5}", + "{\"$set\": []}", + "{\"$set\": \"value\"}"); + return TOOLS.stream().flatMap(tool -> updates.stream().map(json -> Arguments.of(tool, json))); + } + + // updateOne is the only update tool that also serves tables. + @Test + void updateOneAcceptsTableOnlyPullAll() throws Exception { + var payload = toolInput(objectMapper.readTree("{\"$pullAll\": {\"listColumn\": [1]}}")); + assertSchemaAccepts(inputSchema("updateOne"), payload); + assertEquals( + Set.of(UpdateOperator.PULL_ALL), + parseCommand("updateOne", payload).updateClause().updateOperationDefs().keySet()); + } + + @ParameterizedTest + @ValueSource(strings = {"updateOne", "updateMany", "findOneAndUpdate"}) + void updateIsRequiredAndFilterMayBeOmittedOrNull(String tool) throws Exception { + var schema = inputSchema(tool); + var payload = toolInput(objectMapper.readTree("{}")); + assertSchemaAccepts(schema, payload); + assertNotNull(parseCommand(tool, payload).updateClause().buildOperations()); + payload.putNull("filter"); + assertSchemaAccepts(schema, payload); + assertNotNull(parseCommand(tool, payload).updateClause().buildOperations()); + + payload.remove("update"); + assertSchemaRejects(schema, payload); + assertFalse(validator.validate(command(tool, payload)).isEmpty()); + payload.putNull("update"); + assertSchemaRejects(schema, payload); + assertFalse(validator.validate(command(tool, payload)).isEmpty()); + } + + @ParameterizedTest + @ValueSource(strings = {"updateOne", "updateMany", "findOneAndUpdate"}) + void openApiCommandsAcceptAdvertisedUpdateExamples(String tool) throws Exception { + var document = + objectMapper.readTree( + given() + .queryParam("format", "json") + .get("/api/json/openapi") + .then() + .statusCode(200) + .extract() + .asString()); + var name = Character.toUpperCase(tool.charAt(0)) + tool.substring(1) + "Command"; + var schema = objectMapper.createObjectNode().put("$ref", "#/components/schemas/" + name); + schema.set("components", document.path("components")); + var examples = updateSchema(inputSchema(tool)).path("examples"); + assertFalse(examples.isEmpty(), "OpenAPI checks must exercise the advertised update examples"); + for (var update : examples) { + var payload = objectMapper.createObjectNode().set("update", update); + assertSchemaAccepts(schema, payload); + } + } + + // $pullAll is table-only, and every advertised example must also build for collections. + @ParameterizedTest + @EnumSource(value = UpdateOperator.class, names = "PULL_ALL", mode = EnumSource.Mode.EXCLUDE) + void everySupportedOperatorHasAnAdvertisedExample(UpdateOperator operator) throws Exception { + var examples = updateSchema(inputSchema("updateOne")).path("examples"); + assertTrue( + StreamSupport.stream(examples.spliterator(), false) + .anyMatch(example -> example.has(operator.apiName())), + () -> "Missing example for " + operator.apiName()); + } + + private Updatable parseCommand(String tool, ObjectNode payload) throws Exception { + var command = command(tool, payload); + assertTrue(validator.validate(command).isEmpty()); + return (Updatable) command; + } + + private Command command(String tool, ObjectNode payload) throws Exception { + var arguments = payload.deepCopy(); + arguments.remove(List.of("keyspace", "collection")); + return objectMapper.treeToValue( + objectMapper.createObjectNode().set(tool, arguments), Command.class); + } + + private ObjectNode toolInput(JsonNode update) { + var payload = objectMapper.createObjectNode(); + payload.put("keyspace", "default_keyspace").put("collection", "documents"); + payload.set("update", update); + return payload; + } + + private JsonNode inputSchema(String tool) throws Exception { + return objectMapper.readTree(schemaGenerator.generate(toolManager.getTool(tool)).asJson()); + } + + private static JsonNode updateSchema(JsonNode document) { + var schema = document.path("properties").path("update"); + var visited = new HashSet(); + while (schema.has("$ref")) { + var reference = schema.path("$ref").asText(); + assertTrue(reference.startsWith("#/"), reference); + assertTrue(visited.add(reference), "Circular update schema reference"); + schema = document.at(reference.substring(1)); + assertFalse(schema.isMissingNode(), "Unresolved update schema " + reference); + } + return schema; + } + + private void assertSchemaAccepts(JsonNode schema, JsonNode payload) { + var errors = + JsonSchemaFactory.getInstance(SpecVersion.VersionFlag.V202012) + .getSchema(schema) + .validate(payload); + assertTrue(errors.isEmpty(), () -> "Schema validation for " + payload + " returned " + errors); + } + + private void assertSchemaRejects(JsonNode schema, JsonNode payload) { + assertFalse( + JsonSchemaFactory.getInstance(SpecVersion.VersionFlag.V202012) + .getSchema(schema) + .validate(payload) + .isEmpty(), + () -> "Schema accepted malformed update " + payload); + } +}