From 2ce8da766a48d142f2934680e65990af2c64aa26 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 04:32:28 +0000 Subject: [PATCH] HTTP API: 404 for a chat id the list of chats leaves out (closes #5) 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 --- README.md | 4 +- internal/api/api_test.go | 16 +++--- internal/api/chats.go | 91 +++++++++++++++++++++++++++-------- internal/api/chats_test.go | 2 +- internal/api/messages.go | 26 ++-------- internal/api/messages_test.go | 70 ++++++++++++++++++++------- 6 files changed, 140 insertions(+), 69 deletions(-) diff --git a/README.md b/README.md index a38151d..11dceca 100644 --- a/README.md +++ b/README.md @@ -180,7 +180,7 @@ A message has: it. 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` @@ -208,7 +208,7 @@ curl -H "Authorization: Bearer $TOKEN" -H 'Content-Type: application/json' \ Nothing is sent, and the answer is: - `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 chat with the bot (`contact_deleted` is `true`), or have not finished connecting; diff --git a/internal/api/api_test.go b/internal/api/api_test.go index ad0e124..ece56ba 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -29,12 +29,14 @@ const ( var errChat = errors.New("sqlite: database is locked at /var/lib/simplexcalc") // 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 { - contacts []simplex.Contact - items []simplex.ChatItem - sent simplex.ChatItem - err error + contacts []simplex.Contact + items []simplex.ChatItem + sent simplex.ChatItem + contactsErr error + err error userID, contactID int64 count int @@ -48,7 +50,7 @@ func (f *fakeClient) Contacts( f.userID = userID _, f.hadDeadline = ctx.Deadline() - return f.contacts, f.err + return f.contacts, f.contactsErr } func (f *fakeClient) ChatItems( @@ -269,7 +271,7 @@ func TestHeaders(t *testing.T) { {&fakeClient{}, chatsPath, bearer}, {&fakeClient{}, chatsPath, ""}, {&fakeClient{}, "/nothing", bearer}, - {&fakeClient{err: errChat}, chatsPath, bearer}, + {&fakeClient{contactsErr: errChat}, chatsPath, bearer}, } { rec := request(t, newAPI(credential, tc.client), http.MethodGet, tc.path, tc.auth) diff --git a/internal/api/chats.go b/internal/api/chats.go index 10c70fe..9b8bce7 100644 --- a/internal/api/chats.go +++ b/internal/api/chats.go @@ -2,26 +2,55 @@ package api import ( "cmp" + "context" "net/http" "slices" + "strconv" + + "github.com/go-chi/chi/v5" ) -// handleChats lists the bot's chats, ordered by id. The bot talks to -// people only one to one, so its chats are its contacts, and a chat's -// id is its contact's. -func (h *handlers) handleChats() http.HandlerFunc { - type chat struct { - ID int64 `json:"id"` - DisplayName string `json:"display_name"` - ContactDeleted bool `json:"contact_deleted"` +// noSuchChat answers a request naming a chat the bot does not have. +const noSuchChat = "no such chat" + +// chat is how the API shows a chat. The bot talks to people only one to +// one, so its chats are its contacts, and a chat's id is its contact's. +type chat struct { + ID int64 `json:"id"` + DisplayName string `json:"display_name"` + ContactDeleted bool `json:"contact_deleted"` +} + +// chats returns the bot's chats, ordered by id. GET /api/v1/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) + if err != nil { + return nil, err } + chats := make([]chat, 0, len(contacts)) + for _, c := range contacts { + chats = append(chats, chat{ + ID: c.ContactID, + DisplayName: c.Profile.DisplayName, + ContactDeleted: c.Deleted(), + }) + } + + 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) { - contacts, err := h.client.Contacts(r.Context(), h.userID) + chats, err := h.chats(r.Context()) if err != nil { h.log.Error("listing the chats", "error", err) h.respondError(w, http.StatusInternalServerError, @@ -30,17 +59,39 @@ func (h *handlers) handleChats() http.HandlerFunc { return } - chats := make([]chat, 0, len(contacts)) - for _, c := range contacts { - chats = append(chats, chat{ - ID: c.ContactID, - DisplayName: c.Profile.DisplayName, - ContactDeleted: c.Deleted(), - }) - } - - slices.SortFunc(chats, func(a, b chat) int { return cmp.Compare(a.ID, b.ID) }) - 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 +} diff --git a/internal/api/chats_test.go b/internal/api/chats_test.go index 3cc6b9f..14f0c00 100644 --- a/internal/api/chats_test.go +++ b/internal/api/chats_test.go @@ -54,7 +54,7 @@ func TestNoChats(t *testing.T) { func TestChatsFailure(t *testing.T) { t.Parallel() - rec := request(t, newAPI(credential, &fakeClient{err: errChat}), + rec := request(t, newAPI(credential, &fakeClient{contactsErr: errChat}), http.MethodGet, chatsPath, bearer) want := `{"error":"the chats could not be read"}` + "\n" diff --git a/internal/api/messages.go b/internal/api/messages.go index a732b44..7b3ebe6 100644 --- a/internal/api/messages.go +++ b/internal/api/messages.go @@ -9,7 +9,6 @@ import ( "strconv" "time" - "github.com/go-chi/chi/v5" "sneak.berlin/go/simplexcalc/internal/simplex" ) @@ -18,9 +17,6 @@ const ( // request for its messages reads. defaultCount = 20 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. @@ -69,10 +65,8 @@ func (h *handlers) handleMessages() http.HandlerFunc { } return func(w http.ResponseWriter, r *http.Request) { - contactID, ok := chatID(r) + contactID, ok := h.chatID(w, r) if !ok { - h.respondError(w, http.StatusNotFound, noSuchChat) - return } @@ -116,10 +110,8 @@ func (h *handlers) handleSend() http.HandlerFunc { } return func(w http.ResponseWriter, r *http.Request) { - contactID, ok := chatID(r) + contactID, ok := h.chatID(w, r) if !ok { - h.respondError(w, http.StatusNotFound, noSuchChat) - return } @@ -170,9 +162,9 @@ func (h *handlers) handleSend() http.HandlerFunc { } // 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 -// messages, 413 for a text too long to send, and 500 with sentence for -// anything else, which is logged as what. +// for a contact removed after chatID found it, 409 for one who cannot +// receive messages, 413 for a text too long to send, and 500 with +// sentence for anything else, which is logged as what. func (h *handlers) respondChatError( 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, // 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 diff --git a/internal/api/messages_test.go b/internal/api/messages_test.go index 4349549..e5aede9 100644 --- a/internal/api/messages_test.go +++ b/internal/api/messages_test.go @@ -17,6 +17,12 @@ const ( 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 // client sends it, reduced to the fields the API reads. func chatItem(t *testing.T, record string) simplex.ChatItem { @@ -54,7 +60,7 @@ func post( func TestMessages(t *testing.T) { 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"}, "content":{"type":"rcvChatFeature","feature":"calls"}}`), 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) { t.Parallel() - rec := request(t, newAPI(credential, &fakeClient{}), + rec := request(t, newAPI(credential, &fakeClient{contacts: oneChat()}), http.MethodGet, messagesPath, bearer) 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 -// 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) { t.Parallel() @@ -122,7 +128,7 @@ func TestMessagesCount(t *testing.T) { t.Run(query, func(t *testing.T) { t.Parallel() - client := &fakeClient{} + client := &fakeClient{contacts: oneChat()} rec := request(t, newAPI(credential, client), 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 -// without asking the chat client, and so does one it has no contact -// for, whether reading messages or sending one. +// TestNoSuchChat: an id that GET /api/v1/chats does not list gets 404, +// whether reading messages or sending one, and nothing is read or sent. +// 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) { t.Parallel() - for _, id := range []string{"tester", "0", "-3", "3.5", "99999999999999999999"} { - client := &fakeClient{} + for _, id := range []string{ + "1", "2", "4", "tester", "0", "-3", "3.5", "99999999999999999999", + } { + client := &fakeClient{contacts: oneChat()} srv := newAPI(credential, client) path := "/api/v1/chats/" + id + "/messages" @@ -167,11 +177,11 @@ func TestNoSuchChat(t *testing.T) { } 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{ 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 // a chosen sentence, never in the error's own text. func TestMessagesFailure(t *testing.T) { t.Parallel() - rec := request(t, newAPI(credential, &fakeClient{err: errChat}), + rec := request(t, newAPI(credential, &fakeClient{contacts: oneChat(), err: errChat}), http.MethodGet, messagesPath, bearer) want := `{"error":"the messages could not be read"}` + "\n" @@ -203,7 +237,7 @@ func TestMessagesFailure(t *testing.T) { func TestSend(t *testing.T) { 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", "msgContent":{"type":"text","text":"hello"}}}`)} @@ -250,7 +284,7 @@ func TestSendBadBody(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - client := &fakeClient{} + client := &fakeClient{contacts: oneChat()} rec := post(t, newAPI(credential, client), messagesPath, tc.body) if rec.Code != tc.status || rec.Body.String() != tc.answer { @@ -277,19 +311,19 @@ func TestSendRefused(t *testing.T) { answer string }{ "contact deleted": { - &fakeClient{err: simplex.ErrContactNotReady}, + &fakeClient{contacts: oneChat(), err: simplex.ErrContactNotReady}, http.StatusConflict, `{"error":"the contact cannot receive messages"}`, }, "text too long": { - &fakeClient{err: simplex.ErrMessageTooLarge}, + &fakeClient{contacts: oneChat(), err: simplex.ErrMessageTooLarge}, http.StatusRequestEntityTooLarge, `{"error":"the text is too long"}`, }, "anything else": { - &fakeClient{err: errChat}, + &fakeClient{contacts: oneChat(), err: errChat}, http.StatusInternalServerError, `{"error":"the message could not be sent"}`, }, "no message in the answer": { - &fakeClient{sent: chatItem(t, `{"meta":{"itemId":13}, + &fakeClient{contacts: oneChat(), sent: chatItem(t, `{"meta":{"itemId":13}, "content":{"type":"sndDirectEvent"}}`)}, http.StatusInternalServerError, `{"error":"the chat client's answer could not be read"}`,