Skip to content

Commit 9229304

Browse files
mahibimahibi
committed
fix(chat): retry post-upload refresh and preview loading
Refresh the chat immediately after upload/share completion and retry message fetches briefly when the server still returns 304. Also retry media previews on transient 5xx server errors so newly uploaded images are less likely to stay stuck on a placeholder. AI-assistant: Copilot 1.8.2-243 (GPT-5-Codex) Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
1 parent 26fd59e commit 9229304

7 files changed

Lines changed: 146 additions & 16 deletions

File tree

‎app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -483,13 +483,7 @@ class ChatActivity :
483483

484484
override fun onChatMessagesReceived(chatMessages: List<ChatMessageJson>) {
485485
chatViewModel.onSignalingChatMessageReceived(chatMessages)
486-
487-
Log.d(
488-
TAG,
489-
"received message in ChatActivity. This is the chat message received via HPB. It would be " +
490-
"nicer to receive it in the ViewModel or Repository directly. " +
491-
"Otherwise it needs to be passed into it from here..."
492-
)
486+
Log.d(TAG, "received signaling message in ChatActivity")
493487
}
494488
}
495489

‎app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,16 @@ interface ChatMessageRepository : LifecycleAwareManager {
6060

6161
suspend fun startMessagePolling(hasHighPerformanceBackend: Boolean)
6262

63+
/**
64+
* Performs a one-time non-blocking fetch of messages newer than the latest known message.
65+
* Use this to immediately refresh chat after local actions (e.g. file share) that create
66+
* server-side messages which would otherwise only appear after the next insurance-request cycle.
67+
*
68+
* @return `true` if at least one new message was received, `false` when the server returned
69+
* 304 Not Modified or an empty message list.
70+
*/
71+
suspend fun fetchNewMessages(): Boolean
72+
6373
/**
6474
* Loads messages from local storage. If the messages are not found, then it
6575
* synchronizes the database with the server, before retrying exactly once. Only

‎app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -275,10 +275,12 @@ class OfflineFirstChatRepository @Inject constructor(
275275
}
276276

277277
/**
278-
* Fetches messages newer than latest known message
278+
* Fetches messages newer than latest known message.
279+
*
280+
* @return `true` if at least one new message was received and persisted.
279281
*/
280-
private suspend fun fetchNewMessages() {
281-
var fieldMap = getFieldMap(
282+
override suspend fun fetchNewMessages(): Boolean {
283+
val fieldMap = getFieldMap(
282284
lookIntoFuture = true,
283285
timeout = 0,
284286
includeLastKnown = false,
@@ -288,7 +290,7 @@ class OfflineFirstChatRepository @Inject constructor(
288290
val networkParams = Bundle()
289291
networkParams.putSerializable(BundleKeys.KEY_FIELD_MAP, fieldMap)
290292

291-
getAndPersistMessages(networkParams)
293+
return getAndPersistMessages(networkParams)
292294
}
293295

294296
override suspend fun loadMoreMessages(
@@ -497,7 +499,7 @@ class OfflineFirstChatRepository @Inject constructor(
497499
emit(ChatPullResult.Error(IllegalStateException("All attempts failed")))
498500
}.flowOn(Dispatchers.IO)
499501

500-
private suspend fun getAndPersistMessages(bundle: Bundle) {
502+
private suspend fun getAndPersistMessages(bundle: Bundle): Boolean {
501503
if (!networkMonitor.isOnline.value) {
502504
Log.d(TAG, "Device is offline, can't load chat messages from server")
503505
}
@@ -536,21 +538,26 @@ class OfflineFirstChatRepository @Inject constructor(
536538
lookIntoFuture,
537539
hasHistory
538540
)
541+
return true
539542
} else {
540543
Log.d(TAG, "No new messages to update")
544+
return false
541545
}
542546
}
543547

544548
is ChatPullResult.NotModified -> {
545549
Log.d(TAG, "Server returned NOT_MODIFIED, nothing to update")
550+
return false
546551
}
547552

548553
is ChatPullResult.PreconditionFailed -> {
549554
Log.d(TAG, "Server returned PRECONDITION_FAILED, nothing to update")
555+
return false
550556
}
551557

552558
is ChatPullResult.Error -> {
553559
Log.e(TAG, "Error pulling messages from server", result.throwable)
560+
return false
554561
}
555562
}
556563
}

‎app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import com.nextcloud.talk.data.database.mappers.toDomainModel
3737
import com.nextcloud.talk.data.database.model.ChatMessageEntity
3838
import com.nextcloud.talk.data.user.model.User
3939
import com.nextcloud.talk.extensions.toIntOrZero
40+
import com.nextcloud.talk.jobs.ShareOperationWorker
4041
import com.nextcloud.talk.jobs.UploadAndShareFilesWorker
4142
import com.nextcloud.talk.messagesearch.MessageSearchHelper
4243
import com.nextcloud.talk.models.MessageDraft
@@ -83,6 +84,7 @@ import kotlinx.coroutines.flow.combine
8384
import kotlinx.coroutines.flow.debounce
8485
import kotlinx.coroutines.flow.distinctUntilChanged
8586
import kotlinx.coroutines.flow.distinctUntilChangedBy
87+
import kotlinx.coroutines.flow.filter
8688
import kotlinx.coroutines.flow.filterNotNull
8789
import kotlinx.coroutines.flow.first
8890
import kotlinx.coroutines.flow.flatMapLatest
@@ -1200,6 +1202,50 @@ class ChatViewModel @AssistedInject constructor(
12001202
.launchIn(viewModelScope)
12011203
}
12021204

1205+
private fun observeShareCompleted() {
1206+
ShareOperationWorker.shareCompletedFlow
1207+
.filter { it == chatRoomToken }
1208+
.onEach {
1209+
Log.d(TAG, "Share completed for room $chatRoomToken — fetching new messages immediately")
1210+
fetchNewMessagesWithRetry()
1211+
}
1212+
.launchIn(viewModelScope)
1213+
1214+
UploadAndShareFilesWorker.uploadCompletedFlow
1215+
.filter { it == chatRoomToken }
1216+
.onEach {
1217+
Log.d(TAG, "Upload completed for room $chatRoomToken — fetching new messages immediately")
1218+
UploadAndShareFilesWorker.clearUploadCompletedReplay()
1219+
fetchNewMessagesWithRetry()
1220+
}
1221+
.launchIn(viewModelScope)
1222+
}
1223+
1224+
/**
1225+
* Retries [ChatMessageRepository.fetchNewMessages] up to [POST_UPLOAD_FETCH_MAX_ATTEMPTS] times,
1226+
* waiting [POST_UPLOAD_FETCH_RETRY_DELAY_MS] ms between attempts. Stops as soon as at least one
1227+
* new message is received so that the happy-path (server responds quickly) has no unnecessary
1228+
* delay, while a slow server still gets a few extra chances before we fall back to the regular
1229+
* insurance-request cycle.
1230+
*/
1231+
private suspend fun fetchNewMessagesWithRetry() {
1232+
repeat(POST_UPLOAD_FETCH_MAX_ATTEMPTS) { attempt ->
1233+
if (attempt > 0) {
1234+
Log.d(
1235+
TAG,
1236+
"fetchNewMessagesWithRetry: attempt ${attempt + 1}, " +
1237+
"waiting ${POST_UPLOAD_FETCH_RETRY_DELAY_MS}ms"
1238+
)
1239+
delay(POST_UPLOAD_FETCH_RETRY_DELAY_MS)
1240+
}
1241+
val gotMessages = chatRepository.fetchNewMessages()
1242+
if (gotMessages) {
1243+
Log.d(TAG, "fetchNewMessagesWithRetry: new messages received on attempt ${attempt + 1}")
1244+
return
1245+
}
1246+
}
1247+
Log.d(TAG, "fetchNewMessagesWithRetry: no new messages after $POST_UPLOAD_FETCH_MAX_ATTEMPTS attempts")
1248+
}
12031249
private fun handleSystemMessages(chatMessageList: List<ChatMessage>): List<ChatMessage> {
12041250
fun shouldRemoveMessage(currentMessage: MutableMap.MutableEntry<Int, ChatMessage>): Boolean =
12051251
isInfoMessageAboutDeletion(currentMessage) ||
@@ -1289,6 +1335,7 @@ class ChatViewModel @AssistedInject constructor(
12891335

12901336
observeConversationAndUserFirstTime()
12911337
observeConversationAndUserEveryTime()
1338+
observeShareCompleted()
12921339
}
12931340

12941341
fun ConversationModel?.isOneToOneConversation(): Boolean =
@@ -2181,6 +2228,8 @@ class ChatViewModel @AssistedInject constructor(
21812228
private const val MIN_CHARS_FOR_SEARCH = 2
21822229
private const val CONTEXT_MESSAGES_LIMIT = 50
21832230
private const val LOAD_MORE_MESSAGES_LIMIT = 100
2231+
private const val POST_UPLOAD_FETCH_MAX_ATTEMPTS = 4
2232+
private const val POST_UPLOAD_FETCH_RETRY_DELAY_MS = 1_500L
21842233
}
21852234

21862235
sealed class OutOfOfficeUIState {

‎app/src/main/java/com/nextcloud/talk/jobs/ShareOperationWorker.kt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_INTERNAL_USER_ID
2626
import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_META_DATA
2727
import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_ROOM_TOKEN
2828
import io.reactivex.schedulers.Schedulers
29+
import kotlinx.coroutines.flow.MutableSharedFlow
30+
import kotlinx.coroutines.flow.SharedFlow
2931
import retrofit2.HttpException
3032
import javax.inject.Inject
3133

@@ -49,6 +51,7 @@ class ShareOperationWorker(context: Context, workerParams: WorkerParameters) : W
4951
for (filePath in filesArray) {
5052
tryCreateShare(filePath)
5153
}
54+
roomToken?.let { _shareCompletedFlow.tryEmit(it) }
5255
return Result.success()
5356
}
5457

@@ -100,6 +103,9 @@ class ShareOperationWorker(context: Context, workerParams: WorkerParameters) : W
100103
private const val SHARE_MAX_ATTEMPTS = 4
101104
private const val SHARE_RETRY_DELAY_MS = 2000L
102105

106+
private val _shareCompletedFlow: MutableSharedFlow<String> = MutableSharedFlow(extraBufferCapacity = 1)
107+
val shareCompletedFlow: SharedFlow<String> = _shareCompletedFlow
108+
103109
fun shareFile(roomToken: String?, currentUser: User, remotePath: String, metaData: String?) {
104110
val paths: MutableList<String> = ArrayList()
105111
paths.add(remotePath)

‎app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_ROOM_TOKEN
5050
import com.nextcloud.talk.utils.database.user.CurrentUserProviderOld
5151
import com.nextcloud.talk.utils.permissions.PlatformPermissionUtil
5252
import com.nextcloud.talk.utils.preferences.AppPreferences
53+
import kotlinx.coroutines.ExperimentalCoroutinesApi
54+
import kotlinx.coroutines.flow.MutableSharedFlow
55+
import kotlinx.coroutines.flow.SharedFlow
5356
import kotlinx.coroutines.runBlocking
5457
import okhttp3.MediaType.Companion.toMediaTypeOrNull
5558
import okhttp3.OkHttpClient
@@ -126,6 +129,7 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa
126129

127130
if (uploadSuccess) {
128131
cancelNotification()
132+
_uploadCompletedFlow.tryEmit(roomToken)
129133
return Result.success()
130134
} else if (isStopped) {
131135
// since work is cancelled the result would be ignored anyways
@@ -416,6 +420,17 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa
416420
private const val ZERO_PERCENT = 0
417421
const val REQUEST_PERMISSION = 3123
418422

423+
private val _uploadCompletedFlow: MutableSharedFlow<String> = MutableSharedFlow(
424+
replay = 1,
425+
extraBufferCapacity = 1
426+
)
427+
val uploadCompletedFlow: SharedFlow<String> = _uploadCompletedFlow
428+
429+
@OptIn(ExperimentalCoroutinesApi::class)
430+
fun clearUploadCompletedReplay() {
431+
_uploadCompletedFlow.resetReplayCache()
432+
}
433+
419434
fun requestStoragePermission(activity: Activity) {
420435
when {
421436
Build.VERSION

‎app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt‎

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
package com.nextcloud.talk.ui.chat
99

10+
import android.util.Log
1011
import androidx.compose.foundation.clickable
1112
import androidx.compose.foundation.layout.Box
1213
import androidx.compose.foundation.layout.Column
@@ -18,7 +19,12 @@ import androidx.compose.foundation.shape.RoundedCornerShape
1819
import androidx.compose.material3.CircularProgressIndicator
1920
import androidx.compose.material3.Icon
2021
import androidx.compose.runtime.Composable
22+
import androidx.compose.runtime.getValue
23+
import androidx.compose.runtime.mutableIntStateOf
24+
import androidx.compose.runtime.mutableStateOf
2125
import androidx.compose.runtime.remember
26+
import androidx.compose.runtime.rememberCoroutineScope
27+
import androidx.compose.runtime.setValue
2228
import androidx.compose.ui.Alignment
2329
import androidx.compose.ui.Modifier
2430
import androidx.compose.ui.draw.clip
@@ -31,20 +37,26 @@ import androidx.compose.ui.res.painterResource
3137
import androidx.compose.ui.res.stringResource
3238
import androidx.compose.ui.unit.dp
3339
import coil.compose.AsyncImage
40+
import coil.network.HttpException
3441
import com.nextcloud.talk.R
3542
import com.nextcloud.talk.chat.data.model.FileParameters
3643
import com.nextcloud.talk.chat.data.model.decodeBlurhashPlaceholder
3744
import com.nextcloud.talk.chat.ui.model.ChatMessageUi
3845
import com.nextcloud.talk.chat.ui.model.MessageTypeContent
3946
import com.nextcloud.talk.contacts.load
4047
import com.nextcloud.talk.utils.MimetypeUtils
48+
import kotlinx.coroutines.delay
49+
import kotlinx.coroutines.launch
4150

4251
private const val FILE_PLACEHOLDER_MESSAGE = "{file}"
52+
private const val PREVIEW_MAX_RETRIES = 3
53+
private const val PREVIEW_RETRY_DELAY_MS = 2_000L
54+
private const val TAG = "MediaMessage"
4355

4456
private val mediaRadiusBig = 8.dp
4557
private val mediaRadiusSmall = 2.dp
4658

47-
@Suppress("Detekt.LongMethod", "LongParameterList")
59+
@Suppress("Detekt.LongMethod", "LongParameterList", "CyclomaticComplexMethod")
4860
@Composable
4961
fun MediaMessage(
5062
typeContent: MessageTypeContent.Media,
@@ -88,6 +100,7 @@ fun MediaMessage(
88100
content = {
89101
Column {
90102
val context = LocalContext.current
103+
val scope = rememberCoroutineScope()
91104
val resourceName = context.resources.getResourceEntryName(typeContent.drawableResourceId)
92105
val isGif = MimetypeUtils.isGif(typeContent.mimeType)
93106
val showPlayButton = !typeContent.previewUrl.isNullOrEmpty() &&
@@ -97,6 +110,19 @@ fun MediaMessage(
97110
(isGif && !typeContent.animateGif)
98111
)
99112

113+
var retryCount by remember(typeContent.previewUrl) { mutableIntStateOf(0) }
114+
var retryPending by remember(typeContent.previewUrl) { mutableStateOf(false) }
115+
val retryAwarePreviewUrl = remember(typeContent.previewUrl, retryCount) {
116+
typeContent.previewUrl?.let { previewUrl ->
117+
if (retryCount == 0) {
118+
previewUrl
119+
} else {
120+
val delimiter = if (previewUrl.contains("?")) "&" else "?"
121+
"$previewUrl${delimiter}retryAttempt=$retryCount"
122+
}
123+
}
124+
}
125+
100126
val blurhashPainter = remember(typeContent.blurhash, typeContent.width, typeContent.height) {
101127
decodeBlurhashPlaceholder(typeContent.blurhash, typeContent.width, typeContent.height)
102128
?.asImageBitmap()
@@ -107,9 +133,9 @@ fun MediaMessage(
107133
val h = typeContent.height
108134
if (w != null && h != null && w > 0 && h > 0) w.toFloat() / h else null
109135
}
110-
val loadedImage = remember(typeContent.previewUrl) {
136+
val loadedImage = remember(retryAwarePreviewUrl) {
111137
load(
112-
imageUri = typeContent.previewUrl,
138+
imageUri = retryAwarePreviewUrl,
113139
context = context,
114140
errorPlaceholderImage = typeContent.drawableResourceId,
115141
animated = typeContent.animateGif
@@ -130,7 +156,30 @@ fun MediaMessage(
130156
.padding(mediaInset)
131157
.clip(mediaShape)
132158
.clickable { onImageClick(message.id) },
133-
contentScale = ContentScale.FillWidth
159+
contentScale = ContentScale.FillWidth,
160+
onError = { state ->
161+
val cause = state.result.throwable
162+
val isServerError = cause is HttpException && cause.response.code in 500..599
163+
if (
164+
isServerError &&
165+
!typeContent.previewUrl.isNullOrEmpty() &&
166+
retryCount < PREVIEW_MAX_RETRIES &&
167+
!retryPending
168+
) {
169+
retryPending = true
170+
scope.launch {
171+
Log.d(
172+
TAG,
173+
"Preview returned HTTP ${(cause as HttpException).response.code}, " +
174+
"scheduling retry ${retryCount + 1}/$PREVIEW_MAX_RETRIES " +
175+
"for ${typeContent.previewUrl}"
176+
)
177+
delay(PREVIEW_RETRY_DELAY_MS)
178+
retryCount++
179+
retryPending = false
180+
}
181+
}
182+
}
134183
)
135184

136185
if (showPlayButton) {

0 commit comments

Comments
 (0)