Skip to content

Commit df6449e

Browse files
committed
fix: remove MANAGE_EXTERNAL_STORAGE and fix sync workers
1 parent 79fb16f commit df6449e

4 files changed

Lines changed: 22 additions & 42 deletions

File tree

opencloudApp/src/main/AndroidManifest.xml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
API >= 23; the app needs to handle this
2424
-->
2525
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
26-
<uses-permission android:name="android.permission.MANAGE_EXTERNAL_STORAGE" tools:ignore="ScopedStorage" />
2726
<!--
2827
Notifications are off by default since API 33;
2928
See note in https://developer.android.com/develop/ui/views/notifications/notification-permission

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

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -283,25 +283,10 @@ class FileDisplayActivity : FileActivity(),
283283
}
284284

285285
checkNotificationPermission()
286-
checkManageExternalStoragePermission()
287286
Timber.v("onCreate() end")
288287
}
289288

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

306291
private fun checkNotificationPermission() {
307292
// Ask for permission only in case it's api >= 33 and notifications are not granted.

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

Lines changed: 20 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ import java.text.SimpleDateFormat
3636
import java.util.Date
3737
import java.util.Locale
3838
import java.util.UUID
39+
import kotlinx.coroutines.Dispatchers
40+
import kotlinx.coroutines.runBlocking
3941

4042
class SynchronizeFileUseCase(
4143
private val downloadFileUseCase: DownloadFileUseCase,
@@ -44,26 +46,10 @@ class SynchronizeFileUseCase(
4446
private val preferencesProvider: SharedPreferencesProvider,
4547
) : BaseUseCaseWithResult<SynchronizeFileUseCase.SyncType, SynchronizeFileUseCase.Params>() {
4648

47-
override fun run(params: Params): SyncType {
49+
override fun run(params: Params): SyncType = runBlocking(Dispatchers.IO) {
4850
val fileToSynchronize = params.fileToSynchronize
4951
val accountName: String = fileToSynchronize.owner
5052

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?
53-
// `BaseUseCaseWithResult` usually runs in a background thread if invoked correctly?
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.
56-
// `CoroutineScope(Dispatchers.IO).run` returns the result of the block.
57-
// Actually `run` is a standard library extension on T.
58-
// `CoroutineScope(Dispatchers.IO)` creates a stand-alone scope. `run` executes the block on *that object*.
59-
// It does NOT switch threads! `CoroutineScope(Dispatchers.IO)` just creates a new scope object.
60-
// The original code was likely running on the caller's thread!
61-
// `Dispatchers.IO` was completely ignored because `launch` or `async` wasn't called.
62-
// `CoroutineScope(...)` is just a factory. `.run { ... }` just runs the lambda on it.
63-
// This is a subtle but massive bug in the original code too if it expected async/threading.
64-
// However, `BaseUseCaseWithResult` might be handling threading?
65-
// Let's assume we just want to fix the logic bugs for now.
66-
6753
// 1. Check local state first to avoid network calls if possible (optimization)
6854
// Check if file has changed locally by reading ACTUAL file timestamp from filesystem
6955
val storagePath = fileToSynchronize.storagePath
@@ -127,9 +113,14 @@ class SynchronizeFileUseCase(
127113
} else {
128114
// Default: Create conflicted copy of local, download remote.
129115
Timber.i("File ${fileToSynchronize.fileName} has changed locally and remotely. Creating conflicted copy.")
130-
val conflictedCopyPath = createConflictedCopyPath(fileToSynchronize)
116+
val localPath = fileToSynchronize.storagePath
117+
if (localPath.isNullOrEmpty()) {
118+
Timber.e("File ${fileToSynchronize.fileName} has no local storage path. Cannot create conflicted copy.")
119+
return@runBlocking SyncType.AlreadySynchronized
120+
}
121+
val conflictedCopyPath = createConflictedCopyPath(localPath)
131122
// Fix: Rename safety
132-
val renamed = renameLocalFile(fileToSynchronize.storagePath!!, conflictedCopyPath)
123+
val renamed = renameLocalFile(localPath, conflictedCopyPath)
133124

134125
if (renamed) {
135126
Timber.i("Local file renamed to conflicted copy: $conflictedCopyPath")
@@ -184,18 +175,23 @@ class SynchronizeFileUseCase(
184175
)
185176
)
186177

187-
private fun requestForUpload(accountName: String, ocFile: OCFile): UUID? =
188-
uploadFileInConflictUseCase(
178+
private fun requestForUpload(accountName: String, ocFile: OCFile): UUID? {
179+
val localPath = ocFile.storagePath
180+
if (localPath.isNullOrEmpty()) {
181+
Timber.e("Cannot upload file ${ocFile.fileName} because storagePath is null or empty.")
182+
return null
183+
}
184+
return uploadFileInConflictUseCase(
189185
UploadFileInConflictUseCase.Params(
190186
accountName = accountName,
191-
localPath = ocFile.storagePath!!,
187+
localPath = localPath,
192188
uploadFolderPath = ocFile.getParentRemotePath(),
193189
spaceId = ocFile.spaceId,
194190
)
195191
)
192+
}
196193

197-
private fun createConflictedCopyPath(ocFile: OCFile): String {
198-
val originalPath = ocFile.storagePath!!
194+
private fun createConflictedCopyPath(originalPath: String): String {
199195
val file = File(originalPath)
200196
val nameWithoutExt = file.nameWithoutExtension
201197
val extension = file.extension

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ class LocalFileSyncWorker(
210210

211211
companion object {
212212
const val LOCAL_FILE_SYNC_WORKER = "LOCAL_FILE_SYNC_WORKER"
213-
const val repeatInterval: Long = 5L
213+
const val repeatInterval: Long = 15L
214214
val repeatIntervalTimeUnit: TimeUnit = TimeUnit.MINUTES
215215

216216
private const val NOTIFICATION_CHANNEL_ID = "auto_sync_channel"

0 commit comments

Comments
 (0)