Skip to content

Commit 660ca7d

Browse files
committed
FIxing issues with configId on TaskPushNotificationConfig.
Signed-off-by: Emmanuel Hugonnet <[email protected]>
1 parent d2c5795 commit 660ca7d

File tree

5 files changed

+37
-18
lines changed

5 files changed

+37
-18
lines changed

client/transport/jsonrpc/src/test/java/io/a2a/client/transport/jsonrpc/JsonMessages.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ public class JsonMessages {
391391
"jsonrpc":"2.0",
392392
"method":"GetTaskPushNotificationConfig",
393393
"params":{
394-
"name":"tasks/de38c76d-d54c-436c-8b9f-4c2703648d64/pushNotificationConfigs"
394+
"name":"tasks/de38c76d-d54c-436c-8b9f-4c2703648d64"
395395
}
396396
}""";
397397

spec-grpc/src/main/java/io/a2a/grpc/mapper/ResourceNameParser.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,9 @@ public static String[] parseGetTaskPushNotificationConfigName(String resourceNam
7474
return new String[]{taskId, configId};
7575
}
7676
public static String defineGetTaskPushNotificationConfigName(String taskId, String configId) {
77-
String name= "tasks/" + taskId + "/pushNotificationConfigs";
77+
String name= "tasks/" + taskId ;
7878
if(configId != null && !configId.isBlank()) {
79-
name = name + '/' + configId;
79+
name = name + "/pushNotificationConfigs/" + configId;
8080
}
8181
return name;
8282
}

spec-grpc/src/main/java/io/a2a/grpc/mapper/SetTaskPushNotificationConfigMapper.java

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@
1313
* <p>
1414
* <b>Resource Name Handling:</b>
1515
* <ul>
16-
* <li>Extracts taskId from parent resource name (format: "tasks/{task_id}")</li>
17-
* <li>Fallback: Extracts from config.name if parent is blank</li>
18-
* <li>Overrides PushNotificationConfig.id with config_id from request</li>
16+
* <li>Extracts taskId from parent resource name (format: "tasks/{task_id}")</li>
17+
* <li>Fallback: Extracts from config.name if parent is blank</li>
18+
* <li>Overrides PushNotificationConfig.id with config_id from request</li>
1919
* </ul>
2020
* <p>
2121
* <b>Compile-Time Safety:</b> If the proto changes fields, MapStruct will fail to compile.
@@ -38,7 +38,7 @@ public interface SetTaskPushNotificationConfigMapper {
3838
@Mapping(target = "pushNotificationConfig", expression = "java(mapPushNotificationConfigWithId(request))")
3939
TaskPushNotificationConfig fromProto(SetTaskPushNotificationConfigRequest request);
4040

41-
/**
41+
/**
4242
* Converts SetTaskPushNotificationConfigRequest to domain TaskPushNotificationConfig.
4343
* <p>
4444
* Extracts taskId from parent resource name and maps PushNotificationConfig with
@@ -48,9 +48,10 @@ public interface SetTaskPushNotificationConfigMapper {
4848
* @return proto SetTaskPushNotificationConfigRequest
4949
*/
5050
@Mapping(target = "parent", expression = "java(ResourceNameParser.defineTaskName(config.taskId()))")
51-
@Mapping(target = "configId", expression = "java(config.pushNotificationConfig().id())")
51+
@Mapping(target = "configId", expression = "java(extractConfigId(config))")
5252
@Mapping(target = "config", expression = "java(mapPushNotificationConfig(config))")
5353
SetTaskPushNotificationConfigRequest toProto(TaskPushNotificationConfig config);
54+
5455
/**
5556
* Extracts the task ID from the parent resource name.
5657
* <p>
@@ -72,6 +73,19 @@ default String extractTaskId(SetTaskPushNotificationConfigRequest request) {
7273
return ResourceNameParser.extractParentId(parent);
7374
}
7475

76+
/**
77+
* Extracts the config ID from the configuration. If it is not defined, the task ID is used.
78+
*
79+
* @param config the TaskPushNotificationConfig
80+
* @return the extracted config ID
81+
*/
82+
default String extractConfigId(TaskPushNotificationConfig config) {
83+
if (config.pushNotificationConfig() != null && config.pushNotificationConfig().id() != null && !config.pushNotificationConfig().id().isBlank()) {
84+
return config.pushNotificationConfig().id();
85+
}
86+
return config.taskId();
87+
}
88+
7589
/**
7690
* Maps the protobuf PushNotificationConfig to domain, injecting config_id from request.
7791
* <p>
@@ -82,26 +96,26 @@ default String extractTaskId(SetTaskPushNotificationConfigRequest request) {
8296
*/
8397
default PushNotificationConfig mapPushNotificationConfigWithId(SetTaskPushNotificationConfigRequest request) {
8498
// Check if config and push_notification_config exist
85-
if (!request.hasConfig() ||
86-
!request.getConfig().hasPushNotificationConfig() ||
87-
request.getConfig().getPushNotificationConfig()
88-
.equals(io.a2a.grpc.PushNotificationConfig.getDefaultInstance())) {
99+
if (!request.hasConfig()
100+
|| !request.getConfig().hasPushNotificationConfig()
101+
|| request.getConfig().getPushNotificationConfig()
102+
.equals(io.a2a.grpc.PushNotificationConfig.getDefaultInstance())) {
89103
return null;
90104
}
91105

92106
// Map the proto PushNotificationConfig
93107
PushNotificationConfig result = PushNotificationConfigMapper.INSTANCE.fromProto(
94-
request.getConfig().getPushNotificationConfig()
108+
request.getConfig().getPushNotificationConfig()
95109
);
96110

97111
// Override ID with config_id from request
98112
String configId = request.getConfigId();
99113
if (configId != null && !configId.isEmpty() && !configId.equals(result.id())) {
100114
return new PushNotificationConfig(
101-
result.url(),
102-
result.token(),
103-
result.authentication(),
104-
configId
115+
result.url(),
116+
result.token(),
117+
result.authentication(),
118+
configId
105119
);
106120
}
107121

spec-grpc/src/main/java/io/a2a/grpc/utils/JSONRPCUtils.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,10 @@ protected static void parseRequestBody(JsonElement jsonRpc, com.google.protobuf.
294294

295295
public static void parseJsonString(String body, com.google.protobuf.Message.Builder builder) throws JSONRPCError {
296296
try {
297+
System.out.println("****************************************");
298+
System.out.println("Converting to " + builder.toString());
299+
System.out.println(body);
300+
System.out.println("****************************************");
297301
JsonFormat.parser().merge(body, builder);
298302
} catch (InvalidProtocolBufferException e) {
299303
log.log(Level.SEVERE, "Error parsing JSON request body: {0}", body);

tests/server-common/src/test/java/io/a2a/server/apps/common/AbstractA2AServerTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,7 @@ public void testSetPushNotificationSuccess() throws Exception {
534534
assertEquals(MINIMAL_TASK.getId(), config.taskId());
535535
assertEquals("http://example.com", config.pushNotificationConfig().url());
536536
} catch (A2AClientException e) {
537+
e.printStackTrace();;
537538
fail("Unexpected exception during set push notification test: " + e.getMessage(), e);
538539
} finally {
539540
deletePushNotificationConfigInStore(MINIMAL_TASK.getId(), MINIMAL_TASK.getId());
@@ -1334,7 +1335,7 @@ private void testInvalidParams(String invalidParamsRequest) {
13341335
.as(JSONRPCErrorResponse.class);
13351336
assertNotNull(response.getError());
13361337
assertEquals(new InvalidParamsError().getCode(), response.getError().getCode());
1337-
assertEquals("1", response.getId());
1338+
assertEquals("1", response.getId().toString());
13381339
}
13391340

13401341
@Test

0 commit comments

Comments
 (0)