HTTP API: 404 for a chat id the list of chats leaves out (closes #5)
check / check (push) Successful in 1m19s

The chat client keeps contact records that GET /api/v1/chats does not
list, such as the bot's own profile (1 on a new profile) and a contact
it creates itself (2), and reads or sends in them when asked. The
message endpoints now look the id up, through chatID in
internal/api/chats.go, in the same list the chats endpoint answers
with, and answer 404 for any id not in it. The webhook endpoints can
call chatID too.

Model: opus-5-5
This commit is contained in:
2026-09-29 04:32:28 +00:00
parent 2ad0b68e35
commit 2ce8da766a
6 changed files with 140 additions and 69 deletions
+2 -2
View File
@@ -180,7 +180,7 @@ A message has:
it. it.
A `count` other than a whole number from 1 to 100 gets `400`, and an A `count` other than a whole number from 1 to 100 gets `400`, and an
`id` that is not one of the bot's chats gets `404`. `id` that `GET /api/v1/chats` does not list gets `404`.
### `POST /api/v1/chats/{id}/messages` ### `POST /api/v1/chats/{id}/messages`
@@ -208,7 +208,7 @@ curl -H "Authorization: Bearer $TOKEN" -H 'Content-Type: application/json' \
Nothing is sent, and the answer is: Nothing is sent, and the answer is:
- `400` if the body is not JSON of that shape, or `text` is empty; - `400` if the body is not JSON of that shape, or `text` is empty;
- `404` if `id` is not one of the bot's chats; - `404` if `GET /api/v1/chats` does not list `id`;
- `409` if the contact cannot receive messages: they have deleted their - `409` if the contact cannot receive messages: they have deleted their
chat with the bot (`contact_deleted` is `true`), or have not finished chat with the bot (`contact_deleted` is `true`), or have not finished
connecting; connecting;
+5 -3
View File
@@ -29,11 +29,13 @@ const (
var errChat = errors.New("sqlite: database is locked at /var/lib/simplexcalc") var errChat = errors.New("sqlite: database is locked at /var/lib/simplexcalc")
// fakeClient stands in for the chat client. It answers with contacts, // fakeClient stands in for the chat client. It answers with contacts,
// items or sent, or with err, and remembers what it was asked. // items or sent, or with an error: contactsErr from Contacts, err from
// the others. It remembers what it was asked.
type fakeClient struct { type fakeClient struct {
contacts []simplex.Contact contacts []simplex.Contact
items []simplex.ChatItem items []simplex.ChatItem
sent simplex.ChatItem sent simplex.ChatItem
contactsErr error
err error err error
userID, contactID int64 userID, contactID int64
@@ -48,7 +50,7 @@ func (f *fakeClient) Contacts(
f.userID = userID f.userID = userID
_, f.hadDeadline = ctx.Deadline() _, f.hadDeadline = ctx.Deadline()
return f.contacts, f.err return f.contacts, f.contactsErr
} }
func (f *fakeClient) ChatItems( func (f *fakeClient) ChatItems(
@@ -269,7 +271,7 @@ func TestHeaders(t *testing.T) {
{&fakeClient{}, chatsPath, bearer}, {&fakeClient{}, chatsPath, bearer},
{&fakeClient{}, chatsPath, ""}, {&fakeClient{}, chatsPath, ""},
{&fakeClient{}, "/nothing", bearer}, {&fakeClient{}, "/nothing", bearer},
{&fakeClient{err: errChat}, chatsPath, bearer}, {&fakeClient{contactsErr: errChat}, chatsPath, bearer},
} { } {
rec := request(t, newAPI(credential, tc.client), rec := request(t, newAPI(credential, tc.client),
http.MethodGet, tc.path, tc.auth) http.MethodGet, tc.path, tc.auth)
+68 -17
View File
@@ -2,32 +2,31 @@ package api
import ( import (
"cmp" "cmp"
"context"
"net/http" "net/http"
"slices" "slices"
"strconv"
"github.com/go-chi/chi/v5"
) )
// handleChats lists the bot's chats, ordered by id. The bot talks to // noSuchChat answers a request naming a chat the bot does not have.
// people only one to one, so its chats are its contacts, and a chat's const noSuchChat = "no such chat"
// id is its contact's.
func (h *handlers) handleChats() http.HandlerFunc { // chat is how the API shows a chat. The bot talks to people only one to
type chat struct { // one, so its chats are its contacts, and a chat's id is its contact's.
type chat struct {
ID int64 `json:"id"` ID int64 `json:"id"`
DisplayName string `json:"display_name"` DisplayName string `json:"display_name"`
ContactDeleted bool `json:"contact_deleted"` ContactDeleted bool `json:"contact_deleted"`
} }
type response struct { // chats returns the bot's chats, ordered by id. GET /api/v1/chats
Chats []chat `json:"chats"` // answers with this list, and chatID accepts only an id in it.
} func (h *handlers) chats(ctx context.Context) ([]chat, error) {
contacts, err := h.client.Contacts(ctx, h.userID)
return func(w http.ResponseWriter, r *http.Request) {
contacts, err := h.client.Contacts(r.Context(), h.userID)
if err != nil { if err != nil {
h.log.Error("listing the chats", "error", err) return nil, err
h.respondError(w, http.StatusInternalServerError,
"the chats could not be read")
return
} }
chats := make([]chat, 0, len(contacts)) chats := make([]chat, 0, len(contacts))
@@ -41,6 +40,58 @@ func (h *handlers) handleChats() http.HandlerFunc {
slices.SortFunc(chats, func(a, b chat) int { return cmp.Compare(a.ID, b.ID) }) slices.SortFunc(chats, func(a, b chat) int { return cmp.Compare(a.ID, b.ID) })
return chats, nil
}
// handleChats lists the bot's chats.
func (h *handlers) handleChats() http.HandlerFunc {
type response struct {
Chats []chat `json:"chats"`
}
return func(w http.ResponseWriter, r *http.Request) {
chats, err := h.chats(r.Context())
if err != nil {
h.log.Error("listing the chats", "error", err)
h.respondError(w, http.StatusInternalServerError,
"the chats could not be read")
return
}
h.respond(w, http.StatusOK, response{Chats: chats}) h.respond(w, http.StatusOK, response{Chats: chats})
} }
} }
// chatID returns the chat id in the request's path, if it is one of the
// bot's chats. Otherwise it answers the request itself, 404, or 500 if
// the chats could not be read, and returns false.
//
// The id is looked up rather than passed to the chat client, which also
// keeps contact records that are not chats, such as the bot's own
// profile, and would read or send in them.
func (h *handlers) chatID(w http.ResponseWriter, r *http.Request) (int64, bool) {
id, err := strconv.ParseInt(chi.URLParam(r, "id"), 10, 64)
if err != nil {
h.respondError(w, http.StatusNotFound, noSuchChat)
return 0, false
}
chats, err := h.chats(r.Context())
if err != nil {
h.log.Error("looking up a chat", "error", err)
h.respondError(w, http.StatusInternalServerError,
"the chats could not be read")
return 0, false
}
if !slices.ContainsFunc(chats, func(c chat) bool { return c.ID == id }) {
h.respondError(w, http.StatusNotFound, noSuchChat)
return 0, false
}
return id, true
}
+1 -1
View File
@@ -54,7 +54,7 @@ func TestNoChats(t *testing.T) {
func TestChatsFailure(t *testing.T) { func TestChatsFailure(t *testing.T) {
t.Parallel() t.Parallel()
rec := request(t, newAPI(credential, &fakeClient{err: errChat}), rec := request(t, newAPI(credential, &fakeClient{contactsErr: errChat}),
http.MethodGet, chatsPath, bearer) http.MethodGet, chatsPath, bearer)
want := `{"error":"the chats could not be read"}` + "\n" want := `{"error":"the chats could not be read"}` + "\n"
+5 -21
View File
@@ -9,7 +9,6 @@ import (
"strconv" "strconv"
"time" "time"
"github.com/go-chi/chi/v5"
"sneak.berlin/go/simplexcalc/internal/simplex" "sneak.berlin/go/simplexcalc/internal/simplex"
) )
@@ -18,9 +17,6 @@ const (
// request for its messages reads. // request for its messages reads.
defaultCount = 20 defaultCount = 20
maxCount = 100 maxCount = 100
// noSuchChat answers a request naming a chat the bot does not have.
noSuchChat = "no such chat"
) )
// message is how the API shows a message, wherever it shows one. // message is how the API shows a message, wherever it shows one.
@@ -69,10 +65,8 @@ func (h *handlers) handleMessages() http.HandlerFunc {
} }
return func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) {
contactID, ok := chatID(r) contactID, ok := h.chatID(w, r)
if !ok { if !ok {
h.respondError(w, http.StatusNotFound, noSuchChat)
return return
} }
@@ -116,10 +110,8 @@ func (h *handlers) handleSend() http.HandlerFunc {
} }
return func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) {
contactID, ok := chatID(r) contactID, ok := h.chatID(w, r)
if !ok { if !ok {
h.respondError(w, http.StatusNotFound, noSuchChat)
return return
} }
@@ -170,9 +162,9 @@ func (h *handlers) handleSend() http.HandlerFunc {
} }
// respondChatError answers a request the chat client did not serve: 404 // respondChatError answers a request the chat client did not serve: 404
// for a chat the bot does not have, 409 for a contact who cannot receive // for a contact removed after chatID found it, 409 for one who cannot
// messages, 413 for a text too long to send, and 500 with sentence for // receive messages, 413 for a text too long to send, and 500 with
// anything else, which is logged as what. // sentence for anything else, which is logged as what.
func (h *handlers) respondChatError( func (h *handlers) respondChatError(
w http.ResponseWriter, err error, what, sentence string, w http.ResponseWriter, err error, what, sentence string,
) { ) {
@@ -189,14 +181,6 @@ func (h *handlers) respondChatError(
} }
} }
// chatID returns the chat id in the request's path, and false if it is
// not one: a chat's id is its contact's, a positive whole number.
func chatID(r *http.Request) (int64, bool) {
id, err := strconv.ParseInt(chi.URLParam(r, "id"), 10, 64)
return id, err == nil && id > 0
}
// messageCount returns the request's count, defaultCount if it has none, // messageCount returns the request's count, defaultCount if it has none,
// and false if it is anything but a whole number from 1 to maxCount. The // and false if it is anything but a whole number from 1 to maxCount. The
// query is parsed here because r.URL.Query drops a pair it cannot // query is parsed here because r.URL.Query drops a pair it cannot
+52 -18
View File
@@ -17,6 +17,12 @@ const (
notJSON = `{"error":"the body must be JSON such as {\"text\":\"hello\"}"}` + "\n" notJSON = `{"error":"the body must be JSON such as {\"text\":\"hello\"}"}` + "\n"
) )
// oneChat returns the bot's contacts in these tests: one, whose chat is
// the one messagesPath names.
func oneChat() []simplex.Contact {
return []simplex.Contact{{ContactID: 3, Status: "active"}}
}
// chatItem is a chat item decoded from a record shaped as the chat // chatItem is a chat item decoded from a record shaped as the chat
// client sends it, reduced to the fields the API reads. // client sends it, reduced to the fields the API reads.
func chatItem(t *testing.T, record string) simplex.ChatItem { func chatItem(t *testing.T, record string) simplex.ChatItem {
@@ -54,7 +60,7 @@ func post(
func TestMessages(t *testing.T) { func TestMessages(t *testing.T) {
t.Parallel() t.Parallel()
client := &fakeClient{items: []simplex.ChatItem{ client := &fakeClient{contacts: oneChat(), items: []simplex.ChatItem{
chatItem(t, `{"meta":{"itemId":7,"itemTs":"2026-09-29T03:13:46.521359978Z"}, chatItem(t, `{"meta":{"itemId":7,"itemTs":"2026-09-29T03:13:46.521359978Z"},
"content":{"type":"rcvChatFeature","feature":"calls"}}`), "content":{"type":"rcvChatFeature","feature":"calls"}}`),
chatItem(t, `{"meta":{"itemId":9,"itemTs":"2026-09-29T03:14:34Z"}, chatItem(t, `{"meta":{"itemId":9,"itemTs":"2026-09-29T03:14:34Z"},
@@ -91,7 +97,7 @@ func TestMessages(t *testing.T) {
func TestNoMessages(t *testing.T) { func TestNoMessages(t *testing.T) {
t.Parallel() t.Parallel()
rec := request(t, newAPI(credential, &fakeClient{}), rec := request(t, newAPI(credential, &fakeClient{contacts: oneChat()}),
http.MethodGet, messagesPath, bearer) http.MethodGet, messagesPath, bearer)
want := `{"messages":[]}` + "\n" want := `{"messages":[]}` + "\n"
@@ -101,7 +107,7 @@ func TestNoMessages(t *testing.T) {
} }
// TestMessagesCount: count is a whole number from 1 to 100, and 20 when // TestMessagesCount: count is a whole number from 1 to 100, and 20 when
// absent. Anything else is refused before the chat client is asked. // absent. Anything else is refused before the chat's items are read.
func TestMessagesCount(t *testing.T) { func TestMessagesCount(t *testing.T) {
t.Parallel() t.Parallel()
@@ -122,7 +128,7 @@ func TestMessagesCount(t *testing.T) {
t.Run(query, func(t *testing.T) { t.Run(query, func(t *testing.T) {
t.Parallel() t.Parallel()
client := &fakeClient{} client := &fakeClient{contacts: oneChat()}
rec := request(t, newAPI(credential, client), rec := request(t, newAPI(credential, client),
http.MethodGet, messagesPath+query, bearer) http.MethodGet, messagesPath+query, bearer)
@@ -145,14 +151,18 @@ func TestMessagesCount(t *testing.T) {
} }
} }
// TestNoSuchChat: a chat id that is not a positive whole number gets 404 // TestNoSuchChat: an id that GET /api/v1/chats does not list gets 404,
// without asking the chat client, and so does one it has no contact // whether reading messages or sending one, and nothing is read or sent.
// for, whether reading messages or sending one. // That includes 1 and 2, contact records the chat client keeps on a new
// profile and would read or send in. A contact the chat client no
// longer has when asked also gets 404.
func TestNoSuchChat(t *testing.T) { func TestNoSuchChat(t *testing.T) {
t.Parallel() t.Parallel()
for _, id := range []string{"tester", "0", "-3", "3.5", "99999999999999999999"} { for _, id := range []string{
client := &fakeClient{} "1", "2", "4", "tester", "0", "-3", "3.5", "99999999999999999999",
} {
client := &fakeClient{contacts: oneChat()}
srv := newAPI(credential, client) srv := newAPI(credential, client)
path := "/api/v1/chats/" + id + "/messages" path := "/api/v1/chats/" + id + "/messages"
@@ -167,11 +177,11 @@ func TestNoSuchChat(t *testing.T) {
} }
if client.contactID != 0 { if client.contactID != 0 {
t.Errorf("chat %q: the chat client was asked", id) t.Errorf("chat %q: the chat was read or sent to", id)
} }
} }
srv := newAPI(credential, &fakeClient{err: simplex.ErrNoContact}) srv := newAPI(credential, &fakeClient{contacts: oneChat(), err: simplex.ErrNoContact})
for _, rec := range []*httptest.ResponseRecorder{ for _, rec := range []*httptest.ResponseRecorder{
request(t, srv, http.MethodGet, messagesPath, bearer), request(t, srv, http.MethodGet, messagesPath, bearer),
@@ -184,12 +194,36 @@ func TestNoSuchChat(t *testing.T) {
} }
} }
// TestChatLookupFailure: when the chats cannot be read to look the id
// up, the answer is 500 and nothing is read or sent.
func TestChatLookupFailure(t *testing.T) {
t.Parallel()
client := &fakeClient{contactsErr: errChat}
srv := newAPI(credential, client)
want := `{"error":"the chats could not be read"}` + "\n"
for _, rec := range []*httptest.ResponseRecorder{
request(t, srv, http.MethodGet, messagesPath, bearer),
post(t, srv, messagesPath, `{"text":"hello"}`),
} {
if rec.Code != http.StatusInternalServerError || rec.Body.String() != want {
t.Errorf("response = %d %q, want 500 %q", rec.Code, rec.Body.String(), want)
}
}
if client.contactID != 0 {
t.Error("the chat was read or sent to")
}
}
// TestMessagesFailure: when the chat client fails, the answer says so in // TestMessagesFailure: when the chat client fails, the answer says so in
// a chosen sentence, never in the error's own text. // a chosen sentence, never in the error's own text.
func TestMessagesFailure(t *testing.T) { func TestMessagesFailure(t *testing.T) {
t.Parallel() t.Parallel()
rec := request(t, newAPI(credential, &fakeClient{err: errChat}), rec := request(t, newAPI(credential, &fakeClient{contacts: oneChat(), err: errChat}),
http.MethodGet, messagesPath, bearer) http.MethodGet, messagesPath, bearer)
want := `{"error":"the messages could not be read"}` + "\n" want := `{"error":"the messages could not be read"}` + "\n"
@@ -203,7 +237,7 @@ func TestMessagesFailure(t *testing.T) {
func TestSend(t *testing.T) { func TestSend(t *testing.T) {
t.Parallel() t.Parallel()
client := &fakeClient{sent: chatItem(t, `{"meta":{"itemId":12, client := &fakeClient{contacts: oneChat(), sent: chatItem(t, `{"meta":{"itemId":12,
"itemTs":"2026-09-29T03:14:43.519552587Z"},"content":{"type":"sndMsgContent", "itemTs":"2026-09-29T03:14:43.519552587Z"},"content":{"type":"sndMsgContent",
"msgContent":{"type":"text","text":"hello"}}}`)} "msgContent":{"type":"text","text":"hello"}}}`)}
@@ -250,7 +284,7 @@ func TestSendBadBody(t *testing.T) {
t.Run(name, func(t *testing.T) { t.Run(name, func(t *testing.T) {
t.Parallel() t.Parallel()
client := &fakeClient{} client := &fakeClient{contacts: oneChat()}
rec := post(t, newAPI(credential, client), messagesPath, tc.body) rec := post(t, newAPI(credential, client), messagesPath, tc.body)
if rec.Code != tc.status || rec.Body.String() != tc.answer { if rec.Code != tc.status || rec.Body.String() != tc.answer {
@@ -277,19 +311,19 @@ func TestSendRefused(t *testing.T) {
answer string answer string
}{ }{
"contact deleted": { "contact deleted": {
&fakeClient{err: simplex.ErrContactNotReady}, &fakeClient{contacts: oneChat(), err: simplex.ErrContactNotReady},
http.StatusConflict, `{"error":"the contact cannot receive messages"}`, http.StatusConflict, `{"error":"the contact cannot receive messages"}`,
}, },
"text too long": { "text too long": {
&fakeClient{err: simplex.ErrMessageTooLarge}, &fakeClient{contacts: oneChat(), err: simplex.ErrMessageTooLarge},
http.StatusRequestEntityTooLarge, `{"error":"the text is too long"}`, http.StatusRequestEntityTooLarge, `{"error":"the text is too long"}`,
}, },
"anything else": { "anything else": {
&fakeClient{err: errChat}, &fakeClient{contacts: oneChat(), err: errChat},
http.StatusInternalServerError, `{"error":"the message could not be sent"}`, http.StatusInternalServerError, `{"error":"the message could not be sent"}`,
}, },
"no message in the answer": { "no message in the answer": {
&fakeClient{sent: chatItem(t, `{"meta":{"itemId":13}, &fakeClient{contacts: oneChat(), sent: chatItem(t, `{"meta":{"itemId":13},
"content":{"type":"sndDirectEvent"}}`)}, "content":{"type":"sndDirectEvent"}}`)},
http.StatusInternalServerError, http.StatusInternalServerError,
`{"error":"the chat client's answer could not be read"}`, `{"error":"the chat client's answer could not be read"}`,