From 1019419b171aad8d3b225f13f11846050ea9a04c Mon Sep 17 00:00:00 2001 From: clawbot Date: Tue, 20 Oct 2026 11:58:39 +0200 Subject: [PATCH] Page history by message id (closes #125) GET /api/v1/history took a database id for before, but history responses carry only each message's id, so a client had nothing to pass for the next page. before now takes that id: the server finds the message among those sent to the target and returns the page older than it. An id with no such message gets 400 "unknown before id" instead of a silent full page. A handler test pages back through a channel using only ids from earlier responses, and README.md's history section describes the new parameter. Model: opus-5-5 Co-authored-by: clawbot --- README.md | 20 +++-- internal/db/queries.go | 23 ++++- internal/db/queries_test.go | 2 +- internal/handlers/api.go | 18 ++-- internal/handlers/api_test.go | 159 ++++++++++++++++++++++++++++++++++ 5 files changed, 205 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 54a0f23..11cca0e 100644 --- a/README.md +++ b/README.md @@ -1836,14 +1836,18 @@ 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` | integer | `0` | Return only messages with DB ID < this value (for pagination). `0` means 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 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. | + +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`, `404` `channel not found`, `403` +`400` `target required`, `400` `unknown before id` when no message with that +`id` was sent to `target`, `404` `channel not found`, `403` `not a member of this channel`, and `403` `forbidden` for any other nick. **Response:** `200 OK` @@ -1881,8 +1885,8 @@ events). Event messages are delivered via the live queue only. curl -s "http://localhost:8080/api/v1/history?target=%23general&limit=50" \ -b cookies.txt | jq . -# Older messages (pagination) -curl -s "http://localhost:8080/api/v1/history?target=%23general&before=100&limit=50" \ +# The 50 messages before uuid-1 (pagination) +curl -s "http://localhost:8080/api/v1/history?target=%23general&before=uuid-1&limit=50" \ -b cookies.txt | jq . ``` diff --git a/internal/db/queries.go b/internal/db/queries.go index 29239ea..b50a2eb 100644 --- a/internal/db/queries.go +++ b/internal/db/queries.go @@ -1058,17 +1058,34 @@ func (database *Database) PollMessages( return msgs, lastQID, nil } -// GetHistory returns message history for a target. +// 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. func (database *Database) GetHistory( ctx context.Context, - target string, - beforeID int64, + target, before string, limit int, ) ([]IRCMessage, error) { if limit <= 0 { limit = defaultHistLimit } + var beforeID int64 + + if before != "" { + err := database.conn.QueryRowContext(ctx, + `SELECT id FROM messages + WHERE uuid = ? AND msg_to = ?`, + before, target, + ).Scan(&beforeID) + if err != nil { + return nil, fmt.Errorf( + "find message %q: %w", before, err, + ) + } + } + rows, err := database.queryHistory( ctx, target, beforeID, limit, ) diff --git a/internal/db/queries_test.go b/internal/db/queries_test.go index d5467a9..40e8f4d 100644 --- a/internal/db/queries_test.go +++ b/internal/db/queries_test.go @@ -721,7 +721,7 @@ func TestGetHistory(t *testing.T) { const histLimit = 5 msgs, err := database.GetHistory( - ctx, "#hist", 0, histLimit, + ctx, "#hist", "", histLimit, ) if err != nil { t.Fatal(err) diff --git a/internal/handlers/api.go b/internal/handlers/api.go index cde58bd..917eb36 100644 --- a/internal/handlers/api.go +++ b/internal/handlers/api.go @@ -2,6 +2,7 @@ package handlers import ( "context" + "database/sql" "encoding/json" "errors" "fmt" @@ -3558,10 +3559,6 @@ func (hdlr *Handlers) HandleGetHistory() http.HandlerFunc { return } - beforeID, _ := strconv.ParseInt( - request.URL.Query().Get("before"), 10, 64, - ) - limit, _ := strconv.Atoi( request.URL.Query().Get("limit"), ) @@ -3570,8 +3567,19 @@ func (hdlr *Handlers) HandleGetHistory() http.HandlerFunc { } msgs, err := hdlr.params.Database.GetHistory( - request.Context(), target, beforeID, limit, + request.Context(), target, + request.URL.Query().Get("before"), limit, ) + if errors.Is(err, sql.ErrNoRows) { + hdlr.respondError( + writer, request, + "unknown before id", + http.StatusBadRequest, + ) + + return + } + if err != nil { hdlr.log.Error( "get history failed", "error", err, diff --git a/internal/handlers/api_test.go b/internal/handlers/api_test.go index 4fff74e..e74f9fd 100644 --- a/internal/handlers/api_test.go +++ b/internal/handlers/api_test.go @@ -14,6 +14,7 @@ import ( "io" "net/http" "net/http/httptest" + "net/url" "os" "slices" "strconv" @@ -1482,6 +1483,164 @@ func TestHistoryNonMember(t *testing.T) { } } +// getHistory fetches one page of target's history and +// returns the status and, on 200, the messages on it. +func (tserver *testServer) getHistory( + token, target, before string, + limit int, +) (int, []db.IRCMessage) { + tserver.t.Helper() + + query := url.Values{} + query.Set("target", target) + query.Set("before", before) + query.Set("limit", strconv.Itoa(limit)) + + resp, err := doRequestAuth( + tserver.t, http.MethodGet, + tserver.url("/api/v1/history?"+query.Encode()), + token, nil, + ) + if err != nil { + tserver.t.Fatalf("get history: %v", err) + } + + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + return resp.StatusCode, nil + } + + var msgs []db.IRCMessage + + decErr := json.NewDecoder(resp.Body).Decode(&msgs) + if decErr != nil { + tserver.t.Fatalf("decode history: %v", decErr) + } + + return resp.StatusCode, msgs +} + +func TestHistoryPaging(t *testing.T) { + const ( + channel = "#paging" + msgCount = 5 + pageSize = 2 + wantPages = 3 + ) + + tserver := newTestServer(t) + token := tserver.createSession("pager") + + tserver.sendCommand(token, map[string]any{ + commandKey: joinCmd, toKey: channel, + }) + + want := make([]string, 0, msgCount) + + for i := range msgCount { + text := fmt.Sprintf("message %d", i+1) + want = append(want, text) + + tserver.sendCommand(token, map[string]any{ + commandKey: privmsgCmd, + toKey: channel, + bodyKey: []string{text}, + }) + } + + 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 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, + ) + } + + if !slices.Equal(got, want) { + t.Fatalf("expected %q, got %q", want, got) + } +} + +func TestHistoryUnknownBefore(t *testing.T) { + tserver := newTestServer(t) + token := tserver.createSession("lost") + + for _, channel := range []string{"#here", "#elsewhere"} { + tserver.sendCommand(token, map[string]any{ + commandKey: joinCmd, toKey: channel, + }) + tserver.sendCommand(token, map[string]any{ + commandKey: privmsgCmd, + toKey: channel, + bodyKey: []string{"hi there"}, + }) + } + + _, elsewhere := tserver.getHistory( + token, "#elsewhere", "", 1, + ) + if len(elsewhere) != 1 { + t.Fatalf( + "expected 1 message, got %d", len(elsewhere), + ) + } + + cases := []struct{ name, before string }{ + {"made-up id", "no-such-message"}, + {"id from another channel", elsewhere[0].ID}, + } + + for _, tc := range cases { + status, _ := tserver.getHistory( + token, "#here", tc.before, 1, + ) + if status != http.StatusBadRequest { + t.Fatalf( + "%s: expected 400, got %d", tc.name, status, + ) + } + } +} + func TestChannelList(t *testing.T) { tserver := newTestServer(t) token := tserver.createSession("lister")