-
Notifications
You must be signed in to change notification settings - Fork 96
feat: Support multiple payload types in push notifications #494
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,25 +3,33 @@ | |||||||||||||||||||||||||||||||||
| import static io.a2a.client.http.A2AHttpClient.APPLICATION_JSON; | ||||||||||||||||||||||||||||||||||
| import static io.a2a.client.http.A2AHttpClient.CONTENT_TYPE; | ||||||||||||||||||||||||||||||||||
| import static io.a2a.common.A2AHeaders.X_A2A_NOTIFICATION_TOKEN; | ||||||||||||||||||||||||||||||||||
| import jakarta.enterprise.context.ApplicationScoped; | ||||||||||||||||||||||||||||||||||
| import jakarta.inject.Inject; | ||||||||||||||||||||||||||||||||||
| import static io.a2a.spec.Message.MESSAGE; | ||||||||||||||||||||||||||||||||||
| import static io.a2a.spec.Task.TASK; | ||||||||||||||||||||||||||||||||||
| import static io.a2a.spec.TaskArtifactUpdateEvent.ARTIFACT_UPDATE; | ||||||||||||||||||||||||||||||||||
| import static io.a2a.spec.TaskStatusUpdateEvent.STATUS_UPDATE; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import java.io.IOException; | ||||||||||||||||||||||||||||||||||
| import java.util.List; | ||||||||||||||||||||||||||||||||||
| import java.util.concurrent.CompletableFuture; | ||||||||||||||||||||||||||||||||||
| import java.util.concurrent.ExecutionException; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import jakarta.enterprise.context.ApplicationScoped; | ||||||||||||||||||||||||||||||||||
| import jakarta.inject.Inject; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import com.fasterxml.jackson.core.JsonProcessingException; | ||||||||||||||||||||||||||||||||||
| import org.slf4j.Logger; | ||||||||||||||||||||||||||||||||||
| import org.slf4j.LoggerFactory; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import io.a2a.client.http.A2AHttpClient; | ||||||||||||||||||||||||||||||||||
| import io.a2a.client.http.JdkA2AHttpClient; | ||||||||||||||||||||||||||||||||||
| import io.a2a.spec.Message; | ||||||||||||||||||||||||||||||||||
| import io.a2a.spec.PushNotificationConfig; | ||||||||||||||||||||||||||||||||||
| import io.a2a.spec.StreamingEventKind; | ||||||||||||||||||||||||||||||||||
| import io.a2a.spec.Task; | ||||||||||||||||||||||||||||||||||
| import io.a2a.spec.TaskArtifactUpdateEvent; | ||||||||||||||||||||||||||||||||||
| import io.a2a.spec.TaskStatusUpdateEvent; | ||||||||||||||||||||||||||||||||||
| import io.a2a.util.Utils; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import org.slf4j.Logger; | ||||||||||||||||||||||||||||||||||
| import org.slf4j.LoggerFactory; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| @ApplicationScoped | ||||||||||||||||||||||||||||||||||
| public class BasePushNotificationSender implements PushNotificationSender { | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
@@ -42,34 +50,44 @@ public BasePushNotificationSender(PushNotificationConfigStore configStore, A2AHt | |||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||
| public void sendNotification(Task task) { | ||||||||||||||||||||||||||||||||||
| List<PushNotificationConfig> pushConfigs = configStore.getInfo(task.getId()); | ||||||||||||||||||||||||||||||||||
| public void sendNotification(StreamingEventKind kind) { | ||||||||||||||||||||||||||||||||||
| String taskId = switch (kind.getKind()) { | ||||||||||||||||||||||||||||||||||
| case TASK -> ((Task)kind).getId(); | ||||||||||||||||||||||||||||||||||
| case MESSAGE -> ((Message)kind).getTaskId(); | ||||||||||||||||||||||||||||||||||
| case STATUS_UPDATE -> ((TaskStatusUpdateEvent)kind).getTaskId(); | ||||||||||||||||||||||||||||||||||
| case ARTIFACT_UPDATE -> ((TaskArtifactUpdateEvent)kind).getTaskId(); | ||||||||||||||||||||||||||||||||||
| default -> null; | ||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||
| if (taskId == null) { | ||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+54
to
+62
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Suggested change
|
||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| List<PushNotificationConfig> pushConfigs = configStore.getInfo(taskId); | ||||||||||||||||||||||||||||||||||
| if (pushConfigs == null || pushConfigs.isEmpty()) { | ||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| List<CompletableFuture<Boolean>> dispatchResults = pushConfigs | ||||||||||||||||||||||||||||||||||
| .stream() | ||||||||||||||||||||||||||||||||||
| .map(pushConfig -> dispatch(task, pushConfig)) | ||||||||||||||||||||||||||||||||||
| .map(pushConfig -> dispatch(kind, pushConfig)) | ||||||||||||||||||||||||||||||||||
| .toList(); | ||||||||||||||||||||||||||||||||||
| CompletableFuture<Void> allFutures = CompletableFuture.allOf(dispatchResults.toArray(new CompletableFuture[0])); | ||||||||||||||||||||||||||||||||||
| CompletableFuture<Boolean> dispatchResult = allFutures.thenApply(v -> dispatchResults.stream() | ||||||||||||||||||||||||||||||||||
| .allMatch(CompletableFuture::join)); | ||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
| boolean allSent = dispatchResult.get(); | ||||||||||||||||||||||||||||||||||
| if (! allSent) { | ||||||||||||||||||||||||||||||||||
| LOGGER.warn("Some push notifications failed to send for taskId: " + task.getId()); | ||||||||||||||||||||||||||||||||||
| LOGGER.warn("Some push notifications failed to send for taskId: " + taskId); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } catch (InterruptedException | ExecutionException e) { | ||||||||||||||||||||||||||||||||||
| LOGGER.warn("Some push notifications failed to send for taskId " + task.getId() + ": {}", e.getMessage(), e); | ||||||||||||||||||||||||||||||||||
| LOGGER.warn("Some push notifications failed to send for taskId " + taskId + ": {}", e.getMessage(), e); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| private CompletableFuture<Boolean> dispatch(Task task, PushNotificationConfig pushInfo) { | ||||||||||||||||||||||||||||||||||
| return CompletableFuture.supplyAsync(() -> dispatchNotification(task, pushInfo)); | ||||||||||||||||||||||||||||||||||
| private CompletableFuture<Boolean> dispatch(StreamingEventKind kind, PushNotificationConfig pushInfo) { | ||||||||||||||||||||||||||||||||||
| return CompletableFuture.supplyAsync(() -> dispatchNotification(kind, pushInfo)); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| private boolean dispatchNotification(Task task, PushNotificationConfig pushInfo) { | ||||||||||||||||||||||||||||||||||
| private boolean dispatchNotification(StreamingEventKind kind, PushNotificationConfig pushInfo) { | ||||||||||||||||||||||||||||||||||
| String url = pushInfo.url(); | ||||||||||||||||||||||||||||||||||
| String token = pushInfo.token(); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
@@ -80,7 +98,7 @@ private boolean dispatchNotification(Task task, PushNotificationConfig pushInfo) | |||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| String body; | ||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
| body = Utils.OBJECT_MAPPER.writeValueAsString(task); | ||||||||||||||||||||||||||||||||||
| body = Utils.marshalFrom(kind); | ||||||||||||||||||||||||||||||||||
| } catch (JsonProcessingException e) { | ||||||||||||||||||||||||||||||||||
| LOGGER.debug("Error writing value as string: {}", e.getMessage(), e); | ||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,15 @@ | ||
| package io.a2a.server.tasks; | ||
|
|
||
| import io.a2a.spec.Task; | ||
| import io.a2a.spec.StreamingEventKind; | ||
|
|
||
| /** | ||
| * Interface for sending push notifications for tasks. | ||
| */ | ||
| public interface PushNotificationSender { | ||
|
|
||
| /** | ||
| * Sends a push notification containing the latest task state. | ||
| * @param task the task | ||
| * Sends a push notification containing payload about a task. | ||
| * @param kind the payload to push | ||
| */ | ||
| void sendNotification(Task task); | ||
| void sendNotification(StreamingEventKind kind); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The switch statement on
kind.getKind()uses adefault -> nullcase, which will cause push notifications for any new or unhandledStreamingEventKindtypes to be silently dropped. This could lead to hard-to-debug issues.Since
StreamingEventKindis a sealed interface, you can use a pattern-matching switch statement on thekindobject itself. This is safer as the compiler will enforce that all permitted subtypes are handled, eliminating the need for a default case and preventing silent failures.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good suggestion but this requires Java 21+ while the project targets Java 17.