Skip to content

Commit 5bd4916

Browse files
hutiefang76hutiefang76
committed
fix: complete optional push config id handling
1 parent 5a5e304 commit 5bd4916

6 files changed

Lines changed: 83 additions & 11 deletions

File tree

‎extras/push-notification-config-store-database-jpa/src/main/java/org/a2aproject/sdk/extras/pushnotificationconfigstore/database/jpa/JpaDatabasePushNotificationConfigStore.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,8 @@ public TaskPushNotificationConfig setInfo(TaskPushNotificationConfig notificatio
5050
@Override
5151
public TaskPushNotificationConfig setInfo(TaskPushNotificationConfig notificationConfig, @Nullable String protocolVersion) {
5252
String taskId = Assert.checkNotNullParam("taskId", notificationConfig.taskId());
53-
// Ensure config has an ID - default to taskId if not provided (mirroring InMemoryPushNotificationConfigStore behavior)
54-
if (notificationConfig.id().isEmpty()) {
55-
// This means the taskId and configId are same. This will not allow having multiple configs for a single Task.
56-
// The configId is a required field in the spec and should not be empty
53+
// Default missing config IDs to the task ID, matching the in-memory store.
54+
if (notificationConfig.id() == null || notificationConfig.id().isEmpty()) {
5755
notificationConfig = TaskPushNotificationConfig.builder(notificationConfig).id(taskId).build();
5856
}
5957

‎extras/push-notification-config-store-database-jpa/src/test/java/org/a2aproject/sdk/extras/pushnotificationconfigstore/database/jpa/JpaPushNotificationConfigStoreTest.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,24 @@ public void testSetInfoWithoutConfigId() {
161161
assertEquals(updatedConfig.url(), configResult.configs().get(0).url());
162162
}
163163

164+
@Test
165+
@Transactional
166+
public void testSetInfoWithNullConfigId() {
167+
String taskId = "task_null_config_id";
168+
TaskPushNotificationConfig config = TaskPushNotificationConfig.builder()
169+
.url("http://null-id.url/callback")
170+
.taskId(taskId)
171+
.build();
172+
173+
TaskPushNotificationConfig result = configStore.setInfo(config);
174+
175+
assertEquals(taskId, result.id(), "A missing config ID should default to the task ID");
176+
ListTaskPushNotificationConfigsResult configResult = configStore.getInfo(
177+
new ListTaskPushNotificationConfigsParams(taskId));
178+
assertEquals(1, configResult.configs().size());
179+
assertEquals(taskId, configResult.configs().get(0).id());
180+
}
181+
164182
@Test
165183
@Transactional
166184
public void testGetInfoExistingConfig() {

‎server-common/src/main/java/org/a2aproject/sdk/server/requesthandlers/DefaultRequestHandler.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -962,7 +962,8 @@ public TaskPushNotificationConfig onGetTaskPushNotificationConfig(
962962
throw new InternalError("No push notification config found");
963963
}
964964

965-
String configId = params.id();
965+
String requestedConfigId = params.id();
966+
String configId = requestedConfigId == null || requestedConfigId.isEmpty() ? params.taskId() : requestedConfigId;
966967
return getTaskPushNotificationConfig(listTaskPushNotificationConfigsResult, configId);
967968
}
968969

‎server-common/src/test/java/org/a2aproject/sdk/server/requesthandlers/DefaultRequestHandlerTest.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import org.a2aproject.sdk.spec.CancelTaskParams;
4444
import org.a2aproject.sdk.spec.Event;
4545
import org.a2aproject.sdk.spec.EventKind;
46+
import org.a2aproject.sdk.spec.GetTaskPushNotificationConfigParams;
4647
import org.a2aproject.sdk.spec.InvalidParamsError;
4748
import org.a2aproject.sdk.spec.Message;
4849
import org.a2aproject.sdk.spec.MessageSendConfiguration;
@@ -983,6 +984,26 @@ void testVersionStored_OnCreateTaskPushNotificationConfig() throws Exception {
983984
"Protocol version should be stored for the push notification config");
984985
}
985986

987+
@Test
988+
void testGetTaskPushNotificationConfigDefaultsMissingIdToTaskId() throws Exception {
989+
String taskId = "get-default-config-id";
990+
taskStore.save(Task.builder()
991+
.id(taskId)
992+
.contextId("ctx-get-default-config-id")
993+
.status(new TaskStatus(TaskState.TASK_STATE_WORKING))
994+
.build(), false);
995+
requestHandler.onCreateTaskPushNotificationConfig(TaskPushNotificationConfig.builder()
996+
.taskId(taskId)
997+
.url("http://example.com/get-default-config-id")
998+
.build(), NULL_CONTEXT);
999+
1000+
TaskPushNotificationConfig result = requestHandler.onGetTaskPushNotificationConfig(
1001+
new GetTaskPushNotificationConfigParams(taskId), NULL_CONTEXT);
1002+
1003+
assertEquals(taskId, result.id());
1004+
assertEquals("http://example.com/get-default-config-id", result.url());
1005+
}
1006+
9861007
/**
9871008
* Verify that onMessageSend stores the protocol version when the request
9881009
* includes a push notification config (new task path).

‎spec/src/main/java/org/a2aproject/sdk/spec/GetTaskPushNotificationConfigParams.java‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,27 @@
1717
* @see TaskPushNotificationConfig for the returned configuration structure
1818
* @see <a href="https://a2a-protocol.org/latest/">A2A Protocol Specification</a>
1919
*/
20-
public record GetTaskPushNotificationConfigParams(String taskId, String id, @Nullable String tenant) {
20+
public record GetTaskPushNotificationConfigParams(String taskId, @Nullable String id, @Nullable String tenant) {
2121

2222
/**
2323
* Compact constructor that validates required fields.
2424
*
2525
* @param taskId the taskId parameter (see class-level JavaDoc)
2626
* @param id the id parameter (see class-level JavaDoc)
2727
* @param tenant the tenant parameter (see class-level JavaDoc)
28-
* @throws IllegalArgumentException if taskId or tenant is null
28+
* @throws IllegalArgumentException if taskId is null
2929
*/
3030
public GetTaskPushNotificationConfigParams {
3131
Assert.checkNotNullParam("taskId", taskId);
32-
Assert.checkNotNullParam("id", id);
32+
}
33+
34+
/**
35+
* Convenience constructor for retrieving the configuration that uses the task ID as its default ID.
36+
*
37+
* @param taskId the task identifier (required)
38+
*/
39+
public GetTaskPushNotificationConfigParams(String taskId) {
40+
this(taskId, null, null);
3341
}
3442

3543
/**
@@ -38,7 +46,7 @@ public record GetTaskPushNotificationConfigParams(String taskId, String id, @Nul
3846
* @param taskId the task identifier (required)
3947
* @param id optional configuration ID to retrieve
4048
*/
41-
public GetTaskPushNotificationConfigParams(String taskId, String id) {
49+
public GetTaskPushNotificationConfigParams(String taskId, @Nullable String id) {
4250
this(taskId, id, null);
4351
}
4452

@@ -82,7 +90,7 @@ public Builder taskId(String taskId) {
8290
* @param id the configuration ID
8391
* @return this builder for method chaining
8492
*/
85-
public Builder id(String id) {
93+
public Builder id(@Nullable String id) {
8694
this.id = id;
8795
return this;
8896
}
@@ -106,7 +114,7 @@ public Builder tenant(@Nullable String tenant) {
106114
public GetTaskPushNotificationConfigParams build() {
107115
return new GetTaskPushNotificationConfigParams(
108116
Assert.checkNotNullParam("taskId", taskId),
109-
Assert.checkNotNullParam("id", id),
117+
id,
110118
tenant);
111119
}
112120
}

‎spec/src/test/java/org/a2aproject/sdk/spec/GetTaskPushNotificationConfigParamsTest.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package org.a2aproject.sdk.spec;
2+
3+
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
import static org.junit.jupiter.api.Assertions.assertNull;
5+
6+
import org.junit.jupiter.api.Test;
7+
8+
class GetTaskPushNotificationConfigParamsTest {
9+
10+
@Test
11+
void testConstructionAllowsOmittedConfigurationId() {
12+
GetTaskPushNotificationConfigParams params = new GetTaskPushNotificationConfigParams("task-1");
13+
14+
assertEquals("task-1", params.taskId());
15+
assertNull(params.id());
16+
}
17+
18+
@Test
19+
void testBuilderAllowsOmittedConfigurationId() {
20+
GetTaskPushNotificationConfigParams params = GetTaskPushNotificationConfigParams.builder()
21+
.taskId("task-1")
22+
.build();
23+
24+
assertNull(params.id());
25+
}
26+
}

0 commit comments

Comments
 (0)