Skip to content

fix: omit unset proto3 fields from JSON-RPC client requests #1286

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
WMK666666 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: omit unset proto3 fields from JSON-RPC client requests #1286

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...onrpc/src/test/java/org/a2aproject/sdk/client/transport/jsonrpc/JSONRPCTransportTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,14 @@ public void tearDown() {
public void testListTasksWithDefaultBuilder() throws Exception {
JsonObject params = listTasksAndGetRequestParams(ListTasksParams.builder().build());

assertEquals("", params.get("tenant").getAsString());
assertFalse(params.has("tenant"), "an unset tenant must be omitted instead of being sent as \"\"");
}

@Test
public void testListTasksWithDefaultConstructor() throws Exception {
JsonObject params = listTasksAndGetRequestParams(new ListTasksParams());

assertEquals("", params.get("tenant").getAsString());
assertFalse(params.has("tenant"), "an unset tenant must be omitted instead of being sent as \"\"");
}

@Test
Expand Down
59 changes: 11 additions & 48 deletions ...sport/jsonrpc/src/test/java/org/a2aproject/sdk/client/transport/jsonrpc/JsonMessages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,7 @@ public class JsonMessages {
{
"text":"tell me a joke"
}
],
"metadata":{

}
]
}
}
}""";
Expand Down Expand Up @@ -143,18 +140,12 @@ public class JsonMessages {
{
"text":"tell me a joke"
}
],
"metadata":{
}
]
},
"configuration":{
"acceptedOutputModes":[
"text"
],
"returnImmediately":false
},
"metadata":{

]
}
}
}""";
Expand Down Expand Up @@ -192,19 +183,12 @@ public class JsonMessages {
{
"text":"tell me a joke"
}
],
"metadata":{

}
]
},
"configuration":{
"acceptedOutputModes":[
"text"
],
"returnImmediately":false
},
"metadata":{

]
}
}
}""";
Expand Down Expand Up @@ -376,19 +360,12 @@ public class JsonMessages {
"url":"file:///path/to/image.jpg",
"mediaType":"image/jpeg"
}
],
"metadata":{

}
]
},
"configuration":{
"acceptedOutputModes":[
"text"
],
"returnImmediately":false
},
"metadata":{

]
}
}
}""";
Expand Down Expand Up @@ -443,19 +420,12 @@ public class JsonMessages {
"timestamp":"2024-01-15T10:30:00Z"
}
}
],
"metadata":{

}
]
},
"configuration":{
"acceptedOutputModes":[
"text"
],
"returnImmediately":false
},
"metadata":{

]
}
}
}""";
Expand Down Expand Up @@ -514,19 +484,12 @@ public class JsonMessages {
"labels":["Q1", "Q2", "Q3", "Q4"]
}
}
],
"metadata":{

}
]
},
"configuration":{
"acceptedOutputModes":[
"text"
],
"returnImmediately":false
},
"metadata":{

]
}
}
}""";
Expand Down
25 changes: 21 additions & 4 deletions spec-grpc/src/main/java/org/a2aproject/sdk/grpc/utils/JSONRPCUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,25 @@ public class JSONRPCUtils {
private static final Pattern EXTRACT_WRONG_TYPE = Pattern.compile("Expected (.*) but found \".*\"");
static final String ERROR_MESSAGE = "Invalid request content: %s. Please verify the request matches the expected schema for this method.";

/**
* Printer for client requests. Proto3 scalar fields without presence ({@code task_id},
* {@code context_id}, {@code tenant}, ...) are omitted when they are unset, which is what the
* REST client already does. Printing them as {@code ""} would tell the server the field is
* present and empty, and the two readings cannot be told apart again once the request is
* parsed.
*/
private static final JsonFormat.Printer REQUEST_PRINTER = JsonFormat.printer()
.omittingInsignificantWhitespace();

/**
* Printer for server responses. Responses keep printing fields without presence so that a
* client sees the full shape of the payload it is about to parse instead of having to infer
* defaults from missing keys.
*/
private static final JsonFormat.Printer RESPONSE_PRINTER = JsonFormat.printer()
.alwaysPrintFieldsWithNoPresence()
.omittingInsignificantWhitespace();

public static A2ARequest<?> parseRequestBody(String body, @Nullable String tenant) throws JsonMappingException, JsonProcessingException {
JsonElement jelement = JsonParser.parseString(body);
if (!jelement.isJsonObject()) {
Expand Down Expand Up @@ -581,8 +600,7 @@ public static String toJsonRPCRequest(@Nullable String requestId, String method,
output.name("method").value(method);
}
if (payload != null) {
String resultValue = ProtoJsonUtils.toJson(
JsonFormat.printer().alwaysPrintFieldsWithNoPresence().omittingInsignificantWhitespace(), payload);
String resultValue = ProtoJsonUtils.toJson(REQUEST_PRINTER, payload);
output.name("params").jsonValue(resultValue);
}
output.endObject();
Expand All @@ -599,8 +617,7 @@ public static String toJsonRPCResultResponse(@Nullable Object requestId, com.goo
output.beginObject();
output.name("jsonrpc").value("2.0");
JsonUtil.writeJsonRpcId(output, requestId);
String resultValue = ProtoJsonUtils.toJson(
JsonFormat.printer().alwaysPrintFieldsWithNoPresence().omittingInsignificantWhitespace(), builder);
String resultValue = ProtoJsonUtils.toJson(RESPONSE_PRINTER, builder);
output.name("result").jsonValue(resultValue);
output.endObject();
return result.toString();
Expand Down
62 changes: 62 additions & 0 deletions spec-grpc/src/test/java/org/a2aproject/sdk/grpc/utils/JSONRPCUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,68 @@ public void testToJsonRPCResultResponse_doesNotHtmlEscapeAngleBrackets() {
"JSON must not contain HTML-escaped '>' (\\u003e) but got: " + json);
}

@Test
public void testToJsonRPCRequest_omitsFieldsWithoutPresence() throws Exception {
// Issue #770: the client used to force every proto3 field with implicit presence into
// the payload as "" / [] / false, so a peer could no longer tell "not supplied" (mint a
// task id) from "supplied but empty" (reject, or echo an empty id back).
MessageSendParams params = new MessageSendParams(
Message.builder()
.role(Message.Role.ROLE_USER)
.parts(Collections.singletonList(new TextPart("hello")))
.messageId("message-1")
.build(),
null, null
);
org.a2aproject.sdk.grpc.SendMessageRequest protoRequest =
ProtoUtils.ToProto.sendMessageRequest(params);

String json = JSONRPCUtils.toJsonRPCRequest("test-id", SEND_MESSAGE_METHOD, protoRequest);
JsonObject paramsNode = JsonParser.parseString(json).getAsJsonObject().getAsJsonObject("params");
JsonObject message = paramsNode.getAsJsonObject("message");
JsonObject textPart = message.getAsJsonArray("parts").get(0).getAsJsonObject();

assertFalse(paramsNode.has("tenant"),
"unset 'tenant' must be omitted from the request, got: " + json);
assertFalse(message.has("taskId"),
"unset 'taskId' must be omitted from the request, got: " + json);
assertFalse(message.has("contextId"),
"unset 'contextId' must be omitted from the request, got: " + json);
assertFalse(message.has("extensions"),
"unset 'extensions' must be omitted from the request, got: " + json);
assertFalse(message.has("referenceTaskIds"),
"unset 'referenceTaskIds' must be omitted from the request, got: " + json);
assertFalse(textPart.has("filename"),
"unset 'filename' must be omitted from the request, got: " + json);
assertFalse(textPart.has("mediaType"),
"unset 'mediaType' must be omitted from the request, got: " + json);

SendMessageRequest roundTripped = assertInstanceOf(SendMessageRequest.class,
JSONRPCUtils.parseRequestBody(json, null));
assertEquals("message-1", roundTripped.getParams().message().messageId());
assertEquals("hello", ((TextPart) roundTripped.getParams().message().parts().get(0)).text());
}

@Test
public void testToJsonRPCResultResponse_stillPrintsFieldsWithoutPresence() {
// The split is deliberate: only the client request printer omits fields without
// presence. Responses keep printing them so a client sees the full payload shape.
Message message = Message.builder()
.role(Message.Role.ROLE_AGENT)
.parts(Collections.singletonList(new TextPart("hi")))
.messageId("message-1")
.build();
SendMessageResponse protoResponse = ProtoUtils.ToProto.taskOrMessage(message);

String json = JSONRPCUtils.toJsonRPCResultResponse("test-id", protoResponse);
JsonObject resultMessage = JsonParser.parseString(json).getAsJsonObject()
.getAsJsonObject("result").getAsJsonObject("message");

assertTrue(resultMessage.has("contextId"),
"the response printer must keep printing fields without presence, got: " + json);
assertEquals("", resultMessage.get("contextId").getAsString());
}

@Test
void parseGetExtendedAgentCardWithTenantParams() throws Exception {
String json = """
Expand Down
Loading