From e42e749ea6ace6a8dfc447cd5d02313d255b829e Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Tue, 20 Jan 2026 00:19:28 +0700 Subject: [PATCH 01/14] refactor: add shared pagination helpers for Hookdeck API alignment Co-Authored-By: Claude Opus 4.5 --- internal/apirouter/pagination.go | 128 ++++++++++++++ internal/apirouter/pagination_test.go | 245 ++++++++++++++++++++++++++ 2 files changed, 373 insertions(+) create mode 100644 internal/apirouter/pagination.go create mode 100644 internal/apirouter/pagination_test.go diff --git a/internal/apirouter/pagination.go b/internal/apirouter/pagination.go new file mode 100644 index 000000000..4b510824f --- /dev/null +++ b/internal/apirouter/pagination.go @@ -0,0 +1,128 @@ +package apirouter + +import ( + "fmt" + "net/http" + "slices" + "time" + + "github.com/gin-gonic/gin" +) + +// PaginationInfo represents pagination metadata for list responses. +// This aligns with Hookdeck's pagination response format. +type PaginationInfo struct { + OrderBy string `json:"order_by"` + Dir string `json:"dir"` + Limit int `json:"limit"` + Next *string `json:"next"` + Prev *string `json:"prev"` +} + +// CursorToPtr converts an empty cursor string to nil, or returns a pointer to the string. +// Hookdeck returns null for empty cursors instead of empty string. +func CursorToPtr(cursor string) *string { + if cursor == "" { + return nil + } + return &cursor +} + +// ParseDir parses the "dir" query parameter for sort direction. +// Returns the direction (defaulting to "desc") and any validation error response. +// Valid values are "asc" and "desc". +func ParseDir(c *gin.Context) (string, *ErrorResponse) { + dir := c.Query("dir") + if dir == "" { + return "desc", nil + } + if dir != "asc" && dir != "desc" { + return "", &ErrorResponse{ + Code: http.StatusUnprocessableEntity, + Message: "validation error", + Data: map[string]string{ + "query.dir": "must be 'asc' or 'desc'", + }, + } + } + return dir, nil +} + +// ParseOrderBy parses the "order_by" query parameter and validates against allowed values. +// Returns the order_by value (defaulting to defaultValue) and any validation error response. +func ParseOrderBy(c *gin.Context, allowedValues []string, defaultValue string) (string, *ErrorResponse) { + orderBy := c.Query("order_by") + if orderBy == "" { + return defaultValue, nil + } + + if slices.Contains(allowedValues, orderBy) { + return orderBy, nil + } + + return "", &ErrorResponse{ + Code: http.StatusUnprocessableEntity, + Message: "validation error", + Data: map[string]string{ + "query.order_by": fmt.Sprintf("must be one of: %v", allowedValues), + }, + } +} + +// DateFilterResult holds the parsed date filter values. +type DateFilterResult struct { + GTE *time.Time + LTE *time.Time +} + +// ParseDateFilter parses bracket notation date filters (e.g., field[gte], field[lte]). +// Returns the parsed dates and any validation error response. +// Accepts both RFC3339 format (2024-01-01T00:00:00Z) and date-only format (2024-01-01). +func ParseDateFilter(c *gin.Context, fieldName string) (*DateFilterResult, *ErrorResponse) { + result := &DateFilterResult{} + + gteKey := fieldName + "[gte]" + if gteStr := c.Query(gteKey); gteStr != "" { + t, err := parseDateTime(gteStr) + if err != nil { + return nil, &ErrorResponse{ + Code: http.StatusUnprocessableEntity, + Message: "validation error", + Data: map[string]string{ + "query." + gteKey: "invalid format, expected RFC3339 or YYYY-MM-DD", + }, + } + } + result.GTE = &t + } + + lteKey := fieldName + "[lte]" + if lteStr := c.Query(lteKey); lteStr != "" { + t, err := parseDateTime(lteStr) + if err != nil { + return nil, &ErrorResponse{ + Code: http.StatusUnprocessableEntity, + Message: "validation error", + Data: map[string]string{ + "query." + lteKey: "invalid format, expected RFC3339 or YYYY-MM-DD", + }, + } + } + result.LTE = &t + } + + return result, nil +} + +// parseDateTime attempts to parse a date string in RFC3339 or date-only (YYYY-MM-DD) format. +func parseDateTime(s string) (time.Time, error) { + // Try RFC3339 first + if t, err := time.Parse(time.RFC3339, s); err == nil { + return t, nil + } + // Try date-only format + if t, err := time.Parse("2006-01-02", s); err == nil { + return t, nil + } + return time.Time{}, fmt.Errorf("invalid date format") +} diff --git a/internal/apirouter/pagination_test.go b/internal/apirouter/pagination_test.go new file mode 100644 index 000000000..5a0cffb80 --- /dev/null +++ b/internal/apirouter/pagination_test.go @@ -0,0 +1,245 @@ +package apirouter_test + +import ( + "net/http" + "net/http/httptest" + "net/url" + "testing" + "time" + + "github.com/gin-gonic/gin" + "github.com/hookdeck/outpost/internal/apirouter" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func init() { + gin.SetMode(gin.TestMode) +} + +func TestCursorToPtr(t *testing.T) { + t.Run("empty string returns nil", func(t *testing.T) { + result := apirouter.CursorToPtr("") + assert.Nil(t, result) + }) + + t.Run("non-empty string returns pointer", func(t *testing.T) { + result := apirouter.CursorToPtr("abc123") + require.NotNil(t, result) + assert.Equal(t, "abc123", *result) + }) +} + +func TestParseDir(t *testing.T) { + tests := []struct { + name string + queryDir string + wantDir string + wantErrCode int + }{ + { + name: "empty defaults to desc", + queryDir: "", + wantDir: "desc", + }, + { + name: "asc is valid", + queryDir: "asc", + wantDir: "asc", + }, + { + name: "desc is valid", + queryDir: "desc", + wantDir: "desc", + }, + { + name: "invalid value returns error", + queryDir: "invalid", + wantErrCode: http.StatusUnprocessableEntity, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c, _ := createTestContext(map[string]string{"dir": tt.queryDir}) + + dir, errResp := apirouter.ParseDir(c) + + if tt.wantErrCode != 0 { + require.NotNil(t, errResp) + assert.Equal(t, tt.wantErrCode, errResp.Code) + } else { + assert.Nil(t, errResp) + assert.Equal(t, tt.wantDir, dir) + } + }) + } +} + +func TestParseOrderBy(t *testing.T) { + allowedValues := []string{"created_at", "event_time"} + + tests := []struct { + name string + queryOrderBy string + defaultValue string + wantOrderBy string + wantErrCode int + }{ + { + name: "empty returns default", + queryOrderBy: "", + defaultValue: "created_at", + wantOrderBy: "created_at", + }, + { + name: "valid value returns value", + queryOrderBy: "event_time", + defaultValue: "created_at", + wantOrderBy: "event_time", + }, + { + name: "invalid value returns error", + queryOrderBy: "invalid_field", + defaultValue: "created_at", + wantErrCode: http.StatusUnprocessableEntity, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c, _ := createTestContext(map[string]string{"order_by": tt.queryOrderBy}) + + orderBy, errResp := apirouter.ParseOrderBy(c, allowedValues, tt.defaultValue) + + if tt.wantErrCode != 0 { + require.NotNil(t, errResp) + assert.Equal(t, tt.wantErrCode, errResp.Code) + } else { + assert.Nil(t, errResp) + assert.Equal(t, tt.wantOrderBy, orderBy) + } + }) + } +} + +func TestParseDateFilter(t *testing.T) { + tests := []struct { + name string + fieldName string + queryParams map[string]string + wantGTE *time.Time + wantLTE *time.Time + wantErrCode int + }{ + { + name: "no params returns empty result", + fieldName: "event_time", + queryParams: map[string]string{}, + wantGTE: nil, + wantLTE: nil, + }, + { + name: "RFC3339 gte param", + fieldName: "event_time", + queryParams: map[string]string{ + "event_time[gte]": "2024-01-15T10:30:00Z", + }, + wantGTE: timePtr(time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC)), + wantLTE: nil, + }, + { + name: "RFC3339 lte param", + fieldName: "event_time", + queryParams: map[string]string{ + "event_time[lte]": "2024-01-31T23:59:59Z", + }, + wantGTE: nil, + wantLTE: timePtr(time.Date(2024, 1, 31, 23, 59, 59, 0, time.UTC)), + }, + { + name: "both gte and lte params", + fieldName: "created_at", + queryParams: map[string]string{ + "created_at[gte]": "2024-01-01T00:00:00Z", + "created_at[lte]": "2024-01-31T23:59:59Z", + }, + wantGTE: timePtr(time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)), + wantLTE: timePtr(time.Date(2024, 1, 31, 23, 59, 59, 0, time.UTC)), + }, + { + name: "date-only format", + fieldName: "delivery_time", + queryParams: map[string]string{ + "delivery_time[gte]": "2024-01-15", + }, + wantGTE: timePtr(time.Date(2024, 1, 15, 0, 0, 0, 0, time.UTC)), + wantLTE: nil, + }, + { + name: "invalid gte format returns error", + fieldName: "event_time", + queryParams: map[string]string{ + "event_time[gte]": "not-a-date", + }, + wantErrCode: http.StatusUnprocessableEntity, + }, + { + name: "invalid lte format returns error", + fieldName: "event_time", + queryParams: map[string]string{ + "event_time[lte]": "invalid", + }, + wantErrCode: http.StatusUnprocessableEntity, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c, _ := createTestContext(tt.queryParams) + + result, errResp := apirouter.ParseDateFilter(c, tt.fieldName) + + if tt.wantErrCode != 0 { + require.NotNil(t, errResp) + assert.Equal(t, tt.wantErrCode, errResp.Code) + } else { + require.Nil(t, errResp) + require.NotNil(t, result) + + if tt.wantGTE != nil { + require.NotNil(t, result.GTE) + assert.True(t, tt.wantGTE.Equal(*result.GTE), "GTE mismatch: want %v, got %v", tt.wantGTE, result.GTE) + } else { + assert.Nil(t, result.GTE) + } + + if tt.wantLTE != nil { + require.NotNil(t, result.LTE) + assert.True(t, tt.wantLTE.Equal(*result.LTE), "LTE mismatch: want %v, got %v", tt.wantLTE, result.LTE) + } else { + assert.Nil(t, result.LTE) + } + } + }) + } +} + +func createTestContext(queryParams map[string]string) (*gin.Context, *httptest.ResponseRecorder) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + + values := url.Values{} + for k, v := range queryParams { + if v != "" { + values.Set(k, v) + } + } + + c.Request = httptest.NewRequest(http.MethodGet, "/?"+values.Encode(), nil) + return c, w +} + +func timePtr(t time.Time) *time.Time { + return &t +} From 3b086b504768bb70511e7c04a30b88bb5c137edd Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Tue, 20 Jan 2026 00:32:51 +0700 Subject: [PATCH 02/14] refactor: extend pagination helpers with full Hookdeck query operators - Add gt, lt operators to date filter (in addition to gte, lte) - Add any operator for date, string, and number filters - Add contains operator for string filters - Add number filter with all comparison operators - Add array param parser for bracket notation (item[0]=a&item[1]=b) - Remove defaults from ParseDir and ParseOrderBy (store handles defaults) Co-Authored-By: Claude Opus 4.5 --- internal/apirouter/pagination.go | 249 +++++++++++++++-- internal/apirouter/pagination_test.go | 372 +++++++++++++++++++++++--- 2 files changed, 567 insertions(+), 54 deletions(-) diff --git a/internal/apirouter/pagination.go b/internal/apirouter/pagination.go index 4b510824f..69b5825dc 100644 --- a/internal/apirouter/pagination.go +++ b/internal/apirouter/pagination.go @@ -4,6 +4,9 @@ import ( "fmt" "net/http" "slices" + "sort" + "strconv" + "strings" "time" "github.com/gin-gonic/gin" @@ -29,12 +32,12 @@ func CursorToPtr(cursor string) *string { } // ParseDir parses the "dir" query parameter for sort direction. -// Returns the direction (defaulting to "desc") and any validation error response. -// Valid values are "asc" and "desc". +// Returns the direction (empty string if not provided) and any validation error response. +// Valid values are "asc" and "desc". Caller/store should apply default if empty. func ParseDir(c *gin.Context) (string, *ErrorResponse) { dir := c.Query("dir") if dir == "" { - return "desc", nil + return "", nil } if dir != "asc" && dir != "desc" { return "", &ErrorResponse{ @@ -49,11 +52,12 @@ func ParseDir(c *gin.Context) (string, *ErrorResponse) { } // ParseOrderBy parses the "order_by" query parameter and validates against allowed values. -// Returns the order_by value (defaulting to defaultValue) and any validation error response. -func ParseOrderBy(c *gin.Context, allowedValues []string, defaultValue string) (string, *ErrorResponse) { +// Returns the order_by value (empty string if not provided) and any validation error response. +// Caller/store should apply default if empty. +func ParseOrderBy(c *gin.Context, allowedValues []string) (string, *ErrorResponse) { orderBy := c.Query("order_by") if orderBy == "" { - return defaultValue, nil + return "", nil } if slices.Contains(allowedValues, orderBy) { @@ -69,51 +73,250 @@ func ParseOrderBy(c *gin.Context, allowedValues []string, defaultValue string) ( } } +// ParseArrayParam parses bracket notation array parameters (e.g., field[0]=a&field[1]=b). +// Returns the parsed array in index order. Supports both numeric indices [0], [1] and +// simple repeated params field[]=a&field[]=b. +func ParseArrayParam(c *gin.Context, fieldName string) []string { + queryMap := c.Request.URL.Query() + prefix := fieldName + "[" + + // Collect all matching params + type indexedValue struct { + index int + value string + } + var values []indexedValue + var unindexed []string + + for key, vals := range queryMap { + if !strings.HasPrefix(key, prefix) { + continue + } + if len(vals) == 0 { + continue + } + + // Extract the part between [ and ] + suffix := key[len(prefix):] + if !strings.HasSuffix(suffix, "]") { + continue + } + indexStr := suffix[:len(suffix)-1] + + if indexStr == "" { + // field[]=value format - collect all values + unindexed = append(unindexed, vals...) + } else { + // field[0]=value format - parse index + idx, err := strconv.Atoi(indexStr) + if err != nil { + continue // skip non-numeric indices + } + values = append(values, indexedValue{index: idx, value: vals[0]}) + } + } + + // If we have indexed values, sort by index and return + if len(values) > 0 { + sort.Slice(values, func(i, j int) bool { + return values[i].index < values[j].index + }) + result := make([]string, len(values)) + for i, v := range values { + result[i] = v.value + } + return result + } + + // Return unindexed values if any + return unindexed +} + // DateFilterResult holds the parsed date filter values. +// Supports all Hookdeck query operators for dates: gte, gt, lte, lt, any. type DateFilterResult struct { - GTE *time.Time - LTE *time.Time + GTE *time.Time // Greater than or equal + GT *time.Time // Greater than + LTE *time.Time // Less than or equal + LT *time.Time // Less than + Any bool // Field is not null (any value present) } -// ParseDateFilter parses bracket notation date filters (e.g., field[gte], field[lte]). +// ParseDateFilter parses bracket notation date filters (e.g., field[gte], field[lte], field[gt], field[lt], field[any]). // Returns the parsed dates and any validation error response. // Accepts both RFC3339 format (2024-01-01T00:00:00Z) and date-only format (2024-01-01). func ParseDateFilter(c *gin.Context, fieldName string) (*DateFilterResult, *ErrorResponse) { result := &DateFilterResult{} + // Parse [gte] - greater than or equal gteKey := fieldName + "[gte]" if gteStr := c.Query(gteKey); gteStr != "" { t, err := parseDateTime(gteStr) if err != nil { - return nil, &ErrorResponse{ - Code: http.StatusUnprocessableEntity, - Message: "validation error", - Data: map[string]string{ - "query." + gteKey: "invalid format, expected RFC3339 or YYYY-MM-DD", - }, - } + return nil, dateFormatError(gteKey) } result.GTE = &t } + // Parse [gt] - greater than + gtKey := fieldName + "[gt]" + if gtStr := c.Query(gtKey); gtStr != "" { + t, err := parseDateTime(gtStr) + if err != nil { + return nil, dateFormatError(gtKey) + } + result.GT = &t + } + + // Parse [lte] - less than or equal lteKey := fieldName + "[lte]" if lteStr := c.Query(lteKey); lteStr != "" { t, err := parseDateTime(lteStr) if err != nil { - return nil, &ErrorResponse{ - Code: http.StatusUnprocessableEntity, - Message: "validation error", - Data: map[string]string{ - "query." + lteKey: "invalid format, expected RFC3339 or YYYY-MM-DD", - }, - } + return nil, dateFormatError(lteKey) } result.LTE = &t } + // Parse [lt] - less than + ltKey := fieldName + "[lt]" + if ltStr := c.Query(ltKey); ltStr != "" { + t, err := parseDateTime(ltStr) + if err != nil { + return nil, dateFormatError(ltKey) + } + result.LT = &t + } + + // Parse [any] - field is not null + anyKey := fieldName + "[any]" + if anyStr := c.Query(anyKey); anyStr != "" { + result.Any = anyStr == "true" || anyStr == "1" + } + + return result, nil +} + +// dateFormatError returns a validation error for invalid date format. +func dateFormatError(key string) *ErrorResponse { + return &ErrorResponse{ + Code: http.StatusUnprocessableEntity, + Message: "validation error", + Data: map[string]string{ + "query." + key: "invalid format, expected RFC3339 or YYYY-MM-DD", + }, + } +} + +// StringFilterResult holds the parsed string filter values. +// Supports Hookdeck query operators for strings: contains, any. +type StringFilterResult struct { + Contains *string // Contains substring + Any bool // Field is not null (any value present) +} + +// ParseStringFilter parses bracket notation string filters (e.g., field[contains], field[any]). +// Returns the parsed filter values and any validation error response. +func ParseStringFilter(c *gin.Context, fieldName string) *StringFilterResult { + result := &StringFilterResult{} + + // Parse [contains] - contains substring + containsKey := fieldName + "[contains]" + if containsStr := c.Query(containsKey); containsStr != "" { + result.Contains = &containsStr + } + + // Parse [any] - field is not null + anyKey := fieldName + "[any]" + if anyStr := c.Query(anyKey); anyStr != "" { + result.Any = anyStr == "true" || anyStr == "1" + } + + return result +} + +// NumberFilterResult holds the parsed number filter values. +// Supports Hookdeck query operators for numbers: gte, gt, lte, lt, any. +type NumberFilterResult struct { + GTE *float64 // Greater than or equal + GT *float64 // Greater than + LTE *float64 // Less than or equal + LT *float64 // Less than + Any bool // Field is not null (any value present) +} + +// ParseNumberFilter parses bracket notation number filters (e.g., field[gte], field[lte]). +// Returns the parsed numbers and any validation error response. +func ParseNumberFilter(c *gin.Context, fieldName string) (*NumberFilterResult, *ErrorResponse) { + result := &NumberFilterResult{} + + // Parse [gte] - greater than or equal + gteKey := fieldName + "[gte]" + if gteStr := c.Query(gteKey); gteStr != "" { + n, err := parseNumber(gteStr) + if err != nil { + return nil, numberFormatError(gteKey) + } + result.GTE = &n + } + + // Parse [gt] - greater than + gtKey := fieldName + "[gt]" + if gtStr := c.Query(gtKey); gtStr != "" { + n, err := parseNumber(gtStr) + if err != nil { + return nil, numberFormatError(gtKey) + } + result.GT = &n + } + + // Parse [lte] - less than or equal + lteKey := fieldName + "[lte]" + if lteStr := c.Query(lteKey); lteStr != "" { + n, err := parseNumber(lteStr) + if err != nil { + return nil, numberFormatError(lteKey) + } + result.LTE = &n + } + + // Parse [lt] - less than + ltKey := fieldName + "[lt]" + if ltStr := c.Query(ltKey); ltStr != "" { + n, err := parseNumber(ltStr) + if err != nil { + return nil, numberFormatError(ltKey) + } + result.LT = &n + } + + // Parse [any] - field is not null + anyKey := fieldName + "[any]" + if anyStr := c.Query(anyKey); anyStr != "" { + result.Any = anyStr == "true" || anyStr == "1" + } + return result, nil } +// numberFormatError returns a validation error for invalid number format. +func numberFormatError(key string) *ErrorResponse { + return &ErrorResponse{ + Code: http.StatusUnprocessableEntity, + Message: "validation error", + Data: map[string]string{ + "query." + key: "invalid format, expected a number", + }, + } +} + +// parseNumber attempts to parse a number string. +func parseNumber(s string) (float64, error) { + var n float64 + _, err := fmt.Sscanf(s, "%f", &n) + return n, err +} + // parseDateTime attempts to parse a date string in RFC3339 or date-only (YYYY-MM-DD) format. func parseDateTime(s string) (time.Time, error) { // Try RFC3339 first diff --git a/internal/apirouter/pagination_test.go b/internal/apirouter/pagination_test.go index 5a0cffb80..7e305ed15 100644 --- a/internal/apirouter/pagination_test.go +++ b/internal/apirouter/pagination_test.go @@ -38,9 +38,9 @@ func TestParseDir(t *testing.T) { wantErrCode int }{ { - name: "empty defaults to desc", + name: "empty returns empty (no default)", queryDir: "", - wantDir: "desc", + wantDir: "", }, { name: "asc is valid", @@ -82,26 +82,27 @@ func TestParseOrderBy(t *testing.T) { tests := []struct { name string queryOrderBy string - defaultValue string wantOrderBy string wantErrCode int }{ { - name: "empty returns default", + name: "empty returns empty (no default)", queryOrderBy: "", - defaultValue: "created_at", - wantOrderBy: "created_at", + wantOrderBy: "", }, { name: "valid value returns value", queryOrderBy: "event_time", - defaultValue: "created_at", wantOrderBy: "event_time", }, + { + name: "another valid value", + queryOrderBy: "created_at", + wantOrderBy: "created_at", + }, { name: "invalid value returns error", queryOrderBy: "invalid_field", - defaultValue: "created_at", wantErrCode: http.StatusUnprocessableEntity, }, } @@ -110,7 +111,7 @@ func TestParseOrderBy(t *testing.T) { t.Run(tt.name, func(t *testing.T) { c, _ := createTestContext(map[string]string{"order_by": tt.queryOrderBy}) - orderBy, errResp := apirouter.ParseOrderBy(c, allowedValues, tt.defaultValue) + orderBy, errResp := apirouter.ParseOrderBy(c, allowedValues) if tt.wantErrCode != 0 { require.NotNil(t, errResp) @@ -129,15 +130,16 @@ func TestParseDateFilter(t *testing.T) { fieldName string queryParams map[string]string wantGTE *time.Time + wantGT *time.Time wantLTE *time.Time + wantLT *time.Time + wantAny bool wantErrCode int }{ { name: "no params returns empty result", fieldName: "event_time", queryParams: map[string]string{}, - wantGTE: nil, - wantLTE: nil, }, { name: "RFC3339 gte param", @@ -146,7 +148,14 @@ func TestParseDateFilter(t *testing.T) { "event_time[gte]": "2024-01-15T10:30:00Z", }, wantGTE: timePtr(time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC)), - wantLTE: nil, + }, + { + name: "RFC3339 gt param", + fieldName: "event_time", + queryParams: map[string]string{ + "event_time[gt]": "2024-01-15T10:30:00Z", + }, + wantGT: timePtr(time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC)), }, { name: "RFC3339 lte param", @@ -154,11 +163,18 @@ func TestParseDateFilter(t *testing.T) { queryParams: map[string]string{ "event_time[lte]": "2024-01-31T23:59:59Z", }, - wantGTE: nil, wantLTE: timePtr(time.Date(2024, 1, 31, 23, 59, 59, 0, time.UTC)), }, { - name: "both gte and lte params", + name: "RFC3339 lt param", + fieldName: "event_time", + queryParams: map[string]string{ + "event_time[lt]": "2024-01-31T23:59:59Z", + }, + wantLT: timePtr(time.Date(2024, 1, 31, 23, 59, 59, 0, time.UTC)), + }, + { + name: "all operators combined", fieldName: "created_at", queryParams: map[string]string{ "created_at[gte]": "2024-01-01T00:00:00Z", @@ -174,7 +190,30 @@ func TestParseDateFilter(t *testing.T) { "delivery_time[gte]": "2024-01-15", }, wantGTE: timePtr(time.Date(2024, 1, 15, 0, 0, 0, 0, time.UTC)), - wantLTE: nil, + }, + { + name: "any operator with true", + fieldName: "event_time", + queryParams: map[string]string{ + "event_time[any]": "true", + }, + wantAny: true, + }, + { + name: "any operator with 1", + fieldName: "event_time", + queryParams: map[string]string{ + "event_time[any]": "1", + }, + wantAny: true, + }, + { + name: "any operator with false", + fieldName: "event_time", + queryParams: map[string]string{ + "event_time[any]": "false", + }, + wantAny: false, }, { name: "invalid gte format returns error", @@ -184,6 +223,14 @@ func TestParseDateFilter(t *testing.T) { }, wantErrCode: http.StatusUnprocessableEntity, }, + { + name: "invalid gt format returns error", + fieldName: "event_time", + queryParams: map[string]string{ + "event_time[gt]": "invalid", + }, + wantErrCode: http.StatusUnprocessableEntity, + }, { name: "invalid lte format returns error", fieldName: "event_time", @@ -192,6 +239,14 @@ func TestParseDateFilter(t *testing.T) { }, wantErrCode: http.StatusUnprocessableEntity, }, + { + name: "invalid lt format returns error", + fieldName: "event_time", + queryParams: map[string]string{ + "event_time[lt]": "invalid", + }, + wantErrCode: http.StatusUnprocessableEntity, + }, } for _, tt := range tests { @@ -203,24 +258,239 @@ func TestParseDateFilter(t *testing.T) { if tt.wantErrCode != 0 { require.NotNil(t, errResp) assert.Equal(t, tt.wantErrCode, errResp.Code) + return + } + + require.Nil(t, errResp) + require.NotNil(t, result) + + assertTimePtr(t, "GTE", tt.wantGTE, result.GTE) + assertTimePtr(t, "GT", tt.wantGT, result.GT) + assertTimePtr(t, "LTE", tt.wantLTE, result.LTE) + assertTimePtr(t, "LT", tt.wantLT, result.LT) + assert.Equal(t, tt.wantAny, result.Any) + }) + } +} + +func TestParseStringFilter(t *testing.T) { + tests := []struct { + name string + fieldName string + queryParams map[string]string + wantContains *string + wantAny bool + }{ + { + name: "no params returns empty result", + fieldName: "topic", + queryParams: map[string]string{}, + }, + { + name: "contains param", + fieldName: "topic", + queryParams: map[string]string{ + "topic[contains]": "user", + }, + wantContains: strPtr("user"), + }, + { + name: "any operator with true", + fieldName: "topic", + queryParams: map[string]string{ + "topic[any]": "true", + }, + wantAny: true, + }, + { + name: "both contains and any", + fieldName: "topic", + queryParams: map[string]string{ + "topic[contains]": "event", + "topic[any]": "1", + }, + wantContains: strPtr("event"), + wantAny: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c, _ := createTestContext(tt.queryParams) + + result := apirouter.ParseStringFilter(c, tt.fieldName) + + require.NotNil(t, result) + if tt.wantContains != nil { + require.NotNil(t, result.Contains) + assert.Equal(t, *tt.wantContains, *result.Contains) } else { - require.Nil(t, errResp) - require.NotNil(t, result) - - if tt.wantGTE != nil { - require.NotNil(t, result.GTE) - assert.True(t, tt.wantGTE.Equal(*result.GTE), "GTE mismatch: want %v, got %v", tt.wantGTE, result.GTE) - } else { - assert.Nil(t, result.GTE) - } - - if tt.wantLTE != nil { - require.NotNil(t, result.LTE) - assert.True(t, tt.wantLTE.Equal(*result.LTE), "LTE mismatch: want %v, got %v", tt.wantLTE, result.LTE) - } else { - assert.Nil(t, result.LTE) - } + assert.Nil(t, result.Contains) + } + assert.Equal(t, tt.wantAny, result.Any) + }) + } +} + +func TestParseNumberFilter(t *testing.T) { + tests := []struct { + name string + fieldName string + queryParams map[string]string + wantGTE *float64 + wantGT *float64 + wantLTE *float64 + wantLT *float64 + wantAny bool + wantErrCode int + }{ + { + name: "no params returns empty result", + fieldName: "count", + queryParams: map[string]string{}, + }, + { + name: "gte param", + fieldName: "count", + queryParams: map[string]string{ + "count[gte]": "10", + }, + wantGTE: floatPtr(10), + }, + { + name: "gt param", + fieldName: "count", + queryParams: map[string]string{ + "count[gt]": "5", + }, + wantGT: floatPtr(5), + }, + { + name: "lte param", + fieldName: "count", + queryParams: map[string]string{ + "count[lte]": "100", + }, + wantLTE: floatPtr(100), + }, + { + name: "lt param", + fieldName: "count", + queryParams: map[string]string{ + "count[lt]": "50", + }, + wantLT: floatPtr(50), + }, + { + name: "float values", + fieldName: "score", + queryParams: map[string]string{ + "score[gte]": "3.14", + }, + wantGTE: floatPtr(3.14), + }, + { + name: "any operator", + fieldName: "count", + queryParams: map[string]string{ + "count[any]": "true", + }, + wantAny: true, + }, + { + name: "invalid gte format returns error", + fieldName: "count", + queryParams: map[string]string{ + "count[gte]": "not-a-number", + }, + wantErrCode: http.StatusUnprocessableEntity, + }, + { + name: "invalid gt format returns error", + fieldName: "count", + queryParams: map[string]string{ + "count[gt]": "abc", + }, + wantErrCode: http.StatusUnprocessableEntity, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c, _ := createTestContext(tt.queryParams) + + result, errResp := apirouter.ParseNumberFilter(c, tt.fieldName) + + if tt.wantErrCode != 0 { + require.NotNil(t, errResp) + assert.Equal(t, tt.wantErrCode, errResp.Code) + return } + + require.Nil(t, errResp) + require.NotNil(t, result) + + assertFloatPtr(t, "GTE", tt.wantGTE, result.GTE) + assertFloatPtr(t, "GT", tt.wantGT, result.GT) + assertFloatPtr(t, "LTE", tt.wantLTE, result.LTE) + assertFloatPtr(t, "LT", tt.wantLT, result.LT) + assert.Equal(t, tt.wantAny, result.Any) + }) + } +} + +func TestParseArrayParam(t *testing.T) { + tests := []struct { + name string + fieldName string + queryString string + want []string + }{ + { + name: "no params returns empty slice", + fieldName: "item", + queryString: "", + want: nil, + }, + { + name: "indexed format", + fieldName: "item", + queryString: "item[0]=hello&item[1]=world", + want: []string{"hello", "world"}, + }, + { + name: "indexed format out of order", + fieldName: "item", + queryString: "item[2]=c&item[0]=a&item[1]=b", + want: []string{"a", "b", "c"}, + }, + { + name: "unindexed format", + fieldName: "tag", + queryString: "tag[]=foo&tag[]=bar", + want: []string{"foo", "bar"}, + }, + { + name: "sparse indices", + fieldName: "item", + queryString: "item[0]=first&item[5]=sixth", + want: []string{"first", "sixth"}, + }, + { + name: "ignores non-matching params", + fieldName: "item", + queryString: "item[0]=hello&other=value&item[1]=world", + want: []string{"hello", "world"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c, _ := createTestContextWithQuery(tt.queryString) + + result := apirouter.ParseArrayParam(c, tt.fieldName) + + assert.Equal(t, tt.want, result) }) } } @@ -240,6 +510,46 @@ func createTestContext(queryParams map[string]string) (*gin.Context, *httptest.R return c, w } +func createTestContextWithQuery(queryString string) (*gin.Context, *httptest.ResponseRecorder) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + + path := "/" + if queryString != "" { + path = "/?" + queryString + } + c.Request = httptest.NewRequest(http.MethodGet, path, nil) + return c, w +} + func timePtr(t time.Time) *time.Time { return &t } + +func strPtr(s string) *string { + return &s +} + +func floatPtr(f float64) *float64 { + return &f +} + +func assertTimePtr(t *testing.T, name string, want, got *time.Time) { + t.Helper() + if want != nil { + require.NotNil(t, got, "%s should not be nil", name) + assert.True(t, want.Equal(*got), "%s mismatch: want %v, got %v", name, want, got) + } else { + assert.Nil(t, got, "%s should be nil", name) + } +} + +func assertFloatPtr(t *testing.T, name string, want, got *float64) { + t.Helper() + if want != nil { + require.NotNil(t, got, "%s should not be nil", name) + assert.Equal(t, *want, *got, "%s mismatch", name) + } else { + assert.Nil(t, got, "%s should be nil", name) + } +} From caddeb769d0c287b1350ba8a9ed4c4406115b26e Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Tue, 20 Jan 2026 00:49:47 +0700 Subject: [PATCH 03/14] refactor: simplify pagination helpers to only support gte/lte - Remove gt, lt, any, contains operators (stores don't support yet) - Return 400 for unsupported operators with clear message - Remove string and number filter helpers (not needed yet) - Keep array param parser for future use Co-Authored-By: Claude Opus 4.5 --- internal/apirouter/pagination.go | 165 +++------------ internal/apirouter/pagination_test.go | 280 +++----------------------- 2 files changed, 49 insertions(+), 396 deletions(-) diff --git a/internal/apirouter/pagination.go b/internal/apirouter/pagination.go index 69b5825dc..dff415001 100644 --- a/internal/apirouter/pagination.go +++ b/internal/apirouter/pagination.go @@ -133,19 +133,31 @@ func ParseArrayParam(c *gin.Context, fieldName string) []string { } // DateFilterResult holds the parsed date filter values. -// Supports all Hookdeck query operators for dates: gte, gt, lte, lt, any. +// Currently supports gte and lte operators only. type DateFilterResult struct { - GTE *time.Time // Greater than or equal - GT *time.Time // Greater than - LTE *time.Time // Less than or equal - LT *time.Time // Less than - Any bool // Field is not null (any value present) + GTE *time.Time // Greater than or equal (>=) + LTE *time.Time // Less than or equal (<=) } -// ParseDateFilter parses bracket notation date filters (e.g., field[gte], field[lte], field[gt], field[lt], field[any]). -// Returns the parsed dates and any validation error response. +// unsupportedDateOps lists operators we recognize but don't support yet. +// When stores add support, move these to supported and update ParseDateFilter. +var unsupportedDateOps = []string{"gt", "lt", "any"} + +// ParseDateFilter parses bracket notation date filters (e.g., field[gte], field[lte]). +// Returns 400 for unsupported operators (gt, lt, any). // Accepts both RFC3339 format (2024-01-01T00:00:00Z) and date-only format (2024-01-01). func ParseDateFilter(c *gin.Context, fieldName string) (*DateFilterResult, *ErrorResponse) { + // Check for unsupported operators first + for _, op := range unsupportedDateOps { + key := fieldName + "[" + op + "]" + if c.Query(key) != "" { + return nil, &ErrorResponse{ + Code: http.StatusBadRequest, + Message: fmt.Sprintf("operator '%s' is not supported, use 'gte' or 'lte'", op), + } + } + } + result := &DateFilterResult{} // Parse [gte] - greater than or equal @@ -158,16 +170,6 @@ func ParseDateFilter(c *gin.Context, fieldName string) (*DateFilterResult, *Erro result.GTE = &t } - // Parse [gt] - greater than - gtKey := fieldName + "[gt]" - if gtStr := c.Query(gtKey); gtStr != "" { - t, err := parseDateTime(gtStr) - if err != nil { - return nil, dateFormatError(gtKey) - } - result.GT = &t - } - // Parse [lte] - less than or equal lteKey := fieldName + "[lte]" if lteStr := c.Query(lteKey); lteStr != "" { @@ -178,22 +180,6 @@ func ParseDateFilter(c *gin.Context, fieldName string) (*DateFilterResult, *Erro result.LTE = &t } - // Parse [lt] - less than - ltKey := fieldName + "[lt]" - if ltStr := c.Query(ltKey); ltStr != "" { - t, err := parseDateTime(ltStr) - if err != nil { - return nil, dateFormatError(ltKey) - } - result.LT = &t - } - - // Parse [any] - field is not null - anyKey := fieldName + "[any]" - if anyStr := c.Query(anyKey); anyStr != "" { - result.Any = anyStr == "true" || anyStr == "1" - } - return result, nil } @@ -208,114 +194,9 @@ func dateFormatError(key string) *ErrorResponse { } } -// StringFilterResult holds the parsed string filter values. -// Supports Hookdeck query operators for strings: contains, any. -type StringFilterResult struct { - Contains *string // Contains substring - Any bool // Field is not null (any value present) -} - -// ParseStringFilter parses bracket notation string filters (e.g., field[contains], field[any]). -// Returns the parsed filter values and any validation error response. -func ParseStringFilter(c *gin.Context, fieldName string) *StringFilterResult { - result := &StringFilterResult{} - - // Parse [contains] - contains substring - containsKey := fieldName + "[contains]" - if containsStr := c.Query(containsKey); containsStr != "" { - result.Contains = &containsStr - } - - // Parse [any] - field is not null - anyKey := fieldName + "[any]" - if anyStr := c.Query(anyKey); anyStr != "" { - result.Any = anyStr == "true" || anyStr == "1" - } - - return result -} - -// NumberFilterResult holds the parsed number filter values. -// Supports Hookdeck query operators for numbers: gte, gt, lte, lt, any. -type NumberFilterResult struct { - GTE *float64 // Greater than or equal - GT *float64 // Greater than - LTE *float64 // Less than or equal - LT *float64 // Less than - Any bool // Field is not null (any value present) -} - -// ParseNumberFilter parses bracket notation number filters (e.g., field[gte], field[lte]). -// Returns the parsed numbers and any validation error response. -func ParseNumberFilter(c *gin.Context, fieldName string) (*NumberFilterResult, *ErrorResponse) { - result := &NumberFilterResult{} - - // Parse [gte] - greater than or equal - gteKey := fieldName + "[gte]" - if gteStr := c.Query(gteKey); gteStr != "" { - n, err := parseNumber(gteStr) - if err != nil { - return nil, numberFormatError(gteKey) - } - result.GTE = &n - } - - // Parse [gt] - greater than - gtKey := fieldName + "[gt]" - if gtStr := c.Query(gtKey); gtStr != "" { - n, err := parseNumber(gtStr) - if err != nil { - return nil, numberFormatError(gtKey) - } - result.GT = &n - } - - // Parse [lte] - less than or equal - lteKey := fieldName + "[lte]" - if lteStr := c.Query(lteKey); lteStr != "" { - n, err := parseNumber(lteStr) - if err != nil { - return nil, numberFormatError(lteKey) - } - result.LTE = &n - } - - // Parse [lt] - less than - ltKey := fieldName + "[lt]" - if ltStr := c.Query(ltKey); ltStr != "" { - n, err := parseNumber(ltStr) - if err != nil { - return nil, numberFormatError(ltKey) - } - result.LT = &n - } - - // Parse [any] - field is not null - anyKey := fieldName + "[any]" - if anyStr := c.Query(anyKey); anyStr != "" { - result.Any = anyStr == "true" || anyStr == "1" - } - - return result, nil -} - -// numberFormatError returns a validation error for invalid number format. -func numberFormatError(key string) *ErrorResponse { - return &ErrorResponse{ - Code: http.StatusUnprocessableEntity, - Message: "validation error", - Data: map[string]string{ - "query." + key: "invalid format, expected a number", - }, - } -} - -// parseNumber attempts to parse a number string. -func parseNumber(s string) (float64, error) { - var n float64 - _, err := fmt.Sscanf(s, "%f", &n) - return n, err -} +// Note: String and number filters are not yet supported by the stores. +// These types are defined for future use when stores add support. +// Attempting to use them will return 400 errors from the handlers. // parseDateTime attempts to parse a date string in RFC3339 or date-only (YYYY-MM-DD) format. func parseDateTime(s string) (time.Time, error) { diff --git a/internal/apirouter/pagination_test.go b/internal/apirouter/pagination_test.go index 7e305ed15..a0e3c32b8 100644 --- a/internal/apirouter/pagination_test.go +++ b/internal/apirouter/pagination_test.go @@ -77,7 +77,7 @@ func TestParseDir(t *testing.T) { } func TestParseOrderBy(t *testing.T) { - allowedValues := []string{"created_at", "event_time"} + allowedValues := []string{"created_at", "time"} tests := []struct { name string @@ -92,8 +92,8 @@ func TestParseOrderBy(t *testing.T) { }, { name: "valid value returns value", - queryOrderBy: "event_time", - wantOrderBy: "event_time", + queryOrderBy: "time", + wantOrderBy: "time", }, { name: "another valid value", @@ -130,51 +130,32 @@ func TestParseDateFilter(t *testing.T) { fieldName string queryParams map[string]string wantGTE *time.Time - wantGT *time.Time wantLTE *time.Time - wantLT *time.Time - wantAny bool wantErrCode int }{ { name: "no params returns empty result", - fieldName: "event_time", + fieldName: "time", queryParams: map[string]string{}, }, { name: "RFC3339 gte param", - fieldName: "event_time", + fieldName: "time", queryParams: map[string]string{ - "event_time[gte]": "2024-01-15T10:30:00Z", + "time[gte]": "2024-01-15T10:30:00Z", }, wantGTE: timePtr(time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC)), }, - { - name: "RFC3339 gt param", - fieldName: "event_time", - queryParams: map[string]string{ - "event_time[gt]": "2024-01-15T10:30:00Z", - }, - wantGT: timePtr(time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC)), - }, { name: "RFC3339 lte param", - fieldName: "event_time", + fieldName: "time", queryParams: map[string]string{ - "event_time[lte]": "2024-01-31T23:59:59Z", + "time[lte]": "2024-01-31T23:59:59Z", }, wantLTE: timePtr(time.Date(2024, 1, 31, 23, 59, 59, 0, time.UTC)), }, { - name: "RFC3339 lt param", - fieldName: "event_time", - queryParams: map[string]string{ - "event_time[lt]": "2024-01-31T23:59:59Z", - }, - wantLT: timePtr(time.Date(2024, 1, 31, 23, 59, 59, 0, time.UTC)), - }, - { - name: "all operators combined", + name: "both gte and lte params", fieldName: "created_at", queryParams: map[string]string{ "created_at[gte]": "2024-01-01T00:00:00Z", @@ -185,65 +166,25 @@ func TestParseDateFilter(t *testing.T) { }, { name: "date-only format", - fieldName: "delivery_time", + fieldName: "time", queryParams: map[string]string{ - "delivery_time[gte]": "2024-01-15", + "time[gte]": "2024-01-15", }, wantGTE: timePtr(time.Date(2024, 1, 15, 0, 0, 0, 0, time.UTC)), }, - { - name: "any operator with true", - fieldName: "event_time", - queryParams: map[string]string{ - "event_time[any]": "true", - }, - wantAny: true, - }, - { - name: "any operator with 1", - fieldName: "event_time", - queryParams: map[string]string{ - "event_time[any]": "1", - }, - wantAny: true, - }, - { - name: "any operator with false", - fieldName: "event_time", - queryParams: map[string]string{ - "event_time[any]": "false", - }, - wantAny: false, - }, { name: "invalid gte format returns error", - fieldName: "event_time", + fieldName: "time", queryParams: map[string]string{ - "event_time[gte]": "not-a-date", - }, - wantErrCode: http.StatusUnprocessableEntity, - }, - { - name: "invalid gt format returns error", - fieldName: "event_time", - queryParams: map[string]string{ - "event_time[gt]": "invalid", + "time[gte]": "not-a-date", }, wantErrCode: http.StatusUnprocessableEntity, }, { name: "invalid lte format returns error", - fieldName: "event_time", + fieldName: "time", queryParams: map[string]string{ - "event_time[lte]": "invalid", - }, - wantErrCode: http.StatusUnprocessableEntity, - }, - { - name: "invalid lt format returns error", - fieldName: "event_time", - queryParams: map[string]string{ - "event_time[lt]": "invalid", + "time[lte]": "invalid", }, wantErrCode: http.StatusUnprocessableEntity, }, @@ -265,176 +206,25 @@ func TestParseDateFilter(t *testing.T) { require.NotNil(t, result) assertTimePtr(t, "GTE", tt.wantGTE, result.GTE) - assertTimePtr(t, "GT", tt.wantGT, result.GT) assertTimePtr(t, "LTE", tt.wantLTE, result.LTE) - assertTimePtr(t, "LT", tt.wantLT, result.LT) - assert.Equal(t, tt.wantAny, result.Any) }) } } -func TestParseStringFilter(t *testing.T) { - tests := []struct { - name string - fieldName string - queryParams map[string]string - wantContains *string - wantAny bool - }{ - { - name: "no params returns empty result", - fieldName: "topic", - queryParams: map[string]string{}, - }, - { - name: "contains param", - fieldName: "topic", - queryParams: map[string]string{ - "topic[contains]": "user", - }, - wantContains: strPtr("user"), - }, - { - name: "any operator with true", - fieldName: "topic", - queryParams: map[string]string{ - "topic[any]": "true", - }, - wantAny: true, - }, - { - name: "both contains and any", - fieldName: "topic", - queryParams: map[string]string{ - "topic[contains]": "event", - "topic[any]": "1", - }, - wantContains: strPtr("event"), - wantAny: true, - }, - } +func TestParseDateFilter_UnsupportedOperators(t *testing.T) { + unsupportedOps := []string{"gt", "lt", "any"} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - c, _ := createTestContext(tt.queryParams) + for _, op := range unsupportedOps { + t.Run(op+" returns 400", func(t *testing.T) { + c, _ := createTestContext(map[string]string{ + "time[" + op + "]": "2024-01-01T00:00:00Z", + }) - result := apirouter.ParseStringFilter(c, tt.fieldName) + _, errResp := apirouter.ParseDateFilter(c, "time") - require.NotNil(t, result) - if tt.wantContains != nil { - require.NotNil(t, result.Contains) - assert.Equal(t, *tt.wantContains, *result.Contains) - } else { - assert.Nil(t, result.Contains) - } - assert.Equal(t, tt.wantAny, result.Any) - }) - } -} - -func TestParseNumberFilter(t *testing.T) { - tests := []struct { - name string - fieldName string - queryParams map[string]string - wantGTE *float64 - wantGT *float64 - wantLTE *float64 - wantLT *float64 - wantAny bool - wantErrCode int - }{ - { - name: "no params returns empty result", - fieldName: "count", - queryParams: map[string]string{}, - }, - { - name: "gte param", - fieldName: "count", - queryParams: map[string]string{ - "count[gte]": "10", - }, - wantGTE: floatPtr(10), - }, - { - name: "gt param", - fieldName: "count", - queryParams: map[string]string{ - "count[gt]": "5", - }, - wantGT: floatPtr(5), - }, - { - name: "lte param", - fieldName: "count", - queryParams: map[string]string{ - "count[lte]": "100", - }, - wantLTE: floatPtr(100), - }, - { - name: "lt param", - fieldName: "count", - queryParams: map[string]string{ - "count[lt]": "50", - }, - wantLT: floatPtr(50), - }, - { - name: "float values", - fieldName: "score", - queryParams: map[string]string{ - "score[gte]": "3.14", - }, - wantGTE: floatPtr(3.14), - }, - { - name: "any operator", - fieldName: "count", - queryParams: map[string]string{ - "count[any]": "true", - }, - wantAny: true, - }, - { - name: "invalid gte format returns error", - fieldName: "count", - queryParams: map[string]string{ - "count[gte]": "not-a-number", - }, - wantErrCode: http.StatusUnprocessableEntity, - }, - { - name: "invalid gt format returns error", - fieldName: "count", - queryParams: map[string]string{ - "count[gt]": "abc", - }, - wantErrCode: http.StatusUnprocessableEntity, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - c, _ := createTestContext(tt.queryParams) - - result, errResp := apirouter.ParseNumberFilter(c, tt.fieldName) - - if tt.wantErrCode != 0 { - require.NotNil(t, errResp) - assert.Equal(t, tt.wantErrCode, errResp.Code) - return - } - - require.Nil(t, errResp) - require.NotNil(t, result) - - assertFloatPtr(t, "GTE", tt.wantGTE, result.GTE) - assertFloatPtr(t, "GT", tt.wantGT, result.GT) - assertFloatPtr(t, "LTE", tt.wantLTE, result.LTE) - assertFloatPtr(t, "LT", tt.wantLT, result.LT) - assert.Equal(t, tt.wantAny, result.Any) + require.NotNil(t, errResp) + assert.Equal(t, http.StatusBadRequest, errResp.Code) + assert.Contains(t, errResp.Message, "not supported") }) } } @@ -526,14 +316,6 @@ func timePtr(t time.Time) *time.Time { return &t } -func strPtr(s string) *string { - return &s -} - -func floatPtr(f float64) *float64 { - return &f -} - func assertTimePtr(t *testing.T, name string, want, got *time.Time) { t.Helper() if want != nil { @@ -543,13 +325,3 @@ func assertTimePtr(t *testing.T, name string, want, got *time.Time) { assert.Nil(t, got, "%s should be nil", name) } } - -func assertFloatPtr(t *testing.T, name string, want, got *float64) { - t.Helper() - if want != nil { - require.NotNil(t, got, "%s should not be nil", name) - assert.Equal(t, *want, *got, "%s mismatch", name) - } else { - assert.Nil(t, got, "%s should be nil", name) - } -} From 7b8726a3ce09a40da9c08b5b4aba6b7522d3a03d Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Tue, 20 Jan 2026 00:54:43 +0700 Subject: [PATCH 04/14] refactor: update error response format to Hookdeck style Co-Authored-By: Claude Opus 4.5 --- internal/apirouter/errorhandler_middleware.go | 55 ++++- .../apirouter/errorhandler_middleware_test.go | 205 ++++++++++++++++++ 2 files changed, 253 insertions(+), 7 deletions(-) create mode 100644 internal/apirouter/errorhandler_middleware_test.go diff --git a/internal/apirouter/errorhandler_middleware.go b/internal/apirouter/errorhandler_middleware.go index 2bf749012..ff006ced5 100644 --- a/internal/apirouter/errorhandler_middleware.go +++ b/internal/apirouter/errorhandler_middleware.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/http" + "strings" "github.com/gin-gonic/gin" "github.com/go-playground/validator/v10" @@ -31,6 +32,7 @@ func ErrorHandlerMiddleware() gin.HandlerFunc { type ErrorResponse struct { Err error `json:"-"` Code int `json:"-"` + Status int `json:"status"` Message string `json:"message"` Data interface{} `json:"data,omitempty"` } @@ -48,13 +50,13 @@ func (e *ErrorResponse) Parse(err error) { } if validationErrors, ok := err.(validator.ValidationErrors); ok { - out := map[string]string{} + var messages []string for _, err := range validationErrors { - out[err.Field()] = err.Tag() + messages = append(messages, formatValidationError(err.Field(), err.Tag(), err.Param())) } - e.Code = -1 + e.Code = http.StatusUnprocessableEntity e.Message = "validation error" - e.Data = out + e.Data = messages e.Err = err return } @@ -68,13 +70,13 @@ func (e *ErrorResponse) Parse(err error) { // Handle destregistry.ErrDestinationValidation var validationErr *destregistry.ErrDestinationValidation if errors.As(err, &validationErr) { - validationDetails := make(map[string]string) + var messages []string for _, detail := range validationErr.Errors { - validationDetails[detail.Field] = detail.Type + messages = append(messages, formatValidationError(detail.Field, detail.Type, "")) } e.Code = http.StatusUnprocessableEntity e.Message = "validation error" - e.Data = validationDetails + e.Data = messages e.Err = err return } @@ -83,6 +85,44 @@ func (e *ErrorResponse) Parse(err error) { e.Err = err } +// formatValidationError converts a validation error into a human-readable message. +// field is the field name, tag is the validation rule (e.g., "required", "min"), +// and param is the rule parameter (e.g., "6" for min=6). +func formatValidationError(field, tag, param string) string { + // Convert field name to lowercase for consistency + field = strings.ToLower(field) + + switch tag { + case "required": + return fmt.Sprintf("%s is required", field) + case "min": + return fmt.Sprintf("%s must be at least %s characters", field, param) + case "max": + return fmt.Sprintf("%s must be at most %s characters", field, param) + case "email": + return fmt.Sprintf("%s must be a valid email address", field) + case "url": + return fmt.Sprintf("%s must be a valid URL", field) + case "oneof": + return fmt.Sprintf("%s must be one of: %s", field, param) + case "uuid": + return fmt.Sprintf("%s must be a valid UUID", field) + case "gt": + return fmt.Sprintf("%s must be greater than %s", field, param) + case "gte": + return fmt.Sprintf("%s must be greater than or equal to %s", field, param) + case "lt": + return fmt.Sprintf("%s must be less than %s", field, param) + case "lte": + return fmt.Sprintf("%s must be less than or equal to %s", field, param) + default: + if param != "" { + return fmt.Sprintf("%s failed %s=%s validation", field, tag, param) + } + return fmt.Sprintf("%s failed %s validation", field, tag) + } +} + func isInvalidJSON(err error) bool { var syntaxError *json.SyntaxError var unmarshalTypeError *json.UnmarshalTypeError @@ -93,6 +133,7 @@ func isInvalidJSON(err error) bool { } func handleErrorResponse(c *gin.Context, response ErrorResponse) { + response.Status = response.Code c.JSON(response.Code, response) } diff --git a/internal/apirouter/errorhandler_middleware_test.go b/internal/apirouter/errorhandler_middleware_test.go new file mode 100644 index 000000000..6fb44f200 --- /dev/null +++ b/internal/apirouter/errorhandler_middleware_test.go @@ -0,0 +1,205 @@ +package apirouter_test + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gin-gonic/gin" + "github.com/go-playground/validator/v10" + "github.com/hookdeck/outpost/internal/apirouter" + "github.com/hookdeck/outpost/internal/destregistry" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func init() { + gin.SetMode(gin.TestMode) +} + +func TestErrorResponse_Parse_ValidationErrors(t *testing.T) { + t.Parallel() + + type testInput struct { + Email string `validate:"required,email"` + Password string `validate:"required,min=6"` + } + + validate := validator.New() + + t.Run("produces array of human-readable messages for validator.ValidationErrors", func(t *testing.T) { + t.Parallel() + + // Trigger validation errors + input := testInput{Email: "", Password: ""} + err := validate.Struct(input) + require.Error(t, err) + + var errorResponse apirouter.ErrorResponse + errorResponse.Parse(err) + + assert.Equal(t, http.StatusUnprocessableEntity, errorResponse.Code) + assert.Equal(t, "validation error", errorResponse.Message) + + // Data should be []string + messages, ok := errorResponse.Data.([]string) + require.True(t, ok, "Data should be []string, got %T", errorResponse.Data) + assert.Len(t, messages, 2) + + // Check that messages are human-readable (order may vary) + assert.Contains(t, messages, "email is required") + assert.Contains(t, messages, "password is required") + }) + + t.Run("includes validation param in message", func(t *testing.T) { + t.Parallel() + + // Trigger min validation error with param + input := testInput{Email: "test@example.com", Password: "abc"} + err := validate.Struct(input) + require.Error(t, err) + + var errorResponse apirouter.ErrorResponse + errorResponse.Parse(err) + + messages, ok := errorResponse.Data.([]string) + require.True(t, ok) + assert.Contains(t, messages, "password must be at least 6 characters") + }) +} + +func TestErrorResponse_Parse_DestRegistryValidation(t *testing.T) { + t.Parallel() + + t.Run("produces array of human-readable messages for destregistry.ErrDestinationValidation", func(t *testing.T) { + t.Parallel() + + err := destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{ + {Field: "config.url", Type: "required"}, + {Field: "type", Type: "invalid_type"}, + }) + + var errorResponse apirouter.ErrorResponse + errorResponse.Parse(err) + + assert.Equal(t, http.StatusUnprocessableEntity, errorResponse.Code) + assert.Equal(t, "validation error", errorResponse.Message) + + messages, ok := errorResponse.Data.([]string) + require.True(t, ok, "Data should be []string, got %T", errorResponse.Data) + assert.Len(t, messages, 2) + assert.Contains(t, messages, "config.url is required") + assert.Contains(t, messages, "type failed invalid_type validation") + }) +} + +func TestHandleErrorResponse_SetsHandledAndStatus(t *testing.T) { + t.Parallel() + + router := gin.New() + router.Use(apirouter.ErrorHandlerMiddleware()) + router.GET("/test", func(c *gin.Context) { + c.Error(apirouter.NewErrBadRequest(assert.AnError)) + }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/test", nil) + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusBadRequest, w.Code) + + var response map[string]interface{} + err := json.Unmarshal(w.Body.Bytes(), &response) + require.NoError(t, err) + + assert.Equal(t, float64(http.StatusBadRequest), response["status"]) + assert.Equal(t, assert.AnError.Error(), response["message"]) +} + +func TestHandleErrorResponse_ValidationErrorFormat(t *testing.T) { + t.Parallel() + + type requestBody struct { + Name string `json:"name" binding:"required"` + } + + router := gin.New() + router.Use(apirouter.ErrorHandlerMiddleware()) + router.POST("/test", func(c *gin.Context) { + var body requestBody + if err := c.ShouldBindJSON(&body); err != nil { + apirouter.AbortWithValidationError(c, err) + return + } + c.JSON(http.StatusOK, body) + }) + + w := httptest.NewRecorder() + // Use empty JSON object to trigger validation error (not JSON parse error) + req, _ := http.NewRequest("POST", "/test", strings.NewReader("{}")) + req.Header.Set("Content-Type", "application/json") + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusUnprocessableEntity, w.Code) + + var response map[string]interface{} + err := json.Unmarshal(w.Body.Bytes(), &response) + require.NoError(t, err) + + assert.Equal(t, float64(http.StatusUnprocessableEntity), response["status"]) + assert.Equal(t, "validation error", response["message"]) + + // Data should be an array + data, ok := response["data"].([]interface{}) + require.True(t, ok, "data should be an array, got %T", response["data"]) + assert.Len(t, data, 1) + assert.Equal(t, "name is required", data[0]) +} + +func TestErrorResponse_NotFoundFormat(t *testing.T) { + t.Parallel() + + router := gin.New() + router.Use(apirouter.ErrorHandlerMiddleware()) + router.GET("/test", func(c *gin.Context) { + c.Error(apirouter.NewErrNotFound("tenant")) + }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/test", nil) + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusNotFound, w.Code) + + var response map[string]interface{} + err := json.Unmarshal(w.Body.Bytes(), &response) + require.NoError(t, err) + + assert.Equal(t, float64(http.StatusNotFound), response["status"]) + assert.Equal(t, "tenant not found", response["message"]) +} + +func TestErrorResponse_InternalServerErrorFormat(t *testing.T) { + t.Parallel() + + router := gin.New() + router.Use(apirouter.ErrorHandlerMiddleware()) + router.GET("/test", func(c *gin.Context) { + c.Error(apirouter.NewErrInternalServer(assert.AnError)) + }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/test", nil) + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusInternalServerError, w.Code) + + var response map[string]interface{} + err := json.Unmarshal(w.Body.Bytes(), &response) + require.NoError(t, err) + + assert.Equal(t, float64(http.StatusInternalServerError), response["status"]) + assert.Equal(t, "internal server error", response["message"]) +} From 06da6bcd54e1520aff65f7b43dc62c09081f323a Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Tue, 20 Jan 2026 01:13:18 +0700 Subject: [PATCH 05/14] refactor: update list endpoints to Hookdeck response format Co-Authored-By: Claude Opus 4.5 --- internal/apirouter/log_handlers.go | 184 +++++++++++------------- internal/apirouter/log_handlers_test.go | 52 +++---- internal/apirouter/tenant_handlers.go | 32 ++++- internal/models/entity.go | 67 ++++++--- internal/models/entity_test.go | 16 ++- internal/models/entitysuite_test.go | 76 +++++----- 6 files changed, 238 insertions(+), 189 deletions(-) diff --git a/internal/apirouter/log_handlers.go b/internal/apirouter/log_handlers.go index d0ee22d5e..5d0765c4e 100644 --- a/internal/apirouter/log_handlers.go +++ b/internal/apirouter/log_handlers.go @@ -133,16 +133,14 @@ type APIEvent struct { // ListDeliveriesResponse is the response for ListDeliveries type ListDeliveriesResponse struct { - Data []APIDelivery `json:"data"` - Next string `json:"next,omitempty"` - Prev string `json:"prev,omitempty"` + Models []APIDelivery `json:"models"` + Pagination PaginationInfo `json:"pagination"` } // ListEventsResponse is the response for ListEvents type ListEventsResponse struct { - Data []APIEvent `json:"data"` - Next string `json:"next,omitempty"` - Prev string `json:"prev,omitempty"` + Models []APIEvent `json:"models"` + Pagination PaginationInfo `json:"pagination"` } // toAPIDelivery converts a DeliveryEvent to APIDelivery with expand options @@ -203,34 +201,33 @@ func (h *LogHandlers) ListDeliveries(c *gin.Context) { } func (h *LogHandlers) listDeliveriesInternal(c *gin.Context, tenantID string) { - var start, end *time.Time - if startStr := c.Query("start"); startStr != "" { - t, err := time.Parse(time.RFC3339, startStr) - if err != nil { - AbortWithError(c, http.StatusUnprocessableEntity, ErrorResponse{ - Code: http.StatusUnprocessableEntity, - Message: "validation error", - Data: map[string]string{ - "query.start": "invalid format, expected RFC3339", - }, - }) - return - } - start = &t - } - if endStr := c.Query("end"); endStr != "" { - t, err := time.Parse(time.RFC3339, endStr) - if err != nil { - AbortWithError(c, http.StatusUnprocessableEntity, ErrorResponse{ - Code: http.StatusUnprocessableEntity, - Message: "validation error", - Data: map[string]string{ - "query.end": "invalid format, expected RFC3339", - }, - }) - return - } - end = &t + // Parse and validate dir (sort direction) + dir, errResp := ParseDir(c) + if errResp != nil { + AbortWithError(c, errResp.Code, *errResp) + return + } + if dir == "" { + dir = "desc" + } + + // Parse and validate order_by (delivery_time or event_time) + orderBy, errResp := ParseOrderBy(c, []string{"delivery_time", "event_time"}) + if errResp != nil { + AbortWithError(c, errResp.Code, *errResp) + return + } + if orderBy == "" { + orderBy = "delivery_time" + } + // Note: order_by is informational only for now - store always sorts by delivery_time + _ = orderBy + + // Parse delivery_time date filters + deliveryTimeFilter, errResp := ParseDateFilter(c, "delivery_time") + if errResp != nil { + AbortWithError(c, errResp.Code, *errResp) + return } limit := parseLimit(c, 100, 1000) @@ -240,33 +237,18 @@ func (h *LogHandlers) listDeliveriesInternal(c *gin.Context, tenantID string) { destinationIDs = []string{destID} } - sortOrder := c.Query("sort_order") - if sortOrder == "" { - sortOrder = "desc" - } - if sortOrder != "asc" && sortOrder != "desc" { - AbortWithError(c, http.StatusUnprocessableEntity, ErrorResponse{ - Code: http.StatusUnprocessableEntity, - Message: "validation error", - Data: map[string]string{ - "query.sort_order": "must be 'asc' or 'desc'", - }, - }) - return - } - req := logstore.ListDeliveryEventRequest{ TenantID: tenantID, EventID: c.Query("event_id"), DestinationIDs: destinationIDs, Status: c.Query("status"), Topics: parseQueryArray(c, "topic"), - Start: start, - End: end, + Start: deliveryTimeFilter.GTE, + End: deliveryTimeFilter.LTE, Limit: limit, Next: c.Query("next"), Prev: c.Query("prev"), - SortOrder: sortOrder, + SortOrder: dir, } response, err := h.logStore.ListDeliveryEvent(c.Request.Context(), req) @@ -287,9 +269,14 @@ func (h *LogHandlers) listDeliveriesInternal(c *gin.Context, tenantID string) { } c.JSON(http.StatusOK, ListDeliveriesResponse{ - Data: apiDeliveries, - Next: response.Next, - Prev: response.Prev, + Models: apiDeliveries, + Pagination: PaginationInfo{ + OrderBy: orderBy, + Dir: dir, + Limit: limit, + Next: CursorToPtr(response.Next), + Prev: CursorToPtr(response.Prev), + }, }) } @@ -371,34 +358,33 @@ func (h *LogHandlers) ListEvents(c *gin.Context) { } func (h *LogHandlers) listEventsInternal(c *gin.Context, tenantID string) { - var start, end *time.Time - if startStr := c.Query("start"); startStr != "" { - t, err := time.Parse(time.RFC3339, startStr) - if err != nil { - AbortWithError(c, http.StatusUnprocessableEntity, ErrorResponse{ - Code: http.StatusUnprocessableEntity, - Message: "validation error", - Data: map[string]string{ - "query.start": "invalid format, expected RFC3339", - }, - }) - return - } - start = &t - } - if endStr := c.Query("end"); endStr != "" { - t, err := time.Parse(time.RFC3339, endStr) - if err != nil { - AbortWithError(c, http.StatusUnprocessableEntity, ErrorResponse{ - Code: http.StatusUnprocessableEntity, - Message: "validation error", - Data: map[string]string{ - "query.end": "invalid format, expected RFC3339", - }, - }) - return - } - end = &t + // Parse and validate dir (sort direction) + dir, errResp := ParseDir(c) + if errResp != nil { + AbortWithError(c, errResp.Code, *errResp) + return + } + if dir == "" { + dir = "desc" + } + + // Parse and validate order_by (event_time or created_at) + orderBy, errResp := ParseOrderBy(c, []string{"event_time", "created_at"}) + if errResp != nil { + AbortWithError(c, errResp.Code, *errResp) + return + } + if orderBy == "" { + orderBy = "event_time" + } + // Note: order_by is informational only for now - store always sorts by event_time + _ = orderBy + + // Parse event_time date filters + eventTimeFilter, errResp := ParseDateFilter(c, "event_time") + if errResp != nil { + AbortWithError(c, errResp.Code, *errResp) + return } limit := parseLimit(c, 100, 1000) @@ -408,31 +394,16 @@ func (h *LogHandlers) listEventsInternal(c *gin.Context, tenantID string) { destinationIDs = []string{destID} } - sortOrder := c.Query("sort_order") - if sortOrder == "" { - sortOrder = "desc" - } - if sortOrder != "asc" && sortOrder != "desc" { - AbortWithError(c, http.StatusUnprocessableEntity, ErrorResponse{ - Code: http.StatusUnprocessableEntity, - Message: "validation error", - Data: map[string]string{ - "query.sort_order": "must be 'asc' or 'desc'", - }, - }) - return - } - req := logstore.ListEventRequest{ TenantID: tenantID, DestinationIDs: destinationIDs, Topics: parseQueryArray(c, "topic"), - EventStart: start, - EventEnd: end, + EventStart: eventTimeFilter.GTE, + EventEnd: eventTimeFilter.LTE, Limit: limit, Next: c.Query("next"), Prev: c.Query("prev"), - SortOrder: sortOrder, + SortOrder: dir, } response, err := h.logStore.ListEvent(c.Request.Context(), req) @@ -458,8 +429,13 @@ func (h *LogHandlers) listEventsInternal(c *gin.Context, tenantID string) { } c.JSON(http.StatusOK, ListEventsResponse{ - Data: apiEvents, - Next: response.Next, - Prev: response.Prev, + Models: apiEvents, + Pagination: PaginationInfo{ + OrderBy: orderBy, + Dir: dir, + Limit: limit, + Next: CursorToPtr(response.Next), + Prev: CursorToPtr(response.Prev), + }, }) } diff --git a/internal/apirouter/log_handlers_test.go b/internal/apirouter/log_handlers_test.go index f80e2c9a5..72cd5de68 100644 --- a/internal/apirouter/log_handlers_test.go +++ b/internal/apirouter/log_handlers_test.go @@ -46,7 +46,7 @@ func TestListDeliveries(t *testing.T) { var response map[string]interface{} require.NoError(t, json.Unmarshal(w.Body.Bytes(), &response)) - data := response["data"].([]interface{}) + data := response["models"].([]interface{}) assert.Len(t, data, 0) }) @@ -91,7 +91,7 @@ func TestListDeliveries(t *testing.T) { var response map[string]interface{} require.NoError(t, json.Unmarshal(w.Body.Bytes(), &response)) - data := response["data"].([]interface{}) + data := response["models"].([]interface{}) assert.Len(t, data, 1) firstDelivery := data[0].(map[string]interface{}) @@ -111,7 +111,7 @@ func TestListDeliveries(t *testing.T) { var response map[string]interface{} require.NoError(t, json.Unmarshal(w.Body.Bytes(), &response)) - data := response["data"].([]interface{}) + data := response["models"].([]interface{}) require.Len(t, data, 1) firstDelivery := data[0].(map[string]interface{}) @@ -132,7 +132,7 @@ func TestListDeliveries(t *testing.T) { var response map[string]interface{} require.NoError(t, json.Unmarshal(w.Body.Bytes(), &response)) - data := response["data"].([]interface{}) + data := response["models"].([]interface{}) require.Len(t, data, 1) firstDelivery := data[0].(map[string]interface{}) @@ -151,7 +151,7 @@ func TestListDeliveries(t *testing.T) { var response map[string]interface{} require.NoError(t, json.Unmarshal(w.Body.Bytes(), &response)) - data := response["data"].([]interface{}) + data := response["models"].([]interface{}) assert.Len(t, data, 1) }) @@ -165,7 +165,7 @@ func TestListDeliveries(t *testing.T) { var response map[string]interface{} require.NoError(t, json.Unmarshal(w.Body.Bytes(), &response)) - data := response["data"].([]interface{}) + data := response["models"].([]interface{}) assert.Len(t, data, 0) }) @@ -187,7 +187,7 @@ func TestListDeliveries(t *testing.T) { var response map[string]interface{} require.NoError(t, json.Unmarshal(w.Body.Bytes(), &response)) - data := response["data"].([]interface{}) + data := response["models"].([]interface{}) require.Len(t, data, 1) firstDelivery := data[0].(map[string]interface{}) @@ -239,7 +239,7 @@ func TestListDeliveries(t *testing.T) { var response map[string]interface{} require.NoError(t, json.Unmarshal(w.Body.Bytes(), &response)) - data := response["data"].([]interface{}) + data := response["models"].([]interface{}) // Find the delivery we just created var foundDelivery map[string]interface{} for _, d := range data { @@ -266,7 +266,7 @@ func TestListDeliveries(t *testing.T) { var response map[string]interface{} require.NoError(t, json.Unmarshal(w.Body.Bytes(), &response)) - data := response["data"].([]interface{}) + data := response["models"].([]interface{}) require.GreaterOrEqual(t, len(data), 1) firstDelivery := data[0].(map[string]interface{}) @@ -276,17 +276,17 @@ func TestListDeliveries(t *testing.T) { assert.NotNil(t, event["topic"]) }) - t.Run("should return validation error for invalid sort_order", func(t *testing.T) { + t.Run("should return validation error for invalid dir", func(t *testing.T) { w := httptest.NewRecorder() - req, _ := http.NewRequest("GET", baseAPIPath+"/tenants/"+tenantID+"/deliveries?sort_order=invalid", nil) + req, _ := http.NewRequest("GET", baseAPIPath+"/tenants/"+tenantID+"/deliveries?dir=invalid", nil) result.router.ServeHTTP(w, req) assert.Equal(t, http.StatusUnprocessableEntity, w.Code) }) - t.Run("should accept valid sort_order param", func(t *testing.T) { + t.Run("should accept valid dir param", func(t *testing.T) { w := httptest.NewRecorder() - req, _ := http.NewRequest("GET", baseAPIPath+"/tenants/"+tenantID+"/deliveries?sort_order=asc", nil) + req, _ := http.NewRequest("GET", baseAPIPath+"/tenants/"+tenantID+"/deliveries?dir=asc", nil) result.router.ServeHTTP(w, req) assert.Equal(t, http.StatusOK, w.Code) @@ -540,7 +540,7 @@ func TestListEvents(t *testing.T) { var response map[string]interface{} require.NoError(t, json.Unmarshal(w.Body.Bytes(), &response)) - data := response["data"].([]interface{}) + data := response["models"].([]interface{}) assert.Len(t, data, 0) }) @@ -588,7 +588,7 @@ func TestListEvents(t *testing.T) { var response map[string]interface{} require.NoError(t, json.Unmarshal(w.Body.Bytes(), &response)) - data := response["data"].([]interface{}) + data := response["models"].([]interface{}) assert.Len(t, data, 1) firstEvent := data[0].(map[string]interface{}) @@ -607,7 +607,7 @@ func TestListEvents(t *testing.T) { var response map[string]interface{} require.NoError(t, json.Unmarshal(w.Body.Bytes(), &response)) - data := response["data"].([]interface{}) + data := response["models"].([]interface{}) assert.GreaterOrEqual(t, len(data), 1) }) @@ -621,7 +621,7 @@ func TestListEvents(t *testing.T) { var response map[string]interface{} require.NoError(t, json.Unmarshal(w.Body.Bytes(), &response)) - data := response["data"].([]interface{}) + data := response["models"].([]interface{}) assert.Len(t, data, 0) }) @@ -635,7 +635,7 @@ func TestListEvents(t *testing.T) { var response map[string]interface{} require.NoError(t, json.Unmarshal(w.Body.Bytes(), &response)) - data := response["data"].([]interface{}) + data := response["models"].([]interface{}) assert.GreaterOrEqual(t, len(data), 1) for _, item := range data { event := item.(map[string]interface{}) @@ -651,33 +651,33 @@ func TestListEvents(t *testing.T) { assert.Equal(t, http.StatusNotFound, w.Code) }) - t.Run("should return validation error for invalid start time", func(t *testing.T) { + t.Run("should return validation error for invalid event_time filter", func(t *testing.T) { w := httptest.NewRecorder() - req, _ := http.NewRequest("GET", baseAPIPath+"/tenants/"+tenantID+"/events?start=invalid", nil) + req, _ := http.NewRequest("GET", baseAPIPath+"/tenants/"+tenantID+"/events?event_time[gte]=invalid", nil) result.router.ServeHTTP(w, req) assert.Equal(t, http.StatusUnprocessableEntity, w.Code) }) - t.Run("should return validation error for invalid end time", func(t *testing.T) { + t.Run("should return validation error for invalid event_time lte filter", func(t *testing.T) { w := httptest.NewRecorder() - req, _ := http.NewRequest("GET", baseAPIPath+"/tenants/"+tenantID+"/events?end=invalid", nil) + req, _ := http.NewRequest("GET", baseAPIPath+"/tenants/"+tenantID+"/events?event_time[lte]=invalid", nil) result.router.ServeHTTP(w, req) assert.Equal(t, http.StatusUnprocessableEntity, w.Code) }) - t.Run("should return validation error for invalid sort_order", func(t *testing.T) { + t.Run("should return validation error for invalid dir", func(t *testing.T) { w := httptest.NewRecorder() - req, _ := http.NewRequest("GET", baseAPIPath+"/tenants/"+tenantID+"/events?sort_order=invalid", nil) + req, _ := http.NewRequest("GET", baseAPIPath+"/tenants/"+tenantID+"/events?dir=invalid", nil) result.router.ServeHTTP(w, req) assert.Equal(t, http.StatusUnprocessableEntity, w.Code) }) - t.Run("should accept valid sort_order param", func(t *testing.T) { + t.Run("should accept valid dir param", func(t *testing.T) { w := httptest.NewRecorder() - req, _ := http.NewRequest("GET", baseAPIPath+"/tenants/"+tenantID+"/events?sort_order=asc", nil) + req, _ := http.NewRequest("GET", baseAPIPath+"/tenants/"+tenantID+"/events?dir=asc", nil) result.router.ServeHTTP(w, req) assert.Equal(t, http.StatusOK, w.Code) diff --git a/internal/apirouter/tenant_handlers.go b/internal/apirouter/tenant_handlers.go index 0a4a5e492..998a3bee2 100644 --- a/internal/apirouter/tenant_handlers.go +++ b/internal/apirouter/tenant_handlers.go @@ -96,11 +96,35 @@ func (h *TenantHandlers) Retrieve(c *gin.Context) { } func (h *TenantHandlers) List(c *gin.Context) { - // Parse query parameters + // Parse and validate dir (sort direction) + dir, errResp := ParseDir(c) + if errResp != nil { + AbortWithError(c, errResp.Code, *errResp) + return + } + + // Parse and validate order_by (only created_at is allowed for tenants) + orderBy, errResp := ParseOrderBy(c, []string{"created_at"}) + if errResp != nil { + AbortWithError(c, errResp.Code, *errResp) + return + } + // order_by is informational only for tenants (always sorts by created_at) + _ = orderBy + + // Parse created_at date filters + createdAtFilter, errResp := ParseDateFilter(c, "created_at") + if errResp != nil { + AbortWithError(c, errResp.Code, *errResp) + return + } + req := models.ListTenantRequest{ - Next: c.Query("next"), - Prev: c.Query("prev"), - Order: c.Query("order"), + Next: c.Query("next"), + Prev: c.Query("prev"), + Dir: dir, + CreatedAtGTE: createdAtFilter.GTE, + CreatedAtLTE: createdAtFilter.LTE, } // Parse limit if provided diff --git a/internal/models/entity.go b/internal/models/entity.go index ecf776ed8..4c16a029e 100644 --- a/internal/models/entity.go +++ b/internal/models/entity.go @@ -45,18 +45,29 @@ var ( // ListTenantRequest contains parameters for listing tenants. type ListTenantRequest struct { - Limit int // Number of results per page (default: 20) - Next string // Cursor for next page - Prev string // Cursor for previous page - Order string // Sort order: "asc" or "desc" (default: "desc") + Limit int // Number of results per page (default: 20) + Next string // Cursor for next page + Prev string // Cursor for previous page + Dir string // Sort direction: "asc" or "desc" (default: "desc") + CreatedAtGTE *time.Time // Filter: created_at >= this time + CreatedAtLTE *time.Time // Filter: created_at <= this time +} + +// TenantPaginationInfo represents pagination metadata for tenant list responses. +// This aligns with Hookdeck's pagination response format. +type TenantPaginationInfo struct { + OrderBy string `json:"order_by"` + Dir string `json:"dir"` + Limit int `json:"limit"` + Next *string `json:"next"` + Prev *string `json:"prev"` } // ListTenantResponse contains the paginated list of tenants. type ListTenantResponse struct { - Data []Tenant `json:"data"` - Next string `json:"next"` - Prev string `json:"prev"` - Count int `json:"count"` + Models []Tenant `json:"models"` + Pagination TenantPaginationInfo `json:"pagination"` + Count int `json:"count"` } type entityStoreImpl struct { @@ -350,12 +361,12 @@ func (s *entityStoreImpl) ListTenant(ctx context.Context, req ListTenantRequest) limit = maxListTenantLimit } - // Validate and apply order - order := req.Order - if order == "" { - order = "desc" + // Validate and apply dir (sort direction) + dir := req.Dir + if dir == "" { + dir = "desc" } - if order != "asc" && order != "desc" { + if dir != "asc" && dir != "desc" { return nil, ErrInvalidOrder } @@ -364,10 +375,18 @@ func (s *entityStoreImpl) ListTenant(ctx context.Context, req ListTenantRequest) // Filter: -@deleted_at:[1 +inf] excludes deleted tenants baseFilter := "@entity:{tenant} -@deleted_at:[1 +inf]" + // Add created_at date filters if provided + if req.CreatedAtGTE != nil { + baseFilter = fmt.Sprintf("@created_at:[%d +inf] %s", req.CreatedAtGTE.UnixMilli(), baseFilter) + } + if req.CreatedAtLTE != nil { + baseFilter = fmt.Sprintf("@created_at:[-inf %d] %s", req.CreatedAtLTE.UnixMilli(), baseFilter) + } + // Use pagination package for cursor-based pagination with n+1 pattern result, err := pagination.Run(ctx, pagination.Config[Tenant]{ Limit: limit, - Order: order, + Order: dir, Next: req.Next, Prev: req.Prev, Cursor: pagination.Cursor[Tenant]{ @@ -424,11 +443,25 @@ func (s *entityStoreImpl) ListTenant(ctx context.Context, req ListTenantRequest) _, totalCount, _ = s.parseSearchResult(ctx, countResult) } + // Convert empty cursors to nil pointers (Hookdeck returns null for empty cursors) + var nextCursor, prevCursor *string + if result.Next != "" { + nextCursor = &result.Next + } + if result.Prev != "" { + prevCursor = &result.Prev + } + return &ListTenantResponse{ - Data: tenants, + Models: tenants, + Pagination: TenantPaginationInfo{ + OrderBy: "created_at", + Dir: dir, + Limit: limit, + Next: nextCursor, + Prev: prevCursor, + }, Count: totalCount, - Next: result.Next, - Prev: result.Prev, }, nil } diff --git a/internal/models/entity_test.go b/internal/models/entity_test.go index 4cb489948..d6c8f3ab8 100644 --- a/internal/models/entity_test.go +++ b/internal/models/entity_test.go @@ -243,17 +243,25 @@ func runListTenantPaginationSuite(t *testing.T, factory RedisClientFactory, depl List: func(ctx context.Context, opts paginationtest.ListOpts) (paginationtest.ListResult[models.Tenant], error) { resp, err := entityStore.ListTenant(ctx, models.ListTenantRequest{ Limit: opts.Limit, - Order: opts.Order, + Dir: opts.Order, Next: opts.Next, Prev: opts.Prev, }) if err != nil { return paginationtest.ListResult[models.Tenant]{}, err } + // Convert *string cursors to string for test framework + var next, prev string + if resp.Pagination.Next != nil { + next = *resp.Pagination.Next + } + if resp.Pagination.Prev != nil { + prev = *resp.Pagination.Prev + } return paginationtest.ListResult[models.Tenant]{ - Items: resp.Data, - Next: resp.Next, - Prev: resp.Prev, + Items: resp.Models, + Next: next, + Prev: prev, }, nil }, diff --git a/internal/models/entitysuite_test.go b/internal/models/entitysuite_test.go index 2378c783e..17b6a6986 100644 --- a/internal/models/entitysuite_test.go +++ b/internal/models/entitysuite_test.go @@ -1181,9 +1181,9 @@ func (s *ListTenantTestSuite) SetupSuite() { // Test empty list BEFORE creating any data resp, err := s.entityStore.ListTenant(s.ctx, models.ListTenantRequest{}) require.NoError(s.T(), err) - require.Empty(s.T(), resp.Data, "should be empty before creating data") - require.Empty(s.T(), resp.Next) - require.Empty(s.T(), resp.Prev) + require.Empty(s.T(), resp.Models, "should be empty before creating data") + require.Nil(s.T(), resp.Pagination.Next) + require.Nil(s.T(), resp.Pagination.Prev) // Create 25 tenants for pagination tests s.tenants = make([]models.Tenant, 25) @@ -1218,10 +1218,14 @@ func (s *ListTenantTestSuite) TestListTenantEnrichment() { resp1, err := s.entityStore.ListTenant(s.ctx, models.ListTenantRequest{Limit: 2}) require.NoError(t, err) assert.Equal(t, 25, resp1.Count, "count should be total tenants, not page size") - assert.Len(t, resp1.Data, 2, "data should respect limit") + assert.Len(t, resp1.Models, 2, "data should respect limit") // Second page - count should still be total - resp2, err := s.entityStore.ListTenant(s.ctx, models.ListTenantRequest{Limit: 2, Next: resp1.Next}) + var nextCursor string + if resp1.Pagination.Next != nil { + nextCursor = *resp1.Pagination.Next + } + resp2, err := s.entityStore.ListTenant(s.ctx, models.ListTenantRequest{Limit: 2, Next: nextCursor}) require.NoError(t, err) assert.Equal(t, 25, resp2.Count, "count should remain total across pages") }) @@ -1231,7 +1235,7 @@ func (s *ListTenantTestSuite) TestListTenantEnrichment() { require.NoError(t, err) assert.Equal(t, 25, resp.Count, "count should be tenants only") - for _, tenant := range resp.Data { + for _, tenant := range resp.Models { assert.NotContains(t, tenant.ID, "dest_", "destination should not appear in tenant list") } }) @@ -1242,9 +1246,9 @@ func (s *ListTenantTestSuite) TestListTenantEnrichment() { // tenantWithDests has 2 destinations from SetupSuite var tenantWithDests *models.Tenant - for i := range resp.Data { - if resp.Data[i].ID == s.tenantWithDests.ID { - tenantWithDests = &resp.Data[i] + for i := range resp.Models { + if resp.Models[i].ID == s.tenantWithDests.ID { + tenantWithDests = &resp.Models[i] break } } @@ -1254,9 +1258,9 @@ func (s *ListTenantTestSuite) TestListTenantEnrichment() { // Verify tenants without destinations have 0 count var tenantWithoutDests *models.Tenant - for i := range resp.Data { - if resp.Data[i].ID != s.tenantWithDests.ID { - tenantWithoutDests = &resp.Data[i] + for i := range resp.Models { + if resp.Models[i].ID != s.tenantWithDests.ID { + tenantWithoutDests = &resp.Models[i] break } } @@ -1293,7 +1297,7 @@ func (s *ListTenantTestSuite) TestListTenantExcludesDeleted() { assert.Equal(t, initialCount+1, resp.Count) // Verify deleted tenant is not in results - for _, tenant := range resp.Data { + for _, tenant := range resp.Models { assert.NotEqual(t, tenant1.ID, tenant.ID, "deleted tenant should not appear") } @@ -1328,15 +1332,15 @@ func (s *ListTenantTestSuite) TestListTenantExcludesDeleted() { // NOT the deleted ones (index 3,4) which would have been first if not filtered resp, err := s.entityStore.ListTenant(s.ctx, models.ListTenantRequest{ Limit: 2, - Order: "desc", + Dir: "desc", }) require.NoError(t, err) - require.GreaterOrEqual(t, len(resp.Data), 2, "should get at least 2 tenants") + require.GreaterOrEqual(t, len(resp.Models), 2, "should get at least 2 tenants") // Verify deleted tenants don't appear in the first 2 results - for i := 0; i < 2 && i < len(resp.Data); i++ { - assert.NotEqual(t, tenantIDs[3], resp.Data[i].ID, "deleted tenant should not appear") - assert.NotEqual(t, tenantIDs[4], resp.Data[i].ID, "deleted tenant should not appear") + for i := 0; i < 2 && i < len(resp.Models); i++ { + assert.NotEqual(t, tenantIDs[3], resp.Models[i].ID, "deleted tenant should not appear") + assert.NotEqual(t, tenantIDs[4], resp.Models[i].ID, "deleted tenant should not appear") } // Cleanup @@ -1370,10 +1374,10 @@ func (s *ListTenantTestSuite) TestListTenantKeysetPagination() { // Fetch page 1 (items 14, 13, 12, 11, 10 with DESC order) resp1, err := s.entityStore.ListTenant(s.ctx, models.ListTenantRequest{Limit: 5}) require.NoError(t, err) - require.Len(t, resp1.Data, 5, "page 1 should have 5 items") + require.Len(t, resp1.Models, 5, "page 1 should have 5 items") // Verify we got our test tenants - for _, tenant := range resp1.Data { + for _, tenant := range resp1.Models { require.Contains(t, tenant.ID, prefix, "page 1 should contain our test tenants") } @@ -1391,19 +1395,23 @@ func (s *ListTenantTestSuite) TestListTenantKeysetPagination() { // With keyset pagination, the cursor is the timestamp of item 10 // Page 2 will get items with timestamp < cursor, so items 9, 8, 7, 6, 5 // The new tenant has a newer timestamp so it won't appear on page 2 + var nextCursor string + if resp1.Pagination.Next != nil { + nextCursor = *resp1.Pagination.Next + } resp2, err := s.entityStore.ListTenant(s.ctx, models.ListTenantRequest{ Limit: 5, - Next: resp1.Next, + Next: nextCursor, }) require.NoError(t, err) - require.NotEmpty(t, resp2.Data, "page 2 should have items") + require.NotEmpty(t, resp2.Models, "page 2 should have items") // Verify no duplicates - first item on page 2 should NOT be the last from page 1 page1IDs := make(map[string]bool) - for _, tenant := range resp1.Data { + for _, tenant := range resp1.Models { page1IDs[tenant.ID] = true } - for _, tenant := range resp2.Data { + for _, tenant := range resp2.Models { assert.False(t, page1IDs[tenant.ID], "keyset pagination: no duplicates when adding during traversal, but found %s", tenant.ID) } @@ -1417,9 +1425,9 @@ func (s *ListTenantTestSuite) TestListTenantKeysetPagination() { // TestListTenantInputValidation tests input validation and error handling. func (s *ListTenantTestSuite) TestListTenantInputValidation() { - s.T().Run("invalid order returns error", func(t *testing.T) { + s.T().Run("invalid dir returns error", func(t *testing.T) { _, err := s.entityStore.ListTenant(s.ctx, models.ListTenantRequest{ - Order: "invalid", + Dir: "invalid", }) require.Error(t, err) assert.ErrorIs(t, err, models.ErrInvalidOrder) @@ -1466,7 +1474,7 @@ func (s *ListTenantTestSuite) TestListTenantInputValidation() { }) require.NoError(t, err) // Default limit is 20, should return up to 20 of the 25 tenants - assert.Equal(t, 20, len(resp.Data), "default limit should be 20") + assert.Equal(t, 20, len(resp.Models), "default limit should be 20") assert.Equal(t, 25, resp.Count, "total count should be 25") }) @@ -1488,22 +1496,22 @@ func (s *ListTenantTestSuite) TestListTenantInputValidation() { require.NoError(t, err) // Should succeed and return all 25 (capped to 100, but we only have 25) assert.NotNil(t, resp) - assert.Equal(t, 25, len(resp.Data), "should return all 25 tenants") + assert.Equal(t, 25, len(resp.Models), "should return all 25 tenants") assert.Equal(t, 25, resp.Count) }) - s.T().Run("empty order uses default desc", func(t *testing.T) { + s.T().Run("empty dir uses default desc", func(t *testing.T) { // Uses 25 tenants from SetupSuite created with sequential timestamps // s.tenants[0] is oldest, s.tenants[24] is newest resp, err := s.entityStore.ListTenant(s.ctx, models.ListTenantRequest{ - Order: "", // Should default to "desc" + Dir: "", // Should default to "desc" }) require.NoError(t, err) - require.Len(t, resp.Data, 20, "default limit is 20") + require.Len(t, resp.Models, 20, "default limit is 20") - // With DESC order, newest (s.tenants[24]) should be first + // With DESC dir, newest (s.tenants[24]) should be first // and older tenants should follow - assert.Equal(t, s.tenants[24].ID, resp.Data[0].ID, "newest tenant should be first with desc order") - assert.Equal(t, s.tenants[23].ID, resp.Data[1].ID, "second newest should be second") + assert.Equal(t, s.tenants[24].ID, resp.Models[0].ID, "newest tenant should be first with desc dir") + assert.Equal(t, s.tenants[23].ID, resp.Models[1].ID, "second newest should be second") }) } From a532e61f25004ae40b0f605f48fd9ea1625de8d3 Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Tue, 20 Jan 2026 01:17:16 +0700 Subject: [PATCH 06/14] refactor: remove created_at filtering from ListTenant ListTenant does not support date filtering - only basic pagination with dir parameter. Co-Authored-By: Claude Opus 4.5 --- internal/apirouter/tenant_handlers.go | 24 +++--------------------- internal/models/entity.go | 18 ++++-------------- 2 files changed, 7 insertions(+), 35 deletions(-) diff --git a/internal/apirouter/tenant_handlers.go b/internal/apirouter/tenant_handlers.go index 998a3bee2..b1fd33eb2 100644 --- a/internal/apirouter/tenant_handlers.go +++ b/internal/apirouter/tenant_handlers.go @@ -103,28 +103,10 @@ func (h *TenantHandlers) List(c *gin.Context) { return } - // Parse and validate order_by (only created_at is allowed for tenants) - orderBy, errResp := ParseOrderBy(c, []string{"created_at"}) - if errResp != nil { - AbortWithError(c, errResp.Code, *errResp) - return - } - // order_by is informational only for tenants (always sorts by created_at) - _ = orderBy - - // Parse created_at date filters - createdAtFilter, errResp := ParseDateFilter(c, "created_at") - if errResp != nil { - AbortWithError(c, errResp.Code, *errResp) - return - } - req := models.ListTenantRequest{ - Next: c.Query("next"), - Prev: c.Query("prev"), - Dir: dir, - CreatedAtGTE: createdAtFilter.GTE, - CreatedAtLTE: createdAtFilter.LTE, + Next: c.Query("next"), + Prev: c.Query("prev"), + Dir: dir, } // Parse limit if provided diff --git a/internal/models/entity.go b/internal/models/entity.go index 4c16a029e..9a5fc2736 100644 --- a/internal/models/entity.go +++ b/internal/models/entity.go @@ -45,12 +45,10 @@ var ( // ListTenantRequest contains parameters for listing tenants. type ListTenantRequest struct { - Limit int // Number of results per page (default: 20) - Next string // Cursor for next page - Prev string // Cursor for previous page - Dir string // Sort direction: "asc" or "desc" (default: "desc") - CreatedAtGTE *time.Time // Filter: created_at >= this time - CreatedAtLTE *time.Time // Filter: created_at <= this time + Limit int // Number of results per page (default: 20) + Next string // Cursor for next page + Prev string // Cursor for previous page + Dir string // Sort direction: "asc" or "desc" (default: "desc") } // TenantPaginationInfo represents pagination metadata for tenant list responses. @@ -375,14 +373,6 @@ func (s *entityStoreImpl) ListTenant(ctx context.Context, req ListTenantRequest) // Filter: -@deleted_at:[1 +inf] excludes deleted tenants baseFilter := "@entity:{tenant} -@deleted_at:[1 +inf]" - // Add created_at date filters if provided - if req.CreatedAtGTE != nil { - baseFilter = fmt.Sprintf("@created_at:[%d +inf] %s", req.CreatedAtGTE.UnixMilli(), baseFilter) - } - if req.CreatedAtLTE != nil { - baseFilter = fmt.Sprintf("@created_at:[-inf %d] %s", req.CreatedAtLTE.UnixMilli(), baseFilter) - } - // Use pagination package for cursor-based pagination with n+1 pattern result, err := pagination.Run(ctx, pagination.Config[Tenant]{ Limit: limit, From fb4565d338585b65509d3e62659c29f8ea32a768 Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Tue, 20 Jan 2026 01:36:15 +0700 Subject: [PATCH 07/14] docs: update OpenAPI spec and code for new pagination format - Rename list responses to {Entity}PaginatedResult pattern - Rename PaginationInfo to SeekPagination - Update query params: dir, order_by, bracket notation dates - Add APIErrorResponse and EventPaginatedResult schemas Co-Authored-By: Claude Opus 4.5 --- docs/apis/openapi.yaml | 427 +++++++++++++++++------------ internal/apirouter/log_handlers.go | 20 +- internal/apirouter/pagination.go | 7 +- internal/models/entity.go | 23 +- internal/models/tenant.go | 19 -- 5 files changed, 279 insertions(+), 217 deletions(-) diff --git a/docs/apis/openapi.yaml b/docs/apis/openapi.yaml index 3d4bbb37d..a37b5ffb6 100644 --- a/docs/apis/openapi.yaml +++ b/docs/apis/openapi.yaml @@ -86,59 +86,17 @@ components: type: string nullable: true description: Optional metadata to store with the tenant. - TenantListItem: - type: object - description: Tenant object returned in list operations. - properties: - id: - type: string - description: User-defined system ID for the tenant. - example: "123" - destinations_count: - type: integer - description: Number of destinations associated with the tenant. - example: 5 - topics: - type: array - items: - type: string - description: List of subscribed topics across all destinations for this tenant. - example: ["user.created", "user.deleted"] - metadata: - type: object - additionalProperties: - type: string - nullable: true - description: Arbitrary key-value pairs for storing contextual information about the tenant. - created_at: - type: string - format: date-time - description: ISO Date when the tenant was created. - example: "2024-01-01T00:00:00Z" - updated_at: - type: string - format: date-time - description: ISO Date when the tenant was last updated. - example: "2024-01-01T00:00:00Z" - TenantListResponse: + TenantPaginatedResult: type: object description: Paginated list of tenants. properties: - data: + models: type: array items: - $ref: "#/components/schemas/TenantListItem" + $ref: "#/components/schemas/Tenant" description: Array of tenant objects. - next: - type: string - nullable: true - description: Cursor for the next page of results. Null if no more results. - example: "MTcwNDA2NzIwMA==" - prev: - type: string - nullable: true - description: Cursor for the previous page of results. Null if on first page. - example: null + pagination: + $ref: "#/components/schemas/SeekPagination" count: type: integer description: Total count of all tenants. @@ -198,26 +156,57 @@ components: customer: tier: "premium" - PaginatedResponse: + SeekPagination: type: object - required: [count, data, next, prev] + description: Cursor-based pagination metadata for list responses. properties: - count: + order_by: + type: string + description: The field being sorted on. + example: "created_at" + dir: + type: string + enum: [asc, desc] + description: Sort direction. + example: "desc" + limit: type: integer - description: Total number of items across all pages - example: 42 - data: - type: array - items: {} # Will be overridden by specific endpoints - description: Array of items for current page + description: Page size limit. + example: 100 next: type: string - description: Cursor for next page (empty string if no next page) - example: "" + nullable: true + description: Cursor for the next page of results. Null if no more results. + example: "MTcwNDA2NzIwMA==" prev: type: string - description: Cursor for previous page (empty string if no previous page) - example: "" + nullable: true + description: Cursor for the previous page of results. Null if on first page. + example: null + + APIErrorResponse: + type: object + description: Standard error response format. + properties: + status: + type: integer + description: HTTP status code. + example: 422 + message: + type: string + description: Human-readable error message. + example: "validation error" + data: + description: Additional error details. For validation errors, this is an array of human-readable messages. + oneOf: + - type: array + items: + type: string + description: Array of validation error messages. + example: ["email is required", "password must be at least 6 characters"] + - type: object + additionalProperties: true + description: Additional contextual data about the error. # Destination Type Specific Config/Credentials Schemas WebhookConfig: @@ -1878,23 +1867,29 @@ components: additionalProperties: true description: The event payload data. example: { "user_id": "userid", "status": "active" } - DeliveryListResponse: + DeliveryPaginatedResult: type: object description: Paginated list of deliveries. properties: - data: + models: type: array items: $ref: "#/components/schemas/Delivery" description: Array of delivery objects. - next: - type: string - description: Cursor for the next page of results. Empty string if no more results. - example: "MTcwNDA2NzIwMA==" - prev: - type: string - description: Cursor for the previous page of results. Empty string if on first page. - example: "" + pagination: + $ref: "#/components/schemas/SeekPagination" + + EventPaginatedResult: + type: object + description: Paginated list of events. + properties: + models: + type: array + items: + $ref: "#/components/schemas/Event" + description: Array of event objects. + pagination: + $ref: "#/components/schemas/SeekPagination" # Destination Type Schema (for Metadata endpoint) DestinationType: @@ -2174,14 +2169,36 @@ paths: maximum: 100 default: 20 description: Number of tenants to return per page (1-100, default 20). - - name: order + - name: order_by + in: query + required: false + schema: + type: string + enum: [created_at] + default: created_at + description: Field to sort by. + - name: dir in: query required: false schema: type: string enum: [asc, desc] default: desc - description: Sort order by `created_at` timestamp. + description: Sort direction. + - name: created_at[gte] + in: query + required: false + schema: + type: string + format: date-time + description: Filter tenants created at or after this time (RFC3339 or YYYY-MM-DD format). + - name: created_at[lte] + in: query + required: false + schema: + type: string + format: date-time + description: Filter tenants created at or before this time (RFC3339 or YYYY-MM-DD format). - name: next in: query required: false @@ -2200,9 +2217,9 @@ paths: content: application/json: schema: - $ref: "#/components/schemas/TenantListResponse" + $ref: "#/components/schemas/TenantPaginatedResult" example: - data: + models: - id: "tenant_123" metadata: plan: "pro" @@ -2212,8 +2229,12 @@ paths: metadata: null created_at: "2024-01-14T09:00:00Z" updated_at: "2024-01-14T09:00:00Z" - next: "MTcwNDA2NzIwMA==" - prev: null + pagination: + order_by: "created_at" + dir: "desc" + limit: 20 + next: "MTcwNDA2NzIwMA==" + prev: null count: 42 "400": description: Invalid request parameters (e.g., invalid cursor, both next and prev provided). @@ -2341,20 +2362,20 @@ paths: items: type: string description: Filter events by topic(s). Can be specified multiple times or comma-separated. - - name: start + - name: event_time[gte] in: query required: false schema: type: string format: date-time - description: Filter events with time >= start (RFC3339 format). - - name: end + description: Filter events with time >= value (RFC3339 or YYYY-MM-DD format). + - name: event_time[lte] in: query required: false schema: type: string format: date-time - description: Filter events with time <= end (RFC3339 format). + description: Filter events with time <= value (RFC3339 or YYYY-MM-DD format). - name: limit in: query required: false @@ -2376,32 +2397,33 @@ paths: schema: type: string description: Cursor for previous page of results. - - name: sort_order + - name: order_by + in: query + required: false + schema: + type: string + enum: [event_time, created_at] + default: event_time + description: Field to sort by. + - name: dir in: query required: false schema: type: string enum: [asc, desc] default: desc - description: Sort order (ascending or descending). + description: Sort direction. responses: "200": description: A paginated list of events. content: application/json: schema: - allOf: - - $ref: "#/components/schemas/PaginatedResponse" - - type: object - properties: - data: - type: array - items: - $ref: "#/components/schemas/Event" + $ref: "#/components/schemas/EventPaginatedResult" examples: AdminEventsListExample: value: - data: + models: - id: "evt_123" topic: "user.created" time: "2024-01-01T00:00:00Z" @@ -2414,12 +2436,20 @@ paths: eligible_for_retry: true metadata: { "source": "oms" } data: { "order_id": "orderid", "tracking": "1Z..." } - next: "MTcwNDA2NzIwMA==" - prev: "" + pagination: + order_by: "event_time" + dir: "desc" + limit: 100 + next: "MTcwNDA2NzIwMA==" + prev: null "401": description: Unauthorized (Admin API Key missing or invalid). "422": description: Validation error (invalid query parameters). + content: + application/json: + schema: + $ref: "#/components/schemas/APIErrorResponse" /deliveries: get: @@ -2468,34 +2498,34 @@ paths: items: type: string description: Filter deliveries by event topic(s). Can be specified multiple times or comma-separated. - - name: start + - name: delivery_time[gte] in: query required: false schema: type: string format: date-time - description: Filter deliveries with delivered_at >= start (RFC3339 format). - - name: end + description: Filter deliveries with delivered_at >= value (RFC3339 or YYYY-MM-DD format). + - name: delivery_time[lte] in: query required: false schema: type: string format: date-time - description: Filter deliveries with delivered_at <= end (RFC3339 format). - - name: event_start + description: Filter deliveries with delivered_at <= value (RFC3339 or YYYY-MM-DD format). + - name: event_time[gte] in: query required: false schema: type: string format: date-time - description: Filter deliveries by event time >= event_start (RFC3339 format). - - name: event_end + description: Filter deliveries by event time >= value (RFC3339 or YYYY-MM-DD format). + - name: event_time[lte] in: query required: false schema: type: string format: date-time - description: Filter deliveries by event time <= event_end (RFC3339 format). + description: Filter deliveries by event time <= value (RFC3339 or YYYY-MM-DD format). - name: limit in: query required: false @@ -2531,33 +2561,33 @@ paths: - `event`: Include event summary (id, topic, time, eligible_for_retry, metadata) - `event.data`: Include full event with payload data - `response_data`: Include response body and headers - - name: sort_by + - name: order_by in: query required: false schema: type: string enum: [delivery_time, event_time] default: delivery_time - description: Sort results by delivery time or event time. - - name: sort_order + description: Field to sort by. + - name: dir in: query required: false schema: type: string enum: [asc, desc] default: desc - description: Sort order (ascending or descending). + description: Sort direction. responses: "200": description: A paginated list of deliveries. content: application/json: schema: - $ref: "#/components/schemas/DeliveryListResponse" + $ref: "#/components/schemas/DeliveryPaginatedResult" examples: AdminDeliveriesListExample: value: - data: + models: - id: "del_123" status: "success" delivered_at: "2024-01-01T00:00:05Z" @@ -2572,12 +2602,16 @@ paths: attempt: 2 event: "evt_789" destination: "des_789" - next: "MTcwNDA2NzIwMA==" - prev: "" + pagination: + order_by: "delivery_time" + dir: "desc" + limit: 100 + next: "MTcwNDA2NzIwMA==" + prev: null AdminDeliveriesWithIncludeExample: summary: Response with include=event value: - data: + models: - id: "del_123" status: "success" delivered_at: "2024-01-01T00:00:05Z" @@ -2590,12 +2624,20 @@ paths: eligible_for_retry: false metadata: { "source": "crm" } destination: "des_456" - next: "" - prev: "" + pagination: + order_by: "delivery_time" + dir: "desc" + limit: 100 + next: null + prev: null "401": description: Unauthorized (Admin API Key missing or invalid). "422": description: Validation error (invalid query parameters). + content: + application/json: + schema: + $ref: "#/components/schemas/APIErrorResponse" /tenants/{tenant_id}/portal: parameters: @@ -3318,34 +3360,34 @@ paths: items: type: string description: Filter deliveries by event topic(s). Can be specified multiple times or comma-separated. - - name: start + - name: delivery_time[gte] in: query required: false schema: type: string format: date-time - description: Filter deliveries with delivered_at >= start (RFC3339 format). - - name: end + description: Filter deliveries with delivered_at >= value (RFC3339 or YYYY-MM-DD format). + - name: delivery_time[lte] in: query required: false schema: type: string format: date-time - description: Filter deliveries with delivered_at <= end (RFC3339 format). - - name: event_start + description: Filter deliveries with delivered_at <= value (RFC3339 or YYYY-MM-DD format). + - name: event_time[gte] in: query required: false schema: type: string format: date-time - description: Filter deliveries by event time >= event_start (RFC3339 format). - - name: event_end + description: Filter deliveries by event time >= value (RFC3339 or YYYY-MM-DD format). + - name: event_time[lte] in: query required: false schema: type: string format: date-time - description: Filter deliveries by event time <= event_end (RFC3339 format). + description: Filter deliveries by event time <= value (RFC3339 or YYYY-MM-DD format). - name: limit in: query required: false @@ -3381,33 +3423,33 @@ paths: - `event`: Include event summary (id, topic, time, eligible_for_retry, metadata) - `event.data`: Include full event with payload data - `response_data`: Include response body and headers - - name: sort_by + - name: order_by in: query required: false schema: type: string enum: [delivery_time, event_time] default: delivery_time - description: Sort results by delivery time or event time. - - name: sort_order + description: Field to sort by. + - name: dir in: query required: false schema: type: string enum: [asc, desc] default: desc - description: Sort order (ascending or descending). + description: Sort direction. responses: "200": description: A paginated list of deliveries. content: application/json: schema: - $ref: "#/components/schemas/DeliveryListResponse" + $ref: "#/components/schemas/DeliveryPaginatedResult" examples: DeliveriesListExample: value: - data: + models: - id: "del_123" status: "success" delivered_at: "2024-01-01T00:00:05Z" @@ -3422,12 +3464,16 @@ paths: attempt: 2 event: "evt_789" destination: "des_456" - next: "MTcwNDA2NzIwMA==" - prev: "" + pagination: + order_by: "delivery_time" + dir: "desc" + limit: 100 + next: "MTcwNDA2NzIwMA==" + prev: null DeliveriesWithIncludeExample: summary: Response with include=event value: - data: + models: - id: "del_123" status: "success" delivered_at: "2024-01-01T00:00:05Z" @@ -3440,12 +3486,20 @@ paths: eligible_for_retry: false metadata: { "source": "crm" } destination: "des_456" - next: "" - prev: "" + pagination: + order_by: "delivery_time" + dir: "desc" + limit: 100 + next: null + prev: null "404": description: Tenant not found. "422": description: Validation error (invalid query parameters). + content: + application/json: + schema: + $ref: "#/components/schemas/APIErrorResponse" /tenants/{tenant_id}/deliveries/{delivery_id}: parameters: @@ -3565,7 +3619,7 @@ paths: get: tags: [Events] summary: List Events - description: Retrieves a list of events for the tenant, supporting cursor navigation (details TBD) and filtering. + description: Retrieves a list of events for the tenant, supporting cursor navigation and filtering. operationId: listTenantEvents parameters: - name: destination_id @@ -3590,13 +3644,13 @@ paths: required: false schema: type: string - description: Cursor for next page of results + description: Cursor for next page of results. - name: prev in: query required: false schema: type: string - description: Cursor for previous page of results + description: Cursor for previous page of results. - name: limit in: query required: false @@ -3605,40 +3659,48 @@ paths: default: 100 minimum: 1 maximum: 1000 - description: Number of items per page (default 100, max 1000) - - name: start + description: Number of items per page (default 100, max 1000). + - name: event_time[gte] in: query required: false schema: type: string format: date-time - description: Start time filter (RFC3339 format) - - name: end + description: Filter events with time >= value (RFC3339 or YYYY-MM-DD format). + - name: event_time[lte] in: query required: false schema: type: string format: date-time - description: End time filter (RFC3339 format) + description: Filter events with time <= value (RFC3339 or YYYY-MM-DD format). + - name: order_by + in: query + required: false + schema: + type: string + enum: [event_time, created_at] + default: event_time + description: Field to sort by. + - name: dir + in: query + required: false + schema: + type: string + enum: [asc, desc] + default: desc + description: Sort direction. responses: "200": description: A paginated list of events. content: application/json: schema: - allOf: - - $ref: "#/components/schemas/PaginatedResponse" - - type: object - properties: - data: - type: array - items: - $ref: "#/components/schemas/Event" + $ref: "#/components/schemas/EventPaginatedResult" examples: EventsListExample: value: - count: 2 - data: + models: - id: "evt_123" destination_id: "des_456" topic: "user.created" @@ -3653,11 +3715,20 @@ paths: successful_at: null metadata: { "source": "oms" } data: { "order_id": "orderid", "tracking": "1Z..." } - next: "" - prev: "" + pagination: + order_by: "event_time" + dir: "desc" + limit: 100 + next: null + prev: null "404": description: Tenant not found. - # Add other error responses + "422": + description: Validation error (invalid query parameters). + content: + application/json: + schema: + $ref: "#/components/schemas/APIErrorResponse" /tenants/{tenant_id}/events/{event_id}: parameters: @@ -3778,13 +3849,13 @@ paths: required: false schema: type: string - description: Cursor for next page of results + description: Cursor for next page of results. - name: prev in: query required: false schema: type: string - description: Cursor for previous page of results + description: Cursor for previous page of results. - name: limit in: query required: false @@ -3793,40 +3864,48 @@ paths: default: 100 minimum: 1 maximum: 1000 - description: Number of items per page (default 100, max 1000) - - name: start + description: Number of items per page (default 100, max 1000). + - name: event_time[gte] in: query required: false schema: type: string format: date-time - description: Start time filter (RFC3339 format) - - name: end + description: Filter events with time >= value (RFC3339 or YYYY-MM-DD format). + - name: event_time[lte] in: query required: false schema: type: string format: date-time - description: End time filter (RFC3339 format) + description: Filter events with time <= value (RFC3339 or YYYY-MM-DD format). + - name: order_by + in: query + required: false + schema: + type: string + enum: [event_time, created_at] + default: event_time + description: Field to sort by. + - name: dir + in: query + required: false + schema: + type: string + enum: [asc, desc] + default: desc + description: Sort direction. responses: "200": description: A paginated list of events for the destination. content: application/json: schema: - allOf: - - $ref: "#/components/schemas/PaginatedResponse" - - type: object - properties: - data: - type: array - items: - $ref: "#/components/schemas/Event" + $ref: "#/components/schemas/EventPaginatedResult" examples: - EventsListExample: # Same as /{tenant_id}/events example + EventsListExample: value: - count: 2 - data: + models: - id: "evt_123" destination_id: "des_456" topic: "user.created" @@ -3841,8 +3920,12 @@ paths: successful_at: null metadata: { "source": "oms" } data: { "order_id": "orderid", "tracking": "1Z..." } - next: "" - prev: "" + pagination: + order_by: "event_time" + dir: "desc" + limit: 100 + next: null + prev: null "404": description: Tenant or Destination not found. diff --git a/internal/apirouter/log_handlers.go b/internal/apirouter/log_handlers.go index 5d0765c4e..b1da4ffcb 100644 --- a/internal/apirouter/log_handlers.go +++ b/internal/apirouter/log_handlers.go @@ -131,16 +131,16 @@ type APIEvent struct { Data map[string]interface{} `json:"data,omitempty"` } -// ListDeliveriesResponse is the response for ListDeliveries -type ListDeliveriesResponse struct { +// DeliveryPaginatedResult is the paginated response for listing deliveries. +type DeliveryPaginatedResult struct { Models []APIDelivery `json:"models"` - Pagination PaginationInfo `json:"pagination"` + Pagination SeekPagination `json:"pagination"` } -// ListEventsResponse is the response for ListEvents -type ListEventsResponse struct { +// EventPaginatedResult is the paginated response for listing events. +type EventPaginatedResult struct { Models []APIEvent `json:"models"` - Pagination PaginationInfo `json:"pagination"` + Pagination SeekPagination `json:"pagination"` } // toAPIDelivery converts a DeliveryEvent to APIDelivery with expand options @@ -268,9 +268,9 @@ func (h *LogHandlers) listDeliveriesInternal(c *gin.Context, tenantID string) { apiDeliveries[i] = toAPIDelivery(de, includeOpts) } - c.JSON(http.StatusOK, ListDeliveriesResponse{ + c.JSON(http.StatusOK, DeliveryPaginatedResult{ Models: apiDeliveries, - Pagination: PaginationInfo{ + Pagination: SeekPagination{ OrderBy: orderBy, Dir: dir, Limit: limit, @@ -428,9 +428,9 @@ func (h *LogHandlers) listEventsInternal(c *gin.Context, tenantID string) { } } - c.JSON(http.StatusOK, ListEventsResponse{ + c.JSON(http.StatusOK, EventPaginatedResult{ Models: apiEvents, - Pagination: PaginationInfo{ + Pagination: SeekPagination{ OrderBy: orderBy, Dir: dir, Limit: limit, diff --git a/internal/apirouter/pagination.go b/internal/apirouter/pagination.go index dff415001..44cc53b2a 100644 --- a/internal/apirouter/pagination.go +++ b/internal/apirouter/pagination.go @@ -12,9 +12,8 @@ import ( "github.com/gin-gonic/gin" ) -// PaginationInfo represents pagination metadata for list responses. -// This aligns with Hookdeck's pagination response format. -type PaginationInfo struct { +// SeekPagination represents cursor-based pagination metadata for list responses. +type SeekPagination struct { OrderBy string `json:"order_by"` Dir string `json:"dir"` Limit int `json:"limit"` @@ -23,7 +22,7 @@ type PaginationInfo struct { } // CursorToPtr converts an empty cursor string to nil, or returns a pointer to the string. -// Hookdeck returns null for empty cursors instead of empty string. +// Returns null for empty cursors instead of empty string. func CursorToPtr(cursor string) *string { if cursor == "" { return nil diff --git a/internal/models/entity.go b/internal/models/entity.go index 9a5fc2736..ddfc152a2 100644 --- a/internal/models/entity.go +++ b/internal/models/entity.go @@ -21,7 +21,7 @@ type EntityStore interface { RetrieveTenant(ctx context.Context, tenantID string) (*Tenant, error) UpsertTenant(ctx context.Context, tenant Tenant) error DeleteTenant(ctx context.Context, tenantID string) error - ListTenant(ctx context.Context, req ListTenantRequest) (*ListTenantResponse, error) + ListTenant(ctx context.Context, req ListTenantRequest) (*TenantPaginatedResult, error) ListDestinationByTenant(ctx context.Context, tenantID string, options ...ListDestinationByTenantOpts) ([]Destination, error) RetrieveDestination(ctx context.Context, tenantID, destinationID string) (*Destination, error) CreateDestination(ctx context.Context, destination Destination) error @@ -51,9 +51,8 @@ type ListTenantRequest struct { Dir string // Sort direction: "asc" or "desc" (default: "desc") } -// TenantPaginationInfo represents pagination metadata for tenant list responses. -// This aligns with Hookdeck's pagination response format. -type TenantPaginationInfo struct { +// SeekPagination represents cursor-based pagination metadata for list responses. +type SeekPagination struct { OrderBy string `json:"order_by"` Dir string `json:"dir"` Limit int `json:"limit"` @@ -61,11 +60,11 @@ type TenantPaginationInfo struct { Prev *string `json:"prev"` } -// ListTenantResponse contains the paginated list of tenants. -type ListTenantResponse struct { - Models []Tenant `json:"models"` - Pagination TenantPaginationInfo `json:"pagination"` - Count int `json:"count"` +// TenantPaginatedResult contains the paginated list of tenants. +type TenantPaginatedResult struct { + Models []Tenant `json:"models"` + Pagination SeekPagination `json:"pagination"` + Count int `json:"count"` } type entityStoreImpl struct { @@ -340,7 +339,7 @@ const ( ) // ListTenant returns a paginated list of tenants using RediSearch. -func (s *entityStoreImpl) ListTenant(ctx context.Context, req ListTenantRequest) (*ListTenantResponse, error) { +func (s *entityStoreImpl) ListTenant(ctx context.Context, req ListTenantRequest) (*TenantPaginatedResult, error) { if !s.listTenantSupported { return nil, ErrListTenantNotSupported } @@ -442,9 +441,9 @@ func (s *entityStoreImpl) ListTenant(ctx context.Context, req ListTenantRequest) prevCursor = &result.Prev } - return &ListTenantResponse{ + return &TenantPaginatedResult{ Models: tenants, - Pagination: TenantPaginationInfo{ + Pagination: SeekPagination{ OrderBy: "created_at", Dir: dir, Limit: limit, diff --git a/internal/models/tenant.go b/internal/models/tenant.go index a8f302c0e..0b30ef65b 100644 --- a/internal/models/tenant.go +++ b/internal/models/tenant.go @@ -15,25 +15,6 @@ type Tenant struct { UpdatedAt time.Time `json:"updated_at" redis:"updated_at"` } -// TenantListItem is a lightweight tenant representation for list operations. -// It excludes computed fields (destinations_count, topics) to avoid N+1 queries. -type TenantListItem struct { - ID string `json:"id"` - Metadata Metadata `json:"metadata,omitempty"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` -} - -// ToListItem converts a Tenant to a TenantListItem. -func (t *Tenant) ToListItem() TenantListItem { - return TenantListItem{ - ID: t.ID, - Metadata: t.Metadata, - CreatedAt: t.CreatedAt, - UpdatedAt: t.UpdatedAt, - } -} - func (t *Tenant) parseRedisHash(hash map[string]string) error { if _, ok := hash["deleted_at"]; ok { return ErrTenantDeleted From d6deaf81afff1159eee7e25774fa5ae62ca74a88 Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Tue, 20 Jan 2026 01:53:52 +0700 Subject: [PATCH 08/14] test: update e2e tests for new API format - Update list response assertions: data -> models - Update pagination cursor access: body["next"] -> body["pagination"]["next"] - Update date filter tests: start= -> event_time[gte]= - Update sort order tests: sort_order= -> dir= - Update validation error assertions: map format -> array format - Legacy endpoint tests unchanged (backward compatibility) Co-Authored-By: Claude Opus 4.5 --- cmd/e2e/api_test.go | 56 +++++++++-------- cmd/e2e/destwebhook_test.go | 28 ++++----- cmd/e2e/log_test.go | 119 ++++++++++++++++++------------------ cmd/e2e/suites_test.go | 4 +- 4 files changed, 106 insertions(+), 101 deletions(-) diff --git a/cmd/e2e/api_test.go b/cmd/e2e/api_test.go index 27384e1f6..6b02c54f4 100644 --- a/cmd/e2e/api_test.go +++ b/cmd/e2e/api_test.go @@ -534,9 +534,9 @@ func (suite *basicSuite) TestListTenantsAPI() { body, ok := resp.Body.(map[string]interface{}) require.True(t, ok, "response should be a map") - data, ok := body["data"].([]interface{}) - require.True(t, ok, "data should be an array") - assert.GreaterOrEqual(t, len(data), 3, "should have at least 3 tenants") + models, ok := body["models"].([]interface{}) + require.True(t, ok, "models should be an array") + assert.GreaterOrEqual(t, len(models), 3, "should have at least 3 tenants") }) // Test list with limit @@ -550,9 +550,9 @@ func (suite *basicSuite) TestListTenantsAPI() { body, ok := resp.Body.(map[string]interface{}) require.True(t, ok, "response should be a map") - data, ok := body["data"].([]interface{}) - require.True(t, ok, "data should be an array") - assert.Equal(t, 2, len(data), "should have exactly 2 tenants") + models, ok := body["models"].([]interface{}) + require.True(t, ok, "models should be an array") + assert.Equal(t, 2, len(models), "should have exactly 2 tenants") }) // Test invalid limit @@ -577,11 +577,12 @@ func (suite *basicSuite) TestListTenantsAPI() { body, ok := resp.Body.(map[string]interface{}) require.True(t, ok, "response should be a map") - data, ok := body["data"].([]interface{}) - require.True(t, ok, "data should be an array") - assert.Equal(t, 2, len(data), "page 1 should have 2 tenants") + models, ok := body["models"].([]interface{}) + require.True(t, ok, "models should be an array") + assert.Equal(t, 2, len(models), "page 1 should have 2 tenants") - next, _ := body["next"].(string) + pagination, _ := body["pagination"].(map[string]interface{}) + next, _ := pagination["next"].(string) require.NotEmpty(t, next, "should have next cursor") // Get second page using next cursor @@ -594,11 +595,12 @@ func (suite *basicSuite) TestListTenantsAPI() { body, ok = resp.Body.(map[string]interface{}) require.True(t, ok, "response should be a map") - data, ok = body["data"].([]interface{}) - require.True(t, ok, "data should be an array") - assert.GreaterOrEqual(t, len(data), 1, "page 2 should have at least 1 tenant") + models, ok = body["models"].([]interface{}) + require.True(t, ok, "models should be an array") + assert.GreaterOrEqual(t, len(models), 1, "page 2 should have at least 1 tenant") - prev, _ := body["prev"].(string) + pagination, _ = body["pagination"].(map[string]interface{}) + prev, _ := pagination["prev"].(string) assert.NotEmpty(t, prev, "page 2 should have prev cursor") }) @@ -613,7 +615,8 @@ func (suite *basicSuite) TestListTenantsAPI() { body, ok := resp.Body.(map[string]interface{}) require.True(t, ok) - next, _ := body["next"].(string) + pagination, _ := body["pagination"].(map[string]interface{}) + next, _ := pagination["next"].(string) require.NotEmpty(t, next, "should have next cursor") // Go to page 2 @@ -625,7 +628,8 @@ func (suite *basicSuite) TestListTenantsAPI() { body, ok = resp.Body.(map[string]interface{}) require.True(t, ok) - prev, _ := body["prev"].(string) + pagination, _ = body["pagination"].(map[string]interface{}) + prev, _ := pagination["prev"].(string) require.NotEmpty(t, prev, "page 2 should have prev cursor") // Using prev cursor returns items with newer timestamps (keyset pagination) @@ -639,9 +643,9 @@ func (suite *basicSuite) TestListTenantsAPI() { body, ok = resp.Body.(map[string]interface{}) require.True(t, ok, "response should be a map") - data, ok := body["data"].([]interface{}) - require.True(t, ok, "data should be an array") - assert.NotEmpty(t, data, "prev cursor should return items") + models, ok := body["models"].([]interface{}) + require.True(t, ok, "models should be an array") + assert.NotEmpty(t, models, "prev cursor should return items") }) // Cleanup @@ -727,9 +731,9 @@ func (suite *basicSuite) TestDestinationsAPI() { StatusCode: http.StatusUnprocessableEntity, Body: map[string]interface{}{ "message": "validation error", - "data": map[string]interface{}{ - "topics": "required", - "type": "required", + "data": []interface{}{ + "type is required", + "topics is required", }, }, }, @@ -795,8 +799,8 @@ func (suite *basicSuite) TestDestinationsAPI() { StatusCode: http.StatusUnprocessableEntity, Body: map[string]interface{}{ "message": "validation error", - "data": map[string]interface{}{ - "config.url": "required", + "data": []interface{}{ + "config.url is required", }, }, }, @@ -1253,8 +1257,8 @@ func (suite *basicSuite) TestDestinationsAPI() { StatusCode: http.StatusUnprocessableEntity, Body: map[string]interface{}{ "message": "validation error", - "data": map[string]interface{}{ - "config.url": "required", + "data": []interface{}{ + "config.url is required", }, }, }, diff --git a/cmd/e2e/destwebhook_test.go b/cmd/e2e/destwebhook_test.go index 4aa26191f..7d224f00a 100644 --- a/cmd/e2e/destwebhook_test.go +++ b/cmd/e2e/destwebhook_test.go @@ -530,8 +530,8 @@ func (suite *basicSuite) TestDestwebhookTenantSecretManagement() { StatusCode: http.StatusUnprocessableEntity, Body: map[string]interface{}{ "message": "validation error", - "data": map[string]interface{}{ - "credentials.secret": "forbidden", + "data": []interface{}{ + "credentials.secret failed forbidden validation", }, }, }, @@ -593,8 +593,8 @@ func (suite *basicSuite) TestDestwebhookTenantSecretManagement() { StatusCode: http.StatusUnprocessableEntity, Body: map[string]interface{}{ "message": "validation error", - "data": map[string]interface{}{ - "credentials.secret": "forbidden", + "data": []interface{}{ + "credentials.secret failed forbidden validation", }, }, }, @@ -616,8 +616,8 @@ func (suite *basicSuite) TestDestwebhookTenantSecretManagement() { StatusCode: http.StatusUnprocessableEntity, Body: map[string]interface{}{ "message": "validation error", - "data": map[string]interface{}{ - "credentials.previous_secret": "forbidden", + "data": []interface{}{ + "credentials.previous_secret failed forbidden validation", }, }, }, @@ -639,8 +639,8 @@ func (suite *basicSuite) TestDestwebhookTenantSecretManagement() { StatusCode: http.StatusUnprocessableEntity, Body: map[string]interface{}{ "message": "validation error", - "data": map[string]interface{}{ - "credentials.previous_secret_invalid_at": "forbidden", + "data": []interface{}{ + "credentials.previous_secret_invalid_at failed forbidden validation", }, }, }, @@ -865,8 +865,8 @@ func (suite *basicSuite) TestDestwebhookAdminSecretManagement() { StatusCode: http.StatusUnprocessableEntity, Body: map[string]interface{}{ "message": "validation error", - "data": map[string]interface{}{ - "credentials.rotate_secret": "invalid", + "data": []interface{}{ + "credentials.rotate_secret failed invalid validation", }, }, }, @@ -932,8 +932,8 @@ func (suite *basicSuite) TestDestwebhookAdminSecretManagement() { StatusCode: http.StatusUnprocessableEntity, Body: map[string]interface{}{ "message": "validation error", - "data": map[string]interface{}{ - "credentials.previous_secret_invalid_at": "pattern", + "data": []interface{}{ + "credentials.previous_secret_invalid_at failed pattern validation", }, }, }, @@ -1082,8 +1082,8 @@ func (suite *basicSuite) TestDestwebhookAdminSecretManagement() { StatusCode: http.StatusUnprocessableEntity, Body: map[string]interface{}{ "message": "validation error", - "data": map[string]interface{}{ - "credentials.secret": "required", + "data": []interface{}{ + "credentials.secret is required", }, }, }, diff --git a/cmd/e2e/log_test.go b/cmd/e2e/log_test.go index c48e15d51..5e41db9bb 100644 --- a/cmd/e2e/log_test.go +++ b/cmd/e2e/log_test.go @@ -126,11 +126,11 @@ func (suite *basicSuite) TestLogAPI() { suite.Require().Equal(http.StatusOK, resp.StatusCode) body := resp.Body.(map[string]interface{}) - data := body["data"].([]interface{}) - suite.Len(data, 10) + models := body["models"].([]interface{}) + suite.Len(models, 10) // Verify structure - first := data[0].(map[string]interface{}) + first := models[0].(map[string]interface{}) suite.NotEmpty(first["id"]) suite.NotEmpty(first["event"]) suite.Equal(destinationID, first["destination"]) @@ -147,8 +147,8 @@ func (suite *basicSuite) TestLogAPI() { suite.Require().Equal(http.StatusOK, resp.StatusCode) body := resp.Body.(map[string]interface{}) - data := body["data"].([]interface{}) - suite.Len(data, 10) + models := body["models"].([]interface{}) + suite.Len(models, 10) }) suite.Run("filter by event_id", func() { @@ -160,8 +160,8 @@ func (suite *basicSuite) TestLogAPI() { suite.Require().Equal(http.StatusOK, resp.StatusCode) body := resp.Body.(map[string]interface{}) - data := body["data"].([]interface{}) - suite.Len(data, 1) + models := body["models"].([]interface{}) + suite.Len(models, 1) }) suite.Run("include=event returns event object without data", func() { @@ -173,10 +173,10 @@ func (suite *basicSuite) TestLogAPI() { suite.Require().Equal(http.StatusOK, resp.StatusCode) body := resp.Body.(map[string]interface{}) - data := body["data"].([]interface{}) - suite.Require().Len(data, 1) + models := body["models"].([]interface{}) + suite.Require().Len(models, 1) - delivery := data[0].(map[string]interface{}) + delivery := models[0].(map[string]interface{}) event := delivery["event"].(map[string]interface{}) suite.NotEmpty(event["id"]) suite.NotEmpty(event["topic"]) @@ -193,10 +193,10 @@ func (suite *basicSuite) TestLogAPI() { suite.Require().Equal(http.StatusOK, resp.StatusCode) body := resp.Body.(map[string]interface{}) - data := body["data"].([]interface{}) - suite.Require().Len(data, 1) + models := body["models"].([]interface{}) + suite.Require().Len(models, 1) - delivery := data[0].(map[string]interface{}) + delivery := models[0].(map[string]interface{}) event := delivery["event"].(map[string]interface{}) suite.NotEmpty(event["id"]) suite.NotNil(event["data"]) // include=event.data SHOULD include data @@ -211,10 +211,10 @@ func (suite *basicSuite) TestLogAPI() { suite.Require().Equal(http.StatusOK, resp.StatusCode) body := resp.Body.(map[string]interface{}) - data := body["data"].([]interface{}) - suite.Require().Len(data, 1) + models := body["models"].([]interface{}) + suite.Require().Len(models, 1) - delivery := data[0].(map[string]interface{}) + delivery := models[0].(map[string]interface{}) suite.NotNil(delivery["response_data"]) }) }) @@ -232,11 +232,11 @@ func (suite *basicSuite) TestLogAPI() { suite.Require().Equal(http.StatusOK, resp.StatusCode) body := resp.Body.(map[string]interface{}) - data := body["data"].([]interface{}) - suite.Len(data, 10) + models := body["models"].([]interface{}) + suite.Len(models, 10) // Verify structure - first := data[0].(map[string]interface{}) + first := models[0].(map[string]interface{}) suite.NotEmpty(first["id"]) suite.NotEmpty(first["topic"]) suite.NotEmpty(first["time"]) @@ -252,8 +252,8 @@ func (suite *basicSuite) TestLogAPI() { suite.Require().Equal(http.StatusOK, resp.StatusCode) body := resp.Body.(map[string]interface{}) - data := body["data"].([]interface{}) - suite.Len(data, 10) // All events have topic=user.created + models := body["models"].([]interface{}) + suite.Len(models, 10) // All events have topic=user.created }) suite.Run("retrieve single event", func() { @@ -279,18 +279,18 @@ func (suite *basicSuite) TestLogAPI() { suite.Equal(http.StatusNotFound, resp.StatusCode) }) - suite.Run("filter by start time excludes past events", func() { + suite.Run("filter by event_time[gte] excludes past events", func() { futureTime := url.QueryEscape(time.Now().Add(1 * time.Hour).Format(time.RFC3339)) resp, err := suite.client.Do(suite.AuthRequest(httpclient.Request{ Method: httpclient.MethodGET, - Path: "/tenants/" + tenantID + "/events?start=" + futureTime, + Path: "/tenants/" + tenantID + "/events?event_time[gte]=" + futureTime, })) suite.Require().NoError(err) suite.Require().Equal(http.StatusOK, resp.StatusCode) body := resp.Body.(map[string]interface{}) - data := body["data"].([]interface{}) - suite.Len(data, 0) + models := body["models"].([]interface{}) + suite.Len(models, 0) }) }) @@ -301,18 +301,18 @@ func (suite *basicSuite) TestLogAPI() { suite.Run("events desc returns newest first", func() { resp, err := suite.client.Do(suite.AuthRequest(httpclient.Request{ Method: httpclient.MethodGET, - Path: "/tenants/" + tenantID + "/events?sort_order=desc", + Path: "/tenants/" + tenantID + "/events?dir=desc", })) suite.Require().NoError(err) suite.Require().Equal(http.StatusOK, resp.StatusCode) body := resp.Body.(map[string]interface{}) - data := body["data"].([]interface{}) - suite.Require().Len(data, 10) + models := body["models"].([]interface{}) + suite.Require().Len(models, 10) - for i := 0; i < len(data)-1; i++ { - curr := parseTime(data[i].(map[string]interface{})["time"].(string)) - next := parseTime(data[i+1].(map[string]interface{})["time"].(string)) + for i := 0; i < len(models)-1; i++ { + curr := parseTime(models[i].(map[string]interface{})["time"].(string)) + next := parseTime(models[i+1].(map[string]interface{})["time"].(string)) suite.True(curr.After(next) || curr.Equal(next), "events not in descending order at index %d", i) } }) @@ -320,26 +320,26 @@ func (suite *basicSuite) TestLogAPI() { suite.Run("events asc returns oldest first", func() { resp, err := suite.client.Do(suite.AuthRequest(httpclient.Request{ Method: httpclient.MethodGET, - Path: "/tenants/" + tenantID + "/events?sort_order=asc", + Path: "/tenants/" + tenantID + "/events?dir=asc", })) suite.Require().NoError(err) suite.Require().Equal(http.StatusOK, resp.StatusCode) body := resp.Body.(map[string]interface{}) - data := body["data"].([]interface{}) - suite.Require().Len(data, 10) + models := body["models"].([]interface{}) + suite.Require().Len(models, 10) - for i := 0; i < len(data)-1; i++ { - curr := parseTime(data[i].(map[string]interface{})["time"].(string)) - next := parseTime(data[i+1].(map[string]interface{})["time"].(string)) + for i := 0; i < len(models)-1; i++ { + curr := parseTime(models[i].(map[string]interface{})["time"].(string)) + next := parseTime(models[i+1].(map[string]interface{})["time"].(string)) suite.True(curr.Before(next) || curr.Equal(next), "events not in ascending order at index %d", i) } }) - suite.Run("events invalid sort_order returns 422", func() { + suite.Run("events invalid dir returns 422", func() { resp, err := suite.client.Do(suite.AuthRequest(httpclient.Request{ Method: httpclient.MethodGET, - Path: "/tenants/" + tenantID + "/events?sort_order=invalid", + Path: "/tenants/" + tenantID + "/events?dir=invalid", })) suite.Require().NoError(err) suite.Equal(http.StatusUnprocessableEntity, resp.StatusCode) @@ -357,7 +357,7 @@ func (suite *basicSuite) TestLogAPI() { pageCount := 0 for { - path := "/tenants/" + tenantID + "/events?limit=3&sort_order=asc" + path := "/tenants/" + tenantID + "/events?limit=3&dir=asc" if nextCursor != "" { path += "&next=" + nextCursor } @@ -370,15 +370,16 @@ func (suite *basicSuite) TestLogAPI() { suite.Require().Equal(http.StatusOK, resp.StatusCode) body := resp.Body.(map[string]interface{}) - data := body["data"].([]interface{}) + models := body["models"].([]interface{}) pageCount++ - for _, item := range data { + for _, item := range models { event := item.(map[string]interface{}) allEventIDs = append(allEventIDs, event["id"].(string)) } - if next, ok := body["next"].(string); ok && next != "" { + pagination, _ := body["pagination"].(map[string]interface{}) + if next, ok := pagination["next"].(string); ok && next != "" { nextCursor = next } else { break @@ -535,9 +536,9 @@ func (suite *basicSuite) TestRetryAPI() { suite.Require().Equal(http.StatusOK, deliveriesResp.StatusCode) body := deliveriesResp.Body.(map[string]interface{}) - data := body["data"].([]interface{}) - suite.Require().NotEmpty(data, "should have at least one delivery") - firstDelivery := data[0].(map[string]interface{}) + models := body["models"].([]interface{}) + suite.Require().NotEmpty(models, "should have at least one delivery") + firstDelivery := models[0].(map[string]interface{}) deliveryID := firstDelivery["id"].(string) // Update mock to succeed for retry @@ -621,7 +622,7 @@ func (suite *basicSuite) TestRetryAPI() { "body": map[string]interface{}{ "type": "object", "properties": map[string]interface{}{ - "data": map[string]interface{}{ + "models": map[string]interface{}{ "type": "array", "minItems": 2, // Original + retry }, @@ -1208,13 +1209,13 @@ func (suite *basicSuite) TestAdminLogEndpoints() { suite.Require().Equal(http.StatusOK, resp.StatusCode) body := resp.Body.(map[string]interface{}) - data := body["data"].([]interface{}) + models := body["models"].([]interface{}) // Should have at least 2 events (one from each tenant we created) - suite.GreaterOrEqual(len(data), 2) + suite.GreaterOrEqual(len(models), 2) // Verify we have events from both tenants by checking event IDs eventsSeen := map[string]bool{} - for _, item := range data { + for _, item := range models { event := item.(map[string]interface{}) if id, ok := event["id"].(string); ok { eventsSeen[id] = true @@ -1233,13 +1234,13 @@ func (suite *basicSuite) TestAdminLogEndpoints() { suite.Require().Equal(http.StatusOK, resp.StatusCode) body := resp.Body.(map[string]interface{}) - data := body["data"].([]interface{}) + models := body["models"].([]interface{}) // Should have at least 2 deliveries (one from each tenant we created) - suite.GreaterOrEqual(len(data), 2) + suite.GreaterOrEqual(len(models), 2) // Verify we have deliveries from both tenants by checking event IDs eventsSeen := map[string]bool{} - for _, item := range data { + for _, item := range models { delivery := item.(map[string]interface{}) if event, ok := delivery["event"].(map[string]interface{}); ok { if id, ok := event["id"].(string); ok { @@ -1265,11 +1266,11 @@ func (suite *basicSuite) TestAdminLogEndpoints() { suite.Require().Equal(http.StatusOK, resp.StatusCode) body := resp.Body.(map[string]interface{}) - data := body["data"].([]interface{}) - suite.Len(data, 1) + models := body["models"].([]interface{}) + suite.Len(models, 1) // Verify only tenant1 event by ID - event := data[0].(map[string]interface{}) + event := models[0].(map[string]interface{}) suite.Equal(event1ID, event["id"]) }) @@ -1282,11 +1283,11 @@ func (suite *basicSuite) TestAdminLogEndpoints() { suite.Require().Equal(http.StatusOK, resp.StatusCode) body := resp.Body.(map[string]interface{}) - data := body["data"].([]interface{}) - suite.Len(data, 1) + models := body["models"].([]interface{}) + suite.Len(models, 1) // Verify only tenant2 delivery by event ID - delivery := data[0].(map[string]interface{}) + delivery := models[0].(map[string]interface{}) event := delivery["event"].(map[string]interface{}) suite.Equal(event2ID, event["id"]) }) diff --git a/cmd/e2e/suites_test.go b/cmd/e2e/suites_test.go index 37bd823d5..baf5bc3a8 100644 --- a/cmd/e2e/suites_test.go +++ b/cmd/e2e/suites_test.go @@ -61,8 +61,8 @@ func (s *e2eSuite) waitForDeliveries(t *testing.T, path string, minCount int, ti lastStatus = resp.StatusCode if resp.StatusCode == http.StatusOK { if body, ok := resp.Body.(map[string]interface{}); ok { - if data, ok := body["data"].([]interface{}); ok { - lastCount = len(data) + if models, ok := body["models"].([]interface{}); ok { + lastCount = len(models) if lastCount >= minCount { return } From 857158895d98a28cfede4275860dc060d94613a6 Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Tue, 20 Jan 2026 02:21:38 +0700 Subject: [PATCH 09/14] feat: add GT/LT operator support to time filters Co-Authored-By: Claude Opus 4.5 --- cmd/e2e/log_test.go | 4 +- docs/apis/openapi.yaml | 82 +++++--------- internal/apirouter/log_handlers.go | 56 ++++++---- internal/apirouter/log_handlers_test.go | 8 +- internal/apirouter/pagination.go | 31 +++++- internal/apirouter/pagination_test.go | 78 ++++++++++++- internal/logstore/chlogstore/chlogstore.go | 32 ++++-- internal/logstore/driver/driver.go | 15 ++- internal/logstore/drivertest/crud.go | 49 ++++----- internal/logstore/drivertest/misc.go | 109 +++++++++---------- internal/logstore/drivertest/pagination.go | 18 +-- internal/logstore/logstore.go | 1 + internal/logstore/memlogstore/memlogstore.go | 20 +++- internal/logstore/pglogstore/pglogstore.go | 32 ++++-- 14 files changed, 327 insertions(+), 208 deletions(-) diff --git a/cmd/e2e/log_test.go b/cmd/e2e/log_test.go index 5e41db9bb..d7f9687cb 100644 --- a/cmd/e2e/log_test.go +++ b/cmd/e2e/log_test.go @@ -279,11 +279,11 @@ func (suite *basicSuite) TestLogAPI() { suite.Equal(http.StatusNotFound, resp.StatusCode) }) - suite.Run("filter by event_time[gte] excludes past events", func() { + suite.Run("filter by time[gte] excludes past events", func() { futureTime := url.QueryEscape(time.Now().Add(1 * time.Hour).Format(time.RFC3339)) resp, err := suite.client.Do(suite.AuthRequest(httpclient.Request{ Method: httpclient.MethodGET, - Path: "/tenants/" + tenantID + "/events?event_time[gte]=" + futureTime, + Path: "/tenants/" + tenantID + "/events?time[gte]=" + futureTime, })) suite.Require().NoError(err) suite.Require().Equal(http.StatusOK, resp.StatusCode) diff --git a/docs/apis/openapi.yaml b/docs/apis/openapi.yaml index a37b5ffb6..4d39943d7 100644 --- a/docs/apis/openapi.yaml +++ b/docs/apis/openapi.yaml @@ -2362,14 +2362,14 @@ paths: items: type: string description: Filter events by topic(s). Can be specified multiple times or comma-separated. - - name: event_time[gte] + - name: time[gte] in: query required: false schema: type: string format: date-time description: Filter events with time >= value (RFC3339 or YYYY-MM-DD format). - - name: event_time[lte] + - name: time[lte] in: query required: false schema: @@ -2402,8 +2402,8 @@ paths: required: false schema: type: string - enum: [event_time, created_at] - default: event_time + enum: [time] + default: time description: Field to sort by. - name: dir in: query @@ -2437,7 +2437,7 @@ paths: metadata: { "source": "oms" } data: { "order_id": "orderid", "tracking": "1Z..." } pagination: - order_by: "event_time" + order_by: "time" dir: "desc" limit: 100 next: "MTcwNDA2NzIwMA==" @@ -2498,28 +2498,14 @@ paths: items: type: string description: Filter deliveries by event topic(s). Can be specified multiple times or comma-separated. - - name: delivery_time[gte] - in: query - required: false - schema: - type: string - format: date-time - description: Filter deliveries with delivered_at >= value (RFC3339 or YYYY-MM-DD format). - - name: delivery_time[lte] - in: query - required: false - schema: - type: string - format: date-time - description: Filter deliveries with delivered_at <= value (RFC3339 or YYYY-MM-DD format). - - name: event_time[gte] + - name: time[gte] in: query required: false schema: type: string format: date-time description: Filter deliveries by event time >= value (RFC3339 or YYYY-MM-DD format). - - name: event_time[lte] + - name: time[lte] in: query required: false schema: @@ -2566,8 +2552,8 @@ paths: required: false schema: type: string - enum: [delivery_time, event_time] - default: delivery_time + enum: [time] + default: time description: Field to sort by. - name: dir in: query @@ -2603,7 +2589,7 @@ paths: event: "evt_789" destination: "des_789" pagination: - order_by: "delivery_time" + order_by: "time" dir: "desc" limit: 100 next: "MTcwNDA2NzIwMA==" @@ -2625,7 +2611,7 @@ paths: metadata: { "source": "crm" } destination: "des_456" pagination: - order_by: "delivery_time" + order_by: "time" dir: "desc" limit: 100 next: null @@ -3360,28 +3346,14 @@ paths: items: type: string description: Filter deliveries by event topic(s). Can be specified multiple times or comma-separated. - - name: delivery_time[gte] - in: query - required: false - schema: - type: string - format: date-time - description: Filter deliveries with delivered_at >= value (RFC3339 or YYYY-MM-DD format). - - name: delivery_time[lte] - in: query - required: false - schema: - type: string - format: date-time - description: Filter deliveries with delivered_at <= value (RFC3339 or YYYY-MM-DD format). - - name: event_time[gte] + - name: time[gte] in: query required: false schema: type: string format: date-time description: Filter deliveries by event time >= value (RFC3339 or YYYY-MM-DD format). - - name: event_time[lte] + - name: time[lte] in: query required: false schema: @@ -3428,8 +3400,8 @@ paths: required: false schema: type: string - enum: [delivery_time, event_time] - default: delivery_time + enum: [time] + default: time description: Field to sort by. - name: dir in: query @@ -3465,7 +3437,7 @@ paths: event: "evt_789" destination: "des_456" pagination: - order_by: "delivery_time" + order_by: "time" dir: "desc" limit: 100 next: "MTcwNDA2NzIwMA==" @@ -3487,7 +3459,7 @@ paths: metadata: { "source": "crm" } destination: "des_456" pagination: - order_by: "delivery_time" + order_by: "time" dir: "desc" limit: 100 next: null @@ -3660,14 +3632,14 @@ paths: minimum: 1 maximum: 1000 description: Number of items per page (default 100, max 1000). - - name: event_time[gte] + - name: time[gte] in: query required: false schema: type: string format: date-time description: Filter events with time >= value (RFC3339 or YYYY-MM-DD format). - - name: event_time[lte] + - name: time[lte] in: query required: false schema: @@ -3679,8 +3651,8 @@ paths: required: false schema: type: string - enum: [event_time, created_at] - default: event_time + enum: [time] + default: time description: Field to sort by. - name: dir in: query @@ -3716,7 +3688,7 @@ paths: metadata: { "source": "oms" } data: { "order_id": "orderid", "tracking": "1Z..." } pagination: - order_by: "event_time" + order_by: "time" dir: "desc" limit: 100 next: null @@ -3865,14 +3837,14 @@ paths: minimum: 1 maximum: 1000 description: Number of items per page (default 100, max 1000). - - name: event_time[gte] + - name: time[gte] in: query required: false schema: type: string format: date-time description: Filter events with time >= value (RFC3339 or YYYY-MM-DD format). - - name: event_time[lte] + - name: time[lte] in: query required: false schema: @@ -3884,8 +3856,8 @@ paths: required: false schema: type: string - enum: [event_time, created_at] - default: event_time + enum: [time] + default: time description: Field to sort by. - name: dir in: query @@ -3921,7 +3893,7 @@ paths: metadata: { "source": "oms" } data: { "order_id": "orderid", "tracking": "1Z..." } pagination: - order_by: "event_time" + order_by: "time" dir: "desc" limit: 100 next: null diff --git a/internal/apirouter/log_handlers.go b/internal/apirouter/log_handlers.go index b1da4ffcb..0f48a8cbc 100644 --- a/internal/apirouter/log_handlers.go +++ b/internal/apirouter/log_handlers.go @@ -211,20 +211,20 @@ func (h *LogHandlers) listDeliveriesInternal(c *gin.Context, tenantID string) { dir = "desc" } - // Parse and validate order_by (delivery_time or event_time) - orderBy, errResp := ParseOrderBy(c, []string{"delivery_time", "event_time"}) + // Parse and validate order_by (time only) + orderBy, errResp := ParseOrderBy(c, []string{"time"}) if errResp != nil { AbortWithError(c, errResp.Code, *errResp) return } if orderBy == "" { - orderBy = "delivery_time" + orderBy = "time" } - // Note: order_by is informational only for now - store always sorts by delivery_time + // Note: order_by is informational only for now - store always sorts by time _ = orderBy - // Parse delivery_time date filters - deliveryTimeFilter, errResp := ParseDateFilter(c, "delivery_time") + // Parse time date filters + deliveryTimeFilter, errResp := ParseDateFilter(c, "time") if errResp != nil { AbortWithError(c, errResp.Code, *errResp) return @@ -243,12 +243,16 @@ func (h *LogHandlers) listDeliveriesInternal(c *gin.Context, tenantID string) { DestinationIDs: destinationIDs, Status: c.Query("status"), Topics: parseQueryArray(c, "topic"), - Start: deliveryTimeFilter.GTE, - End: deliveryTimeFilter.LTE, - Limit: limit, - Next: c.Query("next"), - Prev: c.Query("prev"), - SortOrder: dir, + TimeFilter: logstore.TimeFilter{ + GTE: deliveryTimeFilter.GTE, + LTE: deliveryTimeFilter.LTE, + GT: deliveryTimeFilter.GT, + LT: deliveryTimeFilter.LT, + }, + Limit: limit, + Next: c.Query("next"), + Prev: c.Query("prev"), + SortOrder: dir, } response, err := h.logStore.ListDeliveryEvent(c.Request.Context(), req) @@ -368,20 +372,20 @@ func (h *LogHandlers) listEventsInternal(c *gin.Context, tenantID string) { dir = "desc" } - // Parse and validate order_by (event_time or created_at) - orderBy, errResp := ParseOrderBy(c, []string{"event_time", "created_at"}) + // Parse and validate order_by (time only) + orderBy, errResp := ParseOrderBy(c, []string{"time"}) if errResp != nil { AbortWithError(c, errResp.Code, *errResp) return } if orderBy == "" { - orderBy = "event_time" + orderBy = "time" } - // Note: order_by is informational only for now - store always sorts by event_time + // Note: order_by is informational only for now - store always sorts by time _ = orderBy - // Parse event_time date filters - eventTimeFilter, errResp := ParseDateFilter(c, "event_time") + // Parse time date filters + eventTimeFilter, errResp := ParseDateFilter(c, "time") if errResp != nil { AbortWithError(c, errResp.Code, *errResp) return @@ -398,12 +402,16 @@ func (h *LogHandlers) listEventsInternal(c *gin.Context, tenantID string) { TenantID: tenantID, DestinationIDs: destinationIDs, Topics: parseQueryArray(c, "topic"), - EventStart: eventTimeFilter.GTE, - EventEnd: eventTimeFilter.LTE, - Limit: limit, - Next: c.Query("next"), - Prev: c.Query("prev"), - SortOrder: dir, + TimeFilter: logstore.TimeFilter{ + GTE: eventTimeFilter.GTE, + LTE: eventTimeFilter.LTE, + GT: eventTimeFilter.GT, + LT: eventTimeFilter.LT, + }, + Limit: limit, + Next: c.Query("next"), + Prev: c.Query("prev"), + SortOrder: dir, } response, err := h.logStore.ListEvent(c.Request.Context(), req) diff --git a/internal/apirouter/log_handlers_test.go b/internal/apirouter/log_handlers_test.go index 72cd5de68..3bfc69acb 100644 --- a/internal/apirouter/log_handlers_test.go +++ b/internal/apirouter/log_handlers_test.go @@ -651,17 +651,17 @@ func TestListEvents(t *testing.T) { assert.Equal(t, http.StatusNotFound, w.Code) }) - t.Run("should return validation error for invalid event_time filter", func(t *testing.T) { + t.Run("should return validation error for invalid time filter", func(t *testing.T) { w := httptest.NewRecorder() - req, _ := http.NewRequest("GET", baseAPIPath+"/tenants/"+tenantID+"/events?event_time[gte]=invalid", nil) + req, _ := http.NewRequest("GET", baseAPIPath+"/tenants/"+tenantID+"/events?time[gte]=invalid", nil) result.router.ServeHTTP(w, req) assert.Equal(t, http.StatusUnprocessableEntity, w.Code) }) - t.Run("should return validation error for invalid event_time lte filter", func(t *testing.T) { + t.Run("should return validation error for invalid time lte filter", func(t *testing.T) { w := httptest.NewRecorder() - req, _ := http.NewRequest("GET", baseAPIPath+"/tenants/"+tenantID+"/events?event_time[lte]=invalid", nil) + req, _ := http.NewRequest("GET", baseAPIPath+"/tenants/"+tenantID+"/events?time[lte]=invalid", nil) result.router.ServeHTTP(w, req) assert.Equal(t, http.StatusUnprocessableEntity, w.Code) diff --git a/internal/apirouter/pagination.go b/internal/apirouter/pagination.go index 44cc53b2a..2653b43bf 100644 --- a/internal/apirouter/pagination.go +++ b/internal/apirouter/pagination.go @@ -132,18 +132,19 @@ func ParseArrayParam(c *gin.Context, fieldName string) []string { } // DateFilterResult holds the parsed date filter values. -// Currently supports gte and lte operators only. type DateFilterResult struct { GTE *time.Time // Greater than or equal (>=) LTE *time.Time // Less than or equal (<=) + GT *time.Time // Greater than (>) + LT *time.Time // Less than (<) } // unsupportedDateOps lists operators we recognize but don't support yet. // When stores add support, move these to supported and update ParseDateFilter. -var unsupportedDateOps = []string{"gt", "lt", "any"} +var unsupportedDateOps = []string{"any"} -// ParseDateFilter parses bracket notation date filters (e.g., field[gte], field[lte]). -// Returns 400 for unsupported operators (gt, lt, any). +// ParseDateFilter parses bracket notation date filters (e.g., field[gte], field[lte], field[gt], field[lt]). +// Returns 400 for unsupported operators (any). // Accepts both RFC3339 format (2024-01-01T00:00:00Z) and date-only format (2024-01-01). func ParseDateFilter(c *gin.Context, fieldName string) (*DateFilterResult, *ErrorResponse) { // Check for unsupported operators first @@ -152,7 +153,7 @@ func ParseDateFilter(c *gin.Context, fieldName string) (*DateFilterResult, *Erro if c.Query(key) != "" { return nil, &ErrorResponse{ Code: http.StatusBadRequest, - Message: fmt.Sprintf("operator '%s' is not supported, use 'gte' or 'lte'", op), + Message: fmt.Sprintf("operator '%s' is not supported, use 'gte', 'lte', 'gt', or 'lt'", op), } } } @@ -179,6 +180,26 @@ func ParseDateFilter(c *gin.Context, fieldName string) (*DateFilterResult, *Erro result.LTE = &t } + // Parse [gt] - greater than + gtKey := fieldName + "[gt]" + if gtStr := c.Query(gtKey); gtStr != "" { + t, err := parseDateTime(gtStr) + if err != nil { + return nil, dateFormatError(gtKey) + } + result.GT = &t + } + + // Parse [lt] - less than + ltKey := fieldName + "[lt]" + if ltStr := c.Query(ltKey); ltStr != "" { + t, err := parseDateTime(ltStr) + if err != nil { + return nil, dateFormatError(ltKey) + } + result.LT = &t + } + return result, nil } diff --git a/internal/apirouter/pagination_test.go b/internal/apirouter/pagination_test.go index a0e3c32b8..449d8ac46 100644 --- a/internal/apirouter/pagination_test.go +++ b/internal/apirouter/pagination_test.go @@ -211,8 +211,84 @@ func TestParseDateFilter(t *testing.T) { } } +func TestParseDateFilter_GTLTOperators(t *testing.T) { + tests := []struct { + name string + fieldName string + queryParams map[string]string + wantGT *time.Time + wantLT *time.Time + wantErrCode int + }{ + { + name: "gt param", + fieldName: "time", + queryParams: map[string]string{ + "time[gt]": "2024-01-15T10:30:00Z", + }, + wantGT: timePtr(time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC)), + }, + { + name: "lt param", + fieldName: "time", + queryParams: map[string]string{ + "time[lt]": "2024-01-31T23:59:59Z", + }, + wantLT: timePtr(time.Date(2024, 1, 31, 23, 59, 59, 0, time.UTC)), + }, + { + name: "all four operators", + fieldName: "time", + queryParams: map[string]string{ + "time[gte]": "2024-01-01T00:00:00Z", + "time[lte]": "2024-01-31T23:59:59Z", + "time[gt]": "2024-01-02T00:00:00Z", + "time[lt]": "2024-01-30T23:59:59Z", + }, + wantGT: timePtr(time.Date(2024, 1, 2, 0, 0, 0, 0, time.UTC)), + wantLT: timePtr(time.Date(2024, 1, 30, 23, 59, 59, 0, time.UTC)), + }, + { + name: "invalid gt format returns error", + fieldName: "time", + queryParams: map[string]string{ + "time[gt]": "not-a-date", + }, + wantErrCode: http.StatusUnprocessableEntity, + }, + { + name: "invalid lt format returns error", + fieldName: "time", + queryParams: map[string]string{ + "time[lt]": "invalid", + }, + wantErrCode: http.StatusUnprocessableEntity, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c, _ := createTestContext(tt.queryParams) + + result, errResp := apirouter.ParseDateFilter(c, tt.fieldName) + + if tt.wantErrCode != 0 { + require.NotNil(t, errResp) + assert.Equal(t, tt.wantErrCode, errResp.Code) + return + } + + require.Nil(t, errResp) + require.NotNil(t, result) + + assertTimePtr(t, "GT", tt.wantGT, result.GT) + assertTimePtr(t, "LT", tt.wantLT, result.LT) + }) + } +} + func TestParseDateFilter_UnsupportedOperators(t *testing.T) { - unsupportedOps := []string{"gt", "lt", "any"} + unsupportedOps := []string{"any"} for _, op := range unsupportedOps { t.Run(op+" returns 400", func(t *testing.T) { diff --git a/internal/logstore/chlogstore/chlogstore.go b/internal/logstore/chlogstore/chlogstore.go index 12cb677a2..1bcdce8ed 100644 --- a/internal/logstore/chlogstore/chlogstore.go +++ b/internal/logstore/chlogstore/chlogstore.go @@ -118,13 +118,21 @@ func buildEventQuery(table string, req driver.ListEventRequest, q pagination.Que args = append(args, req.Topics) } - if req.EventStart != nil { + if req.TimeFilter.GTE != nil { conditions = append(conditions, "event_time >= ?") - args = append(args, *req.EventStart) + args = append(args, *req.TimeFilter.GTE) } - if req.EventEnd != nil { + if req.TimeFilter.LTE != nil { conditions = append(conditions, "event_time <= ?") - args = append(args, *req.EventEnd) + args = append(args, *req.TimeFilter.LTE) + } + if req.TimeFilter.GT != nil { + conditions = append(conditions, "event_time > ?") + args = append(args, *req.TimeFilter.GT) + } + if req.TimeFilter.LT != nil { + conditions = append(conditions, "event_time < ?") + args = append(args, *req.TimeFilter.LT) } if q.CursorPos != "" { @@ -334,13 +342,21 @@ func buildDeliveryQuery(table string, req driver.ListDeliveryEventRequest, q pag args = append(args, req.Topics) } - if req.Start != nil { + if req.TimeFilter.GTE != nil { conditions = append(conditions, "delivery_time >= ?") - args = append(args, *req.Start) + args = append(args, *req.TimeFilter.GTE) } - if req.End != nil { + if req.TimeFilter.LTE != nil { conditions = append(conditions, "delivery_time <= ?") - args = append(args, *req.End) + args = append(args, *req.TimeFilter.LTE) + } + if req.TimeFilter.GT != nil { + conditions = append(conditions, "delivery_time > ?") + args = append(args, *req.TimeFilter.GT) + } + if req.TimeFilter.LT != nil { + conditions = append(conditions, "delivery_time < ?") + args = append(args, *req.TimeFilter.LT) } if q.CursorPos != "" { diff --git a/internal/logstore/driver/driver.go b/internal/logstore/driver/driver.go index dd86fc9d3..fe7cf872d 100644 --- a/internal/logstore/driver/driver.go +++ b/internal/logstore/driver/driver.go @@ -7,6 +7,15 @@ import ( "github.com/hookdeck/outpost/internal/models" ) +// TimeFilter represents time-based filter criteria with support for +// both inclusive (GTE/LTE) and exclusive (GT/LT) comparisons. +type TimeFilter struct { + GTE *time.Time // Greater than or equal (>=) + LTE *time.Time // Less than or equal (<=) + GT *time.Time // Greater than (>) + LT *time.Time // Less than (<) +} + type LogStore interface { ListEvent(context.Context, ListEventRequest) (ListEventResponse, error) ListDeliveryEvent(context.Context, ListDeliveryEventRequest) (ListDeliveryEventResponse, error) @@ -19,8 +28,7 @@ type ListEventRequest struct { Next string Prev string Limit int - EventStart *time.Time // optional - filter events created after this time - EventEnd *time.Time // optional - filter events created before this time + TimeFilter TimeFilter // optional - filter events by time TenantID string // optional - filter by tenant (if empty, returns all tenants) DestinationIDs []string // optional Topics []string // optional @@ -37,8 +45,7 @@ type ListDeliveryEventRequest struct { Next string Prev string Limit int - Start *time.Time // optional - filter deliveries after this time - End *time.Time // optional - filter deliveries before this time + TimeFilter TimeFilter // optional - filter deliveries by time TenantID string // optional - filter by tenant (if empty, returns all tenants) EventID string // optional - filter for specific event DestinationIDs []string // optional diff --git a/internal/logstore/drivertest/crud.go b/internal/logstore/drivertest/crud.go index d4853bc04..45083c556 100644 --- a/internal/logstore/drivertest/crud.go +++ b/internal/logstore/drivertest/crud.go @@ -74,10 +74,10 @@ func testCRUD(t *testing.T, newHarness HarnessMaker) { // Verify via List response, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ - TenantID: tenantID, - EventID: event.ID, - Limit: 10, - Start: &startTime, + TenantID: tenantID, + EventID: event.ID, + Limit: 10, + TimeFilter: driver.TimeFilter{GTE: &startTime}, }) require.NoError(t, err) require.Len(t, response.Data, 1) @@ -138,9 +138,9 @@ func testCRUD(t *testing.T, newHarness HarnessMaker) { // Verify all inserted response, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ - TenantID: tenantID, - Limit: 100, - Start: &startTime, + TenantID: tenantID, + Limit: 100, + TimeFilter: driver.TimeFilter{GTE: &startTime}, }) require.NoError(t, err) // 15 batch + 1 single = 16 @@ -160,7 +160,7 @@ func testCRUD(t *testing.T, newHarness HarnessMaker) { TenantID: tenantID, DestinationIDs: []string{destID}, Limit: 100, - EventStart: &startTime, + TimeFilter: driver.TimeFilter{GTE: &startTime}, }) require.NoError(t, err) require.Len(t, response.Data, len(destinationEvents[destID])) @@ -175,7 +175,7 @@ func testCRUD(t *testing.T, newHarness HarnessMaker) { TenantID: tenantID, DestinationIDs: destIDs, Limit: 100, - EventStart: &startTime, + TimeFilter: driver.TimeFilter{GTE: &startTime}, }) require.NoError(t, err) expectedCount := len(destinationEvents[destIDs[0]]) + len(destinationEvents[destIDs[1]]) @@ -188,7 +188,7 @@ func testCRUD(t *testing.T, newHarness HarnessMaker) { TenantID: tenantID, Topics: []string{topic}, Limit: 100, - EventStart: &startTime, + TimeFilter: driver.TimeFilter{GTE: &startTime}, }) require.NoError(t, err) require.Len(t, response.Data, len(topicEvents[topic])) @@ -199,8 +199,7 @@ func testCRUD(t *testing.T, newHarness HarnessMaker) { eventEnd := baseTime response, err := logStore.ListEvent(ctx, driver.ListEventRequest{ TenantID: tenantID, - EventStart: &eventStart, - EventEnd: &eventEnd, + TimeFilter: driver.TimeFilter{GTE: &eventStart, LTE: &eventEnd}, Limit: 100, }) require.NoError(t, err) @@ -217,7 +216,7 @@ func testCRUD(t *testing.T, newHarness HarnessMaker) { TenantID: tenantID, DestinationIDs: []string{destID}, Limit: 100, - Start: &startTime, + TimeFilter: driver.TimeFilter{GTE: &startTime}, }) require.NoError(t, err) for _, de := range response.Data { @@ -227,10 +226,10 @@ func testCRUD(t *testing.T, newHarness HarnessMaker) { t.Run("ListDeliveryEvent by status", func(t *testing.T) { response, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ - TenantID: tenantID, - Status: "success", - Limit: 100, - Start: &startTime, + TenantID: tenantID, + Status: "success", + Limit: 100, + TimeFilter: driver.TimeFilter{GTE: &startTime}, }) require.NoError(t, err) for _, de := range response.Data { @@ -241,10 +240,10 @@ func testCRUD(t *testing.T, newHarness HarnessMaker) { t.Run("ListDeliveryEvent by topic", func(t *testing.T) { topic := testutil.TestTopics[0] response, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ - TenantID: tenantID, - Topics: []string{topic}, - Limit: 100, - Start: &startTime, + TenantID: tenantID, + Topics: []string{topic}, + Limit: 100, + TimeFilter: driver.TimeFilter{GTE: &startTime}, }) require.NoError(t, err) for _, de := range response.Data { @@ -255,10 +254,10 @@ func testCRUD(t *testing.T, newHarness HarnessMaker) { t.Run("ListDeliveryEvent by event ID", func(t *testing.T) { eventID := "batch_evt_00" response, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ - TenantID: tenantID, - EventID: eventID, - Limit: 100, - Start: &startTime, + TenantID: tenantID, + EventID: eventID, + Limit: 100, + TimeFilter: driver.TimeFilter{GTE: &startTime}, }) require.NoError(t, err) require.Len(t, response.Data, 1) diff --git a/internal/logstore/drivertest/misc.go b/internal/logstore/drivertest/misc.go index 4070ac48e..dc594ef65 100644 --- a/internal/logstore/drivertest/misc.go +++ b/internal/logstore/drivertest/misc.go @@ -86,18 +86,18 @@ func testIsolation(t *testing.T, ctx context.Context, logStore driver.LogStore, t.Run("TenantIsolation", func(t *testing.T) { t.Run("ListDeliveryEvent isolates by tenant", func(t *testing.T) { response, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ - TenantID: tenant1ID, - Limit: 100, - Start: &startTime, + TenantID: tenant1ID, + Limit: 100, + TimeFilter: driver.TimeFilter{GTE: &startTime}, }) require.NoError(t, err) require.Len(t, response.Data, 1) assert.Equal(t, "tenant1-event", response.Data[0].Event.ID) response, err = logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ - TenantID: tenant2ID, - Limit: 100, - Start: &startTime, + TenantID: tenant2ID, + Limit: 100, + TimeFilter: driver.TimeFilter{GTE: &startTime}, }) require.NoError(t, err) require.Len(t, response.Data, 1) @@ -127,7 +127,7 @@ func testIsolation(t *testing.T, ctx context.Context, logStore driver.LogStore, TenantID: "", DestinationIDs: []string{destinationID}, Limit: 100, - EventStart: &startTime, + TimeFilter: driver.TimeFilter{GTE: &startTime}, }) require.NoError(t, err) require.Len(t, response.Data, 2) @@ -145,7 +145,7 @@ func testIsolation(t *testing.T, ctx context.Context, logStore driver.LogStore, TenantID: "", DestinationIDs: []string{destinationID}, Limit: 100, - Start: &startTime, + TimeFilter: driver.TimeFilter{GTE: &startTime}, }) require.NoError(t, err) require.Len(t, response.Data, 2) @@ -222,10 +222,10 @@ func testEdgeCases(t *testing.T, ctx context.Context, logStore driver.LogStore, t.Run("invalid SortOrder uses default (desc)", func(t *testing.T) { response, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ - TenantID: tenantID, - SortOrder: "sideways", - Start: &startTime, - Limit: 10, + TenantID: tenantID, + SortOrder: "sideways", + TimeFilter: driver.TimeFilter{GTE: &startTime}, + Limit: 10, }) require.NoError(t, err) require.Len(t, response.Data, 3) @@ -256,7 +256,7 @@ func testEdgeCases(t *testing.T, ctx context.Context, logStore driver.LogStore, responseNil, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ TenantID: tenantID, DestinationIDs: nil, - Start: &startTime, + TimeFilter: driver.TimeFilter{GTE: &startTime}, Limit: 10, }) require.NoError(t, err) @@ -264,7 +264,7 @@ func testEdgeCases(t *testing.T, ctx context.Context, logStore driver.LogStore, responseEmpty, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ TenantID: tenantID, DestinationIDs: []string{}, - Start: &startTime, + TimeFilter: driver.TimeFilter{GTE: &startTime}, Limit: 10, }) require.NoError(t, err) @@ -312,23 +312,22 @@ func testEdgeCases(t *testing.T, ctx context.Context, logStore driver.LogStore, })) } - t.Run("Start is inclusive (>=)", func(t *testing.T) { + t.Run("GTE is inclusive (>=)", func(t *testing.T) { response, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ - TenantID: tenantID, - Start: &boundaryTime, - Limit: 10, + TenantID: tenantID, + TimeFilter: driver.TimeFilter{GTE: &boundaryTime}, + Limit: 10, }) require.NoError(t, err) require.Len(t, response.Data, 2) }) - t.Run("End is inclusive (<=)", func(t *testing.T) { + t.Run("LTE is inclusive (<=)", func(t *testing.T) { farPast := boundaryTime.Add(-1 * time.Hour) response, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ - TenantID: tenantID, - Start: &farPast, - End: &boundaryTime, - Limit: 10, + TenantID: tenantID, + TimeFilter: driver.TimeFilter{GTE: &farPast, LTE: &boundaryTime}, + Limit: 10, }) require.NoError(t, err) require.Len(t, response.Data, 2) @@ -354,9 +353,9 @@ func testEdgeCases(t *testing.T, ctx context.Context, logStore driver.LogStore, t.Run("modifying ListDeliveryEvent result doesn't affect subsequent queries", func(t *testing.T) { response1, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ - TenantID: tenantID, - Limit: 10, - Start: &startTime, + TenantID: tenantID, + Limit: 10, + TimeFilter: driver.TimeFilter{GTE: &startTime}, }) require.NoError(t, err) require.Len(t, response1.Data, 1) @@ -365,9 +364,9 @@ func testEdgeCases(t *testing.T, ctx context.Context, logStore driver.LogStore, response1.Data[0].Event.ID = "MODIFIED" response2, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ - TenantID: tenantID, - Limit: 10, - Start: &startTime, + TenantID: tenantID, + Limit: 10, + TimeFilter: driver.TimeFilter{GTE: &startTime}, }) require.NoError(t, err) require.Len(t, response2.Data, 1) @@ -416,9 +415,9 @@ func testEdgeCases(t *testing.T, ctx context.Context, logStore driver.LogStore, // Assert: still exactly 1 record response, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ - TenantID: tenantID, - Limit: 100, - Start: &startTime, + TenantID: tenantID, + Limit: 100, + TimeFilter: driver.TimeFilter{GTE: &startTime}, }) require.NoError(t, err) require.Len(t, response.Data, 1, "concurrent duplicate inserts should result in exactly 1 record") @@ -441,11 +440,11 @@ func testCursorValidation(t *testing.T, ctx context.Context, logStore driver.Log for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { _, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ - TenantID: tenantID, - SortOrder: "desc", - Next: tc.cursor, - Start: &startTime, - Limit: 10, + TenantID: tenantID, + SortOrder: "desc", + Next: tc.cursor, + TimeFilter: driver.TimeFilter{GTE: &startTime}, + Limit: 10, }) require.Error(t, err) assert.True(t, errors.Is(err, cursor.ErrInvalidCursor), "expected cursor.ErrInvalidCursor, got: %v", err) @@ -480,20 +479,20 @@ func testCursorValidation(t *testing.T, ctx context.Context, logStore driver.Log t.Run("delivery_time desc", func(t *testing.T) { page1, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ - TenantID: tenantID, - SortOrder: "desc", - Start: &startTime, - Limit: 2, + TenantID: tenantID, + SortOrder: "desc", + TimeFilter: driver.TimeFilter{GTE: &startTime}, + Limit: 2, }) require.NoError(t, err) require.NotEmpty(t, page1.Next) page2, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ - TenantID: tenantID, - SortOrder: "desc", - Next: page1.Next, - Start: &startTime, - Limit: 2, + TenantID: tenantID, + SortOrder: "desc", + Next: page1.Next, + TimeFilter: driver.TimeFilter{GTE: &startTime}, + Limit: 2, }) require.NoError(t, err) require.NotEmpty(t, page2.Data) @@ -501,20 +500,20 @@ func testCursorValidation(t *testing.T, ctx context.Context, logStore driver.Log t.Run("delivery_time asc", func(t *testing.T) { page1, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ - TenantID: tenantID, - SortOrder: "asc", - Start: &startTime, - Limit: 2, + TenantID: tenantID, + SortOrder: "asc", + TimeFilter: driver.TimeFilter{GTE: &startTime}, + Limit: 2, }) require.NoError(t, err) require.NotEmpty(t, page1.Next) page2, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ - TenantID: tenantID, - SortOrder: "asc", - Next: page1.Next, - Start: &startTime, - Limit: 2, + TenantID: tenantID, + SortOrder: "asc", + Next: page1.Next, + TimeFilter: driver.TimeFilter{GTE: &startTime}, + Limit: 2, }) require.NoError(t, err) require.NotEmpty(t, page2.Data) diff --git a/internal/logstore/drivertest/pagination.go b/internal/logstore/drivertest/pagination.go index 0ba297224..5ee57badc 100644 --- a/internal/logstore/drivertest/pagination.go +++ b/internal/logstore/drivertest/pagination.go @@ -79,12 +79,12 @@ func testPagination(t *testing.T, newHarness HarnessMaker) { List: func(ctx context.Context, opts paginationtest.ListOpts) (paginationtest.ListResult[*models.DeliveryEvent], error) { res, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ - TenantID: tenantID, - Limit: opts.Limit, - SortOrder: opts.Order, - Next: opts.Next, - Prev: opts.Prev, - Start: &farPast, + TenantID: tenantID, + Limit: opts.Limit, + SortOrder: opts.Order, + Next: opts.Next, + Prev: opts.Prev, + TimeFilter: driver.TimeFilter{GTE: &farPast}, }) if err != nil { return paginationtest.ListResult[*models.DeliveryEvent]{}, err @@ -171,7 +171,7 @@ func testPagination(t *testing.T, newHarness HarnessMaker) { SortOrder: opts.Order, Next: opts.Next, Prev: opts.Prev, - Start: &farPast, + TimeFilter: driver.TimeFilter{GTE: &farPast}, }) if err != nil { return paginationtest.ListResult[*models.DeliveryEvent]{}, err @@ -255,7 +255,7 @@ func testPagination(t *testing.T, newHarness HarnessMaker) { SortOrder: opts.Order, Next: opts.Next, Prev: opts.Prev, - EventStart: &farPast, + TimeFilter: driver.TimeFilter{GTE: &farPast}, }) if err != nil { return paginationtest.ListResult[*models.Event]{}, err @@ -342,7 +342,7 @@ func testPagination(t *testing.T, newHarness HarnessMaker) { SortOrder: opts.Order, Next: opts.Next, Prev: opts.Prev, - EventStart: &farPast, + TimeFilter: driver.TimeFilter{GTE: &farPast}, }) if err != nil { return paginationtest.ListResult[*models.Event]{}, err diff --git a/internal/logstore/logstore.go b/internal/logstore/logstore.go index 5ac6c7c8e..84a45314a 100644 --- a/internal/logstore/logstore.go +++ b/internal/logstore/logstore.go @@ -13,6 +13,7 @@ import ( "github.com/jackc/pgx/v5/pgxpool" ) +type TimeFilter = driver.TimeFilter type ListEventRequest = driver.ListEventRequest type ListEventResponse = driver.ListEventResponse type ListDeliveryEventRequest = driver.ListDeliveryEventRequest diff --git a/internal/logstore/memlogstore/memlogstore.go b/internal/logstore/memlogstore/memlogstore.go index a68abcda6..fc6c8014b 100644 --- a/internal/logstore/memlogstore/memlogstore.go +++ b/internal/logstore/memlogstore/memlogstore.go @@ -175,10 +175,16 @@ func (s *memLogStore) matchesEventFilter(event *models.Event, req driver.ListEve } } - if req.EventStart != nil && event.Time.Before(*req.EventStart) { + if req.TimeFilter.GTE != nil && event.Time.Before(*req.TimeFilter.GTE) { return false } - if req.EventEnd != nil && event.Time.After(*req.EventEnd) { + if req.TimeFilter.LTE != nil && event.Time.After(*req.TimeFilter.LTE) { + return false + } + if req.TimeFilter.GT != nil && !event.Time.After(*req.TimeFilter.GT) { + return false + } + if req.TimeFilter.LT != nil && !event.Time.Before(*req.TimeFilter.LT) { return false } @@ -391,10 +397,16 @@ func (s *memLogStore) matchesFilter(de *models.DeliveryEvent, req driver.ListDel } } - if req.Start != nil && de.Delivery.Time.Before(*req.Start) { + if req.TimeFilter.GTE != nil && de.Delivery.Time.Before(*req.TimeFilter.GTE) { + return false + } + if req.TimeFilter.LTE != nil && de.Delivery.Time.After(*req.TimeFilter.LTE) { + return false + } + if req.TimeFilter.GT != nil && !de.Delivery.Time.After(*req.TimeFilter.GT) { return false } - if req.End != nil && de.Delivery.Time.After(*req.End) { + if req.TimeFilter.LT != nil && !de.Delivery.Time.Before(*req.TimeFilter.LT) { return false } diff --git a/internal/logstore/pglogstore/pglogstore.go b/internal/logstore/pglogstore/pglogstore.go index 9e026f3f3..90f7319f6 100644 --- a/internal/logstore/pglogstore/pglogstore.go +++ b/internal/logstore/pglogstore/pglogstore.go @@ -88,7 +88,7 @@ func (s *logStore) ListEvent(ctx context.Context, req driver.ListEventRequest) ( } func buildEventQuery(req driver.ListEventRequest, q pagination.QueryInput) (string, []any) { - cursorCondition := fmt.Sprintf("AND ($6::text = '' OR time_id %s $6::text)", q.Compare) + cursorCondition := fmt.Sprintf("AND ($8::text = '' OR time_id %s $8::text)", q.Compare) orderByClause := fmt.Sprintf("time %s, id %s", strings.ToUpper(q.SortDir), strings.ToUpper(q.SortDir)) query := fmt.Sprintf(` @@ -108,19 +108,23 @@ func buildEventQuery(req driver.ListEventRequest, q pagination.QueryInput) (stri AND (array_length($3::text[], 1) IS NULL OR topic = ANY($3)) AND ($4::timestamptz IS NULL OR time >= $4) AND ($5::timestamptz IS NULL OR time <= $5) + AND ($6::timestamptz IS NULL OR time > $6) + AND ($7::timestamptz IS NULL OR time < $7) %s ORDER BY %s - LIMIT $7 + LIMIT $9 `, cursorCondition, orderByClause) args := []any{ req.TenantID, // $1 req.DestinationIDs, // $2 req.Topics, // $3 - req.EventStart, // $4 - req.EventEnd, // $5 - q.CursorPos, // $6 - q.Limit, // $7 + req.TimeFilter.GTE, // $4 + req.TimeFilter.LTE, // $5 + req.TimeFilter.GT, // $6 + req.TimeFilter.LT, // $7 + q.CursorPos, // $8 + q.Limit, // $9 } return query, args @@ -235,7 +239,7 @@ func (s *logStore) ListDeliveryEvent(ctx context.Context, req driver.ListDeliver } func buildDeliveryQuery(req driver.ListDeliveryEventRequest, q pagination.QueryInput) (string, []any) { - cursorCondition := fmt.Sprintf("AND ($8::text = '' OR idx.time_delivery_id %s $8::text)", q.Compare) + cursorCondition := fmt.Sprintf("AND ($10::text = '' OR idx.time_delivery_id %s $10::text)", q.Compare) orderByClause := fmt.Sprintf("idx.delivery_time %s, idx.delivery_id %s", strings.ToUpper(q.SortDir), strings.ToUpper(q.SortDir)) query := fmt.Sprintf(` @@ -266,9 +270,11 @@ func buildDeliveryQuery(req driver.ListDeliveryEventRequest, q pagination.QueryI AND (array_length($5::text[], 1) IS NULL OR idx.topic = ANY($5)) AND ($6::timestamptz IS NULL OR idx.delivery_time >= $6) AND ($7::timestamptz IS NULL OR idx.delivery_time <= $7) + AND ($8::timestamptz IS NULL OR idx.delivery_time > $8) + AND ($9::timestamptz IS NULL OR idx.delivery_time < $9) %s ORDER BY %s - LIMIT $9 + LIMIT $11 `, cursorCondition, orderByClause) args := []any{ @@ -277,10 +283,12 @@ func buildDeliveryQuery(req driver.ListDeliveryEventRequest, q pagination.QueryI req.DestinationIDs, // $3 req.Status, // $4 req.Topics, // $5 - req.Start, // $6 - req.End, // $7 - q.CursorPos, // $8 - q.Limit, // $9 + req.TimeFilter.GTE, // $6 + req.TimeFilter.LTE, // $7 + req.TimeFilter.GT, // $8 + req.TimeFilter.LT, // $9 + q.CursorPos, // $10 + q.Limit, // $11 } return query, args From 0f3b5b11b60d0a3441909b383da59d8281a6747c Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Tue, 20 Jan 2026 03:00:24 +0700 Subject: [PATCH 10/14] test: add cursor pagination with time filter tests Add comprehensive tests for cursor-based pagination combined with time filters (GTE/LTE/GT/LT): - drivertest/pagination.go: TimeFilterWithCursor test suite covering paginating within time windows, boundary exclusion with GT/LT, backward pagination, and both events and deliveries - e2e/log_test.go: API-level test for cursor + time filter interaction using explicit event timestamps for deterministic ordering - pagination.go: Update parseDateTime to try RFC3339Nano first to handle millisecond precision timestamps Co-Authored-By: Claude Opus 4.5 --- cmd/e2e/log_test.go | 86 ++++- internal/apirouter/pagination.go | 8 +- internal/logstore/drivertest/pagination.go | 412 +++++++++++++++++++++ 3 files changed, 500 insertions(+), 6 deletions(-) diff --git a/cmd/e2e/log_test.go b/cmd/e2e/log_test.go index d7f9687cb..c6c1729aa 100644 --- a/cmd/e2e/log_test.go +++ b/cmd/e2e/log_test.go @@ -3,7 +3,6 @@ package e2e_test import ( "fmt" "net/http" - "net/url" "time" "github.com/hookdeck/outpost/cmd/e2e/httpclient" @@ -92,8 +91,10 @@ func (suite *basicSuite) TestLogAPI() { } suite.RunAPITests(suite.T(), setupTests) - // Publish 10 events with small delays for distinct timestamps + // Publish 10 events with explicit timestamps (1 second apart) + baseTime := time.Now().Add(-1 * time.Hour).Truncate(time.Second) for i, eventID := range eventIDs { + eventTime := baseTime.Add(time.Duration(i) * time.Second) resp, err := suite.client.Do(suite.AuthRequest(httpclient.Request{ Method: httpclient.MethodPOST, Path: "/publish", @@ -102,12 +103,12 @@ func (suite *basicSuite) TestLogAPI() { "tenant_id": tenantID, "topic": "user.created", "eligible_for_retry": true, + "time": eventTime.Format(time.RFC3339Nano), "data": map[string]interface{}{"index": i}, }, })) suite.Require().NoError(err) suite.Require().Equal(http.StatusAccepted, resp.StatusCode, "failed to publish event %d", i) - time.Sleep(50 * time.Millisecond) } // Wait for all deliveries (30s timeout for slow CI environments) @@ -280,7 +281,7 @@ func (suite *basicSuite) TestLogAPI() { }) suite.Run("filter by time[gte] excludes past events", func() { - futureTime := url.QueryEscape(time.Now().Add(1 * time.Hour).Format(time.RFC3339)) + futureTime := time.Now().Add(1 * time.Hour).UTC().Format(time.RFC3339) resp, err := suite.client.Do(suite.AuthRequest(httpclient.Request{ Method: httpclient.MethodGET, Path: "/tenants/" + tenantID + "/events?time[gte]=" + futureTime, @@ -394,6 +395,83 @@ func (suite *basicSuite) TestLogAPI() { suite.Equal(4, pageCount, "expected 4 pages (3+3+3+1)") suite.Len(allEventIDs, 10, "should have all 10 events") }) + + suite.Run("cursor pagination with time filter", func() { + // Get all events to establish a time window + resp, err := suite.client.Do(suite.AuthRequest(httpclient.Request{ + Method: httpclient.MethodGET, + Path: "/tenants/" + tenantID + "/events?dir=asc&limit=10", + })) + suite.Require().NoError(err) + suite.Require().Equal(http.StatusOK, resp.StatusCode) + + body := resp.Body.(map[string]interface{}) + models := body["models"].([]interface{}) + suite.Require().Len(models, 10) + + // Use the 3rd and 7th events to create a time window + event3 := models[2].(map[string]interface{}) + event7 := models[6].(map[string]interface{}) + timeGTE := event3["time"].(string) + timeLTE := event7["time"].(string) + timeGTEParsed := parseTime(timeGTE) + timeLTEParsed := parseTime(timeLTE) + + // Paginate within the time window with limit=2 + var windowEvents []map[string]interface{} + nextCursor := "" + pageCount := 0 + + for { + path := "/tenants/" + tenantID + "/events?dir=asc&limit=2" + path += "&time[gte]=" + timeGTE + "&time[lte]=" + timeLTE + if nextCursor != "" { + path += "&next=" + nextCursor + } + + resp, err := suite.client.Do(suite.AuthRequest(httpclient.Request{ + Method: httpclient.MethodGET, + Path: path, + })) + suite.Require().NoError(err) + suite.Require().Equal(http.StatusOK, resp.StatusCode) + + body := resp.Body.(map[string]interface{}) + windowModels := body["models"].([]interface{}) + pageCount++ + + for _, item := range windowModels { + event := item.(map[string]interface{}) + windowEvents = append(windowEvents, event) + } + + pagination, _ := body["pagination"].(map[string]interface{}) + if next, ok := pagination["next"].(string); ok && next != "" { + nextCursor = next + } else { + break + } + + if pageCount > 10 { + suite.Fail("too many pages") + break + } + } + + // Verify time filter worked: should have fewer events than total + suite.Greater(len(windowEvents), 0, "should have some events in window") + suite.Less(len(windowEvents), 10, "time filter should exclude some events") + + // Verify pagination worked: multiple pages needed + suite.Greater(pageCount, 1, "should require multiple pages") + + // Verify all returned events are within the time window + for _, event := range windowEvents { + eventTime := parseTime(event["time"].(string)) + suite.True(!eventTime.Before(timeGTEParsed), "event time %v should be >= %v", eventTime, timeGTEParsed) + suite.True(!eventTime.After(timeLTEParsed), "event time %v should be <= %v", eventTime, timeLTEParsed) + } + }) }) // Cleanup diff --git a/internal/apirouter/pagination.go b/internal/apirouter/pagination.go index 2653b43bf..0cc0c1249 100644 --- a/internal/apirouter/pagination.go +++ b/internal/apirouter/pagination.go @@ -218,9 +218,13 @@ func dateFormatError(key string) *ErrorResponse { // These types are defined for future use when stores add support. // Attempting to use them will return 400 errors from the handlers. -// parseDateTime attempts to parse a date string in RFC3339 or date-only (YYYY-MM-DD) format. +// parseDateTime attempts to parse a date string in RFC3339, RFC3339Nano, or date-only (YYYY-MM-DD) format. func parseDateTime(s string) (time.Time, error) { - // Try RFC3339 first + // Try RFC3339Nano first (handles milliseconds/microseconds like 2024-01-01T00:00:00.123Z) + if t, err := time.Parse(time.RFC3339Nano, s); err == nil { + return t, nil + } + // Try RFC3339 (no fractional seconds) if t, err := time.Parse(time.RFC3339, s); err == nil { return t, nil } diff --git a/internal/logstore/drivertest/pagination.go b/internal/logstore/drivertest/pagination.go index 5ee57badc..6a48877cd 100644 --- a/internal/logstore/drivertest/pagination.go +++ b/internal/logstore/drivertest/pagination.go @@ -369,4 +369,416 @@ func testPagination(t *testing.T, newHarness HarnessMaker) { suite.Run(t) }) + + // Test cursor pagination combined with time filters. + // These tests verify that cursors work correctly when used alongside + // time-based filters (GTE, LTE, GT, LT), which is critical for + // "paginate within a time window" use cases. + // + // IMPORTANT: ListDeliveryEvent filters by DELIVERY time, ListEvent filters by EVENT time. + // In this test, delivery_time = event_time + 100ms. + t.Run("TimeFilterWithCursor", func(t *testing.T) { + tenantID := idgen.String() + destinationID := idgen.Destination() + idPrefix := idgen.String()[:8] + + // Create 20 events with times spread across different ranges: + // - Events 0-4: far past (should be excluded by GTE filter) + // - Events 5-14: within time window (should be included) + // - Events 15-19: far future (should be excluded by LTE filter) + // + // Event times are spaced 2 minutes apart within the window. + // Delivery times are 1 second after event times (not sub-second) + // to ensure GT/LT tests work consistently across databases. + eventWindowStart := baseTime.Add(-10 * time.Minute) + eventWindowEnd := baseTime.Add(10 * time.Minute) + // Delivery window accounts for the 1 second offset + deliveryWindowStart := eventWindowStart.Add(time.Second) + deliveryWindowEnd := eventWindowEnd.Add(time.Second) + + var allEvents []*models.DeliveryEvent + for i := range 20 { + var eventTime time.Time + switch { + case i < 5: + // Far past: outside window (before eventWindowStart) + eventTime = eventWindowStart.Add(-time.Duration(5-i) * time.Hour) + case i < 15: + // Within window: eventWindowStart to eventWindowEnd + offset := time.Duration(i-5) * 2 * time.Minute + eventTime = eventWindowStart.Add(offset) + default: + // Far future: outside window (after eventWindowEnd) + eventTime = eventWindowEnd.Add(time.Duration(i-14) * time.Hour) + } + + deliveryTime := eventTime.Add(time.Second) + + event := &models.Event{ + ID: fmt.Sprintf("%s_evt_%03d", idPrefix, i), + TenantID: tenantID, + DestinationID: destinationID, + Topic: "test.topic", + EligibleForRetry: true, + Time: eventTime, + Metadata: map[string]string{}, + Data: map[string]any{}, + } + delivery := &models.Delivery{ + ID: fmt.Sprintf("%s_del_%03d", idPrefix, i), + EventID: event.ID, + DestinationID: destinationID, + Status: "success", + Time: deliveryTime, + Code: "200", + } + allEvents = append(allEvents, &models.DeliveryEvent{ + ID: fmt.Sprintf("%s_de_%03d", idPrefix, i), + DestinationID: destinationID, + Event: *event, + Delivery: delivery, + }) + } + + require.NoError(t, logStore.InsertManyDeliveryEvent(ctx, allEvents)) + require.NoError(t, h.FlushWrites(ctx)) + + t.Run("paginate within time-bounded window", func(t *testing.T) { + // Paginate through deliveries within the window with limit=3 + // ListDeliveryEvent filters by DELIVERY time, not event time. + // Should only see deliveries 5-14 (10 total), not 0-4 or 15-19 + var collectedIDs []string + var nextCursor string + pageCount := 0 + + for { + res, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ + TenantID: tenantID, + Limit: 3, + SortOrder: "asc", + Next: nextCursor, + TimeFilter: driver.TimeFilter{GTE: &deliveryWindowStart, LTE: &deliveryWindowEnd}, + }) + require.NoError(t, err) + + for _, de := range res.Data { + collectedIDs = append(collectedIDs, de.Event.ID) + } + + pageCount++ + if res.Next == "" { + break + } + nextCursor = res.Next + + // Safety: prevent infinite loop + if pageCount > 10 { + t.Fatal("too many pages") + } + } + + // Should have collected exactly deliveries 5-14 + require.Len(t, collectedIDs, 10, "should have 10 deliveries in window") + for i, id := range collectedIDs { + expectedID := fmt.Sprintf("%s_evt_%03d", idPrefix, i+5) + require.Equal(t, expectedID, id, "delivery %d mismatch", i) + } + require.Equal(t, 4, pageCount, "should take 4 pages (3+3+3+1)") + }) + + t.Run("cursor excludes deliveries outside time filter", func(t *testing.T) { + // First page with no time filter gets all deliveries + resAll, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ + TenantID: tenantID, + Limit: 5, + SortOrder: "asc", + TimeFilter: driver.TimeFilter{GTE: &farPast}, + }) + require.NoError(t, err) + require.Len(t, resAll.Data, 5) + + // Use the cursor but add a time filter that excludes some results + // The cursor points to position after delivery 4 (far past deliveries) + // But with deliveryWindowStart filter, we should start from delivery 5 + res, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ + TenantID: tenantID, + Limit: 5, + SortOrder: "asc", + Next: resAll.Next, + TimeFilter: driver.TimeFilter{GTE: &deliveryWindowStart, LTE: &deliveryWindowEnd}, + }) + require.NoError(t, err) + + // Results should respect the time filter (on delivery time) + for _, de := range res.Data { + require.True(t, !de.Delivery.Time.Before(deliveryWindowStart), "delivery time should be >= deliveryWindowStart") + require.True(t, !de.Delivery.Time.After(deliveryWindowEnd), "delivery time should be <= deliveryWindowEnd") + } + }) + + t.Run("delivery time filter with GT/LT operators", func(t *testing.T) { + // Test exclusive bounds (GT/LT instead of GTE/LTE) on delivery time + // Use delivery times slightly after delivery 5 and slightly before delivery 14 + gtTime := allEvents[5].Delivery.Time.Add(time.Second) // After delivery 5, before delivery 6 + ltTime := allEvents[14].Delivery.Time.Add(-time.Second) // Before delivery 14, after delivery 13 + + var collectedIDs []string + var nextCursor string + + for { + res, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ + TenantID: tenantID, + Limit: 3, + SortOrder: "asc", + Next: nextCursor, + TimeFilter: driver.TimeFilter{GT: >Time, LT: <Time}, + }) + require.NoError(t, err) + + for _, de := range res.Data { + collectedIDs = append(collectedIDs, de.Event.ID) + } + + if res.Next == "" { + break + } + nextCursor = res.Next + } + + // Should have events 6-13 (8 events) + require.Len(t, collectedIDs, 8, "should have 8 events in GT/LT range") + for i, id := range collectedIDs { + expectedID := fmt.Sprintf("%s_evt_%03d", idPrefix, i+6) + require.Equal(t, expectedID, id, "event %d mismatch", i) + } + }) + + t.Run("GT/LT exclude exact timestamp", func(t *testing.T) { + // Verify that GT excludes the exact timestamp (not >=) + // and LT excludes the exact timestamp (not <=). + // + // We truncate times to second precision to ensure consistent + // comparison across databases with different timestamp precision + // (PostgreSQL microseconds, ClickHouse DateTime64, etc.). + // + // Important: ListDeliveryEvent filters by DELIVERY time, not event time. + + // First, retrieve all deliveries to find delivery 10's time + res, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ + TenantID: tenantID, + Limit: 100, + SortOrder: "asc", + TimeFilter: driver.TimeFilter{ + GTE: &farPast, + }, + }) + require.NoError(t, err) + require.GreaterOrEqual(t, len(res.Data), 11, "need at least 11 deliveries") + + // Find delivery 10's stored delivery time, truncated to seconds + var storedDelivery10Time time.Time + for _, de := range res.Data { + if de.Event.ID == allEvents[10].Event.ID { + storedDelivery10Time = de.Delivery.Time.Truncate(time.Second) + break + } + } + require.False(t, storedDelivery10Time.IsZero(), "should find delivery 10") + + // GT with exact time should exclude delivery 10 + resGT, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ + TenantID: tenantID, + Limit: 100, + SortOrder: "asc", + TimeFilter: driver.TimeFilter{GT: &storedDelivery10Time}, + }) + require.NoError(t, err) + + for _, de := range resGT.Data { + deTimeTrunc := de.Delivery.Time.Truncate(time.Second) + require.True(t, deTimeTrunc.After(storedDelivery10Time), + "GT filter should exclude delivery with exact timestamp, got delivery %s with time %v (filter time: %v)", + de.Delivery.ID, deTimeTrunc, storedDelivery10Time) + } + + // LT with exact time should exclude delivery 10 + resLT, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ + TenantID: tenantID, + Limit: 100, + SortOrder: "asc", + TimeFilter: driver.TimeFilter{LT: &storedDelivery10Time}, + }) + require.NoError(t, err) + + for _, de := range resLT.Data { + deTimeTrunc := de.Delivery.Time.Truncate(time.Second) + require.True(t, deTimeTrunc.Before(storedDelivery10Time), + "LT filter should exclude delivery with exact timestamp, got delivery %s with time %v (filter time: %v)", + de.Delivery.ID, deTimeTrunc, storedDelivery10Time) + } + + // Verify delivery 10 is included with GTE/LTE (inclusive bounds) + resGTE, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ + TenantID: tenantID, + Limit: 100, + SortOrder: "asc", + TimeFilter: driver.TimeFilter{GTE: &storedDelivery10Time, LTE: &storedDelivery10Time}, + }) + require.NoError(t, err) + require.GreaterOrEqual(t, len(resGTE.Data), 1, "GTE/LTE with same time should include delivery at that second") + }) + + t.Run("prev cursor respects time filter", func(t *testing.T) { + // Get first page (ListDeliveryEvent filters by delivery time) + res1, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ + TenantID: tenantID, + Limit: 3, + SortOrder: "asc", + TimeFilter: driver.TimeFilter{GTE: &deliveryWindowStart, LTE: &deliveryWindowEnd}, + }) + require.NoError(t, err) + require.NotEmpty(t, res1.Next) + + // Get second page + res2, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ + TenantID: tenantID, + Limit: 3, + SortOrder: "asc", + Next: res1.Next, + TimeFilter: driver.TimeFilter{GTE: &deliveryWindowStart, LTE: &deliveryWindowEnd}, + }) + require.NoError(t, err) + require.NotEmpty(t, res2.Prev) + + // Go back to first page using prev cursor + resPrev, err := logStore.ListDeliveryEvent(ctx, driver.ListDeliveryEventRequest{ + TenantID: tenantID, + Limit: 3, + SortOrder: "asc", + Prev: res2.Prev, + TimeFilter: driver.TimeFilter{GTE: &deliveryWindowStart, LTE: &deliveryWindowEnd}, + }) + require.NoError(t, err) + + // Should get same results as first page + require.Len(t, resPrev.Data, len(res1.Data)) + for i := range res1.Data { + require.Equal(t, res1.Data[i].Event.ID, resPrev.Data[i].Event.ID) + } + }) + + t.Run("ListEvent with time filter pagination", func(t *testing.T) { + // Same test pattern for ListEvent + var collectedIDs []string + var nextCursor string + pageCount := 0 + + for { + res, err := logStore.ListEvent(ctx, driver.ListEventRequest{ + TenantID: tenantID, + Limit: 3, + SortOrder: "asc", + Next: nextCursor, + TimeFilter: driver.TimeFilter{GTE: &eventWindowStart, LTE: &eventWindowEnd}, + }) + require.NoError(t, err) + + for _, e := range res.Data { + collectedIDs = append(collectedIDs, e.ID) + } + + pageCount++ + if res.Next == "" { + break + } + nextCursor = res.Next + + if pageCount > 10 { + t.Fatal("too many pages") + } + } + + // Should have collected exactly events 5-14 + require.Len(t, collectedIDs, 10, "should have 10 events in window") + for i, id := range collectedIDs { + expectedID := fmt.Sprintf("%s_evt_%03d", idPrefix, i+5) + require.Equal(t, expectedID, id, "event %d mismatch", i) + } + require.Equal(t, 4, pageCount, "should take 4 pages (3+3+3+1)") + }) + + t.Run("ListEvent GT/LT exclude exact timestamp", func(t *testing.T) { + // Verify that GT excludes the exact timestamp (not >=) + // and LT excludes the exact timestamp (not <=). + // + // We truncate times to second precision to ensure consistent + // comparison across databases with different timestamp precision. + // + // ListEvent filters by EVENT time. + + // First, retrieve event 10's stored time from the database + res, err := logStore.ListEvent(ctx, driver.ListEventRequest{ + TenantID: tenantID, + Limit: 100, + SortOrder: "asc", + TimeFilter: driver.TimeFilter{ + GTE: &farPast, + }, + }) + require.NoError(t, err) + require.GreaterOrEqual(t, len(res.Data), 11, "need at least 11 events") + + // Find event 10's stored event time, truncated to seconds + var storedEvent10Time time.Time + for _, e := range res.Data { + if e.ID == allEvents[10].Event.ID { + storedEvent10Time = e.Time.Truncate(time.Second) + break + } + } + require.False(t, storedEvent10Time.IsZero(), "should find event 10") + + // GT with exact time should exclude event 10 + resGT, err := logStore.ListEvent(ctx, driver.ListEventRequest{ + TenantID: tenantID, + Limit: 100, + SortOrder: "asc", + TimeFilter: driver.TimeFilter{GT: &storedEvent10Time}, + }) + require.NoError(t, err) + + for _, e := range resGT.Data { + eTimeTrunc := e.Time.Truncate(time.Second) + require.True(t, eTimeTrunc.After(storedEvent10Time), + "GT filter should exclude event with exact timestamp, got event %s with time %v (filter time: %v)", + e.ID, eTimeTrunc, storedEvent10Time) + } + + // LT with exact time should exclude event 10 + resLT, err := logStore.ListEvent(ctx, driver.ListEventRequest{ + TenantID: tenantID, + Limit: 100, + SortOrder: "asc", + TimeFilter: driver.TimeFilter{LT: &storedEvent10Time}, + }) + require.NoError(t, err) + + for _, e := range resLT.Data { + eTimeTrunc := e.Time.Truncate(time.Second) + require.True(t, eTimeTrunc.Before(storedEvent10Time), + "LT filter should exclude event with exact timestamp, got event %s with time %v (filter time: %v)", + e.ID, eTimeTrunc, storedEvent10Time) + } + + // Verify event 10 is included with GTE/LTE (inclusive bounds) + resGTE, err := logStore.ListEvent(ctx, driver.ListEventRequest{ + TenantID: tenantID, + Limit: 100, + SortOrder: "asc", + TimeFilter: driver.TimeFilter{GTE: &storedEvent10Time, LTE: &storedEvent10Time}, + }) + require.NoError(t, err) + require.GreaterOrEqual(t, len(resGTE.Data), 1, "GTE/LTE with same time should include event at that second") + }) + }) } From eb09c581f966f3981c774b3858159595978d89c8 Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Tue, 20 Jan 2026 03:27:45 +0700 Subject: [PATCH 11/14] docs: add time field to PublishRequest schema Co-Authored-By: Claude Opus 4.5 --- docs/apis/openapi.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/apis/openapi.yaml b/docs/apis/openapi.yaml index 4d39943d7..39760b654 100644 --- a/docs/apis/openapi.yaml +++ b/docs/apis/openapi.yaml @@ -1677,6 +1677,11 @@ components: eligible_for_retry: type: boolean description: Should event delivery be retried on failure. + time: + type: string + format: date-time + description: Optional. Custom timestamp for the event. If not provided, defaults to the current time. + example: "2024-01-15T10:30:00Z" metadata: type: object description: Any key-value string pairs for metadata. From c57378cfa1d439d5fa3eeacf705fc68c923fe2b0 Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Tue, 20 Jan 2026 03:31:06 +0700 Subject: [PATCH 12/14] feat: reject requests with both next and prev cursors --- internal/apirouter/legacy_handlers.go | 11 +++++- internal/apirouter/log_handlers.go | 22 +++++++++-- internal/apirouter/pagination.go | 22 +++++++++++ internal/apirouter/pagination_test.go | 53 +++++++++++++++++++++++++++ internal/apirouter/tenant_handlers.go | 11 +++++- 5 files changed, 111 insertions(+), 8 deletions(-) diff --git a/internal/apirouter/legacy_handlers.go b/internal/apirouter/legacy_handlers.go index 1e45ede7d..18195f5ee 100644 --- a/internal/apirouter/legacy_handlers.go +++ b/internal/apirouter/legacy_handlers.go @@ -121,6 +121,13 @@ func (h *LegacyHandlers) ListEventsByDestination(c *gin.Context) { } destinationID := c.Param("destinationID") + // Parse and validate cursors (next/prev are mutually exclusive) + cursors, errResp := ParseCursors(c) + if errResp != nil { + AbortWithError(c, errResp.Code, *errResp) + return + } + // Parse pagination params limit := 100 if limitStr := c.Query("limit"); limitStr != "" { @@ -134,8 +141,8 @@ func (h *LegacyHandlers) ListEventsByDestination(c *gin.Context) { TenantID: tenant.ID, DestinationIDs: []string{destinationID}, Limit: limit, - Next: c.Query("next"), - Prev: c.Query("prev"), + Next: cursors.Next, + Prev: cursors.Prev, SortOrder: "desc", }) if err != nil { diff --git a/internal/apirouter/log_handlers.go b/internal/apirouter/log_handlers.go index 0f48a8cbc..7e2ee0b21 100644 --- a/internal/apirouter/log_handlers.go +++ b/internal/apirouter/log_handlers.go @@ -201,6 +201,13 @@ func (h *LogHandlers) ListDeliveries(c *gin.Context) { } func (h *LogHandlers) listDeliveriesInternal(c *gin.Context, tenantID string) { + // Parse and validate cursors (next/prev are mutually exclusive) + cursors, errResp := ParseCursors(c) + if errResp != nil { + AbortWithError(c, errResp.Code, *errResp) + return + } + // Parse and validate dir (sort direction) dir, errResp := ParseDir(c) if errResp != nil { @@ -250,8 +257,8 @@ func (h *LogHandlers) listDeliveriesInternal(c *gin.Context, tenantID string) { LT: deliveryTimeFilter.LT, }, Limit: limit, - Next: c.Query("next"), - Prev: c.Query("prev"), + Next: cursors.Next, + Prev: cursors.Prev, SortOrder: dir, } @@ -362,6 +369,13 @@ func (h *LogHandlers) ListEvents(c *gin.Context) { } func (h *LogHandlers) listEventsInternal(c *gin.Context, tenantID string) { + // Parse and validate cursors (next/prev are mutually exclusive) + cursors, errResp := ParseCursors(c) + if errResp != nil { + AbortWithError(c, errResp.Code, *errResp) + return + } + // Parse and validate dir (sort direction) dir, errResp := ParseDir(c) if errResp != nil { @@ -409,8 +423,8 @@ func (h *LogHandlers) listEventsInternal(c *gin.Context, tenantID string) { LT: eventTimeFilter.LT, }, Limit: limit, - Next: c.Query("next"), - Prev: c.Query("prev"), + Next: cursors.Next, + Prev: cursors.Prev, SortOrder: dir, } diff --git a/internal/apirouter/pagination.go b/internal/apirouter/pagination.go index 0cc0c1249..13971645b 100644 --- a/internal/apirouter/pagination.go +++ b/internal/apirouter/pagination.go @@ -30,6 +30,28 @@ func CursorToPtr(cursor string) *string { return &cursor } +// CursorParams holds the parsed cursor values for pagination. +type CursorParams struct { + Next string + Prev string +} + +// ParseCursors parses the "next" and "prev" query parameters. +// Returns an error if both are provided (mutually exclusive). +func ParseCursors(c *gin.Context) (CursorParams, *ErrorResponse) { + next := c.Query("next") + prev := c.Query("prev") + + if next != "" && prev != "" { + return CursorParams{}, &ErrorResponse{ + Code: http.StatusBadRequest, + Message: "cannot specify both 'next' and 'prev' cursors", + } + } + + return CursorParams{Next: next, Prev: prev}, nil +} + // ParseDir parses the "dir" query parameter for sort direction. // Returns the direction (empty string if not provided) and any validation error response. // Valid values are "asc" and "desc". Caller/store should apply default if empty. diff --git a/internal/apirouter/pagination_test.go b/internal/apirouter/pagination_test.go index 449d8ac46..3a3b32fca 100644 --- a/internal/apirouter/pagination_test.go +++ b/internal/apirouter/pagination_test.go @@ -30,6 +30,59 @@ func TestCursorToPtr(t *testing.T) { }) } +func TestParseCursors(t *testing.T) { + tests := []struct { + name string + queryParams map[string]string + wantNext string + wantPrev string + wantErrCode int + }{ + { + name: "no cursors", + queryParams: map[string]string{}, + wantNext: "", + wantPrev: "", + }, + { + name: "next only", + queryParams: map[string]string{"next": "cursor123"}, + wantNext: "cursor123", + wantPrev: "", + }, + { + name: "prev only", + queryParams: map[string]string{"prev": "cursor456"}, + wantNext: "", + wantPrev: "cursor456", + }, + { + name: "both next and prev returns error", + queryParams: map[string]string{"next": "cursor123", "prev": "cursor456"}, + wantErrCode: http.StatusBadRequest, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c, _ := createTestContext(tt.queryParams) + + result, errResp := apirouter.ParseCursors(c) + + if tt.wantErrCode != 0 { + require.NotNil(t, errResp) + assert.Equal(t, tt.wantErrCode, errResp.Code) + assert.Contains(t, errResp.Message, "both") + return + } + + assert.Nil(t, errResp) + assert.Equal(t, tt.wantNext, result.Next) + assert.Equal(t, tt.wantPrev, result.Prev) + }) + } +} + func TestParseDir(t *testing.T) { tests := []struct { name string diff --git a/internal/apirouter/tenant_handlers.go b/internal/apirouter/tenant_handlers.go index b1fd33eb2..9fc52983d 100644 --- a/internal/apirouter/tenant_handlers.go +++ b/internal/apirouter/tenant_handlers.go @@ -96,6 +96,13 @@ func (h *TenantHandlers) Retrieve(c *gin.Context) { } func (h *TenantHandlers) List(c *gin.Context) { + // Parse and validate cursors (next/prev are mutually exclusive) + cursors, errResp := ParseCursors(c) + if errResp != nil { + AbortWithError(c, errResp.Code, *errResp) + return + } + // Parse and validate dir (sort direction) dir, errResp := ParseDir(c) if errResp != nil { @@ -104,8 +111,8 @@ func (h *TenantHandlers) List(c *gin.Context) { } req := models.ListTenantRequest{ - Next: c.Query("next"), - Prev: c.Query("prev"), + Next: cursors.Next, + Prev: cursors.Prev, Dir: dir, } From 7f4d69e9666484d954d97761c37620c0d5b1e7d5 Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Tue, 20 Jan 2026 03:35:15 +0700 Subject: [PATCH 13/14] fix: return literal ["*"] for tenant topics with wildcard destination Previously expanded to availableTopics list, now returns ["*"] directly. --- internal/models/entity.go | 2 +- internal/models/entitysuite_test.go | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/models/entity.go b/internal/models/entity.go index ddfc152a2..d60547bd1 100644 --- a/internal/models/entity.go +++ b/internal/models/entity.go @@ -895,7 +895,7 @@ func (s *entityStoreImpl) parseTenantTopics(destinationSummaryList []Destination } if all { - return s.availableTopics + return []string{"*"} } topics := make([]string, 0, len(topicsSet)) diff --git a/internal/models/entitysuite_test.go b/internal/models/entitysuite_test.go index 17b6a6986..ff025a3df 100644 --- a/internal/models/entitysuite_test.go +++ b/internal/models/entitysuite_test.go @@ -542,10 +542,12 @@ func (s *EntityTestSuite) TestMultiDestinationRetrieveTenantDestinationsCount() func (s *EntityTestSuite) TestMultiDestinationRetrieveTenantTopics() { data := s.setupMultiDestination() + // destinations[0] has topics ["*"], so tenant.Topics should be ["*"] tenant, err := s.entityStore.RetrieveTenant(s.ctx, data.tenant.ID) require.NoError(s.T(), err) - require.Equal(s.T(), []string{"user.created", "user.deleted", "user.updated"}, tenant.Topics) + require.Equal(s.T(), []string{"*"}, tenant.Topics) + // After deleting the wildcard destination, tenant.Topics should aggregate remaining topics require.NoError(s.T(), s.entityStore.DeleteDestination(s.ctx, data.tenant.ID, data.destinations[0].ID)) tenant, err = s.entityStore.RetrieveTenant(s.ctx, data.tenant.ID) require.NoError(s.T(), err) From f2935ae431e05e37f2e655768484d55ff7d8c880 Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Tue, 20 Jan 2026 03:40:04 +0700 Subject: [PATCH 14/14] docs: improve date format error message with examples Show example timestamps in the validation error to make it clearer that millisecond precision (RFC3339Nano) is also supported. Co-Authored-By: Claude Opus 4.5 --- internal/apirouter/pagination.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/apirouter/pagination.go b/internal/apirouter/pagination.go index 13971645b..aa96a7b04 100644 --- a/internal/apirouter/pagination.go +++ b/internal/apirouter/pagination.go @@ -231,7 +231,7 @@ func dateFormatError(key string) *ErrorResponse { Code: http.StatusUnprocessableEntity, Message: "validation error", Data: map[string]string{ - "query." + key: "invalid format, expected RFC3339 or YYYY-MM-DD", + "query." + key: "invalid format, expected RFC3339 (e.g. 2024-01-15T09:30:00Z or 2024-01-15T09:30:00.123Z) or YYYY-MM-DD", }, } }