diff --git a/.kiro/specs/job-array-one-based-index/tasks.md b/.kiro/specs/job-array-one-based-index/tasks.md index 4c7cb264..797696a7 100644 --- a/.kiro/specs/job-array-one-based-index/tasks.md +++ b/.kiro/specs/job-array-one-based-index/tasks.md @@ -181,7 +181,7 @@ - _Depends: 5.4, 7.2, 7.4_ - [ ] 8. Command-line client -- [ ] 8.1 (P) Parse identities and selections in the client +- [x] 8.1 (P) Parse identities and selections in the client - Add the client selection parser (written and decimal keys, the shared order, alignment and element bound on decimal keys with an element part, range refusal for queries only after the other checks), the bracket-aware list splitter and the raw `-J` name check - Make identity parsing bounded and change the API-key conversion to take whether ranges are allowed; keep a legacy conversion for peek; update its two command callers mechanically (control allows ranges, the list filter does not) so the client keeps building - Update the conversion tests: `"123456"` and `"32768[3]"` pass through, `"123456[3]"` is rejected under 5.8, a range passes only when allowed; move peek and its test to the legacy conversion @@ -249,3 +249,4 @@ - 7.1: `ParseNameSelection` on a value without `[`/`]` returns `SelBare` with `Name` = the raw untrimmed value (`Raw` is trimmed); 7.2 must match bare names on `Name` and must not treat a bare result as validated. Rule tags on `*SelError` are "5.16", "5.14", "5.17", "5.8", "5.7", "5.5"; decimal keys above uint64 are 5.14. - 7.2: `BuildSelectionPredicates(filters []utils.FilterCondition) (plain []utils.FilterCondition, preds []func(utils.OrderMap) bool, err error)` in `controllers/jobmatch.go`; parser rejections come back as `*SelError`, other rejections with today's texts. Accepted consequence of the one-rule decision: bare `job_id` values under identity operators now go through `ParseIdentitySelection`, so `job_id:eq:+5` / `-1` (previously accepted) are rejected under 5.16. For 7.3: keep the `array_stats` bracket rejection before the builder, and run the builder in the validate-all-first pass before the `exec_hosts` store lookup. - 7.3: query pre-pass is `validateJobQueryFilters`; `returnSelectionError` maps `*SelError` → 400 with its message. `RestartJobByID` (jobs.go ~:863) still builds unevaluated `CompFilterCondition`s — restart is out of scope, so bug-1898 stays partial for that path. Pre-existing: GetJobsInfo `page_token` does not advance (logged separately). +- 8.1: client `format.ParseSelection(raw, allowRange)`, `ParseNameSelection(raw)`, `SplitSelections(list)`, `Selection.APIKey()`; query-range refusal rule tag "range" (`RuleQueryRange`); `ToAPIJobID(s, allowRange)`; peek uses `ToAPIJobIDLegacy`. For 8.2: `filters/job_filter.go` still runs `isValidJobId` before `ToAPIJobID` and both ID lists still split at every comma. diff --git a/src/frontend/xcecli/cmd/jobs/controller.go b/src/frontend/xcecli/cmd/jobs/controller.go index 866ced9b..8b36cf60 100755 --- a/src/frontend/xcecli/cmd/jobs/controller.go +++ b/src/frontend/xcecli/cmd/jobs/controller.go @@ -49,7 +49,7 @@ func (c *JobController) RunJobsControl() error { func (c *JobController) controlJobsByIDs(apiClient *request.APIClient, token string) error { apiIDs := make([]string, 0, len(c.jobIDs)) for _, id := range c.jobIDs { - apiID, err := format.ToAPIJobID(strings.TrimSpace(id)) + apiID, err := format.ToAPIJobID(strings.TrimSpace(id), true) if err != nil { return fmt.Errorf("invalid job ID %q: %w", id, err) } diff --git a/src/frontend/xcecli/cmd/jobs/peek.go b/src/frontend/xcecli/cmd/jobs/peek.go index 23698f2b..0afd056e 100644 --- a/src/frontend/xcecli/cmd/jobs/peek.go +++ b/src/frontend/xcecli/cmd/jobs/peek.go @@ -78,7 +78,7 @@ func (q *PeekQuerier) Run(jobID string) error { } // Convert job ID to API format - apiJobID, err := format.ToAPIJobID(jobID) + apiJobID, err := format.ToAPIJobIDLegacy(jobID) if err != nil { return fmt.Errorf("invalid job ID %q: %w", jobID, err) } diff --git a/src/frontend/xcecli/cmd/jobs/peek_test.go b/src/frontend/xcecli/cmd/jobs/peek_test.go index 5db8bb47..a82f6f4c 100644 --- a/src/frontend/xcecli/cmd/jobs/peek_test.go +++ b/src/frontend/xcecli/cmd/jobs/peek_test.go @@ -15,12 +15,12 @@ func TestPeekJobIDParsing(t *testing.T) { {"12345", "12345"}, {"12345[0]", "12345[0]"}, {"12345[2]", "12345[2]"}, - {"plain_id", "plain_id"}, // ToAPIJobID passes through non-numeric strings + {"plain_id", "plain_id"}, // ToAPIJobIDLegacy passes through non-numeric strings } for _, tc := range tests { t.Run(tc.input, func(t *testing.T) { - apiID, err := format.ToAPIJobID(tc.input) + apiID, err := format.ToAPIJobIDLegacy(tc.input) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/src/frontend/xcecli/common/format/jobid.go b/src/frontend/xcecli/common/format/jobid.go index a337a32b..f4702afc 100644 --- a/src/frontend/xcecli/common/format/jobid.go +++ b/src/frontend/xcecli/common/format/jobid.go @@ -83,32 +83,53 @@ func MakeJobID(clusterID, apiserverID, timestamp uint64, tag, sequence uint64) u } var ( - // regexJobID matches "{int}-{int}-{int}-{int}" — mirrors REGEX_JOBID in config.h. - regexJobID = regexp.MustCompile(`^\s*(\d+)-(\d+)-(\d+)-(\d+)\s*$`) - // regexJobArrayID matches "{int}-{int}-{int}-{int}[{int}]" — mirrors REGEX_JOBARRAY_ID in config.h. - regexJobArrayID = regexp.MustCompile(`^\s*(\d+)-(\d+)-(\d+)-(\d+)\[(\d+)\]\s*$`) + // legacyRegexJobID and legacyRegexJobArrayID are the pre-feature, + // unbounded patterns kept only for ToAPIJobIDLegacy (peek is out of scope + // of job-array-one-based-index and keeps its behaviour). Identity parsing + // for every other caller goes through ParseSelection (jobsel.go). + legacyRegexJobID = regexp.MustCompile(`^\s*(\d+)-(\d+)-(\d+)-(\d+)\s*$`) + legacyRegexJobArrayID = regexp.MustCompile(`^\s*(\d+)-(\d+)-(\d+)-(\d+)\[(\d+)\]\s*$`) ) -// ParseJobID parses a human-readable job ID string into a packed 64-bit job ID, -// mirroring parse_jobid() in src/lib/utils.c:482. -// Accepts: "0-0-12874529-0" (regular) or "0-0-12874529-0[5]" (array child). +// ParseJobID parses one written job identity into a packed 64-bit job ID: +// "0-0-12874529-0" (a job, tag 0) or "0-0-12874529-32768[5]" (element 5 of +// that array, tag TAG_JOB_ARRAY). It is bounded like the C parse_jobid() +// (src/lib/utils.c): the grammar and checks of ParseSelection apply (5.16, +// 5.14, 5.8, 5.7), decimal keys are not accepted, and a range is rejected +// because it names no single job. func ParseJobID(s string) (uint64, error) { - if m := regexJobArrayID.FindStringSubmatch(s); m != nil { + sel, err := ParseSelection(s, false) + if err != nil { + return 0, err + } + if head, _, _ := splitSelection(sel.Raw); isDecimal(head) { + return 0, fmt.Errorf("invalid job ID format: %q", s) + } + if sel.Form == SelBare { + return sel.Key, nil + } + return sel.Key | uint64(tagJobArray)<", "[k]" or "[a-b]"). It is +// ParseSelection(s, allowRange).APIKey(): control commands pass +// allowRange=true, queries false. Text that is neither a written identity nor +// a decimal key is rejected rather than passed through. +func ToAPIJobID(s string, allowRange bool) (string, error) { + sel, err := ParseSelection(s, allowRange) + if err != nil { + return "", err } - return s, nil // already plain int or int[idx] -} \ No newline at end of file + return sel.APIKey(), nil +} + +// ToAPIJobIDLegacy keeps the pre-feature conversion for peek (out of scope of +// job-array-one-based-index): a written ID is converted without bounds or +// alignment checks, and anything else passes through unchanged. +func ToAPIJobIDLegacy(s string) (string, error) { + packed, ok := parseJobIDLegacy(s) + if !ok { + return s, nil // already plain int or int[idx] + } + if IsArrayJob(int64(packed)) { + baseID := ExtractBaseJobID(packed) + arrayIdx := extractBits(packed, 0, nArrayIdxBits) + return fmt.Sprintf("%d[%d]", baseID, arrayIdx), nil + } + return fmt.Sprintf("%d", packed), nil +} diff --git a/src/frontend/xcecli/common/format/jobid_test.go b/src/frontend/xcecli/common/format/jobid_test.go index 35db1302..e765abe1 100644 --- a/src/frontend/xcecli/common/format/jobid_test.go +++ b/src/frontend/xcecli/common/format/jobid_test.go @@ -1,6 +1,7 @@ package format import ( + "errors" "fmt" "testing" ) @@ -70,7 +71,15 @@ func TestParseFormatRoundTrip(t *testing.T) { } func TestParseJobIDInvalid(t *testing.T) { - for _, s := range []string{"abc", "1-2-3", "1[2]", ""} { + for _, s := range []string{"abc", "1-2-3", "1[2]", "", + "12345", // decimal keys are not written identities + "0-0-12874529-0[1-3]", // a range names no single job + "0-0-12874529-5[3]", // misaligned array identity (5.8) + "0-0-12874529-0[40000]", // element number above 32767 (5.7) + "0-0-12874529-1048576[1]", // sequence wider than 20 bits (5.14) + "64-0-12874529-0", // cluster wider than 6 bits (5.14) + "0-0-12874529-0 [1]", // whitespace before '[' (5.16) + } { if _, err := ParseJobID(s); err == nil { t.Errorf("ParseJobID(%q): expected error, got nil", s) } @@ -80,7 +89,7 @@ func TestParseJobIDInvalid(t *testing.T) { func TestToAPIJobIDRegular(t *testing.T) { packed := MakeJobID(0, 0, 12874529, 0, 42) s := FormatJobID(int64(packed)) - got, err := ToAPIJobID(s) + got, err := ToAPIJobID(s, false) if err != nil { t.Fatalf("ToAPIJobID(%q): %v", s, err) } @@ -93,7 +102,7 @@ func TestToAPIJobIDRegular(t *testing.T) { func TestToAPIJobIDArray(t *testing.T) { packed := MakeJobID(0, 0, 12874529, tagJobArray, 5) s := FormatJobID(int64(packed)) - got, err := ToAPIJobID(s) + got, err := ToAPIJobID(s, false) if err != nil { t.Fatalf("ToAPIJobID(%q): %v", s, err) } @@ -105,8 +114,8 @@ func TestToAPIJobIDArray(t *testing.T) { } func TestToAPIJobIDPassthrough(t *testing.T) { - for _, s := range []string{"123456", "123456[3]"} { - got, err := ToAPIJobID(s) + for _, s := range []string{"123456", "32768[3]"} { + got, err := ToAPIJobID(s, false) if err != nil { t.Fatalf("ToAPIJobID(%q): %v", s, err) } @@ -115,3 +124,54 @@ func TestToAPIJobIDPassthrough(t *testing.T) { } } } + +// A decimal key with [...] that no array could have is rejected under 5.8 +// before any request (an upgraded client applies its own 5.8 check whatever +// the server version). +func TestToAPIJobIDMisalignedRejected(t *testing.T) { + for _, allowRange := range []bool{false, true} { + _, err := ToAPIJobID("123456[3]", allowRange) + var se *SelError + if !errors.As(err, &se) || se.Rule != "5.8" { + t.Errorf("ToAPIJobID(%q, %v) error = %v, want 5.8 rejection", "123456[3]", allowRange, err) + } + } +} + +// A range of elements is accepted only by control commands. +func TestToAPIJobIDRange(t *testing.T) { + got, err := ToAPIJobID("32768[1-3]", true) + if err != nil || got != "32768[1-3]" { + t.Errorf("ToAPIJobID(%q, true) = %q, %v; want passthrough", "32768[1-3]", got, err) + } + if got, err := ToAPIJobID("32768[1-3]", false); err == nil { + t.Errorf("ToAPIJobID(%q, false) = %q, want refusal", "32768[1-3]", got) + } +} + +// Malformed text is rejected rather than passed through. +func TestToAPIJobIDRejectsMalformed(t *testing.T) { + for _, s := range []string{"plain_id", "X[1]", "123456[a]", "0-0-12874529-0[40000]", ""} { + if got, err := ToAPIJobID(s, true); err == nil { + t.Errorf("ToAPIJobID(%q, true) = %q, want error", s, got) + } + } +} + +// ToAPIJobIDLegacy keeps the pre-feature passthrough for peek (out of scope). +func TestToAPIJobIDLegacyPassthrough(t *testing.T) { + cases := []struct{ in, want string }{ + {"12345", "12345"}, + {"12345[0]", "12345[0]"}, + {"123456[3]", "123456[3]"}, + {"plain_id", "plain_id"}, + {"0-0-12874529-42", fmt.Sprintf("%d", MakeJobID(0, 0, 12874529, 0, 42))}, + {"0-0-12874529-0[5]", fmt.Sprintf("%d[5]", ExtractBaseJobID(MakeJobID(0, 0, 12874529, tagJobArray, 5)))}, + } + for _, c := range cases { + got, err := ToAPIJobIDLegacy(c.in) + if err != nil || got != c.want { + t.Errorf("ToAPIJobIDLegacy(%q) = %q, %v; want %q", c.in, got, err, c.want) + } + } +} diff --git a/src/frontend/xcecli/common/format/jobsel.go b/src/frontend/xcecli/common/format/jobsel.go new file mode 100644 index 00000000..d27dc0c3 --- /dev/null +++ b/src/frontend/xcecli/common/format/jobsel.go @@ -0,0 +1,334 @@ +package format + +import ( + "errors" + "fmt" + "strconv" + "strings" + "unicode" +) + +// Client element selections (job-array-one-based-index, design "CliSel and +// CliSplit"). The grammar and the check order mirror the API server's +// ParseIdentitySelection / ParseNameSelection (controllers/jobsel.go), so an +// upgraded client applies 5.16, 5.14, 5.8, 5.7 and 5.5 itself whatever the +// server version. Only 5.17 (a marked decimal key without [k]) is left to the +// server. This package stays dependency-free (no apiserver imports). + +// SelForm is the shape of a parsed selection. +type SelForm int + +const ( + SelBare SelForm = iota // no "[...]" + SelSingle // "[k]" (Lo == Hi == k) + SelRange // "[a-b]" (a == b allowed) +) + +// Selection is one parsed identity or name selection. +type Selection struct { + Raw string // as the user wrote it (trimmed) + Name string // name selections only (-J) + Key uint64 // identity selections: exact ID (bare) or array ID + Form SelForm + Lo, Hi int // element numbers, Lo == Hi for SelSingle +} + +// SelError is a rejected selection. Rule is the requirement it breaks +// ("5.16", "5.14", "5.8", "5.7", "5.5"), or "range" for an identity range +// given to a query command; Msg names the selection as written (trimmed). +type SelError struct { + Rule string + Msg string +} + +func (e *SelError) Error() string { return e.Msg } + +// RuleQueryRange tags the client-side refusal of an identity range in a query. +const RuleQueryRange = "range" + +// maxElementNumber is the largest element number a selection may name (5.7): +// the 15-bit array-index field. +const maxElementNumber = 1<= '0' && s[next] <= '9' { + if v > numSaturate/10 { + v = numSaturate + } else { + v = v*10 + int(s[next]-'0') + if v > numSaturate { + v = numSaturate + } + } + next++ + } + start := pos + for start < next && s[start] == '0' { + start++ + } + return v, s[start:next], next, next > pos +} + +// digitsLess reports whether decimal a < b for leading-zero-stripped digit +// strings: a shorter string is smaller, equal lengths compare lexically. +func digitsLess(a, b string) bool { + if len(a) != len(b) { + return len(a) < len(b) + } + return a < b +} + +// splitSelection splits a trimmed selection into the part before the element +// part and the element part. ok is false when the text breaks the 5.16 +// grammar of the element part: a ']' before any '[', an element part that is +// not "[num]" or "[num-num]", or any character after the closing ']'. +func splitSelection(s string) (head string, idx selIndex, ok bool) { + open := strings.IndexAny(s, "[]") + if open < 0 { + return s, idx, true + } + if s[open] == ']' { + return "", idx, false + } + head = s[:open] + idx.present = true + var good bool + pos := open + 1 + idx.lo, idx.loDigits, pos, good = readNum(s, pos) + if !good { + return "", idx, false + } + idx.hi, idx.hiDigits = idx.lo, idx.loDigits + if pos < len(s) && s[pos] == '-' { + idx.hi, idx.hiDigits, pos, good = readNum(s, pos+1) + if !good { + return "", idx, false + } + idx.isRange = true + } + if pos != len(s)-1 || s[pos] != ']' { + return "", idx, false + } + return head, idx, true +} + +// checkIndex applies 5.7, the query-range refusal (when !allowRange) and 5.5, +// in that order, and fills in the selection's form and numbers. +func checkIndex(sel Selection, idx selIndex, allowRange bool) (Selection, error) { + if !idx.present { + sel.Form = SelBare + return sel, nil + } + // 5.7 (a saturated number is always above the bound) + if idx.lo > maxElementNumber || idx.hi > maxElementNumber { + return newSelError("5.7", + "invalid element selection '%s': element numbers must be at most %d", + sel.Raw, maxElementNumber) + } + if idx.isRange && !allowRange { + return newSelError(RuleQueryRange, + "invalid element selection '%s': a range of elements is accepted only by control commands", + sel.Raw) + } + // 5.5 (compared on the digit strings, so the order is exact) + if digitsLess(idx.hiDigits, idx.loDigits) { + return newSelError("5.5", + "invalid element selection '%s': the range is reversed (last element number is less than the first)", + sel.Raw) + } + sel.Lo, sel.Hi = idx.lo, idx.hi + sel.Form = SelSingle + if idx.isRange { + sel.Form = SelRange + } + return sel, nil +} + +func isDecimal(s string) bool { + if s == "" { + return false + } + for i := 0; i < len(s); i++ { + if s[i] < '0' || s[i] > '9' { + return false + } + } + return true +} + +// errWrittenWidth marks a written identity with a field wider than allowed. +var errWrittenWidth = errors.New("field too wide") + +// parseWrittenID parses the strict written form c-a-ts-seq (each field one or +// more ASCII digits, nothing else). ok is false when s does not follow that +// grammar; err is errWrittenWidth when a field is wider than its bit width +// (cluster <= 63, API server <= 15, timestamp < 2^32, sequence < 2^20). +func parseWrittenID(s string) (key uint64, ok bool, err error) { + parts := strings.Split(s, "-") + if len(parts) != 4 { + return 0, false, nil + } + widths := [4]int{nClusterIDBits, nApiserverIDBits, nTimestampBits, nSequenceBits} + var f [4]uint64 + tooWide := false + for i, p := range parts { + if !isDecimal(p) { + return 0, false, nil + } + v, perr := strconv.ParseUint(p, 10, 64) + if perr != nil || v >= uint64(1)<= 0 { + ok = false + } + if !ok { + return newSelError("5.16", + "invalid element selection '%s': expected name[k] or name[a-b] (no whitespace in the name or brackets)", + sel.Raw) + } + sel.Name = head + return checkIndex(sel, idx, true) +} + +// SplitSelections splits a comma-separated list of selections at the commas +// outside "[...]", as the API server's extractJobNames does, so a selection +// such as "X[1,3]" stays whole and is reported as the user wrote it. A +// trailing empty item is dropped; the items are not trimmed. +func SplitSelections(list string) []string { + var out []string + cur := strings.Builder{} + depth := 0 + for _, r := range list { + switch { + case r == '[': + depth++ + case r == ']': + depth-- + case r == ',' && depth == 0: + out = append(out, cur.String()) + cur.Reset() + continue + } + cur.WriteRune(r) + } + if cur.Len() > 0 { + out = append(out, cur.String()) + } + return out +} + +// APIKey renders an identity selection in the form the REST API takes: +// "", "[k]" or "[a-b]". +func (s Selection) APIKey() string { + switch s.Form { + case SelSingle: + return fmt.Sprintf("%d[%d]", s.Key, s.Lo) + case SelRange: + return fmt.Sprintf("%d[%d-%d]", s.Key, s.Lo, s.Hi) + default: + return strconv.FormatUint(s.Key, 10) + } +} diff --git a/src/frontend/xcecli/common/format/jobsel_test.go b/src/frontend/xcecli/common/format/jobsel_test.go new file mode 100644 index 00000000..57531fe4 --- /dev/null +++ b/src/frontend/xcecli/common/format/jobsel_test.go @@ -0,0 +1,399 @@ +package format + +import ( + "errors" + "reflect" + "strconv" + "strings" + "testing" +) + +// Shared conformance table `selection-cases` (design.md, Testing Strategy), +// run for the command-line client: identity rows (written c-a-ts-seq and +// decimal keys) through ParseSelection, name rows through ParseNameSelection. +// `X` stands for a valid, array-capable written identity (sequence a multiple +// of 32768); `c-a-ts-5` for one that no array could have. + +const ( + selX = "3-5-1700000000-65536" + selXBad = "3-5-1700000000-5" // sequence not a multiple of 32768 (5.8) +) + +// selXKey is the packed, untagged value of selX. +var selXKey = MakeJobID(3, 5, 1700000000, 0, 65536) + +type selWant struct { + rule string // "" = accepted; else "5.16", "5.14", "5.8", "5.7", "5.5", "range" + form SelForm + lo int + hi int + exact bool // check key below instead of selXKey + key uint64 // expected Key when exact +} + +func accepted(form SelForm, lo, hi int) selWant { return selWant{form: form, lo: lo, hi: hi} } +func rejected(rule string) selWant { return selWant{rule: rule} } + +type selCase struct { + in string + want selWant +} + +// identitySelectionCases are the identity rows of the shared table (written +// form), as run by the apiserver's jobsel_test.go, with ranges allowed. +func identitySelectionCases() []selCase { + return []selCase{ + // --- Valid --- + {selX, accepted(SelBare, 0, 0)}, + {selX + "[1]", accepted(SelSingle, 1, 1)}, + {selX + "[0]", accepted(SelSingle, 0, 0)}, + {selX + "[01]", accepted(SelSingle, 1, 1)}, + {selX + "[1-3]", accepted(SelRange, 1, 3)}, + {selX + "[3-3]", accepted(SelRange, 3, 3)}, + {selX + "[32767]", accepted(SelSingle, 32767, 32767)}, + // leading zeros (1.5) + {selX + "[001-0003]", accepted(SelRange, 1, 3)}, + {"03-05-01700000000-065536[2]", accepted(SelSingle, 2, 2)}, + // whitespace before the identity / after the closing ']' is ignored (5.16) + {" \t" + selX + "[2] \t", accepted(SelSingle, 2, 2)}, + {" " + selX + " ", accepted(SelBare, 0, 0)}, + // a bare ordinary identity needs no alignment (5.6) + {selXBad, selWant{form: SelBare, exact: true, key: 3<<56 | 5<<52 | 1700000000<<20 | 5}}, + + // --- Syntax (5.16) --- + {selX + " [1]", rejected("5.16")}, + {selX + "[a]", rejected("5.16")}, + {selX + "[]", rejected("5.16")}, + {selX + "[1", rejected("5.16")}, + {selX + "]", rejected("5.16")}, + {selX + "[+1]", rejected("5.16")}, + {selX + "[-1]", rejected("5.16")}, + {selX + "[ 1]", rejected("5.16")}, + {selX + "[1-3:2]", rejected("5.16")}, + {selX + "[1]x", rejected("5.16")}, + {selX + "[1-2-3]", rejected("5.16")}, + {selX + "[1 ]", rejected("5.16")}, + {selX + "[1,3]", rejected("5.16")}, + {selX + "[1][2]", rejected("5.16")}, + {selX + "[1-]", rejected("5.16")}, + {selX + "[-3]", rejected("5.16")}, + {selX + "x", rejected("5.16")}, + {"", rejected("5.16")}, + {" \t ", rejected("5.16")}, + {"[1]", rejected("5.16")}, + {"3-5-1700000000", rejected("5.16")}, + {"3-5-1700000000-65536-1", rejected("5.16")}, + {"+3-5-1700000000-65536", rejected("5.16")}, + {"3 -5-1700000000-65536", rejected("5.16")}, + {"3--5-1700000000-65536", rejected("5.16")}, + {"foo[1]", rejected("5.16")}, + {"plain_id", rejected("5.16")}, + {"99999999999999999999-0-0-0[1]x", rejected("5.16")}, + + // --- Widths (5.14) --- + {"64-0-1700000000-32768[1]", rejected("5.14")}, + {"0-16-1700000000-32768[1]", rejected("5.14")}, + {"0-0-4294967296-32768[1]", rejected("5.14")}, + {"0-0-1700000000-1048576[1]", rejected("5.14")}, + {"64-0-1700000000-32768", rejected("5.14")}, + {"0-0-1700000000-1048576", rejected("5.14")}, + {"18446744073709551616-0-0-0", rejected("5.14")}, + + // --- Alignment (5.8) --- + {selXBad + "[3]", rejected("5.8")}, + {"3-5-1700000000-65537[1-2]", rejected("5.8")}, + + // --- Index (5.7) --- + {selX + "[32768]", rejected("5.7")}, + {selX + "[1-40000]", rejected("5.7")}, + {selX + "[40000-1]", rejected("5.7")}, + {selX + "[99999999999999999999]", rejected("5.7")}, + {selX + "[18446744073709551617]", rejected("5.7")}, + + // --- Reversed (5.5) --- + {selX + "[3-1]", rejected("5.5")}, + {selX + "[32767-0]", rejected("5.5")}, + {selX + "[0010-009]", rejected("5.5")}, + + // --- Precedence (5.12) --- + {"0-0-1700000000-1048576[40000]", rejected("5.14")}, + {selXBad + "[3-1]", rejected("5.8")}, + {selXBad + "[40000]", rejected("5.8")}, + {"64-0-1700000000-5[3-1]", rejected("5.14")}, + {"64-0-1700000000-5 [3-1]", rejected("5.16")}, + } +} + +func checkSel(t *testing.T, fn, in string, got Selection, gotErr error, want selWant, wantKey uint64) { + t.Helper() + if want.rule != "" { + if gotErr == nil { + t.Fatalf("%s(%q) = %+v, want rejection under %s", fn, in, got, want.rule) + } + var se *SelError + if !errors.As(gotErr, &se) { + t.Fatalf("%s(%q) error %T (%v), want *SelError", fn, in, gotErr, gotErr) + } + if se.Rule != want.rule { + t.Errorf("%s(%q) rule = %s (%s), want %s", fn, in, se.Rule, se.Msg, want.rule) + } + if raw := strings.TrimSpace(in); raw != "" && !strings.Contains(se.Msg, raw) { + t.Errorf("%s(%q) message %q does not name the selection", fn, in, se.Msg) + } + return + } + if gotErr != nil { + t.Fatalf("%s(%q) unexpected error: %v", fn, in, gotErr) + } + if got.Form != want.form || got.Lo != want.lo || got.Hi != want.hi { + t.Errorf("%s(%q) = form %d [%d-%d], want form %d [%d-%d]", + fn, in, got.Form, got.Lo, got.Hi, want.form, want.lo, want.hi) + } + if got.Key != wantKey { + t.Errorf("%s(%q) key = %d, want %d", fn, in, got.Key, wantKey) + } + if got.Raw != strings.TrimSpace(in) { + t.Errorf("%s(%q) Raw = %q, want trimmed input", fn, in, got.Raw) + } +} + +func TestParseSelection_SelectionCases(t *testing.T) { + for _, c := range identitySelectionCases() { + t.Run(c.in, func(t *testing.T) { + got, err := ParseSelection(c.in, true) + wantKey := selXKey + if c.want.exact { + wantKey = c.want.key + } + checkSel(t, "ParseSelection", c.in, got, err, c.want, wantKey) + }) + } +} + +// Decimal keys: the client applies 5.14 and, with [...], 5.8 and 5.7 itself +// (tag 0 and low 15 bits clear), whatever the server version. 5.17 (a marked +// decimal key without [k]) is left to the server. +func TestParseSelection_DecimalKeys(t *testing.T) { + xDec := strconv.FormatUint(selXKey, 10) + elem2 := selXKey | uint64(tagJobArray)<