Skip to content

Commit

Permalink
api: rename ascending query param to reverse
Browse files Browse the repository at this point in the history
Using reverse provides a more generic intention that the results should
be returned in the reverse of the default order instead of assuming any
particular order.
  • Loading branch information
lgfa29 committed Mar 3, 2022
1 parent cac9ec4 commit 5ee0639
Show file tree
Hide file tree
Showing 17 changed files with 75 additions and 112 deletions.
10 changes: 5 additions & 5 deletions api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,10 @@ type QueryOptions struct {
// previous response.
NextToken string

// Ascending is used to have results sorted in ascending chronological order.
// Reverse is used to reverse the default order of list results.
//
// Currently only supported by evaluations.List and deployments.list endpoints.
Ascending bool
// Currently only supported by specific endpoints.
Reverse bool

// ctx is an optional context pass through to the underlying HTTP
// request layer. Use Context() and WithContext() to manage this.
Expand Down Expand Up @@ -605,8 +605,8 @@ func (r *request) setQueryOptions(q *QueryOptions) {
if q.NextToken != "" {
r.params.Set("next_token", q.NextToken)
}
if q.Ascending {
r.params.Set("ascending", "true")
if q.Reverse {
r.params.Set("reverse", "true")
}
for k, v := range q.Params {
r.params.Set(k, v)
Expand Down
4 changes: 2 additions & 2 deletions api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ func TestSetQueryOptions(t *testing.T) {
WaitIndex: 1000,
WaitTime: 100 * time.Second,
AuthToken: "foobar",
Ascending: true,
Reverse: true,
}
r.setQueryOptions(q)

Expand All @@ -199,7 +199,7 @@ func TestSetQueryOptions(t *testing.T) {
try("stale", "") // should not be present
try("index", "1000")
try("wait", "100000ms")
try("ascending", "true")
try("reverse", "true")
}

func TestQueryOptionsContext(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions command/agent/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -788,7 +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)
parseReverse(req, b)
return parseWait(resp, req, b)
}

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

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

// parseWriteRequest is a convenience method for endpoints that need to parse a
Expand Down
2 changes: 1 addition & 1 deletion nomad/acl_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ func (a *ACL) ListTokens(args *structs.ACLTokenListRequest, reply *structs.ACLTo
WithID: true,
}
} else {
iter, err = state.ACLTokens(ws, args.Ascending)
iter, err = state.ACLTokens(ws, args.Reverse)
opts = paginator.StructsTokenizerOptions{
WithCreateIndex: true,
WithID: true,
Expand Down
16 changes: 7 additions & 9 deletions nomad/acl_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1084,7 +1084,6 @@ func TestACLEndpoint_ListTokens_PaginationFiltering(t *testing.T) {
Filter: tc.filter,
PerPage: tc.pageSize,
NextToken: tc.nextToken,
Ascending: true, // counting up is easier to think about
},
}
req.AuthToken = bootstrapToken
Expand Down Expand Up @@ -1146,12 +1145,11 @@ func TestACLEndpoint_ListTokens_Order(t *testing.T) {
err = s1.fsm.State().UpsertACLTokens(structs.MsgTypeTestSetup, 1003, []*structs.ACLToken{token2})
require.NoError(t, err)

t.Run("ascending", func(t *testing.T) {
// Lookup the jobs in chronological order (oldest first)
t.Run("default", func(t *testing.T) {
// Lookup the tokens in the default order (oldest first)
get := &structs.ACLTokenListRequest{
QueryOptions: structs.QueryOptions{
Region: "global",
Ascending: true,
Region: "global",
},
}
get.AuthToken = bootstrapToken
Expand All @@ -1173,12 +1171,12 @@ func TestACLEndpoint_ListTokens_Order(t *testing.T) {
require.Equal(t, uuid3, resp.Tokens[2].AccessorID)
})

t.Run("descending", func(t *testing.T) {
// Lookup the jobs in reverse chronological order (newest first)
t.Run("reverse", func(t *testing.T) {
// Lookup the tokens in reverse order (newest first)
get := &structs.ACLTokenListRequest{
QueryOptions: structs.QueryOptions{
Region: "global",
Ascending: false,
Region: "global",
Reverse: true,
},
}
get.AuthToken = bootstrapToken
Expand Down
4 changes: 2 additions & 2 deletions nomad/alloc_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,13 @@ func (a *Alloc) List(args *structs.AllocListRequest, reply *structs.AllocListRes
WithID: true,
}
} else if namespace != structs.AllNamespacesSentinel {
iter, err = state.AllocsByNamespaceOrdered(ws, namespace, args.Ascending)
iter, err = state.AllocsByNamespaceOrdered(ws, namespace, args.Reverse)
opts = paginator.StructsTokenizerOptions{
WithCreateIndex: true,
WithID: true,
}
} else {
iter, err = state.Allocs(ws, args.Ascending)
iter, err = state.Allocs(ws, args.Reverse)
opts = paginator.StructsTokenizerOptions{
WithCreateIndex: true,
WithID: true,
Expand Down
12 changes: 5 additions & 7 deletions nomad/alloc_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,6 @@ func TestAllocEndpoint_List_PaginationFiltering(t *testing.T) {
PerPage: tc.pageSize,
NextToken: tc.nextToken,
Filter: tc.filter,
Ascending: true,
},
Fields: &structs.AllocStubFields{
Resources: false,
Expand Down Expand Up @@ -347,13 +346,12 @@ func TestAllocEndpoint_List_order(t *testing.T) {
err = s1.fsm.State().UpsertAllocs(structs.MsgTypeTestSetup, 1003, []*structs.Allocation{alloc2})
require.NoError(t, err)

t.Run("ascending", func(t *testing.T) {
// Lookup the allocations in reverse chronological order (newest first)
t.Run("default", func(t *testing.T) {
// Lookup the allocations in the default order (oldest first)
get := &structs.AllocListRequest{
QueryOptions: structs.QueryOptions{
Region: "global",
Namespace: "*",
Ascending: true,
},
}

Expand All @@ -374,13 +372,13 @@ func TestAllocEndpoint_List_order(t *testing.T) {
require.Equal(t, uuid3, resp.Allocations[2].ID)
})

t.Run("descending", func(t *testing.T) {
// Lookup the allocations in reverse chronological order
t.Run("reverse", func(t *testing.T) {
// Lookup the allocations in reverse order (newest first)
get := &structs.AllocListRequest{
QueryOptions: structs.QueryOptions{
Region: "global",
Namespace: "*",
Ascending: false,
Reverse: true,
},
}

Expand Down
1 change: 0 additions & 1 deletion nomad/csi_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ func (v *CSIVolume) List(args *structs.CSIVolumeListRequest, reply *structs.CSIV
// Collect results, filter by ACL access
vs := []*structs.CSIVolListStub{}

args.QueryOptions.Ascending = true
paginator, err := paginator.NewPaginator(iter, tokenizer, filters, args.QueryOptions,
func(raw interface{}) error {
vol := raw.(*structs.CSIVolume)
Expand Down
4 changes: 2 additions & 2 deletions nomad/deployment_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,13 +418,13 @@ func (d *Deployment) List(args *structs.DeploymentListRequest, reply *structs.De
WithID: true,
}
} else if namespace != structs.AllNamespacesSentinel {
iter, err = store.DeploymentsByNamespaceOrdered(ws, namespace, args.Ascending)
iter, err = store.DeploymentsByNamespaceOrdered(ws, namespace, args.Reverse)
opts = paginator.StructsTokenizerOptions{
WithCreateIndex: true,
WithID: true,
}
} else {
iter, err = store.Deployments(ws, args.Ascending)
iter, err = store.Deployments(ws, args.Reverse)
opts = paginator.StructsTokenizerOptions{
WithCreateIndex: true,
WithID: true,
Expand Down
8 changes: 3 additions & 5 deletions nomad/deployment_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1066,13 +1066,12 @@ func TestDeploymentEndpoint_List_order(t *testing.T) {
err = s1.fsm.State().UpsertDeployment(1003, dep2)
require.NoError(t, err)

t.Run("ascending", func(t *testing.T) {
t.Run("default", func(t *testing.T) {
// Lookup the deployments in chronological order (oldest first)
get := &structs.DeploymentListRequest{
QueryOptions: structs.QueryOptions{
Region: "global",
Namespace: "*",
Ascending: true,
},
}

Expand All @@ -1093,13 +1092,13 @@ func TestDeploymentEndpoint_List_order(t *testing.T) {
require.Equal(t, uuid3, resp.Deployments[2].ID)
})

t.Run("descending", func(t *testing.T) {
t.Run("reverse", func(t *testing.T) {
// Lookup the deployments in reverse chronological order (newest first)
get := &structs.DeploymentListRequest{
QueryOptions: structs.QueryOptions{
Region: "global",
Namespace: "*",
Ascending: false,
Reverse: true,
},
}

Expand Down Expand Up @@ -1441,7 +1440,6 @@ func TestDeploymentEndpoint_List_Pagination(t *testing.T) {
Filter: tc.filter,
PerPage: tc.pageSize,
NextToken: tc.nextToken,
Ascending: true, // counting up is easier to think about
},
}
req.AuthToken = aclToken
Expand Down
4 changes: 2 additions & 2 deletions nomad/eval_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -423,13 +423,13 @@ func (e *Eval) List(args *structs.EvalListRequest, reply *structs.EvalListRespon
WithID: true,
}
} else if namespace != structs.AllNamespacesSentinel {
iter, err = store.EvalsByNamespaceOrdered(ws, namespace, args.Ascending)
iter, err = store.EvalsByNamespaceOrdered(ws, namespace, args.Reverse)
opts = paginator.StructsTokenizerOptions{
WithCreateIndex: true,
WithID: true,
}
} else {
iter, err = store.Evals(ws, args.Ascending)
iter, err = store.Evals(ws, args.Reverse)
opts = paginator.StructsTokenizerOptions{
WithCreateIndex: true,
WithID: true,
Expand Down
40 changes: 5 additions & 35 deletions nomad/eval_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -751,40 +751,12 @@ func TestEvalEndpoint_List_order(t *testing.T) {
err = s1.fsm.State().UpsertEvals(structs.MsgTypeTestSetup, 1003, []*structs.Evaluation{eval2})
require.NoError(t, err)

t.Run("descending", func(t *testing.T) {
// Lookup the evaluations in reverse chronological order
t.Run("default", func(t *testing.T) {
// Lookup the evaluations in the default order (oldest first)
get := &structs.EvalListRequest{
QueryOptions: structs.QueryOptions{
Region: "global",
Namespace: "*",
Ascending: false,
},
}

var resp structs.EvalListResponse
err = msgpackrpc.CallWithCodec(codec, "Eval.List", get, &resp)
require.NoError(t, err)
require.Equal(t, uint64(1003), resp.Index)
require.Len(t, resp.Evaluations, 3)

// Assert returned order is by CreateIndex (descending)
require.Equal(t, uint64(1002), resp.Evaluations[0].CreateIndex)
require.Equal(t, uuid3, resp.Evaluations[0].ID)

require.Equal(t, uint64(1001), resp.Evaluations[1].CreateIndex)
require.Equal(t, uuid2, resp.Evaluations[1].ID)

require.Equal(t, uint64(1000), resp.Evaluations[2].CreateIndex)
require.Equal(t, uuid1, resp.Evaluations[2].ID)
})

t.Run("ascending", func(t *testing.T) {
// Lookup the evaluations in reverse chronological order (newest first)
get := &structs.EvalListRequest{
QueryOptions: structs.QueryOptions{
Region: "global",
Namespace: "*",
Ascending: true,
},
}

Expand All @@ -805,13 +777,13 @@ func TestEvalEndpoint_List_order(t *testing.T) {
require.Equal(t, uuid3, resp.Evaluations[2].ID)
})

t.Run("descending", func(t *testing.T) {
// Lookup the evaluations in chronological order (oldest first)
t.Run("reverse", func(t *testing.T) {
// Lookup the evaluations in reverse order (newest first)
get := &structs.EvalListRequest{
QueryOptions: structs.QueryOptions{
Region: "global",
Namespace: "*",
Ascending: false,
Reverse: true,
},
}

Expand All @@ -831,7 +803,6 @@ func TestEvalEndpoint_List_order(t *testing.T) {
require.Equal(t, uint64(1000), resp.Evaluations[2].CreateIndex)
require.Equal(t, uuid1, resp.Evaluations[2].ID)
})

}

func TestEvalEndpoint_ListAllNamespaces(t *testing.T) {
Expand Down Expand Up @@ -1304,7 +1275,6 @@ func TestEvalEndpoint_List_PaginationFiltering(t *testing.T) {
PerPage: tc.pageSize,
NextToken: tc.nextToken,
Filter: tc.filter,
Ascending: true, // counting up is easier to think about
},
}
req.AuthToken = aclToken
Expand Down
1 change: 0 additions & 1 deletion nomad/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -1409,7 +1409,6 @@ func (j *Job) List(args *structs.JobListRequest, reply *structs.JobListResponse)
}

var jobs []*structs.JobListStub
args.QueryOptions.Ascending = true
paginator, err := paginator.NewPaginator(iter, tokenizer, filters, args.QueryOptions,
func(raw interface{}) error {
job := raw.(*structs.Job)
Expand Down
11 changes: 6 additions & 5 deletions nomad/state/paginator/paginator.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ type Paginator struct {
itemCount int32
seekingToken string
nextToken string
ascending bool
reverse bool
nextTokenFound bool
pageErr error

Expand Down Expand Up @@ -55,7 +55,7 @@ func NewPaginator(iter Iterator, tokenizer Tokenizer, filters []Filter,
filters: filters,
perPage: opts.PerPage,
seekingToken: opts.NextToken,
ascending: opts.Ascending,
reverse: opts.Reverse,
nextTokenFound: opts.NextToken == "",
appendFunc: appendFunc,
}, nil
Expand Down Expand Up @@ -95,10 +95,11 @@ func (p *Paginator) next() (interface{}, paginatorState) {
p.nextToken = token

var passedToken bool
if p.ascending {
passedToken = token < p.seekingToken
} else {

if p.reverse {
passedToken = token > p.seekingToken
} else {
passedToken = token < p.seekingToken
}

if !p.nextTokenFound && passedToken {
Expand Down
Loading

0 comments on commit 5ee0639

Please sign in to comment.