From 4085fa1f864e2ce4391bdbeae73d1fba30247b5c Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Sun, 16 Aug 2026 10:46:17 +0200 Subject: [PATCH 1/3] feat(conversations): cache versioned conversation avatars immutably Conversation avatar responses are cacheable for one day only (the server sends private, max-age=86400, immutable), so once that lifetime expires every memory-cache miss goes back to the network although a versioned avatar URL cannot change - cold starts refetched long-known avatars and showed the fallback avatar in the meantime. Treat versioned conversation-avatar URLs as immutable content: the avatarVersion parameter is the invalidation token, so a dedicated avatar image loader ignores the cache headers and cached avatars never expire or revalidate - a new version changes the URL and forces the fetch, cold starts and offline serve straight from the disk cache. One-to-one rooms are deliberately outside this scheme: their avatarVersion is a server-side constant derived from a static icon path (the conversation avatar setters reject one-to-one rooms) and never changes when the peer updates their user avatar. With the avatar capability they use the conversation-avatar endpoint, which resolves the peer avatar and handles federation proxying, but without a version parameter and on the default header-respecting loader, so the one-day cache lifetime keeps picking up new peer avatars - matching what talk-ios does on purpose. Everything is gated on the avatar capability (Talk 17+; the app has no minimum Talk version, only the Nextcloud 17 EOL block, so far older servers remain supported): without it, one-to-one rooms keep the unversioned user-avatar endpoint and group/public rooms show themed default icons - which also fixes the previous behavior of unconditionally requesting the conversation-avatar endpoint that doesn't exist on such servers. The avatar content resolution moves to a testable top-level function with unit tests for the endpoint and capability selection. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../talk/conversationlist/ui/AvatarContent.kt | 100 ++++++++++ .../ui/ConversationListItem.kt | 44 +---- .../nextcloud/talk/utils/AvatarImageLoader.kt | 39 ++++ .../ui/ConversationAvatarContentTest.kt | 171 ++++++++++++++++++ 4 files changed, 316 insertions(+), 38 deletions(-) create mode 100644 app/src/main/java/com/nextcloud/talk/conversationlist/ui/AvatarContent.kt create mode 100644 app/src/main/java/com/nextcloud/talk/utils/AvatarImageLoader.kt create mode 100644 app/src/test/java/com/nextcloud/talk/conversationlist/ui/ConversationAvatarContentTest.kt diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/ui/AvatarContent.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/ui/AvatarContent.kt new file mode 100644 index 00000000000..6fbe53bafc9 --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/ui/AvatarContent.kt @@ -0,0 +1,100 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.conversationlist.ui + +import androidx.annotation.DrawableRes +import com.nextcloud.talk.R +import com.nextcloud.talk.data.user.model.User +import com.nextcloud.talk.models.domain.ConversationModel +import com.nextcloud.talk.models.json.conversations.ConversationEnums +import com.nextcloud.talk.utils.ApiUtils +import com.nextcloud.talk.utils.SpreedFeatures + +internal sealed class AvatarContent { + /** + * [versioned] marks an immutable avatar URL: the avatarVersion parameter is the invalidation + * token, so the image may be cached without expiry - a new version changes the URL. Only + * conversation avatars are versioned; a one-to-one room's avatar is the peer's user avatar, + * which is outside the version scheme and must revalidate via the response cache headers. + */ + data class Url(val url: String, val versioned: Boolean) : AvatarContent() + data class Res(@param:DrawableRes val resId: Int) : AvatarContent() + object System : AvatarContent() + object NoteToSelf : AvatarContent() +} + +/** + * Resolves what to show as a conversation's avatar. + * + * On servers with the avatar capability (Talk 17+) rooms use the conversation-avatar endpoint. + * For everything but one-to-one rooms the URL carries the avatarVersion as invalidation token, + * so those avatars are immutable content refreshed solely by version changes from the room list + * sync. One-to-one rooms get the peer's user avatar from the same endpoint (which also handles + * federation proxying), but their avatarVersion is a server-side constant that never changes + * when the peer updates their avatar - their URL therefore stays unversioned and relies on the + * default loader's header-driven revalidation. Servers without the capability fall back to the + * unversioned user-avatar endpoint for one-to-one rooms and to themed default icons for group + * and public rooms, whose endpoint does not exist there. + */ +internal fun buildAvatarContent(model: ConversationModel, currentUser: User, isDark: Boolean): AvatarContent { + val hasConversationAvatars = currentUser.hasSpreedFeatureCapability(SpreedFeatures.AVATAR.value) + val avatarVersion = model.avatarVersion.takeIf { it.isNotEmpty() } + + return when { + model.objectType == ConversationEnums.ObjectType.SHARE_PASSWORD -> + AvatarContent.Res(R.drawable.ic_circular_lock) + + model.objectType == ConversationEnums.ObjectType.FILE -> + AvatarContent.Res(R.drawable.ic_avatar_document) + + model.type == ConversationEnums.ConversationType.ROOM_SYSTEM -> + AvatarContent.System + + model.type == ConversationEnums.ConversationType.NOTE_TO_SELF -> + AvatarContent.NoteToSelf + + hasConversationAvatars && model.type == ConversationEnums.ConversationType.ROOM_TYPE_ONE_TO_ONE_CALL -> + AvatarContent.Url( + ApiUtils.getUrlForConversationAvatarWithVersion( + 1, + currentUser.baseUrl, + model.token, + isDark, + null + ), + versioned = false + ) + + hasConversationAvatars && avatarVersion != null -> + AvatarContent.Url( + ApiUtils.getUrlForConversationAvatarWithVersion( + 1, + currentUser.baseUrl, + model.token, + isDark, + avatarVersion + ), + versioned = true + ) + + model.type == ConversationEnums.ConversationType.ROOM_TYPE_ONE_TO_ONE_CALL || + model.type == ConversationEnums.ConversationType.FORMER_ONE_TO_ONE -> + AvatarContent.Url( + ApiUtils.getUrlForAvatar(currentUser.baseUrl, model.name, false, isDark), + versioned = false + ) + + model.type == ConversationEnums.ConversationType.ROOM_GROUP_CALL -> + AvatarContent.Res(R.drawable.ic_circular_group) + + model.type == ConversationEnums.ConversationType.ROOM_PUBLIC_CALL -> + AvatarContent.Res(R.drawable.ic_circular_link) + + else -> + AvatarContent.Res(R.drawable.account_circle_96dp) + } +} diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/ui/ConversationListItem.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/ui/ConversationListItem.kt index 9c12e7abc4a..5ea1389f8e9 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/ui/ConversationListItem.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/ui/ConversationListItem.kt @@ -61,6 +61,7 @@ import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp import androidx.compose.ui.viewinterop.AndroidView import coil.compose.AsyncImage +import coil.imageLoader import coil.request.ImageRequest import com.nextcloud.talk.R import com.nextcloud.talk.chat.data.model.ChatMessage @@ -76,6 +77,7 @@ import com.nextcloud.talk.models.json.conversations.ConversationEnums import com.nextcloud.talk.models.json.participants.Participant import com.nextcloud.talk.ui.StatusDrawable import com.nextcloud.talk.utils.ApiUtils +import com.nextcloud.talk.utils.AvatarImageLoader import com.nextcloud.talk.utils.CapabilitiesUtil.hasSpreedFeatureCapability import com.nextcloud.talk.utils.DisplayUtils import com.nextcloud.talk.utils.SpreedFeatures @@ -92,44 +94,6 @@ private const val UNREAD_THRESHOLD = 1000 private const val UNREAD_BUBBLE_STROKE_DP = 1.5f private const val MILLIS_PER_SECOND = 1_000L -private sealed class AvatarContent { - data class Url(val url: String) : AvatarContent() - data class Res(@param:DrawableRes val resId: Int) : AvatarContent() - object System : AvatarContent() - object NoteToSelf : AvatarContent() -} - -private fun buildAvatarContent(model: ConversationModel, currentUser: User, isDark: Boolean): AvatarContent { - val avatarVersion = model.avatarVersion.takeIf { it.isNotEmpty() } - return when { - model.objectType == ConversationEnums.ObjectType.SHARE_PASSWORD -> - AvatarContent.Res(R.drawable.ic_circular_lock) - - model.objectType == ConversationEnums.ObjectType.FILE -> - AvatarContent.Res(R.drawable.ic_avatar_document) - - model.type == ConversationEnums.ConversationType.ROOM_SYSTEM -> - AvatarContent.System - - model.type == ConversationEnums.ConversationType.NOTE_TO_SELF -> - AvatarContent.NoteToSelf - - model.type == ConversationEnums.ConversationType.ROOM_TYPE_ONE_TO_ONE_CALL -> - AvatarContent.Url(ApiUtils.getUrlForAvatar(currentUser.baseUrl, model.name, false, isDark)) - - else -> - AvatarContent.Url( - ApiUtils.getUrlForConversationAvatarWithVersion( - 1, - currentUser.baseUrl, - model.token, - isDark, - avatarVersion - ) - ) - } -} - /** Groups the tap callbacks for [ConversationListItem] to keep the parameter count low. */ data class ConversationListItemCallbacks(val onClick: () -> Unit, val onLongClick: () -> Unit) @@ -277,6 +241,9 @@ private fun ConversationAvatarImage(model: ConversationModel, currentUser: User, if (isInPreview) { Box(modifier = modifier.background(Color.LightGray)) } else { + val imageLoader = remember(avatarContent.versioned) { + if (avatarContent.versioned) AvatarImageLoader.get(context) else context.imageLoader + } val request = remember(avatarContent.url, credentials) { ImageRequest.Builder(context) .data(avatarContent.url) @@ -287,6 +254,7 @@ private fun ConversationAvatarImage(model: ConversationModel, currentUser: User, } AsyncImage( model = request, + imageLoader = imageLoader, contentDescription = stringResource(R.string.avatar), contentScale = ContentScale.Crop, placeholder = painterResource(R.drawable.account_circle_96dp), diff --git a/app/src/main/java/com/nextcloud/talk/utils/AvatarImageLoader.kt b/app/src/main/java/com/nextcloud/talk/utils/AvatarImageLoader.kt new file mode 100644 index 00000000000..60dd15798e1 --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/utils/AvatarImageLoader.kt @@ -0,0 +1,39 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.utils + +import android.content.Context +import coil.ImageLoader +import coil.imageLoader + +/** + * Image loader for versioned conversation-avatar URLs, which are immutable content: the + * avatarVersion URL parameter is the invalidation token, and a new version changes the URL and + * forces the fetch. The server marks avatar responses as cacheable for one day only (private, + * max-age=86400, immutable); ignoring the cache headers removes that expiry for URLs that cannot + * change, so cold starts and offline serve avatars straight from the disk cache without a + * network round-trip. + * + * Must only be used for URLs carrying a version parameter. Unversioned avatar URLs - notably a + * one-to-one room's avatar, which is the peer's user avatar and outside the version scheme - + * need the default loader's header-driven revalidation to ever pick up changes. + * + * Derived from the default loader, so memory and disk caches are shared between both. + */ +object AvatarImageLoader { + + @Volatile + private var instance: ImageLoader? = null + + fun get(context: Context): ImageLoader = + instance ?: synchronized(this) { + instance ?: context.imageLoader.newBuilder() + .respectCacheHeaders(false) + .build() + .also { instance = it } + } +} diff --git a/app/src/test/java/com/nextcloud/talk/conversationlist/ui/ConversationAvatarContentTest.kt b/app/src/test/java/com/nextcloud/talk/conversationlist/ui/ConversationAvatarContentTest.kt new file mode 100644 index 00000000000..847d866a5d0 --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/conversationlist/ui/ConversationAvatarContentTest.kt @@ -0,0 +1,171 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ + +package com.nextcloud.talk.conversationlist.ui + +import com.nextcloud.talk.R +import com.nextcloud.talk.data.user.model.User +import com.nextcloud.talk.models.domain.ConversationModel +import com.nextcloud.talk.models.json.capabilities.Capabilities +import com.nextcloud.talk.models.json.capabilities.SpreedCapability +import com.nextcloud.talk.models.json.conversations.Conversation +import com.nextcloud.talk.models.json.conversations.ConversationEnums +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class ConversationAvatarContentTest { + + @Test + fun `group rooms use the versioned conversation avatar endpoint with the avatar capability`() { + val content = buildAvatarContent( + conversation(ConversationEnums.ConversationType.ROOM_GROUP_CALL, avatarVersion = "5"), + user(withAvatarCapability = true), + isDark = false + ) + + val url = content as AvatarContent.Url + assertTrue(url.versioned) + assertTrue(url.url.contains("/room/$ROOM_TOKEN/avatar")) + assertTrue(url.url.contains("avatarVersion=5")) + } + + @Test + fun `one-to-one rooms use the unversioned conversation avatar endpoint with the avatar capability`() { + val content = buildAvatarContent( + conversation(ConversationEnums.ConversationType.ROOM_TYPE_ONE_TO_ONE_CALL, avatarVersion = "3"), + user(withAvatarCapability = true), + isDark = false + ) + + val url = content as AvatarContent.Url + assertFalse("one-to-one avatars are outside the version scheme", url.versioned) + assertTrue(url.url.contains("/room/$ROOM_TOKEN/avatar")) + assertFalse(url.url.contains("avatarVersion")) + } + + @Test + fun `one-to-one endpoint selection depends on the capability only, not on the avatar version`() { + val content = buildAvatarContent( + conversation(ConversationEnums.ConversationType.ROOM_TYPE_ONE_TO_ONE_CALL, avatarVersion = ""), + user(withAvatarCapability = true), + isDark = false + ) + + val url = content as AvatarContent.Url + assertFalse(url.versioned) + assertTrue(url.url.contains("/room/$ROOM_TOKEN/avatar")) + } + + @Test + fun `one-to-one avatar urls never carry an avatar version parameter`() { + listOf( + buildAvatarContent( + conversation(ConversationEnums.ConversationType.ROOM_TYPE_ONE_TO_ONE_CALL, avatarVersion = "7"), + user(withAvatarCapability = true), + isDark = false + ), + buildAvatarContent( + conversation(ConversationEnums.ConversationType.ROOM_TYPE_ONE_TO_ONE_CALL, avatarVersion = "7"), + user(withAvatarCapability = false), + isDark = false + ) + ).forEach { content -> + val url = content as AvatarContent.Url + assertFalse(url.versioned) + assertFalse(url.url.contains("avatarVersion")) + } + } + + @Test + fun `one-to-one rooms fall back to the unversioned user avatar endpoint without the capability`() { + val content = buildAvatarContent( + conversation(ConversationEnums.ConversationType.ROOM_TYPE_ONE_TO_ONE_CALL), + user(withAvatarCapability = false), + isDark = false + ) + + // only the endpoint path is asserted: the name segment needs Uri.encode, which is stubbed here + val url = content as AvatarContent.Url + assertFalse(url.versioned) + assertTrue(url.url.contains("/index.php/avatar/")) + } + + @Test + fun `group rooms fall back to the themed default icon without the capability`() { + val content = buildAvatarContent( + conversation(ConversationEnums.ConversationType.ROOM_GROUP_CALL), + user(withAvatarCapability = false), + isDark = false + ) + + assertEquals(AvatarContent.Res(R.drawable.ic_circular_group), content) + } + + @Test + fun `public rooms fall back to the themed default icon without the capability`() { + val content = buildAvatarContent( + conversation(ConversationEnums.ConversationType.ROOM_PUBLIC_CALL), + user(withAvatarCapability = false), + isDark = false + ) + + assertEquals(AvatarContent.Res(R.drawable.ic_circular_link), content) + } + + @Test + fun `an empty avatar version keeps the fallback even with the capability`() { + val content = buildAvatarContent( + conversation(ConversationEnums.ConversationType.ROOM_GROUP_CALL, avatarVersion = ""), + user(withAvatarCapability = true), + isDark = false + ) + + assertEquals(AvatarContent.Res(R.drawable.ic_circular_group), content) + } + + @Test + fun `note to self is independent of the avatar capability`() { + val content = buildAvatarContent( + conversation(ConversationEnums.ConversationType.NOTE_TO_SELF, avatarVersion = "5"), + user(withAvatarCapability = true), + isDark = false + ) + + assertEquals(AvatarContent.NoteToSelf, content) + } + + private fun conversation(type: ConversationEnums.ConversationType, avatarVersion: String = ""): ConversationModel = + ConversationModel.mapToConversationModel( + Conversation( + token = ROOM_TOKEN, + name = PEER_NAME, + type = type, + avatarVersion = avatarVersion + ), + user(withAvatarCapability = true) + ) + + private fun user(withAvatarCapability: Boolean): User { + val features = if (withAvatarCapability) listOf("avatar") else emptyList() + return User( + id = 1L, + userId = "me", + username = "me", + baseUrl = "https://server.example.com", + capabilities = Capabilities().apply { + spreedCapability = SpreedCapability().apply { this.features = features } + } + ) + } + + companion object { + private const val ROOM_TOKEN = "room1" + private const val PEER_NAME = "peer" + } +} From f7c41c70ae5f6cdee6501afe919b2c0e589cbb40 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Sun, 16 Aug 2026 10:49:39 +0200 Subject: [PATCH 2/3] feat(conversations): keep the last avatar visible while it refreshes When an avatar's cache key changed (avatarVersion bump from a room list sync, theme switch) or a load transiently failed, the list flashed the generic fallback avatar until the fetch completed - even though the previous avatar had just been on screen. Every successful avatar load now also stores its bitmap under a stable per-room alias in the shared memory cache. New requests use that alias as both placeholder and error image, so a refresh renders as a crossfade from the old avatar to the new one and transient failures keep the last known avatar. The generic fallback only remains for avatars that were never shown, and for the rare coincidence of a cold start with a simultaneous version change, since the alias lives in memory only. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../ui/ConversationListItem.kt | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/ui/ConversationListItem.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/ui/ConversationListItem.kt index 5ea1389f8e9..a2ab8c5d40e 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/ui/ConversationListItem.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/ui/ConversationListItem.kt @@ -43,6 +43,8 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip import androidx.compose.ui.graphics.Color +import androidx.compose.ui.graphics.asImageBitmap +import androidx.compose.ui.graphics.painter.BitmapPainter import androidx.compose.ui.graphics.toArgb import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.platform.LocalConfiguration @@ -61,7 +63,9 @@ import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp import androidx.compose.ui.viewinterop.AndroidView import coil.compose.AsyncImage +import coil.memory.MemoryCache import coil.imageLoader +import android.graphics.drawable.BitmapDrawable import coil.request.ImageRequest import com.nextcloud.talk.R import com.nextcloud.talk.chat.data.model.ChatMessage @@ -244,21 +248,35 @@ private fun ConversationAvatarImage(model: ConversationModel, currentUser: User, val imageLoader = remember(avatarContent.versioned) { if (avatarContent.versioned) AvatarImageLoader.get(context) else context.imageLoader } + val aliasKey = remember(currentUser.id, model.token, isDark) { + MemoryCache.Key("avatar-${currentUser.id}-${model.token}-" + if (isDark) "dark" else "light") + } val request = remember(avatarContent.url, credentials) { ImageRequest.Builder(context) .data(avatarContent.url) .diskCacheKey("${avatarContent.url}#v2") .addHeader("Authorization", credentials) .crossfade(true) + .listener( + onSuccess = { _, result -> + (result.drawable as? BitmapDrawable)?.bitmap?.let { bitmap -> + imageLoader.memoryCache?.set(aliasKey, MemoryCache.Value(bitmap)) + } + } + ) .build() } + val lastShownAvatar = remember(request) { + imageLoader.memoryCache?.get(aliasKey)?.bitmap?.let { BitmapPainter(it.asImageBitmap()) } + } + val fallback = painterResource(R.drawable.account_circle_96dp) AsyncImage( model = request, imageLoader = imageLoader, contentDescription = stringResource(R.string.avatar), contentScale = ContentScale.Crop, - placeholder = painterResource(R.drawable.account_circle_96dp), - error = painterResource(R.drawable.account_circle_96dp), + placeholder = lastShownAvatar ?: fallback, + error = lastShownAvatar ?: fallback, modifier = modifier ) } From baea94a4e316b21264f44107e2ff0cefd668c9c3 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Sun, 16 Aug 2026 11:57:29 +0200 Subject: [PATCH 3/3] fix(conversations): don't show the fallback avatar while loading Scrolling the conversation list flashed the generic fallback avatar on rows entering the viewport for the first time in a session: the lazy list disposes off-screen rows, and re-entering rows without a memory cache entry showed the fallback as a de-facto loading indicator until the disk read or fetch completed. Render nothing while an avatar loads (the slot keeps its size and the avatar crossfades in), keep the last shown avatar as the placeholder when one is known, and reserve the fallback icon for what its name says: a failed load with no previously shown avatar. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../talk/conversationlist/ui/ConversationListItem.kt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/ui/ConversationListItem.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/ui/ConversationListItem.kt index a2ab8c5d40e..c318ea28dac 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/ui/ConversationListItem.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/ui/ConversationListItem.kt @@ -269,14 +269,13 @@ private fun ConversationAvatarImage(model: ConversationModel, currentUser: User, val lastShownAvatar = remember(request) { imageLoader.memoryCache?.get(aliasKey)?.bitmap?.let { BitmapPainter(it.asImageBitmap()) } } - val fallback = painterResource(R.drawable.account_circle_96dp) AsyncImage( model = request, imageLoader = imageLoader, contentDescription = stringResource(R.string.avatar), contentScale = ContentScale.Crop, - placeholder = lastShownAvatar ?: fallback, - error = lastShownAvatar ?: fallback, + placeholder = lastShownAvatar, + error = lastShownAvatar ?: painterResource(R.drawable.account_circle_96dp), modifier = modifier ) }