Skip to content

Commit 1c21dcd

Browse files
hutiefang76hutiefang76
committed
Reject duplicate push configs with omitted IDs
Signed-off-by: hutiefang76 <[email protected]>
1 parent a9b96c8 commit 1c21dcd

5 files changed

Lines changed: 42 additions & 13 deletions

File tree

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ public TaskPushNotificationConfig setInfo(TaskPushNotificationConfig notificatio
5151
public TaskPushNotificationConfig setInfo(TaskPushNotificationConfig notificationConfig, @Nullable String protocolVersion) {
5252
String taskId = Assert.checkNotNullParam("taskId", notificationConfig.taskId());
5353
// Default missing config IDs to the task ID, matching the in-memory store.
54-
if (notificationConfig.id() == null || notificationConfig.id().isEmpty()) {
54+
boolean configIdIsMissing = notificationConfig.id() == null || notificationConfig.id().isEmpty();
55+
if (configIdIsMissing) {
5556
notificationConfig = TaskPushNotificationConfig.builder(notificationConfig).id(taskId).build();
5657
}
5758

@@ -63,6 +64,11 @@ public TaskPushNotificationConfig setInfo(TaskPushNotificationConfig notificatio
6364
// Check if entity already exists
6465
JpaPushNotificationConfig existingJpaConfig = em.find(JpaPushNotificationConfig.class, configId);
6566

67+
if (configIdIsMissing && existingJpaConfig != null) {
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+
6672
if (existingJpaConfig != null) {
6773
// Update existing entity
6874
existingJpaConfig.setConfig(notificationConfig);

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

Lines changed: 13 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,17 @@ 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());
164165
}
165166

166167
@Test
@@ -179,6 +180,13 @@ public void testSetInfoWithNullConfigId() {
179180
new ListTaskPushNotificationConfigsParams(taskId));
180181
assertEquals(1, configResult.configs().size());
181182
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));
182190
}
183191

184192
@Test

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,12 @@ public TaskPushNotificationConfig setInfo(TaskPushNotificationConfig notificatio
5959

6060
pushNotificationInfos.compute(taskId, (key, list) -> {
6161
List<TaskPushNotificationConfig> mutable = list == null ? new ArrayList<>() : new ArrayList<>(list);
62+
boolean defaultConfigAlreadyExists = configIdIsMissing
63+
&& mutable.stream().anyMatch(existing -> existing.id() != null && existing.id().equals(configId));
64+
if (defaultConfigAlreadyExists) {
65+
throw new InvalidParamsError("A push notification config with the default ID already exists for task "
66+
+ taskId + "; specify the config ID explicitly to update it");
67+
}
6268
boolean isExistingConfig = mutable.removeIf(
6369
existing -> existing.id() != null && existing.id().equals(configId));
6470
if (!isExistingConfig && mutable.size() >= maxPerTask) {

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,11 @@ 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.
9396
* </p>
9497
*
9598
* @param notificationConfig the task push notification configuration

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -166,18 +166,17 @@ public void testSetInfoWithEmptyConfigId() {
166166
assertEquals(1, configResult.configs().size());
167167
assertEquals(taskId, configResult.configs().get(0).id());
168168

169-
TaskPushNotificationConfig updatedConfig = TaskPushNotificationConfig.builder()
169+
TaskPushNotificationConfig duplicateConfig = TaskPushNotificationConfig.builder()
170170
.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());
181180
}
182181

183182
@Test
@@ -191,6 +190,13 @@ public void testSetInfoWithNullConfigId() {
191190
TaskPushNotificationConfig result = configStore.setInfo(config);
192191

193192
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));
194200
}
195201

196202
@Test

0 commit comments

Comments
 (0)