multica/server/internal/handler/issue_limit_validation_test.go

package handler

import (
	"context"
	"encoding/json"
	"fmt"
	"net/http"
	"net/http/httptest"
	"testing"
	"time"
)

// Backs issue #3563 / MUL-2847: ListIssues must validate and clamp the `limit`
// and `offset` query params the same way the sibling endpoints (SearchIssues,
// ListGroupedIssues) already do. Without these guards:
//   - limit=-1  → Postgres rejects the negative LIMIT → HTTP 500
//   - limit=0   → returns 0 rows today, not 500; guarded only for
//     sibling-consistency (treat as "use default")
//   - limit=100000000 → unbounded read in one response
//   - offset=-1 → same 500 from Postgres
//   - non-numeric limit/offset → silently ignored today (this test pins that)
//
// Default is 100 and clamp is 100, matching the upstream issue's suggestion
// and the current default. All cases below must return 200 and a well-formed
// JSON body, never 500.
func TestListIssues_LimitValidation(t *testing.T) {
	ctx := context.Background()
	suffix := time.Now().UnixNano()

	// Seed three issues in a dedicated project so the test is hermetic and
	// not polluted by other tests' fixtures in the workspace.
	var projectID string
	if err := testPool.QueryRow(ctx, `
		INSERT INTO project (workspace_id, title) VALUES ($1, $2) RETURNING id
	`, testWorkspaceID, fmt.Sprintf("Limit Validation %d", suffix)).Scan(&projectID); err != nil {
		t.Fatalf("create project: %v", err)
	}
	t.Cleanup(func() {
		testPool.Exec(context.Background(), `DELETE FROM issue WHERE project_id = $1`, projectID)
		testPool.Exec(context.Background(), `DELETE FROM project WHERE id = $1`, projectID)
	})

	insertIssue := func(title string) string {
		var number int
		if err := testPool.QueryRow(ctx, `
			UPDATE workspace
			SET issue_counter = GREATEST(issue_counter, (SELECT COALESCE(MAX(number), 0) FROM issue WHERE workspace_id = $1)) + 1
			WHERE id = $1 RETURNING issue_counter
		`, testWorkspaceID).Scan(&number); err != nil {
			t.Fatalf("next issue number: %v", err)
		}
		var id string
		if err := testPool.QueryRow(ctx, `
			INSERT INTO issue (workspace_id, title, status, priority, creator_type, creator_id, position, number, project_id, space_id)
			VALUES ($1, $2, 'todo', 'none', 'member', $3, 0, $4, $5, (SELECT id FROM workspace_space WHERE workspace_id = $1 LIMIT 1)) RETURNING id
		`, testWorkspaceID, title, testUserID, number, projectID).Scan(&id); err != nil {
			t.Fatalf("create issue %q: %v", title, err)
		}
		return id
	}
	_ = insertIssue(fmt.Sprintf("limit-val-1-%d", suffix))
	_ = insertIssue(fmt.Sprintf("limit-val-2-%d", suffix))
	_ = insertIssue(fmt.Sprintf("limit-val-3-%d", suffix))

	type listResp struct {
		Issues []IssueResponse `json:"issues"`
		Total  int64           `json:"total"`
	}

	call := func(query string) (int, listResp, string) {
		path := fmt.Sprintf("/api/issues?workspace_id=%s&project_id=%s%s",
			testWorkspaceID, projectID, query)
		w := httptest.NewRecorder()
		testHandler.ListIssues(w, newRequest("GET", path, nil))
		var resp listResp
		body := w.Body.String()
		if w.Code == http.StatusOK {
			if err := json.NewDecoder(w.Body).Decode(&resp); err != nil {
				t.Fatalf("decode list response (q=%q): %v\nbody: %s", query, err, body)
			}
		}
		return w.Code, resp, body
	}

	// Cases that previously 500'd. All must return 200 with a well-formed
	// body. With only 3 seeded rows the clamp and default-fallback are
	// observably indistinguishable here — those behaviors are covered by
	// TestListIssues_LimitClamp below, which seeds 101 rows.
	cases := []struct {
		name  string
		query string
	}{
		{"negative limit falls back to default", "&limit=-1"},
		{"negative offset falls back to 0", "&offset=-1"},
		{"negative limit and offset", "&limit=-1&offset=-1"},
		{"non-numeric limit falls back to default", "&limit=abc"},
		{"non-numeric offset falls back to default", "&offset=abc"},
	}
	for _, tc := range cases {
		t.Run(tc.name, func(t *testing.T) {
			code, resp, body := call(tc.query)
			if code != http.StatusOK {
				t.Fatalf("expected 200, got %d: %s", code, body)
			}
			// We seeded exactly 3 issues, so any well-formed response must
			// report 3 of them.
			if resp.Total != 3 {
				t.Fatalf("total: want 3, got %d", resp.Total)
			}
		})
	}

	// Sanity: an explicit small limit is honored.
	t.Run("explicit limit below clamp is honored", func(t *testing.T) {
		code, resp, body := call("&limit=1")
		if code != http.StatusOK {
			t.Fatalf("expected 200, got %d: %s", code, body)
		}
		if len(resp.Issues) != 1 {
			t.Fatalf("limit=1: want 1 issue, got %d", len(resp.Issues))
		}
		if resp.Total != 3 {
			t.Fatalf("total: want 3, got %d", resp.Total)
		}
	})

	// Sanity: a positive offset is honored and yields the empty tail of the
	// page when it runs past the seeded set.
	t.Run("positive offset is honored", func(t *testing.T) {
		code, resp, body := call("&limit=2&offset=2")
		if code != http.StatusOK {
			t.Fatalf("expected 200, got %d: %s", code, body)
		}
		if len(resp.Issues) != 1 {
			t.Fatalf("limit=2 offset=2: want 1 issue (the 3rd of 3), got %d", len(resp.Issues))
		}
		if resp.Total != 3 {
			t.Fatalf("total: want 3, got %d", resp.Total)
		}
	})
}

// TestListIssues_LimitClamp proves the upper-bound clamp on `limit` actually
// fires. TestListIssues_LimitValidation above seeds only 3 rows, so a missing
// clamp would still return 3 rows for `limit=100000000` and the assertion
// would pass. Here we seed 101 rows and pin the clamp at exactly 100, the
// same boundary the production endpoint promises.
func TestListIssues_LimitClamp(t *testing.T) {
	const seeded = 101
	ctx := context.Background()
	suffix := time.Now().UnixNano()

	var projectID string
	if err := testPool.QueryRow(ctx, `
		INSERT INTO project (workspace_id, title) VALUES ($1, $2) RETURNING id
	`, testWorkspaceID, fmt.Sprintf("Limit Clamp %d", suffix)).Scan(&projectID); err != nil {
		t.Fatalf("create project: %v", err)
	}
	t.Cleanup(func() {
		testPool.Exec(context.Background(), `DELETE FROM issue WHERE project_id = $1`, projectID)
		testPool.Exec(context.Background(), `DELETE FROM project WHERE id = $1`, projectID)
	})

	// Seed 101 issues. Each row is inserted individually so the workspace's
	// `issue_counter` advances correctly via the same path real issues take;
	// `LIMIT 101` returning exactly 101 rows is itself a sanity check that
	// nothing in the test wiring is off.
	insertIssue := func(idx int) {
		title := fmt.Sprintf("clamp-%d-%d", suffix, idx)
		var number int
		if err := testPool.QueryRow(ctx, `
			UPDATE workspace
			SET issue_counter = GREATEST(issue_counter, (SELECT COALESCE(MAX(number), 0) FROM issue WHERE workspace_id = $1)) + 1
			WHERE id = $1 RETURNING issue_counter
		`, testWorkspaceID).Scan(&number); err != nil {
			t.Fatalf("next issue number: %v", err)
		}
		if _, err := testPool.Exec(ctx, `
			INSERT INTO issue (workspace_id, title, status, priority, creator_type, creator_id, position, number, project_id, space_id)
			VALUES ($1, $2, 'todo', 'none', 'member', $3, 0, $4, $5, (SELECT id FROM workspace_space WHERE workspace_id = $1 LIMIT 1))
		`, testWorkspaceID, title, testUserID, number, projectID); err != nil {
			t.Fatalf("create issue #%d: %v", idx, err)
		}
	}
	for i := 0; i < seeded; i++ {
		insertIssue(i)
	}

	type listResp struct {
		Issues []IssueResponse `json:"issues"`
		Total  int64           `json:"total"`
	}

	call := func(query string) (int, listResp, string) {
		path := fmt.Sprintf("/api/issues?workspace_id=%s&project_id=%s%s",
			testWorkspaceID, projectID, query)
		w := httptest.NewRecorder()
		testHandler.ListIssues(w, newRequest("GET", path, nil))
		var resp listResp
		body := w.Body.String()
		if w.Code == http.StatusOK {
			if err := json.NewDecoder(w.Body).Decode(&resp); err != nil {
				t.Fatalf("decode list response (q=%q): %v\nbody: %s", query, err, body)
			}
		}
		return w.Code, resp, body
	}

	// First sanity-check: with no limit, ListIssues defaults to 100, so
	// exactly 100 rows come back. This would NOT pass if the seeded set
	// were broken or the default were wrong.
	t.Run("no limit returns default page of 100", func(t *testing.T) {
		code, resp, body := call("")
		if code != http.StatusOK {
			t.Fatalf("expected 200, got %d: %s", code, body)
		}
		if got := len(resp.Issues); got != 100 {
			t.Fatalf("default page size: want 100, got %d", got)
		}
		if resp.Total != seeded {
			t.Fatalf("total: want %d, got %d", seeded, resp.Total)
		}
	})

	// The actual clamp assertions. Without the `if limit > 100` clamp, the
	// huge/above-clamp cases would return 101 rows and these would fail.
	clampCases := []struct {
		name  string
		query string
		want  int
	}{
		{"huge limit is clamped to 100", "&limit=100000000", 100},
		{"one above the clamp", "&limit=101", 100},
		{"well above the clamp", "&limit=200", 100},
		{"at the clamp boundary", "&limit=100", 100},
	}
	for _, tc := range clampCases {
		t.Run(tc.name, func(t *testing.T) {
			code, resp, body := call(tc.query)
			if code != http.StatusOK {
				t.Fatalf("expected 200, got %d: %s", code, body)
			}
			if got := len(resp.Issues); got != tc.want {
				t.Fatalf("len(issues) for %q: want %d, got %d", tc.query, tc.want, got)
			}
			if resp.Total != seeded {
				t.Fatalf("total: want %d, got %d", seeded, resp.Total)
			}
		})
	}

	// Limits below the clamp must be honored (clamp is upper-bound only).
	t.Run("limit below clamp is honored", func(t *testing.T) {
		code, resp, body := call("&limit=50")
		if code != http.StatusOK {
			t.Fatalf("expected 200, got %d: %s", code, body)
		}
		if got := len(resp.Issues); got != 50 {
			t.Fatalf("limit=50: want 50 issues, got %d", got)
		}
		if resp.Total != seeded {
			t.Fatalf("total: want %d, got %d", seeded, resp.Total)
		}
	})

	// Offset and clamp compose against the full result set. The SQL is
	// `LIMIT $limit OFFSET $offset` (with the limit already clamped to 100),
	// so `limit=200&offset=50` over 101 rows produces `LIMIT 100 OFFSET 50`,
	// which is rows 50..100 — 51 rows. This subtest pins the composition:
	// it would fail with a 500 if the offset guard were missing, with a
	// different count if the clamp were applied to the wrong axis, and with
	// `len == 0` if a buggy implementation skipped rows beyond the clamped
	// page instead of against the full result set.
	t.Run("offset and clamp compose against the full result set", func(t *testing.T) {
		code, resp, body := call("&limit=200&offset=50")
		if code != http.StatusOK {
			t.Fatalf("expected 200, got %d: %s", code, body)
		}
		if got := len(resp.Issues); got != 51 {
			t.Fatalf("limit=200 offset=50: want 51 issues, got %d", got)
		}
		if resp.Total != seeded {
			t.Fatalf("total: want %d, got %d", seeded, resp.Total)
		}
	})
}