From 1b9c521b6d878c7211db0bfbcb860b5adbb51527 Mon Sep 17 00:00:00 2001 From: Emmanuel Hugonnet Date: Wed, 4 Mar 2026 11:28:10 +0100 Subject: [PATCH] fix!: rewrite PartTypeAdapter to use flat JSON format for Part serialization Replace the nested "file" wrapper with flat fields (raw, url, filename, mediaType) aligned with the proto Part message schema and A2A spec. Signed-off-by: Emmanuel Hugonnet --- .../io/a2a/jsonrpc/common/json/JsonUtil.java | 90 ++++-- .../common/json/TaskSerializationTest.java | 20 +- .../java/io/a2a/grpc/utils/JSONRPCUtils.java | 4 +- .../a2a/grpc/utils/PartTypeAdapterTest.java | 289 ++++++++++++++++++ 4 files changed, 362 insertions(+), 41 deletions(-) create mode 100644 spec-grpc/src/test/java/io/a2a/grpc/utils/PartTypeAdapterTest.java diff --git a/jsonrpc-common/src/main/java/io/a2a/jsonrpc/common/json/JsonUtil.java b/jsonrpc-common/src/main/java/io/a2a/jsonrpc/common/json/JsonUtil.java index b301dc98c..734f3043a 100644 --- a/jsonrpc-common/src/main/java/io/a2a/jsonrpc/common/json/JsonUtil.java +++ b/jsonrpc-common/src/main/java/io/a2a/jsonrpc/common/json/JsonUtil.java @@ -13,7 +13,6 @@ import static io.a2a.spec.A2AErrorCodes.TASK_NOT_FOUND_ERROR_CODE; import static io.a2a.spec.A2AErrorCodes.UNSUPPORTED_OPERATION_ERROR_CODE; import static io.a2a.spec.DataPart.DATA; -import static io.a2a.spec.FilePart.FILE; import static io.a2a.spec.TextPart.TEXT; import static java.lang.String.format; import static java.util.Collections.emptyMap; @@ -521,33 +520,58 @@ public static Map readMetadata(@Nullable String json) throws Jso */ static class PartTypeAdapter extends TypeAdapter> { - private static final Set VALID_KEYS = Set.of(TEXT, FILE, DATA); + private static final String RAW = "raw"; + private static final String URL = "url"; + private static final String FILENAME = "filename"; + private static final String MEDIA_TYPE = "mediaType"; + // The oneOf content-type discriminator keys in the flat JSON format. + // Exactly one must be present (and non-null) in each Part object. + private static final Set VALID_KEYS = Set.of(TEXT, RAW, URL, DATA); private static final Type MAP_TYPE = new TypeToken>(){}.getType(); // Create separate Gson instance without the Part adapter to avoid recursion private final Gson delegateGson = createBaseGsonBuilder().create(); + private void writeMetadata(JsonWriter out, @Nullable Map metadata) throws java.io.IOException { + if (metadata != null && !metadata.isEmpty()) { + out.name("metadata"); + delegateGson.toJson(metadata, MAP_TYPE, out); + } + } + + /** Writes a string field only when the value is non-null and non-empty. */ + private void writeNonEmpty(JsonWriter out, String name, String value) throws java.io.IOException { + if (!value.isEmpty()) { + out.name(name).value(value); + } + } + @Override public void write(JsonWriter out, Part value) throws java.io.IOException { if (value == null) { out.nullValue(); return; } - // Write wrapper object with member name as discriminator out.beginObject(); if (value instanceof TextPart textPart) { - // TextPart: { "text": "value" } - direct string value - out.name(TEXT); - out.value(textPart.text()); - JsonUtil.writeMetadata(out, textPart.metadata()); + out.name(TEXT).value(textPart.text()); + writeMetadata(out, textPart.metadata()); } else if (value instanceof FilePart filePart) { - // FilePart: { "file": {...} } - out.name(FILE); - delegateGson.toJson(filePart.file(), FileContent.class, out); - JsonUtil.writeMetadata(out, filePart.metadata()); + if (filePart.file() instanceof FileWithBytes withBytes) { + out.name(RAW).value(withBytes.bytes()); + writeNonEmpty(out, FILENAME, withBytes.name()); + writeNonEmpty(out, MEDIA_TYPE, withBytes.mimeType()); + } else if (filePart.file() instanceof FileWithUri withUri) { + out.name(URL).value(withUri.uri()); + writeNonEmpty(out, FILENAME, withUri.name()); + writeNonEmpty(out, MEDIA_TYPE, withUri.mimeType()); + } else { + throw new JsonSyntaxException("Unknown FileContent subclass: " + filePart.file().getClass().getName()); + } + writeMetadata(out, filePart.metadata()); + } else if (value instanceof DataPart dataPart) { - // DataPart: { "data": } out.name(DATA); delegateGson.toJson(dataPart.data(), Object.class, out); JsonUtil.writeMetadata(out, dataPart.metadata()); @@ -566,7 +590,6 @@ Part read(JsonReader in) throws java.io.IOException { return null; } - // Read the JSON as a tree to inspect the member name discriminator com.google.gson.JsonElement jsonElement = com.google.gson.JsonParser.parseReader(in); if (!jsonElement.isJsonObject()) { throw new JsonSyntaxException("Part must be a JSON object"); @@ -576,34 +599,47 @@ Part read(JsonReader in) throws java.io.IOException { // Extract metadata if present Map metadata = JsonUtil.readMetadata(jsonObject); - - // Check for member name discriminators (v1.0 protocol) Set keys = jsonObject.keySet(); - if (keys.size() < 1 || keys.size() > 2) { - throw new JsonSyntaxException(format("Part object must have one content key from %s and optionally 'metadata' (found: %s)", VALID_KEYS, keys)); - } - // Find the discriminator (should be one of TEXT, FILE, DATA) + // 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() .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 FILE -> new FilePart(delegateGson.fromJson(jsonObject.get(FILE), FileContent.class), metadata); + case RAW -> new FilePart(new FileWithBytes( + stringOrEmpty(jsonObject, MEDIA_TYPE), + stringOrEmpty(jsonObject, FILENAME), + jsonObject.get(RAW).getAsString()), metadata); + case URL -> new FilePart(new FileWithUri( + stringOrEmpty(jsonObject, MEDIA_TYPE), + stringOrEmpty(jsonObject, FILENAME), + jsonObject.get(URL).getAsString()), metadata); case DATA -> { - // DataPart supports any JSON value: object, array, primitive, or null - Object data = delegateGson.fromJson( - jsonObject.get(DATA), - Object.class - ); + Object data = delegateGson.fromJson(jsonObject.get(DATA), Object.class); yield new DataPart(data, metadata); } - default -> - throw new JsonSyntaxException(format("Part must have one of: %s (found: %s)", VALID_KEYS, discriminator)); + default -> throw new JsonSyntaxException(format("Part must have one of: %s (found: %s)", VALID_KEYS, discriminator)); }; } + + /** 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); + if (el == null || el.isJsonNull()) { + return ""; + } + return el.getAsString(); + } } /** diff --git a/jsonrpc-common/src/test/java/io/a2a/jsonrpc/common/json/TaskSerializationTest.java b/jsonrpc-common/src/test/java/io/a2a/jsonrpc/common/json/TaskSerializationTest.java index 9c0fd2610..ff47e746b 100644 --- a/jsonrpc-common/src/test/java/io/a2a/jsonrpc/common/json/TaskSerializationTest.java +++ b/jsonrpc-common/src/test/java/io/a2a/jsonrpc/common/json/TaskSerializationTest.java @@ -255,8 +255,8 @@ void testTaskWithFilePartBytes() throws JsonProcessingException { // Serialize String json = JsonUtil.toJson(task); - // Verify JSON contains file part data (v1.0 format uses member name "file", not "kind") - assertTrue(json.contains("\"file\"")); + // Verify JSON contains file part data in flat format (raw/filename/mediaType, not "file" wrapper) + assertTrue(json.contains("\"raw\"")); assertFalse(json.contains("\"kind\"")); assertTrue(json.contains("document.pdf")); assertTrue(json.contains("application/pdf")); @@ -492,11 +492,9 @@ void testDeserializeTaskWithFilePartBytesFromJson() throws JsonProcessingExcepti "artifactId": "file-artifact", "parts": [ { - "file": { - "mimeType": "application/pdf", - "name": "document.pdf", - "bytes": "base64encodeddata" - } + "raw": "base64encodeddata", + "filename": "document.pdf", + "mediaType": "application/pdf" } ] } @@ -532,11 +530,9 @@ void testDeserializeTaskWithFilePartUriFromJson() throws JsonProcessingException "artifactId": "uri-artifact", "parts": [ { - "file": { - "mimeType": "image/png", - "name": "photo.png", - "uri": "https://example.com/photo.png" - } + "url": "https://example.com/photo.png", + "filename": "photo.png", + "mediaType": "image/png" } ] } diff --git a/spec-grpc/src/main/java/io/a2a/grpc/utils/JSONRPCUtils.java b/spec-grpc/src/main/java/io/a2a/grpc/utils/JSONRPCUtils.java index 0f6d3ed19..6a0f4525a 100644 --- a/spec-grpc/src/main/java/io/a2a/grpc/utils/JSONRPCUtils.java +++ b/spec-grpc/src/main/java/io/a2a/grpc/utils/JSONRPCUtils.java @@ -576,7 +576,7 @@ public static String toJsonRPCRequest(@Nullable String requestId, String method, output.name("method").value(method); } if (payload != null) { - String resultValue = JsonFormat.printer().includingDefaultValueFields().omittingInsignificantWhitespace().print(payload); + String resultValue = JsonFormat.printer().alwaysPrintFieldsWithNoPresence().omittingInsignificantWhitespace().print(payload); output.name("params").jsonValue(resultValue); } output.endObject(); @@ -599,7 +599,7 @@ public static String toJsonRPCResultResponse(Object requestId, com.google.protob output.name("id").value(number.longValue()); } } - String resultValue = JsonFormat.printer().includingDefaultValueFields().omittingInsignificantWhitespace().print(builder); + String resultValue = JsonFormat.printer().alwaysPrintFieldsWithNoPresence().omittingInsignificantWhitespace().print(builder); output.name("result").jsonValue(resultValue); output.endObject(); return result.toString(); diff --git a/spec-grpc/src/test/java/io/a2a/grpc/utils/PartTypeAdapterTest.java b/spec-grpc/src/test/java/io/a2a/grpc/utils/PartTypeAdapterTest.java new file mode 100644 index 000000000..4ed933db6 --- /dev/null +++ b/spec-grpc/src/test/java/io/a2a/grpc/utils/PartTypeAdapterTest.java @@ -0,0 +1,289 @@ +package io.a2a.grpc.utils; + +import io.a2a.jsonrpc.common.json.JsonProcessingException; +import io.a2a.jsonrpc.common.json.JsonUtil; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import com.google.protobuf.ByteString; +import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.Value; +import com.google.protobuf.util.JsonFormat; +import com.google.protobuf.util.Structs; +import io.a2a.grpc.mapper.A2ACommonFieldMapper; +import java.util.List; +import java.util.Map; + +import io.a2a.spec.DataPart; +import io.a2a.spec.FilePart; +import io.a2a.spec.FileWithBytes; +import io.a2a.spec.FileWithUri; +import io.a2a.spec.Part; +import io.a2a.spec.TextPart; +import java.util.Base64; +import org.junit.jupiter.api.Test; + + +public class PartTypeAdapterTest { + + // ------------------------------------------------------------------------- + // TextPart + // ------------------------------------------------------------------------- + + @Test + public void shouldSerializeTextPart() throws JsonProcessingException { + TextPart part = new TextPart("Hello, world!"); + String json = JsonUtil.toJson(part); + assertEquals("{\"text\":\"Hello, world!\"}", json); + } + + @Test + public void shouldSerializeTextPartWithMetadata() throws JsonProcessingException { + TextPart part = new TextPart("Bonjour!", Map.of("language", "fr")); + String json = JsonUtil.toJson(part); + // Verify the round-trip to avoid ordering issues + Part deserialized = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(TextPart.class, deserialized); + TextPart result = (TextPart) deserialized; + assertEquals("Bonjour!", result.text()); + assertEquals("fr", result.metadata().get("language")); + } + + @Test + public void shouldDeserializeTextPart() throws JsonProcessingException, InvalidProtocolBufferException { + io.a2a.grpc.Part.Builder builder = io.a2a.grpc.Part.newBuilder(); + builder.setText("Hello, world!"); + String json = JsonFormat.printer().alwaysPrintFieldsWithNoPresence().omittingInsignificantWhitespace().print(builder); + Partpart = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(TextPart.class, part); + TextPart textPart = (TextPart) part; + assertEquals("Hello, world!", textPart.text()); + assertNotNull(textPart.metadata()); + assertEquals(0, textPart.metadata().size()); + } + + @Test + public void shouldDeserializeTextPartWithMetadata() throws JsonProcessingException, InvalidProtocolBufferException { + io.a2a.grpc.Part.Builder builder = io.a2a.grpc.Part.newBuilder(); + builder.setText("Hi"); + builder.setMetadata(A2ACommonFieldMapper.INSTANCE.metadataToProto(Map.of("key", "value"))); + String json = JsonFormat.printer().alwaysPrintFieldsWithNoPresence().omittingInsignificantWhitespace().print(builder); + Part part = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(TextPart.class, part); + TextPart textPart = (TextPart) part; + assertEquals("Hi", textPart.text()); + assertEquals("value", textPart.metadata().get("key")); + } + + @Test + public void shouldRoundTripTextPart() throws JsonProcessingException { + TextPart original = new TextPart("round-trip"); + String json = JsonUtil.toJson(original); + Part deserialized = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(TextPart.class, deserialized); + assertEquals(original.text(), ((TextPart) deserialized).text()); + } + + // ------------------------------------------------------------------------- + // FilePart – FileWithBytes + // ------------------------------------------------------------------------- + + @Test + public void shouldSerializeFilePartWithBytes() throws JsonProcessingException { + FilePart part = new FilePart(new FileWithBytes("image/png", "diagram.png", "abc12w==")); + String json = JsonUtil.toJson(part); + // Gson HTML-escapes '=' to '\u003d'; verify via round-trip + Part deserialized = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(FilePart.class, deserialized); + FileWithBytes result = (FileWithBytes) ((FilePart) deserialized).file(); + assertEquals("image/png", result.mimeType()); + assertEquals("diagram.png", result.name()); + assertEquals("abc12w==", result.bytes()); + } + + @Test + public void shouldDeserializeFilePartWithBytes() throws JsonProcessingException, InvalidProtocolBufferException { + io.a2a.grpc.Part.Builder builder = io.a2a.grpc.Part.newBuilder(); + builder.setFilename("diagram.png").setMediaType("image/png").setRaw(ByteString.copyFrom(Base64.getDecoder().decode("abc12w=="))); + builder.setMetadata(A2ACommonFieldMapper.INSTANCE.metadataToProto(Map.of("key", "value"))); + String json = JsonFormat.printer().alwaysPrintFieldsWithNoPresence().omittingInsignificantWhitespace().print(builder); + Part part = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(FilePart.class, part); + FilePart filePart = (FilePart) part; + assertInstanceOf(FileWithBytes.class, filePart.file()); + FileWithBytes fileWithBytes = (FileWithBytes) filePart.file(); + assertEquals("image/png", fileWithBytes.mimeType()); + assertEquals("diagram.png", fileWithBytes.name()); + // Proto's JsonFormat encodes bytes as canonical base64 (zero trailing-padding bits), + // so "abc123==" (non-canonical) becomes "abc12w==" after the proto roundtrip. + assertEquals("abc12w==", fileWithBytes.bytes()); + assertEquals("value", filePart.metadata().get("key")); + + } + + @Test + public void shouldRoundTripFilePartWithBytes() throws JsonProcessingException { + FilePart original = new FilePart(new FileWithBytes("application/pdf", "report.pdf", "AAEC")); + String json = JsonUtil.toJson(original); + Part deserialized = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(FilePart.class, deserialized); + FilePart result = (FilePart) deserialized; + assertInstanceOf(FileWithBytes.class, result.file()); + FileWithBytes bytes = (FileWithBytes) result.file(); + assertEquals("application/pdf", bytes.mimeType()); + assertEquals("report.pdf", bytes.name()); + assertEquals("AAEC", bytes.bytes()); + } + + // ------------------------------------------------------------------------- + // FilePart – FileWithUri + // ------------------------------------------------------------------------- + + @Test + public void shouldSerializeFilePartWithUri() throws JsonProcessingException { + FilePart part = new FilePart(new FileWithUri("image/png", "photo.png", "https://example.com/photo.png")); + String json = JsonUtil.toJson(part); + // Verify the serialized JSON can be deserialized correctly (round-trip) + Part deserialized = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(FilePart.class, deserialized); + FileWithUri result = (FileWithUri) ((FilePart) deserialized).file(); + assertEquals("image/png", result.mimeType()); + assertEquals("photo.png", result.name()); + assertEquals("https://example.com/photo.png", result.uri()); + } + + @Test + public void shouldDeserializeFilePartWithUri() throws JsonProcessingException, InvalidProtocolBufferException { + io.a2a.grpc.Part.Builder builder = io.a2a.grpc.Part.newBuilder(); + builder.setFilename("photo.png").setMediaType("image/png").setUrl("https://example.com/photo.png"); + builder.setMetadata(A2ACommonFieldMapper.INSTANCE.metadataToProto(Map.of("key", "value"))); + String json = JsonFormat.printer().alwaysPrintFieldsWithNoPresence().omittingInsignificantWhitespace().print(builder); + Part part = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(FilePart.class, part); + FilePart filePart = (FilePart) part; + assertInstanceOf(FileWithUri.class, filePart.file()); + FileWithUri fileWithUri = (FileWithUri) filePart.file(); + assertEquals("image/png", fileWithUri.mimeType()); + assertEquals("photo.png", fileWithUri.name()); + assertEquals("https://example.com/photo.png", fileWithUri.uri()); + assertEquals("value", filePart.metadata().get("key")); + } + + @Test + public void shouldRoundTripFilePartWithUri() throws JsonProcessingException { + FilePart original = new FilePart(new FileWithUri("text/plain", "notes.txt", "https://example.com/notes.txt")); + String json = JsonUtil.toJson(original); + Part deserialized = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(FilePart.class, deserialized); + FilePart result = (FilePart) deserialized; + assertInstanceOf(FileWithUri.class, result.file()); + FileWithUri uri = (FileWithUri) result.file(); + assertEquals("text/plain", uri.mimeType()); + assertEquals("notes.txt", uri.name()); + assertEquals("https://example.com/notes.txt", uri.uri()); + } + + @Test + public void shouldRoundTripFilePartWithMetadata() throws JsonProcessingException { + FilePart original = new FilePart( + new FileWithUri("image/jpeg", "pic.jpg", "https://example.com/pic.jpg"), + Map.of("source", "camera")); + String json = JsonUtil.toJson(original); + Part deserialized = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(FilePart.class, deserialized); + FilePart result = (FilePart) deserialized; + assertEquals("camera", result.metadata().get("source")); + } + + // ------------------------------------------------------------------------- + // DataPart + // ------------------------------------------------------------------------- + + @Test + public void shouldSerializeDataPartWithObject() throws JsonProcessingException { + DataPart part = new DataPart(Map.of("status", "ok")); + String json = JsonUtil.toJson(part); + // Verify round-trip to avoid ordering issues with map serialization + Part deserialized = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(DataPart.class, deserialized); + @SuppressWarnings("unchecked") + Map data = (Map) ((DataPart) deserialized).data(); + assertEquals("ok", data.get("status")); + } + + @Test + public void shouldDeserializeDataPartWithObject() throws JsonProcessingException, InvalidProtocolBufferException { + io.a2a.grpc.Part.Builder builder = io.a2a.grpc.Part.newBuilder(); + builder.setData(Value.newBuilder().setStructValue(Structs.of("count", Value.newBuilder().setNumberValue(42).build(), "label", Value.newBuilder().setStringValue("items").build()))); + builder.setMetadata(A2ACommonFieldMapper.INSTANCE.metadataToProto(Map.of("key", "value"))); + String json = JsonFormat.printer().alwaysPrintFieldsWithNoPresence().omittingInsignificantWhitespace().print(builder); + Part part = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(DataPart.class, part); + @SuppressWarnings("unchecked") + Map data = (Map) ((DataPart) part).data(); + assertEquals(42.0, data.get("count")); + assertEquals("items", data.get("label")); + } + + @Test + public void shouldSerializeDataPartWithArray() throws JsonProcessingException { + DataPart part = new DataPart(List.of("a", "b", "c")); + String json = JsonUtil.toJson(part); + assertEquals("{\"data\":[\"a\",\"b\",\"c\"]}", json); + } + + @Test + public void shouldDeserializeDataPartWithArray() throws JsonProcessingException { + String json = "{\"data\":[\"a\",\"b\",\"c\"]}"; + Part part = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(DataPart.class, part); + @SuppressWarnings("unchecked") + List data = (List) ((DataPart) part).data(); + assertEquals(List.of("a", "b", "c"), data); + } + + @Test + public void shouldSerializeDataPartWithString() throws JsonProcessingException { + DataPart part = new DataPart("hello"); + String json = JsonUtil.toJson(part); + assertEquals("{\"data\":\"hello\"}", json); + } + + @Test + public void shouldDeserializeDataPartWithString() throws JsonProcessingException { + String json = "{\"data\":\"hello\"}"; + Part part = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(DataPart.class, part); + assertEquals("hello", ((DataPart) part).data()); + } + + @Test + public void shouldSerializeDataPartWithNumber() throws JsonProcessingException { + DataPart part = new DataPart(42L); + String json = JsonUtil.toJson(part); + assertEquals("{\"data\":42}", json); + } + + @Test + public void shouldDeserializeDataPartWithNumber() throws JsonProcessingException { + String json = "{\"data\":42}"; + Part part = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(DataPart.class, part); + assertEquals(42L, ((DataPart) part).data()); + } + + @Test + public void shouldRoundTripDataPartWithMetadata() throws JsonProcessingException { + DataPart original = new DataPart(Map.of("key", "val"), Map.of("version", "1")); + String json = JsonUtil.toJson(original); + Part deserialized = JsonUtil.fromJson(json, Part.class); + assertInstanceOf(DataPart.class, deserialized); + DataPart result = (DataPart) deserialized; + assertEquals("1", result.metadata().get("version")); + @SuppressWarnings("unchecked") + Map data = (Map) result.data(); + assertEquals("val", data.get("key")); + } +}