Skip to content

Commit 38bf20e

Browse files
nahapetyan-serobnahapetyan-serob
committed
fix listTasks query parsing
1 parent 8c0dd99 commit 38bf20e

2 files changed

Lines changed: 205 additions & 55 deletions

File tree

‎a2asrv/rest.go‎

Lines changed: 81 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -132,20 +132,17 @@ func (h *restHandler) handleStreamMessage(rw http.ResponseWriter, req *http.Requ
132132
func (h *restHandler) handleGetTask(rw http.ResponseWriter, req *http.Request) {
133133
ctx := req.Context()
134134
taskID := req.PathValue("id")
135-
historyLengthRaw := req.URL.Query().Get("historyLength")
136-
var historyLength *int
137-
if historyLengthRaw != "" {
138-
val, err := strconv.Atoi(historyLengthRaw)
139-
if err != nil {
140-
writeRESTError(ctx, rw, a2a.ErrInvalidRequest, a2a.TaskID(taskID))
141-
return
142-
}
143-
historyLength = &val
144-
}
135+
145136
if taskID == "" {
146137
writeRESTError(ctx, rw, a2a.ErrInvalidRequest, a2a.TaskID(""))
147138
return
148139
}
140+
historyLength, err := parseHistoryLength(req.URL.Query())
141+
if err != nil {
142+
writeRESTError(ctx, rw, fmt.Errorf("%w: invalid historyLength %v", a2a.ErrInvalidRequest, err), a2a.TaskID(taskID))
143+
return
144+
}
145+
149146
params := &a2a.GetTaskRequest{
150147
ID: a2a.TaskID(taskID),
151148
HistoryLength: historyLength,
@@ -166,53 +163,13 @@ func (h *restHandler) handleGetTask(rw http.ResponseWriter, req *http.Request) {
166163
func (h *restHandler) handleListTasks(rw http.ResponseWriter, req *http.Request) {
167164
ctx := req.Context()
168165
query := req.URL.Query()
169-
request := &a2a.ListTasksRequest{}
170-
var parseErrors []error
171-
parse := func(key string, target any) {
172-
val := query.Get(key)
173-
if val == "" {
174-
return
175-
}
176-
switch t := target.(type) {
177-
case *string:
178-
*t = val
179-
case *a2a.TaskState:
180-
*t = a2a.TaskState(val)
181-
case *int:
182-
v, err := strconv.Atoi(val)
183-
if err != nil {
184-
parseErrors = append(parseErrors, fmt.Errorf("invalid %s: %w", key, err))
185-
return
186-
}
187-
*t = v
188-
case *bool:
189-
v, err := strconv.ParseBool(val)
190-
if err != nil {
191-
parseErrors = append(parseErrors, fmt.Errorf("invalid %s: %w", key, err))
192-
return
193-
}
194-
*t = v
195-
case *time.Time:
196-
parsedTime, err := time.Parse(time.RFC3339, val)
197-
if err != nil {
198-
parseErrors = append(parseErrors, fmt.Errorf("invalid %s: %w", key, err))
199-
return
200-
}
201-
*t = parsedTime
202-
}
203-
}
204-
parse("contextId", &request.ContextID)
205-
parse("status", &request.Status)
206-
parse("pageSize", &request.PageSize)
207-
parse("pageToken", &request.PageToken)
208-
parse("historyLength", &request.HistoryLength)
209-
parse("statusTimestampAfter", &request.StatusTimestampAfter)
210-
parse("includeArtifacts", &request.IncludeArtifacts)
211-
fillTenant(ctx, &request.Tenant)
212-
if len(parseErrors) > 0 {
213-
writeRESTError(ctx, rw, a2a.ErrInvalidRequest, a2a.TaskID(""))
166+
request, err := parseListTasksQueryParams(query)
167+
if err != nil {
168+
writeRESTError(ctx, rw, err, a2a.TaskID(""))
214169
return
215170
}
171+
172+
fillTenant(ctx, &request.Tenant)
216173
result, err := h.handler.ListTasks(ctx, request)
217174
if err != nil {
218175
writeRESTError(ctx, rw, err, a2a.TaskID(""))
@@ -498,6 +455,75 @@ func writeRESTError(ctx context.Context, rw http.ResponseWriter, err error, task
498455
}
499456
}
500457

458+
func parseListTasksQueryParams(query url.Values) (*a2a.ListTasksRequest, error) {
459+
request := &a2a.ListTasksRequest{}
460+
contextID := query.Get("contextId")
461+
if contextID != "" {
462+
request.ContextID = contextID
463+
}
464+
status := query.Get("status")
465+
if status != "" {
466+
request.Status = a2a.TaskState(status)
467+
}
468+
pageSize := query.Get("pageSize")
469+
if pageSize != "" {
470+
val, err := strconv.Atoi(pageSize)
471+
if err != nil {
472+
return nil, fmt.Errorf("%w: invalid pageSize %v", a2a.ErrInvalidRequest, err)
473+
}
474+
request.PageSize = val
475+
}
476+
pageToken := query.Get("pageToken")
477+
if pageToken != "" {
478+
request.PageToken = pageToken
479+
}
480+
includeArtifacts := query.Get("includeArtifacts")
481+
if includeArtifacts != "" {
482+
val, err := strconv.ParseBool(includeArtifacts)
483+
if err != nil {
484+
return nil, fmt.Errorf("%w: invalid includeArtifacts %v", a2a.ErrInvalidRequest, err)
485+
}
486+
request.IncludeArtifacts = val
487+
}
488+
historyLength, err := parseHistoryLength(query)
489+
if err != nil {
490+
return nil, fmt.Errorf("%w: invalid historyLength %v", a2a.ErrInvalidRequest, err)
491+
}
492+
request.HistoryLength = historyLength
493+
statusTimestampAfter, err := parseStatusTimestampAfter(query)
494+
if err != nil {
495+
return nil, fmt.Errorf("%w: invalid statusTimestampAfter %v", a2a.ErrInvalidRequest, err)
496+
}
497+
request.StatusTimestampAfter = statusTimestampAfter
498+
return request, nil
499+
}
500+
501+
func parseHistoryLength(query url.Values) (*int, error) {
502+
historyLengthRaw := query.Get("historyLength")
503+
var historyLength *int
504+
if historyLengthRaw != "" {
505+
val, err := strconv.Atoi(historyLengthRaw)
506+
if err != nil {
507+
return nil, err
508+
}
509+
historyLength = &val
510+
}
511+
return historyLength, nil
512+
}
513+
514+
func parseStatusTimestampAfter(query url.Values) (*time.Time, error) {
515+
statusTimestampAfterRaw := query.Get("statusTimestampAfter")
516+
var statusTimestampAfter *time.Time
517+
if statusTimestampAfterRaw != "" {
518+
val, err := time.Parse(time.RFC3339Nano, statusTimestampAfterRaw)
519+
if err != nil {
520+
return nil, err
521+
}
522+
statusTimestampAfter = &val
523+
}
524+
return statusTimestampAfter, nil
525+
}
526+
501527
type tenantKeyType struct{}
502528

503529
func fillTenant(ctx context.Context, tenant *string) {

‎a2asrv/rest_test.go‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"iter"
2424
"net/http"
2525
"net/http/httptest"
26+
"net/url"
2627
"slices"
2728
"testing"
2829
"time"
@@ -32,7 +33,9 @@ import (
3233
"github.com/a2aproject/a2a-go/v2/a2asrv/taskstore"
3334
"github.com/a2aproject/a2a-go/v2/internal/rest"
3435
"github.com/a2aproject/a2a-go/v2/internal/testutil"
36+
"github.com/a2aproject/a2a-go/v2/internal/utils"
3537
"github.com/a2aproject/a2a-go/v2/log"
38+
"github.com/google/go-cmp/cmp"
3639
)
3740

3841
func TestREST_RequestRouting(t *testing.T) {
@@ -385,6 +388,14 @@ func TestREST_ListTasksParseErrors(t *testing.T) {
385388
name: "multiple invalid params",
386389
query: "?pageSize=abc&includeArtifacts=notbool",
387390
},
391+
{
392+
name: "invalid historyLength",
393+
query: "?historyLength=abc",
394+
},
395+
{
396+
name: "invalid statusTimestampAfter",
397+
query: "?statusTimestampAfter=not-a-timestamp",
398+
},
388399
}
389400

390401
auth := func(ctx context.Context) (string, error) { return "TestUser", nil }
@@ -423,6 +434,119 @@ func TestREST_ListTasksParseErrors(t *testing.T) {
423434
}
424435
}
425436

437+
// capturingListTasksHandler is a minimal RequestHandler used only to record the
438+
// ListTasksRequest produced by the REST layer's query parsing.
439+
type capturingListTasksHandler struct {
440+
RequestHandler // embed the interface; only ListTasks is exercised
441+
capturedListTasksRequest *a2a.ListTasksRequest
442+
}
443+
444+
func (h *capturingListTasksHandler) ListTasks(_ context.Context, req *a2a.ListTasksRequest) (*a2a.ListTasksResponse, error) {
445+
h.capturedListTasksRequest = req
446+
return &a2a.ListTasksResponse{}, nil
447+
}
448+
449+
func TestREST_ListTasksQueryParsing(t *testing.T) {
450+
t.Parallel()
451+
fixedTime := time.Date(2024, 1, 2, 3, 4, 5, 0, time.UTC)
452+
ts := url.QueryEscape(fixedTime.Format(time.RFC3339))
453+
tests := []struct {
454+
name string
455+
query string
456+
want *a2a.ListTasksRequest
457+
}{
458+
{
459+
name: "empty query",
460+
query: "",
461+
want: &a2a.ListTasksRequest{},
462+
},
463+
{
464+
name: "contextId",
465+
query: "contextId=ctx-123",
466+
want: &a2a.ListTasksRequest{ContextID: "ctx-123"},
467+
},
468+
{
469+
name: "status",
470+
query: "status=TASK_STATE_SUBMITTED",
471+
want: &a2a.ListTasksRequest{Status: a2a.TaskStateSubmitted},
472+
},
473+
{
474+
name: "pageSize",
475+
query: "pageSize=11",
476+
want: &a2a.ListTasksRequest{PageSize: 11},
477+
},
478+
{
479+
name: "pageToken",
480+
query: "pageToken=tok-abc",
481+
want: &a2a.ListTasksRequest{PageToken: "tok-abc"},
482+
},
483+
{
484+
name: "historyLength",
485+
query: "historyLength=5",
486+
want: &a2a.ListTasksRequest{HistoryLength: utils.Ptr(5)},
487+
},
488+
{
489+
name: "statusTimestampAfter",
490+
query: "statusTimestampAfter=" + ts,
491+
want: &a2a.ListTasksRequest{StatusTimestampAfter: utils.Ptr(fixedTime)},
492+
},
493+
{
494+
name: "includeArtifacts",
495+
query: "includeArtifacts=true",
496+
want: &a2a.ListTasksRequest{IncludeArtifacts: true},
497+
},
498+
{
499+
name: "all fields",
500+
query: "contextId=ctx-123" +
501+
"&status=TASK_STATE_SUBMITTED" +
502+
"&pageSize=11" +
503+
"&pageToken=tok-abc" +
504+
"&historyLength=5" +
505+
"&statusTimestampAfter=" + ts +
506+
"&includeArtifacts=true",
507+
want: &a2a.ListTasksRequest{
508+
ContextID: "ctx-123",
509+
Status: a2a.TaskStateSubmitted,
510+
PageSize: 11,
511+
PageToken: "tok-abc",
512+
HistoryLength: utils.Ptr(5),
513+
StatusTimestampAfter: utils.Ptr(fixedTime),
514+
IncludeArtifacts: true,
515+
},
516+
},
517+
}
518+
for _, tc := range tests {
519+
t.Run(tc.name, func(t *testing.T) {
520+
t.Parallel()
521+
ctx := t.Context()
522+
capturingHandler := &capturingListTasksHandler{
523+
RequestHandler: NewHandler(&mockAgentExecutor{}),
524+
}
525+
server := httptest.NewServer(NewRESTHandler(capturingHandler))
526+
t.Cleanup(server.Close)
527+
req, err := http.NewRequestWithContext(ctx, http.MethodGet, server.URL+"/tasks?"+tc.query, nil)
528+
if err != nil {
529+
t.Fatalf("http.NewRequestWithContext() error = %v", err)
530+
}
531+
resp, err := server.Client().Do(req)
532+
if err != nil {
533+
t.Fatalf("server.Client().Do() error = %v", err)
534+
}
535+
defer func() { _ = resp.Body.Close() }()
536+
if resp.StatusCode != http.StatusOK {
537+
body, _ := io.ReadAll(resp.Body)
538+
t.Fatalf("resp.StatusCode = %d, want 200 OK; body=%s", resp.StatusCode, string(body))
539+
}
540+
if capturingHandler.capturedListTasksRequest == nil {
541+
t.Fatalf("capturingListTasksHandler.ListTasks() not called")
542+
}
543+
if diff := cmp.Diff(tc.want, capturingHandler.capturedListTasksRequest); diff != "" {
544+
t.Fatalf("ListTasksRequest wrong result (-want +got) diff = %s", diff)
545+
}
546+
})
547+
}
548+
}
549+
426550
func TestRESTTenant(t *testing.T) {
427551
tid := a2a.NewTaskID()
428552
tests := []struct {

0 commit comments

Comments
 (0)