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")