Upload workers – foreground mode & retry polish

[zerox80]

🚀 Upload Improvements: Foreground Service & Smart Retry

Summary

This PR transforms our upload architecture from simple background workers into robust, resilient Foreground Services. It ensures uploads survive app backgrounding, provides real-time user feedback, and intelligently recovers from network failures.

Important

Dependency Note: This branch is built on top of PR #36 (TUS Implementation).
You will see TUS-related commits in the history, but they are NOT part of this review.
This PR requires the TUS implementation as a foundation. Once PR #36 is merged/rebased, this branch will be updated.
Please review ONLY the 4 commits listed below.

---

🎯 Review Scope

Please focus your review strictly on these 4 commits:

Commit	Type	Description
6349327	feat	🔍 Foreground Service: Keeps uploads alive in background & shows notification
80e334e	feat	📊 Progress Updates: Ensures notification updates during upload
44dd5c9	refactor	⚡ Performance: Caches file size to avoid redundant I/O
87bf1e7	feat	🔄 Retry Policy: Adds exponential backoff for network failures

---

🛠️ Technical Deep Dive

1. Foreground Service Support (6349327)

The Challenge:
Standard CoroutineWorkers are deprioritized by Android's battery optimizer (Doze Mode) when the screen is off. This leads to "silent failures" where uploads just stop halfway.

The Solution:
We promote the worker to a Foreground Service. This tells the OS: "This is a user-initiated, long-running task. Do not kill it."

Implementation Details:

• Manifest: Added android:foregroundServiceType="dataSync" (Required for Android 14+ compliance).
• Worker: Calls setForeground() immediately upon start.

// UploadFileFromContentUriWorker.kt
private suspend fun startForeground() {
    // Create persistent notification
    val notification = NotificationUtils.createNotification(...)
    
    // Promote to Foreground Service (Android 14 compatible)
    setForeground(ForegroundInfo(
        notificationId, 
        notification, 
        ServiceInfo.FOREGROUND_SERVICE_TYPE_DATA_SYNC // <--- Critical for Android 14
    ))
}

2. Real-Time Progress Updates (80e334e)

The Challenge:
TUS uploads chunk data, but the notification wasn't updating between chunks. Users saw "Uploading..." but the bar was stuck, leading to "Is it frozen?" anxiety.

The Solution:
We implemented a dedicated scheduleForegroundUpdate method that forces a notification refresh whenever a chunk completes.

// UploadFileFromFileSystemWorker.kt
private fun updateProgressFromTus(offset: Long, totalSize: Long) {
    val percent = ((offset * 100) / totalSize).toInt()
    
    // 1. Update WorkManager progress (for app UI)
    setProgress(workDataOf("PROGRESS" to percent))
    
    // 2. FORCE update the system notification (for status bar)
    scheduleForegroundUpdate(percent) 
}

3. I/O Optimization (44dd5c9)

The Challenge:
The worker was calling file.length() inside the progress loop. On some file systems, this is a blocking I/O operation. Doing this 100 times per second during a fast upload is wasteful.

The Solution:
Read once, cache forever.

// Before ❌
onProgress { total -> 
    val size = file.length() // Disk read every time!
    calculatePercent(total, size)
}

// After ✅
val cachedSize = file.length() // Read once at start
onProgress { total ->
    calculatePercent(total, cachedSize) // Memory access (fast)
}

4. Exponential Backoff Retry Policy (87bf1e7)

The Challenge:
Mobile networks are flaky. A 1-second tunnel drop shouldn't fail a 2GB upload. However, retrying immediately is bad because it hammers the server if it's down.

The Solution:
We implemented Exponential Backoff. If an upload fails due to network issues, we wait longer and longer between retries.

Configuration:

// UploadFileFromContentUriUseCase.kt
val request = OneTimeWorkRequestBuilder<UploadFileFromContentUriWorker>()
    .setBackoffCriteria(
        BackoffPolicy.EXPONENTIAL, // Wait time doubles: 10s, 20s, 40s...
        10, TimeUnit.SECONDS
    )
    .build()

Smart Exception Handling:
We now distinguish between "Retryable" and "Fatal" errors:

• IOException (Network) ➡️ Retry (Trigger backoff)
• FileNotFoundException (User deleted file) ➡️ Fail (Don't retry)

// TusUploadHelper.kt
if (consecutiveFailures >= MAX_RETRIES) {
    // Throw IOException to trigger WorkManager retry
    throw IOException("Network error, requesting retry...") 
}

---

🧪 Verification

• Background Test: Started upload, switched to YouTube, locked screen. Result: Upload finished successfully.
• Airplane Mode Test: Toggled airplane mode mid-upload. Result: Worker paused, waited 10s, and resumed automatically.
• Android 14: Verified no ForegroundServiceStartNotAllowedException crashes.

---

Ready for review! 🚀

[zerox80]

@guruz Can u start with this PR as next, thanks. Take your time.

[zerox80]

So basically: I fixed the bug with the useless round-trip and there is less overhead now, as guruz mentioned in the now-closed PR (since it was merged).

@kulmann @guruz Hey. I hope you're both doing fine. So my problem is: the detekt and test run failed when I opened a separate PR for those commits. However, the checks passed here (detekt and test run). As far as I know, both the upload-improvement and upload cancellation PRs need those commit changes. Could we keep them in those 2 PRs? I closed the separate PR because it failed and I was really frustrated at the end after wasting hours managing it. I also accidentally almost deleted commits from this current PR branch because I was overwhelmed (luckily I had a backup before the rebase etc.). Could we just make an exception? I told @guruz that this PR should be the next review so I could rebase the cancellation PR to the upstream main after this PR is merged. I am sorry for this complication.

[guruz]

hey :) Thanks, I will take a look at this one.
For future: If you think you lost a commit, you can always try to use the command git reflogand then fish out stuff from there. Then use git cherry-pick to pick stuff to a (new) branch.

[guruz]

Code looks OK i guess :) i will test it and then merge

[guruz]

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.

[zerox80]

Do u mean that?

[guruz]

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?

[zerox80]

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

[zerox80]

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

[zerox80]

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 :)

[zerox80]

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

[zerox80]

TUS Creation-with-Upload Test Walkthrough

Goal

Verify that the CreateTusUploadRemoteOperation correctly parses and returns the Upload-Offset header when using the creation-with-upload extension (sending data in the initial POST request).

Changes

[TusIntegrationTest.kt]

Added a new test method creation_with_upload_returns_offset that:

1. Configures CreateTusUploadRemoteOperation with useCreationWithUpload = true and a firstChunkSize.
2. Mocks a server response containing the Upload-Offset header.
3. Asserts that the returned CreationResult contains the correct offset matching the uploaded chunk size.

    @Test
    fun creation_with_upload_returns_offset() {
        val client = newClient()
        val collectionPath = "/remote.php/dav/uploads/$userId"
        val locationPath = "$collectionPath/UPLD-WITH-DATA"
        val localFile = File.createTempFile("tus", ".bin").apply {
            writeBytes(ByteArray(100) { it.toByte() })
        }
        val firstChunkSize = 50L

        // POST Create with Upload -> 201 + Location + Upload-Offset
        server.enqueue(
            MockResponse()
                .setResponseCode(201)
                .addHeader("Tus-Resumable", "1.0.0")
                .addHeader("Location", locationPath)
                .addHeader("Upload-Offset", firstChunkSize.toString())
        )

        val create = CreateTusUploadRemoteOperation(
            file = localFile,
            remotePath = "/test-with-data.bin",
            mimetype = "application/octet-stream",
            metadata = mapOf("filename" to "test-with-data.bin"),
            useCreationWithUpload = true,
            firstChunkSize = firstChunkSize,
            tusUrl = null,
            collectionUrlOverride = server.url(collectionPath).toString(),
            base64Encoder = object : Base64Encoder {
                override fun encode(bytes: ByteArray): String =
                    Base64.getEncoder().encodeToString(bytes)
            }
        )

        val createResult = create.execute(client)
        assertTrue("Create operation failed", createResult.isSuccess)
        
        val creationResult = createResult.data
        assertNotNull(creationResult)
        assertEquals(firstChunkSize, creationResult!!.uploadOffset)
        assertTrue(creationResult.uploadUrl.endsWith(locationPath))

        // Verify POST request
        val postReq = server.takeRequest()
        assertEquals("POST", postReq.method)
        assertEquals("Bearer $token", postReq.getHeader("Authorization"))
        assertEquals("1.0.0", postReq.getHeader("Tus-Resumable"))
        // creation-with-upload sends Content-Type and Content-Length for the chunk
        assertEquals("application/offset+octet-stream", postReq.getHeader("Content-Type"))
        assertEquals(firstChunkSize.toString(), postReq.getHeader("Content-Length"))
    }

Verification Results

Automated Tests

Ran the unit tests for opencloudComLibrary:
./gradlew :opencloudComLibrary:testDebugUnitTest --tests "eu.opencloud.android.lib.resources.files.tus.TusIntegrationTest"

Result: BUILD SUCCESSFUL

The new test case passed, confirming that the offset is correctly extracted from the creation response.

[guruz]

By the way, I don't know what your workflow is, but it's much less confusing to me if you put a commit only in one PR :)
I accidently commented on your test on the wrong PR.
(Because we said we focus on this PR here only now)
My comment is at #66 (review)

[zerox80]

By the way, I don't know what your workflow is, but it's much less confusing to me if you put a commit only in one PR :) I accidently commented on your test on the wrong PR. (Because we said we focus on this PR here only now) My comment is at #66 (review)

ok lets continue here then
and im afraid of merge conflicts ...

[zerox80]

@guruz is this better now?

I Squashed, better structure now

[zerox80]

By the way, I don't know what your workflow is, but it's much less confusing to me if you put a commit only in one PR :) I accidently commented on your test on the wrong PR. (Because we said we focus on this PR here only now) My comment is at #66 (review)

ok im just stupid git has "auto merge" which git can solve himself, i won't confuse more, just tested out now...

[guruz]

If it has the same file name, the second upload gets cancelled basically

thanks for checking it out.

Please check #36 (comment) first before continuing on this one. :)

[guruz]

This might trigger #71
Just referencing, not 100% sure