Skip to content

Commit ef3c9f4

Browse files
hutiefang76kabir
hutiefang76
and
kabir
authored
fix: allow omitted task push config id (a2aproject#1092)
* fix: allow omitted task push config id Fixes a2aproject#1081 * fix: complete optional push config id handling * test: cover empty and null push config ids * Reject duplicate push configs with omitted IDs Signed-off-by: hutiefang76 <137664623+hutiefang76@users.noreply.github.com> * Preserve v0.3 push config update semantics Signed-off-by: hutiefang76 <137664623+hutiefang76@users.noreply.github.com> * Test v0.3 push config setup in multiversion servers Signed-off-by: hutiefang76 <137664623+hutiefang76@users.noreply.github.com> --------- Signed-off-by: hutiefang76 <137664623+hutiefang76@users.noreply.github.com> Co-authored-by: Kabir Khan <kkhan@redhat.com>
1 parent 4557dae commit ef3c9f4

17 files changed

Lines changed: 251 additions & 44 deletions

File tree

‎compat-0.3/server-conversion/src/test/java/org/a2aproject/sdk/compat03/conversion/AbstractA2AServerServerTest_v0_3.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1740,6 +1740,7 @@ protected void savePushNotificationConfigInStore(String taskId, PushNotification
17401740
.uri(URI.create("http://localhost:" + serverPort + "/test/task/" + taskId))
17411741
.POST(HttpRequest.BodyPublishers.ofString(org.a2aproject.sdk.jsonrpc.common.json.JsonUtil.toJson(v10Config)))
17421742
.header("Content-Type", APPLICATION_JSON)
1743+
.header("A2A-Version", A2AProtocol_v0_3.PROTOCOL_VERSION)
17431744
.build();
17441745

17451746
HttpResponse<String> response = client.send(request, HttpResponse.BodyHandlers.ofString(StandardCharsets.UTF_8));

‎compat-0.3/server-conversion/src/test/java/org/a2aproject/sdk/compat03/conversion/TestUtilsBean_v0_3.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@ public void deleteTaskPushNotificationConfig(String taskId, String configId) {
6464
}
6565

6666
public void saveTaskPushNotificationConfig(String taskId, TaskPushNotificationConfig notificationConfig) {
67-
pushNotificationConfigStore.setInfo(TaskPushNotificationConfig.builder(notificationConfig).taskId(taskId).build());
67+
pushNotificationConfigStore.setInfo(
68+
TaskPushNotificationConfig.builder(notificationConfig).taskId(taskId).build(), "0.3");
6869
}
6970

7071
/**

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,9 @@ 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+
boolean configIdIsMissing = notificationConfig.id() == null || notificationConfig.id().isEmpty();
55+
if (configIdIsMissing) {
5756
notificationConfig = TaskPushNotificationConfig.builder(notificationConfig).id(taskId).build();
5857
}
5958

@@ -65,6 +64,11 @@ public TaskPushNotificationConfig setInfo(TaskPushNotificationConfig notificatio
6564
// Check if entity already exists
6665
JpaPushNotificationConfig existingJpaConfig = em.find(JpaPushNotificationConfig.class, configId);
6766

67+
if (configIdIsMissing && existingJpaConfig != null && !"0.3".equals(resolvedVersion)) {
68+
throw new InvalidParamsError("A push notification config with the default ID already exists for task "
69+
+ taskId + "; specify the config ID explicitly to update it");
70+
}
71+
6872
if (existingJpaConfig != null) {
6973
// Update existing entity
7074
existingJpaConfig.setConfig(notificationConfig);

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

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import static org.junit.jupiter.api.Assertions.assertEquals;
66
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
77
import static org.junit.jupiter.api.Assertions.assertNotNull;
8+
import static org.junit.jupiter.api.Assertions.assertThrows;
89
import static org.junit.jupiter.api.Assertions.assertTrue;
910
import static org.mockito.ArgumentMatchers.any;
1011
import static org.mockito.Mockito.never;
@@ -21,6 +22,7 @@
2122
import org.a2aproject.sdk.server.tasks.BasePushNotificationSender;
2223
import org.a2aproject.sdk.server.tasks.PushNotificationConfigStore;
2324
import org.a2aproject.sdk.server.tasks.PushNotificationUrlValidator;
25+
import org.a2aproject.sdk.spec.InvalidParamsError;
2426
import org.a2aproject.sdk.spec.ListTaskPushNotificationConfigsParams;
2527
import org.a2aproject.sdk.spec.ListTaskPushNotificationConfigsResult;
2628
import org.a2aproject.sdk.spec.Task;
@@ -149,18 +151,65 @@ public void testSetInfoWithoutConfigId() {
149151
assertEquals(1, configResult.configs().size());
150152
assertEquals(taskId, configResult.configs().get(0).id());
151153

152-
TaskPushNotificationConfig updatedConfig = TaskPushNotificationConfig.builder()
154+
TaskPushNotificationConfig duplicateConfig = TaskPushNotificationConfig.builder()
153155
.id("")
154156
.url("http://initial.url/callback_new")
155157
.taskId(taskId)
156158
.build();
157159

158-
TaskPushNotificationConfig updatedResult = configStore.setInfo(updatedConfig);
159-
assertEquals(taskId, updatedResult.id());
160+
assertThrows(InvalidParamsError.class, () -> configStore.setInfo(duplicateConfig));
160161

161162
configResult = configStore.getInfo(new ListTaskPushNotificationConfigsParams(taskId));
162-
assertEquals(1, configResult.configs().size(), "Should replace existing config with same ID rather than adding new one");
163-
assertEquals(updatedConfig.url(), configResult.configs().get(0).url());
163+
assertEquals(1, configResult.configs().size());
164+
assertEquals(initialConfig.url(), configResult.configs().get(0).url());
165+
}
166+
167+
@Test
168+
@Transactional
169+
public void testSetInfoWithNullConfigId() {
170+
String taskId = "task_null_config_id";
171+
TaskPushNotificationConfig config = TaskPushNotificationConfig.builder()
172+
.url("http://null-id.url/callback")
173+
.taskId(taskId)
174+
.build();
175+
176+
TaskPushNotificationConfig result = configStore.setInfo(config);
177+
178+
assertEquals(taskId, result.id(), "A missing config ID should default to the task ID");
179+
ListTaskPushNotificationConfigsResult configResult = configStore.getInfo(
180+
new ListTaskPushNotificationConfigsParams(taskId));
181+
assertEquals(1, configResult.configs().size());
182+
assertEquals(taskId, configResult.configs().get(0).id());
183+
184+
TaskPushNotificationConfig duplicateConfig = TaskPushNotificationConfig.builder()
185+
.url("http://updated.url/callback")
186+
.taskId(taskId)
187+
.build();
188+
189+
assertThrows(InvalidParamsError.class, () -> configStore.setInfo(duplicateConfig));
190+
}
191+
192+
@Test
193+
@Transactional
194+
public void testSetInfoAllowsV03DefaultConfigUpdate() {
195+
String taskId = "task_v03_default_update";
196+
TaskPushNotificationConfig initialConfig = TaskPushNotificationConfig.builder()
197+
.taskId(taskId)
198+
.url("http://initial.url/callback")
199+
.build();
200+
TaskPushNotificationConfig updatedConfig = TaskPushNotificationConfig.builder()
201+
.taskId(taskId)
202+
.url("http://updated.url/callback")
203+
.build();
204+
205+
configStore.setInfo(initialConfig, "0.3");
206+
TaskPushNotificationConfig result = configStore.setInfo(updatedConfig, "0.3");
207+
208+
assertEquals(taskId, result.id());
209+
ListTaskPushNotificationConfigsResult stored =
210+
configStore.getInfo(new ListTaskPushNotificationConfigsParams(taskId));
211+
assertEquals(1, stored.configs().size());
212+
assertEquals(updatedConfig.url(), stored.configs().get(0).url());
164213
}
165214

166215
@Test

‎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
@@ -1126,7 +1126,8 @@ public TaskPushNotificationConfig onGetTaskPushNotificationConfig(
11261126
throw new InternalError("No push notification config found");
11271127
}
11281128

1129-
String configId = params.id();
1129+
String requestedConfigId = params.id();
1130+
String configId = requestedConfigId == null || requestedConfigId.isEmpty() ? params.taskId() : requestedConfigId;
11301131
return getTaskPushNotificationConfig(listTaskPushNotificationConfigsResult, configId);
11311132
}
11321133

‎server-common/src/main/java/org/a2aproject/sdk/server/tasks/InMemoryPushNotificationConfigStore.java‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,17 +46,31 @@ public InMemoryPushNotificationConfigStore() {
4646

4747
@Override
4848
public TaskPushNotificationConfig setInfo(TaskPushNotificationConfig notificationConfig) {
49+
return setInfo(notificationConfig, false);
50+
}
51+
52+
private TaskPushNotificationConfig setInfo(
53+
TaskPushNotificationConfig notificationConfig,
54+
boolean allowDefaultConfigUpdate) {
4955
String taskId = Assert.checkNotNullParam("taskId", notificationConfig.taskId());
5056
TaskPushNotificationConfig.Builder builder = TaskPushNotificationConfig.builder(notificationConfig);
51-
if (notificationConfig.id().isEmpty()) {
57+
String requestedConfigId = notificationConfig.id();
58+
boolean configIdIsMissing = requestedConfigId == null || requestedConfigId.isEmpty();
59+
String configId = configIdIsMissing ? taskId : requestedConfigId;
60+
if (configIdIsMissing) {
5261
builder.id(taskId);
5362
}
5463
TaskPushNotificationConfig config = builder.build();
55-
String configId = config.id();
5664
int maxPerTask = PushNotificationConfigStore.maxPushConfigsPerTask(configProvider);
5765

5866
pushNotificationInfos.compute(taskId, (key, list) -> {
5967
List<TaskPushNotificationConfig> mutable = list == null ? new ArrayList<>() : new ArrayList<>(list);
68+
boolean defaultConfigAlreadyExists = configIdIsMissing
69+
&& mutable.stream().anyMatch(existing -> existing.id() != null && existing.id().equals(configId));
70+
if (defaultConfigAlreadyExists && !allowDefaultConfigUpdate) {
71+
throw new InvalidParamsError("A push notification config with the default ID already exists for task "
72+
+ taskId + "; specify the config ID explicitly to update it");
73+
}
6074
boolean isExistingConfig = mutable.removeIf(
6175
existing -> existing.id() != null && existing.id().equals(configId));
6276
if (!isExistingConfig && mutable.size() >= maxPerTask) {
@@ -71,8 +85,10 @@ public TaskPushNotificationConfig setInfo(TaskPushNotificationConfig notificatio
7185

7286
@Override
7387
public TaskPushNotificationConfig setInfo(TaskPushNotificationConfig config, @Nullable String protocolVersion) {
74-
TaskPushNotificationConfig result = setInfo(config);
75-
protocolVersions.put(result.taskId() + ":" + result.id(), PushNotificationConfigStore.resolveProtocolVersion(protocolVersion));
88+
TaskPushNotificationConfig result = setInfo(config, "0.3".equals(protocolVersion));
89+
protocolVersions.put(
90+
result.taskId() + ":" + result.id(),
91+
PushNotificationConfigStore.resolveProtocolVersion(protocolVersion));
7692
return result;
7793
}
7894

‎server-common/src/main/java/org/a2aproject/sdk/server/tasks/PushNotificationConfigStore.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,12 @@ public interface PushNotificationConfigStore {
8888
/**
8989
* Sets or updates the push notification configuration for a task.
9090
* <p>
91-
* If {@code notificationConfig.id()} is null or empty, it's set to the task ID.
92-
* If a config with the same ID already exists for this task, it's replaced.
91+
* If {@code notificationConfig.id()} is null or empty, the store creates the
92+
* default config with the task ID. Omitting the ID is a create-only shorthand:
93+
* if the default config already exists, the store rejects the request instead
94+
* of silently replacing it. To update the default config, provide the task ID
95+
* explicitly. Configurations beyond the default one must always provide an ID.
96+
* The v0.3 compatibility path keeps its historical single-config update behavior.
9397
* </p>
9498
*
9599
* @param notificationConfig the task push notification configuration

‎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
@@ -45,6 +45,7 @@
4545
import org.a2aproject.sdk.spec.CancelTaskParams;
4646
import org.a2aproject.sdk.spec.Event;
4747
import org.a2aproject.sdk.spec.EventKind;
48+
import org.a2aproject.sdk.spec.GetTaskPushNotificationConfigParams;
4849
import org.a2aproject.sdk.spec.InvalidParamsError;
4950
import org.a2aproject.sdk.spec.ListTasksParams;
5051
import org.a2aproject.sdk.spec.ListTaskPushNotificationConfigsParams;
@@ -989,6 +990,26 @@ void testVersionStored_OnCreateTaskPushNotificationConfig() throws Exception {
989990
"Protocol version should be stored for the push notification config");
990991
}
991992

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

‎server-common/src/test/java/org/a2aproject/sdk/server/tasks/InMemoryPushNotificationConfigStoreTest.java‎

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -151,10 +151,10 @@ public void testSetInfoAppendsToExistingConfig() {
151151
}
152152

153153
@Test
154-
public void testSetInfoWithoutConfigId() {
154+
public void testSetInfoWithEmptyConfigId() {
155155
String taskId = "task1";
156156
TaskPushNotificationConfig initialConfig = TaskPushNotificationConfig.builder()
157-
.id("") // No ID set
157+
.id("")
158158
.url("http://initial.url/callback")
159159
.taskId(taskId)
160160
.build();
@@ -166,18 +166,59 @@ public void testSetInfoWithoutConfigId() {
166166
assertEquals(1, configResult.configs().size());
167167
assertEquals(taskId, configResult.configs().get(0).id());
168168

169-
TaskPushNotificationConfig updatedConfig = TaskPushNotificationConfig.builder()
170-
.id("") // No ID set
169+
TaskPushNotificationConfig duplicateConfig = TaskPushNotificationConfig.builder()
170+
.id("")
171171
.url("http://initial.url/callback_new")
172172
.taskId(taskId)
173173
.build();
174174

175-
TaskPushNotificationConfig updatedResult = configStore.setInfo(updatedConfig);
176-
assertEquals(taskId, updatedResult.id());
175+
assertThrows(InvalidParamsError.class, () -> configStore.setInfo(duplicateConfig));
177176

178177
configResult = configStore.getInfo(new ListTaskPushNotificationConfigsParams(taskId));
179-
assertEquals(1, configResult.configs().size(), "Should replace existing config with same ID rather than adding new one");
180-
assertEquals(updatedConfig.url(), configResult.configs().get(0).url());
178+
assertEquals(1, configResult.configs().size());
179+
assertEquals(initialConfig.url(), configResult.configs().get(0).url());
180+
}
181+
182+
@Test
183+
public void testSetInfoWithNullConfigId() {
184+
String taskId = "task_with_null_config_id";
185+
TaskPushNotificationConfig config = TaskPushNotificationConfig.builder()
186+
.url("http://initial.url/callback")
187+
.taskId(taskId)
188+
.build();
189+
190+
TaskPushNotificationConfig result = configStore.setInfo(config);
191+
192+
assertEquals(taskId, result.id(), "Config ID should default to taskId when null");
193+
194+
TaskPushNotificationConfig duplicateConfig = TaskPushNotificationConfig.builder()
195+
.url("http://updated.url/callback")
196+
.taskId(taskId)
197+
.build();
198+
199+
assertThrows(InvalidParamsError.class, () -> configStore.setInfo(duplicateConfig));
200+
}
201+
202+
@Test
203+
public void testSetInfoAllowsV03DefaultConfigUpdate() {
204+
String taskId = "task_v03_default_update";
205+
TaskPushNotificationConfig initialConfig = TaskPushNotificationConfig.builder()
206+
.taskId(taskId)
207+
.url("http://initial.url/callback")
208+
.build();
209+
TaskPushNotificationConfig updatedConfig = TaskPushNotificationConfig.builder()
210+
.taskId(taskId)
211+
.url("http://updated.url/callback")
212+
.build();
213+
214+
configStore.setInfo(initialConfig, "0.3");
215+
TaskPushNotificationConfig result = configStore.setInfo(updatedConfig, "0.3");
216+
217+
assertEquals(taskId, result.id());
218+
ListTaskPushNotificationConfigsResult stored =
219+
configStore.getInfo(new ListTaskPushNotificationConfigsParams(taskId));
220+
assertEquals(1, stored.configs().size());
221+
assertEquals(updatedConfig.url(), stored.configs().get(0).url());
181222
}
182223

183224
@Test

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

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

2323
/**
2424
* Compact constructor that validates required fields.
2525
*
2626
* @param taskId the taskId parameter (see class-level JavaDoc)
2727
* @param id the id parameter (see class-level JavaDoc)
2828
* @param tenant the tenant parameter (see class-level JavaDoc)
29-
* @throws IllegalArgumentException if taskId or tenant is null
29+
* @throws IllegalArgumentException if taskId is null
3030
*/
3131
public GetTaskPushNotificationConfigParams {
3232
Assert.checkNotNullParam("taskId", taskId);
33-
Assert.checkNotNullParam("id", id);
3433
Utils.validateTenant(tenant);
3534
}
3635

36+
/**
37+
* Convenience constructor for retrieving the configuration that uses the task ID as its default ID.
38+
*
39+
* @param taskId the task identifier (required)
40+
*/
41+
public GetTaskPushNotificationConfigParams(String taskId) {
42+
this(taskId, null, null);
43+
}
44+
3745
/**
3846
* Convenience constructor for creating parameters without tenant.
3947
*
4048
* @param taskId the task identifier (required)
4149
* @param id optional configuration ID to retrieve
4250
*/
43-
public GetTaskPushNotificationConfigParams(String taskId, String id) {
51+
public GetTaskPushNotificationConfigParams(String taskId, @Nullable String id) {
4452
this(taskId, id, null);
4553
}
4654

@@ -84,7 +92,7 @@ public Builder taskId(String taskId) {
8492
* @param id the configuration ID
8593
* @return this builder for method chaining
8694
*/
87-
public Builder id(String id) {
95+
public Builder id(@Nullable String id) {
8896
this.id = id;
8997
return this;
9098
}
@@ -108,7 +116,7 @@ public Builder tenant(@Nullable String tenant) {
108116
public GetTaskPushNotificationConfigParams build() {
109117
return new GetTaskPushNotificationConfigParams(
110118
Assert.checkNotNullParam("taskId", taskId),
111-
Assert.checkNotNullParam("id", id),
119+
id,
112120
tenant);
113121
}
114122
}

0 commit comments

Comments
 (0)