diff --git a/internal/cmd/args_test.go b/internal/cmd/args_test.go index bd51c5c3..9a0e8685 100644 --- a/internal/cmd/args_test.go +++ b/internal/cmd/args_test.go @@ -41,3 +41,65 @@ func TestCleanUseLineStripsFlagsSuffix(t *testing.T) { t.Fatalf("cleanUseLine() = %q", line) } } + +func TestParseIntArgsRejectsNonPositive(t *testing.T) { + for _, tc := range []struct{ arg, want string }{ + {"0", "invalid posting ID: 0 (must be positive)"}, + {"-1", "invalid posting ID: -1 (must be positive)"}, + {"-99999", "invalid posting ID: -99999 (must be positive)"}, + } { + _, err := parseIntArgs([]string{tc.arg}) + if err == nil { + t.Errorf("parseIntArgs(%q): expected an error, got nil", tc.arg) + continue + } + // The clearer message is the point of the change, so assert it rather + // than just the presence of an error. + if err.Error() != tc.want { + t.Errorf("parseIntArgs(%q) = %q, want %q", tc.arg, err.Error(), tc.want) + } + } +} + +func TestParseIntArgsRejectsNonNumeric(t *testing.T) { + for _, tc := range []struct{ arg, want string }{ + {"abc", "invalid posting ID: abc"}, + {"", "invalid posting ID: "}, + {"1.5", "invalid posting ID: 1.5"}, + } { + _, err := parseIntArgs([]string{tc.arg}) + if err == nil { + t.Errorf("parseIntArgs(%q): expected an error, got nil", tc.arg) + continue + } + if err.Error() != tc.want { + t.Errorf("parseIntArgs(%q) = %q, want %q", tc.arg, err.Error(), tc.want) + } + } +} + +func TestParseIntArgsDeduplicatesPreservingOrder(t *testing.T) { + got, err := parseIntArgs([]string{"3", "1", "3", "2", "1"}) + if err != nil { + t.Fatalf("parseIntArgs: %v", err) + } + want := []int64{3, 1, 2} + if len(got) != len(want) { + t.Fatalf("got %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("got %v, want %v (first occurrence order preserved)", got, want) + } + } +} + +func TestParseIntArgsAcceptsValidIDs(t *testing.T) { + got, err := parseIntArgs([]string{"12345", "67890"}) + if err != nil { + t.Fatalf("parseIntArgs: %v", err) + } + if len(got) != 2 || got[0] != 12345 || got[1] != 67890 { + t.Errorf("got %v, want [12345 67890]", got) + } +} diff --git a/internal/cmd/seen.go b/internal/cmd/seen.go index a40866c4..905cf5c3 100644 --- a/internal/cmd/seen.go +++ b/internal/cmd/seen.go @@ -101,14 +101,32 @@ func (c *unseenCommand) run(cmd *cobra.Command, args []string) error { return writeOK(nil, output.WithSummary(summary)) } +// parseIntArgs parses posting IDs, rejecting non-positive values and dropping +// duplicates. Zero and negatives are not valid posting IDs, so including one in +// the posting_ids payload asks the server to act on something the client already +// knows is invalid; rejecting locally gives a clearer message than whatever +// comes back. Duplicates are dropped, first occurrence wins — for the bulk +// seen/unseen calls that only trims the payload, but it matters more for any +// caller that issues one request per ID, where a repeat can come back as a +// failure. func parseIntArgs(args []string) ([]int64, error) { ids := make([]int64, 0, len(args)) + seen := make(map[int64]bool, len(args)) + for _, arg := range args { id, err := strconv.ParseInt(arg, 10, 64) if err != nil { return nil, output.ErrUsage(fmt.Sprintf("invalid posting ID: %s", arg)) } + if id <= 0 { + return nil, output.ErrUsage(fmt.Sprintf("invalid posting ID: %d (must be positive)", id)) + } + if seen[id] { + continue + } + seen[id] = true ids = append(ids, id) } + return ids, nil }