Pin the roster, notification and account decode shapes
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
6f5c2a7a24
commit
7f7692318f
@@ -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)
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user