diff --git a/.gitignore b/.gitignore index dcc2f1cb8..bc62b4d6b 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ pom.xml.releaseBackup pom.xml.versionsBackup release.properties .flattened-pom.xml +*.args # Eclipse .project diff --git a/extras/task-store-database-jpa/src/main/java/io/a2a/extras/taskstore/database/jpa/JpaDatabaseTaskStore.java b/extras/task-store-database-jpa/src/main/java/io/a2a/extras/taskstore/database/jpa/JpaDatabaseTaskStore.java index 49dad1728..840773ea6 100644 --- a/extras/task-store-database-jpa/src/main/java/io/a2a/extras/taskstore/database/jpa/JpaDatabaseTaskStore.java +++ b/extras/task-store-database-jpa/src/main/java/io/a2a/extras/taskstore/database/jpa/JpaDatabaseTaskStore.java @@ -226,6 +226,25 @@ public ListTasksResult list(ListTasksParams params) { LOGGER.debug("Listing tasks with params: contextId={}, status={}, pageSize={}, pageToken={}", params.contextId(), params.status(), params.pageSize(), params.pageToken()); + // Parse pageToken once at the beginning + Instant tokenTimestamp = null; + String tokenId = null; + if (params.pageToken() != null && !params.pageToken().isEmpty()) { + String[] tokenParts = params.pageToken().split(":", 2); + if (tokenParts.length == 2) { + try { + long timestampMillis = Long.parseLong(tokenParts[0]); + tokenId = tokenParts[1]; + tokenTimestamp = Instant.ofEpochMilli(timestampMillis); + } catch (NumberFormatException e) { + throw new io.a2a.spec.InvalidParamsError(null, + "Invalid pageToken format: timestamp must be numeric milliseconds", null); + } + } else { + throw new io.a2a.spec.InvalidParamsError(null, "Invalid pageToken format: expected 'timestamp:id'", null); + } + } + // Build dynamic JPQL query with WHERE clauses for filtering StringBuilder queryBuilder = new StringBuilder("SELECT t FROM JpaTask t WHERE 1=1"); StringBuilder countQueryBuilder = new StringBuilder("SELECT COUNT(t) FROM JpaTask t WHERE 1=1"); @@ -249,18 +268,9 @@ public ListTasksResult list(ListTasksParams params) { } // Apply pagination cursor using keyset pagination for composite sort (timestamp DESC, id ASC) - // PageToken format: "timestamp_millis:taskId" (e.g., "1699999999000:task-123") - if (params.pageToken() != null && !params.pageToken().isEmpty()) { - String[] tokenParts = params.pageToken().split(":", 2); - if (tokenParts.length == 2) { - // Keyset pagination: get tasks where timestamp < tokenTimestamp OR (timestamp = tokenTimestamp AND id > tokenId) - // All tasks have timestamps (TaskStatus canonical constructor ensures this) - queryBuilder.append(" AND (t.statusTimestamp < :tokenTimestamp OR (t.statusTimestamp = :tokenTimestamp AND t.id > :tokenId))"); - } else { - // Legacy ID-only pageToken format is not supported with timestamp-based sorting - // Throw error to prevent incorrect pagination results - throw new io.a2a.spec.InvalidParamsError(null, "Invalid pageToken format: expected 'timestamp:id'", null); - } + if (tokenTimestamp != null) { + // Keyset pagination: get tasks where timestamp < tokenTimestamp OR (timestamp = tokenTimestamp AND id > tokenId) + queryBuilder.append(" AND (t.statusTimestamp < :tokenTimestamp OR (t.statusTimestamp = :tokenTimestamp AND t.id > :tokenId))"); } // Sort by status timestamp descending (most recent first), then by ID for stable ordering @@ -279,25 +289,9 @@ public ListTasksResult list(ListTasksParams params) { if (params.lastUpdatedAfter() != null) { query.setParameter("lastUpdatedAfter", params.lastUpdatedAfter()); } - if (params.pageToken() != null && !params.pageToken().isEmpty()) { - String[] tokenParts = params.pageToken().split(":", 2); - if (tokenParts.length == 2) { - // Parse keyset pagination parameters - try { - long timestampMillis = Long.parseLong(tokenParts[0]); - String tokenId = tokenParts[1]; - - // All tasks have timestamps (TaskStatus canonical constructor ensures this) - Instant tokenTimestamp = Instant.ofEpochMilli(timestampMillis); - query.setParameter("tokenTimestamp", tokenTimestamp); - query.setParameter("tokenId", tokenId); - } catch (NumberFormatException e) { - // Malformed timestamp in pageToken - throw new io.a2a.spec.InvalidParamsError(null, - "Invalid pageToken format: timestamp must be numeric milliseconds", null); - } - } - // Note: Legacy ID-only format already rejected in query building phase + if (tokenTimestamp != null) { + query.setParameter("tokenTimestamp", tokenTimestamp); + query.setParameter("tokenId", tokenId); } // Apply page size limit (+1 to check for next page) @@ -362,7 +356,10 @@ public ListTasksResult list(ListTasksParams params) { private Task transformTask(Task task, int historyLength, boolean includeArtifacts) { // Limit history if needed (keep most recent N messages) List history = task.history(); - if (historyLength > 0 && history != null && history.size() > historyLength) { + if (historyLength == 0) { + // When historyLength is 0, return empty history + history = List.of(); + } else if (historyLength > 0 && history != null && history.size() > historyLength) { history = history.subList(history.size() - historyLength, history.size()); } diff --git a/server-common/src/main/java/io/a2a/server/tasks/InMemoryTaskStore.java b/server-common/src/main/java/io/a2a/server/tasks/InMemoryTaskStore.java index d2d9362ca..f1243e272 100644 --- a/server-common/src/main/java/io/a2a/server/tasks/InMemoryTaskStore.java +++ b/server-common/src/main/java/io/a2a/server/tasks/InMemoryTaskStore.java @@ -90,18 +90,24 @@ public ListTasksResult list(ListTasksParams params) { int mid = left + (right - left) / 2; Task task = allFilteredTasks.get(mid); - // All tasks have timestamps (TaskStatus canonical constructor ensures this) - // Truncate to milliseconds for consistency with pageToken precision - java.time.Instant taskTimestamp = task.status().timestamp().toInstant() - .truncatedTo(java.time.temporal.ChronoUnit.MILLIS); - int timestampCompare = taskTimestamp.compareTo(tokenTimestamp); - - if (timestampCompare < 0 || (timestampCompare == 0 && task.id().compareTo(tokenId) > 0)) { - // This task is after the token, search left half - right = mid; - } else { - // This task is before or equal to token, search right half + java.time.Instant taskTimestamp = (task.status() != null && task.status().timestamp() != null) + ? task.status().timestamp().toInstant().truncatedTo(java.time.temporal.ChronoUnit.MILLIS) + : null; + + if (taskTimestamp == null) { + // Task with null timestamp is always "before" a token with a timestamp, as they are sorted last. + // So, we search in the right half. left = mid + 1; + } else { + int timestampCompare = taskTimestamp.compareTo(tokenTimestamp); + + if (timestampCompare < 0 || (timestampCompare == 0 && task.id().compareTo(tokenId) > 0)) { + // This task is after the token, search left half + right = mid; + } else { + // This task is before or equal to token, search right half + left = mid + 1; + } } } startIndex = left; @@ -144,7 +150,10 @@ public ListTasksResult list(ListTasksParams params) { private Task transformTask(Task task, int historyLength, boolean includeArtifacts) { // Limit history if needed (keep most recent N messages) List history = task.history(); - if (historyLength > 0 && history != null && history.size() > historyLength) { + if (historyLength == 0) { + // When historyLength is 0, return empty history + history = List.of(); + } else if (historyLength > 0 && history != null && history.size() > historyLength) { history = history.subList(history.size() - historyLength, history.size()); } diff --git a/spec-grpc/src/main/java/io/a2a/grpc/mapper/A2ACommonFieldMapper.java b/spec-grpc/src/main/java/io/a2a/grpc/mapper/A2ACommonFieldMapper.java index c3f031e97..723df2e53 100644 --- a/spec-grpc/src/main/java/io/a2a/grpc/mapper/A2ACommonFieldMapper.java +++ b/spec-grpc/src/main/java/io/a2a/grpc/mapper/A2ACommonFieldMapper.java @@ -285,7 +285,23 @@ default Map metadataFromProto(Struct struct) { */ @Named("zeroToNull") default Integer zeroToNull(int value) { - return value > 0 ? value : null; + return value != 0 ? value : null; + } + + /** + * Converts protobuf int to Integer, preserving all values including 0. + *

+ * Unlike zeroToNull, this method preserves 0 values, allowing compact constructor + * validation to catch invalid values (e.g., pageSize=0 must fail validation). + * For truly optional fields where 0 means "unset", use zeroToNull instead. + * Use this with {@code @Mapping(qualifiedByName = "intToIntegerOrNull")}. + * + * @param value the protobuf int value + * @return Integer (never null for primitive int input) + */ + @Named("intToIntegerOrNull") + default Integer intToIntegerOrNull(int value) { + return value; } /** @@ -337,13 +353,20 @@ default long instantToMillis(Instant instant) { * Converts protobuf milliseconds-since-epoch (int64) to domain Instant. *

* Returns null if input is 0 (protobuf default for unset field). + * Throws InvalidParamsError for negative values (invalid timestamps). * Use this with {@code @Mapping(qualifiedByName = "millisToInstant")}. * * @param millis milliseconds since epoch * @return domain Instant, or null if millis is 0 + * @throws InvalidParamsError if millis is negative */ @Named("millisToInstant") default Instant millisToInstant(long millis) { + if (millis < 0L) { + throw new InvalidParamsError(null, + "Timestamp must be a non-negative number of milliseconds since epoch, but got: " + millis, + null); + } return millis > 0L ? Instant.ofEpochMilli(millis) : null; } @@ -351,21 +374,32 @@ default Instant millisToInstant(long millis) { // Enum Conversions (handling UNSPECIFIED/UNKNOWN) // ======================================================================== /** - * Converts protobuf TaskState to domain TaskState, treating UNSPECIFIED/UNKNOWN as null. + * Converts protobuf TaskState to domain TaskState, treating UNSPECIFIED as null. *

- * Protobuf enums default to UNSPECIFIED (0 value) when unset. The domain may also have - * UNKNOWN for unparseable values. Both should map to null for optional fields. + * Protobuf enums default to UNSPECIFIED (0 value) when unset, which maps to null for optional fields. + * However, UNRECOGNIZED (invalid enum values from JSON) throws InvalidParamsError for proper validation. * Use this with {@code @Mapping(qualifiedByName = "taskStateOrNull")}. * * @param state the protobuf TaskState - * @return domain TaskState or null if UNSPECIFIED/UNKNOWN + * @return domain TaskState or null if UNSPECIFIED + * @throws InvalidParamsError if state is UNRECOGNIZED (invalid enum value) */ @Named("taskStateOrNull") default io.a2a.spec.TaskState taskStateOrNull(io.a2a.grpc.TaskState state) { if (state == null || state == io.a2a.grpc.TaskState.TASK_STATE_UNSPECIFIED) { return null; } + // Reject invalid enum values (e.g., "INVALID_STATUS" from JSON) + if (state == io.a2a.grpc.TaskState.UNRECOGNIZED) { + String validStates = java.util.Arrays.stream(io.a2a.spec.TaskState.values()) + .filter(s -> s != io.a2a.spec.TaskState.UNKNOWN) + .map(Enum::name) + .collect(java.util.stream.Collectors.joining(", ")); + throw new InvalidParamsError(null, + "Invalid task state value. Must be one of: " + validStates, + null); + } io.a2a.spec.TaskState result = TaskStateMapper.INSTANCE.fromProto(state); - return result == io.a2a.spec.TaskState.UNKNOWN ? null : result; + return result; } } diff --git a/spec-grpc/src/main/java/io/a2a/grpc/mapper/ListTasksParamsMapper.java b/spec-grpc/src/main/java/io/a2a/grpc/mapper/ListTasksParamsMapper.java index 05d45d68a..3e666a8e5 100644 --- a/spec-grpc/src/main/java/io/a2a/grpc/mapper/ListTasksParamsMapper.java +++ b/spec-grpc/src/main/java/io/a2a/grpc/mapper/ListTasksParamsMapper.java @@ -38,7 +38,8 @@ public interface ListTasksParamsMapper { */ @Mapping(target = "contextId", source = "contextId", qualifiedByName = "emptyToNull") @Mapping(target = "status", source = "status", qualifiedByName = "taskStateOrNull") - @Mapping(target = "pageSize", source = "pageSize", qualifiedByName = "zeroToNull") + // pageSize: Check if field is set using hasPageSize() to distinguish unset (null) from explicit 0 (validation error) + @Mapping(target = "pageSize", expression = "java(request.hasPageSize() ? request.getPageSize() : null)") @Mapping(target = "pageToken", source = "pageToken", qualifiedByName = "emptyToNull") @Mapping(target = "historyLength", source = "historyLength", qualifiedByName = "zeroToNull") @Mapping(target = "lastUpdatedAfter", source = "lastUpdatedAfter", qualifiedByName = "millisToInstant") diff --git a/spec-grpc/src/main/java/io/a2a/grpc/utils/JSONRPCUtils.java b/spec-grpc/src/main/java/io/a2a/grpc/utils/JSONRPCUtils.java index ad46a8f38..c7d1df72f 100644 --- a/spec-grpc/src/main/java/io/a2a/grpc/utils/JSONRPCUtils.java +++ b/spec-grpc/src/main/java/io/a2a/grpc/utils/JSONRPCUtils.java @@ -458,14 +458,12 @@ private static JsonProcessingException convertProtoBufExceptionToJsonProcessingE return new JsonProcessingException(ERROR_MESSAGE.formatted("unknown parsing error")); } - // Extract field name if present in error message - String prefix = "Cannot find field: "; - if (message.contains(prefix)) { - return new InvalidParamsJsonMappingException(ERROR_MESSAGE.formatted(message.substring(message.indexOf(prefix) + prefix.length())), id); - } - prefix = "Invalid value for"; - if (message.contains(prefix)) { - return new InvalidParamsJsonMappingException(ERROR_MESSAGE.formatted(message.substring(message.indexOf(prefix) + prefix.length())), id); + // Extract field name if present in error message - check common prefixes + String[] prefixes = {"Cannot find field: ", "Invalid value for", "Invalid enum value:"}; + for (String prefix : prefixes) { + if (message.contains(prefix)) { + return new InvalidParamsJsonMappingException(ERROR_MESSAGE.formatted(message.substring(message.indexOf(prefix) + prefix.length())), id); + } } // Try to extract specific error details using regex patterns @@ -548,7 +546,7 @@ public static String toJsonRPCRequest(@Nullable String requestId, String method, output.name("method").value(method); } if (payload != null) { - String resultValue = JsonFormat.printer().omittingInsignificantWhitespace().print(payload); + String resultValue = JsonFormat.printer().includingDefaultValueFields().omittingInsignificantWhitespace().print(payload); output.name("params").jsonValue(resultValue); } output.endObject(); @@ -571,7 +569,7 @@ public static String toJsonRPCResultResponse(Object requestId, com.google.protob output.name("id").value(number.longValue()); } } - String resultValue = JsonFormat.printer().omittingInsignificantWhitespace().print(builder); + String resultValue = JsonFormat.printer().includingDefaultValueFields().omittingInsignificantWhitespace().print(builder); output.name("result").jsonValue(resultValue); output.endObject(); return result.toString(); diff --git a/spec/src/main/java/io/a2a/spec/ListTasksParams.java b/spec/src/main/java/io/a2a/spec/ListTasksParams.java index 6813dcf45..6fb9ff5d1 100644 --- a/spec/src/main/java/io/a2a/spec/ListTasksParams.java +++ b/spec/src/main/java/io/a2a/spec/ListTasksParams.java @@ -2,7 +2,6 @@ import java.time.Instant; -import io.a2a.util.Assert; import org.jspecify.annotations.Nullable; /** @@ -27,9 +26,12 @@ public record ListTasksParams( @Nullable Boolean includeArtifacts, String tenant ) { + private static final int MIN_PAGE_SIZE = 1; + private static final int MAX_PAGE_SIZE = 100; + private static final int DEFAULT_PAGE_SIZE = 50; /** * Compact constructor for validation. - * Validates that the tenant parameter is not null. + * Validates that the tenant parameter is not null and parameters are within valid ranges. * * @param contextId filter by context ID * @param status filter by task status @@ -39,9 +41,24 @@ public record ListTasksParams( * @param lastUpdatedAfter filter by last update timestamp * @param includeArtifacts whether to include artifacts * @param tenant the tenant identifier + * @throws InvalidParamsError if tenant is null or if pageSize or historyLength are out of valid range */ public ListTasksParams { - Assert.checkNotNullParam("tenant", tenant); + if (tenant == null) { + throw new InvalidParamsError(null, "Parameter 'tenant' may not be null", null); + } + + // Validate pageSize (1-100) + if (pageSize != null && (pageSize < MIN_PAGE_SIZE || pageSize > MAX_PAGE_SIZE)) { + throw new InvalidParamsError(null, + "pageSize must be between " + MIN_PAGE_SIZE + " and " + MAX_PAGE_SIZE + ", got: " + pageSize, null); + } + + // Validate historyLength (>= 0) + if (historyLength != null && historyLength < 0) { + throw new InvalidParamsError(null, + "historyLength must be non-negative, got: " + historyLength, null); + } } /** * Default constructor for listing all tasks. @@ -61,33 +78,23 @@ public ListTasksParams(Integer pageSize, String pageToken) { } /** - * Validates and returns the effective page size (between 1 and 100, defaults to 50). + * Returns the effective page size (defaults to 50 if not specified). + * Values are validated in the constructor to be within the range [1, 100]. * * @return the effective page size */ public int getEffectivePageSize() { - if (pageSize == null) { - return 50; - } - if (pageSize < 1) { - return 1; - } - if (pageSize > 100) { - return 100; - } - return pageSize; + return pageSize != null ? pageSize : DEFAULT_PAGE_SIZE; } /** - * Returns the effective history length (non-negative, defaults to 0). + * Returns the effective history length (defaults to 0 if not specified). + * Values are validated in the constructor to be non-negative. * * @return the effective history length */ public int getEffectiveHistoryLength() { - if (historyLength == null || historyLength < 0) { - return 0; - } - return historyLength; + return historyLength != null ? historyLength : 0; } /** diff --git a/transport/grpc/src/test/java/io/a2a/transport/grpc/handler/GrpcHandlerTest.java b/transport/grpc/src/test/java/io/a2a/transport/grpc/handler/GrpcHandlerTest.java index 77a2c955f..a3ce7ca2a 100644 --- a/transport/grpc/src/test/java/io/a2a/transport/grpc/handler/GrpcHandlerTest.java +++ b/transport/grpc/src/test/java/io/a2a/transport/grpc/handler/GrpcHandlerTest.java @@ -21,6 +21,8 @@ import io.a2a.grpc.GetTaskRequest; import io.a2a.grpc.ListTaskPushNotificationConfigRequest; import io.a2a.grpc.ListTaskPushNotificationConfigResponse; +import io.a2a.grpc.ListTasksRequest; +import io.a2a.grpc.ListTasksResponse; import io.a2a.grpc.Message; import io.a2a.grpc.Part; import io.a2a.grpc.PushNotificationConfig; @@ -1209,6 +1211,53 @@ private void assertGrpcError(StreamRecorder streamRecorder, Status.Code e Assertions.assertTrue(streamRecorder.getValues().isEmpty()); } + @Test + public void testListTasksNegativeTimestampReturnsInvalidArgument() { + TestGrpcHandler handler = new TestGrpcHandler(CARD, requestHandler, internalExecutor); + StreamRecorder responseObserver = StreamRecorder.create(); + + // Negative timestamp should trigger validation error + ListTasksRequest request = ListTasksRequest.newBuilder() + .setLastUpdatedAfter(-1L) + .setTenant("") + .build(); + + handler.listTasks(request, responseObserver); + + // Should return error with INVALID_ARGUMENT status + Assertions.assertTrue(responseObserver.getError() != null); + StatusRuntimeException error = (StatusRuntimeException) responseObserver.getError(); + assertEquals(Status.INVALID_ARGUMENT.getCode(), error.getStatus().getCode()); + } + + @Test + public void testListTasksEmptyResultIncludesAllFields() throws Exception { + TestGrpcHandler handler = new TestGrpcHandler(CARD, requestHandler, internalExecutor); + StreamRecorder responseObserver = StreamRecorder.create(); + + // Query for a context that doesn't exist - should return empty result + ListTasksRequest request = ListTasksRequest.newBuilder() + .setContextId("nonexistent-context-id") + .setTenant("") + .build(); + + handler.listTasks(request, responseObserver); + + // Should succeed with empty result + Assertions.assertNull(responseObserver.getError()); + List responses = responseObserver.getValues(); + assertEquals(1, responses.size()); + + ListTasksResponse response = responses.get(0); + // Verify all fields are present (even if empty/default) + Assertions.assertNotNull(response.getTasksList(), "tasks field should not be null"); + assertEquals(0, response.getTasksList().size(), "tasks should be empty list"); + assertEquals(0, response.getTotalSize(), "totalSize should be 0"); + assertEquals(0, response.getPageSize(), "pageSize should be 0"); + // nextPageToken will be empty string for empty results (protobuf default) + Assertions.assertNotNull(response.getNextPageToken()); + } + private static class TestGrpcHandler extends GrpcHandler { private final AgentCard card; private final RequestHandler handler; diff --git a/transport/jsonrpc/src/test/java/io/a2a/transport/jsonrpc/handler/JSONRPCHandlerTest.java b/transport/jsonrpc/src/test/java/io/a2a/transport/jsonrpc/handler/JSONRPCHandlerTest.java index a55233adb..3436a34b1 100644 --- a/transport/jsonrpc/src/test/java/io/a2a/transport/jsonrpc/handler/JSONRPCHandlerTest.java +++ b/transport/jsonrpc/src/test/java/io/a2a/transport/jsonrpc/handler/JSONRPCHandlerTest.java @@ -30,12 +30,15 @@ import io.a2a.jsonrpc.common.wrappers.GetTaskResponse; import io.a2a.jsonrpc.common.wrappers.ListTaskPushNotificationConfigRequest; import io.a2a.jsonrpc.common.wrappers.ListTaskPushNotificationConfigResponse; +import io.a2a.jsonrpc.common.wrappers.ListTasksResult; import io.a2a.jsonrpc.common.wrappers.SendMessageRequest; import io.a2a.jsonrpc.common.wrappers.SendMessageResponse; import io.a2a.jsonrpc.common.wrappers.SendStreamingMessageRequest; import io.a2a.jsonrpc.common.wrappers.SendStreamingMessageResponse; import io.a2a.jsonrpc.common.wrappers.SetTaskPushNotificationConfigRequest; import io.a2a.jsonrpc.common.wrappers.SetTaskPushNotificationConfigResponse; +import io.a2a.jsonrpc.common.wrappers.ListTasksRequest; +import io.a2a.jsonrpc.common.wrappers.ListTasksResponse; import io.a2a.jsonrpc.common.wrappers.SubscribeToTaskRequest; import io.a2a.server.ServerCallContext; import io.a2a.server.auth.UnauthenticatedUser; @@ -56,7 +59,9 @@ import io.a2a.spec.Event; import io.a2a.spec.GetTaskPushNotificationConfigParams; import io.a2a.spec.InternalError; +import io.a2a.spec.InvalidParamsError; import io.a2a.spec.InvalidRequestError; +import io.a2a.spec.ListTasksParams; import io.a2a.spec.ListTaskPushNotificationConfigParams; import io.a2a.spec.Message; import io.a2a.spec.MessageSendParams; @@ -1959,4 +1964,27 @@ public void testNoVersionDefaultsToCurrentVersionSuccess() { assertNull(response.getError()); Assertions.assertSame(message, response.getResult()); } + + @Test + public void testListTasksEmptyResultIncludesAllFields() { + JSONRPCHandler handler = new JSONRPCHandler(CARD, requestHandler, internalExecutor); + + // Query for a context that doesn't exist - should return empty result + ListTasksParams params = ListTasksParams.builder() + .contextId("nonexistent-context-id") + .tenant("") + .build(); + ListTasksRequest request = new ListTasksRequest("1", params); + ListTasksResponse response = handler.onListTasks(request, callContext); + + // Should return success with all fields present (not null) + assertNull(response.getError()); + ListTasksResult result = response.getResult(); + Assertions.assertNotNull(result); + Assertions.assertNotNull(result.tasks(), "tasks field should not be null"); + assertEquals(0, result.tasks().size(), "tasks should be empty list"); + assertEquals(0, result.totalSize(), "totalSize should be 0"); + assertEquals(0, result.pageSize(), "pageSize should be 0"); + // nextPageToken can be null for empty results + } } diff --git a/transport/rest/src/main/java/io/a2a/transport/rest/handler/RestHandler.java b/transport/rest/src/main/java/io/a2a/transport/rest/handler/RestHandler.java index 87e57f415..4d9a9667d 100644 --- a/transport/rest/src/main/java/io/a2a/transport/rest/handler/RestHandler.java +++ b/transport/rest/src/main/java/io/a2a/transport/rest/handler/RestHandler.java @@ -69,6 +69,7 @@ public class RestHandler { private static final Logger log = Logger.getLogger(RestHandler.class.getName()); + private static final String TASK_STATE_PREFIX = "TASK_STATE_"; // Fields set by constructor injection cannot be final. We need a noargs constructor for // Jakarta compatibility, and it seems that making fields set by constructor injection @@ -219,21 +220,41 @@ public HTTPRestResponse listTasks(@Nullable String contextId, @Nullable String s paramsBuilder.contextId(contextId); } if (status != null) { + TaskState taskState = null; + + // Try JSON format first (e.g., "working", "completed") try { - paramsBuilder.status(TaskState.fromString(status)); + taskState = TaskState.fromString(status); } catch (IllegalArgumentException e) { - try { - paramsBuilder.status(TaskState.valueOf(status)); - } catch (IllegalArgumentException valueOfError) { - String validStates = Arrays.stream(TaskState.values()) - .map(TaskState::asString) - .collect(Collectors.joining(", ")); - Map errorData = new HashMap<>(); - errorData.put("parameter", "status"); - errorData.put("reason", "Must be one of: " + validStates); - throw new InvalidParamsError(null, "Invalid params", errorData); + // Try protobuf enum format (e.g., "TASK_STATE_WORKING") + if (status.startsWith(TASK_STATE_PREFIX)) { + String enumName = status.substring(TASK_STATE_PREFIX.length()); + try { + taskState = TaskState.valueOf(enumName); + } catch (IllegalArgumentException protoError) { + // Fall through to error handling below + } + } else { + // Try enum constant name directly (e.g., "WORKING") + try { + taskState = TaskState.valueOf(status); + } catch (IllegalArgumentException valueOfError) { + // Fall through to error handling below + } } } + + if (taskState == null) { + String validStates = Arrays.stream(TaskState.values()) + .map(TaskState::asString) + .collect(Collectors.joining(", ")); + Map errorData = new HashMap<>(); + errorData.put("parameter", "status"); + errorData.put("reason", "Must be one of: " + validStates); + throw new InvalidParamsError(null, "Invalid params", errorData); + } + + paramsBuilder.status(taskState); } if (pageSize != null) { paramsBuilder.pageSize(pageSize); @@ -247,12 +268,25 @@ public HTTPRestResponse listTasks(@Nullable String contextId, @Nullable String s paramsBuilder.tenant(tenant); if (lastUpdatedAfter != null) { try { - paramsBuilder.lastUpdatedAfter(Instant.parse(lastUpdatedAfter)); - } catch (DateTimeParseException e) { - Map errorData = new HashMap<>(); - errorData.put("parameter", "lastUpdatedAfter"); - errorData.put("reason", "Must be valid ISO-8601 timestamp"); - throw new InvalidParamsError(null, "Invalid params", errorData); + // Try parsing as Unix milliseconds first (integer) + long millis = Long.parseLong(lastUpdatedAfter); + if (millis < 0L) { + Map errorData = new HashMap<>(); + errorData.put("parameter", "lastUpdatedAfter"); + errorData.put("reason", "Must be a non-negative timestamp value, got: " + millis); + throw new InvalidParamsError(null, "Invalid params", errorData); + } + paramsBuilder.lastUpdatedAfter(Instant.ofEpochMilli(millis)); + } catch (NumberFormatException nfe) { + // Fall back to ISO-8601 format + try { + paramsBuilder.lastUpdatedAfter(Instant.parse(lastUpdatedAfter)); + } catch (DateTimeParseException e) { + Map errorData = new HashMap<>(); + errorData.put("parameter", "lastUpdatedAfter"); + errorData.put("reason", "Must be valid Unix milliseconds or ISO-8601 timestamp"); + throw new InvalidParamsError(null, "Invalid params", errorData); + } } } if (includeArtifacts != null) { @@ -338,7 +372,8 @@ private void validate(String json) { private HTTPRestResponse createSuccessResponse(int statusCode, com.google.protobuf.Message.Builder builder) { try { - String jsonBody = JsonFormat.printer().print(builder); + // Include default value fields to ensure empty arrays, zeros, etc. are present in JSON + String jsonBody = JsonFormat.printer().includingDefaultValueFields().print(builder); return new HTTPRestResponse(statusCode, "application/json", jsonBody); } catch (InvalidProtocolBufferException e) { return createErrorResponse(new InternalError("Failed to serialize response: " + e.getMessage())); diff --git a/transport/rest/src/test/java/io/a2a/transport/rest/handler/RestHandlerTest.java b/transport/rest/src/test/java/io/a2a/transport/rest/handler/RestHandlerTest.java index 1ed1492e1..a2e6ef6e9 100644 --- a/transport/rest/src/test/java/io/a2a/transport/rest/handler/RestHandlerTest.java +++ b/transport/rest/src/test/java/io/a2a/transport/rest/handler/RestHandlerTest.java @@ -879,4 +879,82 @@ public void testNoVersionDefaultsToCurrentVersionSuccess() { Assertions.assertEquals("application/json", response.getContentType()); Assertions.assertNotNull(response.getBody()); } + + @Test + public void testListTasksNegativeTimestampReturns422() { + RestHandler handler = new RestHandler(CARD, requestHandler, internalExecutor); + + // Negative timestamp should return 422 (Invalid params) + RestHandler.HTTPRestResponse response = handler.listTasks(null, null, null, null, + null, "-1", null, "", callContext); + + Assertions.assertEquals(422, response.getStatusCode()); + Assertions.assertEquals("application/json", response.getContentType()); + Assertions.assertTrue(response.getBody().contains("InvalidParamsError")); + } + + @Test + public void testListTasksUnixMillisecondsTimestamp() { + RestHandler handler = new RestHandler(CARD, requestHandler, internalExecutor); + taskStore.save(MINIMAL_TASK); + + // Unix milliseconds timestamp should be accepted + String timestampMillis = String.valueOf(System.currentTimeMillis() - 10000); + RestHandler.HTTPRestResponse response = handler.listTasks(null, null, null, null, + null, timestampMillis, null, "", callContext); + + Assertions.assertEquals(200, response.getStatusCode()); + Assertions.assertEquals("application/json", response.getContentType()); + Assertions.assertTrue(response.getBody().contains("tasks")); + } + + @Test + public void testListTasksProtobufEnumStatus() { + RestHandler handler = new RestHandler(CARD, requestHandler, internalExecutor); + taskStore.save(MINIMAL_TASK); + + // Protobuf enum format (TASK_STATE_SUBMITTED) should be accepted + RestHandler.HTTPRestResponse response = handler.listTasks(null, "TASK_STATE_SUBMITTED", null, null, + null, null, null, "", callContext); + + Assertions.assertEquals(200, response.getStatusCode()); + Assertions.assertEquals("application/json", response.getContentType()); + Assertions.assertTrue(response.getBody().contains(MINIMAL_TASK.id())); + } + + @Test + public void testListTasksEnumConstantStatus() { + RestHandler handler = new RestHandler(CARD, requestHandler, internalExecutor); + taskStore.save(MINIMAL_TASK); + + // Enum constant format (SUBMITTED) should be accepted + RestHandler.HTTPRestResponse response = handler.listTasks(null, "SUBMITTED", null, null, + null, null, null, "", callContext); + + Assertions.assertEquals(200, response.getStatusCode()); + Assertions.assertEquals("application/json", response.getContentType()); + Assertions.assertTrue(response.getBody().contains(MINIMAL_TASK.id())); + } + + @Test + public void testListTasksEmptyResultIncludesAllFields() { + RestHandler handler = new RestHandler(CARD, requestHandler, internalExecutor); + + // Query for a context that doesn't exist - should return empty result with all fields + RestHandler.HTTPRestResponse response = handler.listTasks("nonexistent-context-id", null, null, null, + null, null, null, "", callContext); + + Assertions.assertEquals(200, response.getStatusCode()); + Assertions.assertEquals("application/json", response.getContentType()); + + String body = response.getBody(); + // Verify all required fields are present (not missing) + Assertions.assertTrue(body.contains("\"tasks\""), "Response should contain tasks field"); + Assertions.assertTrue(body.contains("\"totalSize\""), "Response should contain totalSize field"); + Assertions.assertTrue(body.contains("\"pageSize\""), "Response should contain pageSize field"); + Assertions.assertTrue(body.contains("\"nextPageToken\""), "Response should contain nextPageToken field"); + // Verify empty array, not null + Assertions.assertTrue(body.contains("\"tasks\":[]") || body.contains("\"tasks\": []"), + "tasks should be empty array"); + } }