Merge pull request 'feat(appearance): read the admin theme and nav contract into a SiteAppearance (M12 phase 0)' (#33) from feat/m12-phase-0-appearance-store into edge

Reviewed-on: #33
Reviewed-by: Colby Whitlock <whitlocktech@gmail.com>

app/src/main/java/com/runicgateway/app/MainActivity.kt

@@ -23,6 +23,7 @@ import androidx.compose.ui.Modifier
 import com.runicgateway.app.core.auth.sso.SsoAuthManager
 import com.runicgateway.app.core.push.PushNotifier
 import androidx.hilt.navigation.compose.hiltViewModel
+import androidx.lifecycle.compose.LifecycleResumeEffect
 import androidx.lifecycle.compose.collectAsStateWithLifecycle
 import com.runicgateway.app.ui.AppViewModel
 import com.runicgateway.app.ui.AppViewModel.AppState
@@ -69,7 +70,16 @@ class MainActivity : ComponentActivity() {
             val appViewModel: AppViewModel = hiltViewModel()
             val state by appViewModel.state.collectAsStateWithLifecycle()
 
-            val accent = (state as? AppState.Ready)?.brand?.let { parseBrandColor(it.accent) }
+            val appearance = (state as? AppState.Ready)?.appearance
+            val accent = appearance?.brand?.let { parseBrandColor(it.accent) }
+
+            // The admin's theme and nav can change while the app is backgrounded
+            // (THEMING_AND_NAV.md §5.5). Re-read them on resume, beside the session
+            // re-validation RunicApp already does. Best-effort and silent.
+            LifecycleResumeEffect(Unit) {
+                appViewModel.refreshAppearance()
+                onPauseOrDispose { }
+            }
 
             RunicGatewayTheme(accent = accent) {
                 CompositionLocalProvider(LocalAssetResolver provides appViewModel::resolveAsset) {
@@ -83,7 +93,7 @@ class MainActivity : ComponentActivity() {
                                 ConnectScreen(onConnected = appViewModel::onConnected)
                             is AppState.Ready ->
                                 RunicApp(
-                                    brand = s.brand,
+                                    brand = s.appearance.brand,
                                     onChangeServer = appViewModel::changeServer,
                                     deepLinkStream = pendingStream,
                                     onDeepLinkConsumed = { pendingStream = null },

app/src/main/java/com/runicgateway/app/data/api/dto/PublicDto.kt

@@ -5,6 +5,7 @@ package com.runicgateway.app.data.api.dto
 
 import kotlinx.serialization.SerialName
 import kotlinx.serialization.Serializable
+import kotlinx.serialization.json.JsonElement
 
 /**
  * DTOs for the public site/identity endpoints. Shapes mirror the backend
@@ -80,4 +81,28 @@ data class SettingsDto(
     val brand: BrandDto = BrandDto(),
     /** Push relay config (M7); default (null ntfyUrl) on a backend that predates it. */
     val push: PushConfigDto = PushConfigDto(),
+    /**
+     * The admin's **resolved** theme tokens — the CSS custom properties the site
+     * paints, already layered `:root ← preset ← custom` by the server
+     * (THEMING_AND_NAV.md §3). Absent when no `theme_visual` row exists, which
+     * means "the shipped defaults" and is the untouched-instance path.
+     *
+     * Held as a raw [JsonElement] rather than a `Map<String, String>` on
+     * purpose: a single unexpected value must not fail the decode of the whole
+     * settings payload and take `brand` and `push` down with it. It is coerced
+     * field-by-field by `SiteAppearance.from`.
+     *
+     * The raw `theme_visual` / `brand_assets` rows ride along in this same
+     * response and are deliberately **not** modeled — they are inputs, and
+     * re-deriving a palette from them would be a second `resolveThemeTokens` in
+     * Kotlin, guaranteed to drift (§3).
+     */
+    val theme: JsonElement? = null,
+    /**
+     * The public nav overrides, as the raw JSON **string** stored in
+     * `settings.value` (TEXT) — so it is parsed a second time, exactly as the web
+     * client's `parseJsonSetting` does. Absent when the admin never edited the
+     * nav.
+     */
+    @SerialName("nav_public") val navPublic: String? = null,
 )

app/src/main/java/com/runicgateway/app/data/appearance/SettingsJson.kt

@@ -0,0 +1,37 @@
+/*
+ * SPDX-License-Identifier: GPL-3.0-or-later
+ */
+package com.runicgateway.app.data.appearance
+
+import kotlinx.serialization.SerializationException
+import kotlinx.serialization.json.Json
+import kotlinx.serialization.json.JsonObject
+
+/**
+ * Parse a JSON-valued settings row, client side — the second stage of decoding
+ * `nav_public` (THEMING_AND_NAV.md §3).
+ *
+ * The Kotlin counterpart to the web client's `lib/settingsJson.js`, and
+ * deliberately the same three lines of judgement: `settings.value` is TEXT, so
+ * the row arrives as a **string inside** the already-decoded settings object,
+ * and a malformed or wrong-shaped one must read as **absent** — the surface
+ * falls back to the coded default — never as an error and never as a
+ * half-applied object.
+ */
+private val settingsJson = Json { ignoreUnknownKeys = true }
+
+/**
+ * @param raw the raw stored value, as it arrived in the settings payload
+ * @return the parsed object, or null when absent/malformed
+ */
+fun parseJsonSetting(raw: String?): JsonObject? {
+    if (raw.isNullOrEmpty()) return null
+    val parsed = try {
+        settingsJson.parseToJsonElement(raw)
+    } catch (_: SerializationException) {
+        return null
+    }
+    // Only plain objects. A stored `null`, `4`, `"x"` or array is as unusable to
+    // every consumer of these keys as a syntax error is.
+    return parsed as? JsonObject
+}

app/src/main/java/com/runicgateway/app/data/appearance/SiteAppearance.kt

@@ -0,0 +1,73 @@
+/*
+ * SPDX-License-Identifier: GPL-3.0-or-later
+ */
+package com.runicgateway.app.data.appearance
+
+import com.runicgateway.app.data.api.dto.BrandDto
+import com.runicgateway.app.data.api.dto.SettingsDto
+import kotlinx.serialization.json.JsonObject
+import kotlinx.serialization.json.JsonPrimitive
+
+/**
+ * Everything the app renders itself with that the shard's admin controls
+ * (THEMING_AND_NAV.md, M12): the brand block, the resolved theme tokens, and the
+ * public navigation overrides. One value, held once in [com.runicgateway.app.ui.AppViewModel],
+ * so the theme and the drawer can never disagree about which shard they are showing.
+ *
+ * **[NONE] is the shipped app.** An instance with no settings rows, a backend
+ * that predates the feature, and a settings call that failed outright are all the
+ * same state here, and all three must render exactly as the app did before this
+ * milestone existed (§2). That is why nothing on this class is nullable except
+ * [brand], which was already nullable and whose absence already meant "use the
+ * bundled strings".
+ */
+data class SiteAppearance(
+    /** The per-shard branding block; null when settings couldn't be loaded. */
+    val brand: BrandDto? = null,
+    /**
+     * The resolved CSS custom properties, keyed by token (`"--accent"` → `"#7f99bd"`).
+     * Empty means "the shipped defaults" — the server never emits an empty map,
+     * but absent and empty are the same thing to the app and it must not depend
+     * on that.
+     */
+    val theme: Map<String, String> = emptyMap(),
+    /**
+     * The parsed `nav_public` row, or null when the admin never edited the nav.
+     * Kept as the raw object here; reading `items` / `sections` / `links` out of
+     * it is the job of the phases that render them.
+     */
+    val navPublic: JsonObject? = null,
+) {
+    companion object {
+        /** The shipped app: no brand, no overrides. Also what a failed load means. */
+        val NONE = SiteAppearance()
+
+        /**
+         * Build the appearance from a `GET /public/settings` body. Forgiving
+         * field by field (§2): a bad `--accent` must not discard a good `--bg`
+         * beside it, and a malformed `nav_public` must not cost the theme.
+         */
+        fun from(settings: SettingsDto?): SiteAppearance {
+            if (settings == null) return NONE
+            return SiteAppearance(
+                brand = settings.brand,
+                theme = themeTokens(settings.theme as? JsonObject),
+                navPublic = parseJsonSetting(settings.navPublic),
+            )
+        }
+
+        // Every themable token is a string server-side (validated on write, and
+        // resolveThemeTokens only ever copies a validated value). Anything else
+        // is dropped rather than coerced, so an unexpected value costs exactly
+        // its own token and the rest of the palette still applies.
+        private fun themeTokens(raw: JsonObject?): Map<String, String> {
+            if (raw.isNullOrEmpty()) return emptyMap()
+            return buildMap {
+                for ((token, value) in raw) {
+                    val text = (value as? JsonPrimitive)?.takeIf { it.isString }?.content
+                    if (!text.isNullOrBlank()) put(token, text)
+                }
+            }
+        }
+    }
+}

app/src/main/java/com/runicgateway/app/ui/AppViewModel.kt

@@ -8,7 +8,7 @@ import androidx.lifecycle.viewModelScope
 import com.runicgateway.app.core.net.BaseUrlHolder
 import com.runicgateway.app.core.push.PushManager
 import com.runicgateway.app.core.result.ApiResult
-import com.runicgateway.app.data.api.dto.BrandDto
+import com.runicgateway.app.data.appearance.SiteAppearance
 import com.runicgateway.app.data.repository.ConnectionRepository
 import com.runicgateway.app.data.repository.SettingsRepository
 import dagger.hilt.android.lifecycle.HiltViewModel
@@ -20,8 +20,8 @@ import javax.inject.Inject
 
 /**
  * Top-level app gate (PLAN.md §3): decides whether the first-run connect screen
- * or the main UI shows, and holds the per-shard branding the theme is seeded
+ * or the main UI shows, and holds the per-shard [SiteAppearance] the theme and
- * from. Activity-scoped so the whole app observes one state.
+ * the drawer are built from. Activity-scoped so the whole app observes one state.
  */
 @HiltViewModel
 class AppViewModel @Inject constructor(
@@ -38,8 +38,11 @@ class AppViewModel @Inject constructor(
         /** No shard site configured yet — show the connect screen. */
         data object NeedsConnection : AppState
 
-        /** A site is configured; [brand] is null if branding couldn't be loaded (still usable). */
+        /**
-        data class Ready(val brand: BrandDto?) : AppState
+         * A site is configured. [appearance] is [SiteAppearance.NONE] when settings
+         * couldn't be loaded — the shipped app, still fully usable (§2).
+         */
+        data class Ready(val appearance: SiteAppearance) : AppState
     }
 
     private val _state = MutableStateFlow<AppState>(AppState.Loading)
@@ -48,7 +51,7 @@ class AppViewModel @Inject constructor(
     init {
         viewModelScope.launch {
             _state.value = if (connectionRepository.restore()) {
-                AppState.Ready(loadBrand())
+                AppState.Ready(loadAppearance())
             } else {
                 AppState.NeedsConnection
             }
@@ -57,7 +60,30 @@ class AppViewModel @Inject constructor(
 
     /** Called by the connect screen once a site has been validated + saved. */
     fun onConnected() {
-        viewModelScope.launch { _state.value = AppState.Ready(loadBrand()) }
+        viewModelScope.launch { _state.value = AppState.Ready(loadAppearance()) }
+    }
+
+    /**
+     * Re-read the appearance while the app is already running — on resume, beside
+     * the session's own re-validation (§5.5). An admin who re-skins the site from
+     * a laptop and picks the phone up should see it.
+     *
+     * Best-effort, and silent either way: a failed refresh **keeps the last good
+     * appearance** rather than dropping back to the shipped one, so a moment of
+     * no connectivity does not repaint a themed shard. There is no loading state
+     * and no error surface. Ignored unless a site is configured.
+     */
+    fun refreshAppearance() {
+        if (_state.value !is AppState.Ready) return
+        viewModelScope.launch {
+            val settings = (settingsRepository.getSettings() as? ApiResult.Ok)?.data ?: return@launch
+            pushManager.setNtfyUrl(settings.push.ntfyUrl)
+            // changeServer() may have raced us back to the connect screen while the
+            // call was in flight; don't resurrect Ready on top of it.
+            if (_state.value is AppState.Ready) {
+                _state.value = AppState.Ready(SiteAppearance.from(settings))
+            }
+        }
     }
 
     /** Settings → Server switch: hard reset back to the connect screen (§3). */
@@ -69,14 +95,14 @@ class AppViewModel @Inject constructor(
     }
 
     /**
-     * Load public settings for branding and feed the shard's push relay URL into the
+     * Load public settings for the appearance and feed the shard's push relay URL into
-     * [PushManager] (§11) — its arrival is what lets push re-register after a restart
+     * the [PushManager] (§11) — its arrival is what lets push re-register after a restart
-     * or sign-in. Returns the brand block (null if settings couldn't be loaded).
+     * or sign-in. Returns [SiteAppearance.NONE] if settings couldn't be loaded.
      */
-    private suspend fun loadBrand(): BrandDto? {
+    private suspend fun loadAppearance(): SiteAppearance {
         val settings = (settingsRepository.getSettings() as? ApiResult.Ok)?.data
         pushManager.setNtfyUrl(settings?.push?.ntfyUrl)
-        return settings?.brand
+        return SiteAppearance.from(settings)
     }
 
     /**

app/src/test/java/com/runicgateway/app/data/api/dto/PublicDtoTest.kt

@@ -4,6 +4,8 @@
 package com.runicgateway.app.data.api.dto
 
 import kotlinx.serialization.json.Json
+import kotlinx.serialization.json.JsonObject
+import kotlinx.serialization.json.jsonPrimitive
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertTrue
@@ -69,5 +71,32 @@ class PublicDtoTest {
         assertFalse(dto.registration.password)
         assertEquals("", dto.brand.name)
         assertEquals(null, dto.push.ntfyUrl)
+        // …and one that predates admin theming: both M12 fields are simply absent
+        // (THEMING_AND_NAV.md §2 — absence means the shipped defaults).
+        assertEquals(null, dto.theme)
+        assertEquals(null, dto.navPublic)
+    }
+
+    @Test fun settingsDecodesTheThemeAndNavRows() {
+        val dto = json.decodeFromString<SettingsDto>(
+            """{
+                "theme":{"--accent":"#c8a45c","--radius-card":"3px"},
+                "nav_public":"{\"/wiki\":{\"label\":\"Codex\"}}",
+                "theme_visual":"{\"preset\":\"fantasy\"}"
+            }""",
+        )
+        assertEquals("#c8a45c", (dto.theme as JsonObject)["--accent"]?.jsonPrimitive?.content)
+        // nav_public stays a raw string here: settings.value is TEXT, so it is
+        // parsed a second time by SiteAppearance.
+        assertEquals("""{"/wiki":{"label":"Codex"}}""", dto.navPublic)
+    }
+
+    @Test fun anUnexpectedThemeKindStillDecodesTheRest() {
+        // `theme` is a raw JsonElement precisely so a value we did not expect
+        // cannot fail the decode and take brand/push with it.
+        val dto = json.decodeFromString<SettingsDto>(
+            """{"theme":"nonsense","brand":{"name":"UOMysticmoon"}}""",
+        )
+        assertEquals("UOMysticmoon", dto.brand.name)
     }
 }

app/src/test/java/com/runicgateway/app/data/appearance/SettingsJsonTest.kt

@@ -0,0 +1,57 @@
+/*
+ * SPDX-License-Identifier: GPL-3.0-or-later
+ */
+package com.runicgateway.app.data.appearance
+
+import kotlinx.serialization.json.JsonPrimitive
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Test
+
+/**
+ * The second-stage parse of a JSON-valued settings row (THEMING_AND_NAV.md §3).
+ * The rule under test is the one the web client's `parseJsonSetting` states:
+ * anything that is not a plain object reads as **absent**, never as an error.
+ */
+class SettingsJsonTest {
+
+    @Test fun parsesAPlainObject() {
+        val parsed = parseJsonSetting("""{"/wiki":{"label":"Codex","order":0}}""")
+        assertEquals(1, parsed!!.size)
+        assertEquals(setOf("/wiki"), parsed.keys)
+    }
+
+    @Test fun parsesTheWrappedPublicShape() {
+        val parsed = parseJsonSetting(
+            """{"items":{"/":{"hidden":true}},"sections":[{"id":"s1","label":"Play"}],"links":[]}""",
+        )
+        assertEquals(setOf("items", "sections", "links"), parsed!!.keys)
+    }
+
+    @Test fun absentValuesReadAsNull() {
+        assertNull(parseJsonSetting(null))
+        assertNull(parseJsonSetting(""))
+    }
+
+    @Test fun malformedJsonReadsAsNull() {
+        assertNull(parseJsonSetting("{"))
+        assertNull(parseJsonSetting("""{"a":}"""))
+        assertNull(parseJsonSetting("not json at all"))
+    }
+
+    @Test fun nonObjectJsonReadsAsNull() {
+        // A stored `null`, number, string or array is as unusable to every
+        // consumer of these keys as a syntax error is.
+        assertNull(parseJsonSetting("null"))
+        assertNull(parseJsonSetting("4"))
+        assertNull(parseJsonSetting("\"x\""))
+        assertNull(parseJsonSetting("[]"))
+    }
+
+    @Test fun unusualKeysAndValuesSurviveVerbatim() {
+        // The parse stage validates the *kind*, not the shape — a nonsense entry
+        // is dropped later, by the phase that reads it.
+        val parsed = parseJsonSetting("""{"/site/news":{"order":"first"},"nonsense":7}""")
+        assertEquals(JsonPrimitive(7), parsed!!["nonsense"])
+    }
+}

app/src/test/java/com/runicgateway/app/data/appearance/SiteAppearanceTest.kt

@@ -0,0 +1,97 @@
+/*
+ * SPDX-License-Identifier: GPL-3.0-or-later
+ */
+package com.runicgateway.app.data.appearance
+
+import com.runicgateway.app.data.api.dto.SettingsDto
+import kotlinx.serialization.json.Json
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Assert.assertSame
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+/**
+ * [SiteAppearance.from] — the coercion between the settings payload and what the
+ * theme and the drawer read (THEMING_AND_NAV.md §2, §3).
+ *
+ * The claims that matter here are the two the milestone rests on: an untouched
+ * instance resolves to *nothing* (so the shipped app renders), and a bad token
+ * costs exactly its own token.
+ */
+class SiteAppearanceTest {
+
+    private val json = Json {
+        ignoreUnknownKeys = true
+        explicitNulls = false
+        coerceInputValues = true
+    }
+
+    private fun appearanceOf(body: String) =
+        SiteAppearance.from(json.decodeFromString<SettingsDto>(body))
+
+    @Test fun untouchedInstanceResolvesToNoOverrides() {
+        // No theme_visual row, no nav_public row: the shipped app, exactly (§2).
+        val appearance = appearanceOf("""{"brand":{"name":"UOMysticmoon"}}""")
+        assertTrue(appearance.theme.isEmpty())
+        assertNull(appearance.navPublic)
+        assertEquals("UOMysticmoon", appearance.brand?.name)
+    }
+
+    @Test fun failedSettingsCallIsTheSameAsNoOverrides() {
+        assertSame(SiteAppearance.NONE, SiteAppearance.from(null))
+        assertNull(SiteAppearance.NONE.brand)
+        assertTrue(SiteAppearance.NONE.theme.isEmpty())
+        assertNull(SiteAppearance.NONE.navPublic)
+    }
+
+    @Test fun resolvedThemeTokensAreReadAsAMap() {
+        val appearance = appearanceOf(
+            """{"theme":{"--accent":"#c8a45c","--bg":"#1a1410","--radius-card":"10px",
+                         "--shadow-card":"none","--sans":"Inter, sans-serif"}}""",
+        )
+        assertEquals("#c8a45c", appearance.theme["--accent"])
+        assertEquals("#1a1410", appearance.theme["--bg"])
+        assertEquals("10px", appearance.theme["--radius-card"])
+        assertEquals("none", appearance.theme["--shadow-card"])
+        assertEquals("Inter, sans-serif", appearance.theme["--sans"])
+    }
+
+    @Test fun anEmptyThemeMapIsTheSameAsAbsent() {
+        // The server returns null rather than {} — the app must not depend on that.
+        assertTrue(appearanceOf("""{"theme":{}}""").theme.isEmpty())
+    }
+
+    @Test fun aBadTokenCostsOnlyItself() {
+        // AC-2 in miniature at the decode boundary: a non-string or blank value is
+        // dropped field-by-field, and its neighbours still apply.
+        val appearance = appearanceOf(
+            """{"theme":{"--accent":"#c8a45c","--bg":7,"--line":null,"--ink":"  "}}""",
+        )
+        assertEquals(mapOf("--accent" to "#c8a45c"), appearance.theme)
+    }
+
+    @Test fun aThemeOfTheWrongKindDoesNotCostTheBrand() {
+        // The whole reason `theme` is modeled as a raw JsonElement: one unexpected
+        // value must not fail the decode and take brand and push down with it.
+        val appearance = appearanceOf(
+            """{"theme":"not an object","brand":{"name":"UOMysticmoon","accent":"#7f99bd"},
+                "push":{"ntfyUrl":"https://ntfy.example.com"}}""",
+        )
+        assertTrue(appearance.theme.isEmpty())
+        assertEquals("#7f99bd", appearance.brand?.accent)
+    }
+
+    @Test fun navPublicIsParsedASecondTime() {
+        // It arrives as a JSON string inside a JSON object, because settings.value
+        // is TEXT.
+        val appearance = appearanceOf("""{"nav_public":"{\"/wiki\":{\"label\":\"Codex\"}}"}""")
+        assertEquals(setOf("/wiki"), appearance.navPublic?.keys)
+    }
+
+    @Test fun aMalformedNavPublicDoesNotCostTheTheme() {
+        val appearance = appearanceOf("""{"nav_public":"{oops","theme":{"--accent":"#c8a45c"}}""")
+        assertNull(appearance.navPublic)
+        assertEquals("#c8a45c", appearance.theme["--accent"])
+    }
+}