From 1e7bca1202ac35078e62675021836a2f0f401c08 Mon Sep 17 00:00:00 2001 From: IIMacGyverII Date: Fri, 1 May 2026 21:38:13 -0500 Subject: [PATCH 1/4] fix(upload): prevent attachment sharing failure caused by chunked upload filecache bug Files sent as chat attachments never appeared in the conversation. Investigation showed the chunked upload MOVE returned HTTP 201 but a subsequent HEAD on the assembled file returned 404 - Nextcloud's server was not registering the file in oc_filecache on assembly. The OCS sharing API uses the filecache, so it also returned 404 'Wrong path, file/folder does not exist' despite the file existing on disk. Raise the chunked upload threshold from 1 MB to 20 MB so that typical phone photos go through direct PUT upload, which correctly updates the filecache. Chunked upload is still used for files over 20 MB. Additional fixes: - ChunkedFileUploader: build a fresh OkHttpClient without OCS-APIRequest and Accept: application/json headers, which were causing sabre/dav to return JSON 200 instead of XML 207 for PROPFIND, breaking dav4jvm. PROPFIND failures now fall back gracefully to uploading all chunks fresh. - FileUploader: fix createRequestBody() to use readBytes() instead of available(), which returns 0 for content:// URIs. - ShareOperationWorker: retry share creation up to 4 times with 2s delay on 404, as a safety net for large files using chunked upload. Signed-off-by: IIMacGyverII AI-assistant: Copilot 0.45.1 (Claude Sonnet 4.6) --- .../talk/jobs/ShareOperationWorker.kt | 27 ++++++++- .../talk/jobs/UploadAndShareFilesWorker.kt | 2 +- .../upload/chunked/ChunkedFileUploader.kt | 59 +++++++++++++------ .../talk/upload/normal/FileUploader.kt | 45 +++++++------- 4 files changed, 87 insertions(+), 46 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/jobs/ShareOperationWorker.kt b/app/src/main/java/com/nextcloud/talk/jobs/ShareOperationWorker.kt index 0aed90350f..6dbb9fde5a 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/ShareOperationWorker.kt +++ b/app/src/main/java/com/nextcloud/talk/jobs/ShareOperationWorker.kt @@ -26,6 +26,7 @@ import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_INTERNAL_USER_ID import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_META_DATA import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_ROOM_TOKEN import io.reactivex.schedulers.Schedulers +import retrofit2.HttpException import javax.inject.Inject @AutoInjector(NextcloudTalkApplication::class) @@ -46,6 +47,16 @@ class ShareOperationWorker(context: Context, workerParams: WorkerParameters) : W override fun doWork(): Result { for (filePath in filesArray) { + tryCreateShare(filePath) + } + return Result.success() + } + + @Suppress("TooGenericExceptionCaught") + private fun tryCreateShare(filePath: String?) { + for (attempt in 1..SHARE_MAX_ATTEMPTS) { + var succeeded = false + var shouldRetry = false ncApi.createRemoteShare( credentials, ApiUtils.getSharingUrl(baseUrl!!), @@ -56,11 +67,18 @@ class ShareOperationWorker(context: Context, workerParams: WorkerParameters) : W ) .subscribeOn(Schedulers.io()) .blockingSubscribe( - {}, - { e -> Log.w(TAG, "error while creating RemoteShare", e) } + { succeeded = true }, + { e -> + if (e is HttpException && e.code() == HTTP_NOT_FOUND && attempt < SHARE_MAX_ATTEMPTS) { + shouldRetry = true + } else { + Log.w(TAG, "error while creating RemoteShare", e) + } + } ) + if (succeeded || !shouldRetry) return + Thread.sleep(SHARE_RETRY_DELAY_MS) } - return Result.success() } init { @@ -78,6 +96,9 @@ class ShareOperationWorker(context: Context, workerParams: WorkerParameters) : W companion object { private val TAG = ShareOperationWorker::class.simpleName + private const val HTTP_NOT_FOUND = 404 + private const val SHARE_MAX_ATTEMPTS = 4 + private const val SHARE_RETRY_DELAY_MS = 2000L fun shareFile(roomToken: String?, currentUser: User, remotePath: String, metaData: String?) { val paths: MutableList = ArrayList() diff --git a/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt b/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt index db698b6513..688d85a3f6 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt +++ b/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt @@ -409,7 +409,7 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa private const val ROOM_TOKEN = "ROOM_TOKEN" private const val CONVERSATION_NAME = "CONVERSATION_NAME" private const val META_DATA = "META_DATA" - private const val CHUNK_UPLOAD_THRESHOLD_SIZE: Long = 1024000 + private const val CHUNK_UPLOAD_THRESHOLD_SIZE: Long = 20 * 1024 * 1024 private const val NOTIFICATION_FILE_NAME_MAX_LENGTH = 20 private const val THREE_DOTS = "…" private const val HUNDRED_PERCENT = 100 diff --git a/app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt b/app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt index a4a7507dac..fa0e1f1f94 100644 --- a/app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt +++ b/app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt @@ -125,7 +125,7 @@ class ChunkedFileUploader( } } - @Suppress("Detekt.ComplexMethod") + @Suppress("Detekt.ComplexMethod", "Detekt.ReturnCount") private fun getUploadedChunks(davResource: DavResource, uploadFolderUri: String): MutableList { val davResponse = DavResponse() val memberElements: MutableList = ArrayList() @@ -145,9 +145,14 @@ class ChunkedFileUploader( Unit } } catch (e: IOException) { - throw IOException("Error reading remote path", e) + // PROPFIND on Nextcloud chunked-upload folders can return unexpected responses + // (e.g. 200 instead of 207). Treat any failure as "no chunks uploaded yet" so + // we fall back to a full upload rather than aborting entirely. + Log.w(TAG, "PROPFIND failed — assuming no chunks on server, will upload from scratch: ${e.message}") + return ArrayList() } catch (e: DavException) { - throw IOException("Error reading remote path", e) + Log.w(TAG, "PROPFIND failed — assuming no chunks on server, will upload from scratch: ${e.message}") + return ArrayList() } for (memberElement in memberElements) { remoteFiles.add( @@ -274,21 +279,23 @@ class ChunkedFileUploader( } private fun initHttpClient(okHttpClient: OkHttpClient, currentUser: User) { - val okHttpClientBuilder: OkHttpClient.Builder = okHttpClient.newBuilder() - okHttpClientBuilder.followRedirects(false) - okHttpClientBuilder.followSslRedirects(false) - // okHttpClientBuilder.readTimeout(Duration.ofMinutes(30)) // TODO set timeout - okHttpClientBuilder.protocols(listOf(Protocol.HTTP_1_1)) - okHttpClientBuilder.authenticator( - RestModule.HttpAuthenticator( - ApiUtils.getCredentials( - currentUser.username, - currentUser.token - )!!, - "Authorization" + val builder = OkHttpClient.Builder() + .followRedirects(false) + .followSslRedirects(false) + .protocols(listOf(Protocol.HTTP_1_1)) + .sslSocketFactory(okHttpClient.sslSocketFactory, okHttpClient.x509TrustManager!!) + .hostnameVerifier(okHttpClient.hostnameVerifier) + .authenticator( + RestModule.HttpAuthenticator( + ApiUtils.getCredentials( + currentUser.username, + currentUser.token + )!!, + "Authorization" + ) ) - ) - this.okHttpClientNoRedirects = okHttpClientBuilder.build() + okHttpClient.proxy?.let { builder.proxy(it) } + this.okHttpClientNoRedirects = builder.build() } private fun assembleChunks(uploadFolderUri: String, targetPath: String, useConversationSubfolders: Boolean) { @@ -298,6 +305,7 @@ class ChunkedFileUploader( targetPath ) + createRemoteFolder(targetPath) val originUri = "$uploadFolderUri/.file" DavResource( @@ -322,6 +330,23 @@ class ChunkedFileUploader( } } + private fun createRemoteFolder(targetPath: String) { + val folderPath = targetPath.substringBeforeLast('/') + if (folderPath.isEmpty() || folderPath == "/") return + val folderUri = ApiUtils.getUrlForFileUpload(currentUser.baseUrl!!, currentUser.userId!!, folderPath) + try { + DavResource(okHttpClientNoRedirects!!, folderUri.toHttpUrlOrNull()!!).mkCol( + xmlBody = null + ) { _ -> } + } catch (e: HttpException) { + if (e.code != METHOD_NOT_ALLOWED_CODE) { + Log.w(TAG, "Unexpected error creating remote folder $folderPath: ${e.code}") + } + } catch (e: IOException) { + Log.w(TAG, "Failed to create remote folder $folderPath", e) + } + } + fun abortUpload(onSuccess: () -> Unit) { isUploadAborted = true try { diff --git a/app/src/main/java/com/nextcloud/talk/upload/normal/FileUploader.kt b/app/src/main/java/com/nextcloud/talk/upload/normal/FileUploader.kt index 9d7809d358..2143e6ca95 100644 --- a/app/src/main/java/com/nextcloud/talk/upload/normal/FileUploader.kt +++ b/app/src/main/java/com/nextcloud/talk/upload/normal/FileUploader.kt @@ -30,7 +30,6 @@ import okhttp3.RequestBody import okhttp3.Response import java.io.File import java.io.IOException -import java.io.InputStream class FileUploader( okHttpClient: OkHttpClient, @@ -131,37 +130,33 @@ class FileUploader( .flatMap { upload(sourceFileUri, fileName, remotePath, metaData) } @Suppress("Detekt.TooGenericExceptionCaught") - private fun createRequestBody(sourceFileUri: Uri): RequestBody? { - var requestBody: RequestBody? = null + private fun createRequestBody(sourceFileUri: Uri): RequestBody? = try { - val input: InputStream = context.contentResolver.openInputStream(sourceFileUri)!! - input.use { - val buf = ByteArray(input.available()) - while (it.read(buf) != -1) { - requestBody = RequestBody.create("application/octet-stream".toMediaTypeOrNull(), buf) - } - } + val bytes = context.contentResolver.openInputStream(sourceFileUri)!!.use { it.readBytes() } + RequestBody.create("application/octet-stream".toMediaTypeOrNull(), bytes) } catch (e: Exception) { Log.e(TAG, "failed to create RequestBody for $sourceFileUri", e) + null } - return requestBody - } private fun initHttpClient(okHttpClient: OkHttpClient, currentUser: User) { - val okHttpClientBuilder: OkHttpClient.Builder = okHttpClient.newBuilder() - okHttpClientBuilder.followRedirects(false) - okHttpClientBuilder.followSslRedirects(false) - okHttpClientBuilder.protocols(listOf(Protocol.HTTP_1_1)) - okHttpClientBuilder.authenticator( - RestModule.HttpAuthenticator( - ApiUtils.getCredentials( - currentUser.username, - currentUser.token - )!!, - "Authorization" + val builder = OkHttpClient.Builder() + .followRedirects(false) + .followSslRedirects(false) + .protocols(listOf(Protocol.HTTP_1_1)) + .sslSocketFactory(okHttpClient.sslSocketFactory, okHttpClient.x509TrustManager!!) + .hostnameVerifier(okHttpClient.hostnameVerifier) + .authenticator( + RestModule.HttpAuthenticator( + ApiUtils.getCredentials( + currentUser.username, + currentUser.token + )!!, + "Authorization" + ) ) - ) - this.okHttpClientNoRedirects = okHttpClientBuilder.build() + okHttpClient.proxy?.let { builder.proxy(it) } + this.okHttpClientNoRedirects = builder.build() } @Suppress("Detekt.ThrowsCount") From 26fd59ea558aa07d0024f56da69ac48b4b7b8a51 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 13 May 2026 14:40:49 +0200 Subject: [PATCH 2/4] fix(upload): scope OCS headers and reuse shared upload client Apply OCS-specific headers only for OCS endpoints in the global headers interceptor to avoid affecting DAV requests. Switch chunked and normal uploaders back to okHttpClient.newBuilder() so they inherit shared client config while keeping upload-specific redirect/protocol/auth settings. AI-assistant: Copilot 1.8.2-243 (GPT-5-Codex) Signed-off-by: Marcel Hibbe --- .../talk/dagger/modules/RestModule.java | 23 +++++++++++++++---- .../upload/chunked/ChunkedFileUploader.kt | 2 +- .../talk/upload/normal/FileUploader.kt | 2 +- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/dagger/modules/RestModule.java b/app/src/main/java/com/nextcloud/talk/dagger/modules/RestModule.java index 06c2f0d581..42f49dc383 100644 --- a/app/src/main/java/com/nextcloud/talk/dagger/modules/RestModule.java +++ b/app/src/main/java/com/nextcloud/talk/dagger/modules/RestModule.java @@ -240,20 +240,33 @@ OkHttpClient provideHttpClient(Proxy proxy, AppPreferences appPreferences, public static class HeadersInterceptor implements Interceptor { + private static final String OCS_V1_PATH = "/ocs/v1.php"; + private static final String OCS_V2_PATH = "/ocs/v2.php"; + @NonNull @Override public Response intercept(@NonNull Chain chain) throws IOException { Request original = chain.request(); - Request request = original.newBuilder() + Request.Builder requestBuilder = original.newBuilder() .header("User-Agent", ApiUtils.getUserAgent()) - .header("Accept", "application/json") - .header("OCS-APIRequest", "true") .header("ngrok-skip-browser-warning", "true") - .method(original.method(), original.body()) - .build(); + .method(original.method(), original.body()); + + if (isOcsEndpoint(original)) { + requestBuilder + .header("Accept", "application/json") + .header("OCS-APIRequest", "true"); + } + + Request request = requestBuilder.build(); return chain.proceed(request); } + + private boolean isOcsEndpoint(@NonNull Request request) { + String path = request.url().encodedPath(); + return path.contains(OCS_V1_PATH) || path.contains(OCS_V2_PATH); + } } public static class HttpAuthenticator implements Authenticator { diff --git a/app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt b/app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt index fa0e1f1f94..7e8b2b1e34 100644 --- a/app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt +++ b/app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt @@ -279,7 +279,7 @@ class ChunkedFileUploader( } private fun initHttpClient(okHttpClient: OkHttpClient, currentUser: User) { - val builder = OkHttpClient.Builder() + val builder = okHttpClient.newBuilder() .followRedirects(false) .followSslRedirects(false) .protocols(listOf(Protocol.HTTP_1_1)) diff --git a/app/src/main/java/com/nextcloud/talk/upload/normal/FileUploader.kt b/app/src/main/java/com/nextcloud/talk/upload/normal/FileUploader.kt index 2143e6ca95..55e1d334b8 100644 --- a/app/src/main/java/com/nextcloud/talk/upload/normal/FileUploader.kt +++ b/app/src/main/java/com/nextcloud/talk/upload/normal/FileUploader.kt @@ -140,7 +140,7 @@ class FileUploader( } private fun initHttpClient(okHttpClient: OkHttpClient, currentUser: User) { - val builder = OkHttpClient.Builder() + val builder = okHttpClient.newBuilder() .followRedirects(false) .followSslRedirects(false) .protocols(listOf(Protocol.HTTP_1_1)) From 9229304591e002905a29bf299c5434e66b91e7c1 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 13 May 2026 16:32:35 +0200 Subject: [PATCH 3/4] 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 --- .../com/nextcloud/talk/chat/ChatActivity.kt | 8 +-- .../talk/chat/data/ChatMessageRepository.kt | 10 ++++ .../network/OfflineFirstChatRepository.kt | 17 ++++-- .../talk/chat/viewmodels/ChatViewModel.kt | 49 ++++++++++++++++ .../talk/jobs/ShareOperationWorker.kt | 6 ++ .../talk/jobs/UploadAndShareFilesWorker.kt | 15 +++++ .../nextcloud/talk/ui/chat/MediaMessage.kt | 57 +++++++++++++++++-- 7 files changed, 146 insertions(+), 16 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt b/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt index 75d58e7e09..0158fe88b9 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt @@ -483,13 +483,7 @@ class ChatActivity : override fun onChatMessagesReceived(chatMessages: List) { chatViewModel.onSignalingChatMessageReceived(chatMessages) - - Log.d( - TAG, - "received message in ChatActivity. This is the chat message received via HPB. It would be " + - "nicer to receive it in the ViewModel or Repository directly. " + - "Otherwise it needs to be passed into it from here..." - ) + Log.d(TAG, "received signaling message in ChatActivity") } } diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt index ea2d34dec2..ea01130777 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt @@ -60,6 +60,16 @@ interface ChatMessageRepository : LifecycleAwareManager { suspend fun startMessagePolling(hasHighPerformanceBackend: Boolean) + /** + * Performs a one-time non-blocking fetch of messages newer than the latest known message. + * Use this to immediately refresh chat after local actions (e.g. file share) that create + * server-side messages which would otherwise only appear after the next insurance-request cycle. + * + * @return `true` if at least one new message was received, `false` when the server returned + * 304 Not Modified or an empty message list. + */ + suspend fun fetchNewMessages(): Boolean + /** * Loads messages from local storage. If the messages are not found, then it * synchronizes the database with the server, before retrying exactly once. Only diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index 4311687b52..cc980f9b0e 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -275,10 +275,12 @@ class OfflineFirstChatRepository @Inject constructor( } /** - * Fetches messages newer than latest known message + * Fetches messages newer than latest known message. + * + * @return `true` if at least one new message was received and persisted. */ - private suspend fun fetchNewMessages() { - var fieldMap = getFieldMap( + override suspend fun fetchNewMessages(): Boolean { + val fieldMap = getFieldMap( lookIntoFuture = true, timeout = 0, includeLastKnown = false, @@ -288,7 +290,7 @@ class OfflineFirstChatRepository @Inject constructor( val networkParams = Bundle() networkParams.putSerializable(BundleKeys.KEY_FIELD_MAP, fieldMap) - getAndPersistMessages(networkParams) + return getAndPersistMessages(networkParams) } override suspend fun loadMoreMessages( @@ -497,7 +499,7 @@ class OfflineFirstChatRepository @Inject constructor( emit(ChatPullResult.Error(IllegalStateException("All attempts failed"))) }.flowOn(Dispatchers.IO) - private suspend fun getAndPersistMessages(bundle: Bundle) { + private suspend fun getAndPersistMessages(bundle: Bundle): Boolean { if (!networkMonitor.isOnline.value) { Log.d(TAG, "Device is offline, can't load chat messages from server") } @@ -536,21 +538,26 @@ class OfflineFirstChatRepository @Inject constructor( lookIntoFuture, hasHistory ) + return true } else { Log.d(TAG, "No new messages to update") + return false } } is ChatPullResult.NotModified -> { Log.d(TAG, "Server returned NOT_MODIFIED, nothing to update") + return false } is ChatPullResult.PreconditionFailed -> { Log.d(TAG, "Server returned PRECONDITION_FAILED, nothing to update") + return false } is ChatPullResult.Error -> { Log.e(TAG, "Error pulling messages from server", result.throwable) + return false } } } diff --git a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt index a1cd1b5ab7..f0bc6ca0e3 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt @@ -37,6 +37,7 @@ import com.nextcloud.talk.data.database.mappers.toDomainModel import com.nextcloud.talk.data.database.model.ChatMessageEntity import com.nextcloud.talk.data.user.model.User import com.nextcloud.talk.extensions.toIntOrZero +import com.nextcloud.talk.jobs.ShareOperationWorker import com.nextcloud.talk.jobs.UploadAndShareFilesWorker import com.nextcloud.talk.messagesearch.MessageSearchHelper import com.nextcloud.talk.models.MessageDraft @@ -83,6 +84,7 @@ import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.distinctUntilChangedBy +import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flatMapLatest @@ -1200,6 +1202,50 @@ class ChatViewModel @AssistedInject constructor( .launchIn(viewModelScope) } + private fun observeShareCompleted() { + ShareOperationWorker.shareCompletedFlow + .filter { it == chatRoomToken } + .onEach { + Log.d(TAG, "Share completed for room $chatRoomToken — fetching new messages immediately") + fetchNewMessagesWithRetry() + } + .launchIn(viewModelScope) + + UploadAndShareFilesWorker.uploadCompletedFlow + .filter { it == chatRoomToken } + .onEach { + Log.d(TAG, "Upload completed for room $chatRoomToken — fetching new messages immediately") + UploadAndShareFilesWorker.clearUploadCompletedReplay() + fetchNewMessagesWithRetry() + } + .launchIn(viewModelScope) + } + + /** + * Retries [ChatMessageRepository.fetchNewMessages] up to [POST_UPLOAD_FETCH_MAX_ATTEMPTS] times, + * waiting [POST_UPLOAD_FETCH_RETRY_DELAY_MS] ms between attempts. Stops as soon as at least one + * new message is received so that the happy-path (server responds quickly) has no unnecessary + * delay, while a slow server still gets a few extra chances before we fall back to the regular + * insurance-request cycle. + */ + private suspend fun fetchNewMessagesWithRetry() { + repeat(POST_UPLOAD_FETCH_MAX_ATTEMPTS) { attempt -> + if (attempt > 0) { + Log.d( + TAG, + "fetchNewMessagesWithRetry: attempt ${attempt + 1}, " + + "waiting ${POST_UPLOAD_FETCH_RETRY_DELAY_MS}ms" + ) + delay(POST_UPLOAD_FETCH_RETRY_DELAY_MS) + } + val gotMessages = chatRepository.fetchNewMessages() + if (gotMessages) { + Log.d(TAG, "fetchNewMessagesWithRetry: new messages received on attempt ${attempt + 1}") + return + } + } + Log.d(TAG, "fetchNewMessagesWithRetry: no new messages after $POST_UPLOAD_FETCH_MAX_ATTEMPTS attempts") + } private fun handleSystemMessages(chatMessageList: List): List { fun shouldRemoveMessage(currentMessage: MutableMap.MutableEntry): Boolean = isInfoMessageAboutDeletion(currentMessage) || @@ -1289,6 +1335,7 @@ class ChatViewModel @AssistedInject constructor( observeConversationAndUserFirstTime() observeConversationAndUserEveryTime() + observeShareCompleted() } fun ConversationModel?.isOneToOneConversation(): Boolean = @@ -2181,6 +2228,8 @@ class ChatViewModel @AssistedInject constructor( private const val MIN_CHARS_FOR_SEARCH = 2 private const val CONTEXT_MESSAGES_LIMIT = 50 private const val LOAD_MORE_MESSAGES_LIMIT = 100 + private const val POST_UPLOAD_FETCH_MAX_ATTEMPTS = 4 + private const val POST_UPLOAD_FETCH_RETRY_DELAY_MS = 1_500L } sealed class OutOfOfficeUIState { diff --git a/app/src/main/java/com/nextcloud/talk/jobs/ShareOperationWorker.kt b/app/src/main/java/com/nextcloud/talk/jobs/ShareOperationWorker.kt index 6dbb9fde5a..c7f473e41e 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/ShareOperationWorker.kt +++ b/app/src/main/java/com/nextcloud/talk/jobs/ShareOperationWorker.kt @@ -26,6 +26,8 @@ import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_INTERNAL_USER_ID import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_META_DATA import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_ROOM_TOKEN import io.reactivex.schedulers.Schedulers +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.SharedFlow import retrofit2.HttpException import javax.inject.Inject @@ -49,6 +51,7 @@ class ShareOperationWorker(context: Context, workerParams: WorkerParameters) : W for (filePath in filesArray) { tryCreateShare(filePath) } + roomToken?.let { _shareCompletedFlow.tryEmit(it) } return Result.success() } @@ -100,6 +103,9 @@ class ShareOperationWorker(context: Context, workerParams: WorkerParameters) : W private const val SHARE_MAX_ATTEMPTS = 4 private const val SHARE_RETRY_DELAY_MS = 2000L + private val _shareCompletedFlow: MutableSharedFlow = MutableSharedFlow(extraBufferCapacity = 1) + val shareCompletedFlow: SharedFlow = _shareCompletedFlow + fun shareFile(roomToken: String?, currentUser: User, remotePath: String, metaData: String?) { val paths: MutableList = ArrayList() paths.add(remotePath) diff --git a/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt b/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt index 688d85a3f6..a1af4fd44e 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt +++ b/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt @@ -50,6 +50,9 @@ import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_ROOM_TOKEN import com.nextcloud.talk.utils.database.user.CurrentUserProviderOld import com.nextcloud.talk.utils.permissions.PlatformPermissionUtil import com.nextcloud.talk.utils.preferences.AppPreferences +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.runBlocking import okhttp3.MediaType.Companion.toMediaTypeOrNull import okhttp3.OkHttpClient @@ -126,6 +129,7 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa if (uploadSuccess) { cancelNotification() + _uploadCompletedFlow.tryEmit(roomToken) return Result.success() } else if (isStopped) { // since work is cancelled the result would be ignored anyways @@ -416,6 +420,17 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa private const val ZERO_PERCENT = 0 const val REQUEST_PERMISSION = 3123 + private val _uploadCompletedFlow: MutableSharedFlow = MutableSharedFlow( + replay = 1, + extraBufferCapacity = 1 + ) + val uploadCompletedFlow: SharedFlow = _uploadCompletedFlow + + @OptIn(ExperimentalCoroutinesApi::class) + fun clearUploadCompletedReplay() { + _uploadCompletedFlow.resetReplayCache() + } + fun requestStoragePermission(activity: Activity) { when { Build.VERSION diff --git a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt index bd43a92c36..82e5f41b68 100644 --- a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt +++ b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt @@ -7,6 +7,7 @@ package com.nextcloud.talk.ui.chat +import android.util.Log import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -18,7 +19,12 @@ import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.Icon import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableIntStateOf +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip @@ -31,6 +37,7 @@ import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import coil.compose.AsyncImage +import coil.network.HttpException import com.nextcloud.talk.R import com.nextcloud.talk.chat.data.model.FileParameters import com.nextcloud.talk.chat.data.model.decodeBlurhashPlaceholder @@ -38,13 +45,18 @@ import com.nextcloud.talk.chat.ui.model.ChatMessageUi import com.nextcloud.talk.chat.ui.model.MessageTypeContent import com.nextcloud.talk.contacts.load import com.nextcloud.talk.utils.MimetypeUtils +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch private const val FILE_PLACEHOLDER_MESSAGE = "{file}" +private const val PREVIEW_MAX_RETRIES = 3 +private const val PREVIEW_RETRY_DELAY_MS = 2_000L +private const val TAG = "MediaMessage" private val mediaRadiusBig = 8.dp private val mediaRadiusSmall = 2.dp -@Suppress("Detekt.LongMethod", "LongParameterList") +@Suppress("Detekt.LongMethod", "LongParameterList", "CyclomaticComplexMethod") @Composable fun MediaMessage( typeContent: MessageTypeContent.Media, @@ -88,6 +100,7 @@ fun MediaMessage( content = { Column { val context = LocalContext.current + val scope = rememberCoroutineScope() val resourceName = context.resources.getResourceEntryName(typeContent.drawableResourceId) val isGif = MimetypeUtils.isGif(typeContent.mimeType) val showPlayButton = !typeContent.previewUrl.isNullOrEmpty() && @@ -97,6 +110,19 @@ fun MediaMessage( (isGif && !typeContent.animateGif) ) + var retryCount by remember(typeContent.previewUrl) { mutableIntStateOf(0) } + var retryPending by remember(typeContent.previewUrl) { mutableStateOf(false) } + val retryAwarePreviewUrl = remember(typeContent.previewUrl, retryCount) { + typeContent.previewUrl?.let { previewUrl -> + if (retryCount == 0) { + previewUrl + } else { + val delimiter = if (previewUrl.contains("?")) "&" else "?" + "$previewUrl${delimiter}retryAttempt=$retryCount" + } + } + } + val blurhashPainter = remember(typeContent.blurhash, typeContent.width, typeContent.height) { decodeBlurhashPlaceholder(typeContent.blurhash, typeContent.width, typeContent.height) ?.asImageBitmap() @@ -107,9 +133,9 @@ fun MediaMessage( val h = typeContent.height if (w != null && h != null && w > 0 && h > 0) w.toFloat() / h else null } - val loadedImage = remember(typeContent.previewUrl) { + val loadedImage = remember(retryAwarePreviewUrl) { load( - imageUri = typeContent.previewUrl, + imageUri = retryAwarePreviewUrl, context = context, errorPlaceholderImage = typeContent.drawableResourceId, animated = typeContent.animateGif @@ -130,7 +156,30 @@ fun MediaMessage( .padding(mediaInset) .clip(mediaShape) .clickable { onImageClick(message.id) }, - contentScale = ContentScale.FillWidth + contentScale = ContentScale.FillWidth, + onError = { state -> + val cause = state.result.throwable + val isServerError = cause is HttpException && cause.response.code in 500..599 + if ( + isServerError && + !typeContent.previewUrl.isNullOrEmpty() && + retryCount < PREVIEW_MAX_RETRIES && + !retryPending + ) { + retryPending = true + scope.launch { + Log.d( + TAG, + "Preview returned HTTP ${(cause as HttpException).response.code}, " + + "scheduling retry ${retryCount + 1}/$PREVIEW_MAX_RETRIES " + + "for ${typeContent.previewUrl}" + ) + delay(PREVIEW_RETRY_DELAY_MS) + retryCount++ + retryPending = false + } + } + } ) if (showPlayButton) { From 7e8105d2353feef310eddefab7cd44f15823d48b Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 13 May 2026 17:40:09 +0200 Subject: [PATCH 4/4] revert chunk upload threshold to 1MB this is done because for the normal upload there is no upload progress for now. Once there is an upload progress shown also for the normal uploads, it's fine to increase the threshold again Signed-off-by: Marcel Hibbe --- .../java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt b/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt index a1af4fd44e..e84edc0537 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt +++ b/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt @@ -413,7 +413,7 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa private const val ROOM_TOKEN = "ROOM_TOKEN" private const val CONVERSATION_NAME = "CONVERSATION_NAME" private const val META_DATA = "META_DATA" - private const val CHUNK_UPLOAD_THRESHOLD_SIZE: Long = 20 * 1024 * 1024 + private const val CHUNK_UPLOAD_THRESHOLD_SIZE: Long = 1024 * 1024 private const val NOTIFICATION_FILE_NAME_MAX_LENGTH = 20 private const val THREE_DOTS = "…" private const val HUNDRED_PERCENT = 100