Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -1740,6 +1740,7 @@ protected void savePushNotificationConfigInStore(String taskId, PushNotification
.uri(URI.create("http://localhost:" + serverPort + "/test/task/" + taskId))
.POST(HttpRequest.BodyPublishers.ofString(org.a2aproject.sdk.jsonrpc.common.json.JsonUtil.toJson(v10Config)))
.header("Content-Type", APPLICATION_JSON)
.header("A2A-Version", A2AProtocol_v0_3.PROTOCOL_VERSION)
.build();

HttpResponse<String> response = client.send(request, HttpResponse.BodyHandlers.ofString(StandardCharsets.UTF_8));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ public void deleteTaskPushNotificationConfig(String taskId, String configId) {
}

public void saveTaskPushNotificationConfig(String taskId, TaskPushNotificationConfig notificationConfig) {
pushNotificationConfigStore.setInfo(TaskPushNotificationConfig.builder(notificationConfig).taskId(taskId).build());
pushNotificationConfigStore.setInfo(
TaskPushNotificationConfig.builder(notificationConfig).taskId(taskId).build(), "0.3");
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,9 @@ public TaskPushNotificationConfig setInfo(TaskPushNotificationConfig notificatio
@Override
public TaskPushNotificationConfig setInfo(TaskPushNotificationConfig notificationConfig, @Nullable String protocolVersion) {
String taskId = Assert.checkNotNullParam("taskId", notificationConfig.taskId());
// Ensure config has an ID - default to taskId if not provided (mirroring InMemoryPushNotificationConfigStore behavior)
if (notificationConfig.id().isEmpty()) {
// This means the taskId and configId are same. This will not allow having multiple configs for a single Task.
// The configId is a required field in the spec and should not be empty
// Default missing config IDs to the task ID, matching the in-memory store.
boolean configIdIsMissing = notificationConfig.id() == null || notificationConfig.id().isEmpty();
if (configIdIsMissing) {
notificationConfig = TaskPushNotificationConfig.builder(notificationConfig).id(taskId).build();
}

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

if (configIdIsMissing && existingJpaConfig != null && !"0.3".equals(resolvedVersion)) {
throw new InvalidParamsError("A push notification config with the default ID already exists for task "
+ taskId + "; specify the config ID explicitly to update it");
}

if (existingJpaConfig != null) {
// Update existing entity
existingJpaConfig.setConfig(notificationConfig);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.never;
Expand All @@ -21,6 +22,7 @@
import org.a2aproject.sdk.server.tasks.BasePushNotificationSender;
import org.a2aproject.sdk.server.tasks.PushNotificationConfigStore;
import org.a2aproject.sdk.server.tasks.PushNotificationUrlValidator;
import org.a2aproject.sdk.spec.InvalidParamsError;
import org.a2aproject.sdk.spec.ListTaskPushNotificationConfigsParams;
import org.a2aproject.sdk.spec.ListTaskPushNotificationConfigsResult;
import org.a2aproject.sdk.spec.Task;
Expand Down Expand Up @@ -149,18 +151,65 @@ public void testSetInfoWithoutConfigId() {
assertEquals(1, configResult.configs().size());
assertEquals(taskId, configResult.configs().get(0).id());

TaskPushNotificationConfig updatedConfig = TaskPushNotificationConfig.builder()
TaskPushNotificationConfig duplicateConfig = TaskPushNotificationConfig.builder()
.id("")
.url("http://initial.url/callback_new")
.taskId(taskId)
.build();

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

configResult = configStore.getInfo(new ListTaskPushNotificationConfigsParams(taskId));
assertEquals(1, configResult.configs().size(), "Should replace existing config with same ID rather than adding new one");
assertEquals(updatedConfig.url(), configResult.configs().get(0).url());
assertEquals(1, configResult.configs().size());
assertEquals(initialConfig.url(), configResult.configs().get(0).url());
}

@Test
@Transactional
public void testSetInfoWithNullConfigId() {
String taskId = "task_null_config_id";
TaskPushNotificationConfig config = TaskPushNotificationConfig.builder()
.url("http://null-id.url/callback")
.taskId(taskId)
.build();

TaskPushNotificationConfig result = configStore.setInfo(config);

assertEquals(taskId, result.id(), "A missing config ID should default to the task ID");
ListTaskPushNotificationConfigsResult configResult = configStore.getInfo(
new ListTaskPushNotificationConfigsParams(taskId));
assertEquals(1, configResult.configs().size());
assertEquals(taskId, configResult.configs().get(0).id());

TaskPushNotificationConfig duplicateConfig = TaskPushNotificationConfig.builder()
.url("http://updated.url/callback")
.taskId(taskId)
.build();

assertThrows(InvalidParamsError.class, () -> configStore.setInfo(duplicateConfig));
}

@Test
@Transactional
public void testSetInfoAllowsV03DefaultConfigUpdate() {
String taskId = "task_v03_default_update";
TaskPushNotificationConfig initialConfig = TaskPushNotificationConfig.builder()
.taskId(taskId)
.url("http://initial.url/callback")
.build();
TaskPushNotificationConfig updatedConfig = TaskPushNotificationConfig.builder()
.taskId(taskId)
.url("http://updated.url/callback")
.build();

configStore.setInfo(initialConfig, "0.3");
TaskPushNotificationConfig result = configStore.setInfo(updatedConfig, "0.3");

assertEquals(taskId, result.id());
ListTaskPushNotificationConfigsResult stored =
configStore.getInfo(new ListTaskPushNotificationConfigsParams(taskId));
assertEquals(1, stored.configs().size());
assertEquals(updatedConfig.url(), stored.configs().get(0).url());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1126,7 +1126,8 @@ public TaskPushNotificationConfig onGetTaskPushNotificationConfig(
throw new InternalError("No push notification config found");
}

String configId = params.id();
String requestedConfigId = params.id();
String configId = requestedConfigId == null || requestedConfigId.isEmpty() ? params.taskId() : requestedConfigId;
return getTaskPushNotificationConfig(listTaskPushNotificationConfigsResult, configId);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,31 @@ public InMemoryPushNotificationConfigStore() {

@Override
public TaskPushNotificationConfig setInfo(TaskPushNotificationConfig notificationConfig) {
return setInfo(notificationConfig, false);
}

private TaskPushNotificationConfig setInfo(
TaskPushNotificationConfig notificationConfig,
boolean allowDefaultConfigUpdate) {
String taskId = Assert.checkNotNullParam("taskId", notificationConfig.taskId());
TaskPushNotificationConfig.Builder builder = TaskPushNotificationConfig.builder(notificationConfig);
if (notificationConfig.id().isEmpty()) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a comment about this code, but JpaDatabasePushNotificationConfigStore has the same latent NPE, and needs fixing too.

String requestedConfigId = notificationConfig.id();
boolean configIdIsMissing = requestedConfigId == null || requestedConfigId.isEmpty();
String configId = configIdIsMissing ? taskId : requestedConfigId;
if (configIdIsMissing) {
builder.id(taskId);
}
TaskPushNotificationConfig config = builder.build();
String configId = config.id();
int maxPerTask = PushNotificationConfigStore.maxPushConfigsPerTask(configProvider);

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

@Override
public TaskPushNotificationConfig setInfo(TaskPushNotificationConfig config, @Nullable String protocolVersion) {
TaskPushNotificationConfig result = setInfo(config);
protocolVersions.put(result.taskId() + ":" + result.id(), PushNotificationConfigStore.resolveProtocolVersion(protocolVersion));
TaskPushNotificationConfig result = setInfo(config, "0.3".equals(protocolVersion));
protocolVersions.put(
result.taskId() + ":" + result.id(),
PushNotificationConfigStore.resolveProtocolVersion(protocolVersion));
return result;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,12 @@ public interface PushNotificationConfigStore {
/**
* Sets or updates the push notification configuration for a task.
* <p>
* If {@code notificationConfig.id()} is null or empty, it's set to the task ID.
* If a config with the same ID already exists for this task, it's replaced.
* If {@code notificationConfig.id()} is null or empty, the store creates the
* default config with the task ID. Omitting the ID is a create-only shorthand:
* if the default config already exists, the store rejects the request instead
* of silently replacing it. To update the default config, provide the task ID
* explicitly. Configurations beyond the default one must always provide an ID.
* The v0.3 compatibility path keeps its historical single-config update behavior.
* </p>
*
* @param notificationConfig the task push notification configuration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import org.a2aproject.sdk.spec.CancelTaskParams;
import org.a2aproject.sdk.spec.Event;
import org.a2aproject.sdk.spec.EventKind;
import org.a2aproject.sdk.spec.GetTaskPushNotificationConfigParams;
import org.a2aproject.sdk.spec.InvalidParamsError;
import org.a2aproject.sdk.spec.ListTasksParams;
import org.a2aproject.sdk.spec.ListTaskPushNotificationConfigsParams;
Expand Down Expand Up @@ -989,6 +990,26 @@ void testVersionStored_OnCreateTaskPushNotificationConfig() throws Exception {
"Protocol version should be stored for the push notification config");
}

@Test
void testGetTaskPushNotificationConfigDefaultsMissingIdToTaskId() throws Exception {
String taskId = "get-default-config-id";
taskStore.save(Task.builder()
.id(taskId)
.contextId("ctx-get-default-config-id")
.status(new TaskStatus(TaskState.TASK_STATE_WORKING))
.build(), false);
requestHandler.onCreateTaskPushNotificationConfig(TaskPushNotificationConfig.builder()
.taskId(taskId)
.url("http://example.com/get-default-config-id")
.build(), NULL_CONTEXT);

TaskPushNotificationConfig result = requestHandler.onGetTaskPushNotificationConfig(
new GetTaskPushNotificationConfigParams(taskId), NULL_CONTEXT);

assertEquals(taskId, result.id());
assertEquals("http://example.com/get-default-config-id", result.url());
}

/**
* Verify that onMessageSend stores the protocol version when the request
* includes a push notification config (new task path).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,10 @@ public void testSetInfoAppendsToExistingConfig() {
}

@Test
public void testSetInfoWithoutConfigId() {
public void testSetInfoWithEmptyConfigId() {
String taskId = "task1";
TaskPushNotificationConfig initialConfig = TaskPushNotificationConfig.builder()
.id("") // No ID set

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create a new test rather than changing this existing one.

Removing .id("") repurposes this test from the empty-string path to the null path rather than covering both.
The empty-string case is the one production actually hits — the mapper doesn't apply emptyToNull to id, so at runtime a missing id arrives as "", never null.
Recommend keeping the "" case here and adding a separate null-id test (or parameterizing over both), so the load-bearing branch stays asserted.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous comment should still be addressed.

.id("")
.url("http://initial.url/callback")
.taskId(taskId)
.build();
Expand All @@ -166,18 +166,59 @@ public void testSetInfoWithoutConfigId() {
assertEquals(1, configResult.configs().size());
assertEquals(taskId, configResult.configs().get(0).id());

TaskPushNotificationConfig updatedConfig = TaskPushNotificationConfig.builder()
.id("") // No ID set

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create a new test rather than changing this existing one.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous comment should still be addressed.

TaskPushNotificationConfig duplicateConfig = TaskPushNotificationConfig.builder()
.id("")
.url("http://initial.url/callback_new")
.taskId(taskId)
.build();

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

configResult = configStore.getInfo(new ListTaskPushNotificationConfigsParams(taskId));
assertEquals(1, configResult.configs().size(), "Should replace existing config with same ID rather than adding new one");
assertEquals(updatedConfig.url(), configResult.configs().get(0).url());
assertEquals(1, configResult.configs().size());
assertEquals(initialConfig.url(), configResult.configs().get(0).url());
}

@Test
public void testSetInfoWithNullConfigId() {
String taskId = "task_with_null_config_id";
TaskPushNotificationConfig config = TaskPushNotificationConfig.builder()
.url("http://initial.url/callback")
.taskId(taskId)
.build();

TaskPushNotificationConfig result = configStore.setInfo(config);

assertEquals(taskId, result.id(), "Config ID should default to taskId when null");

TaskPushNotificationConfig duplicateConfig = TaskPushNotificationConfig.builder()
.url("http://updated.url/callback")
.taskId(taskId)
.build();

assertThrows(InvalidParamsError.class, () -> configStore.setInfo(duplicateConfig));
}

@Test
public void testSetInfoAllowsV03DefaultConfigUpdate() {
String taskId = "task_v03_default_update";
TaskPushNotificationConfig initialConfig = TaskPushNotificationConfig.builder()
.taskId(taskId)
.url("http://initial.url/callback")
.build();
TaskPushNotificationConfig updatedConfig = TaskPushNotificationConfig.builder()
.taskId(taskId)
.url("http://updated.url/callback")
.build();

configStore.setInfo(initialConfig, "0.3");
TaskPushNotificationConfig result = configStore.setInfo(updatedConfig, "0.3");

assertEquals(taskId, result.id());
ListTaskPushNotificationConfigsResult stored =
configStore.getInfo(new ListTaskPushNotificationConfigsParams(taskId));
assertEquals(1, stored.configs().size());
assertEquals(updatedConfig.url(), stored.configs().get(0).url());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,29 +18,37 @@
* @see TaskPushNotificationConfig for the returned configuration structure
* @see <a href="https://a2a-protocol.org/latest/">A2A Protocol Specification</a>
*/
public record GetTaskPushNotificationConfigParams(String taskId, String id, @Nullable String tenant) {
public record GetTaskPushNotificationConfigParams(String taskId, @Nullable String id, @Nullable String tenant) {

/**
* Compact constructor that validates required fields.
*
* @param taskId the taskId parameter (see class-level JavaDoc)
* @param id the id parameter (see class-level JavaDoc)
* @param tenant the tenant parameter (see class-level JavaDoc)
* @throws IllegalArgumentException if taskId or tenant is null
* @throws IllegalArgumentException if taskId is null
*/
public GetTaskPushNotificationConfigParams {
Assert.checkNotNullParam("taskId", taskId);
Assert.checkNotNullParam("id", id);
Utils.validateTenant(tenant);
}

/**
* Convenience constructor for retrieving the configuration that uses the task ID as its default ID.
*
* @param taskId the task identifier (required)
*/
public GetTaskPushNotificationConfigParams(String taskId) {
this(taskId, null, null);
}

/**
* Convenience constructor for creating parameters without tenant.
*
* @param taskId the task identifier (required)
* @param id optional configuration ID to retrieve
*/
public GetTaskPushNotificationConfigParams(String taskId, String id) {
public GetTaskPushNotificationConfigParams(String taskId, @Nullable String id) {
this(taskId, id, null);
}

Expand Down Expand Up @@ -84,7 +92,7 @@ public Builder taskId(String taskId) {
* @param id the configuration ID
* @return this builder for method chaining
*/
public Builder id(String id) {
public Builder id(@Nullable String id) {
this.id = id;
return this;
}
Expand All @@ -108,7 +116,7 @@ public Builder tenant(@Nullable String tenant) {
public GetTaskPushNotificationConfigParams build() {
return new GetTaskPushNotificationConfigParams(
Assert.checkNotNullParam("taskId", taskId),
Assert.checkNotNullParam("id", id),
id,
tenant);
}
}
Expand Down
Loading
Loading