From 54a1516af0de0371597d26ae99d4d615a20bef87 Mon Sep 17 00:00:00 2001 From: "Rui L. Lopes" Date: Wed, 12 Aug 2026 12:25:58 +0100 Subject: [PATCH] fix(jsonrpc): prefer a populated content key when decoding a Part The discriminator scan took the first content key that was merely non-null, so a producer that emits every key of the flattened oneOf with the unset ones as "" lost its payload to whichever empty placeholder came first on the wire. The comment above it already promised "skipping null/empty values"; only null was ever skipped. This fixes #1062 --- .../sdk/jsonrpc/common/json/JsonUtil.java | 50 ++++-- .../common/json/PartSerializationTest.java | 161 ++++++++++++++++++ 2 files changed, 195 insertions(+), 16 deletions(-) create mode 100644 jsonrpc-common/src/test/java/org/a2aproject/sdk/jsonrpc/common/json/PartSerializationTest.java 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)); + } +}