Skip to content

Upload workers – foreground mode & retry polish #65

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
guruz merged 8 commits into from
Dec 1, 2025
Merged

Upload workers – foreground mode & retry polish #65

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6562587
feat: add foreground service support for upload workers
Nov 4, 2025
fee847c
feat: add foreground update scheduling during file upload progress
Nov 4, 2025
e33830d
refactor: use cached fileSize instead of redundant file system call
Nov 4, 2025
98516c6
feat: add exponential backoff retry policy for upload workers
Nov 4, 2025
4e9be1b
Fix tests and detekt warnings
Nov 25, 2025
c7e1735
Optimize TUS upload: remove redundant HEAD request
Nov 26, 2025
d7ba049
fix: resolve detekt issues and syntax error in TusUploadHelper
Nov 26, 2025
db52ee4
test: add TUS integration tests for upload lifecycle, creation with u…
Nov 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions opencloudApp/src/main/AndroidManifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
<uses-permission android:name="android.permission.WAKE_LOCK" />
<uses-permission android:name="android.permission.FOREGROUND_SERVICE" />
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_MEDIA_PLAYBACK" />
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_DATA_SYNC" />

<application
android:name=".MainApp"
Expand Down Expand Up @@ -178,6 +179,11 @@
</provider>

<service android:name=".services.OperationsService" />
<service
android:name="androidx.work.impl.foreground.SystemForegroundService"
android:foregroundServiceType="dataSync"
android:exported="false"
tools:replace="android:foregroundServiceType" />
<service
android:name=".media.MediaService"
android:foregroundServiceType="mediaPlayback"
Expand Down
108 changes: 91 additions & 17 deletions opencloudApp/src/main/java/eu/opencloud/android/workers/UploadFileFromContentUriWorker.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,14 @@
package eu.opencloud.android.workers

import android.accounts.Account
import android.app.Notification
import android.content.Context
import android.content.pm.ServiceInfo
import android.net.Uri
import androidx.core.net.toUri
import androidx.documentfile.provider.DocumentFile
import androidx.work.CoroutineWorker
import androidx.work.ForegroundInfo
import androidx.work.WorkerParameters
import androidx.work.workDataOf
import eu.opencloud.android.R
Expand All @@ -51,21 +54,22 @@ import eu.opencloud.android.lib.common.network.OnDatatransferProgressListener
import eu.opencloud.android.lib.common.operations.RemoteOperationResult.ResultCode
import eu.opencloud.android.lib.resources.files.CheckPathExistenceRemoteOperation
import eu.opencloud.android.lib.resources.files.CreateRemoteFolderOperation
import eu.opencloud.android.lib.resources.files.UploadFileFromFileSystemOperation
import eu.opencloud.android.presentation.authentication.AccountUtils
import eu.opencloud.android.utils.NotificationUtils
import eu.opencloud.android.utils.UPLOAD_NOTIFICATION_CHANNEL_ID
import eu.opencloud.android.utils.RemoteFileUtils.getAvailableRemotePath
import eu.opencloud.android.lib.resources.files.UploadFileFromFileSystemOperation
import eu.opencloud.android.utils.UPLOAD_NOTIFICATION_CHANNEL_ID
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.launch
import org.koin.core.component.KoinComponent
import org.koin.core.component.inject
import timber.log.Timber
import java.io.File
import java.io.FileOutputStream
import java.io.IOException

import java.io.InputStream
import kotlin.coroutines.cancellation.CancellationException

class UploadFileFromContentUriWorker(
Expand All @@ -91,34 +95,34 @@ class UploadFileFromContentUriWorker(
private lateinit var uploadFileOperation: UploadFileFromFileSystemOperation
private val tusUploadHelper by lazy { TusUploadHelper(transferRepository) }

private var lastPercent = 0
private var lastPercent = -1

private val transferRepository: TransferRepository by inject()
private val getWebdavUrlForSpaceUseCase: GetWebDavUrlForSpaceUseCase by inject()
private val getStoredCapabilitiesUseCase: GetStoredCapabilitiesUseCase by inject()

override suspend fun doWork(): Result = try {
prepareFile()
startForeground()
val clientForThisUpload = getClientForThisUpload()
checkParentFolderExistence(clientForThisUpload)
checkNameCollisionAndGetAnAvailableOneInCase(clientForThisUpload)
uploadDocument(clientForThisUpload)
updateUploadsDatabaseWithResult(null)
Result.success()
}catch (throwable: Throwable) {
} catch (throwable: Throwable) {
Timber.e(throwable)

if (shouldRetry(throwable)) {
Timber.i("Retrying upload %d after transient failure", uploadIdInStorageManager)
Result.retry()
}else {
} else {
showNotification(throwable)
updateUploadsDatabaseWithResult(throwable)
Result.failure()
}
}


private fun prepareFile() {
if (!areParametersValid()) return

Expand Down Expand Up @@ -161,7 +165,7 @@ class UploadFileFromContentUriWorker(
if (it != null) {
Timber.d("Upload with id ($uploadIdInStorageManager) has been found in database.")
Timber.d("Upload info: $it")
}else {
} else {
Timber.w("Upload with id ($uploadIdInStorageManager) has not been found in database.")
Timber.w("$uploadPath won't be uploaded")
}
Expand Down Expand Up @@ -302,7 +306,7 @@ class UploadFileFromContentUriWorker(
spaceWebDavUrl = spaceWebDavUrl,
)
true
}catch (throwable: Throwable) {
} catch (throwable: Throwable) {
Timber.w(throwable, "TUS upload failed, falling back to single PUT")
if (shouldRetry(throwable)) {
throw throwable
Expand All @@ -315,7 +319,7 @@ class UploadFileFromContentUriWorker(
Timber.d("TUS upload completed for %s", uploadPath)
return
}
}else {
} else {
Timber.d(
"Skipping TUS: file too small or unsupported (size=%d, threshold=%d, supportsTus=%s)",
fileSize,
Expand All @@ -324,12 +328,9 @@ class UploadFileFromContentUriWorker(
)
}

if (attemptedTus) {
clearTusState()
}

Timber.d("Falling back to single PUT upload for %s", uploadPath)
uploadPlainFile(client)
clearTusState()
removeCacheFile()
}

Expand All @@ -345,7 +346,10 @@ class UploadFileFromContentUriWorker(
addDataTransferProgressListener(this@UploadFileFromContentUriWorker)
}

executeRemoteOperation { uploadFileOperation.execute(client) }
val result = executeRemoteOperation { uploadFileOperation.execute(client) }
if (result == Unit) {
clearTusState()
}
}

private fun updateProgressFromTus(offset: Long, totalSize: Long) {
Expand All @@ -357,6 +361,7 @@ class UploadFileFromContentUriWorker(
val progress = workDataOf(DownloadFileWorker.WORKER_KEY_PROGRESS to percent)
setProgress(progress)
}
scheduleForegroundUpdate(percent)
lastPercent = percent
}

Expand Down Expand Up @@ -398,7 +403,7 @@ class UploadFileFromContentUriWorker(
private fun getUploadStatusForThrowable(throwable: Throwable?): TransferStatus =
if (throwable == null) {
TransferStatus.TRANSFER_SUCCEEDED
}else {
} else {
TransferStatus.TRANSFER_FAILED
}

Expand All @@ -411,7 +416,7 @@ class UploadFileFromContentUriWorker(

val pendingIntent = if (needsToUpdateCredentials) {
NotificationUtils.composePendingIntentToRefreshCredentials(appContext, account)
}else {
} else {
NotificationUtils.composePendingIntentToUploadList(appContext)
}

Expand Down Expand Up @@ -442,9 +447,78 @@ class UploadFileFromContentUriWorker(
setProgress(progress)
}

scheduleForegroundUpdate(percent)
lastPercent = percent
}

private suspend fun startForeground() {
if (foregroundInitialized) return
foregroundInitialized = true
currentForegroundProgress = Int.MIN_VALUE
try {
setForeground(createForegroundInfo(-1))
} catch (e: Exception) {
Timber.w(e, "Failed to set foreground for upload worker")
}
currentForegroundProgress = -1
}

private fun scheduleForegroundUpdate(progress: Int) {
if (!foregroundInitialized) return
if (progress == currentForegroundProgress) return
currentForegroundProgress = progress
foregroundScope.launch {
try {
setForeground(createForegroundInfo(progress))
} catch (e: Exception) {
Timber.w(e, "Failed to update foreground notification")
}
}
}

private fun createForegroundInfo(progress: Int): ForegroundInfo =
ForegroundInfo(
computeNotificationId(),
buildForegroundNotification(progress),
ServiceInfo.FOREGROUND_SERVICE_TYPE_DATA_SYNC
)

private fun buildForegroundNotification(progress: Int): Notification {
val fileName = File(uploadPath).name
val builder = NotificationUtils
.newNotificationBuilder(appContext, UPLOAD_NOTIFICATION_CHANNEL_ID)
.setContentTitle(appContext.getString(R.string.uploader_upload_in_progress_ticker))
.setContentIntent(NotificationUtils.composePendingIntentToUploadList(appContext))
.setOnlyAlertOnce(true)
.setOngoing(true)
.setSubText(fileName)
Copy link
Contributor

@guruz guruz Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how this behaves if there is multiple (big) uploads going on at same time. But fine for now, can solve this later if there is an issue.

Copy link
Contributor Author

@zerox80 zerox80 Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Do u mean that?

Copy link
Contributor

@guruz guruz Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe. Are those two differently named files? At least on server one of them should be (2) if it's two concurrent uploads?
How does it look in the uploads tab in the app?

Copy link
Contributor Author

@zerox80 zerox80 Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No they have same file name, wait. Let me reproduce it to u with real different file names.

Copy link
Contributor Author

@zerox80 zerox80 Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And i just uploaded the same file twice. We need to see how it behaves at end! If it does rename automatically to (2) !! :)

Copy link
Contributor Author

@zerox80 zerox80 Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really good catch from u. If it has the same file name, the second upload gets cancelled basically. Thats not a good result. Can we merge the PRs first, then we go for the fix in a seperate PR? It is really a chaotic situation otherwise. We will find a good solution, pretty sure :)

Copy link
Contributor Author

@zerox80 zerox80 Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can also do it in this PR if u dont mind...


if (progress in 0..100) {
builder.setContentText(
appContext.getString(
R.string.uploader_upload_in_progress_content,
progress,
fileName
)
)
builder.setProgress(100, progress, false)
} else {
builder.setContentText(appContext.getString(R.string.uploader_upload_in_progress_ticker))
builder.setProgress(0, 0, true)
}

return builder.build()
}

private fun computeNotificationId(): Int {
val id = uploadIdInStorageManager
return if (id in Int.MIN_VALUE.toLong()..Int.MAX_VALUE.toLong()) {
id.toInt()
} else {
id.hashCode()
}
}

companion object {
const val KEY_PARAM_ACCOUNT_NAME = "KEY_PARAM_ACCOUNT_NAME"
const val KEY_PARAM_BEHAVIOR = "KEY_PARAM_BEHAVIOR"
Expand Down
Loading