diff --git a/internal/api/handler_recommendations.go b/internal/api/handler_recommendations.go index 0c2d33e75..1f13a0b40 100644 --- a/internal/api/handler_recommendations.go +++ b/internal/api/handler_recommendations.go @@ -94,6 +94,10 @@ func (h *Handler) getRecommendations(ctx context.Context, req *events.LambdaFunc if err != nil { return nil, err } + page, err := parseRecommendationsPage(params) + if err != nil { + return nil, err + } // Resolve the account scope once: it decides both whether this read may // kick a background collection (issue #627) and the post-filter below. @@ -122,7 +126,7 @@ func (h *Handler) getRecommendations(ctx context.Context, req *events.LambdaFunc return nil, err } - return buildRecommendationsResponse(recommendations), nil + return buildRecommendationsPageResponse(recommendations, page) } // filterRecommendationsByAllowedAccounts filters recommendations to only include @@ -195,17 +199,28 @@ func (h *Handler) filterRecommendationsByEnabledProviders(ctx context.Context, r } // buildRecommendationsResponse calculates summary statistics and collects -// unique regions from a set of recommendations. +// unique regions from a set of recommendations, returning all of them as rows +// (the legacy unpaginated shape). func buildRecommendationsResponse(recommendations []config.RecommendationRecord) *RecommendationsResponse { + return summarizeRecommendations(recommendations, recommendations) +} + +// summarizeRecommendations computes the summary and region list over all (the +// full filtered set) and returns rows as the response body. rows is either all +// itself or one page of it (issue #723), so the totals never shrink to the +// page. +func summarizeRecommendations(all, rows []config.RecommendationRecord) *RecommendationsResponse { regionSet := make(map[string]struct{}) var totalSavings, totalUpfront float64 - for i := range recommendations { - rec := &recommendations[i] + for i := range all { + rec := &all[i] totalSavings += rec.Savings totalUpfront += rec.UpfrontCost if rec.Region != "" { regionSet[rec.Region] = struct{}{} } + } + for i := range rows { // Surface the compute size (VCPU / MemoryGB) at the top level so // the frontend Capacity column can render it without parsing the // opaque Details blob. The canonical values live in the typed @@ -214,8 +229,8 @@ func buildRecommendationsResponse(recommendations []config.RecommendationRecord) // pointers when this is a compute rec with a known size. Non-compute // recs, legacy rows with empty Details, and decode failures all // leave the pointers nil, so JSON omits them and the frontend - // renders "—". - stampComputeCapacity(rec) + // renders "—". Only returned rows are decoded. + stampComputeCapacity(&rows[i]) } regions := make([]string, 0, len(regionSet)) @@ -229,9 +244,9 @@ func buildRecommendationsResponse(recommendations []config.RecommendationRecord) } return &RecommendationsResponse{ - Recommendations: recommendations, + Recommendations: rows, Summary: RecommendationsSummary{ - TotalCount: len(recommendations), + TotalCount: len(all), TotalMonthlySavings: totalSavings, TotalUpfrontCost: totalUpfront, AvgPaybackMonths: avgPayback, diff --git a/internal/api/handler_recommendations_page.go b/internal/api/handler_recommendations_page.go new file mode 100644 index 000000000..dd40dbb04 --- /dev/null +++ b/internal/api/handler_recommendations_page.go @@ -0,0 +1,235 @@ +package api + +import ( + "encoding/base64" + "encoding/json" + "fmt" + "sort" + "strconv" + "strings" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/logging" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" +) + +// Opt-in cursor pagination for GET /api/recommendations (refs issue #723). +// +// The list endpoint used to return the whole filtered cache in one body, +// which passes the 6 MB Lambda response cap once a tenant accumulates a few +// thousand recommendations. A caller that sends `limit` (and, for later +// pages, the `cursor` returned as `next_cursor`) gets one bounded page. +// This bounds the response only: every request still reads, filters and +// sorts the whole filtered set in memory and then slices the page from it +// (store-side paging is tracked in #981). A +// page is bounded twice: by row count (limit) and by serialized bytes +// (recommendationsPageByteBudget), because the opaque Details blob makes the +// size of a row data-dependent. +// A request carrying neither parameter keeps the existing unpaginated shape +// unchanged, so the current frontend (which sorts, groups and totals the +// full set client-side) keeps working until it adopts pagination. +// +// In both modes `summary` and `regions` describe the full filtered set, not +// just the returned page, so headline totals stay correct while paging. +const ( + // DefaultRecommendationsPageSize is the page size used when a caller + // sends a cursor without a limit. + DefaultRecommendationsPageSize = 500 + + // MaxRecommendationsPageSize caps the row count of one page. It is an + // upper bound only: recommendationsPageByteBudget can cut a page shorter. + MaxRecommendationsPageSize = 1000 + + // recommendationsPageByteBudget caps the bytes one page adds to the + // Lambda response payload. The Lambda limit (6 MiB, 6291456 bytes) + // applies to the whole invoke response: the JSON envelope with status + // code and headers, plus the body re-encoded as a JSON string, which + // escapes every quote and backslash again. Rows are therefore measured + // as they appear inside that string (see envelopeLen), and the budget is + // checked against uncompressed bytes because the #775 gzip only runs + // when the client sends Accept-Encoding: gzip. 4 MiB leaves about 2 MiB + // for the envelope, headers and next_cursor, which are a few KiB. + recommendationsPageByteBudget = 4 << 20 + + // maxRecommendationsCursorLen bounds the encoded cursor so a hostile + // query string cannot force a large decode. + maxRecommendationsCursorLen = 1024 +) + +// recommendationsPage is the parsed pagination request. enabled is false when +// the caller sent neither limit nor cursor (legacy unpaginated mode). +type recommendationsPage struct { + enabled bool + limit int + afterID string +} + +// parseRecommendationsPage validates the optional limit and cursor query +// parameters. limit must be an integer in 1..MaxRecommendationsPageSize; +// out-of-range values are rejected rather than clamped so a client bug does +// not silently return a different page size than it asked for. Legacy +// unpaginated mode is selected only when neither parameter is present: a +// present but empty value (?limit= or ?cursor=) is a 400, so a client bug +// cannot silently fall back to the unbounded response. +func parseRecommendationsPage(params map[string]string) (recommendationsPage, error) { + rawLimit, hasLimit := params["limit"] + rawCursor, hasCursor := params["cursor"] + if !hasLimit && !hasCursor { + return recommendationsPage{}, nil + } + // From here a present but empty value fails the checks below: "" is not + // an integer, and an empty cursor does not decode to an ID. + rawLimit = strings.TrimSpace(rawLimit) + rawCursor = strings.TrimSpace(rawCursor) + + page := recommendationsPage{enabled: true, limit: DefaultRecommendationsPageSize} + if hasLimit { + n, err := strconv.Atoi(rawLimit) + if err != nil || n < 1 || n > MaxRecommendationsPageSize { + return recommendationsPage{}, NewClientError(400, + "invalid limit: must be an integer between 1 and "+strconv.Itoa(MaxRecommendationsPageSize)) + } + page.limit = n + } + if hasCursor { + afterID, err := decodeRecommendationsCursor(rawCursor) + if err != nil { + return recommendationsPage{}, err + } + page.afterID = afterID + } + return page, nil +} + +// encodeRecommendationsCursor turns the last row ID of a page into an opaque, +// URL-safe cursor. Recommendation IDs can carry characters such as "|", so +// the raw ID is not used directly in the query string. +func encodeRecommendationsCursor(id string) string { + return base64.RawURLEncoding.EncodeToString([]byte(id)) +} + +// decodeRecommendationsCursor reverses encodeRecommendationsCursor and +// rejects malformed or oversized cursors with a 400. +func decodeRecommendationsCursor(cursor string) (string, error) { + if len(cursor) > maxRecommendationsCursorLen { + return "", NewClientError(400, "invalid cursor: too long") + } + raw, err := base64.RawURLEncoding.DecodeString(cursor) + if err != nil || len(raw) == 0 { + return "", NewClientError(400, "invalid cursor") + } + return string(raw), nil +} + +// sortRecommendationsAfter sorts recs by ID in place (it is this request's +// own slice, freshly read from the store) and returns the index of the first +// row strictly after afterID. +// +// The ID is the application recommendation ID that the scheduler builds from +// the natural-key columns (recommendationID in internal/scheduler), not the +// cache table's UUID primary key. Paging needs ID order to be a total order: +// two rows sharing an ID would let the strict "after" comparison skip the +// second one at a page boundary. A duplicate is therefore an error rather +// than a silently short walk. +// +// Resuming after the last ID (rather than at an offset) means a row added or +// removed between two requests does not shift later pages: the next page +// always resumes after the last ID the client saw, even if that row has +// since been deleted. +func sortRecommendationsAfter(recs []config.RecommendationRecord, afterID string) (int, error) { + sort.Slice(recs, func(i, j int) bool { return recs[i].ID < recs[j].ID }) + for i := 1; i < len(recs); i++ { + if recs[i].ID == recs[i-1].ID { + return 0, fmt.Errorf("duplicate recommendation ID %q: cannot page the recommendations list", recs[i].ID) + } + } + if afterID == "" { + return 0, nil + } + return sort.Search(len(recs), func(i int) bool { return recs[i].ID > afterID }), nil +} + +// envelopeLen returns how many bytes v adds to the Lambda invoke response: +// its JSON encoding, re-encoded as the content of the response's body +// string (without the surrounding quotes). +func envelopeLen(v any) (int, error) { + raw, err := json.Marshal(v) + if err != nil { + return 0, err + } + quoted, err := json.Marshal(string(raw)) + if err != nil { + return 0, err + } + return len(quoted) - 2, nil +} + +// selectRecommendationsPage stamps and measures rows from sorted[start:] and +// returns how many fit in one page: at most limit rows, and no more bytes +// than budget once the fixed overhead (summary, regions, keys) and the next +// cursor are counted. It always takes at least one row when any remain, so a +// row that alone exceeds the budget cannot stall the cursor walk; that row +// is returned on its own page with a logged warning (it only fails at Lambda +// if it is over the 6 MiB hard cap by itself, which no page size could fix). +func selectRecommendationsPage(sorted []config.RecommendationRecord, start, limit, overhead, budget int) (int, error) { + used := overhead + n := 0 + for i := start; i < len(sorted) && n < limit; i++ { + stampComputeCapacity(&sorted[i]) + size, err := envelopeLen(&sorted[i]) + if err != nil { + return 0, fmt.Errorf("failed to size recommendation %q: %w", sorted[i].ID, err) + } + // +1 for the separating comma; the cursor that would point at this + // row is reserved so next_cursor always fits. + cost := size + 1 + cursorLen := base64.RawURLEncoding.EncodedLen(len(sorted[i].ID)) + if n > 0 && used+cost+cursorLen > budget { + break + } + if n == 0 && used+cost+cursorLen > budget { + logging.Warnf("recommendation %q alone is %d bytes, over the %d-byte page budget; returning it on its own page", sorted[i].ID, size, budget) + } + used += cost + n++ + } + return n, nil +} + +// buildRecommendationsPageResponse builds the list response for either mode. +// Unpaginated requests get exactly the legacy buildRecommendationsResponse +// output; paginated requests get one page of rows plus next_cursor, with the +// summary and regions still computed over the full filtered set. +func buildRecommendationsPageResponse(recs []config.RecommendationRecord, page recommendationsPage) (*RecommendationsResponse, error) { + if !page.enabled { + return buildRecommendationsResponse(recs), nil + } + return buildBudgetedPage(recs, page, recommendationsPageByteBudget) +} + +// buildBudgetedPage is the paginated branch of +// buildRecommendationsPageResponse with an explicit byte budget. +func buildBudgetedPage(recs []config.RecommendationRecord, page recommendationsPage, budget int) (*RecommendationsResponse, error) { + start, err := sortRecommendationsAfter(recs, page.afterID) + if err != nil { + return nil, err + } + resp := summarizeRecommendations(recs, recs[start:start]) + // Overhead: everything except the rows and the cursor value, measured + // with an empty next_cursor key present. + resp.NextCursor = " " + overhead, err := envelopeLen(resp) + if err != nil { + return nil, fmt.Errorf("failed to size recommendations page: %w", err) + } + n, err := selectRecommendationsPage(recs, start, page.limit, overhead, budget) + if err != nil { + return nil, err + } + end := start + n + resp.Recommendations = recs[start:end] + resp.NextCursor = "" + if end < len(recs) { + resp.NextCursor = encodeRecommendationsCursor(recs[end-1].ID) + } + return resp, nil +} diff --git a/internal/api/handler_recommendations_page_test.go b/internal/api/handler_recommendations_page_test.go new file mode 100644 index 000000000..101986cfc --- /dev/null +++ b/internal/api/handler_recommendations_page_test.go @@ -0,0 +1,407 @@ +package api + +import ( + "context" + "encoding/json" + "fmt" + "strconv" + "strings" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/aws/aws-lambda-go/events" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// lambdaResponseCapBytes is the AWS Lambda synchronous response payload cap +// that GET /api/recommendations used to exceed (issue #723). +const lambdaResponseCapBytes = 6 * 1024 * 1024 + +// pageTestRecs returns n recommendations in deliberately non-ID order, each +// padded so a single row is about 1 KB of JSON. +func pageTestRecs(n int) []config.RecommendationRecord { + pad := strings.Repeat("x", 900) + recs := make([]config.RecommendationRecord, n) + for i := range n { + // Reverse order so the handler has to sort to page stably. + idx := n - 1 - i + recs[i] = config.RecommendationRecord{ + ID: fmt.Sprintf("rec-%06d", idx), + Provider: "aws", + Service: "ec2", + Region: "us-east-" + strconv.Itoa(idx%3+1), + ResourceType: "m5.large-" + pad, + Savings: 10, + UpfrontCost: 20, + } + } + return recs +} + +func pageTestHandler(t *testing.T, recs []config.RecommendationRecord) *Handler { + t.Helper() + ms := new(MockScheduler) + ms.On("ListRecommendations", mock.Anything, mock.Anything).Return(recs, nil) + return &Handler{scheduler: ms, apiKey: "test-key"} +} + +func pageTestReq() *events.LambdaFunctionURLRequest { + return &events.LambdaFunctionURLRequest{Headers: map[string]string{"x-api-key": "test-key"}} +} + +// TestGetRecommendations_PaginationBoundsResponse is the #723 regression: a +// tenant whose full list is well over 6 MB must be able to fetch it in pages +// that each fit under the Lambda cap, and walking the cursor must return +// every row exactly once. +func TestGetRecommendations_PaginationBoundsResponse(t *testing.T) { + const total = 8000 + recs := pageTestRecs(total) + h := pageTestHandler(t, recs) + + full, err := json.Marshal(buildRecommendationsResponse(append([]config.RecommendationRecord(nil), recs...))) + require.NoError(t, err) + require.Greater(t, len(full), lambdaResponseCapBytes, "fixture must exceed the cap to be meaningful") + + seen := make(map[string]int, total) + var order []string + params := map[string]string{"limit": strconv.Itoa(MaxRecommendationsPageSize)} + for pages := 0; ; pages++ { + require.Less(t, pages, total, "cursor walk did not terminate") + resp, err := h.getRecommendations(context.Background(), pageTestReq(), params) + require.NoError(t, err) + + body, err := json.Marshal(resp) + require.NoError(t, err) + assert.Less(t, len(body), lambdaResponseCapBytes, "page %d exceeds the Lambda response cap", pages) + assert.LessOrEqual(t, len(resp.Recommendations), MaxRecommendationsPageSize) + + // Summary describes the full filtered set on every page. + assert.Equal(t, total, resp.Summary.TotalCount) + assert.InDelta(t, float64(total*10), resp.Summary.TotalMonthlySavings, 1e-6) + assert.Len(t, resp.Regions, 3) + + for i := range resp.Recommendations { + id := resp.Recommendations[i].ID + seen[id]++ + order = append(order, id) + } + if resp.NextCursor == "" { + break + } + params = map[string]string{"limit": strconv.Itoa(MaxRecommendationsPageSize), "cursor": resp.NextCursor} + } + + assert.Len(t, seen, total, "every row must be returned") + for id, n := range seen { + assert.Equal(t, 1, n, "row %s returned %d times", id, n) + } + assert.IsIncreasing(t, order, "rows must be returned in ID order across pages") +} + +// TestGetRecommendations_UnpaginatedShapeUnchanged pins backward +// compatibility: without limit/cursor the endpoint still returns every row, +// in the order the scheduler gave them, with no next_cursor. +func TestGetRecommendations_UnpaginatedShapeUnchanged(t *testing.T) { + recs := pageTestRecs(5) + // Copy the expected order before the call: the mock hands the handler + // this same backing array, so an in-place sort would also reorder recs. + wantIDs := make([]string, len(recs)) + for i := range recs { + wantIDs[i] = recs[i].ID + } + h := pageTestHandler(t, recs) + + resp, err := h.getRecommendations(context.Background(), pageTestReq(), map[string]string{}) + require.NoError(t, err) + require.Len(t, resp.Recommendations, 5) + gotIDs := make([]string, len(resp.Recommendations)) + for i := range resp.Recommendations { + gotIDs[i] = resp.Recommendations[i].ID + } + assert.Equal(t, wantIDs, gotIDs, "legacy mode must keep the scheduler's order") + assert.Empty(t, resp.NextCursor) + assert.Equal(t, 5, resp.Summary.TotalCount) + + body, err := json.Marshal(resp) + require.NoError(t, err) + assert.NotContains(t, string(body), "next_cursor") +} + +// TestGetRecommendations_PaginationRejectsDuplicateIDs pins the total-order +// guard: two rows sharing an ID would let the strict "after" comparison skip +// one of them at a page boundary (a,b,b,c with limit=2 used to return only +// three rows), so a paginated request fails instead of walking short. Legacy +// mode does not sort or page and still returns every row. +func TestGetRecommendations_PaginationRejectsDuplicateIDs(t *testing.T) { + recs := pageTestRecs(4) + recs[1].ID = recs[2].ID + h := pageTestHandler(t, recs) + ctx := context.Background() + + _, err := h.getRecommendations(ctx, pageTestReq(), map[string]string{"limit": "2"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "duplicate recommendation ID") + + legacy, err := h.getRecommendations(ctx, pageTestReq(), map[string]string{}) + require.NoError(t, err) + assert.Len(t, legacy.Recommendations, 4) +} + +func TestGetRecommendations_PaginationPages(t *testing.T) { + recs := pageTestRecs(5) // rec-000000 .. rec-000004, stored in reverse + h := pageTestHandler(t, recs) + ctx := context.Background() + + first, err := h.getRecommendations(ctx, pageTestReq(), map[string]string{"limit": "2"}) + require.NoError(t, err) + require.Len(t, first.Recommendations, 2) + assert.Equal(t, "rec-000000", first.Recommendations[0].ID) + assert.Equal(t, "rec-000001", first.Recommendations[1].ID) + assert.Equal(t, encodeRecommendationsCursor("rec-000001"), first.NextCursor) + assert.Equal(t, 5, first.Summary.TotalCount) + + last, err := h.getRecommendations(ctx, pageTestReq(), map[string]string{ + "limit": "2", "cursor": encodeRecommendationsCursor("rec-000003"), + }) + require.NoError(t, err) + require.Len(t, last.Recommendations, 1) + assert.Equal(t, "rec-000004", last.Recommendations[0].ID) + assert.Empty(t, last.NextCursor, "last page must not carry a cursor") + + // Exact fit: the page ending on the final row has no next cursor. + exact, err := h.getRecommendations(ctx, pageTestReq(), map[string]string{"limit": "5"}) + require.NoError(t, err) + assert.Len(t, exact.Recommendations, 5) + assert.Empty(t, exact.NextCursor) + + // Keyset: a cursor naming a row that has since disappeared resumes at + // the next ID instead of failing or restarting. + gone, err := h.getRecommendations(ctx, pageTestReq(), map[string]string{ + "limit": "2", "cursor": encodeRecommendationsCursor("rec-000001x"), + }) + require.NoError(t, err) + require.Len(t, gone.Recommendations, 2) + assert.Equal(t, "rec-000002", gone.Recommendations[0].ID) + + // Cursor past the end yields an empty page, not an error. + past, err := h.getRecommendations(ctx, pageTestReq(), map[string]string{ + "cursor": encodeRecommendationsCursor("rec-999999"), + }) + require.NoError(t, err) + assert.Empty(t, past.Recommendations) + assert.Empty(t, past.NextCursor) + assert.Equal(t, 5, past.Summary.TotalCount) +} + +func TestGetRecommendations_CursorWithoutLimitUsesDefaultPageSize(t *testing.T) { + recs := pageTestRecs(DefaultRecommendationsPageSize + 3) + h := pageTestHandler(t, recs) + + resp, err := h.getRecommendations(context.Background(), pageTestReq(), map[string]string{ + "cursor": encodeRecommendationsCursor("rec-"), + }) + require.NoError(t, err) + assert.Len(t, resp.Recommendations, DefaultRecommendationsPageSize) + assert.NotEmpty(t, resp.NextCursor) +} + +func TestGetRecommendations_PaginationRejectsBadParams(t *testing.T) { + cases := map[string]map[string]string{ + "zero limit": {"limit": "0"}, + "negative limit": {"limit": "-1"}, + "limit above max": {"limit": strconv.Itoa(MaxRecommendationsPageSize + 1)}, + "non-numeric limit": {"limit": "abc"}, + "fractional limit": {"limit": "1.5"}, + "non-base64 cursor": {"cursor": "!!!"}, + // Valid base64 (length is a multiple of 4), so only the length cap + // can reject it. + "oversized cursor": {"cursor": strings.Repeat("A", maxRecommendationsCursorLen+4)}, + "bad cursor + limit": {"limit": "10", "cursor": "%%%"}, + // Present but empty must not fall back to the unbounded legacy shape. + "empty limit": {"limit": ""}, + "blank limit": {"limit": " "}, + "empty cursor": {"cursor": ""}, + "blank cursor": {"cursor": " \t"}, + "valid limit + empty cursor": {"limit": "10", "cursor": ""}, + "empty limit + valid cursor": {"limit": "", "cursor": encodeRecommendationsCursor("rec-000001")}, + } + for name, params := range cases { + t.Run(name, func(t *testing.T) { + // No scheduler: validation must reject before any store read. + h := &Handler{apiKey: "test-key"} + _, err := h.getRecommendations(context.Background(), pageTestReq(), params) + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok, "expected ClientError, got %T: %v", err, err) + assert.Equal(t, 400, ce.code) + }) + } +} + +func TestGetRecommendations_PaginationMaxLimitAccepted(t *testing.T) { + h := pageTestHandler(t, pageTestRecs(3)) + resp, err := h.getRecommendations(context.Background(), pageTestReq(), map[string]string{ + "limit": strconv.Itoa(MaxRecommendationsPageSize), + }) + require.NoError(t, err) + assert.Len(t, resp.Recommendations, 3) + assert.Empty(t, resp.NextCursor) +} + +// bigDetailsRecs returns n recommendations whose Details blob is detailBytes +// of quote-heavy JSON. Each quote doubles again when the body is re-encoded +// into the Lambda response envelope, so measuring raw JSON would undercount. +func bigDetailsRecs(n, detailBytes int) []config.RecommendationRecord { + recs := pageTestRecs(n) + note := strings.Repeat(`"<&\\`, detailBytes/4) + details, err := json.Marshal(map[string]string{"note": note}) + if err != nil { + panic(err) + } + for i := range recs { + recs[i].ResourceType = "m5.large" + recs[i].Details = details + } + return recs +} + +// lambdaPayloadLen is the size of the Lambda invoke response that would carry +// resp, matching how handler.go and the Lambda runtime encode it. +func lambdaPayloadLen(t *testing.T, resp *RecommendationsResponse) (bodyInEnvelope, total int) { + t.Helper() + body, err := json.Marshal(resp) + require.NoError(t, err) + quoted, err := json.Marshal(string(body)) + require.NoError(t, err) + envelope, err := json.Marshal(events.LambdaFunctionURLResponse{ + StatusCode: 200, + Headers: map[string]string{"Content-Type": "application/json"}, + Body: string(body), + }) + require.NoError(t, err) + return len(quoted) - 2, len(envelope) +} + +// TestGetRecommendations_PageCutByByteBudget: with large Details, limit rows +// would be far over the Lambda cap, so pages must be cut by serialized bytes +// while still visiting every row exactly once, in order, with full totals. +func TestGetRecommendations_PageCutByByteBudget(t *testing.T) { + const total = 1500 + recs := bigDetailsRecs(total, 1500) // ~10 KB per row in the envelope + h := pageTestHandler(t, recs) + + _, fullSize := lambdaPayloadLen(t, buildRecommendationsResponse(append([]config.RecommendationRecord(nil), recs...))) + require.Greater(t, fullSize, 2*lambdaResponseCapBytes, "fixture must be far over the cap") + + seen := make(map[string]int, total) + var order []string + cutByBytes := false + params := map[string]string{"limit": strconv.Itoa(MaxRecommendationsPageSize)} + for pages := 0; ; pages++ { + require.Less(t, pages, total, "cursor walk did not terminate") + resp, err := h.getRecommendations(context.Background(), pageTestReq(), params) + require.NoError(t, err) + + inEnvelope, payload := lambdaPayloadLen(t, resp) + assert.LessOrEqual(t, inEnvelope, recommendationsPageByteBudget, "page %d over the byte budget", pages) + assert.Less(t, payload, lambdaResponseCapBytes, "page %d over the Lambda cap", pages) + require.NotEmpty(t, resp.Recommendations, "a page must never be empty mid-walk") + if resp.NextCursor != "" && len(resp.Recommendations) < MaxRecommendationsPageSize { + cutByBytes = true + } + + assert.Equal(t, total, resp.Summary.TotalCount) + assert.InDelta(t, float64(total*10), resp.Summary.TotalMonthlySavings, 1e-6) + assert.InDelta(t, float64(total*20), resp.Summary.TotalUpfrontCost, 1e-6) + + for i := range resp.Recommendations { + seen[resp.Recommendations[i].ID]++ + order = append(order, resp.Recommendations[i].ID) + } + if resp.NextCursor == "" { + break + } + params = map[string]string{"limit": strconv.Itoa(MaxRecommendationsPageSize), "cursor": resp.NextCursor} + } + + assert.True(t, cutByBytes, "expected at least one page cut by the byte budget before limit") + assert.Len(t, seen, total) + for id, n := range seen { + assert.Equal(t, 1, n, "row %s returned %d times", id, n) + } + assert.IsIncreasing(t, order) +} + +// TestBuildBudgetedPage_OversizedRowAlone: a row that alone exceeds the +// budget is returned on its own page so the walk advances, and the rows +// around it still pack normally. +func TestBuildBudgetedPage_OversizedRowAlone(t *testing.T) { + recs := pageTestRecs(6) // ~1 KB rows: rec-000000 .. rec-000005 + huge := bigDetailsRecs(1, 8000)[0] + recs[2].Details = huge.Details // rec-000003 (pageTestRecs is reversed) + const budget = 4000 // fits a few small rows, never the big one + + var pages [][]string + page := recommendationsPage{enabled: true, limit: MaxRecommendationsPageSize} + for range len(recs) + 1 { + resp, err := buildBudgetedPage(append([]config.RecommendationRecord(nil), recs...), page, budget) + require.NoError(t, err) + require.NotEmpty(t, resp.Recommendations) + ids := make([]string, 0, len(resp.Recommendations)) + for i := range resp.Recommendations { + ids = append(ids, resp.Recommendations[i].ID) + } + pages = append(pages, ids) + if len(ids) > 1 { + inEnvelope, _ := lambdaPayloadLen(t, resp) + assert.LessOrEqual(t, inEnvelope, budget, "multi-row page over budget: %v", ids) + } + if resp.NextCursor == "" { + break + } + cursor, err := decodeRecommendationsCursor(resp.NextCursor) + require.NoError(t, err) + page.afterID = cursor + } + + assert.Contains(t, pages, []string{"rec-000003"}, "oversized row must be alone on its page") + all := make([]string, 0, len(recs)) + for _, p := range pages { + all = append(all, p...) + } + assert.Equal(t, []string{"rec-000000", "rec-000001", "rec-000002", "rec-000003", "rec-000004", "rec-000005"}, all) + assert.Greater(t, len(pages[0]), 1, "small rows before the big one must still share a page") +} + +// TestBuildBudgetedPage_TightBudgetSweep sweeps the budget one byte at a time +// so the accounting is checked at its exact edges: every multi-row page must +// fit (counting the summary, regions, keys and next_cursor), and the page +// must be maximal, i.e. one more row would not have fit. +func TestBuildBudgetedPage_TightBudgetSweep(t *testing.T) { + recs := pageTestRecs(6) + page := recommendationsPage{enabled: true, limit: MaxRecommendationsPageSize} + for budget := 1500; budget <= 7000; budget++ { + resp, err := buildBudgetedPage(append([]config.RecommendationRecord(nil), recs...), page, budget) + require.NoError(t, err) + n := len(resp.Recommendations) + require.GreaterOrEqual(t, n, 1) + if n > 1 { + inEnvelope, _ := lambdaPayloadLen(t, resp) + require.LessOrEqual(t, inEnvelope, budget, "budget %d: %d rows do not fit", budget, n) + } + if n < len(recs) { + // The same page plus the next row, with that row's cursor. + bigger, err := buildBudgetedPage(append([]config.RecommendationRecord(nil), recs...), + recommendationsPage{enabled: true, limit: n + 1}, 1<<30) + require.NoError(t, err) + bigger.NextCursor = encodeRecommendationsCursor(bigger.Recommendations[n].ID) + inEnvelope, _ := lambdaPayloadLen(t, bigger) + // The paginator overcounts by at most 2 bytes (one comma and the + // one-byte cursor placeholder in the overhead), so anything it + // rejected must have been within 2 bytes of overflowing. + require.Greater(t, inEnvelope+2, budget, "budget %d: page of %d rows is not maximal", budget, n) + } + } +} diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index 2ba2afd0d..f3ce19c76 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -294,6 +294,30 @@ paths: schema: type: string - $ref: '#/components/parameters/AccountID' + - name: limit + in: query + description: >- + Opt-in maximum page size in rows (1-1000). When neither limit nor + cursor is present, every matching recommendation is returned in + one response (legacy behavior); a present but empty value is a + 400. When either is sent, rows are returned in ascending id order, + one page at a time, and a page can hold fewer than limit rows + (follow next_cursor, not the row count) because each page is also + capped at 4 MiB of serialized JSON. A single row larger than that + is returned alone. summary and regions still describe the full + filtered set. + schema: + type: integer + minimum: 1 + maximum: 1000 + - name: cursor + in: query + description: >- + Opaque next_cursor value from the previous page. Implies + pagination; the page size defaults to 500 when limit is omitted. + schema: + type: string + maxLength: 1024 responses: '200': description: Recommendations list @@ -301,6 +325,17 @@ paths: application/json: schema: $ref: '#/components/schemas/RecommendationsResponse' + '400': + description: > + Invalid query parameter: a limit that is empty, not an integer or + outside 1-1000; a cursor that is empty, longer than 1024 + characters or not a cursor this endpoint issued; or an invalid + provider, service, region, account_ids, min_savings_usd or + min_savings_pct filter. + content: + application/json: + schema: + $ref: '#/components/schemas/Error' '401': $ref: '#/components/responses/Unauthorized' '403': @@ -3432,11 +3467,33 @@ components: type: array items: $ref: '#/components/schemas/RecommendationRecord' - total_savings: - type: number - format: double - count: - type: integer + regions: + type: array + items: + type: string + summary: + type: object + description: Totals over the full filtered set, not just the returned page. + properties: + total_count: + type: integer + total_monthly_savings: + type: number + format: double + total_upfront_cost: + type: number + format: double + avg_payback_months: + type: number + format: double + next_cursor: + type: string + description: >- + Present only on a paginated request that has more rows; pass it as + the cursor query parameter to fetch the next page. Its absence is + the only end-of-list signal: a page with fewer rows than limit is + not necessarily the last one, because each page is also capped by + serialized size. RecommendationRecord: type: object diff --git a/internal/api/types.go b/internal/api/types.go index 9ff2a91f1..3c9a860cd 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -463,6 +463,9 @@ type RecommendationsResponse struct { Recommendations []config.RecommendationRecord `json:"recommendations"` Regions []string `json:"regions"` Summary RecommendationsSummary `json:"summary"` + // NextCursor is set only on a paginated request (limit or cursor sent) + // that has more rows; pass it back as ?cursor= for the next page. + NextCursor string `json:"next_cursor,omitempty"` } // UsagePoint is a single sample in the per-recommendation usage time