diff --git a/jsonrpc-common/src/main/java/org/a2aproject/sdk/jsonrpc/common/json/JsonUtil.java b/jsonrpc-common/src/main/java/org/a2aproject/sdk/jsonrpc/common/json/JsonUtil.java index 9d3208cd4..dd98b811b 100644 --- a/jsonrpc-common/src/main/java/org/a2aproject/sdk/jsonrpc/common/json/JsonUtil.java +++ b/jsonrpc-common/src/main/java/org/a2aproject/sdk/jsonrpc/common/json/JsonUtil.java @@ -17,6 +17,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Stream; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -508,8 +509,8 @@ public static Map readMetadata(@Nullable String json) throws Jso * This adapter handles polymorphic deserialization, creating the * appropriate subclass instance (TextPart, FilePart, or DataPart) based on available fields. *

- * The adapter uses a two-pass approach: first reads the JSON as a tree to inspect the "kind" - * field, then deserializes to the appropriate concrete type. + * The adapter reads the JSON as a tree so that the content key can be inspected before + * deserializing to the appropriate concrete type. * * @see Part * @see TextPart @@ -599,29 +600,23 @@ Part read(JsonReader in) throws java.io.IOException { Map metadata = JsonUtil.readMetadata(jsonObject); Set keys = jsonObject.keySet(); - // Find the oneOf discriminator, skipping null/empty values to tolerate formats - // where multiple content keys may be present with only one populated - // (e.g., proto serialization with alwaysPrintFieldsWithNoPresence). - // Unknown extra fields are ignored. - String discriminator = keys.stream() - .filter(VALID_KEYS::contains) - .filter(key -> { - com.google.gson.JsonElement el = jsonObject.get(key); - return el != null && !el.isJsonNull(); - }) - .findFirst() + // A producer that emits every content key, leaving the unset ones as "", would otherwise + // have its payload discarded by whichever empty placeholder came first on the wire. + // The fallback keeps a deliberately empty TextPart decodable. + String discriminator = contentKeys(jsonObject).filter(key -> !isEmptyString(jsonObject.get(key))).findFirst() + .or(() -> contentKeys(jsonObject).findFirst()) .orElseThrow(() -> new JsonSyntaxException(format("Part must have one of: %s (found: %s)", VALID_KEYS, keys))); return switch (discriminator) { - case TEXT -> new TextPart(jsonObject.get(TEXT).getAsString(), metadata); + case TEXT -> new TextPart(requireString(jsonObject, TEXT), metadata); case RAW -> new FilePart(new FileWithBytes( stringOrEmpty(jsonObject, MEDIA_TYPE), stringOrEmpty(jsonObject, FILENAME), - jsonObject.get(RAW).getAsString()), metadata); + requireString(jsonObject, RAW)), metadata); case URL -> new FilePart(new FileWithUri( stringOrEmpty(jsonObject, MEDIA_TYPE), stringOrEmpty(jsonObject, FILENAME), - jsonObject.get(URL).getAsString()), metadata); + requireString(jsonObject, URL)), metadata); case DATA -> { Object data = delegateGson.fromJson(jsonObject.get(DATA), Object.class); yield new DataPart(data, metadata); @@ -630,6 +625,29 @@ Part read(JsonReader in) throws java.io.IOException { }; } + /** Returns the content keys carrying a non-null value, in document order. */ + private Stream contentKeys(com.google.gson.JsonObject obj) { + return obj.keySet().stream() + .filter(VALID_KEYS::contains) + .filter(key -> { + JsonElement el = obj.get(key); + return el != null && !el.isJsonNull(); + }); + } + + private boolean isEmptyString(@Nullable JsonElement el) { + return el != null && el.isJsonPrimitive() && el.getAsJsonPrimitive().isString() && el.getAsString().isEmpty(); + } + + /** Guards {@code getAsString()}, which throws an unchecked UnsupportedOperationException on an object or array. */ + private String requireString(com.google.gson.JsonObject obj, String key) { + JsonElement el = obj.get(key); + if (el == null || !el.isJsonPrimitive()) { + throw new JsonSyntaxException(format("Part '%s' must be a JSON string", key)); + } + return el.getAsString(); + } + /** Returns the string value of the field, or an empty string if absent or null. */ private String stringOrEmpty(com.google.gson.JsonObject obj, String key) { com.google.gson.JsonElement el = obj.get(key); diff --git a/jsonrpc-common/src/test/java/org/a2aproject/sdk/jsonrpc/common/json/PartSerializationTest.java b/jsonrpc-common/src/test/java/org/a2aproject/sdk/jsonrpc/common/json/PartSerializationTest.java new file mode 100644 index 000000000..9876d5a21 --- /dev/null +++ b/jsonrpc-common/src/test/java/org/a2aproject/sdk/jsonrpc/common/json/PartSerializationTest.java @@ -0,0 +1,161 @@ +package org.a2aproject.sdk.jsonrpc.common.json; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.Map; + +import org.a2aproject.sdk.spec.DataPart; +import org.a2aproject.sdk.spec.FilePart; +import org.a2aproject.sdk.spec.FileWithBytes; +import org.a2aproject.sdk.spec.FileWithUri; +import org.a2aproject.sdk.spec.Part; +import org.a2aproject.sdk.spec.TextPart; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** + * Decoding of the flat Part oneOf when a producer emits more than one content key. + */ +public class PartSerializationTest { + + @Test + void testPopulatedDataWinsOverEmptyTextPlaceholder() throws JsonProcessingException { + String json = """ + {"text": "", "data": {"answer": "42"}} + """; + Part part = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(DataPart.class, part); + @SuppressWarnings("unchecked") + Map data = (Map) ((DataPart) part).data(); + assertEquals("42", data.get("answer")); + } + + @Test + void testDataPartDecodesRegardlessOfContentKeyOrder() throws JsonProcessingException { + String json = """ + {"data": {"answer": "42"}, "text": ""} + """; + Part part = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(DataPart.class, part); + @SuppressWarnings("unchecked") + Map data = (Map) ((DataPart) part).data(); + assertEquals("42", data.get("answer")); + } + + @Test + void testPopulatedDataWinsOverAllEmptyPlaceholders() throws JsonProcessingException { + String json = """ + {"text": "", "raw": "", "url": "", "data": {"answer": "42"}} + """; + Part part = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(DataPart.class, part); + @SuppressWarnings("unchecked") + Map data = (Map) ((DataPart) part).data(); + assertEquals("42", data.get("answer")); + } + + @Test + void testPopulatedUrlWinsOverEmptyTextPlaceholder() throws JsonProcessingException { + String json = """ + {"text": "", "url": "https://example.org/report.pdf", "mediaType": "application/pdf"} + """; + Part part = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(FilePart.class, part); + FileWithUri file = (FileWithUri) ((FilePart) part).file(); + assertEquals("https://example.org/report.pdf", file.uri()); + assertEquals("application/pdf", file.mimeType()); + } + + @Test + void testPopulatedRawWinsOverEmptyTextPlaceholder() throws JsonProcessingException { + String json = """ + {"text": "", "raw": "abc12w==", "filename": "diagram.png", "mediaType": "image/png"} + """; + Part part = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(FilePart.class, part); + FileWithBytes file = (FileWithBytes) ((FilePart) part).file(); + assertEquals("abc12w==", file.bytes()); + assertEquals("diagram.png", file.name()); + assertEquals("image/png", file.mimeType()); + } + + /** Emptiness is decided on the JSON string only, so a falsy data payload still outranks a placeholder. */ + @ParameterizedTest + @ValueSource(strings = {"0", "false", "{}", "[]"}) + void testFalsyDataPayloadWinsOverEmptyTextPlaceholder(String payload) throws JsonProcessingException { + Part part = JsonUtil.fromJson("{\"text\": \"\", \"data\": " + payload + "}", Part.class); + assertInstanceOf(DataPart.class, part); + } + + @Test + void testJsonNullContentKeyIsSkipped() throws JsonProcessingException { + String json = """ + {"text": "hello", "data": null} + """; + Part part = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(TextPart.class, part); + assertEquals("hello", ((TextPart) part).text()); + } + + @Test + void testPopulatedDataWinsOverJsonNullText() throws JsonProcessingException { + String json = """ + {"text": null, "data": {"answer": "42"}} + """; + Part part = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(DataPart.class, part); + @SuppressWarnings("unchecked") + Map data = (Map) ((DataPart) part).data(); + assertEquals("42", data.get("answer")); + } + + @Test + void testDeliberatelyEmptyTextPartDecodes() throws JsonProcessingException { + String json = """ + {"text": ""} + """; + Part part = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(TextPart.class, part); + assertEquals("", ((TextPart) part).text()); + } + + @Test + void testEmptyTextPartRoundTrips() throws JsonProcessingException { + String json = JsonUtil.toJson(new TextPart("")); + Part part = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(TextPart.class, part); + assertEquals("", ((TextPart) part).text()); + } + + @Test + void testDataPartWithEmptyStringPayloadDecodes() throws JsonProcessingException { + String json = """ + {"data": ""} + """; + Part part = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(DataPart.class, part); + assertEquals("", ((DataPart) part).data()); + } + + /** Nothing distinguishes the intended content when every key is an empty string, so order decides. */ + @Test + void testAllEmptyContentKeysFallBackToTheFirst() throws JsonProcessingException { + String json = """ + {"text": "", "data": ""} + """; + Part part = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(TextPart.class, part); + assertEquals("", ((TextPart) part).text()); + } + + @Test + void testNonStringContentKeyIsRejected() { + String json = """ + {"text": "", "raw": {"nested": 1}} + """; + assertThrows(JsonProcessingException.class, () -> JsonUtil.fromJson(json, Part.class)); + } +}