Skip to content

Commit

Permalink
initial base work for implementing sorting and filter across API endp…
Browse files Browse the repository at this point in the history
…oints (#12076)
  • Loading branch information
lgfa29 authored Feb 16, 2022
1 parent 3ebfd7b commit 36e31c5
Show file tree
Hide file tree
Showing 11 changed files with 60 additions and 30 deletions.
3 changes: 0 additions & 3 deletions command/agent/deployment_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ func (s *HTTPServer) DeploymentsRequest(resp http.ResponseWriter, req *http.Requ
return nil, nil
}

query := req.URL.Query()
args.OrderAscending = query.Get("ascending") == "true"

var out structs.DeploymentListResponse
if err := s.agent.RPC("Deployment.List", &args, &out); err != nil {
return nil, err
Expand Down
1 change: 0 additions & 1 deletion command/agent/eval_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ func (s *HTTPServer) EvalsRequest(resp http.ResponseWriter, req *http.Request) (
query := req.URL.Query()
args.FilterEvalStatus = query.Get("status")
args.FilterJobID = query.Get("job")
args.OrderAscending = query.Get("ascending") == "true"

var out structs.EvalListResponse
if err := s.agent.RPC("Eval.List", &args, &out); err != nil {
Expand Down
7 changes: 7 additions & 0 deletions command/agent/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -788,6 +788,7 @@ func (s *HTTPServer) parse(resp http.ResponseWriter, req *http.Request, r *strin
parseNamespace(req, &b.Namespace)
parsePagination(req, b)
parseFilter(req, b)
parseAscending(req, b)
return parseWait(resp, req, b)
}

Expand All @@ -813,6 +814,12 @@ func parseFilter(req *http.Request, b *structs.QueryOptions) {
}
}

// parseAscending parses the ascending query parameter for QueryOptions
func parseAscending(req *http.Request, b *structs.QueryOptions) {
query := req.URL.Query()
b.Ascending = query.Get("ascending") == "true"
}

// parseWriteRequest is a convenience method for endpoints that need to parse a
// write request.
func (s *HTTPServer) parseWriteRequest(req *http.Request, w *structs.WriteRequest) {
Expand Down
7 changes: 4 additions & 3 deletions nomad/deployment_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,19 +413,20 @@ func (d *Deployment) List(args *structs.DeploymentListRequest, reply *structs.De
if prefix := args.QueryOptions.Prefix; prefix != "" {
iter, err = store.DeploymentsByIDPrefix(ws, namespace, prefix)
} else if namespace != structs.AllNamespacesSentinel {
iter, err = store.DeploymentsByNamespaceOrdered(ws, namespace, args.OrderAscending)
iter, err = store.DeploymentsByNamespaceOrdered(ws, namespace, args.Ascending)
} else {
iter, err = store.Deployments(ws, args.OrderAscending)
iter, err = store.Deployments(ws, args.Ascending)
}
if err != nil {
return err
}

var deploys []*structs.Deployment
paginator, err := state.NewPaginator(iter, args.QueryOptions,
func(raw interface{}) {
func(raw interface{}) error {
deploy := raw.(*structs.Deployment)
deploys = append(deploys, deploy)
return nil
})
if err != nil {
return structs.NewErrRPCCodedf(
Expand Down
6 changes: 3 additions & 3 deletions nomad/deployment_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1072,8 +1072,8 @@ func TestDeploymentEndpoint_List_order(t *testing.T) {
QueryOptions: structs.QueryOptions{
Region: "global",
Namespace: "*",
Ascending: true,
},
OrderAscending: true,
}

var resp structs.DeploymentListResponse
Expand All @@ -1099,8 +1099,8 @@ func TestDeploymentEndpoint_List_order(t *testing.T) {
QueryOptions: structs.QueryOptions{
Region: "global",
Namespace: "*",
Ascending: false,
},
OrderAscending: false,
}

var resp structs.DeploymentListResponse
Expand Down Expand Up @@ -1399,14 +1399,14 @@ func TestDeploymentEndpoint_List_Pagination(t *testing.T) {
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
req := &structs.DeploymentListRequest{
OrderAscending: true, // counting up is easier to think about
QueryOptions: structs.QueryOptions{
Region: "global",
Namespace: tc.namespace,
Prefix: tc.prefix,
Filter: tc.filter,
PerPage: tc.pageSize,
NextToken: tc.nextToken,
Ascending: true, // counting up is easier to think about
},
}
req.AuthToken = aclToken
Expand Down
7 changes: 4 additions & 3 deletions nomad/eval_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,9 +418,9 @@ func (e *Eval) List(args *structs.EvalListRequest, reply *structs.EvalListRespon
if prefix := args.QueryOptions.Prefix; prefix != "" {
iter, err = store.EvalsByIDPrefix(ws, namespace, prefix)
} else if namespace != structs.AllNamespacesSentinel {
iter, err = store.EvalsByNamespaceOrdered(ws, namespace, args.OrderAscending)
iter, err = store.EvalsByNamespaceOrdered(ws, namespace, args.Ascending)
} else {
iter, err = store.Evals(ws, args.OrderAscending)
iter, err = store.Evals(ws, args.Ascending)
}
if err != nil {
return err
Expand All @@ -435,9 +435,10 @@ func (e *Eval) List(args *structs.EvalListRequest, reply *structs.EvalListRespon

var evals []*structs.Evaluation
paginator, err := state.NewPaginator(iter, args.QueryOptions,
func(raw interface{}) {
func(raw interface{}) error {
eval := raw.(*structs.Evaluation)
evals = append(evals, eval)
return nil
})
if err != nil {
return structs.NewErrRPCCodedf(
Expand Down
8 changes: 4 additions & 4 deletions nomad/eval_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -757,8 +757,8 @@ func TestEvalEndpoint_List_order(t *testing.T) {
QueryOptions: structs.QueryOptions{
Region: "global",
Namespace: "*",
Ascending: false,
},
OrderAscending: false,
}

var resp structs.EvalListResponse
Expand All @@ -784,8 +784,8 @@ func TestEvalEndpoint_List_order(t *testing.T) {
QueryOptions: structs.QueryOptions{
Region: "global",
Namespace: "*",
Ascending: true,
},
OrderAscending: true,
}

var resp structs.EvalListResponse
Expand All @@ -811,8 +811,8 @@ func TestEvalEndpoint_List_order(t *testing.T) {
QueryOptions: structs.QueryOptions{
Region: "global",
Namespace: "*",
Ascending: false,
},
OrderAscending: false,
}

var resp structs.EvalListResponse
Expand Down Expand Up @@ -1249,14 +1249,14 @@ func TestEvalEndpoint_List_PaginationFiltering(t *testing.T) {
req := &structs.EvalListRequest{
FilterJobID: tc.filterJobID,
FilterEvalStatus: tc.filterStatus,
OrderAscending: true, // counting up is easier to think about
QueryOptions: structs.QueryOptions{
Region: "global",
Namespace: tc.namespace,
Prefix: tc.prefix,
PerPage: tc.pageSize,
NextToken: tc.nextToken,
Filter: tc.filter,
Ascending: true, // counting up is easier to think about
},
}
req.AuthToken = aclToken
Expand Down
12 changes: 8 additions & 4 deletions nomad/state/filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,10 @@ func BenchmarkEvalListFilter(b *testing.B) {
for i := 0; i < b.N; i++ {
iter, _ := state.EvalsByNamespace(nil, structs.DefaultNamespace)
var evals []*structs.Evaluation
paginator, err := NewPaginator(iter, opts, func(raw interface{}) {
paginator, err := NewPaginator(iter, opts, func(raw interface{}) error {
eval := raw.(*structs.Evaluation)
evals = append(evals, eval)
return nil
})
if err != nil {
b.Fatalf("failed: %v", err)
Expand All @@ -98,9 +99,10 @@ func BenchmarkEvalListFilter(b *testing.B) {
for i := 0; i < b.N; i++ {
iter, _ := state.Evals(nil, false)
var evals []*structs.Evaluation
paginator, err := NewPaginator(iter, opts, func(raw interface{}) {
paginator, err := NewPaginator(iter, opts, func(raw interface{}) error {
eval := raw.(*structs.Evaluation)
evals = append(evals, eval)
return nil
})
if err != nil {
b.Fatalf("failed: %v", err)
Expand Down Expand Up @@ -133,9 +135,10 @@ func BenchmarkEvalListFilter(b *testing.B) {
for i := 0; i < b.N; i++ {
iter, _ := state.EvalsByNamespace(nil, structs.DefaultNamespace)
var evals []*structs.Evaluation
paginator, err := NewPaginator(iter, opts, func(raw interface{}) {
paginator, err := NewPaginator(iter, opts, func(raw interface{}) error {
eval := raw.(*structs.Evaluation)
evals = append(evals, eval)
return nil
})
if err != nil {
b.Fatalf("failed: %v", err)
Expand Down Expand Up @@ -169,9 +172,10 @@ func BenchmarkEvalListFilter(b *testing.B) {
for i := 0; i < b.N; i++ {
iter, _ := state.Evals(nil, false)
var evals []*structs.Evaluation
paginator, err := NewPaginator(iter, opts, func(raw interface{}) {
paginator, err := NewPaginator(iter, opts, func(raw interface{}) error {
eval := raw.(*structs.Evaluation)
evals = append(evals, eval)
return nil
})
if err != nil {
b.Fatalf("failed: %v", err)
Expand Down
10 changes: 7 additions & 3 deletions nomad/state/paginator.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ type Paginator struct {
// appendFunc is the function the caller should use to append raw
// entries to the results set. The object is guaranteed to be
// non-nil.
appendFunc func(interface{})
appendFunc func(interface{}) error
}

func NewPaginator(iter Iterator, opts structs.QueryOptions, appendFunc func(interface{})) (*Paginator, error) {
func NewPaginator(iter Iterator, opts structs.QueryOptions, appendFunc func(interface{}) error) (*Paginator, error) {
var evaluator *bexpr.Evaluator
var err error

Expand Down Expand Up @@ -64,7 +64,11 @@ DONE:
raw, andThen := p.next()
switch andThen {
case paginatorInclude:
p.appendFunc(raw)
err := p.appendFunc(raw)
if err != nil {
p.pageErr = err
break DONE
}
case paginatorSkip:
continue
case paginatorComplete:
Expand Down
24 changes: 20 additions & 4 deletions nomad/state/paginator_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package state

import (
"errors"
"testing"

"github.com/stretchr/testify/require"
Expand All @@ -19,6 +20,7 @@ func TestPaginator(t *testing.T) {
nextToken string
expected []string
expectedNextToken string
expectedError string
}{
{
name: "size-3 page-1",
Expand Down Expand Up @@ -47,6 +49,10 @@ func TestPaginator(t *testing.T) {
expected: []string{},
expectedNextToken: "",
},
{
name: "error during append",
expectedError: "failed to append",
},
}

for _, tc := range cases {
Expand All @@ -59,17 +65,27 @@ func TestPaginator(t *testing.T) {
structs.QueryOptions{
PerPage: tc.perPage, NextToken: tc.nextToken,
},
func(raw interface{}) {
func(raw interface{}) error {
if tc.expectedError != "" {
return errors.New(tc.expectedError)
}

result := raw.(*mockObject)
results = append(results, result.GetID())
return nil
},
)
require.NoError(t, err)

nextToken, err := paginator.Page()
require.NoError(t, err)
require.Equal(t, tc.expected, results)
require.Equal(t, tc.expectedNextToken, nextToken)
if tc.expectedError == "" {
require.NoError(t, err)
require.Equal(t, tc.expected, results)
require.Equal(t, tc.expectedNextToken, nextToken)
} else {
require.Error(t, err)
require.Contains(t, err.Error(), tc.expectedError)
}
})
}

Expand Down
5 changes: 3 additions & 2 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,9 @@ type QueryOptions struct {
// previous response.
NextToken string

// Ascending is used to have results sorted in ascending chronological order.
Ascending bool

InternalRpcInfo
}

Expand Down Expand Up @@ -863,7 +866,6 @@ type EvalDequeueRequest struct {
type EvalListRequest struct {
FilterJobID string
FilterEvalStatus string
OrderAscending bool
QueryOptions
}

Expand Down Expand Up @@ -1098,7 +1100,6 @@ type GenericRequest struct {

// DeploymentListRequest is used to list the deployments
type DeploymentListRequest struct {
OrderAscending bool
QueryOptions
}

Expand Down

0 comments on commit 36e31c5

Please sign in to comment.