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

README.md

@@ -1836,14 +1836,18 @@ Fetch historical messages for a channel. Returns messages in chronological order
 
 **Query Parameters:**
 
-| Param    | Type    | Default    | Description                                                                      |
+| Param    | Type    | Default    | Description                                                                                      |
-| -------- | ------- | ---------- | -------------------------------------------------------------------------------- |
+| -------- | ------- | ---------- | ------------------------------------------------------------------------------------------------ |
-| `target` | string  | (required) | Channel name (e.g., `#general`)                                                  |
+| `target` | string  | (required) | Channel name (e.g., `#general`)                                                                  |
-| `before` | integer | `0`        | Return only messages with DB ID < this value (for pagination). `0` means latest. |
+| `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.                     |
+| `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)
+# The 50 messages before uuid-1 (pagination)
-curl -s "http://localhost:8080/api/v1/history?target=%23general&before=100&limit=50" \
+curl -s "http://localhost:8080/api/v1/history?target=%23general&before=uuid-1&limit=50" \
   -b cookies.txt | jq .
 ```
 

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,
+	target, before string,
-	beforeID int64,
 	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,
 	)

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)

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,

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