diff --git a/README.md b/README.md index cfb9eb1..8fc3ff5 100644 --- a/README.md +++ b/README.md @@ -376,9 +376,10 @@ balancer, and CDN handles it correctly. - DMs are **fan-out to both sender and recipient** — the sender sees their own DM echoed back in their message queue, enabling multi-client consistency (your laptop sees DMs you sent from your phone). -- DM history is stored in the `messages` table with the recipient nick as the - `msg_to` field. This means DM history is queryable per-nick, but if a user - changes their nick, old DMs are associated with the old nick. +- DM history is stored in the `messages` table with the recipient's session as + the `to_session_id` field. A user's DM history therefore follows them across + nick changes, and a user who later takes a released nick sees none of the DMs + sent to its previous holder. - DMs are **not stored long-term** by default — they follow the same rotation policy as channel messages. @@ -1839,18 +1840,21 @@ Fetch historical messages for a channel. Returns messages in chronological order **Query Parameters:** -| Param | Type | Default | Description | -| -------- | ------- | ---------- | ------------------------------------------------------------------------------------------------ | -| `target` | string | (required) | Channel name (e.g., `#general`) | -| `before` | string | (none) | The `id` of a message sent to `target`; return only messages older than it. Omit for the latest. | -| `limit` | integer | `50` | Maximum messages to return, 1–500; any other value gives 50. | +| Param | Type | Default | Description | +| -------- | ------- | ---------- | ----------------------------------------------------------------------------------------------- | +| `target` | string | (required) | Channel name (e.g., `#general`) | +| `before` | string | (none) | The `id` of a message in this history; return only messages older than it. Omit for the latest. | +| `limit` | integer | `50` | Maximum messages to return, 1–500; any other value gives 50. | To page back, pass the `id` of the first (oldest) message of a page as `before` for the next; an empty page means there is nothing older. -`target` may also be your own nick, which returns the DMs sent to you. Errors: -`400` `target required`, `400` `unknown before id` when no message with that -`id` was sent to `target`, `404` `channel not found`, `403` +`target` may also be your own nick, which returns the DMs sent to your session, +including those sent under its earlier nicks. A channel created again after its +last member left starts with no history, and a user who takes a released nick +gets none of the DMs sent to its previous holder. Errors: `400` +`target required`, `400` `unknown before id` when no message with that `id` is +in this history, `404` `channel not found`, `403` `not a member of this channel`, and `403` `forbidden` for any other nick. **Response:** `200 OK` @@ -2508,19 +2512,25 @@ Unique constraint on `(channel_id, session_id)`. #### `messages` -| Column | Type | Description | -| ------------ | -------- | --------------------------------------------------------------- | -| `id` | INTEGER | Primary key (auto-increment). Internal ID for queue references. | -| `uuid` | TEXT | UUID v4, exposed to clients as the message `id` | -| `command` | TEXT | IRC command (`PRIVMSG`, `JOIN`, etc.) | -| `msg_from` | TEXT | Sender nick | -| `msg_to` | TEXT | Target (`#channel` or nick) | -| `params` | TEXT | JSON-encoded IRC-style positional parameters | -| `body` | TEXT | JSON-encoded body (array or object) | -| `meta` | TEXT | JSON-encoded metadata | -| `created_at` | DATETIME | Server timestamp | +| Column | Type | Description | +| --------------- | -------- | ----------------------------------------------------------------------------------- | +| `id` | INTEGER | Primary key (auto-increment). Internal ID for queue references. | +| `uuid` | TEXT | UUID v4, exposed to clients as the message `id` | +| `command` | TEXT | IRC command (`PRIVMSG`, `JOIN`, etc.) | +| `msg_from` | TEXT | Sender nick | +| `msg_to` | TEXT | Target (`#channel` or nick) | +| `to_session_id` | INTEGER | FK → sessions.id (set to null on delete): the session a DM or `INVITE` was sent to | +| `to_channel_id` | INTEGER | FK → channels.id (set to null on delete): the channel a channel message was sent to | +| `params` | TEXT | JSON-encoded IRC-style positional parameters | +| `body` | TEXT | JSON-encoded body (array or object) | +| `meta` | TEXT | JSON-encoded metadata | +| `created_at` | DATETIME | Server timestamp | -Indexes on `(msg_to, id)` and `(created_at)`. +Indexes on `(to_session_id, id)`, `(to_channel_id, id)` and `(created_at)`. +`to_session_id` and `to_channel_id` are null on a message sent to neither, such +as `QUIT`. `GET /api/v1/history` selects by one of these two columns rather than +by `msg_to`, because nicks and channel names are reused once released, and +session and channel ids are not. #### `spent_hashcash` diff --git a/internal/db/errors.go b/internal/db/errors.go index de7768c..c4f1da4 100644 --- a/internal/db/errors.go +++ b/internal/db/errors.go @@ -17,3 +17,14 @@ func IsUniqueConstraintError(err error) bool { return sqliteErr.Code() == sqlite3.SQLITE_CONSTRAINT_UNIQUE } + +// IsForeignKeyConstraintError reports whether err is a +// SQLite foreign-key constraint violation. +func IsForeignKeyConstraintError(err error) bool { + var sqliteErr *sqlite.Error + if !errors.As(err, &sqliteErr) { + return false + } + + return sqliteErr.Code() == sqlite3.SQLITE_CONSTRAINT_FOREIGNKEY +} diff --git a/internal/db/queries.go b/internal/db/queries.go index a552d95..be27ff3 100644 --- a/internal/db/queries.go +++ b/internal/db/queries.go @@ -9,6 +9,7 @@ import ( "encoding/json" "errors" "fmt" + "math" "strconv" "strings" "time" @@ -940,9 +941,14 @@ func (database *Database) GetSessionChannelIDs( } // InsertMessage stores a message and returns its DB ID. +// toSessionID is the session a DM or INVITE is sent to and +// toChannelID the channel a channel message is sent to; +// GetHistory selects by them. Pass 0 for one that does not +// apply, which stores NULL. func (database *Database) InsertMessage( ctx context.Context, command, from, target string, + toSessionID, toChannelID int64, params json.RawMessage, body json.RawMessage, meta json.RawMessage, @@ -965,9 +971,12 @@ func (database *Database) InsertMessage( res, err := database.conn.ExecContext(ctx, `INSERT INTO messages (uuid, command, msg_from, msg_to, + to_session_id, to_channel_id, params, body, meta, created_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, + VALUES (?, ?, ?, ?, NULLIF(?, 0), NULLIF(?, 0), + ?, ?, ?, ?)`, msgUUID, command, from, target, + toSessionID, toChannelID, string(params), string(body), string(meta), now) if err != nil { return 0, "", fmt.Errorf( @@ -1058,26 +1067,31 @@ func (database *Database) PollMessages( return msgs, lastQID, nil } -// GetHistory returns message history for a target. A -// non-empty before is the id of a message sent to target, -// and only messages older than it are returned; an id with -// no such message gives an error wrapping sql.ErrNoRows. +// GetHistory returns the PRIVMSG history of one recipient: +// the DMs sent to session toSessionID, or the messages sent +// to channel toChannelID. Pass 0 for the other. A non-empty +// before is the id of a PRIVMSG in that history, and only +// messages older than it are returned; an id with no such +// message gives an error wrapping sql.ErrNoRows. func (database *Database) GetHistory( ctx context.Context, - target, before string, + toSessionID, toChannelID int64, + before string, limit int, ) ([]IRCMessage, error) { if limit <= 0 { limit = defaultHistLimit } - var beforeID int64 + // With no before, every message is older than this. + beforeID := int64(math.MaxInt64) if before != "" { err := database.conn.QueryRowContext(ctx, `SELECT id FROM messages - WHERE uuid = ? AND msg_to = ?`, - before, target, + WHERE uuid = ? AND command = 'PRIVMSG' + AND (to_session_id = ? OR to_channel_id = ?)`, + before, toSessionID, toChannelID, ).Scan(&beforeID) if err != nil { return nil, fmt.Errorf( @@ -1087,7 +1101,7 @@ func (database *Database) GetHistory( } rows, err := database.queryHistory( - ctx, target, beforeID, limit, + ctx, toSessionID, toChannelID, beforeID, limit, ) if err != nil { return nil, err @@ -1107,21 +1121,23 @@ func (database *Database) GetHistory( return msgs, nil } +// queryHistory selects by to_session_id when toSessionID is +// set and by to_channel_id otherwise, so that the column's +// (column, id) index serves the order and the limit. func (database *Database) queryHistory( ctx context.Context, - target string, - beforeID int64, + toSessionID, toChannelID, beforeID int64, limit int, ) (*sql.Rows, error) { - if beforeID > 0 { + if toSessionID != 0 { rows, err := database.conn.QueryContext(ctx, `SELECT id, uuid, command, msg_from, msg_to, params, body, meta, created_at FROM messages - WHERE msg_to = ? AND id < ? + WHERE to_session_id = ? AND id < ? AND command = 'PRIVMSG' ORDER BY id DESC LIMIT ?`, - target, beforeID, limit) + toSessionID, beforeID, limit) if err != nil { return nil, fmt.Errorf( "query history: %w", err, @@ -1135,10 +1151,10 @@ func (database *Database) queryHistory( `SELECT id, uuid, command, msg_from, msg_to, params, body, meta, created_at FROM messages - WHERE msg_to = ? + WHERE to_channel_id = ? AND id < ? AND command = 'PRIVMSG' ORDER BY id DESC LIMIT ?`, - target, limit) + toChannelID, beforeID, limit) if err != nil { return nil, fmt.Errorf("query history: %w", err) } diff --git a/internal/db/queries_test.go b/internal/db/queries_test.go index 40e8f4d..746f3d7 100644 --- a/internal/db/queries_test.go +++ b/internal/db/queries_test.go @@ -619,7 +619,7 @@ func TestInsertMessage(t *testing.T) { body := json.RawMessage(`["hello"]`) dbID, msgUUID, err := database.InsertMessage( - ctx, "PRIVMSG", "poller", "#test", nil, body, nil, + ctx, "PRIVMSG", "poller", "#test", 0, 0, nil, body, nil, ) if err != nil { t.Fatal(err) @@ -653,7 +653,7 @@ func TestPollMessages(t *testing.T) { body := json.RawMessage(`["hello"]`) dbID, _, err := database.InsertMessage( - ctx, "PRIVMSG", "poller", "#test", nil, body, nil, + ctx, "PRIVMSG", "poller", "#test", 0, 0, nil, body, nil, ) if err != nil { t.Fatal(err) @@ -706,11 +706,16 @@ func TestGetHistory(t *testing.T) { database := setupTestDB(t) ctx := t.Context() + chID, err := database.GetOrCreateChannel(ctx, "#hist") + if err != nil { + t.Fatal(err) + } + const msgCount = 10 for range msgCount { - _, _, err := database.InsertMessage( - ctx, "PRIVMSG", "user", "#hist", + _, _, err = database.InsertMessage( + ctx, "PRIVMSG", "user", "#hist", 0, chID, nil, json.RawMessage(`["msg"]`), nil, ) if err != nil { @@ -721,7 +726,7 @@ func TestGetHistory(t *testing.T) { const histLimit = 5 msgs, err := database.GetHistory( - ctx, "#hist", "", histLimit, + ctx, 0, chID, "", histLimit, ) if err != nil { t.Fatal(err) @@ -737,6 +742,90 @@ func TestGetHistory(t *testing.T) { } } +// TestGetHistoryReusedNames checks that history follows the +// session and the channel row, not the nick or the channel +// name: a session that takes a released nick, and a channel +// created again under a deleted one's name, start with none. +func TestGetHistoryReusedNames(t *testing.T) { + t.Parallel() + + database := setupTestDB(t) + ctx := t.Context() + + historyLen := func(toSessionID, toChannelID int64) int { + t.Helper() + + msgs, err := database.GetHistory( + ctx, toSessionID, toChannelID, "", 0, + ) + if err != nil { + t.Fatal(err) + } + + return len(msgs) + } + + oldSID, _, _, err := database.CreateSession( + ctx, "reused", "", "", "", + ) + if err != nil { + t.Fatal(err) + } + + oldChID, err := database.GetOrCreateChannel(ctx, "#reused") + if err != nil { + t.Fatal(err) + } + + body := json.RawMessage(`["hi"]`) + + _, _, err = database.InsertMessage( + ctx, "PRIVMSG", "sender", "reused", oldSID, 0, + nil, body, nil, + ) + if err != nil { + t.Fatal(err) + } + + _, _, err = database.InsertMessage( + ctx, "PRIVMSG", "sender", "#reused", 0, oldChID, + nil, body, nil, + ) + if err != nil { + t.Fatal(err) + } + + if historyLen(oldSID, 0) != 1 || historyLen(0, oldChID) != 1 { + t.Fatal("expected one DM and one channel message") + } + + err = database.DeleteSession(ctx, oldSID) + if err != nil { + t.Fatal(err) + } + + err = database.DeleteChannelIfEmpty(ctx, oldChID) + if err != nil { + t.Fatal(err) + } + + newSID, _, _, err := database.CreateSession( + ctx, "reused", "", "", "", + ) + if err != nil { + t.Fatal(err) + } + + newChID, err := database.GetOrCreateChannel(ctx, "#reused") + if err != nil { + t.Fatal(err) + } + + if historyLen(newSID, 0) != 0 || historyLen(0, newChID) != 0 { + t.Fatal("expected no history for the names' new holders") + } +} + func TestDeleteSession(t *testing.T) { t.Parallel() @@ -863,7 +952,7 @@ func TestEnqueueToClient(t *testing.T) { body := json.RawMessage(`["test"]`) dbID, _, err := database.InsertMessage( - ctx, "PRIVMSG", "sender", "#ch", nil, body, nil, + ctx, "PRIVMSG", "sender", "#ch", 0, 0, nil, body, nil, ) if err != nil { t.Fatal(err) diff --git a/internal/db/schema/001_initial.sql b/internal/db/schema/001_initial.sql index 1a1b27f..fd66e15 100644 --- a/internal/db/schema/001_initial.sql +++ b/internal/db/schema/001_initial.sql @@ -85,19 +85,25 @@ CREATE TABLE IF NOT EXISTS channel_members ( UNIQUE(channel_id, session_id) ); --- Messages: IRC envelope format +-- Messages: IRC envelope format. to_session_id is the session a DM +-- or INVITE was sent to, to_channel_id the channel a channel message +-- was sent to. History is selected by these ids because nicks and +-- channel names are reused once released, and ids are not. CREATE TABLE IF NOT EXISTS messages ( id INTEGER PRIMARY KEY AUTOINCREMENT, uuid TEXT NOT NULL UNIQUE, command TEXT NOT NULL DEFAULT 'PRIVMSG', msg_from TEXT NOT NULL DEFAULT '', msg_to TEXT NOT NULL DEFAULT '', + to_session_id INTEGER REFERENCES sessions(id) ON DELETE SET NULL, + to_channel_id INTEGER REFERENCES channels(id) ON DELETE SET NULL, params TEXT NOT NULL DEFAULT '[]', body TEXT NOT NULL DEFAULT '[]', meta TEXT NOT NULL DEFAULT '{}', created_at DATETIME DEFAULT CURRENT_TIMESTAMP ); -CREATE INDEX IF NOT EXISTS idx_messages_to_id ON messages(msg_to, id); +CREATE INDEX IF NOT EXISTS idx_messages_to_session ON messages(to_session_id, id); +CREATE INDEX IF NOT EXISTS idx_messages_to_channel ON messages(to_channel_id, id); CREATE INDEX IF NOT EXISTS idx_messages_created ON messages(created_at); -- Spent hashcash tokens for replay prevention (1-year TTL) diff --git a/internal/handlers/api.go b/internal/handlers/api.go index a9471a9..9e10b46 100644 --- a/internal/handlers/api.go +++ b/internal/handlers/api.go @@ -566,7 +566,7 @@ func (hdlr *Handlers) enqueueNumeric( } dbID, _, insertErr := hdlr.params.Database.InsertMessage( - ctx, command, hdlr.serverName(), nick, + ctx, command, hdlr.serverName(), nick, 0, 0, paramsJSON, json.RawMessage(body), nil, ) if insertErr != nil { @@ -652,7 +652,7 @@ func (hdlr *Handlers) initChannelState( dbID, _, insErr := hdlr.params.Database. InsertMessage( ctx, "JOIN", nick, chanInfo.Name, - nil, nil, nil, + 0, chanInfo.ID, nil, nil, nil, ) if insErr != nil { hdlr.log.Error( @@ -2404,7 +2404,7 @@ func (hdlr *Handlers) executeBanChange( ) if mErr == nil { _ = hdlr.fanOutSilent( - request, irc.CmdMode, nick, channel, + request, irc.CmdMode, nick, channel, 0, chID, json.RawMessage(modeBody), memberIDs, ) } @@ -2522,7 +2522,7 @@ func (hdlr *Handlers) setChannelKeyMode( modeBody, mErr := json.Marshal([]string{"+k", key}) if mErr == nil { _ = hdlr.fanOutSilent( - request, irc.CmdMode, nick, channel, + request, irc.CmdMode, nick, channel, 0, chID, json.RawMessage(modeBody), memberIDs, ) } @@ -2571,7 +2571,7 @@ func (hdlr *Handlers) clearChannelKeyMode( modeBody, mErr := json.Marshal([]string{"-k", "*"}) if mErr == nil { _ = hdlr.fanOutSilent( - request, irc.CmdMode, nick, channel, + request, irc.CmdMode, nick, channel, 0, chID, json.RawMessage(modeBody), memberIDs, ) } @@ -2646,7 +2646,7 @@ func (hdlr *Handlers) setChannelLimitMode( ) if mErr == nil { _ = hdlr.fanOutSilent( - request, irc.CmdMode, nick, channel, + request, irc.CmdMode, nick, channel, 0, chID, json.RawMessage(modeBody), memberIDs, ) } @@ -2696,7 +2696,7 @@ func (hdlr *Handlers) clearChannelLimitMode( modeBody, mErr := json.Marshal([]string{"-l"}) if mErr == nil { _ = hdlr.fanOutSilent( - request, irc.CmdMode, nick, channel, + request, irc.CmdMode, nick, channel, 0, chID, json.RawMessage(modeBody), memberIDs, ) } @@ -2755,7 +2755,7 @@ func (hdlr *Handlers) handleInvite( if mErr == nil { _ = hdlr.fanOutSilent( request, irc.CmdInvite, nick, targetNick, - json.RawMessage(invBody), + targetSID, 0, json.RawMessage(invBody), []int64{targetSID}, ) } @@ -3250,9 +3250,10 @@ func (hdlr *Handlers) HandleGetHistory() http.HandlerFunc { return } - if !hdlr.canAccessHistory( + toSessionID, toChannelID, ok := hdlr.canAccessHistory( writer, request, sessionID, nick, target, - ) { + ) + if !ok { return } @@ -3264,7 +3265,7 @@ func (hdlr *Handlers) HandleGetHistory() http.HandlerFunc { } msgs, err := hdlr.params.Database.GetHistory( - request.Context(), target, + request.Context(), toSessionID, toChannelID, request.URL.Query().Get("before"), limit, ) if errors.Is(err, sql.ErrNoRows) { @@ -3297,17 +3298,23 @@ func (hdlr *Handlers) HandleGetHistory() http.HandlerFunc { } // canAccessHistory verifies the user can read history -// for the given target (channel or DM participant). +// for the given target (channel or DM participant) and +// returns the session id and channel id to pass to +// GetHistory: the user's own session for its nick, or the +// channel's id, with 0 for the other. On false it has +// written the error response. func (hdlr *Handlers) canAccessHistory( writer http.ResponseWriter, request *http.Request, sessionID int64, nick, target string, -) bool { +) (int64, int64, bool) { if strings.HasPrefix(target, "#") { - return hdlr.canAccessChannelHistory( + chID, ok := hdlr.canAccessChannelHistory( writer, request, sessionID, target, ) + + return 0, chID, ok } // DM history: only allow if the target is the @@ -3319,18 +3326,20 @@ func (hdlr *Handlers) canAccessHistory( http.StatusForbidden, ) - return false + return 0, 0, false } - return true + return sessionID, 0, true } +// canAccessChannelHistory returns the id of the channel +// named target if the user is a member of it. func (hdlr *Handlers) canAccessChannelHistory( writer http.ResponseWriter, request *http.Request, sessionID int64, target string, -) bool { +) (int64, bool) { chID, err := hdlr.params.Database.GetChannelByName( request.Context(), target, ) @@ -3341,7 +3350,7 @@ func (hdlr *Handlers) canAccessChannelHistory( http.StatusNotFound, ) - return false + return 0, false } isMember, err := hdlr.params.Database.IsChannelMember( @@ -3357,7 +3366,7 @@ func (hdlr *Handlers) canAccessChannelHistory( http.StatusInternalServerError, ) - return false + return 0, false } if !isMember { @@ -3367,10 +3376,10 @@ func (hdlr *Handlers) canAccessChannelHistory( http.StatusForbidden, ) - return false + return 0, false } - return true + return chID, true } // HandleLogout deletes the authenticated client's token @@ -3679,17 +3688,19 @@ func (hdlr *Handlers) deliverWhoisIdle( } // fanOut inserts a message and enqueues it to all given -// sessions. +// sessions. toSessionID and toChannelID are the recipient +// InsertMessage records for history, 0 when there is none. func (hdlr *Handlers) fanOut( request *http.Request, command, from, target string, + toSessionID, toChannelID int64, body json.RawMessage, meta json.RawMessage, sessionIDs []int64, ) (string, error) { dbID, msgUUID, err := hdlr.params.Database.InsertMessage( request.Context(), command, from, target, - nil, body, meta, + toSessionID, toChannelID, nil, body, meta, ) if err != nil { return "", fmt.Errorf("insert message: %w", err) @@ -3714,12 +3725,13 @@ func (hdlr *Handlers) fanOut( func (hdlr *Handlers) fanOutSilent( request *http.Request, command, from, target string, + toSessionID, toChannelID int64, body json.RawMessage, sessionIDs []int64, ) error { _, err := hdlr.fanOut( request, command, from, target, - body, nil, sessionIDs, + toSessionID, toChannelID, body, nil, sessionIDs, ) return err diff --git a/internal/handlers/api_test.go b/internal/handlers/api_test.go index e7882f2..31861f3 100644 --- a/internal/handlers/api_test.go +++ b/internal/handlers/api_test.go @@ -1551,6 +1551,60 @@ func (tserver *testServer) getHistory( return resp.StatusCode, msgs } +// getAllHistory pages back through target's history +// pageSize messages at a time and returns the message texts, +// oldest first, and the number of pages that held any. It +// stops after maxPages pages, so a before that is ignored +// cannot loop forever. +func (tserver *testServer) getAllHistory( + token, target string, + pageSize, maxPages int, +) ([]string, int) { + tserver.t.Helper() + + var ( + got []string + pages int + ) + + // The first page has no before; each later one passes the + // id of the oldest message on the page fetched before it. + before := "" + + for range maxPages { + status, page := tserver.getHistory( + token, target, before, pageSize, + ) + if status != http.StatusOK { + tserver.t.Fatalf("expected 200, got %d", status) + } + + if len(page) == 0 { + break + } + + pages++ + + texts := make([]string, 0, len(page)) + + for _, msg := range page { + var body []string + + err := json.Unmarshal(msg.Body, &body) + if err != nil { + tserver.t.Fatalf("decode body: %v", err) + } + + texts = append(texts, body...) + } + + got = slices.Insert(got, 0, texts...) + before = page[0].ID + } + + return got, pages +} + func TestHistoryPaging(t *testing.T) { const ( channel = "#paging" @@ -1579,46 +1633,10 @@ func TestHistoryPaging(t *testing.T) { }) } - var ( - got []string - pages int + got, pages := tserver.getAllHistory( + token, channel, pageSize, msgCount, ) - // The first page has no before; each later one passes the - // id of the oldest message on the page fetched before it. - before := "" - - for range msgCount { - status, page := tserver.getHistory( - token, channel, before, pageSize, - ) - if status != http.StatusOK { - t.Fatalf("expected 200, got %d", status) - } - - if len(page) == 0 { - break - } - - pages++ - - texts := make([]string, 0, len(page)) - - for _, msg := range page { - var body []string - - err := json.Unmarshal(msg.Body, &body) - if err != nil { - t.Fatalf("decode body: %v", err) - } - - texts = append(texts, body...) - } - - got = slices.Insert(got, 0, texts...) - before = page[0].ID - } - if pages != wantPages { t.Fatalf( "expected %d pages, got %d", wantPages, pages, @@ -1630,6 +1648,81 @@ func TestHistoryPaging(t *testing.T) { } } +// TestHistoryDMPaging pages through a nick's DM history, +// which holds the DMs its session got under an earlier nick +// and none sent to another session, and checks that the id +// of a DM sent to another session is not accepted as before. +func TestHistoryDMPaging(t *testing.T) { + const ( + firstNick = "readerFirst" + laterNick = "readerLater" + otherNick = "otherReader" + // DMs sent to firstNick before the nick change. + sentEarly = 2 + pageSize = 2 + wantPages = 3 + ) + + tserver := newTestServer(t) + senderToken := tserver.createSession("dmPager") + readerToken := tserver.createSession(firstNick) + otherToken := tserver.createSession(otherNick) + + sendDM := func(to, text string) { + tserver.sendCommand(senderToken, map[string]any{ + commandKey: privmsgCmd, + toKey: to, + bodyKey: []string{text}, + }) + } + + want := []string{"dm 1", "dm 2", "dm 3", "dm 4", "dm 5"} + + for _, text := range want[:sentEarly] { + sendDM(firstNick, text) + } + + _, result := tserver.sendCommand(readerToken, map[string]any{ + commandKey: irc.CmdNick, + bodyKey: []string{laterNick}, + }) + if result[nickKey] != laterNick { + t.Fatalf("expected nick %s, got %v", laterNick, result) + } + + sendDM(otherNick, "not for the reader") + + for _, text := range want[sentEarly:] { + sendDM(laterNick, text) + } + + got, pages := tserver.getAllHistory( + readerToken, laterNick, pageSize, len(want), + ) + + if pages != wantPages { + t.Fatalf( + "expected %d pages, got %d", wantPages, pages, + ) + } + + if !slices.Equal(got, want) { + t.Fatalf("expected %q, got %q", want, got) + } + + _, otherDMs := tserver.getHistory(otherToken, otherNick, "", 1) + if len(otherDMs) != 1 { + t.Fatalf("expected 1 DM, got %d", len(otherDMs)) + } + + status, _ := tserver.getHistory( + readerToken, laterNick, otherDMs[0].ID, pageSize, + ) + if status != http.StatusBadRequest { + t.Fatalf("expected 400, got %d", status) + } +} + func TestHistoryUnknownBefore(t *testing.T) { tserver := newTestServer(t) token := tserver.createSession("lost") @@ -1654,9 +1747,25 @@ func TestHistoryUnknownBefore(t *testing.T) { ) } + // History holds only PRIVMSG, so the id of the JOIN to + // #here is not in it. + var joinID string + + queued, _ := tserver.pollMessages(token, 0) + for _, msg := range queued { + if msg[commandKey] == joinCmd && msg[toKey] == "#here" { + joinID, _ = msg["id"].(string) + } + } + + if joinID == "" { + t.Fatal("expected a JOIN to #here in the queue") + } + cases := []struct{ name, before string }{ {"made-up id", "no-such-message"}, {"id from another channel", elsewhere[0].ID}, + {"id of a JOIN", joinID}, } for _, tc := range cases { @@ -1671,6 +1780,101 @@ func TestHistoryUnknownBefore(t *testing.T) { } } +// TestHistoryReusedNick checks that a user who takes a nick +// released by QUIT does not get the DMs sent to its previous +// holder. +func TestHistoryReusedNick(t *testing.T) { + const victim = "victimV" + + tserver := newTestServer(t) + senderToken := tserver.createSession("dmA") + victimToken := tserver.createSession(victim) + newcomerToken := tserver.createSession("newcomer") + + tserver.sendCommand(senderToken, map[string]any{ + commandKey: privmsgCmd, + toKey: victim, + bodyKey: []string{"for the first victimV only"}, + }) + + _, msgs := tserver.getHistory(victimToken, victim, "", 10) + if len(msgs) != 1 { + t.Fatalf("expected 1 DM for %s, got %d", victim, len(msgs)) + } + + tserver.sendCommand(victimToken, map[string]any{ + commandKey: irc.CmdQuit, + }) + + _, result := tserver.sendCommand(newcomerToken, map[string]any{ + commandKey: irc.CmdNick, + bodyKey: []string{victim}, + }) + if result[nickKey] != victim { + t.Fatalf("expected nick %s, got %v", victim, result) + } + + status, msgs := tserver.getHistory(newcomerToken, victim, "", 10) + if status != http.StatusOK { + t.Fatalf("expected 200, got %d", status) + } + + if len(msgs) != 0 { + t.Fatalf("expected no history, got %v", msgs) + } +} + +// TestHistoryRecreatedChannel checks that a channel created +// again under the name of a deleted one starts with no +// history, even when the old one had a key. +func TestHistoryRecreatedChannel(t *testing.T) { + const channel = "#priv" + + tserver := newTestServer(t) + ownerToken := tserver.createSession("roomR") + strangerToken := tserver.createSession("stranger") + + tserver.sendCommand(ownerToken, map[string]any{ + commandKey: joinCmd, toKey: channel, + }) + tserver.sendCommand(ownerToken, map[string]any{ + commandKey: irc.CmdMode, + toKey: channel, + bodyKey: []string{"+k", "topsecret"}, + }) + tserver.sendCommand(ownerToken, map[string]any{ + commandKey: privmsgCmd, + toKey: channel, + bodyKey: []string{"members only"}, + }) + + _, msgs := tserver.getHistory(ownerToken, channel, "", 10) + if len(msgs) != 1 { + t.Fatalf("expected 1 message, got %d", len(msgs)) + } + + // The last member leaving deletes the channel. + tserver.sendCommand(ownerToken, map[string]any{ + commandKey: irc.CmdPart, toKey: channel, + }) + + _, result := tserver.sendCommand(strangerToken, map[string]any{ + commandKey: joinCmd, toKey: channel, + }) + if result[statusKey] != joinedStatus { + t.Fatalf("expected the join to succeed, got %v", result) + } + + status, msgs := tserver.getHistory(strangerToken, channel, "", 10) + if status != http.StatusOK { + t.Fatalf("expected 200, got %d", status) + } + + if len(msgs) != 0 { + t.Fatalf("expected no history, got %v", msgs) + } +} + func TestChannelList(t *testing.T) { tserver := newTestServer(t) token := tserver.createSession("lister") diff --git a/internal/ircserver/commands.go b/internal/ircserver/commands.go index 639c52e..960dac7 100644 --- a/internal/ircserver/commands.go +++ b/internal/ircserver/commands.go @@ -1268,7 +1268,7 @@ func (c *Conn) handleInvite( ) _, _, _ = c.svc.FanOut( //nolint:dogsled // fire-and-forget broadcast - ctx, "INVITE", c.nick, targetNick, + ctx, "INVITE", c.nick, targetNick, targetID, 0, nil, body, nil, []int64{targetID}, ) } diff --git a/internal/ircserver/relay_test.go b/internal/ircserver/relay_test.go index 8ab07a4..767fb69 100644 --- a/internal/ircserver/relay_test.go +++ b/internal/ircserver/relay_test.go @@ -28,7 +28,7 @@ func (env *testEnv) queueMessage( } msgID, _, err := env.database.InsertMessage( - ctx, command, from, to, nil, bodyJSON, nil, + ctx, command, from, to, 0, 0, nil, bodyJSON, nil, ) if err != nil { t.Fatalf("insert message: %v", err) diff --git a/internal/service/service.go b/internal/service/service.go index dabadd7..4a43946 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -311,7 +311,7 @@ func (s *Service) SendWallops( } _, _, err = s.FanOut( - ctx, irc.CmdWallops, nick, "*", + ctx, irc.CmdWallops, nick, "*", 0, 0, nil, body, nil, recipients, ) if err != nil { @@ -344,15 +344,19 @@ type DirectMsgResult struct { } // FanOut inserts a message and enqueues it to all given -// session IDs, notifying each via the broker. +// session IDs, notifying each via the broker. toSessionID +// and toChannelID are the recipient InsertMessage records +// for history, 0 when there is none. func (s *Service) FanOut( ctx context.Context, command, from, to string, + toSessionID, toChannelID int64, params, body, meta json.RawMessage, sessionIDs []int64, ) (int64, string, error) { dbID, msgUUID, err := s.db.InsertMessage( - ctx, command, from, to, params, body, meta, + ctx, command, from, to, + toSessionID, toChannelID, params, body, meta, ) if err != nil { return 0, "", fmt.Errorf("insert message: %w", err) @@ -396,13 +400,13 @@ func (s *Service) SendChannelMessage( nick, command, channel string, body, meta json.RawMessage, ) (int64, string, error) { + noSuchChannel := &IRCError{ + irc.ErrNoSuchChannel, []string{channel}, msgNoSuchChannel, + } + chID, err := s.db.GetChannelByName(ctx, channel) if err != nil { - return 0, "", &IRCError{ - irc.ErrNoSuchChannel, - []string{channel}, - msgNoSuchChannel, - } + return 0, "", noSuchChannel } if command == irc.CmdPrivmsg { @@ -459,9 +463,14 @@ func (s *Service) SendChannelMessage( recipients := excludeSession(memberIDs, sessionID) dbID, uuid, fanErr := s.FanOut( - ctx, command, nick, channel, + ctx, command, nick, channel, 0, chID, nil, body, meta, recipients, ) + // The channel was deleted after the lookup above. + if db.IsForeignKeyConstraintError(fanErr) { + return 0, "", noSuchChannel + } + if fanErr != nil { return 0, "", fanErr } @@ -478,13 +487,13 @@ func (s *Service) SendDirectMessage( nick, command, target string, body, meta json.RawMessage, ) (*DirectMsgResult, error) { + noSuchNick := &IRCError{ + irc.ErrNoSuchNick, []string{target}, "No such nick", + } + targetSID, err := s.db.GetSessionByNick(ctx, target) if err != nil { - return nil, &IRCError{ - irc.ErrNoSuchNick, - []string{target}, - "No such nick", - } + return nil, noSuchNick } away, _ := s.db.GetAway(ctx, targetSID) @@ -495,9 +504,14 @@ func (s *Service) SendDirectMessage( } _, uuid, fanErr := s.FanOut( - ctx, command, nick, target, + ctx, command, nick, target, targetSID, 0, nil, body, meta, recipients, ) + // The target's session was deleted after the lookup above. + if db.IsForeignKeyConstraintError(fanErr) { + return nil, noSuchNick + } + if fanErr != nil { return nil, fanErr } @@ -561,7 +575,7 @@ func (s *Service) JoinChannel( body, _ := json.Marshal([]string{channel}) //nolint:errchkjson _, _, _ = s.FanOut( //nolint:dogsled // fire-and-forget broadcast - ctx, irc.CmdJoin, nick, channel, + ctx, irc.CmdJoin, nick, channel, 0, chID, nil, body, nil, memberIDs, ) @@ -604,7 +618,7 @@ func (s *Service) PartChannel( body, _ := json.Marshal([]string{reason}) //nolint:errchkjson _, _, _ = s.FanOut( //nolint:dogsled // fire-and-forget broadcast - ctx, irc.CmdPart, nick, channel, + ctx, irc.CmdPart, nick, channel, 0, chID, nil, body, nil, recipients, ) @@ -666,7 +680,7 @@ func (s *Service) SetTopic( body, _ := json.Marshal([]string{topic}) //nolint:errchkjson _, _, _ = s.FanOut( //nolint:dogsled // fire-and-forget broadcast - ctx, irc.CmdTopic, nick, channel, + ctx, irc.CmdTopic, nick, channel, 0, chID, nil, body, nil, memberIDs, ) @@ -730,7 +744,7 @@ func (s *Service) KickUser( ) _, _, _ = s.FanOut( //nolint:dogsled // fire-and-forget broadcast - ctx, irc.CmdKick, nick, channel, + ctx, irc.CmdKick, nick, channel, 0, chID, params, body, nil, memberIDs, ) @@ -886,7 +900,7 @@ func (s *Service) BroadcastQuit( body, _ := json.Marshal([]string{reason}) //nolint:errchkjson _, _, _ = s.FanOut( - ctx, irc.CmdQuit, nick, "", + ctx, irc.CmdQuit, nick, "", 0, 0, nil, body, nil, recipients, ) } @@ -1074,7 +1088,7 @@ func (s *Service) BroadcastMode( body, _ := json.Marshal([]string{modeText}) //nolint:errchkjson _, _, _ = s.FanOut( //nolint:dogsled // fire-and-forget broadcast - ctx, irc.CmdMode, nick, channel, + ctx, irc.CmdMode, nick, channel, 0, chID, nil, body, nil, memberIDs, ) } @@ -1351,7 +1365,7 @@ func (s *Service) broadcastNickChange( notified := make(map[int64]bool) dbID, _, insErr := s.db.InsertMessage( - ctx, irc.CmdNick, oldNick, "", + ctx, irc.CmdNick, oldNick, "", 0, 0, nil, body, nil, ) if insErr != nil { diff --git a/internal/service/service_test.go b/internal/service/service_test.go index e619054..5081be3 100644 --- a/internal/service/service_test.go +++ b/internal/service/service_test.go @@ -164,7 +164,7 @@ func TestFanOut(t *testing.T) { body, _ := json.Marshal([]string{"hello"}) //nolint:errchkjson dbID, uuid, err := env.svc.FanOut( - ctx, irc.CmdPrivmsg, "alice", "#test", + ctx, irc.CmdPrivmsg, "alice", "#test", 0, 0, nil, body, nil, []int64{sid1, sid2}, ) @@ -339,6 +339,78 @@ func TestBroadcastQuit(t *testing.T) { } } +// TestSendDirectMessageTargetQuits checks that a DM whose +// target quits after its nick is looked up gets the same +// error as a DM to an unknown nick. The trigger deletes the +// target's session just before the DM is stored. +func TestSendDirectMessageTargetQuits(t *testing.T) { + env := newTestEnv(t) + ctx := t.Context() + + sid := createSession(ctx, t, env.db, "alice") + createSession(ctx, t, env.db, "bob") + + _, err := env.db.GetDB().ExecContext(ctx, + `CREATE TRIGGER delete_recipient + BEFORE INSERT ON messages + BEGIN + DELETE FROM sessions WHERE id = NEW.to_session_id; + END`) + if err != nil { + t.Fatalf("create trigger: %v", err) + } + + body, _ := json.Marshal([]string{"hi"}) //nolint:errchkjson + + _, err = env.svc.SendDirectMessage( + ctx, sid, "alice", irc.CmdPrivmsg, "bob", body, nil, + ) + + var ircErr *service.IRCError + if !errors.As(err, &ircErr) || ircErr.Code != irc.ErrNoSuchNick { + t.Fatalf("want ERR_NOSUCHNICK, got %v", err) + } +} + +// TestSendChannelMessageChannelDeleted checks that a channel +// message whose channel is deleted after it is looked up gets +// the same error as a message to an unknown channel. The +// trigger deletes the channel just before the message is +// stored. +func TestSendChannelMessageChannelDeleted(t *testing.T) { + env := newTestEnv(t) + ctx := t.Context() + + sid := createSession(ctx, t, env.db, "alice") + + _, err := env.svc.JoinChannel(ctx, sid, "alice", "#gone", "") + if err != nil { + t.Fatalf("join alice: %v", err) + } + + _, err = env.db.GetDB().ExecContext(ctx, + `CREATE TRIGGER delete_recipient + BEFORE INSERT ON messages + BEGIN + DELETE FROM channels WHERE id = NEW.to_channel_id; + END`) + if err != nil { + t.Fatalf("create trigger: %v", err) + } + + body, _ := json.Marshal([]string{"hi"}) //nolint:errchkjson + + _, _, err = env.svc.SendChannelMessage( + ctx, sid, "alice", irc.CmdPrivmsg, "#gone", body, nil, + ) + + var ircErr *service.IRCError + if !errors.As(err, &ircErr) || + ircErr.Code != irc.ErrNoSuchChannel { + t.Fatalf("want ERR_NOSUCHCHANNEL, got %v", err) + } +} + func TestSendChannelMessage_Moderated(t *testing.T) { env := newTestEnv(t) ctx := t.Context()