From 1b596367c4869f0a83d76e8944851e98c9b5c7cf Mon Sep 17 00:00:00 2001 From: jevb Date: Tue, 17 Mar 2026 03:20:37 +0100 Subject: [PATCH] fix: address PR review findings (issues #3-#8) - Fix double-close panic in Hub.Stop/GracefulStop using sync.Once (#3) - Bump golangci-lint action to v9 with v2.11.3 for Go 1.25 support (#4) - Add input validation guards to SearchMessages (#5) - Handle promise rejections in InviteManager with error toasts (#6) - Add missing reply_to and edited_at columns to admin test schema (#7) - Add ClientCount to HubBroadcaster interface and wire into stats endpoint (#8) --- .github/workflows/ci.yml | 3 ++- Client/tauri-client/src/components/InviteManager.ts | 5 +++++ .../tauri-client/src/pages/main-page/OverlayManagers.ts | 4 ++++ Server/admin/api.go | 3 ++- Server/admin/api_test.go | 9 ++++++++- Server/admin/handlers_users.go | 3 ++- Server/db/message_queries.go | 7 +++++++ Server/db/models.go | 1 + Server/ws/hub.go | 7 ++++--- 9 files changed, 35 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a2289fec..b2d70643 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,8 +40,9 @@ jobs: retention-days: 7 - name: Lint - uses: golangci/golangci-lint-action@v6 + uses: golangci/golangci-lint-action@v9 with: + version: v2.11.3 working-directory: Server/ client-check: diff --git a/Client/tauri-client/src/components/InviteManager.ts b/Client/tauri-client/src/components/InviteManager.ts index 355c11c1..062c2dbc 100644 --- a/Client/tauri-client/src/components/InviteManager.ts +++ b/Client/tauri-client/src/components/InviteManager.ts @@ -25,6 +25,7 @@ export interface InviteManagerOptions { onRevokeInvite(code: string): Promise; onCopyLink(code: string): void; onClose(): void; + onError?(message: string): void; } // --------------------------------------------------------------------------- @@ -82,6 +83,8 @@ export function createInviteManager( void options.onRevokeInvite(invite.code).then(() => { invites = invites.filter((i) => i.code !== invite.code); renderList(); + }).catch(() => { + options.onError?.("Failed to revoke invite"); }); }, { signal: ac.signal }); @@ -114,6 +117,8 @@ export function createInviteManager( void options.onCreateInvite().then((newInvite) => { invites = [...invites, newInvite]; renderList(); + }).catch(() => { + options.onError?.("Failed to create invite"); }); }, { signal: ac.signal }); diff --git a/Client/tauri-client/src/pages/main-page/OverlayManagers.ts b/Client/tauri-client/src/pages/main-page/OverlayManagers.ts index 6a56002b..1f6bbe2b 100644 --- a/Client/tauri-client/src/pages/main-page/OverlayManagers.ts +++ b/Client/tauri-client/src/pages/main-page/OverlayManagers.ts @@ -159,6 +159,10 @@ export function createInviteManagerController(opts: { void navigator.clipboard.writeText(code); }, onClose: close, + onError: (message: string) => { + log.error(message); + opts.getToast()?.show(message, "error"); + }, }); if (root !== null) { instance.mount(root); diff --git a/Server/admin/api.go b/Server/admin/api.go index 0405661e..b5603984 100644 --- a/Server/admin/api.go +++ b/Server/admin/api.go @@ -49,6 +49,7 @@ type HubBroadcaster interface { BroadcastChannelDelete(channelID int64) BroadcastMemberBan(userID int64) BroadcastMemberUpdate(userID int64, roleName string) + ClientCount() int } // ─── adminUserResponse ────────────────────────────────────────────────────── @@ -124,7 +125,7 @@ func NewAdminAPI(database *db.DB, version string, hub HubBroadcaster, u *updater r.Group(func(r chi.Router) { r.Use(adminAuthMiddleware(database)) - r.Get("/stats", handleGetStats(database)) + r.Get("/stats", handleGetStats(database, hub)) r.Get("/users", handleListUsers(database)) r.Patch("/users/{id}", handlePatchUser(database, hub)) r.Delete("/users/{id}/sessions", handleForceLogout(database)) diff --git a/Server/admin/api_test.go b/Server/admin/api_test.go index f303722f..043f41b2 100644 --- a/Server/admin/api_test.go +++ b/Server/admin/api_test.go @@ -81,7 +81,9 @@ CREATE TABLE IF NOT EXISTS messages ( content TEXT NOT NULL, deleted INTEGER NOT NULL DEFAULT 0, pinned INTEGER NOT NULL DEFAULT 0, - timestamp TEXT NOT NULL DEFAULT (datetime('now')) + timestamp TEXT NOT NULL DEFAULT (datetime('now')), + reply_to INTEGER REFERENCES messages(id) ON DELETE SET NULL, + edited_at TEXT ); CREATE TABLE IF NOT EXISTS invites ( @@ -972,6 +974,7 @@ type mockHub struct { channelDeleteIDs []int64 memberBanIDs []int64 memberUpdates []memberUpdateCall + clientCount int } type memberUpdateCall struct { @@ -1008,6 +1011,10 @@ func (m *mockHub) BroadcastMemberUpdate(userID int64, roleName string) { m.memberUpdates = append(m.memberUpdates, memberUpdateCall{userID, roleName}) } +func (m *mockHub) ClientCount() int { + return m.clientCount +} + func TestAdminAPI_CreateChannel_BroadcastsChannelCreate(t *testing.T) { database := openAdminTestDB(t) hub := &mockHub{} diff --git a/Server/admin/handlers_users.go b/Server/admin/handlers_users.go index 3eff9f32..a06be0cf 100644 --- a/Server/admin/handlers_users.go +++ b/Server/admin/handlers_users.go @@ -11,13 +11,14 @@ import ( // ─── User Handlers ─────────────────────────────────────────────────────────── -func handleGetStats(database *db.DB) http.HandlerFunc { +func handleGetStats(database *db.DB, hub HubBroadcaster) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { stats, err := database.GetServerStats() if err != nil { writeErr(w, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to get stats") return } + stats.OnlineCount = hub.ClientCount() writeJSON(w, http.StatusOK, stats) } } diff --git a/Server/db/message_queries.go b/Server/db/message_queries.go index 2ccb2ae5..f1e2414d 100644 --- a/Server/db/message_queries.go +++ b/Server/db/message_queries.go @@ -186,6 +186,13 @@ func (d *DB) GetReactions(messageID int64) ([]ReactionCount, error) { // When channelID is non-nil the search is scoped to that channel. // Deleted messages are excluded from results. func (d *DB) SearchMessages(query string, channelID *int64, limit int) ([]MessageSearchResult, error) { + if query == "" { + return []MessageSearchResult{}, nil + } + if limit < 1 { + return []MessageSearchResult{}, nil + } + var ( rows *sql.Rows err error diff --git a/Server/db/models.go b/Server/db/models.go index 0360819f..e7e70132 100644 --- a/Server/db/models.go +++ b/Server/db/models.go @@ -170,6 +170,7 @@ type ServerStats struct { ChannelCount int64 `json:"channel_count"` InviteCount int64 `json:"invite_count"` DBSizeBytes int64 `json:"db_size_bytes"` + OnlineCount int `json:"online_count"` } // UserWithRole extends User with the name of the user's role. diff --git a/Server/ws/hub.go b/Server/ws/hub.go index eaa02b9d..7d9de905 100644 --- a/Server/ws/hub.go +++ b/Server/ws/hub.go @@ -26,6 +26,7 @@ type Hub struct { register chan *Client unregister chan *Client stop chan struct{} + stopOnce sync.Once sfu *SFU voiceRooms map[int64]*VoiceRoom voiceRoomsMu sync.RWMutex @@ -149,9 +150,9 @@ func (h *Hub) Run() { } } -// Stop signals Run to exit. +// Stop signals Run to exit. Safe to call multiple times. func (h *Hub) Stop() { - close(h.stop) + h.stopOnce.Do(func() { close(h.stop) }) } // GracefulStop closes all PeerConnections, voice rooms, and then stops the hub. @@ -166,7 +167,7 @@ func (h *Hub) GracefulStop() { h.mu.RUnlock() h.CloseAllVoiceRooms() - close(h.stop) + h.stopOnce.Do(func() { close(h.stop) }) } // CleanupVoiceForChannel removes the voice room for the given channel and