Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 64 additions & 17 deletions internal/api/handler_history.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"sort"
"strconv"
"strings"
"time"

"github.com/LeanerCloud/cloud-commitments-go/pkg/logging"
Expand Down Expand Up @@ -192,7 +193,7 @@ const approvalExpiryWindow = 7 * 24 * time.Hour
// the class reached its MaxListLimit cap, so older executions may have been
// dropped before the filters ran, or its listing failed (issue #686).
func (h *Handler) fetchExecutionsAsHistory(ctx context.Context, filters historyFilters) ([]config.PurchaseHistoryRecord, []config.PurchaseExecution, bool) {
executions, ok := h.fetchExecutionsByClass(ctx, actionableExecutionStatuses, config.MaxListLimit)
executions, ok := h.fetchExecutionsByClass(ctx, actionableExecutionStatuses, filters.PlanID, config.MaxListLimit)
incomplete := !ok || len(executions) >= config.MaxListLimit
if len(executions) == 0 {
return nil, nil, incomplete
Expand Down Expand Up @@ -222,8 +223,17 @@ func (h *Handler) fetchExecutionsAsHistory(ctx context.Context, filters historyF
// the class skipped (ok=false) so the other class and completed history still
// render; the caller flags the response as incomplete so the hidden rows are
// not silent (issue #686).
func (h *Handler) fetchExecutionsByClass(ctx context.Context, statuses []string, limit int) ([]config.PurchaseExecution, bool) {
rows, err := h.config.GetExecutionsByStatuses(ctx, statuses, limit)
//
// A non-empty planID selects only that plan's executions in SQL, so a plan view
// is not crowded out by other plans' rows under the limit.
func (h *Handler) fetchExecutionsByClass(ctx context.Context, statuses []string, planID string, limit int) ([]config.PurchaseExecution, bool) {
var rows []config.PurchaseExecution
var err error
if planID != "" {
rows, err = h.config.ListHistoryExecutionsPage(ctx, config.HistoryExecutionsPageQuery{Statuses: statuses, PlanID: planID, Limit: limit})
} else {
rows, err = h.config.GetExecutionsByStatuses(ctx, statuses, limit)
}
if err != nil {
logging.Warnf("history: failed to load executions with statuses %v: %v", statuses, err)
return nil, false
Expand Down Expand Up @@ -721,9 +731,12 @@ type historyFilters struct {
ExternalIDsByProvider map[string][]string
Provider string
LegacyAccountID string
AccountIDs []string
Limit int
HasDate bool
// PlanID narrows the history to one purchase plan's purchases and
// executions (the plan-history view's plan_id); empty means no plan filter.
PlanID string
AccountIDs []string
Limit int
HasDate bool
// Cursor is the stream position the page starts after (issue #674); nil
// for the first page.
Cursor *config.HistoryPageKey
Expand Down Expand Up @@ -752,6 +765,12 @@ func parseHistoryFilters(params map[string]string) (historyFilters, error) {
f.AccountIDs = accountIDs
f.LegacyAccountID = params["account_id"]

planID, err := parseHistoryPlanID(params["plan_id"])
if err != nil {
return f, err
}
f.PlanID = planID

start, end, hasDate, err := parseHistoryDateRange(params["start"], params["end"])
if err != nil {
return f, err
Expand All @@ -760,11 +779,29 @@ func parseHistoryFilters(params map[string]string) (historyFilters, error) {
f.Start = start
f.End = end

limit, err := parseHistoryLimit(params["limit"])
if err != nil {
return f, err
}
f.Limit = limit

cursor, pagingErr := parseHistoryPaging(params)
if pagingErr != nil {
return f, pagingErr
}
f.Cursor = cursor

return f, nil
}

// parseHistoryLimit parses the optional limit, clamped to [1, MaxListLimit]
// with DefaultListLimit for empty or non-positive values.
func parseHistoryLimit(raw string) (int, error) {
limit := config.DefaultListLimit
if s := params["limit"]; s != "" {
n, err := strconv.Atoi(s)
if raw != "" {
n, err := strconv.Atoi(raw)
if err != nil {
return f, NewClientError(400, "invalid limit: must be a positive integer")
return 0, NewClientError(400, "invalid limit: must be a positive integer")
}
limit = n
}
Expand All @@ -774,15 +811,21 @@ func parseHistoryFilters(params map[string]string) (historyFilters, error) {
if limit > config.MaxListLimit {
limit = config.MaxListLimit
}
f.Limit = limit

cursor, pagingErr := parseHistoryPaging(params)
if pagingErr != nil {
return f, pagingErr
}
f.Cursor = cursor
return limit, nil
}

return f, nil
// parseHistoryPlanID validates the optional plan_id filter; empty means none.
// The value is trimmed and lowercased so the SQL predicate and the in-memory
// execution check compare the same canonical text.
func parseHistoryPlanID(raw string) (string, error) {
id := strings.TrimSpace(raw)
if id == "" {
return "", nil
}
if err := validateUUID(id); err != nil {
return "", NewClientError(400, "invalid plan_id: must be a UUID")
}
return strings.ToLower(id), nil
}

// parseHistoryDateRange parses optional YYYY-MM-DD start/end strings. Both
Expand Down Expand Up @@ -878,6 +921,9 @@ func (f historyFilters) matchesExecution(exec config.PurchaseExecution) bool {
if !f.matchesExecutionProviderAndAccount(exec) {
return false
}
if f.PlanID != "" && exec.PlanID != f.PlanID {
return false
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if f.HasDate {
if exec.ScheduledDate.Before(f.Start) || exec.ScheduledDate.After(f.End) {
return false
Expand Down Expand Up @@ -1016,6 +1062,7 @@ func (h *Handler) fetchPurchaseHistory(ctx context.Context, filters *historyFilt
func (f historyFilters) purchaseHistoryFilter() config.PurchaseHistoryFilter {
out := config.PurchaseHistoryFilter{
Provider: f.Provider,
PlanID: f.PlanID,
AccountIDs: f.AccountIDs,
ExternalIDsByProvider: f.ExternalIDsByProvider,
Limit: f.Limit,
Expand Down
6 changes: 3 additions & 3 deletions internal/api/handler_history_page.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,10 @@ func (st *historyStream) fetchPurchases(ctx context.Context, after *config.Histo

// fetchTerminalExecutions reads a batch of terminal executions and projects
// the ones matching the request's filters into History rows. The date bound
// is pushed into SQL as well as checked by matchesExecution; provider and
// account are checked in Go only, like the actionable class.
// is pushed into SQL as well as checked by matchesExecution; the plan is pushed into
// SQL too; provider and account are checked in Go only, like the actionable class.
func (st *historyStream) fetchTerminalExecutions(ctx context.Context, after *config.HistoryPageKey) ([]historyStreamRow, error) {
q := config.HistoryExecutionsPageQuery{Statuses: terminalExecutionStatuses, After: after, Limit: st.executions.batch}
q := config.HistoryExecutionsPageQuery{Statuses: terminalExecutionStatuses, PlanID: st.filters.PlanID, After: after, Limit: st.executions.batch}
if st.filters.HasDate {
start, end := st.filters.Start, st.filters.End
q.Start, q.End = &start, &end
Expand Down
176 changes: 176 additions & 0 deletions internal/api/handler_history_plan_filter_integration_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
//go:build integration

package api

import (
"context"
"encoding/json"
"strings"
"testing"
"time"

"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
"github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
)

type planFilterEnv struct {
*historySummaryEnv
container *testhelpers.PostgresContainer
planA, planB string
admin, scoped *Handler
}

func (e *planFilterEnv) createPlan(ctx context.Context, t *testing.T, name string) string {
t.Helper()
next := time.Now().Add(time.Hour).Truncate(time.Microsecond)
plan := &config.PurchasePlan{
Name: name, Enabled: true,
Services: map[string]config.ServiceConfig{"ec2": {Provider: "aws", Service: "ec2", Enabled: true}},
RampSchedule: config.RampSchedule{Type: "weekly", PercentPerStep: 25, StepIntervalDays: 7, TotalSteps: 4, StartDate: time.Now().AddDate(0, 0, -7)},
NextExecutionDate: &next,
}
require.NoError(t, plan.Validate())
require.NoError(t, e.store.CreatePurchasePlan(ctx, plan))
return plan.ID
}

func (e *planFilterEnv) addPlanPurchase(ctx context.Context, t *testing.T, tenant, planID string, at time.Time) {
t.Helper()
acct := e.account(tenant)
up := 10.0
require.NoError(t, e.store.SavePurchaseHistory(ctx, &config.PurchaseHistoryRecord{
AccountID: acct.ExternalID, PurchaseID: uuid.NewString(), Timestamp: at, Provider: "aws", Service: "ec2",
Region: "us-east-1", ResourceType: "t3.micro", Count: 1, Term: 1, Payment: "no-upfront",
UpfrontCost: &up, EstimatedSavings: 1, PlanID: planID, CloudAccountID: &acct.ID,
}))
}

func (e *planFilterEnv) addPlanExecution(ctx context.Context, t *testing.T, tenant, planID, status, execErr string, at time.Time) {
t.Helper()
exec := &config.PurchaseExecution{
PlanID: planID, ExecutionID: uuid.NewString(), IdempotencyKey: uuid.NewString(), Status: status, Error: execErr,
ScheduledDate: at, TotalUpfrontCost: 500, EstimatedSavings: 50,
}
if tenant != "" {
id := e.account(tenant).ID
exec.CloudAccountID = &id
exec.Recommendations = []config.RecommendationRecord{{Provider: "aws", Service: "ec2", Region: "us-east-1", CloudAccountID: &id}}
}
require.NoError(t, e.store.SavePurchaseExecution(ctx, exec))
}

// newPlanFilterEnv seeds plan A (tenant A) and plan B (tenant B), each with two
// purchases, a failed and a pending execution, plus tenant A rows tied to no
// plan. The pending rows are recent, so the stale-expiry sweep leaves them.
func newPlanFilterEnv(ctx context.Context, t *testing.T) *planFilterEnv {
t.Helper()
base, container := setupHistorySummaryEnv(ctx, t)
e := &planFilterEnv{historySummaryEnv: base, container: container}
e.planA, e.planB = e.createPlan(ctx, t, "plan-a"), e.createPlan(ctx, t, "plan-b")
at := time.Now().UTC().Add(-3 * 24 * time.Hour).Truncate(time.Second)
for i, p := range []struct{ tenant, plan string }{{"a", e.planA}, {"b", e.planB}, {"a", ""}} {
o := time.Duration(i) * time.Minute
e.addPlanPurchase(ctx, t, p.tenant, p.plan, at.Add(o))
e.addPlanPurchase(ctx, t, p.tenant, p.plan, at.Add(o+time.Hour))
e.addPlanExecution(ctx, t, p.tenant, p.plan, "failed", "boom", at.Add(o+2*time.Hour))
e.addPlanExecution(ctx, t, p.tenant, p.plan, "pending", "", at.Add(o+3*time.Hour))
}
scopedAuth := new(MockAuthService)
scopedAuth.On("ValidateSession", mock.Anything, scopedToken).Return(&Session{UserID: scopedUserID}, nil).Maybe()
scopedAuth.On("GetUser", mock.Anything, mock.Anything).Return(&User{Email: "creator@example.com"}, nil).Maybe()
scopedAuth.grantScoped(e.tenantA.Name)
adminAuth := new(MockAuthService)
adminAuth.On("GetUser", mock.Anything, mock.Anything).Return(&User{Email: "creator@example.com"}, nil).Maybe()
adminAuth.On("ValidateSession", mock.Anything, "admin-token").Return(&Session{UserID: "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"}, nil).Maybe()
adminAuth.grantAdmin()
e.admin = &Handler{config: e.store, auth: adminAuth}
e.scoped = &Handler{config: e.store, auth: scopedAuth}
return e
}

func historyBody(ctx context.Context, t *testing.T, h *Handler, token string, params map[string]string) (HistoryResponse, string) {
t.Helper()
resp, _ := getHistoryJSON(ctx, t, h, token, params)
raw, err := json.Marshal(resp)
require.NoError(t, err)
return resp, string(raw)
}

func assertAllPlan(t *testing.T, resp HistoryResponse, plan string) {
t.Helper()
for i := range resp.Purchases {
assert.Equal(t, plan, resp.Purchases[i].PlanID, "row %s", resp.Purchases[i].PurchaseID)
}
}

// TestIntegration_HistoryPlanFilter_Canonicalised: an uppercase or padded
// plan_id selects the same purchases and executions as the lowercase one, so
// the SQL predicate and the in-memory execution check agree.
func TestIntegration_HistoryPlanFilter_Canonicalised(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)
defer cancel()
e := newPlanFilterEnv(ctx, t)

resp, want := historyBody(ctx, t, e.admin, "admin-token", map[string]string{"plan_id": e.planA})
require.Len(t, resp.Purchases, 4, "2 purchases + failed + pending execution of plan A")
assertAllPlan(t, resp, e.planA)
assert.Equal(t, 4, resp.Summary.TotalPurchases)
for _, variant := range []string{strings.ToUpper(e.planA), " " + e.planA + " "} {
_, got := historyBody(ctx, t, e.admin, "admin-token", map[string]string{"plan_id": variant})
assert.Equal(t, want, got, variant)
}
all, _ := historyBody(ctx, t, e.admin, "admin-token", map[string]string{})
assert.Len(t, all.Purchases, 12, "no plan filter returns every plan and planless rows")
}

// TestIntegration_HistoryPlanFilter_CrossTenant: a user scoped to tenant A who
// passes tenant B's plan_id gets exactly what a random plan_id gets, and
// tenant A's own plan returns only tenant A's rows.
func TestIntegration_HistoryPlanFilter_CrossTenant(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)
defer cancel()
e := newPlanFilterEnv(ctx, t)

adminB, _ := historyBody(ctx, t, e.admin, "admin-token", map[string]string{"plan_id": e.planB})
require.Len(t, adminB.Purchases, 4, "tenant B's plan is not empty for an unrestricted viewer")

foreign, foreignBody := historyBody(ctx, t, e.scoped, scopedToken, map[string]string{"plan_id": e.planB})
_, randomBody := historyBody(ctx, t, e.scoped, scopedToken, map[string]string{"plan_id": uuid.NewString()})
assert.Empty(t, foreign.Purchases)
assertSummaryEqual(t, HistorySummary{TotalUpfront: new(float64)}, foreign.Summary)
assert.Equal(t, randomBody, foreignBody, "another tenant's plan must be indistinguishable from a missing one")

own, _ := historyBody(ctx, t, e.scoped, scopedToken, map[string]string{"plan_id": e.planA})
require.Len(t, own.Purchases, 4)
assertAllPlan(t, own, e.planA)
assert.Equal(t, 4, own.Summary.TotalPurchases)
}

// TestIntegration_HistoryPlanFilter_ActionableOverCap: more than MaxListLimit
// other-plan actionable executions, all newer, must not push the plan's own
// pending execution out of the listing.
func TestIntegration_HistoryPlanFilter_ActionableOverCap(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)
defer cancel()
e := newPlanFilterEnv(ctx, t)
_, err := e.container.DB.Pool().Exec(ctx, `
INSERT INTO purchase_executions (plan_id, status, scheduled_date, idempotency_key)
SELECT $1, 'approved', now() - interval '1 hour' - g * interval '1 second', gen_random_uuid()::text
FROM generate_series(1, $2) g`, e.planB, config.MaxListLimit+5)
require.NoError(t, err)

resp, _ := historyBody(ctx, t, e.admin, "admin-token", map[string]string{"plan_id": e.planA})
require.Len(t, resp.Purchases, 4)
assertAllPlan(t, resp, e.planA)
pending := 0
for i := range resp.Purchases {
if resp.Purchases[i].Status == "pending" {
pending++
}
}
assert.Equal(t, 1, pending, "the plan's pending execution survives the other plan's rows")
}
Loading
Loading