Skip to content

Commit a05e7d6

Browse files
zerox80guruz
authored andcommitted
fix: fetch ETag via PROPFIND after TUS chunked upload
TUS PATCH responses from openCloud do not include ETag headers. This caused large file uploads (>10MB) to have null ETags in the local database, breaking thumbnail preview URLs and caching. Changes: - TusUploadHelper: after TUS upload completes, perform a PROPFIND to fetch the file's ETag from the server - PatchTusUploadChunkRemoteOperation: extract ETag from PATCH response header (for future backend support) - UploadFileFromFileSystemWorker: persist ETag to database for both overwrite and non-overwrite uploads - ThumbnailsRequester: use .orEmpty() for null-safe ETag formatting
1 parent 277fd6d commit a05e7d6

4 files changed

Lines changed: 53 additions & 12 deletions

File tree

opencloudApp/src/main/java/eu/opencloud/android/presentation/thumbnails/ThumbnailsRequester.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ object ThumbnailsRequester : KoinComponent {
9797
val path = if (remotePath?.startsWith("/") == true) remotePath else "/$remotePath"
9898
val encodedPath = Uri.encode(path, "/")
9999

100-
return String.format(Locale.US, FILE_PREVIEW_URI, baseUrl, encodedPath, width, height, etag)
100+
return String.format(Locale.US, FILE_PREVIEW_URI, baseUrl, encodedPath, width, height, etag.orEmpty())
101101
}
102102

103103
fun getCoilImageLoader(): ImageLoader {

opencloudApp/src/main/java/eu/opencloud/android/workers/TusUploadHelper.kt

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class TusUploadHelper(
5050
progressListener: OnDatatransferProgressListener?,
5151
progressCallback: ((Long, Long) -> Unit)? = null,
5252
spaceWebDavUrl: String? = null,
53-
) {
53+
) : String? {
5454
// Reset cancelled state for new upload
5555
cancelled = false
5656
Timber.d("TUS: starting upload for %s size=%d", remotePath, fileSize)
@@ -135,7 +135,7 @@ class TusUploadHelper(
135135
Timber.d("TUS: resume offset %d / %d", offset, fileSize)
136136
progressCallback?.invoke(offset, fileSize)
137137

138-
offset = performUploadLoop(
138+
val loopResult = performUploadLoop(
139139
client = client,
140140
resolvedTusUrl = resolvedTusUrl,
141141
localPath = localPath,
@@ -146,6 +146,8 @@ class TusUploadHelper(
146146
initialOffset = offset,
147147
uploadId = uploadId,
148148
)
149+
offset = loopResult.first
150+
val finalEtag = loopResult.second
149151

150152
// Verify upload is actually complete
151153
if (offset != fileSize) {
@@ -162,7 +164,30 @@ class TusUploadHelper(
162164
tusUploadExpires = null,
163165
tusUploadConcat = null,
164166
)
165-
Timber.i("TUS: upload completed for %s (size=%d)", remotePath, fileSize)
167+
// If TUS PATCH didn't return an ETag (which is normal for openCloud's TUS implementation),
168+
// fetch it via a PROPFIND request on the newly uploaded file.
169+
var resultEtag = finalEtag
170+
if (resultEtag.isNullOrBlank()) {
171+
Timber.d("TUS: no etag from PATCH, attempting PROPFIND for %s (spaceWebDavUrl=%s)", remotePath, spaceWebDavUrl)
172+
try {
173+
val readOp = eu.opencloud.android.lib.resources.files.ReadRemoteFileOperation(
174+
remotePath = remotePath,
175+
spaceWebDavUrl = spaceWebDavUrl,
176+
)
177+
val readResult = readOp.execute(client)
178+
if (readResult.isSuccess && readResult.data != null) {
179+
resultEtag = readResult.data!!.etag
180+
Timber.d("TUS: fetched ETag via PROPFIND: %s", resultEtag)
181+
} else {
182+
Timber.w("TUS PROPFIND failed. code=%s, httpCode=%d", readResult.code, readResult.httpCode)
183+
}
184+
} catch (e: Throwable) {
185+
Timber.e(e, "TUS: exception during PROPFIND for ETag")
186+
}
187+
}
188+
189+
Timber.i("TUS: upload completed for %s (size=%d) resulting etag=%s", remotePath, fileSize, resultEtag)
190+
return resultEtag
166191
}
167192

168193
private fun performUploadLoop(
@@ -175,8 +200,9 @@ class TusUploadHelper(
175200
progressCallback: ((Long, Long) -> Unit)?,
176201
initialOffset: Long,
177202
uploadId: Long,
178-
): Long {
203+
): Pair<Long, String?> {
179204
var offset = initialOffset
205+
var lastEtag: String? = null
180206
val serverMaxChunk = tusSupport?.maxChunkSize?.takeIf { it > 0 }?.toLong()
181207
val httpOverride = tusSupport?.httpMethodOverride
182208
var consecutiveFailures = 0
@@ -203,6 +229,7 @@ class TusUploadHelper(
203229
activePatchOperation = patchOperation
204230

205231
val patchResult = patchOperation.execute(client)
232+
lastEtag = patchOperation.etag.takeIf { it.isNotBlank() }
206233
activePatchOperation = null
207234
if (!patchResult.isSuccess || patchResult.data == null || patchResult.data!! < offset) {
208235
consecutiveFailures++
@@ -281,7 +308,7 @@ class TusUploadHelper(
281308
progressCallback?.invoke(offset, fileSize)
282309
consecutiveFailures = 0
283310
}
284-
return offset
311+
return Pair(offset, lastEtag)
285312
}
286313

287314
private fun resolveTusCollectionUrl(

opencloudApp/src/main/java/eu/opencloud/android/workers/UploadFileFromFileSystemWorker.kt

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,8 @@ class UploadFileFromFileSystemWorker(
101101
private val transferRepository: TransferRepository by inject()
102102
private val tusUploadHelper by lazy { TusUploadHelper(transferRepository) }
103103

104+
private var finalEtag: String = ""
105+
104106
private val foregroundJob = SupervisorJob()
105107
private val foregroundScope = CoroutineScope(Dispatchers.Default + foregroundJob)
106108
@Volatile
@@ -262,17 +264,15 @@ class UploadFileFromFileSystemWorker(
262264
val hasPendingTusSession = !ocTransfer.tusUploadUrl.isNullOrBlank()
263265
val shouldTryTus = hasPendingTusSession || (supportsTus && fileSize >= TusUploadHelper.DEFAULT_CHUNK_SIZE)
264266

265-
var attemptedTus = false
266267
if (shouldTryTus) {
267-
attemptedTus = true
268268
Timber.d(
269269
"Attempting TUS upload (size=%d, threshold=%d, resume=%s)",
270270
fileSize,
271271
TusUploadHelper.DEFAULT_CHUNK_SIZE,
272272
hasPendingTusSession
273273
)
274274
val tusSucceeded = try {
275-
tusUploadHelper.upload(
275+
val returnedEtag = tusUploadHelper.upload(
276276
client = client,
277277
transfer = ocTransfer,
278278
uploadId = uploadIdInStorageManager,
@@ -286,6 +286,9 @@ class UploadFileFromFileSystemWorker(
286286
progressCallback = ::updateProgressFromTus,
287287
spaceWebDavUrl = spaceWebDavUrl,
288288
)
289+
if (!returnedEtag.isNullOrBlank()) {
290+
finalEtag = returnedEtag
291+
}
289292
true
290293
} catch (throwable: Throwable) {
291294
Timber.w(throwable, "TUS upload failed, falling back to single PUT")
@@ -299,7 +302,6 @@ class UploadFileFromFileSystemWorker(
299302
if (removeLocal) {
300303
removeLocalFile()
301304
}
302-
Timber.d("TUS upload completed for %s", uploadPath)
303305
return
304306
}
305307
} else {
@@ -330,6 +332,7 @@ class UploadFileFromFileSystemWorker(
330332
val result = executeRemoteOperation { uploadFileOperation.execute(client) }
331333

332334
if (result == Unit) {
335+
finalEtag = uploadFileOperation.etag
333336
clearTusState()
334337
if (removeLocal) {
335338
removeLocalFile() // Removed file from tmp folder
@@ -412,15 +415,20 @@ class UploadFileFromFileSystemWorker(
412415
if (ocTransfer.forceOverwrite) {
413416
ocFile.copy(
414417
needsToUpdateThumbnail = true,
415-
etag = uploadFileOperation.etag,
418+
etag = finalEtag,
416419
length = fileSize,
417420
lastSyncDateForData = currentTime,
418421
modifiedAtLastSyncForData = currentTime,
419422
)
420423
} else {
421-
// Uploading a file should remove any conflicts on the file.
424+
// Uploading a file should remove any conflicts and update etag.
422425
ocFile.copy(
423426
storagePath = null,
427+
needsToUpdateThumbnail = true,
428+
etag = finalEtag.ifBlank { ocFile.etag },
429+
length = fileSize,
430+
lastSyncDateForData = currentTime,
431+
modifiedAtLastSyncForData = currentTime,
424432
)
425433
}
426434
saveFileOrFolderUseCase(SaveFileOrFolderUseCase.Params(fileWithNewDetails))

opencloudComLibrary/src/main/java/eu/opencloud/android/lib/resources/files/tus/PatchTusUploadChunkRemoteOperation.kt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ class PatchTusUploadChunkRemoteOperation(
3838
private val dataTransferListeners: MutableSet<OnDatatransferProgressListener> = HashSet()
3939
private var activeMethod: HttpBaseMethod? = null
4040

41+
var etag: String = ""
42+
4143
@Suppress("ExpressionBodySyntax")
4244
override fun run(client: OpenCloudClient): RemoteOperationResult<Long> {
4345
// Fast-path: if caller requested cancellation before execution, honour it without hitting the network.
@@ -85,6 +87,10 @@ class PatchTusUploadChunkRemoteOperation(
8587
)
8688

8789
if (isSuccess(status)) {
90+
val rawEtag = eu.opencloud.android.lib.common.network.WebdavUtils.getEtagFromResponse(method)
91+
if (rawEtag != null) {
92+
this@PatchTusUploadChunkRemoteOperation.etag = rawEtag.replace("\"", "")
93+
}
8894
val newOffset = method.getResponseHeader(HttpConstants.UPLOAD_OFFSET)?.toLongOrNull()
8995
if (newOffset != null) {
9096
RemoteOperationResult<Long>(ResultCode.OK).apply { data = newOffset }

0 commit comments

Comments
 (0)