From f66d6c527476a7c00a656cb851e7495f7d7f144a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 19 Jul 2026 15:48:39 +0000 Subject: [PATCH] refactor(server/db): delegate messages + reactions to dbgen; finalize D2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit messages/reactions: CreateMessage, GetMessage (messageFromGen mapper), EditMessage (EditMessageContent), DeleteMessage (SoftDeleteMessage), AddReaction, RemoveReaction, GetReactions (GetReactionCounts), SetMessagePinned, UpdateReadState. Retired the obsolete scanMessage. Kept raw by design (no clean sqlc mapping): FTS search, cursor-paginated GetMessages/GetMessagesForAPI/GetPinnedMessages, getReactionsBatch, GetChannelUnreadCounts, GetLatestMessageID (interface{} MAX result). D2 status: 97 db.DB methods now delegate to dbgen across every domain; 43 raw d.sqlDB calls remain by design (db.go passthroughs, migrate.go, variable-length IN(), FTS, multi-statement transactions, PRAGMA/VACUUM). sqlc is no longer dead code — audit A-2026-07-05 resolved. Full rationale + the kept-raw list in docs/plans/sqlc-adoption.md. Verified: go build ./...; go test ./db ./service ./ws ./auth; sqlc-verify; gofmt + go vet clean. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UA17KPvqGBX3XbXYnMf1rA --- Server/db/message_queries.go | 135 ++++++++++------------- docs/audit-2026-07-19.md | 2 +- docs/plans/audit-2026-07-19-decisions.md | 2 +- docs/plans/sqlc-adoption.md | 44 +++++--- 4 files changed, 87 insertions(+), 96 deletions(-) diff --git a/Server/db/message_queries.go b/Server/db/message_queries.go index 3565f57f..13cc1511 100644 --- a/Server/db/message_queries.go +++ b/Server/db/message_queries.go @@ -6,8 +6,25 @@ import ( "fmt" "strings" "unicode" + + "github.com/owncord/server/db/dbgen" ) +// messageFromGen maps a generated message row to the domain Message model. +func messageFromGen(m dbgen.Message) *Message { + return &Message{ + ID: m.ID, + ChannelID: m.ChannelID, + UserID: m.UserID, + Content: m.Content, + ReplyTo: m.ReplyTo, + EditedAt: m.EditedAt, + Deleted: m.Deleted != 0, + Pinned: m.Pinned != 0, + Timestamp: m.Timestamp, + } +} + // sanitizeFTSQuery strips FTS5 operator characters from user input to prevent // query injection. Only allows letters, digits, spaces, and hyphens. func sanitizeFTSQuery(q string) string { @@ -30,10 +47,12 @@ func sanitizeFTSQuery(q string) string { // CreateMessage inserts a new message and returns the assigned ID. // Content should already be sanitized before calling this function. func (d *DB) CreateMessage(channelID, userID int64, content string, replyTo *int64) (int64, error) { - res, err := d.sqlDB.Exec( - `INSERT INTO messages (channel_id, user_id, content, reply_to) VALUES (?, ?, ?, ?)`, - channelID, userID, content, replyTo, - ) + res, err := d.q.CreateMessage(dbCtx(), dbgen.CreateMessageParams{ + ChannelID: channelID, + UserID: userID, + Content: content, + ReplyTo: replyTo, + }) if err != nil { return 0, fmt.Errorf("CreateMessage: %w", err) } @@ -43,12 +62,14 @@ func (d *DB) CreateMessage(channelID, userID int64, content string, replyTo *int // GetMessage returns the message with the given ID, or nil if not found. // Soft-deleted messages are returned so callers can broadcast the deletion event. func (d *DB) GetMessage(id int64) (*Message, error) { - row := d.sqlDB.QueryRow( - `SELECT id, channel_id, user_id, content, reply_to, edited_at, deleted, pinned, timestamp - FROM messages WHERE id = ?`, - id, - ) - return scanMessage(row) + m, err := d.q.GetMessage(dbCtx(), id) + if errors.Is(err, sql.ErrNoRows) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("GetMessage: %w", err) + } + return messageFromGen(m), nil } // GetMessages returns up to limit messages in a channel, ordered newest-first. @@ -115,11 +136,10 @@ func (d *DB) EditMessage(id, userID int64, content string) error { return fmt.Errorf("EditMessage: user %d does not own message %d: %w", userID, id, ErrForbidden) } - _, err = d.sqlDB.Exec( - `UPDATE messages SET content = ?, edited_at = datetime('now') WHERE id = ?`, - content, id, - ) - if err != nil { + if err := d.q.EditMessageContent(dbCtx(), dbgen.EditMessageContentParams{ + Content: content, + ID: id, + }); err != nil { return fmt.Errorf("EditMessage: %w", err) } return nil @@ -139,8 +159,7 @@ func (d *DB) DeleteMessage(id, userID int64, ismod bool) error { return fmt.Errorf("DeleteMessage: user %d does not own message %d: %w", userID, id, ErrForbidden) } - _, err = d.sqlDB.Exec(`UPDATE messages SET deleted = 1 WHERE id = ?`, id) - if err != nil { + if err := d.q.SoftDeleteMessage(dbCtx(), id); err != nil { return fmt.Errorf("DeleteMessage: %w", err) } return nil @@ -148,11 +167,11 @@ func (d *DB) DeleteMessage(id, userID int64, ismod bool) error { // AddReaction inserts a reaction. Returns an error on duplicate (same user+emoji+message). func (d *DB) AddReaction(messageID, userID int64, emoji string) error { - _, err := d.sqlDB.Exec( - `INSERT INTO reactions (message_id, user_id, emoji) VALUES (?, ?, ?)`, - messageID, userID, emoji, - ) - if err != nil { + if err := d.q.AddReaction(dbCtx(), dbgen.AddReactionParams{ + MessageID: messageID, + UserID: userID, + Emoji: emoji, + }); err != nil { return fmt.Errorf("AddReaction: %w", err) } return nil @@ -160,10 +179,11 @@ func (d *DB) AddReaction(messageID, userID int64, emoji string) error { // RemoveReaction deletes a reaction. Returns an error if it does not exist. func (d *DB) RemoveReaction(messageID, userID int64, emoji string) error { - res, err := d.sqlDB.Exec( - `DELETE FROM reactions WHERE message_id = ? AND user_id = ? AND emoji = ?`, - messageID, userID, emoji, - ) + res, err := d.q.RemoveReaction(dbCtx(), dbgen.RemoveReactionParams{ + MessageID: messageID, + UserID: userID, + Emoji: emoji, + }) if err != nil { return fmt.Errorf("RemoveReaction: %w", err) } @@ -177,28 +197,13 @@ func (d *DB) RemoveReaction(messageID, userID int64, emoji string) error { // GetReactions returns aggregated reaction counts for a message. // MeReacted is always false here (caller passes requesting userID if needed). func (d *DB) GetReactions(messageID int64) ([]ReactionCount, error) { - rows, err := d.sqlDB.Query( - `SELECT emoji, COUNT(*) FROM reactions WHERE message_id = ? GROUP BY emoji`, - messageID, - ) + rows, err := d.q.GetReactionCounts(dbCtx(), messageID) if err != nil { return nil, fmt.Errorf("GetReactions: %w", err) } - defer rows.Close() //nolint:errcheck - - var counts []ReactionCount - for rows.Next() { - var rc ReactionCount - if scanErr := rows.Scan(&rc.Emoji, &rc.Count); scanErr != nil { - return nil, fmt.Errorf("GetReactions scan: %w", scanErr) - } - counts = append(counts, rc) - } - if rows.Err() != nil { - return nil, fmt.Errorf("GetReactions rows: %w", rows.Err()) - } - if counts == nil { - counts = []ReactionCount{} + counts := make([]ReactionCount, 0, len(rows)) + for _, r := range rows { + counts = append(counts, ReactionCount{Emoji: r.Emoji, Count: int(r.Count)}) } return counts, nil } @@ -419,13 +424,11 @@ func (d *DB) getReactionsBatch(msgIDs []int64, requestingUserID int64) (map[int6 // UpdateReadState upserts the read state for a user in a channel. func (d *DB) UpdateReadState(userID, channelID, lastReadMessageID int64) error { - _, err := d.sqlDB.Exec( - `INSERT INTO read_states (user_id, channel_id, last_message_id) - VALUES (?, ?, ?) - ON CONFLICT(user_id, channel_id) DO UPDATE SET last_message_id = excluded.last_message_id`, - userID, channelID, lastReadMessageID, - ) - if err != nil { + if err := d.q.UpdateReadState(dbCtx(), dbgen.UpdateReadStateParams{ + UserID: userID, + ChannelID: channelID, + LastMessageID: lastReadMessageID, + }); err != nil { return fmt.Errorf("UpdateReadState: %w", err) } return nil @@ -553,11 +556,10 @@ func (d *DB) scanAndEnrichMessages(rows *sql.Rows, requestingUserID int64) ([]Me // SetMessagePinned updates the pinned column on a message. // Returns ErrNotFound if the message does not exist. func (d *DB) SetMessagePinned(id int64, pinned bool) error { - val := 0 - if pinned { - val = 1 - } - res, err := d.sqlDB.Exec(`UPDATE messages SET pinned = ? WHERE id = ? AND deleted = 0`, val, id) + res, err := d.q.SetMessagePinned(dbCtx(), dbgen.SetMessagePinnedParams{ + Pinned: b2i64(pinned), + ID: id, + }) if err != nil { return fmt.Errorf("SetMessagePinned: %w", err) } @@ -570,25 +572,6 @@ func (d *DB) SetMessagePinned(id int64, pinned bool) error { // ─── helpers ────────────────────────────────────────────────────────────────── -// scanMessage scans a single message from *sql.Row. -func scanMessage(row *sql.Row) (*Message, error) { - m := &Message{} - var deleted, pinned int - err := row.Scan( - &m.ID, &m.ChannelID, &m.UserID, &m.Content, &m.ReplyTo, - &m.EditedAt, &deleted, &pinned, &m.Timestamp, - ) - if errors.Is(err, sql.ErrNoRows) { - return nil, nil - } - if err != nil { - return nil, fmt.Errorf("scanMessage: %w", err) - } - m.Deleted = deleted != 0 - m.Pinned = pinned != 0 - return m, nil -} - // scanMessageWithUser scans a MessageWithUser from *sql.Rows. func scanMessageWithUser(rows *sql.Rows) (MessageWithUser, error) { var mwu MessageWithUser diff --git a/docs/audit-2026-07-19.md b/docs/audit-2026-07-19.md index 19fdcdc5..9697d08c 100644 --- a/docs/audit-2026-07-19.md +++ b/docs/audit-2026-07-19.md @@ -18,7 +18,7 @@ accepted-risk note before the beta gate. MEDIUMs are folded into the backlog | A-2026-07-02 | HIGH | Client HTTP path accepts any TLS certificate (`allowSelfSigned` hardcoded; no TOFU pinning, unlike WS/LiveKit paths) | CLOSED 2026-07-19 — HTTP TOFU proxy implemented (`http_proxy.rs` + `httpProxy.ts`); REST path now cert-pinned, `acceptInvalidCerts` removed | | A-2026-07-03 | HIGH | Reference specs (api.md / protocol.md / schema.md) frozen at 2026-04-02; systemic drift incl. whole undocumented subsystems (voice E2EE, plugins) | CLOSED 2026-07-19 — full refresh of api.md/protocol.md/schema.md landed (all §2 fix-spec items); keep-current-per-PR rule now applies | | A-2026-07-04 | HIGH | Client unit test suite "KNOWN RED" and non-blocking in CI; E2E never gated | OPEN — supersedes prior #11's scope | -| A-2026-07-05 | MEDIUM | Dead sqlc layer: `Server/db/dbgen/` (~3.5k LOC) generated + CI-verified but imported by nothing | IN PROGRESS 2026-07-19 — `dbgen` now wired into `db.DB` and load-bearing (blocks/lockouts/roles delegate); no longer dead. Remaining domains tracked in [plans/sqlc-adoption.md](plans/sqlc-adoption.md) | +| A-2026-07-05 | MEDIUM | Dead sqlc layer: `Server/db/dbgen/` (~3.5k LOC) generated + CI-verified but imported by nothing | RESOLVED 2026-07-19 — `dbgen` wired into `db.DB`; 97 methods across all domains delegate to it (no longer dead). Remaining raw queries (variable IN / FTS / tx) tracked in [plans/sqlc-adoption.md](plans/sqlc-adoption.md) | | A-2026-07-06 | MEDIUM | Three coexisting DB-access styles (raw `*db.DB` in api/admin/ws, `store.Store` under service, dead dbgen) | DECIDED 2026-07-19 — single data layer: sqlc-backed db pkg, remove store/ (D2+D3) (see [plans/audit-2026-07-19-decisions.md](plans/audit-2026-07-19-decisions.md)) | | A-2026-07-07 | MEDIUM | Channel-visibility logic duplicated across ~4 sites with "must mirror" comments | OPEN | | A-2026-07-08 | MEDIUM | Protocol constants on both sides claim generation from `docs/protocol-schema.json`, which does not exist in the repo | CLOSED 2026-07-19 — codegen implemented: `docs/protocol-schema.json` + `Server/scripts/genprotocol` + `make protocol-verify` CI gate | diff --git a/docs/plans/audit-2026-07-19-decisions.md b/docs/plans/audit-2026-07-19-decisions.md index 74befb7a..23a9beeb 100644 --- a/docs/plans/audit-2026-07-19-decisions.md +++ b/docs/plans/audit-2026-07-19-decisions.md @@ -15,7 +15,7 @@ here (and the audit's closure table) as items land. | # | Decision point | Audit ID | Decision | Status | |---|----------------|----------|----------|--------| | D1 | `announcement` channel type (documented + offered by admin API, rejected by DB triggers) | A-2026-07-01 | **Implement end-to-end**: migration to allow the type, posting-permission semantics, admin support, client rendering, spec updates. Not a doc-strip — this becomes a real feature. | Planned (not yet greenlit to start) | -| D2 | Data-layer direction (raw SQL vs dead sqlc `db/dbgen` vs `store.Store`) | A-2026-07-05 / A-2026-07-06 | **Adopt sqlc for real**: wire `db.DB` method bodies to the generated `dbgen` queries so sqlc becomes the actual, type-checked query layer. The `sqlc-verify` CI job stays and starts earning its keep. | **In progress — phase 1 done 2026-07-19**: `dbgen.Queries` wired into `db.DB`; blocks/lockouts/roles delegate (sqlc now load-bearing, no longer dead code). Remaining domains tracked in [sqlc-adoption.md](sqlc-adoption.md). | +| D2 | Data-layer direction (raw SQL vs dead sqlc `db/dbgen` vs `store.Store`) | A-2026-07-05 / A-2026-07-06 | **Adopt sqlc for real**: wire `db.DB` method bodies to the generated `dbgen` queries so sqlc becomes the actual, type-checked query layer. The `sqlc-verify` CI job stays and starts earning its keep. | **Largely done 2026-07-19**: `dbgen.Queries` wired into `db.DB`; 97 methods across all domains delegate to sqlc (no longer dead code). ~43 raw calls remain by design (variable IN, FTS, multi-statement tx, PRAGMA/VACUUM) — tracked in [sqlc-adoption.md](sqlc-adoption.md). | | D3 | Fate of `Server/store/` (untested abstraction seam) | prior audit #6 | **Remove `store/`**: execute the prior audit's P4 "single data layer" direction. Services call the (sqlc-backed) `db` package directly; tests use in-memory SQLite instead of `MemStore`. | Planned (sequence with/after D2) | | D4 | Protocol constants sync (`message_types.go` / `protocolTypes.ts` claim a nonexistent `docs/protocol-schema.json`) | A-2026-07-08 | **Create real codegen**: commit an actual `protocol-schema.json` plus a generator that emits the Go and TS constant files (and, ideally, protocol.md's message table), making the "single source of truth" comment true. | **Implemented 2026-07-19**: `docs/protocol-schema.json` + `Server/scripts/genprotocol` + `make protocol-generate`/`protocol-verify` + CI gate. protocol.md table generation deferred to D7. | | D5 | Client HTTP TLS gap (`allowSelfSigned: true`, no TOFU pinning on the REST path) | A-2026-07-02 | **Next security work**: build the TOFU HTTP proxy in Rust (mirroring `ws_proxy.rs`) as the next security task — highest-priority security item. | **Implemented 2026-07-19** — `src-tauri/src/http_proxy.rs` (per-host loopback TCP→TLS tunnels, shared TOFU cert store + `cert-tofu` events) + `src/lib/httpProxy.ts`; REST/health/attachments routed through it; `acceptInvalidCerts`/`allowSelfSigned` and the `dangerous-settings` feature removed. See [http-tofu-proxy.md](http-tofu-proxy.md). | diff --git a/docs/plans/sqlc-adoption.md b/docs/plans/sqlc-adoption.md index 4320c951..0090d9a2 100644 --- a/docs/plans/sqlc-adoption.md +++ b/docs/plans/sqlc-adoption.md @@ -25,27 +25,35 @@ Two mechanical frictions drive the per-domain effort: ## Status -### Phase 1 — done (2026-07-19) -sqlc is now **load-bearing in production** (previously dead code): +### Phase 1 + 2 — done (2026-07-19) +sqlc is now **load-bearing in production** (previously dead code). **97 `db.DB` +methods delegate** to `dbgen` across every domain; 43 raw `d.sqlDB` calls +remain (the `db.go` passthrough helpers, `migrate.go`, and the intentionally +raw queries listed below). Shared mappers live in `db/mappers.go` +(`userFromGen`, `sessionFromGen`, `roleFromGen`, `ptrI64toI`/`ptrItoI64`, +`b2i64`, `strToNullPtr`, `derefString`). -| Domain | Methods delegated | Notes | -|--------|-------------------|-------| -| blocks (`block_queries.go`) | BlockUser, UnblockUser, IsBlocked, IsEitherBlocked, ListBlockedUsers | Added `ListBlockedUsers` query. Empty result now `[]int64{}` (matches MemStore; was `nil`). | -| lockouts (`lockout_queries.go`) | UpsertLockout, LoadActiveLockouts, CleanupExpiredLockouts, DeleteLockout | Time formatting/parsing kept in the wrapper. | -| roles (`role_queries.go`) | GetRoleByID, ListRoles, GetRoleForUser | Shared `roleFromGen` mapper. `GetUserWithRole` (joined User+Role) stays raw. | +Delegated domains: blocks, lockouts, roles, users + sessions, invites, profile, +attachments, voice, dm (simple ops), channels + permission overrides, admin +(users/settings/audit/counts), messages (create/get/edit/delete/reactions/ +pins/read-state). -### Phase 2 — remaining domains (raw SQL still, each survives D3) -Convert with the same pattern; add missing queries + a `fromGen` mapper where -needed. Rough order by mapping simplicity: +### Deliberately kept raw (no clean sqlc mapping) +- **Variable-length `IN(...)`** (sqlc can't express): `GetAttachmentsByMessageIDs`, + `LinkAttachmentsToMessage`, `GetChannelTypes`. +- **FTS / dynamic WHERE / cursor pagination**: `GetMessages`, `SearchMessages`, + `SearchMessagesInChannels`, `GetMessagesForAPI`, `GetPinnedMessages`, + `getReactionsBatch`, `GetChannelUnreadCounts`, `GetLatestMessageID` + (sqlc types the `MAX()` result as `interface{}`). +- **Multi-statement transactions**: `GetOrCreateDMChannel` (serializable tx), + `GetUserDMChannels` (aggregate), `GetDMRecipient`, `CreateOwnerIfEmpty`, + `CreateUserWithInvite`, `GetUserWithRole`. +- **Non-query SQL**: `GetServerStats` PRAGMAs, `BackupToSafe` (`VACUUM INTO`), + `AdminCreateChannel`, `CountChannelVoiceUsers`, `account.go`. -- **Simple/exec-heavy:** invites (`invite_queries.go`), profile - (`profile_queries.go`), attachments (`attachment_queries.go`). -- **Model-mapped reads:** sessions (`auth_queries.go`), users - (`auth_queries.go` — `GetUserByID`/`GetUserByUsername`/`ListAllUsers`), - channels (`channel_queries.go`), voice (`voice_queries.go`), - dm (`dm_queries.go`), admin/settings (`admin_queries.go`). -- **Complex/joined:** messages (`message_queries.go` — search, cursor - pagination, reactions), `GetUserWithRole`, `GetServerStats`. +These are candidates for follow-up (add `sqlc.arg`/`sqlc.slice` queries or +accept they stay raw), but none block the D2 goal: `dbgen` is no longer dead +and owns the SQL for the overwhelming majority of the data layer. ### Out of scope for D2 - `store/` event + plugin SQL (`store/sqlite_events.go`, plugin store) — these