diff --git a/.claude/skills/android-eventbus-to-workmanager-flow/SKILL.md b/.claude/skills/android-eventbus-to-workmanager-flow/SKILL.md new file mode 100644 index 000000000000..c18998440ba1 --- /dev/null +++ b/.claude/skills/android-eventbus-to-workmanager-flow/SKILL.md @@ -0,0 +1,135 @@ +--- +name: android-eventbus-to-workmanager-flow +description: > + Use when replacing a GreenRobot EventBus event that a WorkManager worker posts (progress, + state, completion) with WorkManager's own observation APIs, consumed through a ViewModel + Flow; when the user mentions "event bus", "EventBus", "EventBusFactory", "@Subscribe", + "worker progress", "setProgress", "observe worker", "getWorkInfosForUniqueWorkFlow" or + "listen to the worker through the viewmodel". Covers publishing progress from the worker, + building the ViewModel Flow keyed by the work name, DI registration, collecting the Flow + from Kotlin or legacy Java fragments/activities, and deleting the event classes. +license: AGPL-3.0-or-later +SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors +SPDX-License-Identifier: AGPL-3.0-or-later +metadata: + author: Nextcloud Android + version: "1.0.0" +--- + +# EventBus → WorkManager Flow via ViewModel + +Workers used to tell the UI what they were doing by posting GreenRobot EventBus events +(`EventBusFactory.downloadProgressEventBus.post(FileDownloadProgressEvent(percent))`). That +has three problems: the event is global (every subscriber gets every worker's events, +whatever file it is about), it is lost when nobody is registered, and the subscriber has to +register and unregister by hand in `onStart`/`onStop`. + +WorkManager already stores each worker's state and progress and exposes them as `Flow`s. The +UI asks WorkManager about **one specific piece of work**, through a ViewModel, and collects +the Flow with the lifecycle. Official guide: +https://developer.android.com/develop/background-work/background-tasks/persistent/how-to/observe + +Converted so far: `FileDownloadProgressEvent` → `FileDetailFragmentViewModel.downloadProgress` +(see `assets/worked-example.md`). The earlier `FileDisplayActivityViewModel.observeOfflineWorker` +is the simplest form of the same pattern (state only, Kotlin consumer). + +## The Prime Directive: Behaviour Must Not Change + +The screen must show the same thing at the same moments as before. The only intended +differences are the ones the global bus got wrong: a screen now only reacts to the work it is +about. Write them down for the PR description. If you find an unrelated bug, report it +instead of fixing it in the same change. + +## Workflow + +### Step 0: Inventory (do not edit yet) + +1. `git grep -n ""` across `app/src`. List every **poster** (usually a worker) and + every **subscriber** (`@Subscribe fun on…(event: )`). +2. If the event lives on a dedicated bus (`EventBusFactory.EventBus`), also grep for the + bus. When it has no other users, delete it together with the event. +3. For each poster, find how its work is enqueued in `BackgroundJobManagerImpl`: the + **unique work name** (`enqueueUniqueWork(name, …)`) and the **tags** (`oneTimeRequestBuilder` + adds `formatNameTag`, `formatClassTag`, `formatUserTag`, plus any custom tag). The UI + must be able to rebuild exactly one of these from what it already knows (user, file id…). +4. List what each event field carries and what the subscriber does with it. Decide whether + it is **progress** (while running → `setProgress`), a **result** (at the end → + `Result.success(outputData)`), or only **state** (`WorkInfo.state`, nothing to publish). +5. Look for older parallel mechanisms the subscriber also uses for the same purpose (bound + listeners, `LocalBroadcastManager`). If you can prove one is dead (e.g. a field that is + only ever set to `null`), remove it with all its plumbing, including test doubles in + `app/src/debug` and mocks in `androidTest`. Otherwise leave it and mention it in the summary. + +### Step 1: Make the work addressable + +If the unique work name / tag builder is `private` in `BackgroundJobManagerImpl`, move it +into the `companion object` next to `formatNameTag`/`formatClassTag` as a public +`format…Tag(...)` function, and have the enqueue and cancel call sites use it. The UI and +the scheduler must build the name with the same function, never with a copied string. + +### Step 2: Publish from the worker + +- Add the `Data` key as a `const val` in the worker's `companion object` + (`const val PROGRESS_PERCENT = "PROGRESS_PERCENT"`). +- From a `suspend` context call `setProgress(workDataOf(KEY to value))`. From a non-suspend + callback (e.g. `OnDatatransferProgressListener.onTransferProgress`) call + `setProgressAsync(...)`. +- **Throttle it.** WorkManager writes progress to its database. Publish only when the value + changed, and reuse an existing throttle (the notification update interval) when the + worker has one. +- Results go in `Result.success(workDataOf(...))` and are read from `WorkInfo.outputData`. +- Delete the `post(...)` call and the event imports. + +### Step 3: ViewModel + +One ViewModel per screen, in a package next to the screen +(`ui/fragment//FragmentViewModel.kt`, mirroring +`ui/activity/filedisplayactivity/FileDisplayActivityViewModel.kt`). Template: +`references/VIEWMODEL-TEMPLATE.md`. + +- Constructor-inject `WorkManager` (`@Inject constructor(private val workManager: WorkManager)`). +- Hold the work name the screen is interested in a `MutableStateFlow`. A setter + function (`observeDownloadProgress(user, file)`) builds it with the Step 1 function. +- Expose a **cold** `Flow`: `filterNotNull()` → `flatMapLatest { getWorkInfosForUniqueWorkFlow(it) }` + → map the `WorkInfo` list to the UI value → `distinctUntilChanged()`. Use + `getWorkInfoByIdFlow(id)` when you have the request id, `getWorkInfosByTagFlow(tag)` when + the work is not unique. +- Progress is only present while `state == RUNNING`. It is empty after the work finishes, so + filter on `RUNNING` and drop missing keys (`progress.keyValueMap[KEY] as? Int`). +- Register it in `di/ViewModelModule.kt` with `@Binds @IntoMap @ViewModelKey(X::class)`. + Without this, `ViewModelFactory` throws at runtime. + +### Step 4: Consumer + +- Get the ViewModel from `ViewModelFactory`: Kotlin `by viewModels { vmFactory }`, Java + `new ViewModelProvider(this, vmFactory).get(X.class)` in `onCreate`. Injection of + `Injectable` fragments happens before `onAttach`, so `vmFactory` is ready in `onCreate`. +- Tell the ViewModel what to watch where the screen learns its subject (`onViewCreated`, + and every public "show this other file" method such as `updateFileDetails(file, user)`). +- Collect with the **view** lifecycle in fragments: + - Kotlin: `viewLifecycleOwner.lifecycleScope.launch { viewLifecycleOwner.repeatOnLifecycle(STARTED) { vm.flow.collect { … } } }` + - Java: `LifecycleOwnerExtensionsKt.collectWhenStarted(getViewLifecycleOwner(), viewModel.getX(), this::onX);` + (`utils/extensions/LifecycleOwnerExtensions.kt`). Do not switch to `LiveData` for Java + callers. See `references/PITFALLS.md` for why. +- Turn the old `@Subscribe public void onX(Event e)` into `private void onX(T value)`. Keep + its body unchanged apart from reading the value directly. +- Remove the `register`/`unregister` lines for the bus. Keep `EventBus.getDefault()` registration + if the class still has other `@Subscribe` methods. + +### Step 5: Delete and verify + +1. Delete the event class (and the dedicated bus if it has no users left). +2. `git grep -n "\|"` returns nothing. +3. `./gradlew :app:compileGplayDebugJavaWithJavac spotlessKotlinCheck detekt`, then + `lintGplayDebug spotbugsGplayDebug`. Fix findings in every file you touched. +4. Manual check (hand it to the developer if you cannot run the app): start the work and + open the screen, so the value shows up; open the screen first, then start the work; rotate + while it runs; leave the screen for more than 5 s, let the work finish, come back (no stale + value); open the screen for a *different* subject while work runs for another one (it must + not react). + +## References + +- `references/VIEWMODEL-TEMPLATE.md`: ViewModel, worker and consumer snippets. +- `references/PITFALLS.md`: what went wrong or nearly went wrong. +- `assets/worked-example.md`: `FileDownloadProgressEvent` → `FileDetailFragmentViewModel`. diff --git a/.claude/skills/android-eventbus-to-workmanager-flow/assets/worked-example.md b/.claude/skills/android-eventbus-to-workmanager-flow/assets/worked-example.md new file mode 100644 index 000000000000..4ab8eee61785 --- /dev/null +++ b/.claude/skills/android-eventbus-to-workmanager-flow/assets/worked-example.md @@ -0,0 +1,56 @@ + + +# Worked Example: `FileDownloadProgressEvent` → `FileDetailFragmentViewModel` + +## Inventory findings + +- Event: `ui/events/FileDownloadProgressEvent(percent: Int)` on a dedicated bus, + `ui/events/EventBusFactory.downloadProgressEventBus`. Neither had other users. +- Poster: `FileDownloadWorker.onTransferProgress`, on every transfer chunk, unthrottled, with + no file information. +- Subscriber: `FileDetailFragment` (Java). It registered the bus in `onStart`, unregistered + it in `onStop`, and `@Subscribe(threadMode = MAIN) onDownloadProgress` showed the progress + block with the "downloading" text and the percent. +- Enqueue: `BackgroundJobManagerImpl.startFileDownloadJob` → + `enqueueUniqueWork(startFileDownloadJobTag(accountName, fileId), KEEP, request)`, and the + same string as a tag. The builder was `private`. +- Parallel mechanism: `containerActivity.getFileDownloadProgressListener()` / + `FileDownloadWorker.FileDownloadProgressListener`. The field in `FileActivity` is never + assigned, so it is always `null`, which is why the bus was added (commit `37df0dca9d`). + Removed in the same change, since the ViewModel replaces it. +- Folders use `FolderDownloadWorker`, which never posted the event. + +## Changes + +| File | Change | +|---|---| +| `jobs/BackgroundJobManagerImpl.kt` | `startFileDownloadJobTag` → public companion `formatFileDownloadTag`, used by start and cancel | +| `jobs/download/FileDownloadWorker.kt` | `PROGRESS_PERCENT` key; `setProgressAsync(workDataOf(PROGRESS_PERCENT to percent))` inside the notification throttle; bus post removed | +| `ui/fragment/filedetail/FileDetailFragmentViewModel.kt` | new; `downloadProgress: Flow` keyed by `formatFileDownloadTag` | +| `di/ViewModelModule.kt` | `@Binds @IntoMap @ViewModelKey(FileDetailFragmentViewModel::class)` | +| `utils/extensions/LifecycleOwnerExtensions.kt` | new; `collectWhenStarted(flow, Consumer)` for Java consumers | +| `ui/fragment/FileDetailFragment.java` | `ViewModelFactory` inject, VM in `onCreate`, `observeDownloadProgress` in `onViewCreated` and `updateFileDetails(file, user)`, `collectWhenStarted` on the view lifecycle, `@Subscribe` → `private void onDownloadProgress(int)`, bus register/unregister removed | +| `ui/events/FileDownloadProgressEvent.kt`, `ui/events/EventBusFactory.kt` | deleted | +| `FileDownloadWorker.FileDownloadProgressListener` and its plumbing | deleted: inner class, worker field, `ComponentsGetter.getFileDownloadProgressListener()`, `FileActivity` field and getter, the two `= null` assignments in `FileDisplayActivity`, the download branches of `FileDetailFragment.listenForTransferProgress`/`leaveTransferProgress`, the `TestActivity` override and the `FileMenuFilterIT` mock | + +## Intended behaviour differences + +- Progress is shown only for the file the details screen is about, not for whichever file is + downloading. +- The UI updates at the notification throttle (750 ms) instead of on every chunk. + +## Decisions + +- **Cold `Flow` + `collectWhenStarted`, not `LiveData`.** `LiveData` replayed a stale + percent after a finish that happened while the view was stopped (see `PITFALLS.md`). +- **The ViewModel is created in `onCreate`, not `onViewCreated`.** The activity can call + `updateFileDetails(file, user)` before the view exists, and the ViewModel must survive view + re-creation from the back stack. +- **The finished state is still handled by the existing broadcast path.** + `FileDisplayActivity` calls `updateFileDetails(false, true)` on `ACTION_DOWNLOAD_COMPLETED`. + Moving that to `WorkInfo.State.SUCCEEDED` is a possible follow-up, not part of this change. diff --git a/.claude/skills/android-eventbus-to-workmanager-flow/references/PITFALLS.md b/.claude/skills/android-eventbus-to-workmanager-flow/references/PITFALLS.md new file mode 100644 index 000000000000..e8763900a224 --- /dev/null +++ b/.claude/skills/android-eventbus-to-workmanager-flow/references/PITFALLS.md @@ -0,0 +1,67 @@ + + +# Pitfalls + +## `LiveData` replays a stale value + +The first draft exposed `asLiveData()` so the Java fragment could `observe(...)`. `LiveData` +keeps its last value and stops collecting upstream 5 s after the last observer goes away. If +the download finishes during those 5 s, the cached value is the last running percent, and +the next observer gets it right away. The progress block is shown again for a download that +has already finished. A cold `Flow` collected with `repeatOnLifecycle` asks WorkManager again +on every start and only gets the current state. Use `collectWhenStarted` from Java instead +of `LiveData`. + +## Progress disappears when the work finishes + +`WorkInfo.progress` is empty as soon as the state leaves `RUNNING`. Do not read progress +from `SUCCEEDED`/`FAILED` infos, and do not treat the missing key as `0`. Use `mapNotNull` +and let the existing "finished" path (broadcast, `updateFileDetails(false, true)`) hide the +UI, as it did before. Put final values in `Result.success(outputData)` if the UI needs them. + +## Every `setProgress` is a database write + +WorkManager persists progress. Calling it on every transfer chunk writes to the database +hundreds of times per file. Publish only on change and inside the worker's existing +throttle (`minProgressUpdateInterval`). + +## Unique work name vs tag + +`getWorkInfosForUniqueWorkFlow(name)` matches the `enqueueUniqueWork` name. +`getWorkInfosByTagFlow(tag)` matches `addTag`. In `startFileDownloadJob` the same string +is used for both, but that is not true for every job. Check the enqueue call. Never build the +string by hand in the ViewModel. Make the builder public in the +`BackgroundJobManagerImpl` companion. + +## The bus was global; the Flow is not + +`FileDownloadProgressEvent` reached `FileDetailFragment` for **any** running download, so +the details of file A could show the progress of file B. The ViewModel is keyed by +account + file id, so it only reacts to its own file. This is the intended change; mention it +in the PR. Check which worker really runs for each subject: folders are downloaded by +`FolderDownloadWorker`, not `FileDownloadWorker`, and never posted the event. + +## The subject can change without a new view + +`FileDetailFragment.updateFileDetails(OCFile, User)` swaps the file on a live fragment. Call +`viewModel.observeX(...)` there too. Thanks to `flatMapLatest`, the old work's Flow is +cancelled automatically. Guard with `viewModel != null` because the activity can call it on a +fragment that has not reached `onCreate` yet. + +## Missing ViewModel binding + +Forgetting `@Binds @IntoMap @ViewModelKey(X::class)` in `ViewModelModule` compiles, but +`ViewModelFactory` throws `IllegalArgumentException` when the screen opens. + +## Keep other `@Subscribe` methods registered + +`FileDetailFragment` still receives `FavoriteEvent` on `EventBus.getDefault()`. Remove only +the registration for the bus you are replacing. Removing the default registration silently +stops the remaining events. The other way round fails too: if the last `@Subscribe` method +goes away but `register(this)` stays, EventBus throws `EventBusException` ("has no public +methods with the @Subscribe annotation") when the screen starts. diff --git a/.claude/skills/android-eventbus-to-workmanager-flow/references/VIEWMODEL-TEMPLATE.md b/.claude/skills/android-eventbus-to-workmanager-flow/references/VIEWMODEL-TEMPLATE.md new file mode 100644 index 000000000000..35894d31172f --- /dev/null +++ b/.claude/skills/android-eventbus-to-workmanager-flow/references/VIEWMODEL-TEMPLATE.md @@ -0,0 +1,126 @@ + + +# Templates + +## Worker + +```kotlin +companion object { + const val PROGRESS_PERCENT = "PROGRESS_PERCENT" +} + +// suspend context +setProgress(workDataOf(PROGRESS_PERCENT to percent)) + +// non-suspend callback, inside the existing throttle +if (percent != lastPercent && (currentTime - lastUpdateTime) >= minProgressUpdateInterval) { + notificationManager.updateDownloadProgress(percent, totalToTransfer) + setProgressAsync(workDataOf(PROGRESS_PERCENT to percent)) + lastUpdateTime = currentTime +} +``` + +## Work name (BackgroundJobManagerImpl companion) + +```kotlin +fun formatFileDownloadTag(accountName: String, fileId: Long): String = + JOB_FOLDER_DOWNLOAD + accountName + fileId +``` + +Use it in `startXJob` (`enqueueUniqueWork(tag, …)`), in `cancelXJob`, and in the ViewModel. + +## ViewModel + +```kotlin +class FileDetailFragmentViewModel @Inject constructor(private val workManager: WorkManager) : ViewModel() { + + private val downloadWorkName = MutableStateFlow(null) + + @OptIn(ExperimentalCoroutinesApi::class) + val downloadProgress: Flow = downloadWorkName + .filterNotNull() + .flatMapLatest { workManager.getWorkInfosForUniqueWorkFlow(it) } + .mapNotNull { workInfos -> workInfos.firstOrNull { it.state == WorkInfo.State.RUNNING }?.downloadPercent() } + .distinctUntilChanged() + + fun observeDownloadProgress(user: User, file: OCFile) { + downloadWorkName.value = BackgroundJobManagerImpl.formatFileDownloadTag(user.accountName, file.fileId) + } + + private fun WorkInfo.downloadPercent(): Int? = progress.keyValueMap[FileDownloadWorker.PROGRESS_PERCENT] as? Int +} +``` + +State-only variant (no progress published), from `FileDisplayActivityViewModel`: + +```kotlin +suspend fun observeOfflineWorker(onComplete: () -> Unit) { + workManager + .getWorkInfosByTagFlow(BackgroundJobManagerImpl.formatClassTag(OfflineOperationsWorker::class)) + .collect { workInfos -> + if (workInfos.any { it.state == WorkInfo.State.SUCCEEDED }) { + onComplete() + } + } +} +``` + +Prefer exposing a `Flow` property over a `suspend fun` with a callback: the consumer decides +the lifecycle, and Java can collect it through `collectWhenStarted`. + +## DI (`di/ViewModelModule.kt`) + +```kotlin +@Binds +@IntoMap +@ViewModelKey(FileDetailFragmentViewModel::class) +abstract fun fileDetailFragmentViewModel(vm: FileDetailFragmentViewModel): ViewModel +``` + +## Kotlin consumer (fragment) + +```kotlin +@Inject lateinit var vmFactory: ViewModelFactory +private val viewModel by viewModels { vmFactory } + +override fun onViewCreated(view: View, savedInstanceState: Bundle?) { + viewModel.observeDownloadProgress(user, file) + viewLifecycleOwner.lifecycleScope.launch { + viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) { + viewModel.downloadProgress.collect(::onDownloadProgress) + } + } +} +``` + +## Java consumer (fragment) + +```java +@Inject ViewModelFactory vmFactory; +private FileDetailFragmentViewModel viewModel; + +@Override +public void onCreate(@Nullable Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + viewModel = new ViewModelProvider(this, vmFactory).get(FileDetailFragmentViewModel.class); +} + +@Override +public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceState) { + viewModel.observeDownloadProgress(user, getFile()); + LifecycleOwnerExtensionsKt.collectWhenStarted(getViewLifecycleOwner(), + viewModel.getDownloadProgress(), + this::onDownloadProgress); +} + +private void onDownloadProgress(int percent) { … } +``` + +`collectWhenStarted` (`utils/extensions/LifecycleOwnerExtensions.kt`) wraps +`lifecycleScope.launch { repeatOnLifecycle(STARTED) { flow.collect(collector::accept) } }` +and takes an `androidx.core.util.Consumer`, so a Java method reference fits. diff --git a/AGENTS.md b/AGENTS.md index 69a51a148e40..62d0a7127e8e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,6 +38,7 @@ Project-specific skills live in `./.claude/skills//`. Load a skill w - **`android-java-to-kotlin`** (`./.claude/skills/android-java-to-kotlin/SKILL.md`) — Completes a Java-to-Kotlin conversion in this Android app. Use it when finishing a conversion, when the user mentions "java to kotlin", "j2k", "convert java", or "make it idiomatic", or when a freshly IDE-converted `.kt` file needs cleanup. The workflow is two-person: the developer first runs the Android Studio converter (`Code > Convert Java File to Kotlin File`), then the agent drives the idiomatic second pass — fail-fast control flow, function decomposition, `lifecycleScope`/coroutines instead of Java threads, modern Android APIs, and project conventions (SPDX headers, no magic numbers, `@JvmStatic`). The conversion must preserve behaviour, and the agent must write a behaviour-locking test before declaring it done. - **`android-activity-to-navigator-fragment`** (`./.claude/skills/android-activity-to-navigator-fragment/SKILL.md`) — Converts a standalone Activity (typically a drawer destination) into a Fragment hosted by `NavigatorActivity`. Use it when the user mentions "navigator", `NavigatorActivity`, `NavigatorScreen`, or "convert activity to fragment". It covers registering the `NavigatorScreen`, the fragment and its layout, decoupling collaborators from `FileActivity`, rewiring every entry point with `NavigatorActivity.intent(...)`, DI and manifest cleanup, and migrating the screenshot test. +- **`android-eventbus-to-workmanager-flow`** (`./.claude/skills/android-eventbus-to-workmanager-flow/SKILL.md`) — Replaces an EventBus event posted by a WorkManager worker with WorkManager observation through a ViewModel `Flow`. Use it when the user mentions "event bus", `EventBusFactory`, `@Subscribe`, "worker progress", or "observe worker". It covers publishing with `setProgress`, a ViewModel keyed by the unique work name, `ViewModelModule` registration, collecting from Kotlin or Java (`collectWhenStarted`) and deleting the event. The live example is `FileDetailFragmentViewModel`. ### Android Skills diff --git a/app/src/androidTest/java/com/owncloud/android/files/FileMenuFilterIT.kt b/app/src/androidTest/java/com/owncloud/android/files/FileMenuFilterIT.kt index c0a0cdfbdcc5..ce02972076ef 100644 --- a/app/src/androidTest/java/com/owncloud/android/files/FileMenuFilterIT.kt +++ b/app/src/androidTest/java/com/owncloud/android/files/FileMenuFilterIT.kt @@ -10,7 +10,6 @@ package com.owncloud.android.files import androidx.test.core.app.launchActivity import androidx.test.ext.junit.runners.AndroidJUnit4 import com.nextcloud.client.account.User -import com.nextcloud.client.jobs.download.FileDownloadWorker import com.nextcloud.client.jobs.upload.FileUploadHelper import com.nextcloud.test.TestActivity import com.nextcloud.utils.EditorUtils @@ -48,9 +47,6 @@ class FileMenuFilterIT : AbstractIT() { @MockK private lateinit var mockFileUploaderBinder: FileUploadHelper - @MockK - private lateinit var mockFileDownloadProgressListener: FileDownloadWorker.FileDownloadProgressListener - @MockK private lateinit var mockOperationsServiceBinder: OperationsService.OperationsServiceBinder @@ -64,7 +60,6 @@ class FileMenuFilterIT : AbstractIT() { MockKAnnotations.init(this) every { mockFileUploaderBinder.isUploading(any(), any()) } returns false every { mockComponentsGetter.fileUploaderHelper } returns mockFileUploaderBinder - every { mockComponentsGetter.fileDownloadProgressListener } returns mockFileDownloadProgressListener every { mockOperationsServiceBinder.isSynchronizing(any(), any()) } returns false every { mockComponentsGetter.operationsServiceBinder } returns mockOperationsServiceBinder every { mockStorageManager.getFileById(any()) } returns OCFile("/") diff --git a/app/src/debug/java/com/nextcloud/test/TestActivity.kt b/app/src/debug/java/com/nextcloud/test/TestActivity.kt index bc51c8bd8532..a847c62eb07a 100644 --- a/app/src/debug/java/com/nextcloud/test/TestActivity.kt +++ b/app/src/debug/java/com/nextcloud/test/TestActivity.kt @@ -12,7 +12,6 @@ import android.view.View import androidx.fragment.app.Fragment import androidx.swiperefreshlayout.widget.SwipeRefreshLayout import com.nextcloud.client.database.NextcloudDatabase -import com.nextcloud.client.jobs.download.FileDownloadWorker import com.nextcloud.client.jobs.upload.FileUploadHelper import com.nextcloud.client.network.ConnectivityManagerFactory import com.nextcloud.utils.EditorUtils @@ -100,8 +99,6 @@ class TestActivity : override fun getFileUploaderHelper(): FileUploadHelper = FileUploadHelper.instance() - override fun getFileDownloadProgressListener(): FileDownloadWorker.FileDownloadProgressListener? = null - override fun getStorageManager(): FileDataStorageManager { if (!this::storage.isInitialized) { storage = FileDataStorageManager(user.get(), contentResolver) diff --git a/app/src/main/java/com/nextcloud/client/di/ViewModelModule.kt b/app/src/main/java/com/nextcloud/client/di/ViewModelModule.kt index 5189243dc673..0dc9da9cb15f 100644 --- a/app/src/main/java/com/nextcloud/client/di/ViewModelModule.kt +++ b/app/src/main/java/com/nextcloud/client/di/ViewModelModule.kt @@ -18,6 +18,7 @@ import com.nextcloud.ui.fileactions.FileActionsViewModel import com.nextcloud.ui.tags.TagManagementViewModel import com.nextcloud.ui.trashbinFileActions.TrashbinFileActionsViewModel import com.owncloud.android.ui.activity.filedisplayactivity.FileDisplayActivityViewModel +import com.owncloud.android.ui.fragment.filedetail.FileDetailFragmentViewModel import com.owncloud.android.ui.preview.pdf.PreviewPdfViewModel import com.owncloud.android.ui.unifiedsearch.UnifiedSearchViewModel import dagger.Binds @@ -78,4 +79,9 @@ abstract class ViewModelModule { @IntoMap @ViewModelKey(FileDisplayActivityViewModel::class) abstract fun fileDisplayActivityViewModel(vm: FileDisplayActivityViewModel): ViewModel + + @Binds + @IntoMap + @ViewModelKey(FileDetailFragmentViewModel::class) + abstract fun fileDetailFragmentViewModel(vm: FileDetailFragmentViewModel): ViewModel } diff --git a/app/src/main/java/com/nextcloud/client/jobs/BackgroundJobManagerImpl.kt b/app/src/main/java/com/nextcloud/client/jobs/BackgroundJobManagerImpl.kt index 122b5a8ddd59..415950263b4a 100644 --- a/app/src/main/java/com/nextcloud/client/jobs/BackgroundJobManagerImpl.kt +++ b/app/src/main/java/com/nextcloud/client/jobs/BackgroundJobManagerImpl.kt @@ -137,6 +137,8 @@ internal class BackgroundJobManagerImpl( fun formatUserTag(user: User): String = "$TAG_PREFIX_USER:${user.accountName}" fun formatClassTag(jobClass: KClass): String = "$TAG_PREFIX_CLASS:${jobClass.simpleName}" fun formatTimeTag(startTimestamp: Long): String = "$TAG_PREFIX_START_TIMESTAMP:$startTimestamp" + fun formatFileDownloadTag(accountName: String, fileId: Long): String = + JOB_FOLDER_DOWNLOAD + accountName + fileId fun parseTag(tag: String): Pair? { val key = tag.substringBefore(":", "") @@ -779,9 +781,6 @@ internal class BackgroundJobManagerImpl( } } - private fun startFileDownloadJobTag(accountName: String, fileId: Long): String = - JOB_FOLDER_DOWNLOAD + accountName + fileId - override fun startFileDownloadJob( user: User, file: OCFile, @@ -791,7 +790,7 @@ internal class BackgroundJobManagerImpl( packageName: String, conflictUploadId: Long? ) { - val tag = startFileDownloadJobTag(user.accountName, file.fileId) + val tag = formatFileDownloadTag(user.accountName, file.fileId) val data = workDataOf( FileDownloadWorker.ACCOUNT_NAME to user.accountName, @@ -824,7 +823,7 @@ internal class BackgroundJobManagerImpl( } override fun cancelFilesDownloadJob(accountName: String, fileId: Long) { - workManager.cancelAllWorkByTag(startFileDownloadJobTag(accountName, fileId)) + workManager.cancelAllWorkByTag(formatFileDownloadTag(accountName, fileId)) } override fun startPdfGenerateAndUploadWork( diff --git a/app/src/main/java/com/nextcloud/client/jobs/download/FileDownloadWorker.kt b/app/src/main/java/com/nextcloud/client/jobs/download/FileDownloadWorker.kt index f2fa055b28a3..48a7f45a3b7a 100644 --- a/app/src/main/java/com/nextcloud/client/jobs/download/FileDownloadWorker.kt +++ b/app/src/main/java/com/nextcloud/client/jobs/download/FileDownloadWorker.kt @@ -18,6 +18,7 @@ import androidx.localbroadcastmanager.content.LocalBroadcastManager import androidx.work.CoroutineWorker import androidx.work.ForegroundInfo import androidx.work.WorkerParameters +import androidx.work.workDataOf import com.nextcloud.client.account.User import com.nextcloud.client.account.UserAccountManager import com.nextcloud.utils.ForegroundServiceHelper @@ -35,13 +36,10 @@ import com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCo import com.owncloud.android.lib.common.utils.Log_OC import com.owncloud.android.operations.DownloadFileOperation import com.owncloud.android.operations.DownloadType -import com.owncloud.android.ui.events.EventBusFactory -import com.owncloud.android.ui.events.FileDownloadProgressEvent import com.owncloud.android.utils.theme.ViewThemeUtils import java.util.AbstractList import java.util.Optional import java.util.Vector -import java.util.concurrent.ConcurrentHashMap import kotlin.random.Random @Suppress("LongParameterList", "TooManyFunctions", "TooGenericExceptionCaught") @@ -77,6 +75,7 @@ class FileDownloadWorker( const val ACTIVITY_NAME = "ACTIVITY_NAME" const val PACKAGE_NAME = "PACKAGE_NAME" const val CONFLICT_UPLOAD_ID = "CONFLICT_UPLOAD_ID" + const val PROGRESS_PERCENT = "PROGRESS_PERCENT" } private var currentDownload: DownloadFileOperation? = null @@ -93,8 +92,6 @@ class FileDownloadWorker( viewThemeUtils ) - private var downloadProgressListener = FileDownloadProgressListener() - private var user: User? = null private var currentUser = Optional.empty() @@ -412,49 +409,10 @@ class FileDownloadWorker( notificationManager.run { updateDownloadProgress(percent, totalToTransfer) } + setProgressAsync(workDataOf(PROGRESS_PERCENT to percent)) lastUpdateTime = currentTime } lastPercent = percent - EventBusFactory.downloadProgressEventBus.post(FileDownloadProgressEvent(percent)) - downloadProgressListener.onTransferProgress(progressRate, totalTransferredSoFar, totalToTransfer, filePath) - } - - inner class FileDownloadProgressListener : OnDatatransferProgressListener { - private val boundListeners = ConcurrentHashMap() - - fun addDataTransferProgressListener(listener: OnDatatransferProgressListener?, file: OCFile?) { - if (file == null || listener == null) { - return - } - - boundListeners[file.fileId] = listener - } - - fun removeDataTransferProgressListener(listener: OnDatatransferProgressListener?, file: OCFile?) { - if (file == null || listener == null) { - return - } - - val fileId = file.fileId - if (boundListeners[fileId] === listener) { - boundListeners.remove(fileId) - } - } - - override fun onTransferProgress( - progressRate: Long, - totalTransferredSoFar: Long, - totalToTransfer: Long, - fileName: String - ) { - val listener = boundListeners[currentDownload?.file?.fileId] - listener?.onTransferProgress( - progressRate, - totalTransferredSoFar, - totalToTransfer, - fileName - ) - } } } diff --git a/app/src/main/java/com/nextcloud/utils/extensions/LifecycleOwnerExtensions.kt b/app/src/main/java/com/nextcloud/utils/extensions/LifecycleOwnerExtensions.kt new file mode 100644 index 000000000000..08c7e87d15b3 --- /dev/null +++ b/app/src/main/java/com/nextcloud/utils/extensions/LifecycleOwnerExtensions.kt @@ -0,0 +1,23 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Alper Ozturk + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +package com.nextcloud.utils.extensions + +import androidx.core.util.Consumer +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.lifecycleScope +import androidx.lifecycle.repeatOnLifecycle +import kotlinx.coroutines.Job +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.launch + +fun LifecycleOwner.collectWhenStarted(flow: Flow, collector: Consumer): Job = lifecycleScope.launch { + repeatOnLifecycle(Lifecycle.State.STARTED) { + flow.collect { collector.accept(it) } + } +} diff --git a/app/src/main/java/com/owncloud/android/ui/activity/ComponentsGetter.java b/app/src/main/java/com/owncloud/android/ui/activity/ComponentsGetter.java index 7ce91df97af4..554746206ece 100644 --- a/app/src/main/java/com/owncloud/android/ui/activity/ComponentsGetter.java +++ b/app/src/main/java/com/owncloud/android/ui/activity/ComponentsGetter.java @@ -10,7 +10,6 @@ */ package com.owncloud.android.ui.activity; -import com.nextcloud.client.jobs.download.FileDownloadWorker; import com.nextcloud.client.jobs.upload.FileUploadHelper; import com.owncloud.android.datamodel.FileDataStorageManager; import com.owncloud.android.services.OperationsService.OperationsServiceBinder; @@ -18,12 +17,6 @@ public interface ComponentsGetter { - /** - * To be invoked when the parent activity is fully created to get a reference - * to the FileDownloadWorker. - */ - public FileDownloadWorker.FileDownloadProgressListener getFileDownloadProgressListener(); - /** * To be invoked when the parent activity is fully created to get a reference * to the FileUploader service API. diff --git a/app/src/main/java/com/owncloud/android/ui/activity/FileActivity.java b/app/src/main/java/com/owncloud/android/ui/activity/FileActivity.java index 25d3f80c2a09..3761024e591b 100644 --- a/app/src/main/java/com/owncloud/android/ui/activity/FileActivity.java +++ b/app/src/main/java/com/owncloud/android/ui/activity/FileActivity.java @@ -32,7 +32,6 @@ import com.nextcloud.client.account.User; import com.nextcloud.client.account.UserAccountManager; import com.nextcloud.client.jobs.BackgroundJobManager; -import com.nextcloud.client.jobs.download.FileDownloadWorker; import com.nextcloud.client.jobs.upload.FileUploadHelper; import com.nextcloud.client.network.ConnectivityService; import com.nextcloud.client.network.NetworkChangeListener; @@ -165,7 +164,6 @@ public abstract class FileActivity extends DrawerActivity private boolean mResumed; - protected FileDownloadWorker.FileDownloadProgressListener fileDownloadProgressListener; protected FileUploadHelper fileUploadHelper = FileUploadHelper.Companion.instance(); protected boolean isFileDisplayActivityResumed = false; @@ -632,11 +630,6 @@ public void onServiceDisconnected(ComponentName component) { } } - @Override - public FileDownloadWorker.FileDownloadProgressListener getFileDownloadProgressListener() { - return fileDownloadProgressListener; - } - @Override public FileUploadHelper getFileUploaderHelper() { return fileUploadHelper; diff --git a/app/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.kt b/app/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.kt index 27d5ebeea7b7..af7ac237f9d9 100644 --- a/app/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.kt +++ b/app/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.kt @@ -1914,8 +1914,6 @@ class FileDisplayActivity : updateFileDownloadIndicator(FileDownloadIndicator.Downloaded, it) } - fileDownloadProgressListener = null - if (fileIDForImmediatePreview == -1L) { Log_OC.d(TAG, "updating ui for file download") updateUIForFileDownload(intent) @@ -2164,7 +2162,6 @@ class FileDisplayActivity : override fun onServiceDisconnected(component: ComponentName) { if (component == ComponentName(this@FileDisplayActivity, FileDownloadWorker::class.java)) { Log_OC.d(TAG, "Download service disconnected") - fileDownloadProgressListener = null } } } diff --git a/app/src/main/java/com/owncloud/android/ui/events/EventBusFactory.kt b/app/src/main/java/com/owncloud/android/ui/events/EventBusFactory.kt deleted file mode 100644 index e0b5596974b2..000000000000 --- a/app/src/main/java/com/owncloud/android/ui/events/EventBusFactory.kt +++ /dev/null @@ -1,18 +0,0 @@ -/* - * Nextcloud - Android Client - * - * SPDX-FileCopyrightText: 2025 Alper Ozturk - * SPDX-License-Identifier: AGPL-3.0-or-later - */ - -package com.owncloud.android.ui.events - -import org.greenrobot.eventbus.EventBus - -object EventBusFactory { - - val downloadProgressEventBus: EventBus = EventBus.builder() - .logNoSubscriberMessages(false) - .sendNoSubscriberEvent(false) - .build() -} diff --git a/app/src/main/java/com/owncloud/android/ui/events/FileDownloadProgressEvent.kt b/app/src/main/java/com/owncloud/android/ui/events/FileDownloadProgressEvent.kt deleted file mode 100644 index 2995a128d4c1..000000000000 --- a/app/src/main/java/com/owncloud/android/ui/events/FileDownloadProgressEvent.kt +++ /dev/null @@ -1,10 +0,0 @@ -/* - * Nextcloud - Android Client - * - * SPDX-FileCopyrightText: 2025 Alper Ozturk - * SPDX-License-Identifier: AGPL-3.0-or-later - */ - -package com.owncloud.android.ui.events - -data class FileDownloadProgressEvent(val percent: Int) diff --git a/app/src/main/java/com/owncloud/android/ui/fragment/FileDetailFragment.java b/app/src/main/java/com/owncloud/android/ui/fragment/FileDetailFragment.java index 4813eb0d44c7..8c7dafe2de80 100644 --- a/app/src/main/java/com/owncloud/android/ui/fragment/FileDetailFragment.java +++ b/app/src/main/java/com/owncloud/android/ui/fragment/FileDetailFragment.java @@ -22,22 +22,25 @@ import com.google.android.material.chip.Chip; import com.google.android.material.floatingactionbutton.FloatingActionButton; import com.google.android.material.tabs.TabLayout; +import com.nextcloud.android.common.ui.theme.utils.ColorRole; import com.nextcloud.client.account.User; import com.nextcloud.client.account.UserAccountManager; import com.nextcloud.client.di.Injectable; +import com.nextcloud.client.di.ViewModelFactory; import com.nextcloud.client.jobs.BackgroundJobManager; import com.nextcloud.client.jobs.download.FileDownloadHelper; import com.nextcloud.client.jobs.upload.FileUploadHelper; import com.nextcloud.client.network.ClientFactory; import com.nextcloud.client.preferences.AppPreferences; -import com.nextcloud.ui.fileactions.FileAction; import com.nextcloud.ui.fileDetail.FileDetailPreview; +import com.nextcloud.ui.fileactions.FileAction; import com.nextcloud.ui.fileactions.FileActionsBottomSheet; import com.nextcloud.ui.tags.TagManagementBottomSheet; import com.nextcloud.utils.HumanReadableFormatter; import com.nextcloud.utils.MenuUtils; import com.nextcloud.utils.SnackbarUtil; import com.nextcloud.utils.extensions.BundleExtensionsKt; +import com.nextcloud.utils.extensions.LifecycleOwnerExtensionsKt; import com.nextcloud.utils.mdm.MDMConfig; import com.nextcloud.utils.text.DisplayTextFormatter; import com.nextcloud.utils.thumbnail.FolderThumbnailGenerator; @@ -60,9 +63,8 @@ import com.owncloud.android.ui.adapter.progressListener.DownloadProgressListener; import com.owncloud.android.ui.dialog.RemoveFilesDialogFragment; import com.owncloud.android.ui.dialog.RenameFileDialogFragment; -import com.owncloud.android.ui.events.EventBusFactory; import com.owncloud.android.ui.events.FavoriteEvent; -import com.owncloud.android.ui.events.FileDownloadProgressEvent; +import com.owncloud.android.ui.fragment.filedetail.FileDetailFragmentViewModel; import com.owncloud.android.utils.EncryptionUtils; import com.owncloud.android.utils.MimeTypeUtil; import com.owncloud.android.utils.theme.CapabilityUtils; @@ -82,6 +84,7 @@ import androidx.annotation.Nullable; import androidx.core.content.res.ResourcesCompat; import androidx.fragment.app.FragmentManager; +import androidx.lifecycle.ViewModelProvider; import androidx.viewpager2.widget.ViewPager2; /** @@ -105,6 +108,7 @@ public class FileDetailFragment extends FileFragment implements OnClickListener, private DownloadProgressListener progressListener; private ToolbarActivity toolbarActivity; private int activeTab; + private FileDetailFragmentViewModel viewModel; @Inject AppPreferences preferences; @Inject UserAccountManager accountManager; @@ -113,6 +117,7 @@ public class FileDetailFragment extends FileFragment implements OnClickListener, @Inject ViewThemeUtils viewThemeUtils; @Inject BackgroundJobManager backgroundJobManager; @Inject FolderThumbnailGenerator folderThumbnailGenerator; + @Inject ViewModelFactory vmFactory; /** * Public factory method to create new FileDetailFragment instances. @@ -199,6 +204,12 @@ public void goBackToOCFileListFragment() { requireActivity().getOnBackPressedDispatcher().onBackPressed(); } + @Override + public void onCreate(@Nullable Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + viewModel = new ViewModelProvider(this, vmFactory).get(FileDetailFragmentViewModel.class); + } + @Override public void onActivityCreated(Bundle savedInstanceState) { super.onActivityCreated(savedInstanceState); @@ -250,7 +261,7 @@ public View onCreateView(@NonNull LayoutInflater inflater, ViewGroup container, @Override public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceState) { if (getFile() != null && user != null) { - viewThemeUtils.platform.themeHorizontalProgressBar(binding.progressBar); + viewThemeUtils.material.colorProgressBar(binding.progressBar, ColorRole.PRIMARY); viewThemeUtils.platform.themeCheckbox(binding.folderSyncButton); progressListener = new DownloadProgressListener(binding.progressBar); binding.cancelBtn.setOnClickListener(this); @@ -260,8 +271,13 @@ public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceStat binding.folderSyncButton.setOnClickListener(this); updateFileDetails(false, false); + viewModel.observeDownloadProgress(user, getFile()); } + LifecycleOwnerExtensionsKt.collectWhenStarted(getViewLifecycleOwner(), + viewModel.getDownloadProgress(), + this::onDownloadProgress); + getChildFragmentManager().setFragmentResultListener( TagManagementBottomSheet.REQUEST_KEY, getViewLifecycleOwner(), @@ -434,7 +450,6 @@ public void onStart() { super.onStart(); listenForTransferProgress(); EventBus.getDefault().register(this); - EventBusFactory.INSTANCE.getDownloadProgressEventBus().register(this); } @Override @@ -458,7 +473,6 @@ public void onStop() { } EventBus.getDefault().unregister(this); - EventBusFactory.INSTANCE.getDownloadProgressEventBus().unregister(this); super.onStop(); } @@ -576,6 +590,9 @@ public void updateFileDetails(OCFile file, User user) { setFile(file); this.user = user; updateFileDetails(false, false); + if (viewModel != null && file != null && user != null) { + viewModel.observeDownloadProgress(user, file); + } } /** @@ -652,15 +669,13 @@ public void updateFileDetails(boolean transferring, boolean refresh) { } } - @Subscribe(threadMode = ThreadMode.MAIN) - public void onDownloadProgress(FileDownloadProgressEvent event) { + public void onDownloadProgress(int percent) { if (binding.progressBlock.getVisibility() != View.VISIBLE) { binding.progressBlock.setVisibility(View.VISIBLE); } binding.progressText.setText(R.string.downloader_download_in_progress_ticker); - binding.progressBar.setProgress(event.getPercent()); - binding.progressBar.invalidate(); + binding.progressBar.setProgressCompat(percent, true); } private void setFileModificationTimestamp(OCFile file, boolean showDetailedTimestamp) { @@ -752,11 +767,6 @@ public void listenForTransferProgress() { return; } - if (containerActivity.getFileDownloadProgressListener() != null) { - containerActivity.getFileDownloadProgressListener(). - addDataTransferProgressListener(progressListener, getFile()); - } - if (containerActivity.getFileUploaderHelper() != null) { OCFile file = getFile(); if (user == null || file == null) { @@ -770,10 +780,6 @@ public void listenForTransferProgress() { private void leaveTransferProgress() { if (progressListener != null) { - if (containerActivity.getFileDownloadProgressListener() != null) { - containerActivity.getFileDownloadProgressListener(). - removeDataTransferProgressListener(progressListener, getFile()); - } if (containerActivity.getFileUploaderHelper() != null) { OCFile file = getFile(); diff --git a/app/src/main/java/com/owncloud/android/ui/fragment/filedetail/FileDetailFragmentViewModel.kt b/app/src/main/java/com/owncloud/android/ui/fragment/filedetail/FileDetailFragmentViewModel.kt new file mode 100644 index 000000000000..3c854bd40926 --- /dev/null +++ b/app/src/main/java/com/owncloud/android/ui/fragment/filedetail/FileDetailFragmentViewModel.kt @@ -0,0 +1,42 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Alper Ozturk + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +package com.owncloud.android.ui.fragment.filedetail + +import androidx.lifecycle.ViewModel +import androidx.work.WorkInfo +import androidx.work.WorkManager +import com.nextcloud.client.account.User +import com.nextcloud.client.jobs.BackgroundJobManagerImpl +import com.nextcloud.client.jobs.download.FileDownloadWorker +import com.owncloud.android.datamodel.OCFile +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.filterNotNull +import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.mapNotNull +import javax.inject.Inject + +class FileDetailFragmentViewModel @Inject constructor(private val workManager: WorkManager) : ViewModel() { + + private val downloadWorkName = MutableStateFlow(null) + + @OptIn(ExperimentalCoroutinesApi::class) + val downloadProgress: Flow = downloadWorkName + .filterNotNull() + .flatMapLatest { workManager.getWorkInfosForUniqueWorkFlow(it) } + .mapNotNull { workInfos -> workInfos.firstOrNull { it.state == WorkInfo.State.RUNNING }?.downloadPercent() } + .distinctUntilChanged() + + fun observeDownloadProgress(user: User, file: OCFile) { + downloadWorkName.value = BackgroundJobManagerImpl.formatFileDownloadTag(user.accountName, file.fileId) + } + + private fun WorkInfo.downloadPercent(): Int? = progress.keyValueMap[FileDownloadWorker.PROGRESS_PERCENT] as? Int +} diff --git a/app/src/main/res/layout/file_details_fragment.xml b/app/src/main/res/layout/file_details_fragment.xml index aac508c36f68..80cb6a7420bb 100644 --- a/app/src/main/res/layout/file_details_fragment.xml +++ b/app/src/main/res/layout/file_details_fragment.xml @@ -147,13 +147,14 @@ android:textColor="@color/list_item_lastmod_and_filesize_text" android:text="@string/downloader_download_in_progress_ticker" /> - + android:layout_marginTop="@dimen/standard_quarter_margin" + app:indicatorColor="?attr/colorPrimary" + app:trackColor="?attr/colorSurfaceContainerHighest" />