diff --git a/internal/assets/openapi.json b/internal/assets/openapi.json index 5aec428..998b556 100644 --- a/internal/assets/openapi.json +++ b/internal/assets/openapi.json @@ -773,14 +773,14 @@ "in": "query", "required": false, "schema": { "type": "string" }, - "description": "Filter by client network (requires client_type)" + "description": "Filter by client network (requires client_type). Returns 400 if client_type is not also provided." }, { "name": "client_channel", "in": "query", "required": false, "schema": { "type": "string" }, - "description": "Filter by client channel (requires client_type and client_network)" + "description": "Filter by client channel (requires client_type and client_network). Returns 400 if dependencies are missing." } ], "responses": { @@ -800,6 +800,16 @@ } } }, + "400": { + "description": "Invalid client filter parameters (e.g., client_network without client_type)", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/APIError" + } + } + } + }, "500": { "description": "Server error", "content": { @@ -828,7 +838,7 @@ }, "responses": { "201": { - "description": "Link created successfully. If duplicate detection finds a prior submission (scoped per client when client fields are provided), the response includes is_duplicate=true and previous_submissions. Returns 208 instead when the link is an exact duplicate within the same client scope.", + "description": "Link created successfully. If duplicate detection finds a prior submission (scoped per client when client fields are provided), the response includes is_duplicate=true and previous_submissions.", "content": { "application/json": { "schema": { @@ -1016,14 +1026,14 @@ "in": "query", "required": false, "schema": { "type": "string" }, - "description": "Filter by client network (requires client_type)" + "description": "Filter by client network (requires client_type). Returns 400 if client_type is not also provided." }, { "name": "client_channel", "in": "query", "required": false, "schema": { "type": "string" }, - "description": "Filter by client channel (requires client_type and client_network)" + "description": "Filter by client channel (requires client_type and client_network). Returns 400 if dependencies are missing." } ], "responses": { @@ -1043,6 +1053,16 @@ } } }, + "400": { + "description": "Invalid client filter parameters (e.g., client_network without client_type)", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/APIError" + } + } + } + }, "500": { "description": "Server error", "content": { @@ -1843,14 +1863,14 @@ "in": "query", "required": false, "schema": { "type": "string" }, - "description": "Filter by client network (requires client_type)" + "description": "Filter by client network (requires client_type). Returns 400 if client_type is not also provided." }, { "name": "client_channel", "in": "query", "required": false, "schema": { "type": "string" }, - "description": "Filter by client channel (requires client_type and client_network)" + "description": "Filter by client channel (requires client_type and client_network). Returns 400 if dependencies are missing." } ], "responses": { @@ -1870,6 +1890,16 @@ } } }, + "400": { + "description": "Invalid client filter parameters (e.g., client_network without client_type)", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/APIError" + } + } + } + }, "422": { "description": "Validation error (query too short)", "content": { diff --git a/internal/data/client_filter_test.go b/internal/data/client_filter_test.go index b27382c..3716f98 100644 --- a/internal/data/client_filter_test.go +++ b/internal/data/client_filter_test.go @@ -50,6 +50,63 @@ func TestClientFilter_IsEmpty(t *testing.T) { } } +func TestClientFilter_Validate(t *testing.T) { + tests := []struct { + name string + filter ClientFilter + wantError bool + }{ + { + name: "empty filter is valid", + filter: ClientFilter{}, + }, + { + name: "type only is valid", + filter: ClientFilter{ClientType: strPtr("irc")}, + }, + { + name: "type and network is valid", + filter: ClientFilter{ClientType: strPtr("irc"), ClientNetwork: strPtr("libera")}, + }, + { + name: "all three is valid", + filter: ClientFilter{ClientType: strPtr("irc"), ClientNetwork: strPtr("libera"), ClientChannel: strPtr("#general")}, + }, + { + name: "network without type is invalid", + filter: ClientFilter{ClientNetwork: strPtr("libera")}, + wantError: true, + }, + { + name: "channel without type is invalid", + filter: ClientFilter{ClientChannel: strPtr("#general")}, + wantError: true, + }, + { + name: "channel without network is invalid", + filter: ClientFilter{ClientType: strPtr("irc"), ClientChannel: strPtr("#general")}, + wantError: true, + }, + { + name: "channel and network without type is invalid", + filter: ClientFilter{ClientNetwork: strPtr("libera"), ClientChannel: strPtr("#general")}, + wantError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.filter.Validate() + if tt.wantError && err == nil { + t.Error("expected error, got nil") + } + if !tt.wantError && err != nil { + t.Errorf("expected no error, got %v", err) + } + }) + } +} + func strPtr(s string) *string { return &s } diff --git a/internal/data/gorm_store.go b/internal/data/gorm_store.go index 9d2435f..6eda258 100644 --- a/internal/data/gorm_store.go +++ b/internal/data/gorm_store.go @@ -277,6 +277,18 @@ func (s *GormStore) DeleteQuote(ctx context.Context, id int) error { return nil } +func (s *GormStore) CountIRCLinks(ctx context.Context) (int64, error) { + var count int64 + err := s.db.WithContext(ctx).Model(&IRCLink{}).Count(&count).Error + return count, err +} + +func (s *GormStore) CountQuotes(ctx context.Context) (int64, error) { + var count int64 + err := s.db.WithContext(ctx).Model(&Quote{}).Count(&count).Error + return count, err +} + func (s *GormStore) GetUserStats(ctx context.Context, sortBy string, limit int, offset int) ([]UserStat, error) { var stats []UserStat diff --git a/internal/data/store.go b/internal/data/store.go index 82c2067..6669450 100644 --- a/internal/data/store.go +++ b/internal/data/store.go @@ -2,6 +2,7 @@ package data import ( "context" + "fmt" "time" ) @@ -95,6 +96,18 @@ func (f ClientFilter) IsEmpty() bool { return f.ClientType == nil && f.ClientNetwork == nil && f.ClientChannel == nil } +// Validate checks that hierarchical filter dependencies are satisfied. +// client_network requires client_type, and client_channel requires both. +func (f ClientFilter) Validate() error { + if f.ClientNetwork != nil && f.ClientType == nil { + return fmt.Errorf("client_network requires client_type") + } + if f.ClientChannel != nil && (f.ClientType == nil || f.ClientNetwork == nil) { + return fmt.Errorf("client_channel requires client_type and client_network") + } + return nil +} + type Tag struct { ID int `json:"id" gorm:"column:id;primaryKey;autoIncrement"` Tag string `json:"tag" gorm:"column:tag;type:varchar(255);index"` @@ -153,6 +166,8 @@ type Store interface { DeleteQuote(ctx context.Context, id int) error // Stats + CountIRCLinks(ctx context.Context) (int64, error) + CountQuotes(ctx context.Context) (int64, error) GetUserStats(ctx context.Context, sortBy string, limit int, offset int) ([]UserStat, error) GetLinksByUser(ctx context.Context, user string, limit int, offset int) ([]IRCLink, error) GetUserTimeline(ctx context.Context, user string, filterType string, limit int, offset int) ([]TimelineItem, error) diff --git a/internal/handler/api_v1_integration_test.go b/internal/handler/api_v1_integration_test.go index fe7989d..8d69a8b 100644 --- a/internal/handler/api_v1_integration_test.go +++ b/internal/handler/api_v1_integration_test.go @@ -107,6 +107,20 @@ func (m *integrationMockStore) DeleteQuote(ctx context.Context, id int) error { return m.err } +func (m *integrationMockStore) CountIRCLinks(ctx context.Context) (int64, error) { + if m.err != nil { + return 0, m.err + } + return int64(len(m.links)), nil +} + +func (m *integrationMockStore) CountQuotes(ctx context.Context) (int64, error) { + if m.err != nil { + return 0, m.err + } + return int64(len(m.quotes)), nil +} + func (m *integrationMockStore) GetUserStats(ctx context.Context, sortBy string, limit int, offset int) ([]data.UserStat, error) { if m.err != nil { return nil, m.err diff --git a/internal/handler/api_v1_links.go b/internal/handler/api_v1_links.go index c05ff9b..3774052 100644 --- a/internal/handler/api_v1_links.go +++ b/internal/handler/api_v1_links.go @@ -81,6 +81,10 @@ func (h *Handler) apiV1ListLinks(w http.ResponseWriter, r *http.Request) { if sc := r.URL.Query().Get("client_channel"); sc != "" { clientFilter.ClientChannel = &sc } + if err := clientFilter.Validate(); err != nil { + writeAPIError(w, http.StatusBadRequest, "invalid_params", err.Error()) + return + } // Fetch all links from the last year // We fetch more than needed so we can paginate in-memory diff --git a/internal/handler/api_v1_quotes.go b/internal/handler/api_v1_quotes.go index 40e2be2..deefe8f 100644 --- a/internal/handler/api_v1_quotes.go +++ b/internal/handler/api_v1_quotes.go @@ -81,6 +81,10 @@ func (h *Handler) apiV1ListQuotes(w http.ResponseWriter, r *http.Request) { if sc := r.URL.Query().Get("client_channel"); sc != "" { clientFilter.ClientChannel = &sc } + if err := clientFilter.Validate(); err != nil { + writeAPIError(w, http.StatusBadRequest, "invalid_params", err.Error()) + return + } // Fetch all quotes from the last year // We fetch more than needed so we can paginate in-memory diff --git a/internal/handler/api_v1_search.go b/internal/handler/api_v1_search.go index 7a16e2c..84e91d2 100644 --- a/internal/handler/api_v1_search.go +++ b/internal/handler/api_v1_search.go @@ -74,6 +74,10 @@ func (h *Handler) APIv1SearchHandler(w http.ResponseWriter, r *http.Request) { if sc := r.URL.Query().Get("client_channel"); sc != "" { clientFilter.ClientChannel = &sc } + if err := clientFilter.Validate(); err != nil { + writeAPIError(w, http.StatusBadRequest, "invalid_params", err.Error()) + return + } // Initialize response resp := APISearchResponse{ diff --git a/internal/handler/api_v1_stats.go b/internal/handler/api_v1_stats.go index edf804c..c57bf90 100644 --- a/internal/handler/api_v1_stats.go +++ b/internal/handler/api_v1_stats.go @@ -4,8 +4,6 @@ import ( "fmt" "net/http" "strings" - - "tumble/internal/data" ) // APIv1StatsHandler routes requests to /api/v1/stats endpoint. @@ -62,15 +60,15 @@ func (h *Handler) apiV1GetStats(w http.ResponseWriter, r *http.Request) { } // Get total links and quotes for site stats - links, err := h.Store.GetRecentIRCLinks(ctx, 36500, 0, data.ClientFilter{}) // ~100 years to get all + totalLinks, err := h.Store.CountIRCLinks(ctx) if err != nil { - writeAPIError(w, http.StatusInternalServerError, "internal_error", "Failed to fetch links") + writeAPIError(w, http.StatusInternalServerError, "internal_error", "Failed to count links") return } - quotes, err := h.Store.GetRecentQuotes(ctx, 36500, 0, data.ClientFilter{}) // ~100 years to get all + totalQuotes, err := h.Store.CountQuotes(ctx) if err != nil { - writeAPIError(w, http.StatusInternalServerError, "internal_error", "Failed to fetch quotes") + writeAPIError(w, http.StatusInternalServerError, "internal_error", "Failed to count quotes") return } @@ -86,8 +84,8 @@ func (h *Handler) apiV1GetStats(w http.ResponseWriter, r *http.Request) { resp := APIStatsResponse{ Site: APISiteStats{ - TotalLinks: len(links), - TotalQuotes: len(quotes), + TotalLinks: int(totalLinks), + TotalQuotes: int(totalQuotes), TotalUsers: totalUsers, }, Leaderboard: leaderboardData, diff --git a/internal/handler/api_v1_stats_test.go b/internal/handler/api_v1_stats_test.go index 77cd47d..5521891 100644 --- a/internal/handler/api_v1_stats_test.go +++ b/internal/handler/api_v1_stats_test.go @@ -16,8 +16,8 @@ type mockStatsStore struct { data.Store userStats []data.UserStat userStatsFn func(sortBy string, limit int, offset int) ([]data.UserStat, error) - links []data.IRCLink - quotes []data.Quote + linkCount int64 + quoteCount int64 err error } @@ -31,18 +31,18 @@ func (m *mockStatsStore) GetUserStats(ctx context.Context, sortBy string, limit return m.userStats, nil } -func (m *mockStatsStore) GetRecentIRCLinks(ctx context.Context, days int, offsetDays int, filter data.ClientFilter) ([]data.IRCLink, error) { +func (m *mockStatsStore) CountIRCLinks(ctx context.Context) (int64, error) { if m.err != nil { - return nil, m.err + return 0, m.err } - return m.links, nil + return m.linkCount, nil } -func (m *mockStatsStore) GetRecentQuotes(ctx context.Context, days int, offsetDays int, filter data.ClientFilter) ([]data.Quote, error) { +func (m *mockStatsStore) CountQuotes(ctx context.Context) (int64, error) { if m.err != nil { - return nil, m.err + return 0, m.err } - return m.quotes, nil + return m.quoteCount, nil } func TestAPIv1_Stats(t *testing.T) { @@ -51,8 +51,8 @@ func TestAPIv1_Stats(t *testing.T) { method string path string userStats []data.UserStat - links []data.IRCLink - quotes []data.Quote + linkCount int64 + quoteCount int64 storeErr error expectedStatus int checkBody func(t *testing.T, body []byte) @@ -62,8 +62,8 @@ func TestAPIv1_Stats(t *testing.T) { method: http.MethodGet, path: "/api/v1/stats", userStats: []data.UserStat{}, - links: []data.IRCLink{}, - quotes: []data.Quote{}, + linkCount: 0, + quoteCount: 0, expectedStatus: http.StatusOK, checkBody: func(t *testing.T, body []byte) { var resp APIStatsResponse @@ -101,8 +101,8 @@ func TestAPIv1_Stats(t *testing.T) { {User: "alice", LinkCount: 500, QuoteCount: 120}, {User: "bob", LinkCount: 300, QuoteCount: 80}, }, - links: make([]data.IRCLink, 15000), - quotes: make([]data.Quote, 3200), + linkCount: 15000, + quoteCount: 3200, expectedStatus: http.StatusOK, checkBody: func(t *testing.T, body []byte) { var resp APIStatsResponse @@ -143,8 +143,6 @@ func TestAPIv1_Stats(t *testing.T) { {User: "alice", LinkCount: 500, QuoteCount: 120}, {User: "bob", LinkCount: 300, QuoteCount: 80}, }, - links: []data.IRCLink{}, - quotes: []data.Quote{}, expectedStatus: http.StatusOK, checkBody: func(t *testing.T, body []byte) { var resp APIStatsResponse @@ -171,8 +169,6 @@ func TestAPIv1_Stats(t *testing.T) { {User: "alice", LinkCount: 500, QuoteCount: 120}, {User: "bob", LinkCount: 300, QuoteCount: 80}, }, - links: []data.IRCLink{}, - quotes: []data.Quote{}, expectedStatus: http.StatusOK, checkBody: func(t *testing.T, body []byte) { var resp APIStatsResponse @@ -195,8 +191,6 @@ func TestAPIv1_Stats(t *testing.T) { method: http.MethodGet, path: "/api/v1/stats?limit=5000", userStats: []data.UserStat{}, - links: []data.IRCLink{}, - quotes: []data.Quote{}, expectedStatus: http.StatusOK, checkBody: func(t *testing.T, body []byte) { var resp APIStatsResponse @@ -213,8 +207,6 @@ func TestAPIv1_Stats(t *testing.T) { method: http.MethodGet, path: "/api/v1/stats?limit=abc", userStats: []data.UserStat{}, - links: []data.IRCLink{}, - quotes: []data.Quote{}, expectedStatus: http.StatusOK, checkBody: func(t *testing.T, body []byte) { var resp APIStatsResponse @@ -233,8 +225,6 @@ func TestAPIv1_Stats(t *testing.T) { userStats: []data.UserStat{ {User: "alice", LinkCount: 500, QuoteCount: 120}, }, - links: []data.IRCLink{}, - quotes: []data.Quote{}, expectedStatus: http.StatusOK, checkBody: func(t *testing.T, body []byte) { var resp APIStatsResponse @@ -254,8 +244,6 @@ func TestAPIv1_Stats(t *testing.T) { method: http.MethodPost, path: "/api/v1/stats", userStats: []data.UserStat{}, - links: []data.IRCLink{}, - quotes: []data.Quote{}, expectedStatus: http.StatusMethodNotAllowed, checkBody: func(t *testing.T, body []byte) { var resp APIErrorResponse @@ -274,8 +262,6 @@ func TestAPIv1_Stats(t *testing.T) { userStats: []data.UserStat{ {User: "alice", LinkCount: 500, QuoteCount: 120}, }, - links: []data.IRCLink{}, - quotes: []data.Quote{}, expectedStatus: http.StatusOK, checkBody: func(t *testing.T, body []byte) { var resp APIStatsResponse @@ -292,10 +278,10 @@ func TestAPIv1_Stats(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { store := &mockStatsStore{ - userStats: tt.userStats, - links: tt.links, - quotes: tt.quotes, - err: tt.storeErr, + userStats: tt.userStats, + linkCount: tt.linkCount, + quoteCount: tt.quoteCount, + err: tt.storeErr, } handler := &Handler{ Store: store,