From 7f7692318f4d0f12f32761a77c160e3910a206c7 Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Sun, 30 Aug 2026 09:45:27 -0400 Subject: [PATCH] Pin the roster, notification and account decode shapes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three separate field-name bugs are recorded in comments in MessengerApi, and all three shipped. The block list read `user_id` where the relay sends `id`, so the Block button did nothing โ€” and it survived because the list it produced was always empty, so nothing ever disagreed with it. Notifications read `kind`/`body`/`read`, names that exist nowhere, for months: every alert decoded as a blank unread row and the screen looked permanently empty. The roster route does not use the `user_id` shape the ยง8 routes use, and guessing otherwise threw "No value for user_id" at the exact moment a contact was added. None of these are hard bugs. They are all one bug: a decoder and a producer that disagree, where disagreement yields empty rather than an error, and empty is indistinguishable from "nothing to show yet". Feeding the relay's real response shape catches every one in a second, and no such test existed. Each decode test is therefore paired with a wrong-shape test asserting the historical mistake yields nothing, so the pairing is load-bearing rather than decorative. The mutating routes assert method and path too โ€” POST vs DELETE on blocks, `buddies/7` vs `buddies` for removing one contact against the roster's one-way door. Also pins two defaults that are compatibility decisions rather than conveniences: `accepted` defaults true so a relay predating the field does not relabel every contact a stranger, and `setStatus` returns what the relay stored rather than what we asked for, because the relay once accepted unknown values with 200 and stored nothing. MessengerApi 35.2% -> 87.2%, client 51.0% -> 57.7% lines. Co-Authored-By: Claude Opus 4.8 --- .../flamenet/client/MessengerApiShapeTest.kt | 311 ++++++++++++++++++ 1 file changed, 311 insertions(+) create mode 100644 client/src/test/kotlin/io/flamenet/client/MessengerApiShapeTest.kt diff --git a/client/src/test/kotlin/io/flamenet/client/MessengerApiShapeTest.kt b/client/src/test/kotlin/io/flamenet/client/MessengerApiShapeTest.kt new file mode 100644 index 0000000..4ba85e1 --- /dev/null +++ b/client/src/test/kotlin/io/flamenet/client/MessengerApiShapeTest.kt @@ -0,0 +1,311 @@ +package io.flamenet.client + +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** + * What the roster, notification and account routes actually decode. + * + * ๐Ÿ”ด **Three separate field-name bugs are recorded in comments in this one + * file, and all three shipped.** The block list read `user_id` where the relay + * sends `id`, so the Block button did nothing and survived review because the + * list was always empty. Notifications read `kind`/`body`/`read` โ€” names that + * exist nowhere โ€” for months, so every alert decoded as a blank unread row and + * the screen looked permanently empty. The roster route does not use the + * `user_id` shape the ยง8 routes use, and guessing otherwise threw + * "No value for user_id" at the exact moment a contact was added. + * + * None of them are hard bugs. They are all the same bug: a decoder and a + * producer that disagree, where the disagreement produces empty rather than an + * error, and empty looks exactly like "nothing to show yet". A test that feeds + * the relay's real response shape catches every one of them in a second, and + * there was no such test. + * + * So each decode test below is paired with a wrong-shape test asserting that + * the historical mistake yields nothing โ€” which is what makes the pairing + * meaningful rather than decorative. + */ +class MessengerApiShapeTest { + + private lateinit var server: MockWebServer + + @BeforeTest fun start() { server = MockWebServer().also { it.start() } } + @AfterTest fun stop() { server.shutdown() } + + private fun api() = MessengerApi( + server.url("/").toString().trimEnd('/'), "someone", "app-password") + + private fun json(body: String) = MockResponse() + .setResponseCode(200).setHeader("Content-Type", "application/json").setBody(body) + + // -- Roster -------------------------------------------------------------- + + @Test + fun `the roster decodes the relay's actual field names`() { + server.enqueue(json(""" + {"buddies":[ + {"id":7,"display_name":"Ada L","screen_name":"ada","presence":"away","accepted":true}, + {"id":8,"display_name":"Bob","screen_name":"bob","presence":"online","accepted":false} + ]}""")) + + val list = api().buddies() + assertEquals(2, list.size) + assertEquals(7L, list[0].userId) + assertEquals("Ada L", list[0].displayName) + assertEquals("ada", list[0].handle) + assertEquals("away", list[0].presence) + assertTrue(list[0].accepted) + assertFalse(list[1].accepted) + } + + /** + * ๐Ÿ”ด **Defaults to true, and that default is the compatibility story.** A + * relay predating the field says nothing about acceptance, and the only + * safe reading of silence is "these are your contacts" โ€” the alternative + * labels every contact on every older relay a stranger. + */ + @Test + fun `a relay that predates acceptance does not turn every contact into a stranger`() { + server.enqueue(json("""{"buddies":[{"id":7,"display_name":"Ada","screen_name":"ada"}]}""")) + assertTrue(api().buddies().single().accepted) + } + + /** + * โš ๏ธ `handle` was null on this relay for a while, having started life as an + * OSCAR screen name โ€” which meant you could not find your own contacts by + * the name you knew them by. + */ + @Test + fun `a missing handle decodes as empty rather than throwing`() { + server.enqueue(json("""{"buddies":[{"id":7}]}""")) + val b = api().buddies().single() + assertEquals("", b.handle) + assertEquals("", b.displayName) + assertEquals("offline", b.presence) + } + + @Test + fun `an empty roster is not an error`() { + server.enqueue(json("""{"buddies":[]}""")) + assertEquals(0, api().buddies().size) + server.enqueue(json("""{}""")) + assertEquals(0, api().buddies().size) + } + + @Test + fun `the roster is fetched from the messenger prefix`() { + server.enqueue(json("""{"buddies":[]}""")) + api().buddies() + assertEquals("/api/flamenet/v1/messenger/buddies", server.takeRequest().path) + } + + // -- Blocks -------------------------------------------------------------- + + /** + * ๐Ÿ”ด The bug: this parsed `user_id`, the relay sends `id`. The Block button + * did nothing, and it survived because the list it produced was always + * empty โ€” so nothing ever disagreed with it. + */ + @Test + fun `the block list decodes the id field the relay actually sends`() { + server.enqueue(json("""{"blocked":[{"id":11},{"id":12}]}""")) + assertEquals(listOf(11L, 12L), api().blocked()) + } + + @Test + fun `blocking posts and unblocking deletes`() { + server.enqueue(json("""{"ok":true}""")) + api().setBlocked(11, true) + val blockReq = server.takeRequest() + assertEquals("POST", blockReq.method) + assertEquals("/api/flamenet/v1/messenger/blocks/11", blockReq.path) + + server.enqueue(json("""{"ok":true}""")) + api().setBlocked(11, false) + val unblockReq = server.takeRequest() + assertEquals("DELETE", unblockReq.method) + assertEquals("/api/flamenet/v1/messenger/blocks/11", unblockReq.path) + } + + // -- Notifications ------------------------------------------------------- + + /** + * ๐Ÿ”ด This read `kind`/`body`/`read` for months. Those names exist nowhere, + * so every alert decoded as an empty unread row and the screen looked + * permanently blank โ€” the failure mode of a decode mismatch is silence. + */ + @Test + fun `notifications decode type message and is_read`() { + server.enqueue(json(""" + {"notifications":[ + {"id":1,"type":"buddy_request","message":"Ada added you","is_read":false}, + {"id":2,"type":"message","message":"New message","is_read":true} + ]}""")) + + val list = api().notifications() + assertEquals(2, list.size) + assertEquals("buddy_request", list[0].kind) + assertEquals("Ada added you", list[0].body) + assertFalse(list[0].read) + assertTrue(list[1].read) + } + + /** The exact historical mistake, asserted to produce nothing useful. */ + @Test + fun `the old field names decode to a blank unread row`() { + server.enqueue(json(""" + {"notifications":[{"id":1,"kind":"buddy_request","body":"Ada added you","read":true}]}""")) + val n = api().notifications().single() + assertEquals("", n.kind) + assertEquals("", n.body) + assertFalse(n.read) + } + + @Test + fun `an empty notification page is not an error`() { + server.enqueue(json("""{}""")) + assertEquals(0, api().notifications().size) + } + + // -- Lookup by id -------------------------------------------------------- + + /** + * ๐Ÿ”ด This is how a message from a stranger becomes visible. An E2E envelope + * names its sender by id only, so without this a first contact from someone + * who added US arrived, decrypted, was filed and counted as unread, and + * never appeared anywhere. + */ + @Test + fun `a user lookup names the sender`() { + server.enqueue(json("""{"id":7,"display_name":"Ada L","screen_name":"ada"}""")) + val b = api().userInfo(7)!! + assertEquals(7L, b.userId) + assertEquals("Ada L", b.displayName) + assertEquals("ada", b.handle) + } + + @Test + fun `a lookup that fails returns null rather than throwing`() { + server.enqueue(MockResponse().setResponseCode(404).setBody("""{"reason":"nope"}""")) + assertNull(api().userInfo(7)) + } + + @Test + fun `a lookup for an id with no name field is refused rather than half-decoded`() { + server.enqueue(json("""{"display_name":"Ada"}""")) + assertNull(api().userInfo(7)) + } + + // -- Contact management -------------------------------------------------- + + @Test + fun `adding a contact posts the username and the auto flag`() { + server.enqueue(json("""{"ok":true}""")) + api().addBuddy("ada", auto = true) + val req = server.takeRequest() + assertEquals("POST", req.method) + assertEquals("/api/flamenet/v1/messenger/buddies", req.path) + val body = req.body.readUtf8() + assertTrue(body.contains("\"username\":\"ada\"")) + assertTrue(body.contains("\"auto\":true")) + } + + /** + * The auto flag is what separates "the user typed this name" from "they + * wrote to us first". Defaulting it wrong would file strangers as chosen + * contacts. + */ + @Test + fun `adding a contact is not automatic unless asked`() { + server.enqueue(json("""{"ok":true}""")) + api().addBuddy("ada") + assertTrue(server.takeRequest().body.readUtf8().contains("\"auto\":false")) + } + + @Test + fun `accepting a correspondent posts the accept action`() { + server.enqueue(json("""{"ok":true}""")) + api().acceptConversation(7) + val req = server.takeRequest() + assertEquals("/api/flamenet/v1/messenger/conversation", req.path) + val body = req.body.readUtf8() + assertTrue(body.contains("\"action\":\"accept\"")) + assertTrue(body.contains("\"buddy\":7")) + } + + @Test + fun `removing a contact deletes just that contact`() { + server.enqueue(json("""{"ok":true}""")) + api().removeBuddy(7) + val req = server.takeRequest() + assertEquals("DELETE", req.method) + assertEquals("/api/flamenet/v1/messenger/buddies/7", req.path) + } + + /** P1's one-way door: every contact row, not one. */ + @Test + fun `forgetting the roster deletes the collection`() { + server.enqueue(json("""{"ok":true}""")) + api().forgetRoster() + val req = server.takeRequest() + assertEquals("DELETE", req.method) + assertEquals("/api/flamenet/v1/messenger/buddies", req.path) + } + + // -- Presence ------------------------------------------------------------ + + /** + * ๐Ÿ”ด **Return the server's answer, never the value we asked for.** The + * relay used to accept an unknown status with 200 and store nothing, so a + * caller that assumed success showed a state the server had discarded โ€” + * "offline" on this screen while every contact still saw us online. + */ + @Test + fun `setting a status reports what the relay stored, not what we asked for`() { + server.enqueue(json("""{"status":"online"}""")) + assertEquals("online", api().setStatus("busy")) + } + + @Test + fun `a relay that returns no status falls back to the requested value`() { + server.enqueue(json("""{}""")) + assertEquals("busy", api().setStatus("busy")) + } + + @Test + fun `the current status defaults to online when the relay says nothing`() { + server.enqueue(json("""{}""")) + assertEquals("online", api().currentStatus()) + } + + @Test + fun `the current status is whatever the relay reports`() { + server.enqueue(json("""{"status":"away"}""")) + assertEquals("away", api().currentStatus()) + } + + // -- The account itself -------------------------------------------------- + + @Test + fun `me decodes the account route's own field names`() { + server.enqueue(json("""{"id":42,"name":"Ada L","slug":"ada"}""")) + val me = api().me() + assertEquals(42L, me.userId) + assertEquals("Ada L", me.displayName) + assertEquals("ada", me.handle) + } + + @Test + fun `me is fetched outside the messenger prefix`() { + server.enqueue(json("""{"id":1,"name":"a","slug":"a"}""")) + api().me() + assertEquals("/api/flamenet/v1/messenger/me", server.takeRequest().path) + } +}