fix(shard): decode in-game serials as hex strings, not numbers
All checks were successful
PR Checks / android-build (pull_request) Successful in 5m44s
All checks were successful
PR Checks / android-build (pull_request) Successful in 5m44s
The public shard board DTOs typed in-game serials (and actor webId) as
Long, but the wire protocol (docs/link/INTEGRATION.md §1) sends them as
opaque hex strings ("0x1A2B"). The website returns board payloads
verbatim, so a guild leader / champ / governor carrying a hex serial
threw JsonDecodingException out of the Retrofit converter and crashed the
app on the Guilds/Champs/Governors boards. The API is the source of
truth, so the DTOs are corrected to match it.
- ActorDto.serial/webId, ChampDto.serial, HouseDto.serial,
OnlineStaffDto.serial: Long -> String
- champ.remove / house.decay live frames now read serial via stringField;
longField returned null on a hex serial, silently dropping every board
removal and live IDOC update
- safeApiCall now catches SerializationException -> ErrorKind.SERVER, so
any future contract drift degrades to a retry-able error instead of a
crash (defense in depth)
- DTO + result tests updated to the real hex-string wire shapes
AI-assisted: authored with Claude Code (Opus 4.8).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NgyHnrNa8WwG3doxvxjuCr
This commit is contained in:
@@ -4,6 +4,7 @@
|
|||||||
package com.runicgateway.app.core.result
|
package com.runicgateway.app.core.result
|
||||||
|
|
||||||
import kotlinx.coroutines.CancellationException
|
import kotlinx.coroutines.CancellationException
|
||||||
|
import kotlinx.serialization.SerializationException
|
||||||
import retrofit2.HttpException
|
import retrofit2.HttpException
|
||||||
import java.io.IOException
|
import java.io.IOException
|
||||||
|
|
||||||
@@ -37,6 +38,16 @@ inline fun <T, R> ApiResult<T>.map(transform: (T) -> R): ApiResult<R> = when (th
|
|||||||
* Run a suspending Retrofit call and normalize every outcome into an [ApiResult].
|
* Run a suspending Retrofit call and normalize every outcome into an [ApiResult].
|
||||||
* Coroutine cancellation is rethrown so structured concurrency still works — it
|
* Coroutine cancellation is rethrown so structured concurrency still works — it
|
||||||
* is control flow, not a network failure.
|
* is control flow, not a network failure.
|
||||||
|
*
|
||||||
|
* A body the app can't decode (a field whose type/shape doesn't match its DTO, e.g.
|
||||||
|
* a live-shaped `guild.update` snapshot carrying an unexpected value) throws a
|
||||||
|
* [SerializationException] out of the Retrofit converter. That is a broken contract
|
||||||
|
* with the backend, not a bug to crash on: the request completed but the response is
|
||||||
|
* unusable — an invalid upstream response — so it is surfaced as a server-side error
|
||||||
|
* (`502` → [ErrorKind.SERVER]) the screen renders as "something went wrong, retry",
|
||||||
|
* exactly the graceful-degradation the layer promises (never throw for an expected
|
||||||
|
* failure). Without this catch the exception escapes the collecting coroutine and
|
||||||
|
* takes down the whole app.
|
||||||
*/
|
*/
|
||||||
suspend fun <T> safeApiCall(block: suspend () -> T): ApiResult<T> = try {
|
suspend fun <T> safeApiCall(block: suspend () -> T): ApiResult<T> = try {
|
||||||
ApiResult.Ok(block())
|
ApiResult.Ok(block())
|
||||||
@@ -46,4 +57,9 @@ suspend fun <T> safeApiCall(block: suspend () -> T): ApiResult<T> = try {
|
|||||||
ApiResult.HttpError(e.code(), e.message())
|
ApiResult.HttpError(e.code(), e.message())
|
||||||
} catch (e: IOException) {
|
} catch (e: IOException) {
|
||||||
ApiResult.NetworkError(e)
|
ApiResult.NetworkError(e)
|
||||||
|
} catch (e: SerializationException) {
|
||||||
|
ApiResult.HttpError(MALFORMED_RESPONSE_STATUS, e.message)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Synthetic status for a 2xx body the app couldn't decode — an invalid upstream response. */
|
||||||
|
private const val MALFORMED_RESPONSE_STATUS = 502
|
||||||
|
|||||||
@@ -14,8 +14,8 @@ import kotlinx.serialization.json.JsonObject
|
|||||||
* `CharacterSheet.jsx` / `GameAccounts.jsx` and `docs/link/INTEGRATION.md` §5).
|
* `CharacterSheet.jsx` / `GameAccounts.jsx` and `docs/link/INTEGRATION.md` §5).
|
||||||
* Presentation is text-only for v1 (no item icons / paperdoll).
|
* Presentation is text-only for v1 (no item icons / paperdoll).
|
||||||
*
|
*
|
||||||
* In-game serials are hex strings (e.g. "0x24C"), unlike the numeric serials on
|
* In-game serials are hex strings (e.g. "0x24C"), the same opaque-key form used on
|
||||||
* the public boards — these are separate endpoints with separate shapes.
|
* the public boards (`ShardDto.ActorDto`/`ChampDto`/`HouseDto`) — never numbers.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
// ── Game-account linking ─────────────────────────────────────────────────────
|
// ── Game-account linking ─────────────────────────────────────────────────────
|
||||||
|
|||||||
@@ -15,13 +15,18 @@ import kotlinx.serialization.json.JsonObject
|
|||||||
* `*.update` frames on `/public/shard/stream` decode into these same DTOs.
|
* `*.update` frames on `/public/shard/stream` decode into these same DTOs.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
/** A game actor (player/leader/governor) as embedded in board payloads. */
|
/**
|
||||||
|
* A game actor (player/leader/governor) as embedded in board payloads. Per the wire
|
||||||
|
* spec (`docs/link/INTEGRATION.md` §1), in-game [serial]s are opaque hex-string keys
|
||||||
|
* (e.g. `"0x1A2B"`), never numbers, and [webId] is the linked site-user id as a
|
||||||
|
* string (e.g. `"9931"`) — both are decoded as strings, not parsed.
|
||||||
|
*/
|
||||||
@Serializable
|
@Serializable
|
||||||
data class ActorDto(
|
data class ActorDto(
|
||||||
val serial: Long? = null,
|
val serial: String? = null,
|
||||||
val name: String? = null,
|
val name: String? = null,
|
||||||
val acct: String? = null,
|
val acct: String? = null,
|
||||||
val webId: Long? = null,
|
val webId: String? = null,
|
||||||
) {
|
) {
|
||||||
/** Best display label for this actor. */
|
/** Best display label for this actor. */
|
||||||
val label: String get() = name ?: acct ?: "Someone"
|
val label: String get() = name ?: acct ?: "Someone"
|
||||||
@@ -73,7 +78,7 @@ data class FeedEventDto(
|
|||||||
*/
|
*/
|
||||||
@Serializable
|
@Serializable
|
||||||
data class OnlineStaffDto(
|
data class OnlineStaffDto(
|
||||||
val serial: Long? = null,
|
val serial: String? = null,
|
||||||
val name: String? = null,
|
val name: String? = null,
|
||||||
val map: String? = null,
|
val map: String? = null,
|
||||||
val x: Int? = null,
|
val x: Int? = null,
|
||||||
@@ -87,7 +92,7 @@ data class OnlineStaffDto(
|
|||||||
*/
|
*/
|
||||||
@Serializable
|
@Serializable
|
||||||
data class HouseDto(
|
data class HouseDto(
|
||||||
val serial: Long = 0,
|
val serial: String = "",
|
||||||
val name: String? = null,
|
val name: String? = null,
|
||||||
val region: String? = null,
|
val region: String? = null,
|
||||||
val map: String? = null,
|
val map: String? = null,
|
||||||
@@ -104,7 +109,7 @@ data class HouseDto(
|
|||||||
*/
|
*/
|
||||||
@Serializable
|
@Serializable
|
||||||
data class ChampDto(
|
data class ChampDto(
|
||||||
val serial: Long = 0,
|
val serial: String = "",
|
||||||
val category: String? = null,
|
val category: String? = null,
|
||||||
val type: String? = null,
|
val type: String? = null,
|
||||||
val name: String? = null,
|
val name: String? = null,
|
||||||
|
|||||||
@@ -69,7 +69,9 @@ class ChampsViewModel @Inject constructor(
|
|||||||
private fun applyFrame(frame: ShardStreamEvent.Frame) {
|
private fun applyFrame(frame: ShardStreamEvent.Frame) {
|
||||||
when (frame.kind) {
|
when (frame.kind) {
|
||||||
"champ.update" -> repository.champFrame(frame.data)?.let { board.upsert(it) }
|
"champ.update" -> repository.champFrame(frame.data)?.let { board.upsert(it) }
|
||||||
"champ.remove" -> FrameFields.longField(frame.data, "serial")?.let { board.remove(it.toString()) }
|
// Serial is an opaque hex-string key ("0x…"), not a number — read as a
|
||||||
|
// string (reading it as a Long silently dropped every champ.remove).
|
||||||
|
"champ.remove" -> FrameFields.stringField(frame.data, "serial")?.let { board.remove(it) }
|
||||||
else -> return
|
else -> return
|
||||||
}
|
}
|
||||||
// Only republish when the board actually changed (Success state only).
|
// Only republish when the board actually changed (Success state only).
|
||||||
|
|||||||
@@ -69,7 +69,9 @@ class HousesViewModel @Inject constructor(
|
|||||||
|
|
||||||
private fun applyFrame(frame: ShardStreamEvent.Frame) {
|
private fun applyFrame(frame: ShardStreamEvent.Frame) {
|
||||||
if (frame.kind != "house.decay") return
|
if (frame.kind != "house.decay") return
|
||||||
val serial = FrameFields.longField(frame.data, "serial") ?: return
|
// Serials are opaque hex-string keys ("0x…"), not numbers — read as a string
|
||||||
|
// (reading it as a Long silently dropped every live IDOC update).
|
||||||
|
val serial = FrameFields.stringField(frame.data, "serial") ?: return
|
||||||
// `to` is the new decay stage; only IDOC belongs on the public board.
|
// `to` is the new decay stage; only IDOC belongs on the public board.
|
||||||
val stage = FrameFields.stringField(frame.data, "to")
|
val stage = FrameFields.stringField(frame.data, "to")
|
||||||
?: FrameFields.stringField(frame.data, "stage")
|
?: FrameFields.stringField(frame.data, "stage")
|
||||||
|
|||||||
@@ -35,6 +35,20 @@ class ApiResultTest {
|
|||||||
assertTrue(result is ApiResult.NetworkError)
|
assertTrue(result is ApiResult.NetworkError)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A body the app can't decode (a field whose type doesn't match its DTO) throws a
|
||||||
|
* [SerializationException] out of the Retrofit converter. It must degrade to a
|
||||||
|
* server-side error the UI renders, not escape and crash the app — the guild-board
|
||||||
|
* crash this fixes. `502` folds to [ui.ErrorKind.SERVER] via `toUiState`.
|
||||||
|
*/
|
||||||
|
@Test fun serializationExceptionBecomesServerError() = runTest {
|
||||||
|
val result = safeApiCall {
|
||||||
|
throw kotlinx.serialization.SerializationException("Unexpected symbol 'm' at path: \$[0].members")
|
||||||
|
}
|
||||||
|
assertTrue(result is ApiResult.HttpError)
|
||||||
|
assertEquals(502, (result as ApiResult.HttpError).status)
|
||||||
|
}
|
||||||
|
|
||||||
@Test fun cancellationIsRethrown() = runTest {
|
@Test fun cancellationIsRethrown() = runTest {
|
||||||
assertThrows(CancellationException::class.java) {
|
assertThrows(CancellationException::class.java) {
|
||||||
kotlinx.coroutines.runBlocking {
|
kotlinx.coroutines.runBlocking {
|
||||||
|
|||||||
@@ -43,25 +43,31 @@ class ShardDtoTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Test fun champUpdateFrameDecodesWithKindAndExtras() {
|
@Test fun champUpdateFrameDecodesWithKindAndExtras() {
|
||||||
// A live champ.update frame: has `kind`, `serial`, and category extras. The
|
// A live champ.update frame: has `kind`, a hex-string `serial` (INTEGRATION.md
|
||||||
// `kind` field is ignored (not on the DTO) and the extras decode.
|
// §1 — serials are opaque hex keys, never numbers), and category extras. The
|
||||||
|
// `kind`/`rank`/`autoRestart` fields are ignored (not on the DTO); extras decode.
|
||||||
val dto = json.decodeFromString<ChampDto>(
|
val dto = json.decodeFromString<ChampDto>(
|
||||||
"""{"kind":"champ.update","serial":12345,"category":"champion","name":"Barracoon",
|
"""{"kind":"champ.update","serial":"0x40012345","category":"champion","name":"Barracoon",
|
||||||
"status":"active","active":true,"level":10,"maxKills":250,"kills":120,
|
"status":"active","active":true,"level":10,"rank":3,"maxKills":250,"kills":120,
|
||||||
"bossUp":false,"map":"Felucca","x":5571,"y":1379,"z":0,"t":1721426400000}""",
|
"autoRestart":true,"bossUp":false,"map":"Felucca","x":5571,"y":1379,"z":0,"t":1721426400000}""",
|
||||||
)
|
)
|
||||||
assertEquals(12345L, dto.serial)
|
assertEquals("0x40012345", dto.serial)
|
||||||
assertEquals("champion", dto.category)
|
assertEquals("champion", dto.category)
|
||||||
assertEquals(120, dto.kills)
|
assertEquals(120, dto.kills)
|
||||||
assertTrue(dto.active)
|
assertTrue(dto.active)
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test fun guildFrameDecodesLeaderActor() {
|
@Test fun guildFrameDecodesLeaderActor() {
|
||||||
|
// The leader actor carries a hex-string serial and a string webId (the linked
|
||||||
|
// site-user id) — the exact wire shape from INTEGRATION.md §7.
|
||||||
val dto = json.decodeFromString<GuildDto>(
|
val dto = json.decodeFromString<GuildDto>(
|
||||||
"""{"kind":"guild.update","id":7,"name":"Knights","abbr":"KNT","members":12,
|
"""{"kind":"guild.update","id":7,"name":"Knights","abbr":"KNT","members":12,
|
||||||
"online":3,"alliance":"Light","leader":{"serial":1,"name":"Arthur","acct":"art"}}""",
|
"online":3,"alliance":"Light",
|
||||||
|
"leader":{"serial":"0x1A2B","name":"Arthur","acct":"art","webId":"9931","player":true}}""",
|
||||||
)
|
)
|
||||||
assertEquals(7L, dto.id)
|
assertEquals(7L, dto.id)
|
||||||
|
assertEquals("0x1A2B", dto.leader?.serial)
|
||||||
|
assertEquals("9931", dto.leader?.webId)
|
||||||
assertEquals("Arthur", dto.leader?.label)
|
assertEquals("Arthur", dto.leader?.label)
|
||||||
assertEquals(12, dto.members)
|
assertEquals(12, dto.members)
|
||||||
}
|
}
|
||||||
@@ -86,13 +92,21 @@ class ShardDtoTest {
|
|||||||
|
|
||||||
@Test fun houseDecodesPublicIdocShape() {
|
@Test fun houseDecodesPublicIdocShape() {
|
||||||
val dto = json.decodeFromString<HouseDto>(
|
val dto = json.decodeFromString<HouseDto>(
|
||||||
"""{"serial":999,"name":"Tower","region":"Britain","map":"Felucca",
|
"""{"serial":"0x40001234","name":"Tower","region":"Britain","map":"Felucca",
|
||||||
"x":1,"y":2,"z":3,"isIdoc":true}""",
|
"x":1,"y":2,"z":3,"isIdoc":true}""",
|
||||||
)
|
)
|
||||||
assertEquals(999L, dto.serial)
|
assertEquals("0x40001234", dto.serial)
|
||||||
assertTrue(dto.isIdoc)
|
assertTrue(dto.isIdoc)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test fun onlineStaffDecodesHexSerial() {
|
||||||
|
val dto = json.decodeFromString<OnlineStaffDto>(
|
||||||
|
"""{"serial":"0x24C","name":"Darrow"}""",
|
||||||
|
)
|
||||||
|
assertEquals("0x24C", dto.serial)
|
||||||
|
assertEquals("Darrow", dto.name)
|
||||||
|
}
|
||||||
|
|
||||||
@Test fun actorLabelFallsBackToAcctThenSomeone() {
|
@Test fun actorLabelFallsBackToAcctThenSomeone() {
|
||||||
assertEquals("bob", ActorDto(acct = "bob").label)
|
assertEquals("bob", ActorDto(acct = "bob").label)
|
||||||
assertEquals("Someone", ActorDto().label)
|
assertEquals("Someone", ActorDto().label)
|
||||||
|
|||||||
Reference in New Issue
Block a user