Skip to content

Commit f0df07b

Browse files
zerox80zerox80
committed
Fix Detekt issues in download-sync feature
1 parent 7918e5b commit f0df07b

8 files changed

Lines changed: 78 additions & 79 deletions

File tree

‎opencloudApp/src/main/java/eu/opencloud/android/presentation/documentsprovider/DocumentsStorageProvider.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ import eu.opencloud.android.usecases.synchronization.SynchronizeFolderUseCase
6666
import eu.opencloud.android.usecases.transfers.downloads.DownloadFileUseCase
6767
import eu.opencloud.android.usecases.transfers.uploads.UploadFilesFromSystemUseCase
6868
import eu.opencloud.android.utils.FileStorageUtils
69-
import eu.opencloud.android.utils.NotificationUtils
69+
7070
import kotlinx.coroutines.CoroutineScope
7171
import kotlinx.coroutines.Dispatchers
7272
import kotlinx.coroutines.launch

‎opencloudApp/src/main/java/eu/opencloud/android/presentation/files/details/FileDetailsFragment.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ import eu.opencloud.android.presentation.authentication.EXTRA_ACCOUNT
6161
import eu.opencloud.android.presentation.authentication.EXTRA_ACTION
6262
import eu.opencloud.android.presentation.authentication.LoginActivity
6363
import eu.opencloud.android.presentation.common.UIResult
64-
import eu.opencloud.android.presentation.conflicts.ConflictsResolveActivity
64+
6565
import eu.opencloud.android.presentation.files.details.FileDetailsViewModel.ActionsInDetailsView.NONE
6666
import eu.opencloud.android.presentation.files.details.FileDetailsViewModel.ActionsInDetailsView.SYNC
6767
import eu.opencloud.android.presentation.files.details.FileDetailsViewModel.ActionsInDetailsView.SYNC_AND_OPEN

‎opencloudApp/src/main/java/eu/opencloud/android/ui/activity/FileDisplayActivity.kt‎

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ import androidx.core.content.ContextCompat
5555
import androidx.core.view.isVisible
5656
import androidx.fragment.app.Fragment
5757
import androidx.localbroadcastmanager.content.LocalBroadcastManager
58-
import androidx.work.WorkManager
5958
import eu.opencloud.android.AppRater
6059
import eu.opencloud.android.BuildConfig
6160
import eu.opencloud.android.MainApp
@@ -95,7 +94,6 @@ import eu.opencloud.android.presentation.accounts.ManageAccountsViewModel
9594
import eu.opencloud.android.presentation.authentication.AccountUtils.getCurrentOpenCloudAccount
9695
import eu.opencloud.android.presentation.capabilities.CapabilityViewModel
9796
import eu.opencloud.android.presentation.common.UIResult
98-
import eu.opencloud.android.presentation.conflicts.ConflictsResolveActivity
9997
import eu.opencloud.android.presentation.files.details.FileDetailsFragment
10098
import eu.opencloud.android.presentation.files.filelist.MainEmptyListFragment
10199
import eu.opencloud.android.presentation.files.filelist.MainFileListFragment
@@ -113,13 +111,10 @@ import eu.opencloud.android.presentation.transfers.TransfersViewModel
113111
import eu.opencloud.android.presentation.settings.security.SettingsSecurityFragment
114112
import eu.opencloud.android.providers.WorkManagerProvider
115113
import eu.opencloud.android.syncadapter.FileSyncAdapter
116-
import eu.opencloud.android.ui.dialog.FileAlreadyExistsDialog
117114
import eu.opencloud.android.ui.fragment.FileFragment
118115
import eu.opencloud.android.ui.fragment.TaskRetainerFragment
119116
import eu.opencloud.android.ui.helpers.FilesUploadHelper
120117
import eu.opencloud.android.ui.preview.PreviewAudioFragment
121-
import eu.opencloud.android.ui.preview.PreviewImageActivity
122-
import eu.opencloud.android.ui.preview.PreviewImageFragment
123118
import eu.opencloud.android.ui.preview.PreviewTextFragment
124119
import eu.opencloud.android.ui.preview.PreviewVideoActivity
125120
import eu.opencloud.android.usecases.synchronization.SynchronizeFileUseCase
@@ -283,27 +278,24 @@ class FileDisplayActivity : FileActivity(),
283278
AppRater.appLaunched(this, packageName)
284279
}
285280

286-
287281
checkNotificationPermission()
288282
checkManageExternalStoragePermission()
289283
Timber.v("onCreate() end")
290284
}
291285

292286
private fun checkManageExternalStoragePermission() {
293-
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) {
294-
if (!android.os.Environment.isExternalStorageManager()) {
295-
val builder = AlertDialog.Builder(this)
296-
builder.setTitle(getString(R.string.app_name))
297-
builder.setMessage("To save offline files, the app needs access to all files.")
298-
builder.setPositiveButton("Settings") { _, _ ->
299-
val intent = Intent(android.provider.Settings.ACTION_MANAGE_APP_ALL_FILES_ACCESS_PERMISSION)
300-
intent.addCategory("android.intent.category.DEFAULT")
301-
intent.data = Uri.parse("package:$packageName")
302-
startActivity(intent)
303-
}
304-
builder.setNegativeButton("Cancel", null)
305-
builder.show()
287+
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R && !android.os.Environment.isExternalStorageManager()) {
288+
val builder = AlertDialog.Builder(this)
289+
builder.setTitle(getString(R.string.app_name))
290+
builder.setMessage("To save offline files, the app needs access to all files.")
291+
builder.setPositiveButton("Settings") { _, _ ->
292+
val intent = Intent(android.provider.Settings.ACTION_MANAGE_APP_ALL_FILES_ACCESS_PERMISSION)
293+
intent.addCategory("android.intent.category.DEFAULT")
294+
intent.data = Uri.parse("package:$packageName")
295+
startActivity(intent)
306296
}
297+
builder.setNegativeButton("Cancel", null)
298+
builder.show()
307299
}
308300
}
309301

@@ -346,7 +338,6 @@ class FileDisplayActivity : FileActivity(),
346338
isLightUser = manageAccountsViewModel.checkUserLight(account.name)
347339
isMultiPersonal = capabilitiesViewModel.checkMultiPersonal()
348340
navigateTo(fileListOption, initialState = true)
349-
350341
}
351342

352343
startListeningToOperations()
@@ -396,12 +387,12 @@ class FileDisplayActivity : FileActivity(),
396387
syncProfileOperation.syncUserProfile()
397388
val workManagerProvider = WorkManagerProvider(context = baseContext)
398389
workManagerProvider.enqueueAvailableOfflinePeriodicWorker()
399-
390+
400391
// Enqueue Download Everything worker if enabled
401392
if (sharedPreferences.getBoolean(SettingsSecurityFragment.PREFERENCE_DOWNLOAD_EVERYTHING, false)) {
402393
workManagerProvider.enqueueDownloadEverythingWorker()
403394
}
404-
395+
405396
// Enqueue Local File Sync worker if enabled
406397
if (sharedPreferences.getBoolean(SettingsSecurityFragment.PREFERENCE_AUTO_SYNC, false)) {
407398
workManagerProvider.enqueueLocalFileSyncWorker()
@@ -763,10 +754,7 @@ class FileDisplayActivity : FileActivity(),
763754
* 2. close FAB if open (only if drawer isn't open)
764755
* 3. navigate up (only if drawer and FAB aren't open)
765756
*/
766-
if (isDrawerOpen() && isFabOpen) {
767-
// close drawer first
768-
super.onBackPressed()
769-
} else if (isDrawerOpen() && !isFabOpen) {
757+
if (isDrawerOpen()) {
770758
// close drawer
771759
super.onBackPressed()
772760
} else if (!isDrawerOpen() && isFabOpen) {

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

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,10 @@ import eu.opencloud.android.domain.BaseUseCaseWithResult
2626
import eu.opencloud.android.domain.exceptions.FileNotFoundException
2727
import eu.opencloud.android.domain.files.FileRepository
2828
import eu.opencloud.android.domain.files.model.OCFile
29-
import eu.opencloud.android.domain.files.usecases.SaveConflictUseCase
29+
3030
import eu.opencloud.android.presentation.settings.security.SettingsSecurityFragment
3131
import eu.opencloud.android.usecases.transfers.downloads.DownloadFileUseCase
3232
import eu.opencloud.android.usecases.transfers.uploads.UploadFileInConflictUseCase
33-
import kotlinx.coroutines.CoroutineScope
34-
import kotlinx.coroutines.Dispatchers
3533
import timber.log.Timber
3634
import java.io.File
3735
import java.text.SimpleDateFormat
@@ -42,7 +40,6 @@ import java.util.UUID
4240
class SynchronizeFileUseCase(
4341
private val downloadFileUseCase: DownloadFileUseCase,
4442
private val uploadFileInConflictUseCase: UploadFileInConflictUseCase,
45-
private val saveConflictUseCase: SaveConflictUseCase,
4643
private val fileRepository: FileRepository,
4744
private val preferencesProvider: SharedPreferencesProvider,
4845
) : BaseUseCaseWithResult<SynchronizeFileUseCase.SyncType, SynchronizeFileUseCase.Params>() {
@@ -51,13 +48,13 @@ class SynchronizeFileUseCase(
5148
val fileToSynchronize = params.fileToSynchronize
5249
val accountName: String = fileToSynchronize.owner
5350

54-
// Fix: Use correct dispatcher usage. `CoroutineScope(Dispatchers.IO).run` blocks the current thread if called with `runBlocking` or similar upstream,
55-
// but here it returns a value, so it implies this is synchronous code?
51+
// Fix: Use correct dispatcher usage. `CoroutineScope(Dispatchers.IO).run` blocks the current thread
52+
// if called with `runBlocking` or similar upstream, but here it returns a value, so it implies this is synchronous code?
5653
// `BaseUseCaseWithResult` usually runs in a background thread if invoked correctly?
57-
// The original code used `CoroutineScope(Dispatchers.IO).run`, which creates a scope and runs the block.
58-
// But `run` is incorrect here if we want to return from the function.
54+
// The original code used `CoroutineScope(Dispatchers.IO).run`, which creates a scope and runs the block.
55+
// But `run` is incorrect here if we want to return from the function.
5956
// `CoroutineScope(Dispatchers.IO).run` returns the result of the block.
60-
// Actually `run` is a standard library extension on T.
57+
// Actually `run` is a standard library extension on T.
6158
// `CoroutineScope(Dispatchers.IO)` creates a stand-alone scope. `run` executes the block on *that object*.
6259
// It does NOT switch threads! `CoroutineScope(Dispatchers.IO)` just creates a new scope object.
6360
// The original code was likely running on the caller's thread!
@@ -66,20 +63,23 @@ class SynchronizeFileUseCase(
6663
// This is a subtle but massive bug in the original code too if it expected async/threading.
6764
// However, `BaseUseCaseWithResult` might be handling threading?
6865
// Let's assume we just want to fix the logic bugs for now.
69-
66+
7067
// 1. Check local state first to avoid network calls if possible (optimization)
7168
// Check if file has changed locally by reading ACTUAL file timestamp from filesystem
7269
val storagePath = fileToSynchronize.storagePath
73-
val localFile = if (storagePath != null) File(storagePath) else null
70+
val localFile = storagePath?.let { File(it) }
7471
val fileExistsLocally = localFile?.exists() == true
75-
72+
7673
var changedLocally = false
7774
if (fileExistsLocally) {
7875
val actualFileModificationTime = localFile!!.lastModified()
7976
// Fix: Null safety for lastSyncDateForData
8077
changedLocally = actualFileModificationTime > (fileToSynchronize.lastSyncDateForData ?: 0)
81-
82-
Timber.d("File ${fileToSynchronize.fileName}: localTimestamp=$actualFileModificationTime, lastSync=${fileToSynchronize.lastSyncDateForData}, changedLocally=$changedLocally")
78+
79+
Timber.d(
80+
"File ${fileToSynchronize.fileName}: localTimestamp=$actualFileModificationTime, " +
81+
"lastSync=${fileToSynchronize.lastSyncDateForData}, changedLocally=$changedLocally"
82+
)
8383
}
8484

8585
// 2. Perform propfind to check remote state
@@ -91,11 +91,11 @@ class SynchronizeFileUseCase(
9191
)
9292
} catch (exception: FileNotFoundException) {
9393
Timber.i(exception, "File does not exist anymore in remote")
94-
94+
9595
// 2.1 File does not exist anymore in remote
9696
// If it still exists locally, but file has different path, another operation could have been done simultaneously
9797
val localDbFile = fileToSynchronize.id?.let { fileRepository.getFileById(it) }
98-
98+
9999
if (localDbFile != null && (localDbFile.remotePath == fileToSynchronize.remotePath && localDbFile.spaceId == fileToSynchronize.spaceId)) {
100100
fileRepository.deleteFiles(listOf(fileToSynchronize), true)
101101
}
@@ -118,7 +118,7 @@ class SynchronizeFileUseCase(
118118
val preferLocal = preferencesProvider.getBoolean(
119119
SettingsSecurityFragment.PREFERENCE_PREFER_LOCAL_ON_CONFLICT, false
120120
)
121-
121+
122122
if (preferLocal) {
123123
// User prefers local version - upload it (overwrites remote)
124124
Timber.i("File ${fileToSynchronize.fileName} has conflict. User prefers local version, uploading.")
@@ -130,7 +130,7 @@ class SynchronizeFileUseCase(
130130
val conflictedCopyPath = createConflictedCopyPath(fileToSynchronize)
131131
// Fix: Rename safety
132132
val renamed = renameLocalFile(fileToSynchronize.storagePath!!, conflictedCopyPath)
133-
133+
134134
if (renamed) {
135135
Timber.i("Local file renamed to conflicted copy: $conflictedCopyPath")
136136
// Refresh parent folder so the conflicted copy appears in the file list
@@ -144,7 +144,7 @@ class SynchronizeFileUseCase(
144144
} catch (e: Exception) {
145145
Timber.w(e, "Failed to refresh parent folder after creating conflicted copy")
146146
}
147-
147+
148148
// Only download if renamed successfully
149149
val uuid = requestForDownload(accountName, fileToSynchronize)
150150
SyncType.ConflictResolvedWithCopy(uuid, conflictedCopyPath)
@@ -158,7 +158,7 @@ class SynchronizeFileUseCase(
158158
}
159159
} else if (changedRemotely) {
160160
// 5.2 File has changed ONLY remotely -> download new version
161-
// Fix: Check if we have unsaved local changes that we missed?
161+
// Fix: Check if we have unsaved local changes that we missed?
162162
// (Already covered by changedLocally check above)
163163
Timber.i("File ${fileToSynchronize.fileName} has changed remotely. Let's download the new version")
164164
val uuid = requestForDownload(accountName, fileToSynchronize)
@@ -208,13 +208,11 @@ class SynchronizeFileUseCase(
208208
return File(file.parent, conflictedName).absolutePath
209209
}
210210

211-
private fun renameLocalFile(oldPath: String, newPath: String): Boolean {
212-
return try {
213-
File(oldPath).renameTo(File(newPath))
214-
} catch (e: Exception) {
215-
Timber.e(e, "Failed to rename local file from $oldPath to $newPath")
216-
false
217-
}
211+
private fun renameLocalFile(oldPath: String, newPath: String): Boolean = try {
212+
File(oldPath).renameTo(File(newPath))
213+
} catch (e: Exception) {
214+
Timber.e(e, "Failed to rename local file from $oldPath to $newPath")
215+
false
218216
}
219217

220218
data class Params(

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ import java.util.concurrent.TimeUnit
4747
/**
4848
* Worker that downloads ALL files from all accounts for offline access.
4949
* This is an opt-in feature that can be enabled in Security Settings.
50-
*
50+
*
5151
* This worker:
5252
* 1. Iterates through all connected accounts
5353
* 2. Discovers all spaces (personal + project) for each account
@@ -78,7 +78,7 @@ class DownloadEverythingWorker(
7878

7979
override suspend fun doWork(): Result {
8080
Timber.i("DownloadEverythingWorker started")
81-
81+
8282
// Create notification channel and show initial notification
8383
createNotificationChannel()
8484
updateNotification("Starting download of all files...")
@@ -98,7 +98,11 @@ class DownloadEverythingWorker(
9898
try {
9999
// Get capabilities for account
100100
val capabilities = getStoredCapabilitiesUseCase(GetStoredCapabilitiesUseCase.Params(accountName))
101-
val spacesAvailableForAccount = AccountUtils.isSpacesFeatureAllowedForAccount(appContext, account, capabilities)
101+
val spacesAvailableForAccount = AccountUtils.isSpacesFeatureAllowedForAccount(
102+
appContext,
103+
account,
104+
capabilities
105+
)
102106

103107
if (!spacesAvailableForAccount) {
104108
// Account does not support spaces - process legacy root
@@ -125,7 +129,8 @@ class DownloadEverythingWorker(
125129
}
126130
}
127131

128-
val summary = "Done! Files: $totalFilesFound, Downloaded: $filesDownloaded, Already local: $filesAlreadyLocal, Skipped: $filesSkipped, Folders: $foldersProcessed"
132+
val summary = "Done! Files: $totalFilesFound, Downloaded: $filesDownloaded, " +
133+
"Already local: $filesAlreadyLocal, Skipped: $filesSkipped, Folders: $foldersProcessed"
129134
Timber.i("DownloadEverythingWorker completed: $summary")
130135
updateNotification(summary)
131136

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,12 @@ class DownloadFileWorker(
112112
*/
113113
private val finalLocationForFile: String
114114
get() = ocFile.storagePath.takeUnless { it.isNullOrBlank() }
115-
?: localStorageProvider.getDefaultSavePathFor(accountName = account.name, remotePath = ocFile.remotePath, spaceId = ocFile.spaceId, spaceName = spaceName)
115+
?: localStorageProvider.getDefaultSavePathFor(
116+
accountName = account.name,
117+
remotePath = ocFile.remotePath,
118+
spaceId = ocFile.spaceId,
119+
spaceName = spaceName
120+
)
116121

117122
override suspend fun doWork(): Result {
118123
if (!areParametersValid()) return Result.failure()
@@ -316,7 +321,10 @@ class DownloadFileWorker(
316321
}
317322

318323
private fun getClientForThisDownload(): OpenCloudClient = SingleSessionManager.getDefaultSingleton()
319-
.getClientFor(OpenCloudAccount(AccountUtils.getOpenCloudAccountByName(appContext, account.name), appContext), appContext)
324+
.getClientFor(
325+
OpenCloudAccount(AccountUtils.getOpenCloudAccountByName(appContext, account.name), appContext),
326+
appContext
327+
)
320328

321329
override fun onTransferProgress(
322330
progressRate: Long,
@@ -330,7 +338,8 @@ class DownloadFileWorker(
330338
downloadRemoteFileOperation.removeDatatransferProgressListener(this)
331339
}
332340

333-
val percent: Int = if (totalToTransfer == -1L) -1 else (100.0 * totalTransferredSoFar.toDouble() / totalToTransfer.toDouble()).toInt()
341+
val percent: Int = if (totalToTransfer == -1L) -1 else (100.0 * totalTransferredSoFar.toDouble() /
342+
totalToTransfer.toDouble()).toInt()
334343
if (percent == lastPercent) return
335344

336345
// Set current progress. Observers will listen.

0 commit comments

Comments
 (0)