From c0cd2ed449d24b806fd2511bb80a74609d92500a Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Tue, 29 Oct 2024 18:22:36 -0300 Subject: [PATCH] WIP: to test: Don't allow running app data check while backup is running --- .../seedvault/BackupStateManager.kt | 25 +++++++++++++------ .../seedvault/settings/SettingsFragment.kt | 14 +++++------ .../seedvault/settings/SettingsViewModel.kt | 22 ++++++++++++---- .../seedvault/worker/AppBackupWorker.kt | 7 ++++++ .../seedvault/worker/AppCheckerWorker.kt | 18 ++++++------- app/src/main/res/xml/settings.xml | 2 +- 6 files changed, 58 insertions(+), 30 deletions(-) diff --git a/app/src/main/java/com/stevesoltys/seedvault/BackupStateManager.kt b/app/src/main/java/com/stevesoltys/seedvault/BackupStateManager.kt index f9028dbe3..de1254f2f 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/BackupStateManager.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/BackupStateManager.kt @@ -16,6 +16,7 @@ import com.stevesoltys.seedvault.storage.StorageBackupService import com.stevesoltys.seedvault.transport.ConfigurableBackupTransportService import com.stevesoltys.seedvault.worker.AppBackupPruneWorker import com.stevesoltys.seedvault.worker.AppBackupWorker.Companion.UNIQUE_WORK_NAME +import com.stevesoltys.seedvault.worker.AppCheckerWorker import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.combine @@ -32,18 +33,28 @@ class BackupStateManager( flow = ConfigurableBackupTransportService.isRunning, flow2 = StorageBackupService.isRunning, flow3 = workManager.getWorkInfosForUniqueWorkFlow(UNIQUE_WORK_NAME), - flow4 = workManager.getWorkInfosForUniqueWorkFlow(AppBackupPruneWorker.UNIQUE_WORK_NAME), - ) { appBackupRunning, filesBackupRunning, workInfo1, workInfo2 -> + ) { appBackupRunning, filesBackupRunning, workInfo1 -> val workInfoState1 = workInfo1.getOrNull(0)?.state - val workInfoState2 = workInfo2.getOrNull(0)?.state Log.i( TAG, "appBackupRunning: $appBackupRunning, " + "filesBackupRunning: $filesBackupRunning, " + - "appBackupWorker: ${workInfoState1?.name}, " + - "pruneBackupWorker: ${workInfoState2?.name}" + "appBackupWorker: ${workInfoState1?.name}" ) - appBackupRunning || filesBackupRunning || - workInfoState1 == RUNNING || workInfoState2 == RUNNING + appBackupRunning || filesBackupRunning || workInfoState1 == RUNNING + } + + val isCheckOrPruneRunning: Flow = combine( + flow = workManager.getWorkInfosForUniqueWorkFlow(AppBackupPruneWorker.UNIQUE_WORK_NAME), + flow2 = workManager.getWorkInfosForUniqueWorkFlow(AppCheckerWorker.UNIQUE_WORK_NAME), + ) { pruneInfo, checkInfo -> + val pruneInfoState = pruneInfo.getOrNull(0)?.state + val checkInfoState = checkInfo.getOrNull(0)?.state + Log.i( + TAG, + "pruneBackupWorker: ${pruneInfoState?.name}, " + + "appCheckerWorker: ${checkInfoState?.name}" + ) + pruneInfoState == RUNNING || checkInfoState == RUNNING } val isAutoRestoreEnabled: Boolean diff --git a/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsFragment.kt b/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsFragment.kt index 26d2bc52a..32ed987a6 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsFragment.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsFragment.kt @@ -49,6 +49,7 @@ class SettingsFragment : PreferenceFragmentCompat() { private lateinit var backupLocation: Preference private lateinit var backupStatus: Preference private lateinit var backupScheduling: Preference + private lateinit var backupAppCheck: Preference private lateinit var backupStorage: TwoStatePreference private lateinit var backupRecoveryCode: Preference @@ -92,14 +93,8 @@ class SettingsFragment : PreferenceFragmentCompat() { backupLocation = findPreference("backup_location")!! backupLocation.setOnPreferenceClickListener { - if (viewModel.isBackupRunning.value) { - // don't allow changing backup destination while backup is running - // TODO we could show toast or snackbar here - false - } else { - viewModel.chooseBackupLocation() - true - } + viewModel.chooseBackupLocation() + true } autoRestore = findPreference(PREF_KEY_AUTO_RESTORE)!! @@ -117,6 +112,7 @@ class SettingsFragment : PreferenceFragmentCompat() { backupStatus = findPreference("backup_status")!! backupScheduling = findPreference("backup_scheduling")!! + backupAppCheck = findPreference("backup_app_check")!! backupStorage = findPreference("backup_storage")!! backupStorage.onPreferenceChangeListener = OnPreferenceChangeListener { _, newValue -> @@ -148,6 +144,8 @@ class SettingsFragment : PreferenceFragmentCompat() { viewModel.backupPossible.observe(viewLifecycleOwner) { possible -> toolbar.menu.findItem(R.id.action_backup)?.isEnabled = possible toolbar.menu.findItem(R.id.action_restore)?.isEnabled = possible + backupLocation.isEnabled = possible + backupAppCheck.isEnabled = possible } viewModel.lastBackupTime.observe(viewLifecycleOwner) { time -> diff --git a/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsViewModel.kt b/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsViewModel.kt index 8fc07fbca..45f5b76be 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsViewModel.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsViewModel.kt @@ -83,7 +83,8 @@ internal class SettingsViewModel( override val isRestoreOperation = false val isFirstStart get() = settingsManager.isFirstStart - val isBackupRunning: StateFlow + private val isBackupRunning: StateFlow + private val isCheckOrPruneRunning: StateFlow private val mBackupPossible = MutableLiveData(false) val backupPossible: LiveData = mBackupPossible @@ -144,12 +145,23 @@ internal class SettingsViewModel( started = SharingStarted.Eagerly, initialValue = false, ) + isCheckOrPruneRunning = backupStateManager.isCheckOrPruneRunning.stateIn( + scope = viewModelScope, + started = SharingStarted.Eagerly, + initialValue = false, + ) scope.launch { // update running state isBackupRunning.collect { onBackupRunningStateChanged() } } + scope.launch { + // update running state + isCheckOrPruneRunning.collect { + onBackupRunningStateChanged() + } + } onStoragePropertiesChanged() loadFilesSummary() } @@ -172,11 +184,11 @@ internal class SettingsViewModel( } private fun onBackupRunningStateChanged() { - if (isBackupRunning.value) mBackupPossible.postValue(false) - else viewModelScope.launch(Dispatchers.IO) { - val canDo = !isBackupRunning.value && !backendManager.isOnUnavailableUsb() + val backupAllowed = !isBackupRunning.value && !isCheckOrPruneRunning.value + if (backupAllowed) viewModelScope.launch(Dispatchers.IO) { + val canDo = !backendManager.isOnUnavailableUsb() mBackupPossible.postValue(canDo) - } + } else mBackupPossible.postValue(false) } private fun onStoragePropertiesChanged() { diff --git a/app/src/main/java/com/stevesoltys/seedvault/worker/AppBackupWorker.kt b/app/src/main/java/com/stevesoltys/seedvault/worker/AppBackupWorker.kt index 5ba281f66..5327ada6c 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/worker/AppBackupWorker.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/worker/AppBackupWorker.kt @@ -22,12 +22,14 @@ import androidx.work.OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST import androidx.work.PeriodicWorkRequestBuilder import androidx.work.WorkManager import androidx.work.WorkerParameters +import com.stevesoltys.seedvault.BackupStateManager import com.stevesoltys.seedvault.R import com.stevesoltys.seedvault.backend.BackendManager import com.stevesoltys.seedvault.repo.AppBackupManager import com.stevesoltys.seedvault.settings.SettingsManager import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager import com.stevesoltys.seedvault.ui.notification.NOTIFICATION_ID_OBSERVER +import kotlinx.coroutines.flow.first import org.koin.core.component.KoinComponent import org.koin.core.component.inject import java.util.concurrent.TimeUnit @@ -100,6 +102,7 @@ class AppBackupWorker( } } + private val backupStateManager: BackupStateManager by inject() private val backupRequester: BackupRequester by inject() private val settingsManager: SettingsManager by inject() private val apkBackupManager: ApkBackupManager by inject() @@ -109,6 +112,10 @@ class AppBackupWorker( override suspend fun doWork(): Result { Log.i(TAG, "Start worker $this ($id)") + if (backupStateManager.isCheckOrPruneRunning.first()) { + Log.i(TAG, "isCheckOrPruneRunning was true, so retrying later...") + return Result.retry() + } try { setForeground(createForegroundInfo()) } catch (e: Exception) { diff --git a/app/src/main/java/com/stevesoltys/seedvault/worker/AppCheckerWorker.kt b/app/src/main/java/com/stevesoltys/seedvault/worker/AppCheckerWorker.kt index e6fe566a0..4dc332fca 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/worker/AppCheckerWorker.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/worker/AppCheckerWorker.kt @@ -17,10 +17,12 @@ import androidx.work.OneTimeWorkRequestBuilder import androidx.work.OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST import androidx.work.WorkManager import androidx.work.WorkerParameters +import com.stevesoltys.seedvault.BackupStateManager import com.stevesoltys.seedvault.repo.Checker import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager import com.stevesoltys.seedvault.ui.notification.NOTIFICATION_ID_CHECKING import io.github.oshai.kotlinlogging.KotlinLogging +import kotlinx.coroutines.flow.first import org.koin.core.component.KoinComponent import org.koin.core.component.inject import java.time.Duration @@ -50,12 +52,16 @@ class AppCheckerWorker( } private val log = KotlinLogging.logger {} + private val backupStateManager: BackupStateManager by inject() private val checker: Checker by inject() private val nm: BackupNotificationManager by inject() override suspend fun doWork(): Result { - // TODO don't let backup/restore happen while we check log.info { "Start worker $this ($id)" } + if (backupStateManager.isBackupRunning.first()) { + Log.i(TAG, "isBackupRunning was true, so retrying later...") + return Result.retry() + } try { setForeground(createForegroundInfo()) } catch (e: Exception) { @@ -64,14 +70,8 @@ class AppCheckerWorker( val percent = inputData.getInt(PERCENT, -1) check(percent in 0..100) { "Percent $percent out of bounds." } - return try { - checker.check(percent) - Result.success() - } catch (e: Exception) { - // TODO maybe show error notification - log.error(e) { "Error while checking data: " } - Result.retry() - } + checker.check(percent) + return Result.success() } private fun createForegroundInfo() = ForegroundInfo( diff --git a/app/src/main/res/xml/settings.xml b/app/src/main/res/xml/settings.xml index 0ab94037c..5fc0eca05 100644 --- a/app/src/main/res/xml/settings.xml +++ b/app/src/main/res/xml/settings.xml @@ -53,7 +53,7 @@