feat(job-array-one-based-index): query handlers evaluate element selections (task 7.3)
GetJobsInfo validates every filter entry in request order before any store access (including the exec_hosts lookup) and GetJobByIDs every key before querying; the first rejection returns 400 with the parser's message. Bracketed job_id/job_name entries are evaluated as row predicates before sorting and pagination, so job_id:eq:X[2] lists only element 2, GET /jobs/X[1-3] lists elements 1-3 and <ordinary-id>[0] lists nothing. The unevaluated compound conditions and the old index parsing are gone from both handlers. Fixes bug-1898 on the query paths. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
675d7d7f6c
commit
3457d892b6
GPG key ID:
80533536B0EB651C
3 changed files with 364 additions and 370 deletions
|
|
@ -19,6 +19,8 @@ import (
|
|||
"errors"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"sort"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
|
|
@ -734,3 +736,243 @@ func TestJobHandler_SubmitArrayLogsArrayIdentity(t *testing.T) {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ========== Query by selection (task 7.3, bug-1898) ==========
|
||||
|
||||
// querySeed is the store content of the query tests: arrays X ("foo",
|
||||
// elements 1-4), Y ("foobar", elements 1-2), Z (unnamed, element 1) and W
|
||||
// ("baz", elements 1-2), and the ordinary job O ("bar").
|
||||
type querySeed struct {
|
||||
X, Y, Z, W, O uint64
|
||||
}
|
||||
|
||||
func (s querySeed) elem(key uint64, k int) uint64 { return backend.MakeElementID(key, k) }
|
||||
|
||||
func seedQueryJobs(f *handlerFixture) querySeed {
|
||||
f.t.Helper()
|
||||
s := querySeed{
|
||||
X: handlerTestArrayBase + 10<<15,
|
||||
Y: handlerTestArrayBase + 11<<15,
|
||||
Z: handlerTestArrayBase + 12<<15,
|
||||
W: handlerTestArrayBase + 13<<15,
|
||||
// array-capable (low 15 bits clear) but ordinary: element-only
|
||||
// matching must not select it for O[0] (5.11)
|
||||
O: handlerTestOrdinaryBase + 20<<15,
|
||||
}
|
||||
f.seedArrayElements(s.X, "foo", "RUN", 1, 2, 3, 4)
|
||||
f.seedArrayElements(s.Y, "foobar", "RUN", 1, 2)
|
||||
f.seedArrayElements(s.Z, "", "RUN", 1)
|
||||
f.seedArrayElements(s.W, "baz", "RUN", 1, 2)
|
||||
f.seedOrdinaryJob(s.O, "bar", "RUN")
|
||||
return s
|
||||
}
|
||||
|
||||
// allQueryIDs is every unique_job_id seedQueryJobs stores.
|
||||
func (s querySeed) all() []uint64 {
|
||||
return []uint64{
|
||||
s.elem(s.X, 1), s.elem(s.X, 2), s.elem(s.X, 3), s.elem(s.X, 4),
|
||||
s.elem(s.Y, 1), s.elem(s.Y, 2), s.elem(s.Z, 1),
|
||||
s.elem(s.W, 1), s.elem(s.W, 2), s.O,
|
||||
}
|
||||
}
|
||||
|
||||
// listJobs calls GetJobsInfo with filter (URL-escaped here).
|
||||
func (f *handlerFixture) listJobs(filter string) *httptest.ResponseRecorder {
|
||||
f.t.Helper()
|
||||
return f.do(http.MethodGet, "/xce/v1/jobs?filter="+url.QueryEscape(filter), nil)
|
||||
}
|
||||
|
||||
// getJobs calls GetJobByIDs with the path segment jobids (URL-escaped here).
|
||||
func (f *handlerFixture) getJobs(jobids string) *httptest.ResponseRecorder {
|
||||
f.t.Helper()
|
||||
return f.do(http.MethodGet, "/xce/v1/jobs/"+url.PathEscape(jobids), nil)
|
||||
}
|
||||
|
||||
// rowIDs returns the sorted unique_job_id of every row of a 200 list reply.
|
||||
func rowIDs(t *testing.T, w *httptest.ResponseRecorder) []uint64 {
|
||||
t.Helper()
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200; body: %s", w.Code, w.Body.String())
|
||||
}
|
||||
dec := json.NewDecoder(bytes.NewReader(w.Body.Bytes()))
|
||||
dec.UseNumber()
|
||||
var raw struct {
|
||||
Data struct {
|
||||
Count int `json:"count"`
|
||||
Rows []struct {
|
||||
Data map[string]interface{} `json:"Data"`
|
||||
} `json:"rows"`
|
||||
} `json:"data"`
|
||||
}
|
||||
if err := dec.Decode(&raw); err != nil {
|
||||
t.Fatalf("decode list reply: %v; body: %s", err, w.Body.String())
|
||||
}
|
||||
ids := make([]uint64, 0, len(raw.Data.Rows))
|
||||
for _, r := range raw.Data.Rows {
|
||||
n, ok := r.Data["unique_job_id"].(json.Number)
|
||||
if !ok {
|
||||
t.Fatalf("row without unique_job_id: %v", r.Data)
|
||||
}
|
||||
id, err := strconv.ParseUint(n.String(), 10, 64)
|
||||
if err != nil {
|
||||
t.Fatalf("unique_job_id %q: %v", n, err)
|
||||
}
|
||||
ids = append(ids, id)
|
||||
}
|
||||
if raw.Data.Count != len(ids) {
|
||||
t.Errorf("count = %d, rows = %d", raw.Data.Count, len(ids))
|
||||
}
|
||||
sort.Slice(ids, func(i, j int) bool { return ids[i] < ids[j] })
|
||||
return ids
|
||||
}
|
||||
|
||||
func sortedIDs(ids ...uint64) []uint64 {
|
||||
out := append([]uint64{}, ids...)
|
||||
sort.Slice(out, func(i, j int) bool { return out[i] < out[j] })
|
||||
return out
|
||||
}
|
||||
|
||||
func assertIDs(t *testing.T, name string, got, want []uint64) {
|
||||
t.Helper()
|
||||
want = sortedIDs(want...)
|
||||
if len(got) != len(want) {
|
||||
t.Errorf("%s: rows %v, want %v", name, got, want)
|
||||
return
|
||||
}
|
||||
for i := range got {
|
||||
if got[i] != want[i] {
|
||||
t.Errorf("%s: rows %v, want %v", name, got, want)
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// assert400 checks a 400 reply whose message contains want.
|
||||
func assert400(t *testing.T, name string, w *httptest.ResponseRecorder, want string) {
|
||||
t.Helper()
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("%s: status = %d, want 400; body: %s", name, w.Code, w.Body.String())
|
||||
return
|
||||
}
|
||||
r := decodeHandlerResponse(t, w)
|
||||
msg, _ := r.Msg.(string)
|
||||
if !strings.Contains(msg, want) {
|
||||
t.Errorf("%s: msg = %q, want it to contain %q", name, msg, want)
|
||||
}
|
||||
if len(r.Data) != 0 && string(r.Data) != "null" {
|
||||
t.Errorf("%s: a rejected query carries data: %s", name, r.Data)
|
||||
}
|
||||
}
|
||||
|
||||
// TestJobHandler_QuerySelections covers the GetJobsInfo rows of the design's
|
||||
// Testing Strategy (Query).
|
||||
func TestJobHandler_QuerySelections(t *testing.T) {
|
||||
f := newHandlerFixture(t)
|
||||
s := seedQueryJobs(f)
|
||||
x, y, z, w := s.X, s.Y, s.Z, s.W
|
||||
X := jobIDString(x)
|
||||
Y := jobIDString(y)
|
||||
O := jobIDString(s.O)
|
||||
writtenX := backend.FormatJobID(int64(x))
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
filter string
|
||||
want []uint64
|
||||
}{
|
||||
{"eq single element (bug-1898)", "{job_id:eq:" + X + "[2]}", []uint64{s.elem(x, 2)}},
|
||||
{"eq range", "{job_id:eq:" + X + "[2-3]}", []uint64{s.elem(x, 2), s.elem(x, 3)}},
|
||||
{"ordinary-id[0] lists nothing (5.11)", "{job_id:eq:" + O + "[0]}", nil},
|
||||
{"array-id[0] without element 0 lists nothing (5.11)", "{job_id:eq:" + X + "[0]}", nil},
|
||||
{"unnamed: eq [1] lists nothing", "{job_name:eq:[1]}", nil},
|
||||
{"unnamed: contains [1] lists element 1 of every array",
|
||||
"{job_name:contains:[1]}", []uint64{s.elem(x, 1), s.elem(y, 1), s.elem(z, 1), s.elem(w, 1)}},
|
||||
{"notcontains foo[1]", "{job_name:notcontains:foo[1]}", []uint64{s.elem(z, 1), s.elem(w, 1)}},
|
||||
{"in + ne entries on the same field are AND'ed",
|
||||
"{job_id:in:(" + X + "[1]," + O + ")},{job_id:ne:" + O + "}", []uint64{s.elem(x, 1)}},
|
||||
{"job_id and job_name selections intersect",
|
||||
"{job_id:in:(" + X + "[1]," + X + "[2])},{job_name:eq:foo[2]}", []uint64{s.elem(x, 2)}},
|
||||
{"in with a bare and a bracketed value matches both",
|
||||
"{job_id:in:(" + O + "," + X + "[1])}", []uint64{s.O, s.elem(x, 1)}},
|
||||
{"notin excludes the element", "{job_id:notin:(" + X + "[1])}",
|
||||
[]uint64{s.elem(x, 2), s.elem(x, 3), s.elem(x, 4), s.elem(y, 1), s.elem(y, 2),
|
||||
s.elem(z, 1), s.elem(w, 1), s.elem(w, 2), s.O}},
|
||||
{"startwith foo[1]", "{job_name:startwith:foo[1]}", []uint64{s.elem(x, 1), s.elem(y, 1)}},
|
||||
{"written bare key alone (5.6)", "{job_id:eq:" + writtenX + "}",
|
||||
[]uint64{s.elem(x, 1), s.elem(x, 2), s.elem(x, 3), s.elem(x, 4)}},
|
||||
{"written bare key mixed with a bracketed value (5.6)",
|
||||
"{job_id:in:(" + writtenX + "," + Y + "[1])}",
|
||||
[]uint64{s.elem(x, 1), s.elem(x, 2), s.elem(x, 3), s.elem(x, 4), s.elem(y, 1)}},
|
||||
{"between on job_id evaluates like in", "{job_id:between:(" + X + "[1]," + Y + "[2])}",
|
||||
[]uint64{s.elem(x, 1), s.elem(y, 2)}},
|
||||
}
|
||||
for _, c := range cases {
|
||||
assertIDs(t, c.name, rowIDs(t, f.listJobs(c.filter)), c.want)
|
||||
}
|
||||
|
||||
// sanity: an unfiltered (default submit_user) listing sees every seeded row
|
||||
assertIDs(t, "all rows", rowIDs(t, f.listJobs("{submit_user:eq:"+handlerTestUser+"}")), s.all())
|
||||
}
|
||||
|
||||
// TestJobHandler_QuerySelectionRejections: rejected selections reject the
|
||||
// whole query with 400 and list nothing.
|
||||
func TestJobHandler_QuerySelectionRejections(t *testing.T) {
|
||||
f := newHandlerFixture(t)
|
||||
s := seedQueryJobs(f)
|
||||
X := jobIDString(s.X)
|
||||
O := jobIDString(s.O)
|
||||
// a host exists (so GetHostList accepts it) but runs no job, so the
|
||||
// exec_hosts lookup alone would answer count 0
|
||||
store.Hosts.Put("h1", models.HostInfo{HostName: "h1"})
|
||||
|
||||
cases := []struct {
|
||||
name, filter, want string
|
||||
}{
|
||||
{"5.7 with an exec_hosts filter matching no job",
|
||||
"{exec_hosts:eq:h1},{job_id:eq:" + X + "[40000]}", "invalid"},
|
||||
{"null operator on a bracketed name", "{job_name:notnull:foo[1]}", "operator doesn't support"},
|
||||
{"ne on a bracketed identity", "{job_id:ne:" + X + "[1]}", "operator doesn't support"},
|
||||
{"valid selection followed by an invalid one",
|
||||
"{job_id:eq:" + X + "[1]},{job_name:eq:foo[5-3]}", "reversed"},
|
||||
{"5.8: element of a non-array identity", "{job_id:eq:" + jobIDString(s.O+1) + "[1]}", "not an array identity"},
|
||||
{"5.16 syntax", "{job_id:eq:" + X + "[a]}", "invalid job identity or element selection"},
|
||||
{"5.17 marked decimal without index",
|
||||
"{job_id:eq:" + jobIDString(s.elem(s.X, 1)) + "}", "must be named as"},
|
||||
{"5.14 too-wide written key", "{job_id:in:(" + O + ",64-0-1-0[1])}", "wider than allowed"},
|
||||
}
|
||||
for _, c := range cases {
|
||||
assert400(t, c.name, f.listJobs(c.filter), c.want)
|
||||
}
|
||||
|
||||
// the exec_hosts row's message names the selection (5.7) and is not
|
||||
// replaced by "bad job array index"
|
||||
w := f.listJobs("{exec_hosts:eq:h1},{job_id:eq:" + X + "[40000]}")
|
||||
if strings.Contains(w.Body.String(), "bad job array index") {
|
||||
t.Errorf("message replaced by the old text: %s", w.Body.String())
|
||||
}
|
||||
if !strings.Contains(w.Body.String(), X+"[40000]") {
|
||||
t.Errorf("message does not name the selection: %s", w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// TestJobHandler_GetJobByIDsSelections covers GET /jobs/{jobids}.
|
||||
func TestJobHandler_GetJobByIDsSelections(t *testing.T) {
|
||||
f := newHandlerFixture(t)
|
||||
s := seedQueryJobs(f)
|
||||
X := jobIDString(s.X)
|
||||
O := jobIDString(s.O)
|
||||
|
||||
assertIDs(t, "X[1-3]", rowIDs(t, f.getJobs(X+"[1-3]")),
|
||||
[]uint64{s.elem(s.X, 1), s.elem(s.X, 2), s.elem(s.X, 3)})
|
||||
assertIDs(t, "X[2],Y[1]", rowIDs(t, f.getJobs(X+"[2],"+jobIDString(s.Y)+"[1]")),
|
||||
[]uint64{s.elem(s.X, 2), s.elem(s.Y, 1)})
|
||||
assertIDs(t, "bare X lists every element (5.6)", rowIDs(t, f.getJobs(X)),
|
||||
[]uint64{s.elem(s.X, 1), s.elem(s.X, 2), s.elem(s.X, 3), s.elem(s.X, 4)})
|
||||
assertIDs(t, "written bare O", rowIDs(t, f.getJobs(backend.FormatJobID(int64(s.O)))), []uint64{s.O})
|
||||
assertIDs(t, "O[0] lists nothing (5.11)", rowIDs(t, f.getJobs(O+"[0]")), nil)
|
||||
|
||||
assert400(t, "valid then invalid", f.getJobs(X+"[1],"+X+"[40000]"), X+"[40000]")
|
||||
assert400(t, "5.17", f.getJobs(jobIDString(s.elem(s.X, 1))), "must be named as")
|
||||
assert400(t, "5.14", f.getJobs("64-0-1-0"), "wider than allowed")
|
||||
assert400(t, "5.5", f.getJobs(X+"[3-1]"), "reversed")
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue