Skip to content

Commit 6836c24

Browse files
sumitvairagarsumitvairagar
authored
fix(rest): include missing query params in listTasks and listTaskPushNotificationConfigs (#1239)
* fix(rest): include missing query params in listTasks and listTaskPushNotificationConfigs RestTransport.buildListTasksQueryString() iterated all fields of ListTasksParams except statusTimestampAfter, which was silently dropped from every GET /tasks request, making the server-side timestamp filter unreachable from the Java client. RestTransport.listTaskPushNotificationConfigurations() built its URL as a plain string with no query parameters at all. The pageSize and pageToken fields on ListTaskPushNotificationConfigsParams were ignored, making pagination impossible. Fix: - Add statusTimestampAfter to buildListTasksQueryString(), formatted as ISO-8601 and URL-encoded (same pattern as existing string fields). - Extract buildListPushNotificationConfigsQueryString() for the push notification configs method and append the query string to the URL. - Add regression tests for both bugs using MockServer; each test asserts the query parameter is present in the outbound request. This fixes #1233 * fix(rest): set missing proto builder fields and URL-encode status param Three issues identified in review by @ehsavoie: 1. listTasks proto builder did not set statusTimestampAfter. Interceptors (auth, tracing) receive the builder before the REST URL is built, so missing fields there are invisible to the interceptor chain. Added the Timestamp conversion using getEpochSecond()/getNano(). 2. listTaskPushNotificationConfigurations proto builder set only taskId. pageSize and pageToken were never set, making pagination invisible to interceptors. Added the two missing fields. 3. status enum value in buildListTasksQueryString was appended raw without URL-encoding. Applied URLEncoder.encode() for consistency with all other string parameters in the same method. Added two regression tests using a capturing ClientCallInterceptor to assert the proto builder carries the correct fields before the HTTP request is sent. This fixes #1233 * style: match original file conventions in RestTransport and RestTransportTest - Restore java.util.List<String>/java.util.ArrayList<> FQN style in buildListTasksQueryString and buildListPushNotificationConfigsQueryString to match the pre-existing pattern in the original file - Replace org.a2aproject.sdk.spec.AgentCard FQN with short name in test interceptors (AgentCard is already imported) - Fix AgentInterface import to correct alphabetical position in test file This fixes #1233
1 parent 4e98c62 commit 6836c24

3 files changed

Lines changed: 195 additions & 3 deletions

File tree

‎client/transport/rest/src/main/java/org/a2aproject/sdk/client/transport/rest/RestTransport.java‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,12 @@ public ListTasksResult listTasks(ListTasksParams request, @Nullable ClientCallCo
205205
if (request.includeArtifacts() != null && request.includeArtifacts()) {
206206
builder.setIncludeArtifacts(true);
207207
}
208+
if (request.statusTimestampAfter() != null) {
209+
builder.setStatusTimestampAfter(com.google.protobuf.Timestamp.newBuilder()
210+
.setSeconds(request.statusTimestampAfter().getEpochSecond())
211+
.setNanos(request.statusTimestampAfter().getNano())
212+
.build());
213+
}
208214

209215
PayloadAndHeaders payloadAndHeaders = applyInterceptors(LIST_TASK_METHOD, builder, agentCard, context);
210216

@@ -247,7 +253,7 @@ private String buildListTasksQueryString(ListTasksParams request) {
247253
queryParts.add("contextId=" + URLEncoder.encode(request.contextId(), StandardCharsets.UTF_8));
248254
}
249255
if (request.status() != null) {
250-
queryParts.add("status=" + request.status());
256+
queryParts.add("status=" + URLEncoder.encode(request.status().toString(), StandardCharsets.UTF_8));
251257
}
252258
if (request.pageSize() != null) {
253259
queryParts.add("pageSize=" + request.pageSize());
@@ -258,12 +264,27 @@ private String buildListTasksQueryString(ListTasksParams request) {
258264
if (request.historyLength() != null) {
259265
queryParts.add("historyLength=" + request.historyLength());
260266
}
267+
if (request.statusTimestampAfter() != null) {
268+
queryParts.add("statusTimestampAfter=" + URLEncoder.encode(
269+
request.statusTimestampAfter().toString(), StandardCharsets.UTF_8));
270+
}
261271
if (request.includeArtifacts() != null && request.includeArtifacts()) {
262272
queryParts.add("includeArtifacts=true");
263273
}
264274
return String.join("&", queryParts);
265275
}
266276

277+
private String buildListPushNotificationConfigsQueryString(ListTaskPushNotificationConfigsParams request) {
278+
java.util.List<String> queryParts = new java.util.ArrayList<>();
279+
if (request.pageSize() > 0) {
280+
queryParts.add("pageSize=" + request.pageSize());
281+
}
282+
if (request.pageToken() != null && !request.pageToken().isEmpty()) {
283+
queryParts.add("pageToken=" + URLEncoder.encode(request.pageToken(), StandardCharsets.UTF_8));
284+
}
285+
return String.join("&", queryParts);
286+
}
287+
267288
@Override
268289
public TaskPushNotificationConfig createTaskPushNotificationConfiguration(TaskPushNotificationConfig request, @Nullable ClientCallContext context) throws A2AClientException {
269290
checkNotNullParam("request", request);
@@ -323,11 +344,22 @@ public ListTaskPushNotificationConfigsResult listTaskPushNotificationConfigurati
323344
org.a2aproject.sdk.grpc.ListTaskPushNotificationConfigsRequest.Builder builder
324345
= org.a2aproject.sdk.grpc.ListTaskPushNotificationConfigsRequest.newBuilder();
325346
builder.setTaskId(request.id());
347+
if (request.pageSize() > 0) {
348+
builder.setPageSize(request.pageSize());
349+
}
350+
if (request.pageToken() != null && !request.pageToken().isEmpty()) {
351+
builder.setPageToken(request.pageToken());
352+
}
326353
PayloadAndHeaders payloadAndHeaders = applyInterceptors(LIST_TASK_PUSH_NOTIFICATION_CONFIG_METHOD, builder,
327354
agentCard, context);
328355
try {
329-
String url = Utils.buildBaseUrl(agentInterface, request.tenant()) + String.format("/tasks/%1s/pushNotificationConfigs", request.id());
330-
A2AHttpClient.GetBuilder getBuilder = httpClient.createGet().url(url);
356+
StringBuilder urlBuilder = new StringBuilder(Utils.buildBaseUrl(agentInterface, request.tenant()));
357+
urlBuilder.append(String.format("/tasks/%1s/pushNotificationConfigs", request.id()));
358+
String queryString = buildListPushNotificationConfigsQueryString(request);
359+
if (!queryString.isEmpty()) {
360+
urlBuilder.append("?").append(queryString);
361+
}
362+
A2AHttpClient.GetBuilder getBuilder = httpClient.createGet().url(urlBuilder.toString());
331363
addStandardHeaders(getBuilder, payloadAndHeaders);
332364
A2AHttpResponse response = getBuilder.get();
333365
if (!response.success()) {

‎client/transport/rest/src/test/java/org/a2aproject/sdk/client/transport/rest/JsonRestMessages.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,4 +387,20 @@ public class JsonRestMessages {
387387
"id": "task-1234"
388388
}
389389
}""";
390+
391+
static final String LIST_TASKS_TEST_RESPONSE = """
392+
{
393+
"tasks": [
394+
{
395+
"id": "de38c76d-d54c-436c-8b9f-4c2703648d64",
396+
"contextId": "c295ea44-7543-4f78-b524-7a38915ad6e4",
397+
"status": {
398+
"state": "TASK_STATE_COMPLETED"
399+
},
400+
"metadata": {}
401+
}
402+
],
403+
"totalSize": 1,
404+
"nextPageToken": ""
405+
}""";
390406
}

‎client/transport/rest/src/test/java/org/a2aproject/sdk/client/transport/rest/RestTransportTest.java‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import static org.a2aproject.sdk.client.transport.rest.JsonRestMessages.GET_TASK_PUSH_NOTIFICATION_CONFIG_TEST_RESPONSE;
77
import static org.a2aproject.sdk.client.transport.rest.JsonRestMessages.GET_TASK_TEST_RESPONSE;
88
import static org.a2aproject.sdk.client.transport.rest.JsonRestMessages.LIST_TASK_PUSH_NOTIFICATION_CONFIG_TEST_RESPONSE;
9+
import static org.a2aproject.sdk.client.transport.rest.JsonRestMessages.LIST_TASKS_TEST_RESPONSE;
910
import static org.a2aproject.sdk.client.transport.rest.JsonRestMessages.SEND_MESSAGE_STREAMING_TEST_REQUEST;
1011
import static org.a2aproject.sdk.client.transport.rest.JsonRestMessages.SEND_MESSAGE_STREAMING_TEST_RESPONSE;
1112
import static org.a2aproject.sdk.client.transport.rest.JsonRestMessages.SEND_MESSAGE_TEST_REQUEST;
@@ -22,6 +23,7 @@
2223
import static org.mockserver.model.HttpResponse.response;
2324

2425
import java.io.IOException;
26+
import java.time.Instant;
2527
import java.util.Collections;
2628
import java.util.List;
2729
import java.util.concurrent.CountDownLatch;
@@ -31,9 +33,12 @@
3133
import java.util.logging.Logger;
3234

3335
import org.a2aproject.sdk.client.transport.spi.interceptors.ClientCallContext;
36+
import org.a2aproject.sdk.client.transport.spi.interceptors.ClientCallInterceptor;
37+
import org.a2aproject.sdk.client.transport.spi.interceptors.PayloadAndHeaders;
3438
import org.a2aproject.sdk.spec.A2AClientException;
3539
import org.a2aproject.sdk.spec.AgentCapabilities;
3640
import org.a2aproject.sdk.spec.AgentCard;
41+
import org.a2aproject.sdk.spec.AgentInterface;
3742
import org.a2aproject.sdk.spec.AgentSkill;
3843
import org.a2aproject.sdk.spec.Artifact;
3944
import org.a2aproject.sdk.spec.AuthenticationInfo;
@@ -48,6 +53,7 @@
4853
import org.a2aproject.sdk.spec.GetTaskPushNotificationConfigParams;
4954
import org.a2aproject.sdk.spec.ListTaskPushNotificationConfigsParams;
5055
import org.a2aproject.sdk.spec.ListTaskPushNotificationConfigsResult;
56+
import org.a2aproject.sdk.spec.ListTasksParams;
5157
import org.a2aproject.sdk.spec.Message;
5258
import org.a2aproject.sdk.spec.MessageSendConfiguration;
5359
import org.a2aproject.sdk.spec.MessageSendParams;
@@ -717,4 +723,142 @@ public void testVersionNotSupportedErrorUnmarshallingStreaming() throws Exceptio
717723
assertTrue(versionError.getMessage().contains("2.0"));
718724
assertTrue(versionError.getMessage().contains("1.0"));
719725
}
726+
727+
/**
728+
* Regression test for #1233: listTasks proto builder must set statusTimestampAfter so
729+
* interceptors (auth, tracing, etc.) see the complete request.
730+
*/
731+
@Test
732+
public void testListTasksProtoBuilderIncludesStatusTimestampAfter() throws Exception {
733+
Instant cutoff = Instant.parse("2026-10-01T00:00:00Z");
734+
AtomicReference<Object> capturedPayload = new AtomicReference<>();
735+
736+
ClientCallInterceptor capturingInterceptor = new ClientCallInterceptor() {
737+
@Override
738+
public PayloadAndHeaders intercept(String methodName, Object payload,
739+
java.util.Map<String, String> headers, AgentCard agentCard,
740+
ClientCallContext clientCallContext) {
741+
capturedPayload.set(payload);
742+
return new PayloadAndHeaders(payload, headers);
743+
}
744+
};
745+
746+
this.server.when(
747+
request().withMethod("GET").withPath("/tasks")
748+
).respond(response().withStatusCode(200).withBody(LIST_TASKS_TEST_RESPONSE));
749+
750+
AgentInterface agentInterface = new AgentInterface("HTTP+JSON", "http://localhost:4001");
751+
RestTransport client = new RestTransport(null, CARD, agentInterface, List.of(capturingInterceptor));
752+
753+
ListTasksParams params = ListTasksParams.builder().statusTimestampAfter(cutoff).build();
754+
client.listTasks(params, null);
755+
756+
assertNotNull(capturedPayload.get());
757+
org.a2aproject.sdk.grpc.ListTasksRequest proto =
758+
((org.a2aproject.sdk.grpc.ListTasksRequest.Builder) capturedPayload.get()).build();
759+
assertTrue(proto.hasStatusTimestampAfter(), "proto builder must set statusTimestampAfter");
760+
assertEquals(cutoff.getEpochSecond(), proto.getStatusTimestampAfter().getSeconds());
761+
assertEquals(cutoff.getNano(), proto.getStatusTimestampAfter().getNanos());
762+
}
763+
764+
/**
765+
* Regression test for #1233: listTaskPushNotificationConfigurations proto builder must set
766+
* pageSize and pageToken so interceptors see the complete request.
767+
*/
768+
@Test
769+
public void testListTaskPushNotificationConfigurationsProtoBuilderIncludesPagination() throws Exception {
770+
AtomicReference<Object> capturedPayload = new AtomicReference<>();
771+
772+
ClientCallInterceptor capturingInterceptor = new ClientCallInterceptor() {
773+
@Override
774+
public PayloadAndHeaders intercept(String methodName, Object payload,
775+
java.util.Map<String, String> headers, AgentCard agentCard,
776+
ClientCallContext clientCallContext) {
777+
capturedPayload.set(payload);
778+
return new PayloadAndHeaders(payload, headers);
779+
}
780+
};
781+
782+
this.server.when(
783+
request().withMethod("GET")
784+
.withPath("/tasks/de38c76d-d54c-436c-8b9f-4c2703648d64/pushNotificationConfigs")
785+
).respond(response().withStatusCode(200).withBody(LIST_TASK_PUSH_NOTIFICATION_CONFIG_TEST_RESPONSE));
786+
787+
AgentInterface agentInterface = new AgentInterface("HTTP+JSON", "http://localhost:4001");
788+
RestTransport client = new RestTransport(null, CARD, agentInterface, List.of(capturingInterceptor));
789+
790+
ListTaskPushNotificationConfigsParams params = ListTaskPushNotificationConfigsParams.builder()
791+
.id("de38c76d-d54c-436c-8b9f-4c2703648d64")
792+
.pageSize(10)
793+
.pageToken("next-page-token")
794+
.build();
795+
client.listTaskPushNotificationConfigurations(params, null);
796+
797+
assertNotNull(capturedPayload.get());
798+
org.a2aproject.sdk.grpc.ListTaskPushNotificationConfigsRequest proto =
799+
((org.a2aproject.sdk.grpc.ListTaskPushNotificationConfigsRequest.Builder) capturedPayload.get()).build();
800+
assertEquals(10, proto.getPageSize(), "proto builder must set pageSize");
801+
assertEquals("next-page-token", proto.getPageToken(), "proto builder must set pageToken");
802+
}
803+
804+
/**
805+
* Regression test for #1233: listTasks must include statusTimestampAfter in the query string.
806+
* The mock server only responds when the parameter is present; without the fix the mock will
807+
* not match and the client throws an exception, causing the assertion to fail.
808+
*/
809+
@Test
810+
public void testListTasksIncludesStatusTimestampAfterQueryParam() throws Exception {
811+
Instant cutoff = Instant.parse("2026-10-01T00:00:00Z");
812+
this.server.when(
813+
request()
814+
.withMethod("GET")
815+
.withPath("/tasks")
816+
.withQueryStringParameter("statusTimestampAfter", "2026-10-01T00:00:00Z")
817+
)
818+
.respond(
819+
response()
820+
.withStatusCode(200)
821+
.withBody(LIST_TASKS_TEST_RESPONSE)
822+
);
823+
824+
RestTransport client = new RestTransport(CARD);
825+
ListTasksParams params = ListTasksParams.builder()
826+
.statusTimestampAfter(cutoff)
827+
.build();
828+
829+
var result = client.listTasks(params, null);
830+
assertEquals(1, result.tasks().size());
831+
assertEquals("de38c76d-d54c-436c-8b9f-4c2703648d64", result.tasks().get(0).id());
832+
}
833+
834+
/**
835+
* Regression test for #1233: listTaskPushNotificationConfigurations must include pageSize and
836+
* pageToken in the query string. The mock server only responds when both parameters are present;
837+
* without the fix the mock will not match and the client throws an exception.
838+
*/
839+
@Test
840+
public void testListTaskPushNotificationConfigurationsIncludesPaginationQueryParams() throws Exception {
841+
this.server.when(
842+
request()
843+
.withMethod("GET")
844+
.withPath("/tasks/de38c76d-d54c-436c-8b9f-4c2703648d64/pushNotificationConfigs")
845+
.withQueryStringParameter("pageSize", "10")
846+
.withQueryStringParameter("pageToken", "next-page-token")
847+
)
848+
.respond(
849+
response()
850+
.withStatusCode(200)
851+
.withBody(LIST_TASK_PUSH_NOTIFICATION_CONFIG_TEST_RESPONSE)
852+
);
853+
854+
RestTransport client = new RestTransport(CARD);
855+
ListTaskPushNotificationConfigsParams params = ListTaskPushNotificationConfigsParams.builder()
856+
.id("de38c76d-d54c-436c-8b9f-4c2703648d64")
857+
.pageSize(10)
858+
.pageToken("next-page-token")
859+
.build();
860+
861+
ListTaskPushNotificationConfigsResult result = client.listTaskPushNotificationConfigurations(params, null);
862+
assertEquals(2, result.configs().size());
863+
}
720864
}

0 commit comments

Comments
 (0)