Skip to content

Commit 722bcab

Browse files
HavenXiaHavenXia
committed
Rework List endpoints to align with API style guide.
1 parent c1ab095 commit 722bcab

15 files changed

Lines changed: 492 additions & 194 deletions

File tree

‎cmd/ateapi/internal/controlapi/functional_test.go‎

Lines changed: 62 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1061,13 +1061,12 @@ func TestListActors_PageSizeValidation(t *testing.T) {
10611061
t.Errorf("expected InvalidArgument error for negative page_size, got: %v", err)
10621062
}
10631063

1064-
// 2. Page size exceeding maxPageSize (1000)
1065-
_, err = tc.client.ListActors(context.Background(), &ateapipb.ListActorsRequest{
1064+
// 2. Page size exceeding maxPageSize (1000) is coerced silently.
1065+
if _, err := tc.client.ListActors(context.Background(), &ateapipb.ListActorsRequest{
10661066
Atespace: testAtespace,
10671067
PageSize: 1001,
1068-
})
1069-
if status.Code(err) != codes.InvalidArgument {
1070-
t.Errorf("expected InvalidArgument error for page_size > 1000, got: %v", err)
1068+
}); err != nil {
1069+
t.Errorf("expected page_size > 1000 to be coerced, got error: %v", err)
10711070
}
10721071
}
10731072

@@ -2295,6 +2294,64 @@ func TestValidation(t *testing.T) {
22952294
})
22962295
}
22972296
})
2297+
2298+
t.Run("ListActors", func(t *testing.T) {
2299+
tests := []struct {
2300+
name string
2301+
req *ateapipb.ListActorsRequest
2302+
wantMsg string
2303+
}{{
2304+
"invalid atespace",
2305+
&ateapipb.ListActorsRequest{Atespace: "NS1"},
2306+
"atespace: Invalid value",
2307+
}, {
2308+
"negative page_size",
2309+
&ateapipb.ListActorsRequest{Atespace: "ns1", PageSize: -1},
2310+
"page_size: Invalid value",
2311+
}}
2312+
for _, tt := range tests {
2313+
t.Run(tt.name, func(t *testing.T) {
2314+
_, err := tc.client.ListActors(context.Background(), tt.req)
2315+
assertGrpcErrorRegex(t, err, codes.InvalidArgument, tt.wantMsg)
2316+
})
2317+
}
2318+
})
2319+
2320+
t.Run("ListWorkers", func(t *testing.T) {
2321+
tests := []struct {
2322+
name string
2323+
req *ateapipb.ListWorkersRequest
2324+
wantMsg string
2325+
}{{
2326+
"negative page_size",
2327+
&ateapipb.ListWorkersRequest{PageSize: -1},
2328+
"page_size: Invalid value",
2329+
}}
2330+
for _, tt := range tests {
2331+
t.Run(tt.name, func(t *testing.T) {
2332+
_, err := tc.client.ListWorkers(context.Background(), tt.req)
2333+
assertGrpcErrorRegex(t, err, codes.InvalidArgument, tt.wantMsg)
2334+
})
2335+
}
2336+
})
2337+
2338+
t.Run("ListAtespaces", func(t *testing.T) {
2339+
tests := []struct {
2340+
name string
2341+
req *ateapipb.ListAtespacesRequest
2342+
wantMsg string
2343+
}{{
2344+
"negative page_size",
2345+
&ateapipb.ListAtespacesRequest{PageSize: -1},
2346+
"page_size: Invalid value",
2347+
}}
2348+
for _, tt := range tests {
2349+
t.Run(tt.name, func(t *testing.T) {
2350+
_, err := tc.client.ListAtespaces(context.Background(), tt.req)
2351+
assertGrpcErrorRegex(t, err, codes.InvalidArgument, tt.wantMsg)
2352+
})
2353+
}
2354+
})
22982355
}
22992356

23002357
func TestResumeActor_LockConflict(t *testing.T) {

‎cmd/ateapi/internal/controlapi/list_actors.go‎

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,20 +22,26 @@ import (
2222
"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
2323
"google.golang.org/grpc/codes"
2424
"google.golang.org/grpc/status"
25+
"k8s.io/apimachinery/pkg/util/validation/field"
2526
)
2627

2728
const maxPageSize = 1000
2829

30+
// effectivePageSize applies the server-chosen default for an unset page_size
31+
// and silently coerces oversized values.
32+
func effectivePageSize(requested int32) int32 {
33+
if requested == 0 || requested > maxPageSize {
34+
return maxPageSize
35+
}
36+
return requested
37+
}
38+
2939
func (s *Service) ListActors(ctx context.Context, req *ateapipb.ListActorsRequest) (*ateapipb.ListActorsResponse, error) {
3040
if err := validateListActorsRequest(req); err != nil {
31-
return nil, status.Error(codes.InvalidArgument, err.Error())
32-
}
33-
pageSize := req.GetPageSize()
34-
if pageSize == 0 {
35-
pageSize = maxPageSize
41+
return nil, err
3642
}
3743

38-
actors, nextToken, err := s.persistence.ListActors(ctx, req.GetAtespace(), pageSize, req.GetPageToken())
44+
actors, nextToken, err := s.persistence.ListActors(ctx, req.GetAtespace(), effectivePageSize(req.GetPageSize()), req.GetPageToken())
3945
if err != nil {
4046
return nil, fmt.Errorf("while listing actors in db: %w", err)
4147
}
@@ -46,17 +52,22 @@ func (s *Service) ListActors(ctx context.Context, req *ateapipb.ListActorsReques
4652
}
4753

4854
func validateListActorsRequest(req *ateapipb.ListActorsRequest) error {
49-
// An empty atespace is allowed here and means "all atespaces"(used by `kubectl ate get actors -A`).
50-
// A non-empty atespace is validated and scopes the listing to that atespace.
51-
if req.GetAtespace() != "" && !resources.IsValidResourceName(req.GetAtespace()) {
52-
return fmt.Errorf("invalid atespace %q: must be a valid resource name", req.GetAtespace())
55+
var fldPath *field.Path
56+
var errs field.ErrorList
57+
58+
// An empty atespace is allowed here and means "all atespaces" (used by
59+
// `kubectl ate get actors -A`). A non-empty atespace is validated and
60+
// scopes the listing to that atespace.
61+
if val, fldPath := req.Atespace, fldPath.Child("atespace"); val != "" {
62+
errs = append(errs, resources.ValidateResourceName(val, fldPath)...)
5363
}
54-
pageSize := req.GetPageSize()
55-
if pageSize < 0 {
56-
return fmt.Errorf("page_size cannot be negative")
64+
65+
if val, fldPath := req.PageSize, fldPath.Child("page_size"); val < 0 {
66+
errs = append(errs, field.Invalid(fldPath, val, "must be greater than or equal to 0"))
5767
}
58-
if pageSize > maxPageSize {
59-
return fmt.Errorf("page_size %d exceeds maximum page size %d", pageSize, maxPageSize)
68+
69+
if len(errs) > 0 {
70+
return status.Error(codes.InvalidArgument, errs.ToAggregate().Error())
6071
}
6172
return nil
6273
}

‎cmd/ateapi/internal/controlapi/list_atespaces.go‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,36 @@ import (
1919
"fmt"
2020

2121
"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
22+
"google.golang.org/grpc/codes"
23+
"google.golang.org/grpc/status"
24+
"k8s.io/apimachinery/pkg/util/validation/field"
2225
)
2326

2427
func (s *Service) ListAtespaces(ctx context.Context, req *ateapipb.ListAtespacesRequest) (*ateapipb.ListAtespacesResponse, error) {
25-
atespaces, err := s.persistence.ListAtespaces(ctx)
28+
if err := validateListAtespacesRequest(req); err != nil {
29+
return nil, err
30+
}
31+
32+
atespaces, nextToken, err := s.persistence.ListAtespaces(ctx, effectivePageSize(req.GetPageSize()), req.GetPageToken())
2633
if err != nil {
2734
return nil, fmt.Errorf("while listing atespaces in db: %w", err)
2835
}
29-
return &ateapipb.ListAtespacesResponse{Atespaces: atespaces}, nil
36+
return &ateapipb.ListAtespacesResponse{
37+
Atespaces: atespaces,
38+
NextPageToken: nextToken,
39+
}, nil
40+
}
41+
42+
func validateListAtespacesRequest(req *ateapipb.ListAtespacesRequest) error {
43+
var fldPath *field.Path
44+
var errs field.ErrorList
45+
46+
if val, fldPath := req.PageSize, fldPath.Child("page_size"); val < 0 {
47+
errs = append(errs, field.Invalid(fldPath, val, "must be greater than or equal to 0"))
48+
}
49+
50+
if len(errs) > 0 {
51+
return status.Error(codes.InvalidArgument, errs.ToAggregate().Error())
52+
}
53+
return nil
3054
}

‎cmd/ateapi/internal/controlapi/list_workers.go‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,22 +19,36 @@ import (
1919
"fmt"
2020

2121
"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
22+
"google.golang.org/grpc/codes"
23+
"google.golang.org/grpc/status"
24+
"k8s.io/apimachinery/pkg/util/validation/field"
2225
)
2326

2427
func (s *Service) ListWorkers(ctx context.Context, req *ateapipb.ListWorkersRequest) (*ateapipb.ListWorkersResponse, error) {
2528
if err := validateListWorkersRequest(req); err != nil {
2629
return nil, err
2730
}
28-
workers, err := s.persistence.ListWorkers(ctx)
31+
32+
workers, nextToken, err := s.persistence.ListWorkers(ctx, effectivePageSize(req.GetPageSize()), req.GetPageToken())
2933
if err != nil {
3034
return nil, fmt.Errorf("while listing workers in db: %w", err)
3135
}
3236
return &ateapipb.ListWorkersResponse{
33-
Workers: workers,
37+
Workers: workers,
38+
NextPageToken: nextToken,
3439
}, nil
3540
}
3641

37-
func validateListWorkersRequest(_ *ateapipb.ListWorkersRequest) error {
38-
// No fields to validate for now.
42+
func validateListWorkersRequest(req *ateapipb.ListWorkersRequest) error {
43+
var fldPath *field.Path
44+
var errs field.ErrorList
45+
46+
if val, fldPath := req.PageSize, fldPath.Child("page_size"); val < 0 {
47+
errs = append(errs, field.Invalid(fldPath, val, "must be greater than or equal to 0"))
48+
}
49+
50+
if len(errs) > 0 {
51+
return status.Error(codes.InvalidArgument, errs.ToAggregate().Error())
52+
}
3953
return nil
4054
}

‎cmd/ateapi/internal/controlapi/syncer_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ func TestSyncer_Lifecycle(t *testing.T) {
8888
defer cleanup()
8989

9090
// 1. Verify no workers in Redis initially
91-
workers, err := persistence.ListWorkers(context.Background())
91+
workers, _, err := persistence.ListWorkers(context.Background(), 1000, "")
9292
if err != nil {
9393
t.Fatalf("failed to list workers: %v", err)
9494
}

0 commit comments

Comments
 (0)