Skip to content

fix: Encapsulate the payload of Push notifications #492

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+44 −17
Open

fix: Encapsulate the payload of Push notifications #492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 6 additions & 6 deletions server-common/src/main/java/io/a2a/server/tasks/BasePushNotificationSender.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,25 @@
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 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.PushNotificationConfig;
import io.a2a.spec.Task;
import io.a2a.util.Utils;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

@ApplicationScoped
public class BasePushNotificationSender implements PushNotificationSender {

Expand Down Expand Up @@ -80,7 +80,7 @@ private boolean dispatchNotification(Task task, PushNotificationConfig pushInfo)

String body;
try {
body = Utils.OBJECT_MAPPER.writeValueAsString(task);
body = Utils.marshalFrom(task);
} catch (JsonProcessingException e) {
LOGGER.debug("Error writing value as string: {}", e.getMessage(), e);
return false;
Expand Down
21 changes: 13 additions & 8 deletions server-common/src/test/java/io/a2a/server/requesthandlers/AbstractA2ARequestHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CountDownLatch;
Expand All @@ -15,6 +16,12 @@

import jakarta.enterprise.context.Dependent;

import com.fasterxml.jackson.databind.JsonNode;
import io.quarkus.arc.profile.IfBuildProfile;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;

import io.a2a.client.http.A2AHttpClient;
import io.a2a.client.http.A2AHttpResponse;
import io.a2a.server.agentexecution.AgentExecutor;
Expand All @@ -30,20 +37,14 @@
import io.a2a.server.tasks.TaskStore;
import io.a2a.spec.AgentCapabilities;
import io.a2a.spec.AgentCard;
import io.a2a.spec.Event;
import io.a2a.spec.JSONRPCError;
import io.a2a.spec.Message;
import io.a2a.spec.Task;
import io.a2a.spec.TaskState;
import io.a2a.spec.TaskStatus;
import io.a2a.spec.Event;
import io.a2a.spec.TextPart;
import io.a2a.util.Utils;
import io.quarkus.arc.profile.IfBuildProfile;
import java.util.Map;

import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;

public class AbstractA2ARequestHandlerTest {

Expand Down Expand Up @@ -199,7 +200,11 @@ public PostBuilder body(String body) {

@Override
public A2AHttpResponse post() throws IOException, InterruptedException {
tasks.add(Utils.OBJECT_MAPPER.readValue(body, Task.TYPE_REFERENCE));
JsonNode root = Utils.OBJECT_MAPPER.readTree(body);
// This will need to be updated for #490 to unmarshall based on the kind of payload
JsonNode taskNode = root.elements().next();
Task task = Utils.OBJECT_MAPPER.treeToValue(taskNode, Task.TYPE_REFERENCE);
Comment on lines 201 to +206
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 27, 2025

Choose a reason for hiding this comment

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

medium

This block of code for deserializing the wrapped task has a couple of areas for improvement:

  1. Robustness: root.elements().next() will throw a NoSuchElementException if the JSON object is empty. It's better to validate that the object contains exactly one field, as per the specification, for example by checking root.size().

  2. Code Duplication: This logic is duplicated in PushNotificationSenderTest.java.

To address both points and improve maintainability, you could extract this logic into a new, more robust helper method in io.a2a.util.Utils. This method would be the inverse of marshalFrom. For example:

// In Utils.java
public static <T> T unmarshalWrappedFrom(String data, TypeReference<T> typeRef) throws JsonProcessingException {
    JsonNode root = OBJECT_MAPPER.readTree(data);
    if (root.size() != 1) {
        throw new com.fasterxml.jackson.databind.exc.MismatchedInputException(null, "Expected a single field in the JSON payload, but found " + root.size());
    }
    JsonNode payloadNode = root.elements().next();
    return OBJECT_MAPPER.treeToValue(payloadNode, typeRef);
}

Then you could simplify this block to just:

tasks.add(Utils.unmarshalWrappedFrom(body, Task.TYPE_REFERENCE));

Copy link
Collaborator Author

@jmesnil jmesnil Nov 27, 2025

Choose a reason for hiding this comment

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

this code will be refined for #490 when additional kind of payload will be supported and will follow these guidelines

tasks.add(task);
try {
return new A2AHttpResponse() {
@Override
Expand Down
11 changes: 8 additions & 3 deletions server-common/src/test/java/io/a2a/server/tasks/PushNotificationSenderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package io.a2a.server.tasks;

import static io.a2a.client.http.A2AHttpClient.APPLICATION_JSON;
import static io.a2a.client.http.A2AHttpClient.APPLICATION_JSON;
import static io.a2a.client.http.A2AHttpClient.CONTENT_TYPE;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
Expand All @@ -16,17 +16,18 @@
import java.util.concurrent.TimeUnit;
import java.util.function.Consumer;

import com.fasterxml.jackson.databind.JsonNode;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import io.a2a.client.http.A2AHttpClient;
import io.a2a.client.http.A2AHttpResponse;
import io.a2a.common.A2AHeaders;
import io.a2a.util.Utils;
import io.a2a.spec.PushNotificationConfig;
import io.a2a.spec.Task;
import io.a2a.spec.TaskState;
import io.a2a.spec.TaskStatus;
import io.a2a.util.Utils;

public class PushNotificationSenderTest {

Expand Down Expand Up @@ -77,7 +78,11 @@ public A2AHttpResponse post() throws IOException, InterruptedException {
}

try {
Task task = Utils.OBJECT_MAPPER.readValue(body, Task.TYPE_REFERENCE);
JsonNode root = Utils.OBJECT_MAPPER.readTree(body);
// This assumes there is always one field in the outer JSON object.
// This will need to be updated for #490 to unmarshall based on the kind of payload
JsonNode taskNode = root.elements().next();
Task task = Utils.OBJECT_MAPPER.treeToValue(taskNode, Task.TYPE_REFERENCE);
Comment on lines 80 to +85
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 27, 2025

Choose a reason for hiding this comment

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

medium

This block of code for deserializing the wrapped task has a couple of areas for improvement:

  1. Robustness: root.elements().next() will throw a NoSuchElementException if the JSON object is empty. It's better to validate that the object contains exactly one field, as per the specification, for example by checking root.size().

  2. Code Duplication: This logic is duplicated in AbstractA2ARequestHandlerTest.java.

To address both points and improve maintainability, you could extract this logic into a new, more robust helper method in io.a2a.util.Utils. This method would be the inverse of marshalFrom. For example:

// In Utils.java
public static <T> T unmarshalWrappedFrom(String data, TypeReference<T> typeRef) throws JsonProcessingException {
    JsonNode root = OBJECT_MAPPER.readTree(data);
    if (root.size() != 1) {
        throw new com.fasterxml.jackson.databind.exc.MismatchedInputException(null, "Expected a single field in the JSON payload, but found " + root.size());
    }
    JsonNode payloadNode = root.elements().next();
    return OBJECT_MAPPER.treeToValue(payloadNode, typeRef);
}

Then you could simplify this block to just:

tasks.add(Utils.unmarshalWrappedFrom(body, Task.TYPE_REFERENCE));

Copy link
Collaborator Author

@jmesnil jmesnil Nov 27, 2025

Choose a reason for hiding this comment

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

idem, will be refined for #490

tasks.add(task);
urls.add(url);
headers.add(new java.util.HashMap<>(requestHeaders));
Expand Down
17 changes: 17 additions & 0 deletions spec/src/main/java/io/a2a/util/Utils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.logging.Logger;

import com.fasterxml.jackson.core.JsonProcessingException;
Expand All @@ -11,6 +12,7 @@

import io.a2a.spec.Artifact;
import io.a2a.spec.Part;
import io.a2a.spec.StreamingEventKind;
import io.a2a.spec.Task;
import io.a2a.spec.TaskArtifactUpdateEvent;

Expand Down Expand Up @@ -69,6 +71,21 @@ public static <T> T unmarshalFrom(String data, TypeReference<T> typeRef) throws
return OBJECT_MAPPER.readValue(data, typeRef);
}

/**
* Serializes a StreamingEventKind in a JSON string
* <p>
* The StreamingEventKind object is wrapped in a JSON field named from its kind (e.g. "task") before
* it is serialized
*
* @param kind the StreamingEventKind to deserialize
* @return a JSON String
* @throws JsonProcessingException if JSON parsing fails
*/
public static String marshalFrom(StreamingEventKind kind) throws JsonProcessingException {
Map<String, StreamingEventKind> wrapper = Map.of(kind.getKind(), kind);
return OBJECT_MAPPER.writeValueAsString(wrapper);
}

/**
* Returns the provided value if non-null, otherwise returns the default value.
* <p>
Expand Down