From 0e27ed884363695e34ae67700652410f6f3a49af Mon Sep 17 00:00:00 2001 From: yuzhou Date: Tue, 13 Oct 2026 18:28:31 +0800 Subject: [PATCH] fix(compat-0.3): preserve explicit JSON nulls when converting Struct to Java objects An explicit JSON null inside a Struct made the v0.3 compatibility layer throw instead of round-tripping the value. `FromProto.struct` collected its fields with `Collectors.toMap`, whose accumulator rejects null values, and `FromProto.value` threw `InvalidRequestError_v0_3` for `NULL_VALUE` instead of returning null. The domain records then copied the result with `Map.copyOf`, which rejects null values as well, so a null survived none of the three layers. A message part whose structured data contains a null field anywhere in its object tree therefore failed to deserialize on the 0.3 clients, and the caller never saw the task or message content. This is the same defect the 1.x mapper fixed for `A2ACommonFieldMapper`; the 0.3 path is a separate implementation and was not covered by it. `FromProto.struct` now builds the map directly and `value` returns null for `NULL_VALUE` and for an unset kind, matching the 1.x mapper. The 0.3 spec records copy JSON maps with a new null-preserving `Utils_v0_3.unmodifiableShallowMap`, replacing `Map.copyOf` on every metadata, data, and security-schemes field. This fixes #1207 --- .../compat03/grpc/utils/ProtoUtils_v0_3.java | 8 +- .../FromProtoExplicitNull_v0_3_Test.java | 100 ++++++++++++++++++ .../sdk/compat03/spec/AgentCard_v0_3.java | 3 +- .../sdk/compat03/spec/Artifact_v0_3.java | 3 +- .../sdk/compat03/spec/DataPart_v0_3.java | 5 +- .../sdk/compat03/spec/FilePart_v0_3.java | 3 +- .../sdk/compat03/spec/Message_v0_3.java | 3 +- .../spec/TaskArtifactUpdateEvent_v0_3.java | 3 +- .../spec/TaskStatusUpdateEvent_v0_3.java | 3 +- .../sdk/compat03/spec/Task_v0_3.java | 3 +- .../sdk/compat03/spec/TextPart_v0_3.java | 3 +- .../sdk/compat03/util/Utils_v0_3.java | 32 ++++++ 12 files changed, 156 insertions(+), 13 deletions(-) create mode 100644 compat-0.3/spec-grpc/src/test/java/org/a2aproject/sdk/compat03/grpc/utils/FromProtoExplicitNull_v0_3_Test.java diff --git a/compat-0.3/spec-grpc/src/main/java/org/a2aproject/sdk/compat03/grpc/utils/ProtoUtils_v0_3.java b/compat-0.3/spec-grpc/src/main/java/org/a2aproject/sdk/compat03/grpc/utils/ProtoUtils_v0_3.java index 7a4e6f284..36befb019 100644 --- a/compat-0.3/spec-grpc/src/main/java/org/a2aproject/sdk/compat03/grpc/utils/ProtoUtils_v0_3.java +++ b/compat-0.3/spec-grpc/src/main/java/org/a2aproject/sdk/compat03/grpc/utils/ProtoUtils_v0_3.java @@ -6,6 +6,7 @@ import java.time.OffsetDateTime; import java.time.ZoneOffset; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -1009,8 +1010,9 @@ private static DataPart_v0_3 dataPart(org.a2aproject.sdk.compat03.grpc.DataPartO if (struct == null || struct.getFieldsCount() == 0) { return null; } - return struct.getFieldsMap().entrySet().stream() - .collect(Collectors.toMap(Map.Entry::getKey, e -> value(e.getValue()))); + Map map = new HashMap<>(); + struct.getFieldsMap().forEach((key, value) -> map.put(key, value(value))); + return map; } private static @Nullable Object value(Value value) { @@ -1029,7 +1031,7 @@ private static DataPart_v0_3 dataPart(org.a2aproject.sdk.compat03.grpc.DataPartO return value.getStringValue(); case NULL_VALUE: default: - throw new InvalidRequestError_v0_3(); + return null; } } } diff --git a/compat-0.3/spec-grpc/src/test/java/org/a2aproject/sdk/compat03/grpc/utils/FromProtoExplicitNull_v0_3_Test.java b/compat-0.3/spec-grpc/src/test/java/org/a2aproject/sdk/compat03/grpc/utils/FromProtoExplicitNull_v0_3_Test.java new file mode 100644 index 000000000..58f66474b --- /dev/null +++ b/compat-0.3/spec-grpc/src/test/java/org/a2aproject/sdk/compat03/grpc/utils/FromProtoExplicitNull_v0_3_Test.java @@ -0,0 +1,100 @@ +package org.a2aproject.sdk.compat03.grpc.utils; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Map; + +import com.google.protobuf.Struct; +import com.google.protobuf.util.JsonFormat; + +import org.a2aproject.sdk.compat03.spec.DataPart_v0_3; +import org.a2aproject.sdk.compat03.spec.Part_v0_3; +import org.junit.jupiter.api.Test; + +class FromProtoExplicitNull_v0_3_Test { + + private static Struct struct(String json) throws Exception { + Struct.Builder builder = Struct.newBuilder(); + JsonFormat.parser().merge(json, builder); + return builder.build(); + } + + @Test + void partDataPartWithExplicitNullField() throws Exception { + var dataPart = org.a2aproject.sdk.compat03.grpc.DataPart.newBuilder() + .setData(struct("{\"some_optional_field\":null,\"other_field\":\"value\"}")) + .build(); + var part = org.a2aproject.sdk.compat03.grpc.Part.newBuilder().setData(dataPart).build(); + + Part_v0_3 result = ProtoUtils_v0_3.FromProto.part(part); + + assertTrue(result instanceof DataPart_v0_3); + Map data = ((DataPart_v0_3) result).data(); + assertNotNull(data); + assertEquals(2, data.size()); + assertTrue(data.containsKey("some_optional_field")); + assertNull(data.get("some_optional_field")); + assertEquals("value", data.get("other_field")); + } + + @Test + void partMetadataWithExplicitNullField() throws Exception { + var textPart = org.a2aproject.sdk.compat03.grpc.Part.newBuilder() + .setText("hello") + .setMetadata(struct("{\"nullable\":null,\"source\":\"ID\"}")) + .build(); + + Part_v0_3 result = ProtoUtils_v0_3.FromProto.part(textPart); + + Map metadata = ((org.a2aproject.sdk.compat03.spec.TextPart_v0_3) result).metadata(); + assertNotNull(metadata); + assertEquals(2, metadata.size()); + assertTrue(metadata.containsKey("nullable")); + assertNull(metadata.get("nullable")); + assertEquals("ID", metadata.get("source")); + } + + @Test + void partDataPartWithNestedNullAndNullInList() throws Exception { + var dataPart = org.a2aproject.sdk.compat03.grpc.DataPart.newBuilder() + .setData(struct("{\"nested\":{\"optional\":null},\"items\":[null,1]}")) + .build(); + var part = org.a2aproject.sdk.compat03.grpc.Part.newBuilder().setData(dataPart).build(); + + Part_v0_3 result = ProtoUtils_v0_3.FromProto.part(part); + + Map data = ((DataPart_v0_3) result).data(); + assertNotNull(data.get("nested")); + assertNull(((Map) data.get("nested")).get("optional")); + } + + @Test + void taskWithDataPartThatHasExplicitNullField() throws Exception { + var message = org.a2aproject.sdk.compat03.grpc.Message.newBuilder() + .setRole(org.a2aproject.sdk.compat03.grpc.Role.ROLE_AGENT) + .setMessageId("message-1") + .addContent(org.a2aproject.sdk.compat03.grpc.Part.newBuilder() + .setData(org.a2aproject.sdk.compat03.grpc.DataPart.newBuilder() + .setData(struct("{\"some_optional_field\":null,\"other_field\":\"value\"}")))) + .build(); + var task = org.a2aproject.sdk.compat03.grpc.Task.newBuilder() + .setId("task-1") + .setContextId("context-1") + .setStatus(org.a2aproject.sdk.compat03.grpc.TaskStatus.newBuilder() + .setState(org.a2aproject.sdk.compat03.grpc.TaskState.TASK_STATE_COMPLETED)) + .addHistory(message) + .build(); + + var result = ProtoUtils_v0_3.FromProto.task(task); + + assertEquals("task-1", result.id()); + assertEquals(1, result.history().size()); + var dataPart = (DataPart_v0_3) result.history().get(0).parts().get(0); + assertTrue(dataPart.data().containsKey("some_optional_field")); + assertNull(dataPart.data().get("some_optional_field")); + assertEquals("value", dataPart.data().get("other_field")); + } +} diff --git a/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/AgentCard_v0_3.java b/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/AgentCard_v0_3.java index d5ab8010d..f11d20c57 100644 --- a/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/AgentCard_v0_3.java +++ b/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/AgentCard_v0_3.java @@ -4,6 +4,7 @@ import java.util.List; import java.util.Map; +import org.a2aproject.sdk.compat03.util.Utils_v0_3; import org.a2aproject.sdk.util.Assert; /** @@ -82,7 +83,7 @@ public Builder(AgentCard_v0_3 card) { this.defaultOutputModes = card.defaultOutputModes != null ? new ArrayList<>(card.defaultOutputModes) : null; this.skills = card.skills != null ? new ArrayList<>(card.skills) : null; this.supportsAuthenticatedExtendedCard = card.supportsAuthenticatedExtendedCard; - this.securitySchemes = card.securitySchemes != null ? Map.copyOf(card.securitySchemes) : null; + this.securitySchemes = Utils_v0_3.unmodifiableNullableShallowMap(card.securitySchemes); this.security = card.security != null ? new ArrayList<>(card.security) : null; this.iconUrl = card.iconUrl; this.additionalInterfaces = card.additionalInterfaces != null ? new ArrayList<>(card.additionalInterfaces) : null; diff --git a/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/Artifact_v0_3.java b/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/Artifact_v0_3.java index e10c267c1..8fcc67aef 100644 --- a/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/Artifact_v0_3.java +++ b/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/Artifact_v0_3.java @@ -3,6 +3,7 @@ import java.util.List; import java.util.Map; +import org.a2aproject.sdk.compat03.util.Utils_v0_3; import org.a2aproject.sdk.util.Assert; /** @@ -66,7 +67,7 @@ public Builder parts(Part_v0_3... parts) { } public Builder metadata(Map metadata) { - this.metadata = Map.copyOf(metadata); + this.metadata = Utils_v0_3.unmodifiableShallowMap(metadata); return this; } diff --git a/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/DataPart_v0_3.java b/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/DataPart_v0_3.java index f18d784a3..de5bd4a20 100644 --- a/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/DataPart_v0_3.java +++ b/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/DataPart_v0_3.java @@ -2,6 +2,7 @@ import java.util.Map; +import org.a2aproject.sdk.compat03.util.Utils_v0_3; import org.a2aproject.sdk.util.Assert; import org.jspecify.annotations.Nullable; @@ -17,8 +18,8 @@ public DataPart_v0_3(Map data, @Nullable Map met if (kind != Kind.DATA) { throw new IllegalArgumentException("Invalid DataPart kind: " + kind); } - this.data = Map.copyOf(data); - this.metadata = metadata == null ? Map.of() : Map.copyOf(metadata); + this.data = Utils_v0_3.unmodifiableShallowMap(data); + this.metadata = metadata == null ? Map.of() : Utils_v0_3.unmodifiableShallowMap(metadata); this.kind = kind; } diff --git a/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/FilePart_v0_3.java b/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/FilePart_v0_3.java index d3c89abdf..b89d4b8bd 100644 --- a/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/FilePart_v0_3.java +++ b/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/FilePart_v0_3.java @@ -2,6 +2,7 @@ import java.util.Map; +import org.a2aproject.sdk.compat03.util.Utils_v0_3; import org.a2aproject.sdk.util.Assert; import org.jspecify.annotations.Nullable; @@ -19,7 +20,7 @@ public FilePart_v0_3 (FileContent_v0_3 file, @Nullable Map metad throw new IllegalArgumentException("Invalid FilePart kind: " + kind); } this.file = file; - this.metadata = metadata == null ? Map.of() : Map.copyOf(metadata); + this.metadata = metadata == null ? Map.of() : Utils_v0_3.unmodifiableShallowMap(metadata); this.kind = kind; } diff --git a/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/Message_v0_3.java b/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/Message_v0_3.java index 474d47aff..51e015f8a 100644 --- a/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/Message_v0_3.java +++ b/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/Message_v0_3.java @@ -4,6 +4,7 @@ import java.util.Map; import java.util.UUID; +import org.a2aproject.sdk.compat03.util.Utils_v0_3; import org.a2aproject.sdk.util.Assert; import org.jspecify.annotations.Nullable; @@ -42,7 +43,7 @@ public Message_v0_3(Role role, List> parts, @Nullable String messag this.contextId = contextId; this.taskId = taskId; this.referenceTaskIds = referenceTaskIds != null ? List.copyOf(referenceTaskIds) : null; - this.metadata = metadata != null ? Map.copyOf(metadata) : null; + this.metadata = Utils_v0_3.unmodifiableNullableShallowMap(metadata); this.extensions = extensions != null ? List.copyOf(extensions) : null; } diff --git a/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/TaskArtifactUpdateEvent_v0_3.java b/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/TaskArtifactUpdateEvent_v0_3.java index 64c8dc0c5..2de4fdc0c 100644 --- a/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/TaskArtifactUpdateEvent_v0_3.java +++ b/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/TaskArtifactUpdateEvent_v0_3.java @@ -2,6 +2,7 @@ import java.util.Map; +import org.a2aproject.sdk.compat03.util.Utils_v0_3; import org.a2aproject.sdk.util.Assert; import org.jspecify.annotations.Nullable; @@ -35,7 +36,7 @@ public TaskArtifactUpdateEvent_v0_3 (String taskId, @Nullable Boolean append, @N this.lastChunk = lastChunk; this.artifact = artifact; this.contextId = contextId; - this.metadata = metadata != null ? Map.copyOf(metadata) : null; + this.metadata = Utils_v0_3.unmodifiableNullableShallowMap(metadata); } public TaskArtifactUpdateEvent_v0_3(String taskId, Artifact_v0_3 artifact, String contextId, diff --git a/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/TaskStatusUpdateEvent_v0_3.java b/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/TaskStatusUpdateEvent_v0_3.java index d527815b3..0a33ce448 100644 --- a/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/TaskStatusUpdateEvent_v0_3.java +++ b/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/TaskStatusUpdateEvent_v0_3.java @@ -3,6 +3,7 @@ import com.google.gson.annotations.SerializedName; import java.util.Map; +import org.a2aproject.sdk.compat03.util.Utils_v0_3; import org.a2aproject.sdk.util.Assert; import org.jspecify.annotations.Nullable; @@ -34,7 +35,7 @@ public TaskStatusUpdateEvent_v0_3 (String taskId, TaskStatus_v0_3 status, String this.status = status; this.contextId = contextId; this.isFinal = isFinal; - this.metadata = metadata != null ? Map.copyOf(metadata) : null; + this.metadata = Utils_v0_3.unmodifiableNullableShallowMap(metadata); } public TaskStatusUpdateEvent_v0_3(String taskId, TaskStatus_v0_3 status, String contextId, boolean isFinal, diff --git a/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/Task_v0_3.java b/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/Task_v0_3.java index 3b0a28cdc..ea5e5c5fc 100644 --- a/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/Task_v0_3.java +++ b/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/Task_v0_3.java @@ -3,6 +3,7 @@ import java.util.List; import java.util.Map; +import org.a2aproject.sdk.compat03.util.Utils_v0_3; import org.a2aproject.sdk.util.Assert; import org.jspecify.annotations.Nullable; @@ -35,7 +36,7 @@ public Task_v0_3 (String id, String contextId, TaskStatus_v0_3 status, List artifacts, diff --git a/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/TextPart_v0_3.java b/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/TextPart_v0_3.java index 70f8563c7..5a02bfa78 100644 --- a/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/TextPart_v0_3.java +++ b/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/TextPart_v0_3.java @@ -2,6 +2,7 @@ import java.util.Map; +import org.a2aproject.sdk.compat03.util.Utils_v0_3; import org.a2aproject.sdk.util.Assert; import org.jspecify.annotations.Nullable; @@ -18,7 +19,7 @@ public TextPart_v0_3 (String text, @Nullable Map metadata, Kind throw new IllegalArgumentException("Invalid TextPart kind: " + kind); } this.text = text; - this.metadata = metadata == null ? Map.of() : Map.copyOf(metadata); + this.metadata = metadata == null ? Map.of() : Utils_v0_3.unmodifiableShallowMap(metadata); this.kind = kind; } diff --git a/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/util/Utils_v0_3.java b/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/util/Utils_v0_3.java index f12830974..c4a8a68af 100644 --- a/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/util/Utils_v0_3.java +++ b/compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/util/Utils_v0_3.java @@ -1,7 +1,10 @@ package org.a2aproject.sdk.compat03.util; import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import org.a2aproject.sdk.compat03.json.JsonProcessingException_v0_3; import org.a2aproject.sdk.compat03.json.JsonUtil_v0_3; @@ -50,6 +53,35 @@ public static T defaultIfNull(T value, T defaultValue) { return value; } + /** + * Creates an unmodifiable shallow copy of a non-null map while preserving null values. + *

+ * Intended for JSON-like maps such as {@code data}, {@code metadata} and {@code headers}, + * where an explicit JSON null is valid protocol content. It intentionally does not use + * {@link Map#copyOf(Map)}, which rejects null values. + * + * @param map the source map, must not be {@code null} + * @param the key type + * @param the value type + * @return an unmodifiable shallow copy preserving null values + */ + public static Map unmodifiableShallowMap(Map map) { + return Collections.unmodifiableMap(new HashMap<>(map)); + } + + /** + * Creates an unmodifiable shallow copy of a nullable map while preserving null values. + * + * @param map the source map, or {@code null} + * @param the key type + * @param the value type + * @return {@code null} if the source is {@code null}, otherwise an unmodifiable shallow copy + * preserving null values + */ + public static Map unmodifiableNullableShallowMap(Map map) { + return map == null ? null : unmodifiableShallowMap(map); + } + /** * Appends or updates an artifact in a task based on a {@link TaskArtifactUpdateEvent_v0_3}. *