Fix image avatars never appearing on the watch

CHANGELOG.md

@@ -28,6 +28,26 @@ uses semantic versioning (`MAJOR.MINOR.PATCH`).
 
 ### Fixed
 
+- **Image avatars now appear on the watch, on instances that serve files
+  themselves.** A member with an uploaded avatar showed nothing where their
+  picture should be: no image, and not even the initials circle, on Currently
+  Fronting, on Switch Front, and on every tile. Emoji and initials avatars were
+  unaffected, and the same avatars were fine on the phone.
+
+  Whether this happened depended on where the instance keeps its files, which
+  is why it was never visible on an instance backed by separate file hosting.
+  An instance that serves files through its own API hands back an avatar as a
+  path rather than a full address, and the watch asked for that path as though
+  it already were one, which failed silently in both of the places the watch
+  fetches images. An instance with a separate file host hands back a full
+  address, which the watch happened to cope with because there was nothing to
+  work out. Serving files directly is the default, so self-hosted instances
+  were the ones affected.
+
+  The watch now works out the full address the way the phone does, honouring a
+  separate file host when one is configured, and an avatar that still fails to
+  load falls back to the initials circle rather than leaving an empty slot.
+
 - **A field type this build has not heard of no longer reads as an error.** A
   custom field whose type the server knows and the app does not showed "Unknown
   field type" in red where its value should be, so the value a member had put

sheaf/wear/src/main/java/systems/lupine/sheaf/wear/WearApp.kt

@@ -1,5 +1,35 @@
 package systems.lupine.sheaf.wear
 
 import android.app.Application
+import coil.ImageLoader
+import coil.ImageLoaderFactory
+import systems.lupine.sheaf.wear.data.WearAuthManager
+import systems.lupine.sheaf.wear.data.WearFileUrlInterceptor
+import systems.lupine.sheaf.wear.data.WearSettingsStore
 
-class WearApp : Application()
+/**
+ * Teaches Coil where this instance's files actually live.
+ *
+ * The API returns avatars as server-relative paths, so handing one straight to
+ * `AsyncImage` gave Coil a string with no scheme or host, which it failed on
+ * silently: every image avatar on the watch was an empty circle. The phone has
+ * resolved these since the CDN work; the watch had nothing equivalent.
+ *
+ * Done here, on the loader, rather than at each `MemberAvatar` call site:
+ * seven screens draw avatars and none of them should have to know about
+ * `file_cdn_base`.
+ */
+class WearApp : Application(), ImageLoaderFactory {
+
+    override fun newImageLoader(): ImageLoader =
+        ImageLoader.Builder(this)
+            .components {
+                add(
+                    WearFileUrlInterceptor(
+                        auth = WearAuthManager(this@WearApp),
+                        settings = WearSettingsStore(this@WearApp),
+                    ),
+                )
+            }
+            .build()
+}

sheaf/wear/src/main/java/systems/lupine/sheaf/wear/data/FileUrls.kt

@@ -0,0 +1,47 @@
+package systems.lupine.sheaf.wear.data
+
+/**
+ * Resolving the file URLs the API hands back.
+ *
+ * The server returns avatars in its canonical stored form, a server-relative
+ * path: `/v1/files/avatars/<system-id>/<file-id>.png`. Clients are expected to
+ * resolve it, and an instance may serve files from somewhere else entirely,
+ * which `file_cdn_base` in the public auth config names.
+ *
+ * The phone has done this since the CDN work (`RelativeUrlInterceptor`, a Coil
+ * interceptor). The watch never did, so it handed Coil and OkHttp a path with
+ * no scheme or host: Coil failed silently and drew nothing, and
+ * `Request.Builder().url("/v1/...")` threw inside a `runCatching` that
+ * swallowed it, so a tile avatar came back null. A member with an image avatar
+ * showed an empty slot on the watch while the same avatar was fine on the
+ * phone.
+ *
+ * The rules are the phone's, deliberately, so the two clients cannot disagree
+ * about where a file lives.
+ */
+
+/**
+ * The absolute URL to fetch [raw] from, or null when there is nothing usable.
+ *
+ * - An absolute URL on the API's own host moves to the CDN when one is set.
+ * - A leading-slash path resolves against the CDN if set, else the API base.
+ * - A bare relative path is treated as server-relative and resolved the same.
+ * - Any other absolute URL is left exactly as it is, because it belongs to
+ *   somebody else and rewriting it would point it at the wrong host.
+ *
+ * Null means "cannot be fetched": a relative path with no base to resolve it
+ * against. Callers draw their fallback rather than requesting a bad URL.
+ */
+fun resolveFileUrl(raw: String?, baseUrl: String?, cdnBase: String?): String? {
+    val data = raw?.trim()?.ifBlank { null } ?: return null
+    val cdn = cdnBase?.trim()?.trimEnd('/')?.ifBlank { null }
+    val base = baseUrl?.trim()?.trimEnd('/')?.ifBlank { null }
+    val preferred = cdn ?: base
+    return when {
+        cdn != null && base != null && data.startsWith("$base/") ->
+            cdn + data.removePrefix(base)
+        data.startsWith("/") -> preferred?.let { "$it$data" }
+        !data.contains("://") -> preferred?.let { "$it/$data" }
+        else -> data
+    }
+}

sheaf/wear/src/main/java/systems/lupine/sheaf/wear/data/Models.kt

@@ -61,6 +61,17 @@ internal data class TokenPair(
     @Json(name = "refresh_token") val refreshToken: String,
 )
 
+/**
+ * The public auth config, of which the watch needs exactly one field: where
+ * this instance serves files from. Everything else on that payload is the
+ * phone's business (registration mode, support links, captcha), so it is not
+ * modelled here; unknown keys are skipped.
+ */
+@JsonClass(generateAdapter = true)
+internal data class WearAuthConfig(
+    @Json(name = "file_cdn_base") val fileCdnBase: String? = null,
+)
+
 // Request bodies. Top-level (not nested inside WearApiClient) so Moshi codegen
 // can generate adapters for them — KSP won't generate adapters for private
 // nested classes.

sheaf/wear/src/main/java/systems/lupine/sheaf/wear/data/WearApiClient.kt

@@ -50,6 +50,9 @@ class WearApiClient(private val auth: WearAuthManager) {
     // their own WearApiClient over the same on-disk credential store, so a
     // per-instance mutex would not serialize their refreshes.
 
+    /** The instance this watch is signed in to, for resolving file URLs. */
+    val baseUrl: String get() = auth.baseUrl
+
     private fun url(path: String) = "${auth.baseUrl.trimEnd('/')}$path"
 
     // Executes the built request, auto-retries once after a 401 by refreshing the token.
@@ -164,6 +167,23 @@ class WearApiClient(private val auth: WearAuthManager) {
         return moshi.adapter<List<WearMember>>(type).fromJson(body)!!
     }
 
+    /**
+     * Where this instance serves files from, or null for the API host.
+     *
+     * Unauthenticated, like the login screen's read of the same endpoint, so
+     * it works before a token exists and cannot 401 in the middle of a
+     * refresh. Returns null rather than throwing on anything unexpected: not
+     * knowing about a CDN costs nothing on an instance without one, and must
+     * not be the reason a whole refresh fails.
+     */
+    suspend fun getFileCdnBase(): String? = runCatching {
+        val body = execute {
+            Request.Builder().url(url("/v1/auth/config")).build()
+        }
+        moshi.adapter(WearAuthConfig::class.java).fromJson(body)?.fileCdnBase
+            ?.trim()?.ifBlank { null }
+    }.getOrNull()
+
     suspend fun getCurrentFronts(): List<WearFront> {
         val body = execute {
             Request.Builder()

sheaf/wear/src/main/java/systems/lupine/sheaf/wear/data/WearFileUrlInterceptor.kt

@@ -0,0 +1,35 @@
+package systems.lupine.sheaf.wear.data
+
+import coil.intercept.Interceptor
+import coil.request.ImageResult
+
+/**
+ * Resolves a server-relative file path into something Coil can fetch, using
+ * [resolveFileUrl]. The watch's counterpart to the phone's
+ * `RelativeUrlInterceptor`, with the same rules.
+ *
+ * Both sources are read per request rather than captured once: the base URL
+ * arrives when the watch is paired, and the CDN base is refreshed on each
+ * sync, so a loader built at process start would otherwise hold whatever was
+ * known before the watch had been signed in.
+ */
+class WearFileUrlInterceptor(
+    private val auth: WearAuthManager,
+    private val settings: WearSettingsStore,
+) : Interceptor {
+
+    override suspend fun intercept(chain: Interceptor.Chain): ImageResult {
+        val data = chain.request.data
+        if (data is String) {
+            val resolved = resolveFileUrl(
+                raw = data,
+                baseUrl = auth.baseUrl,
+                cdnBase = settings.fileCdnBase,
+            )
+            if (resolved != null && resolved != data) {
+                return chain.proceed(chain.request.newBuilder().data(resolved).build())
+            }
+        }
+        return chain.proceed(chain.request)
+    }
+}

sheaf/wear/src/main/java/systems/lupine/sheaf/wear/data/WearSettingsStore.kt

@@ -27,8 +27,32 @@ class WearSettingsStore(context: Context) {
         _endExistingFronts.value = value
     }
 
+    /**
+     * Where this instance serves files from, when it serves them from
+     * somewhere other than the API host. Null means the API host.
+     *
+     * Not a secret, so it sits here rather than in the encrypted credential
+     * store: it is public configuration, readable from the same unauthenticated
+     * endpoint the login screen already reads. Stored at all because the tile
+     * avatars are rendered on whatever refresh happens to run, which may be
+     * long after the last time the config was fetched.
+     */
+    var fileCdnBase: String?
+        get() = prefs.getString(KEY_FILE_CDN_BASE, null)?.ifBlank { null }
+        set(value) {
+            val cleaned = value?.trim()?.ifBlank { null }
+            // An instance that drops its CDN must stop being asked for files
+            // there, so clearing is a real operation rather than a no-op.
+            if (cleaned == null) {
+                prefs.edit().remove(KEY_FILE_CDN_BASE).apply()
+            } else {
+                prefs.edit().putString(KEY_FILE_CDN_BASE, cleaned).apply()
+            }
+        }
+
     private companion object {
         const val KEY_END_EXISTING_FRONTS = "end_existing_fronts"
+        const val KEY_FILE_CDN_BASE = "file_cdn_base"
         // Mirrors the most common phone-side default — switching front almost
         // always means replacing whoever was there. Power users with cofronter
         // workflows can flip this once and have it stick.

sheaf/wear/src/main/java/systems/lupine/sheaf/wear/data/WearStore.kt

@@ -100,6 +100,13 @@ class WearStore(
             }
         }
         try {
+            // Where this instance serves files from, refreshed before anything
+            // that draws an avatar. An operator can add, change or drop a CDN
+            // at any time, and the watch would otherwise keep asking the old
+            // host until it was re-paired. Best-effort by construction: the
+            // call swallows its own failures and an instance without a CDN
+            // simply answers null.
+            WearSettingsStore(context).fileCdnBase = apiClient.getFileCdnBase()
             members.value = apiClient.getMembers()
             currentFronts.value = apiClient.getCurrentFronts()
             groups.value = apiClient.getGroups()
@@ -147,8 +154,17 @@ class WearStore(
         // shows arbitrary members regardless of whether they're fronting,
         // so we cache the lot. Run on Dispatchers.IO since this can hit
         // the network for URL avatars.
+        //
+        // The tile pipeline builds its own OkHttp requests rather than going
+        // through Coil, so it does not get the loader's URL resolution and has
+        // to be handed the two bases itself.
         kotlinx.coroutines.withContext(kotlinx.coroutines.Dispatchers.IO) {
-            systems.lupine.sheaf.wear.tile.renderTileAvatars(context, members.value)
+            systems.lupine.sheaf.wear.tile.renderTileAvatars(
+                context = context,
+                members = members.value,
+                baseUrl = apiClient.baseUrl,
+                cdnBase = WearSettingsStore(context).fileCdnBase,
+            )
         }
     }
 

sheaf/wear/src/main/java/systems/lupine/sheaf/wear/presentation/MemberAvatar.kt

@@ -5,6 +5,10 @@ import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.size
 import androidx.compose.foundation.shape.CircleShape
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.remember
+import androidx.compose.runtime.setValue
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.draw.clip
@@ -17,6 +21,19 @@ import androidx.wear.compose.material.Text
 import coil.compose.AsyncImage
 import systems.lupine.sheaf.wear.data.WearMember
 
+/**
+ * A member's avatar: their image when there is one, otherwise their emoji or
+ * initials on their colour.
+ *
+ * The URL is resolved by the Coil interceptor installed in
+ * [systems.lupine.sheaf.wear.WearApp], since the API hands back
+ * server-relative paths.
+ *
+ * An image that fails anyway falls back to the circle rather than leaving a
+ * hole. That is what made the unresolved-URL bug so quiet: a member with an
+ * image avatar rendered as nothing at all, on a watch face where an empty slot
+ * next to a name reads as a layout fault rather than a missing picture.
+ */
 @Composable
 fun MemberAvatar(
     member: WearMember,
@@ -27,14 +44,19 @@ fun MemberAvatar(
         ?: MaterialTheme.colors.primary
     val onColor = if (color.luminance() > 0.5f) Color.Black else Color.White
 
-    if (!member.avatarUrl.isNullOrBlank()) {
+    // Keyed on the URL so a member whose avatar changes gets a fresh attempt
+    // rather than inheriting the last one's failure.
+    var failed by remember(member.avatarUrl) { mutableStateOf(false) }
+
+    if (!member.avatarUrl.isNullOrBlank() && !failed) {
         AsyncImage(
             model = member.avatarUrl,
             contentDescription = member.displayNameOrName,
             modifier = modifier
                 .size(size)
                 .clip(CircleShape),
             contentScale = ContentScale.Crop,
+            onError = { failed = true },
         )
     } else {
         Box(

sheaf/wear/src/main/java/systems/lupine/sheaf/wear/tile/TileAvatars.kt

@@ -13,6 +13,7 @@ import androidx.wear.protolayout.ResourceBuilders
 import okhttp3.OkHttpClient
 import okhttp3.Request
 import systems.lupine.sheaf.wear.data.WearMember
+import systems.lupine.sheaf.wear.data.resolveFileUrl
 import java.io.File
 import java.io.FileOutputStream
 import java.nio.ByteBuffer
@@ -51,7 +52,12 @@ internal fun tileAvatarFile(context: Context, memberId: String): File =
  * watch's network has settled) doesn't degrade a good avatar to initials.
  * The next refresh re-tries the fetch.
  */
-internal fun renderTileAvatars(context: Context, members: List<WearMember>) {
+internal fun renderTileAvatars(
+    context: Context,
+    members: List<WearMember>,
+    baseUrl: String? = null,
+    cdnBase: String? = null,
+) {
     val dir = tileAvatarDir(context)
     val live = members.map { "${it.id}.png" }.toSet()
     dir.listFiles()?.forEach { if (it.name !in live) it.delete() }
@@ -66,7 +72,7 @@ internal fun renderTileAvatars(context: Context, members: List<WearMember>) {
     for (m in members) {
         val file = tileAvatarFile(context, m.id)
         val hasUrl = !m.avatarUrl.isNullOrBlank()
-        val fetched = if (hasUrl) fetchAvatarBitmap(m, http) else null
+        val fetched = if (hasUrl) fetchAvatarBitmap(m, http, baseUrl, cdnBase) else null
         val bm = when {
             fetched != null -> cropCircle(scaleSquare(fetched, AVATAR_PX))
             // URL avatar that failed to fetch but is already cached: keep
@@ -85,9 +91,21 @@ internal fun renderTileAvatars(context: Context, members: List<WearMember>) {
     }
 }
 
-/** Download and decode a member's avatar bitmap, or null on any failure. */
-private fun fetchAvatarBitmap(member: WearMember, http: OkHttpClient): Bitmap? {
-    val url = member.avatarUrl?.takeIf { it.isNotBlank() } ?: return null
+/**
+ * Download and decode a member's avatar bitmap, or null on any failure.
+ *
+ * The URL is resolved first. The API hands back a server-relative path, and
+ * `Request.Builder().url("/v1/files/...")` throws on one, which the
+ * `runCatching` below then swallowed: every image avatar on every tile came
+ * back null, and the tile quietly drew the initials fallback instead.
+ */
+private fun fetchAvatarBitmap(
+    member: WearMember,
+    http: OkHttpClient,
+    baseUrl: String?,
+    cdnBase: String?,
+): Bitmap? {
+    val url = resolveFileUrl(member.avatarUrl, baseUrl, cdnBase) ?: return null
     return runCatching {
         http.newCall(Request.Builder().url(url).build()).execute().use { resp ->
             if (!resp.isSuccessful) return@runCatching null