Skip to content

Commit 82ab5ab

Browse files
zerox80zerox80
committed
Fix sync etag handling for offline downloads
1 parent 2e87c6c commit 82ab5ab

5 files changed

Lines changed: 137 additions & 53 deletions

File tree

‎opencloudApp/src/main/java/eu/opencloud/android/usecases/synchronization/SynchronizeFileUseCase.kt‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ class SynchronizeFileUseCase(
7777

7878
if (changedLocally) {
7979
Timber.w("File deleted remotely but changed locally. Uploading local version instead of deleting.")
80-
val uuid = requestForUpload(accountName, fileToSynchronize)
80+
val uuid = requestForUpload(accountName, fileToSynchronize, currentRemoteEtag = "")
8181
return SyncType.UploadEnqueued(uuid)
8282
} else {
8383
val localDbFile = fileToSynchronize.id?.let { fileRepository.getFileById(it) }
@@ -114,7 +114,7 @@ class SynchronizeFileUseCase(
114114
Timber.i("So it has changed remotely: $changedRemotely")
115115

116116
if (changedLocally && changedRemotely) {
117-
return handleConflict(fileToSynchronize, accountName)
117+
return handleConflict(fileToSynchronize, accountName, serverFile.etag)
118118
} else if (changedRemotely) {
119119
// 5.2 File has changed ONLY remotely -> download new version
120120
Timber.i("File ${fileToSynchronize.fileName} has changed remotely. Let's download the new version")
@@ -123,7 +123,7 @@ class SynchronizeFileUseCase(
123123
} else if (changedLocally) {
124124
// 5.3 File has change ONLY locally -> upload new version
125125
Timber.i("File ${fileToSynchronize.fileName} has changed locally. Let's upload the new version")
126-
val uuid = requestForUpload(accountName, fileToSynchronize)
126+
val uuid = requestForUpload(accountName, fileToSynchronize, serverFile.etag)
127127
return SyncType.UploadEnqueued(uuid)
128128
} else {
129129
// 5.4 File has not change locally not remotely -> do nothing
@@ -137,14 +137,14 @@ class SynchronizeFileUseCase(
137137
}
138138
}
139139

140-
private fun handleConflict(fileToSynchronize: OCFile, accountName: String): SyncType {
140+
private fun handleConflict(fileToSynchronize: OCFile, accountName: String, currentRemoteEtag: String?): SyncType {
141141
val preferLocal = preferencesProvider.getBoolean(
142142
SettingsSecurityFragment.PREFERENCE_PREFER_LOCAL_ON_CONFLICT, false
143143
)
144144

145145
if (preferLocal) {
146146
Timber.i("File ${fileToSynchronize.fileName} has conflict. User prefers local version, uploading.")
147-
val uuid = requestForUpload(accountName, fileToSynchronize)
147+
val uuid = requestForUpload(accountName, fileToSynchronize, currentRemoteEtag)
148148
return SyncType.UploadEnqueued(uuid)
149149
}
150150

@@ -188,7 +188,7 @@ class SynchronizeFileUseCase(
188188
)
189189
)
190190

191-
private fun requestForUpload(accountName: String, ocFile: OCFile): UUID? {
191+
private fun requestForUpload(accountName: String, ocFile: OCFile, currentRemoteEtag: String?): UUID? {
192192
val localPath = ocFile.storagePath
193193
if (localPath.isNullOrEmpty()) {
194194
Timber.e("Cannot upload file ${ocFile.fileName} because storagePath is null or empty.")
@@ -200,6 +200,7 @@ class SynchronizeFileUseCase(
200200
localPath = localPath,
201201
uploadFolderPath = ocFile.getParentRemotePath(),
202202
spaceId = ocFile.spaceId,
203+
currentRemoteEtag = currentRemoteEtag,
203204
)
204205
)
205206
}

‎opencloudApp/src/main/java/eu/opencloud/android/usecases/transfers/uploads/UploadFileInConflictUseCase.kt‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,11 @@
2323
package eu.opencloud.android.usecases.transfers.uploads
2424

2525
import androidx.work.Constraints
26+
import androidx.work.Data
2627
import androidx.work.ExistingWorkPolicy
2728
import androidx.work.NetworkType
2829
import androidx.work.OneTimeWorkRequestBuilder
2930
import androidx.work.WorkManager
30-
import androidx.work.workDataOf
3131
import eu.opencloud.android.domain.BaseUseCase
3232
import eu.opencloud.android.domain.automaticuploads.model.UploadBehavior
3333
import eu.opencloud.android.domain.transfers.TransferRepository
@@ -73,6 +73,7 @@ class UploadFileInConflictUseCase(
7373
lastModifiedInSeconds = localFile.lastModified().div(1_000).toString(),
7474
accountName = params.accountName,
7575
uploadIdInStorageManager = uploadId,
76+
currentRemoteEtag = params.currentRemoteEtag,
7677
)
7778
}
7879

@@ -105,16 +106,21 @@ class UploadFileInConflictUseCase(
105106
lastModifiedInSeconds: String,
106107
uploadIdInStorageManager: Long,
107108
uploadPath: String,
109+
currentRemoteEtag: String?,
108110
): UUID {
109-
val inputData = workDataOf(
110-
UploadFileFromFileSystemWorker.KEY_PARAM_ACCOUNT_NAME to accountName,
111-
UploadFileFromFileSystemWorker.KEY_PARAM_BEHAVIOR to UploadBehavior.COPY.name,
112-
UploadFileFromFileSystemWorker.KEY_PARAM_LOCAL_PATH to localPath,
113-
UploadFileFromFileSystemWorker.KEY_PARAM_LAST_MODIFIED to lastModifiedInSeconds,
114-
UploadFileFromFileSystemWorker.KEY_PARAM_UPLOAD_PATH to uploadPath,
115-
UploadFileFromFileSystemWorker.KEY_PARAM_UPLOAD_ID to uploadIdInStorageManager,
116-
UploadFileFromFileSystemWorker.KEY_PARAM_REMOVE_LOCAL to false
117-
)
111+
val inputDataBuilder = Data.Builder()
112+
.putString(UploadFileFromFileSystemWorker.KEY_PARAM_ACCOUNT_NAME, accountName)
113+
.putString(UploadFileFromFileSystemWorker.KEY_PARAM_BEHAVIOR, UploadBehavior.COPY.name)
114+
.putString(UploadFileFromFileSystemWorker.KEY_PARAM_LOCAL_PATH, localPath)
115+
.putString(UploadFileFromFileSystemWorker.KEY_PARAM_LAST_MODIFIED, lastModifiedInSeconds)
116+
.putString(UploadFileFromFileSystemWorker.KEY_PARAM_UPLOAD_PATH, uploadPath)
117+
.putLong(UploadFileFromFileSystemWorker.KEY_PARAM_UPLOAD_ID, uploadIdInStorageManager)
118+
.putBoolean(UploadFileFromFileSystemWorker.KEY_PARAM_REMOVE_LOCAL, false)
119+
120+
currentRemoteEtag?.let {
121+
inputDataBuilder.putString(UploadFileFromFileSystemWorker.KEY_PARAM_OVERWRITE_ETAG, it)
122+
}
123+
val inputData = inputDataBuilder.build()
118124

119125
val constraints = Constraints.Builder()
120126
.setRequiredNetworkType(NetworkType.CONNECTED)
@@ -144,5 +150,6 @@ class UploadFileInConflictUseCase(
144150
val localPath: String,
145151
val uploadFolderPath: String,
146152
val spaceId: String?,
153+
val currentRemoteEtag: String? = null,
147154
)
148155
}

‎opencloudApp/src/main/java/eu/opencloud/android/workers/DownloadEverythingWorker.kt‎

Lines changed: 96 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import eu.opencloud.android.MainApp
3535
import eu.opencloud.android.R
3636
import eu.opencloud.android.data.download.DownloadProgress
3737
import eu.opencloud.android.data.download.DownloadProgressDataStore
38+
import eu.opencloud.android.domain.UseCaseResult
3839
import eu.opencloud.android.domain.capabilities.usecases.GetStoredCapabilitiesUseCase
3940
import eu.opencloud.android.domain.files.FileRepository
4041
import eu.opencloud.android.domain.files.model.OCFile
@@ -43,6 +44,7 @@ import eu.opencloud.android.domain.files.usecases.GetFileByRemotePathUseCase
4344
import eu.opencloud.android.domain.spaces.usecases.GetPersonalAndProjectSpacesForAccountUseCase
4445
import eu.opencloud.android.domain.spaces.usecases.RefreshSpacesFromServerAsyncUseCase
4546
import eu.opencloud.android.presentation.authentication.AccountUtils
47+
import eu.opencloud.android.usecases.synchronization.SynchronizeFileUseCase
4648
import eu.opencloud.android.usecases.transfers.downloads.DownloadFileUseCase
4749
import eu.opencloud.android.utils.FileStorageUtils
4850
import org.koin.core.component.KoinComponent
@@ -82,6 +84,7 @@ class DownloadEverythingWorker(
8284
private val getFileByRemotePathUseCase: GetFileByRemotePathUseCase by inject()
8385
private val fileRepository: FileRepository by inject()
8486
private val downloadFileUseCase: DownloadFileUseCase by inject()
87+
private val synchronizeFileUseCase: SynchronizeFileUseCase by inject()
8588
private val progressDataStore: DownloadProgressDataStore by inject()
8689

8790
private var totalFilesFound = 0
@@ -425,34 +428,14 @@ class DownloadEverythingWorker(
425428

426429
try {
427430
if (file.isAvailableLocally) {
428-
// File is already downloaded
429-
filesAlreadyLocal++
430-
Timber.d("File already local: ${file.fileName}")
431-
} else {
432-
// Skip files larger than currently available space to avoid guaranteed failures
433-
val usableSpace = FileStorageUtils.getUsableSpace() - bytesEnqueuedInThisRun
434-
if (file.length > 0 && file.length > usableSpace) {
435-
filesSkipped++
436-
Timber.w("Skipping ${file.fileName} (${file.length} bytes) — not enough free space")
431+
if (file.isDownloadedRemoteVersionCurrent()) {
432+
filesAlreadyLocal++
433+
Timber.d("File already local and current: ${file.fileName}")
437434
} else {
438-
// Enqueue download
439-
val downloadId = downloadFileUseCase(
440-
DownloadFileUseCase.Params(accountName, file)
441-
)
442-
if (downloadId != null) {
443-
filesDownloaded++
444-
filesEnqueuedInThisRun++
445-
bytesEnqueuedInThisRun += maxOf(0L, file.length)
446-
Timber.i("Enqueued download for: ${file.fileName}")
447-
if (filesEnqueuedInThisRun >= 500) {
448-
Timber.i("Reached max enqueued files for this run (500). Requesting retry.")
449-
wasInterrupted = true
450-
}
451-
} else {
452-
filesSkipped++
453-
Timber.d("Download already enqueued or skipped: ${file.fileName}")
454-
}
435+
synchronizeStaleLocalFile(file)
455436
}
437+
} else {
438+
enqueueDownloadIfPossible(accountName, file)
456439
}
457440

458441
// Update notification periodically (every 20 files)
@@ -465,6 +448,92 @@ class DownloadEverythingWorker(
465448
}
466449
}
467450

451+
private fun OCFile.isDownloadedRemoteVersionCurrent(): Boolean {
452+
val currentRemoteEtag = remoteEtag
453+
return currentRemoteEtag.isNullOrBlank() || etag == currentRemoteEtag
454+
}
455+
456+
private fun synchronizeStaleLocalFile(file: OCFile) {
457+
Timber.i(
458+
"File ${file.fileName} is local but stale. Synced etag=${file.etag}, remote etag=${file.remoteEtag}"
459+
)
460+
461+
when (val useCaseResult = synchronizeFileUseCase(SynchronizeFileUseCase.Params(file))) {
462+
is UseCaseResult.Success -> handleStaleLocalSyncResult(file, useCaseResult.data)
463+
is UseCaseResult.Error -> {
464+
filesSkipped++
465+
Timber.e(useCaseResult.throwable, "Error synchronizing stale local file ${file.fileName}")
466+
}
467+
}
468+
}
469+
470+
private fun handleStaleLocalSyncResult(file: OCFile, syncType: SynchronizeFileUseCase.SyncType) {
471+
when (syncType) {
472+
is SynchronizeFileUseCase.SyncType.DownloadEnqueued -> {
473+
if (syncType.workerId != null) {
474+
filesDownloaded++
475+
markDownloadEnqueued(file)
476+
Timber.i("Enqueued download for stale local file: ${file.fileName}")
477+
} else {
478+
filesSkipped++
479+
Timber.d("Download already enqueued or skipped for stale local file: ${file.fileName}")
480+
}
481+
}
482+
is SynchronizeFileUseCase.SyncType.ConflictResolvedWithCopy -> {
483+
if (syncType.workerId != null) {
484+
filesDownloaded++
485+
markDownloadEnqueued(file)
486+
Timber.i("Resolved conflict and enqueued download for stale local file: ${file.fileName}")
487+
} else {
488+
filesSkipped++
489+
Timber.d("Conflict was handled but download was not enqueued for stale local file: ${file.fileName}")
490+
}
491+
}
492+
is SynchronizeFileUseCase.SyncType.UploadEnqueued -> {
493+
filesAlreadyLocal++
494+
Timber.i("Local changes for ${file.fileName} were queued for upload")
495+
}
496+
SynchronizeFileUseCase.SyncType.AlreadySynchronized -> {
497+
filesAlreadyLocal++
498+
Timber.d("File already synchronized after stale check: ${file.fileName}")
499+
}
500+
SynchronizeFileUseCase.SyncType.FileNotFound -> {
501+
filesSkipped++
502+
Timber.w("File no longer exists on server: ${file.fileName}")
503+
}
504+
}
505+
}
506+
507+
private fun enqueueDownloadIfPossible(accountName: String, file: OCFile) {
508+
val usableSpace = FileStorageUtils.getUsableSpace() - bytesEnqueuedInThisRun
509+
if (file.length > 0 && file.length > usableSpace) {
510+
filesSkipped++
511+
Timber.w("Skipping ${file.fileName} (${file.length} bytes) — not enough free space")
512+
return
513+
}
514+
515+
val downloadId = downloadFileUseCase(
516+
DownloadFileUseCase.Params(accountName, file)
517+
)
518+
if (downloadId != null) {
519+
filesDownloaded++
520+
markDownloadEnqueued(file)
521+
Timber.i("Enqueued download for: ${file.fileName}")
522+
} else {
523+
filesSkipped++
524+
Timber.d("Download already enqueued or skipped: ${file.fileName}")
525+
}
526+
}
527+
528+
private fun markDownloadEnqueued(file: OCFile) {
529+
filesEnqueuedInThisRun++
530+
bytesEnqueuedInThisRun += maxOf(0L, file.length)
531+
if (filesEnqueuedInThisRun >= MAX_ENQUEUED_FILES_PER_RUN) {
532+
Timber.i("Reached max enqueued files for this run ($MAX_ENQUEUED_FILES_PER_RUN). Requesting retry.")
533+
wasInterrupted = true
534+
}
535+
}
536+
468537
private fun createNotificationChannel() {
469538
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
470539
val channel = NotificationChannel(
@@ -513,5 +582,6 @@ class DownloadEverythingWorker(
513582
private const val NOTIFICATION_ID = 9001
514583
private const val MB = 1024L * 1024L
515584
private const val MIN_FREE_SPACE_BYTES = 100L * MB
585+
private const val MAX_ENQUEUED_FILES_PER_RUN = 500
516586
}
517587
}

‎opencloudApp/src/main/java/eu/opencloud/android/workers/DownloadFileWorker.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,7 @@ class DownloadFileWorker(
260260
needsToUpdateThumbnail = true
261261
modificationTimestamp = downloadRemoteFileOperation.modificationTimestamp
262262
etag = downloadRemoteFileOperation.etag
263+
remoteEtag = downloadRemoteFileOperation.etag
263264
storagePath = finalLocationForFile
264265
length = finalFile.length()
265266
// Use the file's actual mtime, not the current time. SynchronizeFileUseCase

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

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ class UploadFileFromFileSystemWorker(
9595

9696
// Etag in conflict required to overwrite files in server. Otherwise, the upload will be rejected.
9797
private var overwriteEtag: String = ""
98+
private var overwriteEtagFromInput: String? = null
9899

99100
private var lastPercent = -1
100101

@@ -151,6 +152,7 @@ class UploadFileFromFileSystemWorker(
151152
val paramFileSystemUri = workerParameters.inputData.getString(KEY_PARAM_LOCAL_PATH)
152153
val paramUploadId = workerParameters.inputData.getLong(KEY_PARAM_UPLOAD_ID, -1)
153154
val paramRemoveLocal = workerParameters.inputData.getBoolean(KEY_PARAM_REMOVE_LOCAL, false)
155+
overwriteEtagFromInput = workerParameters.inputData.getString(KEY_PARAM_OVERWRITE_ETAG)
154156

155157
account = AccountUtils.getOpenCloudAccountByName(appContext, paramAccountName) ?: return false
156158
fileSystemPath = paramFileSystemUri.takeUnless { it.isNullOrBlank() } ?: return false
@@ -223,17 +225,17 @@ class UploadFileFromFileSystemWorker(
223225
private fun checkNameCollisionAndGetAnAvailableOneInCase(client: OpenCloudClient) {
224226
if (ocTransfer.forceOverwrite) {
225227

226-
val getFileByRemotePathUseCase: GetFileByRemotePathUseCase by inject()
227-
val useCaseResult = getFileByRemotePathUseCase(
228-
GetFileByRemotePathUseCase.Params(
229-
ocTransfer.accountName,
230-
ocTransfer.remotePath,
231-
ocTransfer.spaceId
228+
overwriteEtag = overwriteEtagFromInput ?: run {
229+
val getFileByRemotePathUseCase: GetFileByRemotePathUseCase by inject()
230+
val useCaseResult = getFileByRemotePathUseCase(
231+
GetFileByRemotePathUseCase.Params(
232+
ocTransfer.accountName,
233+
ocTransfer.remotePath,
234+
ocTransfer.spaceId
235+
)
232236
)
233-
)
234-
235-
val remoteFile = useCaseResult.getDataOrNull()
236-
overwriteEtag = remoteFile?.etag.orEmpty()
237+
useCaseResult.getDataOrNull()?.etag.orEmpty()
238+
}
237239

238240
Timber.d("Upload will overwrite current server file with required etag: $overwriteEtag")
239241
} else {
@@ -416,7 +418,8 @@ class UploadFileFromFileSystemWorker(
416418
if (ocTransfer.forceOverwrite) {
417419
ocFile.copy(
418420
needsToUpdateThumbnail = true,
419-
etag = finalEtag,
421+
etag = finalEtag.ifBlank { ocFile.etag },
422+
remoteEtag = finalEtag.ifBlank { ocFile.remoteEtag },
420423
length = fileSize,
421424
lastSyncDateForData = currentTime,
422425
modifiedAtLastSyncForData = currentTime,
@@ -427,6 +430,7 @@ class UploadFileFromFileSystemWorker(
427430
storagePath = null,
428431
needsToUpdateThumbnail = true,
429432
etag = finalEtag.ifBlank { ocFile.etag },
433+
remoteEtag = finalEtag.ifBlank { ocFile.remoteEtag },
430434
length = fileSize,
431435
lastSyncDateForData = currentTime,
432436
modifiedAtLastSyncForData = currentTime,
@@ -570,5 +574,6 @@ class UploadFileFromFileSystemWorker(
570574
const val KEY_PARAM_UPLOAD_PATH: String = "KEY_PARAM_UPLOAD_PATH"
571575
const val KEY_PARAM_UPLOAD_ID: String = "KEY_PARAM_UPLOAD_ID"
572576
const val KEY_PARAM_REMOVE_LOCAL: String = "KEY_REMOVE_LOCAL"
577+
const val KEY_PARAM_OVERWRITE_ETAG: String = "KEY_PARAM_OVERWRITE_ETAG"
573578
}
574579
}

0 commit comments

Comments
 (0)