From c1e998c51d608decd138860015e71fadc00cb66e Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Fri, 30 Oct 2026 04:26:29 +0530 Subject: [PATCH 1/7] feat(profiles): switch to a profile from the list Tapping a profile now asks before switching, since the app restarts. The switch goes through the safety checks, and if a sync or backup is running it says so instead of restarting. Assisted-by: Claude Opus 5.5 --- .../profiles/SwitchProfilesFragment.kt | 14 ++- .../profiles/SwitchProfilesScreen.kt | 48 +++++++++ .../profiles/SwitchProfilesViewModel.kt | 55 ++++++++++- .../profiles/SwitchProfilesScreenTest.kt | 68 +++++++++++-- .../profiles/SwitchProfilesViewModelTest.kt | 98 ++++++++++++++++++- .../src/main/res/values/03-dialogs.xml | 6 ++ 6 files changed, 277 insertions(+), 12 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesFragment.kt b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesFragment.kt index e84ccc100a90..6aab75963ca1 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesFragment.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesFragment.kt @@ -16,6 +16,7 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.ichi2.anki.AnkiDroidApp import com.ichi2.anki.CommonString import com.ichi2.anki.common.utils.android.showThemedToast +import com.ichi2.anki.multiprofile.safeRestartApp import com.ichi2.compose.theme.AnkiDroidTheme import timber.log.Timber @@ -42,7 +43,10 @@ class SwitchProfilesFragment : Fragment() { } return View(requireContext()) } - val viewModel: SwitchProfilesViewModel by viewModels { SwitchProfilesViewModel.factory(profileManager) } + val appContext = requireContext().applicationContext + val viewModel: SwitchProfilesViewModel by viewModels { + SwitchProfilesViewModel.factory(profileManager) { safeRestartApp(appContext) } + } return ComposeView(requireContext()).apply { setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed) setContent { @@ -50,17 +54,25 @@ class SwitchProfilesFragment : Fragment() { val profiles by viewModel.profiles.collectAsStateWithLifecycle() val isAddProfileDialogVisible by viewModel.isAddProfileDialogVisible.collectAsStateWithLifecycle() val profileBeingRenamed by viewModel.profileBeingRenamed.collectAsStateWithLifecycle() + val profileToSwitchTo by viewModel.profileToSwitchTo.collectAsStateWithLifecycle() + val isSwitchBlocked by viewModel.isSwitchBlocked.collectAsStateWithLifecycle() SwitchProfilesScreen( profiles = profiles, isAddProfileDialogVisible = isAddProfileDialogVisible, profileBeingRenamed = profileBeingRenamed, + profileToSwitchTo = profileToSwitchTo, + isSwitchBlocked = isSwitchBlocked, onNavigateUp = { requireActivity().onBackPressedDispatcher.onBackPressed() }, + onProfileClick = viewModel::requestSwitch, onAddProfileClick = viewModel::showAddProfileDialog, onAddProfileConfirm = viewModel::addProfile, onAddProfileDismiss = viewModel::dismissAddProfileDialog, onEditProfile = viewModel::showRenameProfileDialog, onRenameProfileConfirm = viewModel::renameProfile, onRenameProfileDismiss = viewModel::dismissRenameProfileDialog, + onSwitchProfileConfirm = viewModel::switchProfile, + onSwitchProfileDismiss = viewModel::dismissSwitchProfileDialog, + onSwitchBlockedDismiss = viewModel::dismissSwitchBlockedMessage, onDeleteProfile = viewModel::deleteProfile, ) } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesScreen.kt b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesScreen.kt index 71d2425ddc07..b35fecc73c67 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesScreen.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesScreen.kt @@ -4,6 +4,7 @@ package com.ichi2.anki.preferences.profiles import androidx.compose.foundation.background +import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxSize @@ -13,12 +14,14 @@ import androidx.compose.foundation.layout.size import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.items import androidx.compose.foundation.shape.CircleShape +import androidx.compose.material3.AlertDialog import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Scaffold import androidx.compose.material3.Text +import androidx.compose.material3.TextButton import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment @@ -49,13 +52,19 @@ fun SwitchProfilesScreen( profiles: List, isAddProfileDialogVisible: Boolean, profileBeingRenamed: ProfileItem?, + profileToSwitchTo: ProfileItem?, + isSwitchBlocked: Boolean, onNavigateUp: () -> Unit, + onProfileClick: (ProfileItem) -> Unit, onAddProfileClick: () -> Unit, onAddProfileConfirm: (ProfileName) -> Unit, onAddProfileDismiss: () -> Unit, onEditProfile: (ProfileItem) -> Unit, onRenameProfileConfirm: (ProfileName) -> Unit, onRenameProfileDismiss: () -> Unit, + onSwitchProfileConfirm: () -> Unit, + onSwitchProfileDismiss: () -> Unit, + onSwitchBlockedDismiss: () -> Unit, onDeleteProfile: (ProfileItem) -> Unit, modifier: Modifier = Modifier, ) { @@ -91,6 +100,7 @@ fun SwitchProfilesScreen( items(profiles, key = { it.id.value }) { profile -> ProfileRow( profile = profile, + onClick = { onProfileClick(profile) }, onEditClick = { onEditProfile(profile) }, onDeleteClick = { onDeleteProfile(profile) }, ) @@ -114,6 +124,36 @@ fun SwitchProfilesScreen( onConfirm = onRenameProfileConfirm, ) } + + profileToSwitchTo?.let { profile -> + AlertDialog( + onDismissRequest = onSwitchProfileDismiss, + title = { Text(stringResource(CommonString.switch_profile_title, profile.name)) }, + text = { Text(stringResource(CommonString.switch_profile_message)) }, + confirmButton = { + TextButton(onClick = onSwitchProfileConfirm) { + Text(stringResource(CommonString.switch_profile_confirm)) + } + }, + dismissButton = { + TextButton(onClick = onSwitchProfileDismiss) { + Text(stringResource(CommonString.dialog_cancel)) + } + }, + ) + } + + if (isSwitchBlocked) { + AlertDialog( + onDismissRequest = onSwitchBlockedDismiss, + text = { Text(stringResource(CommonString.switch_profile_blocked)) }, + confirmButton = { + TextButton(onClick = onSwitchBlockedDismiss) { + Text(stringResource(CommonString.dialog_ok)) + } + }, + ) + } } private val AvatarSize = 40.dp @@ -121,6 +161,7 @@ private val AvatarSize = 40.dp @Composable private fun ProfileRow( profile: ProfileItem, + onClick: () -> Unit, onEditClick: () -> Unit, onDeleteClick: () -> Unit, modifier: Modifier = Modifier, @@ -129,6 +170,7 @@ private fun ProfileRow( modifier = modifier .fillMaxWidth() + .clickable(onClick = onClick) .padding( horizontal = MaterialTheme.dimensions.space200, vertical = MaterialTheme.dimensions.space100, @@ -190,13 +232,19 @@ private fun SwitchProfilesScreenPreview() { ), isAddProfileDialogVisible = false, profileBeingRenamed = null, + profileToSwitchTo = null, + isSwitchBlocked = false, onNavigateUp = {}, + onProfileClick = {}, onAddProfileClick = {}, onAddProfileConfirm = {}, onAddProfileDismiss = {}, onEditProfile = {}, onRenameProfileConfirm = {}, onRenameProfileDismiss = {}, + onSwitchProfileConfirm = {}, + onSwitchProfileDismiss = {}, + onSwitchBlockedDismiss = {}, onDeleteProfile = {}, ) } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModel.kt index 6d961146d03e..d16d41e8560d 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModel.kt @@ -5,12 +5,20 @@ package com.ichi2.anki.preferences.profiles import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModelProvider +import androidx.lifecycle.viewModelScope import androidx.lifecycle.viewmodel.initializer import androidx.lifecycle.viewmodel.viewModelFactory +import com.ichi2.anki.ioDispatcher import com.ichi2.anki.multiprofile.ProfileManager import com.ichi2.anki.multiprofile.ProfileName +import com.ichi2.anki.multiprofile.ProfileSwitchGuard +import com.ichi2.anki.multiprofile.backupCheck +import com.ichi2.anki.multiprofile.mediaSyncCheck +import com.ichi2.anki.multiprofile.syncCheck import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import timber.log.Timber /** @@ -19,6 +27,8 @@ import timber.log.Timber */ class SwitchProfilesViewModel( private val profileManager: ProfileManager, + private val switchGuard: ProfileSwitchGuard, + private val restartApp: suspend () -> Unit, ) : ViewModel() { /** Profiles shown in the list. */ val profiles: StateFlow> @@ -30,6 +40,12 @@ class SwitchProfilesViewModel( val profileBeingRenamed: StateFlow field = MutableStateFlow(null) + val profileToSwitchTo: StateFlow + field = MutableStateFlow(null) + + val isSwitchBlocked: StateFlow + field = MutableStateFlow(false) + fun showAddProfileDialog() { isAddProfileDialogVisible.value = true } @@ -62,6 +78,34 @@ class SwitchProfilesViewModel( refresh() } + fun requestSwitch(profile: ProfileItem) { + if (profile.id == profileManager.activeProfileContext.profileId) return + profileToSwitchTo.value = profile + } + + fun dismissSwitchProfileDialog() { + profileToSwitchTo.value = null + } + + fun switchProfile() { + val profile = profileToSwitchTo.value ?: return + profileToSwitchTo.value = null + Timber.i("Switch profile confirmed: %s", profile.id) + viewModelScope.launch { + when (val result = withContext(ioDispatcher) { switchGuard(profile.id) }) { + ProfileSwitchGuard.Result.Success -> restartApp() + is ProfileSwitchGuard.Result.Blocked -> { + Timber.i("Profile switch blocked: %s", result.reasons) + isSwitchBlocked.value = true + } + } + } + } + + fun dismissSwitchBlockedMessage() { + isSwitchBlocked.value = false + } + fun deleteProfile(profile: ProfileItem) { Timber.i("Delete profile requested: %s", profile.id) // TODO: implement profile deletion via ProfileManager @@ -73,10 +117,17 @@ class SwitchProfilesViewModel( } companion object { - fun factory(profileManager: ProfileManager): ViewModelProvider.Factory = + fun factory( + profileManager: ProfileManager, + restartApp: suspend () -> Unit, + ): ViewModelProvider.Factory = viewModelFactory { initializer { - SwitchProfilesViewModel(profileManager) + SwitchProfilesViewModel( + profileManager = profileManager, + switchGuard = ProfileSwitchGuard(profileManager, listOf(syncCheck(), mediaSyncCheck(), backupCheck())), + restartApp = restartApp, + ) } } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesScreenTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesScreenTest.kt index f0ebec799745..c5ba602b95a7 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesScreenTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesScreenTest.kt @@ -3,6 +3,7 @@ package com.ichi2.anki.preferences.profiles +import androidx.compose.ui.test.assertIsDisplayed import androidx.compose.ui.test.hasSetTextAction import androidx.compose.ui.test.junit4.v2.createComposeRule import androidx.compose.ui.test.onNodeWithText @@ -18,37 +19,88 @@ import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import kotlin.test.assertEquals +import kotlin.test.assertTrue @RunWith(AndroidJUnit4::class) class SwitchProfilesScreenTest : RobolectricTest() { @get:Rule val composeRule = createComposeRule() + private val default = ProfileItem(id = ProfileId.DEFAULT, name = "Default") + @Test fun `a profile can be renamed to a different case of its own name`() { val work = ProfileItem(id = ProfileId("p_work"), name = "work") var renamedTo: ProfileName? = null + setScreen(profiles = listOf(default, work), profileBeingRenamed = work, onRenameProfileConfirm = { renamedTo = it }) + + composeRule.onNode(hasSetTextAction()).performTextReplacement("Work") + composeRule.onNodeWithText(targetContext.getString(CommonString.rename)).performClick() + + assertEquals("Work", renamedTo?.value) + } + + @Test + fun `tapping a profile asks to switch to it`() { + val work = ProfileItem(id = ProfileId("p_work"), name = "Work") + var tapped: ProfileItem? = null + setScreen(profiles = listOf(default, work), onProfileClick = { tapped = it }) + + composeRule.onNodeWithText("Work").performClick() + + assertEquals(work, tapped) + } + + @Test + fun `confirming the switch dialog switches the profile`() { + val work = ProfileItem(id = ProfileId("p_work"), name = "Work") + var confirmed = false + setScreen(profiles = listOf(default, work), profileToSwitchTo = work, onSwitchProfileConfirm = { confirmed = true }) + + composeRule.onNodeWithText(targetContext.getString(CommonString.switch_profile_title, "Work")).assertIsDisplayed() + composeRule.onNodeWithText(targetContext.getString(CommonString.switch_profile_confirm)).performClick() + + assertTrue(confirmed) + } + + @Test + fun `a blocked switch explains why`() { + setScreen(profiles = listOf(default), isSwitchBlocked = true) + + composeRule.onNodeWithText(targetContext.getString(CommonString.switch_profile_blocked)).assertIsDisplayed() + } + + private fun setScreen( + profiles: List, + profileBeingRenamed: ProfileItem? = null, + profileToSwitchTo: ProfileItem? = null, + isSwitchBlocked: Boolean = false, + onProfileClick: (ProfileItem) -> Unit = {}, + onRenameProfileConfirm: (ProfileName) -> Unit = {}, + onSwitchProfileConfirm: () -> Unit = {}, + ) { composeRule.setContent { AnkiDroidTheme { SwitchProfilesScreen( - profiles = listOf(ProfileItem(id = ProfileId.DEFAULT, name = "Default"), work), + profiles = profiles, isAddProfileDialogVisible = false, - profileBeingRenamed = work, + profileBeingRenamed = profileBeingRenamed, + profileToSwitchTo = profileToSwitchTo, + isSwitchBlocked = isSwitchBlocked, onNavigateUp = {}, + onProfileClick = onProfileClick, onAddProfileClick = {}, onAddProfileConfirm = {}, onAddProfileDismiss = {}, onEditProfile = {}, - onRenameProfileConfirm = { renamedTo = it }, + onRenameProfileConfirm = onRenameProfileConfirm, onRenameProfileDismiss = {}, + onSwitchProfileConfirm = onSwitchProfileConfirm, + onSwitchProfileDismiss = {}, + onSwitchBlockedDismiss = {}, onDeleteProfile = {}, ) } } - - composeRule.onNode(hasSetTextAction()).performTextReplacement("Work") - composeRule.onNodeWithText(targetContext.getString(CommonString.rename)).performClick() - - assertEquals("Work", renamedTo?.value) } } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModelTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModelTest.kt index 79a6b653a266..fc79524ac7ba 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModelTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModelTest.kt @@ -7,11 +7,20 @@ import android.content.Context import androidx.core.content.edit import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.ichi2.anki.ioDispatcher import com.ichi2.anki.multiprofile.ProfileId import com.ichi2.anki.multiprofile.ProfileManager +import com.ichi2.anki.multiprofile.ProfileManager.Companion.PREF_LAST_ACTIVE_PROFILE_ID import com.ichi2.anki.multiprofile.ProfileManager.Companion.PROFILE_REGISTRY_FILENAME import com.ichi2.anki.multiprofile.ProfileName import com.ichi2.anki.multiprofile.ProfileName.ValidationResult +import com.ichi2.anki.multiprofile.ProfileSwitchGuard +import com.ichi2.anki.multiprofile.ProfileSwitchGuard.BlockReason +import com.ichi2.anki.multiprofile.ProfileSwitchGuard.SafetyCheck +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.setMain import org.junit.After import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue @@ -29,8 +38,13 @@ class SwitchProfilesViewModelTest { private val prefs get() = context.getSharedPreferences(PROFILE_REGISTRY_FILENAME, Context.MODE_PRIVATE) + private var restarts = 0 + @Before fun setUp() { + val dispatcher = UnconfinedTestDispatcher() + Dispatchers.setMain(dispatcher) + ioDispatcher = dispatcher context = ApplicationProvider.getApplicationContext() prefs.edit(commit = true) { clear() } profileManager = ProfileManager.create(context) @@ -39,9 +53,19 @@ class SwitchProfilesViewModelTest { @After fun tearDown() { prefs.edit(commit = true) { clear() } + Dispatchers.resetMain() + ioDispatcher = Dispatchers.IO } - private fun viewModel() = SwitchProfilesViewModel(profileManager) + private fun viewModel(vararg checks: SafetyCheck) = + SwitchProfilesViewModel( + profileManager = profileManager, + switchGuard = ProfileSwitchGuard(profileManager, checks.toList()), + restartApp = { restarts++ }, + ) + + private val lastActiveProfileId: String? + get() = prefs.getString(PREF_LAST_ACTIVE_PROFILE_ID, null) private val validName: ProfileName get() = (ProfileName.validate("Work") as ValidationResult.Valid).name @@ -119,6 +143,78 @@ class SwitchProfilesViewModelTest { assertNull(viewModel.profileBeingRenamed.value) } + @Test + fun `tapping another profile asks before switching to it`() { + createProfile("Work") + val viewModel = viewModel() + val work = viewModel.profiles.value.single { it.name == "Work" } + + viewModel.requestSwitch(work) + + assertEquals(work, viewModel.profileToSwitchTo.value) + } + + @Test + fun `tapping the active profile does not ask to switch`() { + val viewModel = viewModel() + + viewModel.requestSwitch(viewModel.profiles.value.single()) + + assertNull(viewModel.profileToSwitchTo.value) + } + + @Test + fun `dismissing the switch dialog keeps the current profile`() { + createProfile("Work") + val viewModel = viewModel() + viewModel.requestSwitch(viewModel.profiles.value.single { it.name == "Work" }) + + viewModel.dismissSwitchProfileDialog() + + assertNull(viewModel.profileToSwitchTo.value) + assertEquals(ProfileId.DEFAULT.value, lastActiveProfileId) + assertEquals(0, restarts) + } + + @Test + fun `confirming a switch saves the profile and restarts the app`() { + val work = createProfile("Work") + val viewModel = viewModel() + viewModel.requestSwitch(viewModel.profiles.value.single { it.id == work }) + + viewModel.switchProfile() + + assertNull(viewModel.profileToSwitchTo.value) + assertEquals(work.value, lastActiveProfileId) + assertEquals(1, restarts) + } + + @Test + fun `a blocked switch keeps the current profile and says why`() { + val work = createProfile("Work") + val viewModel = viewModel(SafetyCheck { BlockReason.SYNC_IN_PROGRESS }) + viewModel.requestSwitch(viewModel.profiles.value.single { it.id == work }) + + viewModel.switchProfile() + + assertTrue(viewModel.isSwitchBlocked.value) + assertEquals(ProfileId.DEFAULT.value, lastActiveProfileId) + assertEquals(0, restarts) + } + + @Test + fun `dismissing the blocked message hides it`() { + val viewModel = viewModel(SafetyCheck { BlockReason.BACKUP_IN_PROGRESS }) + createProfile("Work") + viewModel.refresh() + viewModel.requestSwitch(viewModel.profiles.value.single { it.name == "Work" }) + viewModel.switchProfile() + + viewModel.dismissSwitchBlockedMessage() + + assertFalse(viewModel.isSwitchBlocked.value) + } + @Test fun `the list starts with the Default profile`() { assertEquals(listOf(ProfileId.DEFAULT), viewModel().profiles.value.map { it.id }) diff --git a/common/android/src/main/res/values/03-dialogs.xml b/common/android/src/main/res/values/03-dialogs.xml index 33106a563179..dadd84326793 100644 --- a/common/android/src/main/res/values/03-dialogs.xml +++ b/common/android/src/main/res/values/03-dialogs.xml @@ -302,4 +302,10 @@ also changes the interval of the card" Profile name Profile name can’t be empty Profile name must be %1$d characters or fewer + + + Switch to “%1$s”? + AnkiDroid will restart to open this profile. + Switch + AnkiDroid is syncing or backing up. Try again when it finishes. From 3497f5d797830b8334c26e2737571134dfc5b722 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:23:26 +0530 Subject: [PATCH 2/7] fix(profiles): finish a confirmed switch, one at a time Leaving the screen right after confirming could cancel a switch that was already saved, so the new profile only opened at the next start. A confirmed switch now always reaches the restart, and other profiles can't be picked until it does, even from a reopened screen, so a second switch can't overwrite it. Co-authored-by: David Allison <62114487+david-allison@users.noreply.github.com> Assisted-by: Claude Opus 5.5 --- .../ichi2/anki/multiprofile/ProfileManager.kt | 3 + .../profiles/SwitchProfilesViewModel.kt | 20 ++-- .../SwitchProfilesReentryRegressionTest.kt | 87 +++++++++++++++++ .../profiles/SwitchProfilesReviewTest.kt | 69 ++++++++++++++ .../profiles/SwitchProfilesThirdReviewTest.kt | 95 +++++++++++++++++++ .../profiles/SwitchProfilesViewModelTest.kt | 74 +++++++++++++++ 6 files changed, 341 insertions(+), 7 deletions(-) create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesReentryRegressionTest.kt create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesReviewTest.kt create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesThirdReviewTest.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileManager.kt b/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileManager.kt index e502d5c7a626..c0d180a59d79 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileManager.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileManager.kt @@ -21,6 +21,7 @@ import com.ichi2.anki.startup.getDefaultAnkiDroidDirectory import org.json.JSONObject import timber.log.Timber import java.io.File +import java.util.concurrent.atomic.AtomicBoolean /** * Manages the creation, loading, and switching of user profiles. @@ -34,6 +35,8 @@ class ProfileManager private constructor( lateinit var activeProfileContext: ProfileContextWrapper private set + val isSwitching = AtomicBoolean(false) + /** * Stores the Registry of all profiles (ID -> Display Name) and the * ID of the currently active profile. diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModel.kt index d16d41e8560d..c80abbbc5b05 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModel.kt @@ -15,6 +15,8 @@ import com.ichi2.anki.multiprofile.ProfileSwitchGuard import com.ichi2.anki.multiprofile.backupCheck import com.ichi2.anki.multiprofile.mediaSyncCheck import com.ichi2.anki.multiprofile.syncCheck +import kotlinx.coroutines.CoroutineStart +import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.launch @@ -79,7 +81,7 @@ class SwitchProfilesViewModel( } fun requestSwitch(profile: ProfileItem) { - if (profile.id == profileManager.activeProfileContext.profileId) return + if (profileManager.isSwitching.get() || profile.id == profileManager.activeProfileContext.profileId) return profileToSwitchTo.value = profile } @@ -90,13 +92,17 @@ class SwitchProfilesViewModel( fun switchProfile() { val profile = profileToSwitchTo.value ?: return profileToSwitchTo.value = null + if (!profileManager.isSwitching.compareAndSet(false, true)) return Timber.i("Switch profile confirmed: %s", profile.id) - viewModelScope.launch { - when (val result = withContext(ioDispatcher) { switchGuard(profile.id) }) { - ProfileSwitchGuard.Result.Success -> restartApp() - is ProfileSwitchGuard.Result.Blocked -> { - Timber.i("Profile switch blocked: %s", result.reasons) - isSwitchBlocked.value = true + viewModelScope.launch(start = CoroutineStart.UNDISPATCHED) { + withContext(NonCancellable) { + when (val result = withContext(ioDispatcher) { switchGuard(profile.id) }) { + ProfileSwitchGuard.Result.Success -> restartApp() + is ProfileSwitchGuard.Result.Blocked -> { + Timber.i("Profile switch blocked: %s", result.reasons) + profileManager.isSwitching.set(false) + isSwitchBlocked.value = true + } } } } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesReentryRegressionTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesReentryRegressionTest.kt new file mode 100644 index 000000000000..61b3141ff438 --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesReentryRegressionTest.kt @@ -0,0 +1,87 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package com.ichi2.anki.preferences.profiles + +import android.content.Context +import androidx.lifecycle.ViewModelStore +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.ichi2.anki.AnkiDroidApp +import com.ichi2.anki.RobolectricTest +import com.ichi2.anki.ioDispatcher +import com.ichi2.anki.multiprofile.ProfileManager +import com.ichi2.anki.multiprofile.ProfileName +import com.ichi2.anki.multiprofile.ProfileSwitchGuard +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestCoroutineScheduler +import kotlinx.coroutines.test.setMain +import org.junit.Test +import org.junit.runner.RunWith +import kotlin.coroutines.CoroutineContext +import kotlin.test.assertEquals + +@RunWith(AndroidJUnit4::class) +class SwitchProfilesReentryRegressionTest : RobolectricTest() { + @Test + fun `reopening the screen cannot let an older switch replace a later confirmation`() { + val mainScheduler = TestCoroutineScheduler() + Dispatchers.setMain(StandardTestDispatcher(mainScheduler)) + val queuedIo = QueuedDispatcher() + ioDispatcher = queuedIo + val app = AnkiDroidApp.instance + val manager = requireNotNull(app.profileManager) + val work = manager.createNewProfile(name("Work")) + val personal = manager.createNewProfile(name("Personal")) + val registry = app.baseContext.getSharedPreferences(ProfileManager.PROFILE_REGISTRY_FILENAME, Context.MODE_PRIVATE) + val profilesAtRestart = mutableListOf() + + fun model() = + SwitchProfilesViewModel(manager, ProfileSwitchGuard(manager, emptyList())) { + profilesAtRestart += registry.getString(ProfileManager.PREF_LAST_ACTIVE_PROFILE_ID, null) + } + val firstStore = ViewModelStore() + val secondStore = ViewModelStore() + try { + val first = model().also { firstStore.put("profiles", it) } + first.requestSwitch(first.profiles.value.single { it.id == work }) + first.switchProfile() + firstStore.clear() // Back navigation clears the first Fragment's ViewModelStore. + + val second = model().also { secondStore.put("profiles", it) } + second.requestSwitch(second.profiles.value.single { it.id == personal }) + // Refusing another switch while one is pending is also a valid outcome. + val lastConfirmed = second.profileToSwitchTo.value?.id ?: work + second.switchProfile() + + // IO is a pool: the newer save may finish before the older one. + while (queuedIo.tasks.isNotEmpty()) queuedIo.tasks.removeLast().run() + mainScheduler.runCurrent() + + assertEquals( + lastConfirmed.value, + registry.getString(ProfileManager.PREF_LAST_ACTIVE_PROFILE_ID, null), + "The original screen overwrote the last confirmation after it was destroyed", + ) + assertEquals>(listOf(lastConfirmed.value), profilesAtRestart, "Only one process handover should be requested") + } finally { + firstStore.clear() + secondStore.clear() + while (queuedIo.tasks.isNotEmpty()) queuedIo.tasks.removeFirst().run() + mainScheduler.runCurrent() + } + } + + private fun name(value: String) = (ProfileName.validate(value) as ProfileName.ValidationResult.Valid).name + + private class QueuedDispatcher : CoroutineDispatcher() { + val tasks = ArrayDeque() + + override fun dispatch( + context: CoroutineContext, + block: Runnable, + ) { + tasks.addLast(block) + } + } +} diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesReviewTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesReviewTest.kt new file mode 100644 index 000000000000..5c47f2f84134 --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesReviewTest.kt @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package com.ichi2.anki.preferences.profiles + +import android.content.Context +import androidx.core.content.edit +import androidx.lifecycle.ViewModelStore +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.ichi2.anki.ioDispatcher +import com.ichi2.anki.multiprofile.ProfileId +import com.ichi2.anki.multiprofile.ProfileManager +import com.ichi2.anki.multiprofile.ProfileName +import com.ichi2.anki.multiprofile.ProfileSwitchGuard +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestCoroutineScheduler +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.setMain +import org.junit.Test +import org.junit.runner.RunWith +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +@RunWith(AndroidJUnit4::class) +class SwitchProfilesReviewTest { + @Test + fun `leaving the screen cannot abandon a committed profile switch`() { + val mainScheduler = TestCoroutineScheduler() + val ioScheduler = TestCoroutineScheduler() + Dispatchers.setMain(StandardTestDispatcher(mainScheduler)) + ioDispatcher = StandardTestDispatcher(ioScheduler) + val context = ApplicationProvider.getApplicationContext() + val prefs = context.getSharedPreferences(ProfileManager.PROFILE_REGISTRY_FILENAME, Context.MODE_PRIVATE) + prefs.edit(commit = true) { clear() } + val store = ViewModelStore() + try { + val manager = ProfileManager.create(context) + val name = (ProfileName.validate("Work") as ProfileName.ValidationResult.Valid).name + val target = manager.createNewProfile(name) + var restarts = 0 + val model = SwitchProfilesViewModel(manager, ProfileSwitchGuard(manager, emptyList())) { restarts++ } + store.put("profiles", model) + model.requestSwitch(model.profiles.value.single { it.id == target }) + model.switchProfile() + + mainScheduler.runCurrent() // Dispatch the guard to IO. + ioScheduler.runCurrent() // Commit the new profile; queue the continuation on Main. + assertEquals(target.value, prefs.getString(ProfileManager.PREF_LAST_ACTIVE_PROFILE_ID, null)) + assertEquals(0, restarts) + + store.clear() // Back navigation destroys the Fragment and clears its ViewModelStore. + mainScheduler.runCurrent() + ioScheduler.runCurrent() + + val persisted = prefs.getString(ProfileManager.PREF_LAST_ACTIVE_PROFILE_ID, null) + assertEquals(ProfileId.DEFAULT, manager.activeProfileContext.profileId) + assertTrue( + restarts == 1 || persisted == ProfileId.DEFAULT.value, + "Switch abandoned: persisted=$persisted, running=${manager.activeProfileContext.profileId}, restarts=$restarts", + ) + } finally { + store.clear() + prefs.edit(commit = true) { clear() } + Dispatchers.resetMain() + ioDispatcher = Dispatchers.IO + } + } +} diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesThirdReviewTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesThirdReviewTest.kt new file mode 100644 index 000000000000..26e65a0eeb88 --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesThirdReviewTest.kt @@ -0,0 +1,95 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package com.ichi2.anki.preferences.profiles + +import android.content.Context +import androidx.lifecycle.ViewModelStore +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.ichi2.anki.AnkiDroidApp +import com.ichi2.anki.RobolectricTest +import com.ichi2.anki.backupLock +import com.ichi2.anki.ioDispatcher +import com.ichi2.anki.multiprofile.ProfileManager +import com.ichi2.anki.multiprofile.ProfileName +import com.ichi2.anki.multiprofile.ProfileSwitchGuard +import com.ichi2.anki.multiprofile.backupCheck +import com.ichi2.anki.multiprofile.mediaSyncCheck +import com.ichi2.anki.multiprofile.safeRestartApp +import com.ichi2.anki.multiprofile.syncCheck +import com.ichi2.anki.worker.mediaSyncLock +import com.ichi2.anki.worker.syncLock +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestCoroutineScheduler +import kotlinx.coroutines.test.setMain +import org.junit.Test +import org.junit.runner.RunWith +import kotlin.coroutines.CoroutineContext +import kotlin.test.assertEquals +import kotlin.test.assertNull + +@RunWith(AndroidJUnit4::class) +class SwitchProfilesThirdReviewTest : RobolectricTest() { + private fun name(value: String) = (ProfileName.validate(value) as ProfileName.ValidationResult.Valid).name + + @Test + fun `an older switch cannot overwrite the last confirmed profile`() { + val mainScheduler = TestCoroutineScheduler() + Dispatchers.setMain(StandardTestDispatcher(mainScheduler)) + val queuedIo = QueuedDispatcher() + ioDispatcher = queuedIo + val app = AnkiDroidApp.instance + val manager = requireNotNull(app.profileManager) + val work = manager.createNewProfile(name("Work")) + val personal = manager.createNewProfile(name("Personal")) + val registry = app.baseContext.getSharedPreferences(ProfileManager.PROFILE_REGISTRY_FILENAME, Context.MODE_PRIVATE) + val store = ViewModelStore() + var restarts = 0 + val model = + SwitchProfilesViewModel( + manager, + ProfileSwitchGuard(manager, listOf(syncCheck(), mediaSyncCheck(), backupCheck())), + ) { + restarts++ + safeRestartApp(app) + } + store.put("profiles", model) + try { + model.requestSwitch(model.profiles.value.single { it.id == work }) + model.switchProfile() + mainScheduler.runCurrent() + + model.requestSwitch(model.profiles.value.single { it.id == personal }) + assertNull(model.profileToSwitchTo.value, "Personal must not be offered while the switch to Work is saving") + model.switchProfile() + mainScheduler.runCurrent() + assertEquals(1, queuedIo.tasks.size, "only the first confirmation was accepted") + + queuedIo.tasks.removeFirst().run() + mainScheduler.runCurrent() + + assertEquals( + work.value, + registry.getString(ProfileManager.PREF_LAST_ACTIVE_PROFILE_ID, null), + "Work was confirmed and Personal was never offered; restarts entered=$restarts", + ) + assertEquals(1, restarts) + } finally { + store.clear() + mainScheduler.runCurrent() + listOf(syncLock, mediaSyncLock, backupLock).filter { it.isLocked }.forEach { it.unlock() } + } + } + + private class QueuedDispatcher : CoroutineDispatcher() { + val tasks = ArrayDeque() + + override fun dispatch( + context: CoroutineContext, + block: Runnable, + ) { + tasks.addLast(block) + } + } +} diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModelTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModelTest.kt index fc79524ac7ba..2543f84a4a92 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModelTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModelTest.kt @@ -5,6 +5,7 @@ package com.ichi2.anki.preferences.profiles import android.content.Context import androidx.core.content.edit +import androidx.lifecycle.ViewModelStore import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import com.ichi2.anki.ioDispatcher @@ -17,6 +18,7 @@ import com.ichi2.anki.multiprofile.ProfileName.ValidationResult import com.ichi2.anki.multiprofile.ProfileSwitchGuard import com.ichi2.anki.multiprofile.ProfileSwitchGuard.BlockReason import com.ichi2.anki.multiprofile.ProfileSwitchGuard.SafetyCheck +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.resetMain @@ -72,6 +74,12 @@ class SwitchProfilesViewModelTest { private fun createProfile(name: String) = profileManager.createNewProfile(nameOf(name)) + private fun waitingFor(saving: CompletableDeferred) = + SafetyCheck { + saving.await() + null + } + private fun nameOf(raw: String) = (ProfileName.validate(raw) as ValidationResult.Valid).name @Test @@ -202,6 +210,72 @@ class SwitchProfilesViewModelTest { assertEquals(0, restarts) } + @Test + fun `another profile cannot be picked while a switch is saving`() { + val work = createProfile("Work") + createProfile("Personal") + val saving = CompletableDeferred() + val viewModel = viewModel(waitingFor(saving)) + viewModel.requestSwitch(viewModel.profiles.value.single { it.id == work }) + viewModel.switchProfile() + + viewModel.requestSwitch(viewModel.profiles.value.single { it.name == "Personal" }) + saving.complete(Unit) + + assertNull(viewModel.profileToSwitchTo.value, "the app is about to restart into Work") + assertEquals(work.value, lastActiveProfileId) + assertEquals(1, restarts) + } + + @Test + fun `confirming twice switches once`() { + val work = createProfile("Work") + var checks = 0 + val viewModel = + viewModel( + SafetyCheck { + checks++ + null + }, + ) + viewModel.requestSwitch(viewModel.profiles.value.single { it.id == work }) + + viewModel.switchProfile() + viewModel.switchProfile() + + assertEquals(1, checks) + assertEquals(1, restarts) + } + + @Test + fun `another profile can be picked after a blocked switch`() { + createProfile("Work") + val personal = createProfile("Personal") + val viewModel = viewModel(SafetyCheck { BlockReason.SYNC_IN_PROGRESS }) + viewModel.requestSwitch(viewModel.profiles.value.single { it.name == "Work" }) + viewModel.switchProfile() + + viewModel.requestSwitch(viewModel.profiles.value.single { it.id == personal }) + + assertEquals(personal, viewModel.profileToSwitchTo.value?.id) + } + + @Test + fun `leaving the screen while a switch is saving still restarts into it`() { + val work = createProfile("Work") + val saving = CompletableDeferred() + val store = ViewModelStore() + val viewModel = viewModel(waitingFor(saving)).also { store.put("profiles", it) } + viewModel.requestSwitch(viewModel.profiles.value.single { it.id == work }) + viewModel.switchProfile() + + store.clear() + saving.complete(Unit) + + assertEquals(work.value, lastActiveProfileId) + assertEquals(1, restarts) + } + @Test fun `dismissing the blocked message hides it`() { val viewModel = viewModel(SafetyCheck { BlockReason.BACKUP_IN_PROGRESS }) From 8c186f6eae013ffe79d620df1b4a15834ecc2958 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:23:26 +0530 Subject: [PATCH 3/7] fix(profiles): don't restart when a switch can't be saved If saving the new profile failed, the app restarted anyway, and the failed value stayed in memory where a later save could still store it. It's now rolled back and the screen says the switch didn't work. If the restart itself fails, the saved profile is switched back too. Co-authored-by: David Allison <62114487+david-allison@users.noreply.github.com> Assisted-by: Claude Opus 5.5 --- .../ichi2/anki/multiprofile/ProfileManager.kt | 23 +++- .../anki/multiprofile/ProfileSwitchGuard.kt | 6 +- .../profiles/SwitchProfilesFragment.kt | 3 + .../profiles/SwitchProfilesScreen.kt | 16 +++ .../profiles/SwitchProfilesViewModel.kt | 27 +++- .../anki/multiprofile/ProfileManagerTest.kt | 44 +++++++ .../multiprofile/ProfileSwitchGuardTest.kt | 15 ++- .../SwitchProfilesRoundTripReviewTest.kt | 124 ++++++++++++++++++ .../profiles/SwitchProfilesScreenTest.kt | 23 ++++ .../profiles/SwitchProfilesViewModelTest.kt | 99 ++++++++++++++ .../src/main/res/values/03-dialogs.xml | 1 + 11 files changed, 370 insertions(+), 11 deletions(-) create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesRoundTripReviewTest.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileManager.kt b/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileManager.kt index c0d180a59d79..e481ef6a3f88 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileManager.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileManager.kt @@ -153,13 +153,16 @@ class ProfileManager private constructor( * Persists [newProfileId] as the active profile. * * @param newProfileId The [ProfileId] to activate on next launch. + * @return false if the switch could not be saved, in which case the current profile stays active */ @VisibleForTesting context(_: ProfileSwitchContext) - fun switchActiveProfile(newProfileId: ProfileId) { + fun switchActiveProfile(newProfileId: ProfileId): Boolean { Timber.i("Switching profile to ID: $newProfileId") // commit, not apply: the app restarts straight after a switch, and a pending apply() would be lost - profileRegistry.setLastActiveProfileId(newProfileId, commit = true) + val saved = profileRegistry.commitLastActiveProfileId(newProfileId) + if (!saved) Timber.w("Could not save the switch to $newProfileId") + return saved } private fun loadProfileData(profileId: ProfileId) { @@ -602,11 +605,17 @@ class ProfileManager private constructor( return id?.let { ProfileId(it) } } - fun setLastActiveProfileId( - id: ProfileId, - commit: Boolean = false, - ) { - globalPrefs.edit(commit = commit) { putString(PREF_LAST_ACTIVE_PROFILE_ID, id.value) } + fun setLastActiveProfileId(id: ProfileId) { + globalPrefs.edit { putString(PREF_LAST_ACTIVE_PROFILE_ID, id.value) } + } + + fun commitLastActiveProfileId(id: ProfileId): Boolean { + val previous = globalPrefs.getString(PREF_LAST_ACTIVE_PROFILE_ID, null) + val editor = globalPrefs.edit() + editor.putString(PREF_LAST_ACTIVE_PROFILE_ID, id.value) + if (editor.commit()) return true + globalPrefs.edit { putString(PREF_LAST_ACTIVE_PROFILE_ID, previous) } + return false } fun getWebViewProfileId(): ProfileId? { diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileSwitchGuard.kt b/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileSwitchGuard.kt index 3f7c66b16f75..4475e517cadf 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileSwitchGuard.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileSwitchGuard.kt @@ -27,6 +27,8 @@ class ProfileSwitchGuard( data class Blocked( val reasons: Set, ) : Result() + + data object Failed : Result() } // TODO: COLLECTION_BUSY has no check yet: CollectionManager serialises on a @@ -62,8 +64,8 @@ class ProfileSwitchGuard( } return if (activeBlockedReasons.isEmpty()) { - with(ProfileSwitchContext) { profileManager.switchActiveProfile(newProfileId) } - Result.Success + val switched = with(ProfileSwitchContext) { profileManager.switchActiveProfile(newProfileId) } + if (switched) Result.Success else Result.Failed } else { Result.Blocked(activeBlockedReasons) } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesFragment.kt b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesFragment.kt index 6aab75963ca1..0b6da3ae11a8 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesFragment.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesFragment.kt @@ -56,12 +56,14 @@ class SwitchProfilesFragment : Fragment() { val profileBeingRenamed by viewModel.profileBeingRenamed.collectAsStateWithLifecycle() val profileToSwitchTo by viewModel.profileToSwitchTo.collectAsStateWithLifecycle() val isSwitchBlocked by viewModel.isSwitchBlocked.collectAsStateWithLifecycle() + val profileSwitchFailed by viewModel.profileSwitchFailed.collectAsStateWithLifecycle() SwitchProfilesScreen( profiles = profiles, isAddProfileDialogVisible = isAddProfileDialogVisible, profileBeingRenamed = profileBeingRenamed, profileToSwitchTo = profileToSwitchTo, isSwitchBlocked = isSwitchBlocked, + profileSwitchFailed = profileSwitchFailed, onNavigateUp = { requireActivity().onBackPressedDispatcher.onBackPressed() }, onProfileClick = viewModel::requestSwitch, onAddProfileClick = viewModel::showAddProfileDialog, @@ -73,6 +75,7 @@ class SwitchProfilesFragment : Fragment() { onSwitchProfileConfirm = viewModel::switchProfile, onSwitchProfileDismiss = viewModel::dismissSwitchProfileDialog, onSwitchBlockedDismiss = viewModel::dismissSwitchBlockedMessage, + onSwitchFailedDismiss = viewModel::dismissSwitchFailedMessage, onDeleteProfile = viewModel::deleteProfile, ) } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesScreen.kt b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesScreen.kt index b35fecc73c67..6ca0cd2800a9 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesScreen.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesScreen.kt @@ -54,6 +54,7 @@ fun SwitchProfilesScreen( profileBeingRenamed: ProfileItem?, profileToSwitchTo: ProfileItem?, isSwitchBlocked: Boolean, + profileSwitchFailed: ProfileItem?, onNavigateUp: () -> Unit, onProfileClick: (ProfileItem) -> Unit, onAddProfileClick: () -> Unit, @@ -65,6 +66,7 @@ fun SwitchProfilesScreen( onSwitchProfileConfirm: () -> Unit, onSwitchProfileDismiss: () -> Unit, onSwitchBlockedDismiss: () -> Unit, + onSwitchFailedDismiss: () -> Unit, onDeleteProfile: (ProfileItem) -> Unit, modifier: Modifier = Modifier, ) { @@ -154,6 +156,18 @@ fun SwitchProfilesScreen( }, ) } + + profileSwitchFailed?.let { profile -> + AlertDialog( + onDismissRequest = onSwitchFailedDismiss, + text = { Text(stringResource(CommonString.switch_profile_failed, profile.name)) }, + confirmButton = { + TextButton(onClick = onSwitchFailedDismiss) { + Text(stringResource(CommonString.dialog_ok)) + } + }, + ) + } } private val AvatarSize = 40.dp @@ -234,6 +248,7 @@ private fun SwitchProfilesScreenPreview() { profileBeingRenamed = null, profileToSwitchTo = null, isSwitchBlocked = false, + profileSwitchFailed = null, onNavigateUp = {}, onProfileClick = {}, onAddProfileClick = {}, @@ -245,6 +260,7 @@ private fun SwitchProfilesScreenPreview() { onSwitchProfileConfirm = {}, onSwitchProfileDismiss = {}, onSwitchBlockedDismiss = {}, + onSwitchFailedDismiss = {}, onDeleteProfile = {}, ) } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModel.kt index c80abbbc5b05..1118a5080f67 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModel.kt @@ -12,6 +12,7 @@ import com.ichi2.anki.ioDispatcher import com.ichi2.anki.multiprofile.ProfileManager import com.ichi2.anki.multiprofile.ProfileName import com.ichi2.anki.multiprofile.ProfileSwitchGuard +import com.ichi2.anki.multiprofile.ProfileSwitchGuard.BlockReason import com.ichi2.anki.multiprofile.backupCheck import com.ichi2.anki.multiprofile.mediaSyncCheck import com.ichi2.anki.multiprofile.syncCheck @@ -48,6 +49,9 @@ class SwitchProfilesViewModel( val isSwitchBlocked: StateFlow field = MutableStateFlow(false) + val profileSwitchFailed: StateFlow + field = MutableStateFlow(null) + fun showAddProfileDialog() { isAddProfileDialogVisible.value = true } @@ -97,21 +101,42 @@ class SwitchProfilesViewModel( viewModelScope.launch(start = CoroutineStart.UNDISPATCHED) { withContext(NonCancellable) { when (val result = withContext(ioDispatcher) { switchGuard(profile.id) }) { - ProfileSwitchGuard.Result.Success -> restartApp() + ProfileSwitchGuard.Result.Success -> restartInto(profile) is ProfileSwitchGuard.Result.Blocked -> { Timber.i("Profile switch blocked: %s", result.reasons) profileManager.isSwitching.set(false) isSwitchBlocked.value = true } + ProfileSwitchGuard.Result.Failed -> onSwitchFailed(profile) } } } } + private suspend fun restartInto(profile: ProfileItem) { + try { + restartApp() + } catch (e: Exception) { + Timber.w(e, "Restarting into %s failed", profile.id) + val runningProfile = profileManager.activeProfileContext.profileId + withContext(ioDispatcher) { switchGuard(runningProfile, skipReasons = BlockReason.entries.toSet()) } + onSwitchFailed(profile) + } + } + + private fun onSwitchFailed(profile: ProfileItem) { + profileManager.isSwitching.set(false) + profileSwitchFailed.value = profile + } + fun dismissSwitchBlockedMessage() { isSwitchBlocked.value = false } + fun dismissSwitchFailedMessage() { + profileSwitchFailed.value = null + } + fun deleteProfile(profile: ProfileItem) { Timber.i("Delete profile requested: %s", profile.id) // TODO: implement profile deletion via ProfileManager diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/ProfileManagerTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/ProfileManagerTest.kt index 2388e8f9bef5..36656f74569a 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/ProfileManagerTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/ProfileManagerTest.kt @@ -364,6 +364,23 @@ class ProfileManagerTest { assertEquals(0, registry.editors.sumOf { it.applies }) } + @Test + fun `a switch that cannot be saved keeps the current profile`() { + val registry = DiskFullPreferences(context.getSharedPreferences(PROFILE_REGISTRY_FILENAME, Context.MODE_PRIVATE)) + val manager = ProfileManager.create(contextWithRegistry(registry)) + val work = manager.createNewProfile(ProfileName.fromTrustedSource("Work")) + registry.isDiskFull = true + + val switched = with(ProfileManager.ProfileSwitchContext) { manager.switchActiveProfile(work) } + + assertFalse(switched) + assertEquals( + "a failed commit keeps the new value in memory, so the next write would save the switch after all", + ProfileId.DEFAULT.value, + registry.getString(PREF_LAST_ACTIVE_PROFILE_ID, null), + ) + } + @Test fun `ProfileId DEFAULT must be strictly 'default' to preserve legacy compatibility`() { assertEquals("default", ProfileId.DEFAULT.value) @@ -840,6 +857,33 @@ class ProfileManagerTest { override fun edit(): SharedPreferences.Editor = RecordingEditor(delegate.edit()).also { editors += it } } + private class DiskFullPreferences( + private val delegate: SharedPreferences, + ) : SharedPreferences by delegate { + var isDiskFull = false + + override fun edit(): SharedPreferences.Editor { + val editor = delegate.edit() + return object : SharedPreferences.Editor by editor { + override fun commit(): Boolean { + if (!isDiskFull) return editor.commit() + editor.apply() + return false + } + } + } + } + + private fun contextWithRegistry(registry: SharedPreferences) = + object : ContextWrapper(context) { + override fun getApplicationContext(): Context? = null + + override fun getSharedPreferences( + name: String, + mode: Int, + ): SharedPreferences = if (name == PROFILE_REGISTRY_FILENAME) registry else super.getSharedPreferences(name, mode) + } + private fun makeActiveProfileUnloadable(): ProfileId { val profileId = ProfileManager.create(context).createNewProfile(ProfileName.fromTrustedSource("Work")) File(appDataRoot, profileId.value).writeText("a file where the profile folder belongs") diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/ProfileSwitchGuardTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/ProfileSwitchGuardTest.kt index 556fe55f45cd..be7a754269bd 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/ProfileSwitchGuardTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/ProfileSwitchGuardTest.kt @@ -8,6 +8,7 @@ import com.ichi2.anki.multiprofile.ProfileSwitchGuard.BlockReason import com.ichi2.anki.multiprofile.ProfileSwitchGuard.Result import com.ichi2.anki.multiprofile.ProfileSwitchGuard.SafetyCheck import io.mockk.coEvery +import io.mockk.every import io.mockk.mockk import io.mockk.verify import kotlinx.coroutines.test.runTest @@ -16,7 +17,10 @@ import org.junit.Assert.assertTrue import org.junit.Test class ProfileSwitchGuardTest { - private val profileManager: ProfileManager = mockk(relaxed = true) + private val profileManager: ProfileManager = + mockk(relaxed = true) { + with(ProfileSwitchContext) { every { switchActiveProfile(any()) } returns true } + } private val targetId = ProfileId("p_12345678") @Test @@ -121,4 +125,13 @@ class ProfileSwitchGuardTest { (result as Result.Blocked).reasons, ) } + + @Test + fun `invoke returns Failed when the switch cannot be saved`() = + runTest { + with(ProfileSwitchContext) { every { profileManager.switchActiveProfile(targetId) } returns false } + val guard = ProfileSwitchGuard(profileManager, emptyList()) + + assertEquals(Result.Failed, guard(targetId)) + } } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesRoundTripReviewTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesRoundTripReviewTest.kt new file mode 100644 index 000000000000..0d9d2c4d90ed --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesRoundTripReviewTest.kt @@ -0,0 +1,124 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package com.ichi2.anki.preferences.profiles + +import android.content.Context +import android.content.ContextWrapper +import android.content.SharedPreferences +import androidx.core.content.edit +import androidx.preference.SwitchPreferenceCompat +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.ichi2.anki.AnkiDroidApp +import com.ichi2.anki.R +import com.ichi2.anki.RobolectricTest +import com.ichi2.anki.common.preferences.sharedPrefs +import com.ichi2.anki.ioDispatcher +import com.ichi2.anki.multiprofile.ProfileManager +import com.ichi2.anki.multiprofile.ProfileName +import com.ichi2.anki.multiprofile.ProfileSwitchGuard +import com.ichi2.anki.preferences.DeveloperOptionsFragment +import com.ichi2.anki.preferences.PreferencesActivity +import com.ichi2.anki.preferences.PreferencesFragment +import com.ichi2.anki.preferences.requirePreference +import com.ichi2.anki.settings.Prefs +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.setMain +import org.junit.Test +import org.junit.runner.RunWith +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +@RunWith(AndroidJUnit4::class) +class SwitchProfilesRoundTripReviewTest : RobolectricTest() { + @Test + fun `failed profile persistence must not restart the app`() { + val dispatcher = UnconfinedTestDispatcher() + Dispatchers.setMain(dispatcher) + ioDispatcher = dispatcher + val base = AnkiDroidApp.instance.baseContext + val prefs = base.getSharedPreferences(ProfileManager.PROFILE_REGISTRY_FILENAME, Context.MODE_PRIVATE) + var rejectCommits = false + var rejectedCommits = 0 + val failurePrefs = + object : SharedPreferences by prefs { + override fun edit(): SharedPreferences.Editor { + val editor = prefs.edit() + return object : SharedPreferences.Editor by editor { + override fun commit(): Boolean { + if (!rejectCommits) return editor.commit() + rejectedCommits++ + return false + } + } + } + } + val context = + object : ContextWrapper(base) { + override fun getApplicationContext(): Context? = null + + override fun getSharedPreferences( + name: String, + mode: Int, + ): SharedPreferences = + if (name == ProfileManager.PROFILE_REGISTRY_FILENAME) failurePrefs else super.getSharedPreferences(name, mode) + } + val manager = ProfileManager.create(context) + val name = (ProfileName.validate("Work") as ProfileName.ValidationResult.Valid).name + val target = manager.createNewProfile(name) + var restarts = 0 + val model = SwitchProfilesViewModel(manager, ProfileSwitchGuard(manager, emptyList())) { restarts++ } + rejectCommits = true + + model.requestSwitch(model.profiles.value.single { it.id == target }) + model.switchProfile() + + assertEquals(1, rejectedCommits, "the persistence operation must have failed") + assertEquals(0, restarts, "restart was requested even though commit returned false") + } + + @Test + fun `switch profile option can be enabled from a non-default profile`() { + val app = AnkiDroidApp.instance + val originalManager = app.profileManager + val base = app.baseContext + val flag = app.getString(R.string.pref_enable_switch_profile_key) + base.sharedPrefs().edit(commit = true) { putBoolean(flag, true) } + val manager = requireNotNull(originalManager) + val name = (ProfileName.validate("Work") as ProfileName.ValidationResult.Valid).name + val target = manager.createNewProfile(name) + runBlocking { ProfileSwitchGuard(manager, emptyList())(target) } + + // Reload using the framework base as Application.attachBaseContext does after a restart. + app.profileManager = + ProfileManager.create( + object : ContextWrapper(base) { + override fun getApplicationContext(): Context? = null + }, + ) + try { + assertFalse(Prefs.switchProfileEnabled, "new profile starts with this developer flag off") + val activity = + startRegularActivity( + PreferencesActivity.getIntent(targetContext, DeveloperOptionsFragment::class), + ) + val settings = (activity.fragment as PreferencesFragment).childFragmentManager + val developerOptions = settings.fragments.filterIsInstance().single() + val toggle = developerOptions.requirePreference(R.string.pref_enable_switch_profile_key) + + // Switching the preference off and on follows TwoStatePreference's real persistence path. + toggle.isChecked = false + toggle.isChecked = true + + assertTrue( + Prefs.switchProfileEnabled, + "Toggle=${toggle.isChecked}, Default=${base.sharedPrefs().getBoolean(flag, false)}, " + + "active profile=${app.sharedPrefs().getBoolean(flag, false)}; the profile list remains hidden", + ) + } finally { + app.profileManager = originalManager + } + } +} diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesScreenTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesScreenTest.kt index c5ba602b95a7..0b68985a8168 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesScreenTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesScreenTest.kt @@ -70,14 +70,35 @@ class SwitchProfilesScreenTest : RobolectricTest() { composeRule.onNodeWithText(targetContext.getString(CommonString.switch_profile_blocked)).assertIsDisplayed() } + @Test + fun `a failed switch names the profile it could not open`() { + val work = ProfileItem(id = ProfileId("p_work"), name = "Work") + setScreen(profiles = listOf(default, work), profileSwitchFailed = work) + + composeRule.onNodeWithText(targetContext.getString(CommonString.switch_profile_failed, "Work")).assertIsDisplayed() + } + + @Test + fun `the failure message closes with OK`() { + val work = ProfileItem(id = ProfileId("p_work"), name = "Work") + var dismissed = false + setScreen(profiles = listOf(default, work), profileSwitchFailed = work, onSwitchFailedDismiss = { dismissed = true }) + + composeRule.onNodeWithText(targetContext.getString(CommonString.dialog_ok)).performClick() + + assertTrue(dismissed) + } + private fun setScreen( profiles: List, profileBeingRenamed: ProfileItem? = null, profileToSwitchTo: ProfileItem? = null, isSwitchBlocked: Boolean = false, + profileSwitchFailed: ProfileItem? = null, onProfileClick: (ProfileItem) -> Unit = {}, onRenameProfileConfirm: (ProfileName) -> Unit = {}, onSwitchProfileConfirm: () -> Unit = {}, + onSwitchFailedDismiss: () -> Unit = {}, ) { composeRule.setContent { AnkiDroidTheme { @@ -87,6 +108,7 @@ class SwitchProfilesScreenTest : RobolectricTest() { profileBeingRenamed = profileBeingRenamed, profileToSwitchTo = profileToSwitchTo, isSwitchBlocked = isSwitchBlocked, + profileSwitchFailed = profileSwitchFailed, onNavigateUp = {}, onProfileClick = onProfileClick, onAddProfileClick = {}, @@ -98,6 +120,7 @@ class SwitchProfilesScreenTest : RobolectricTest() { onSwitchProfileConfirm = onSwitchProfileConfirm, onSwitchProfileDismiss = {}, onSwitchBlockedDismiss = {}, + onSwitchFailedDismiss = onSwitchFailedDismiss, onDeleteProfile = {}, ) } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModelTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModelTest.kt index 2543f84a4a92..5b7638d95732 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModelTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesViewModelTest.kt @@ -4,6 +4,8 @@ package com.ichi2.anki.preferences.profiles import android.content.Context +import android.content.ContextWrapper +import android.content.SharedPreferences import androidx.core.content.edit import androidx.lifecycle.ViewModelStore import androidx.test.core.app.ApplicationProvider @@ -66,6 +68,34 @@ class SwitchProfilesViewModelTest { restartApp = { restarts++ }, ) + private fun viewModelWithRestart( + vararg checks: SafetyCheck, + restartApp: suspend () -> Unit, + ) = SwitchProfilesViewModel(profileManager, ProfileSwitchGuard(profileManager, checks.toList()), restartApp) + + private fun viewModelThatCannotSave(): SwitchProfilesViewModel { + val registry = prefs + val cannotSave = + object : SharedPreferences by registry { + override fun edit(): SharedPreferences.Editor = + object : SharedPreferences.Editor by registry.edit() { + override fun commit() = false + } + } + val manager = + ProfileManager.create( + object : ContextWrapper(context) { + override fun getApplicationContext(): Context? = null + + override fun getSharedPreferences( + name: String, + mode: Int, + ): SharedPreferences = if (name == PROFILE_REGISTRY_FILENAME) cannotSave else super.getSharedPreferences(name, mode) + }, + ) + return SwitchProfilesViewModel(manager, ProfileSwitchGuard(manager, emptyList())) { restarts++ } + } + private val lastActiveProfileId: String? get() = prefs.getString(PREF_LAST_ACTIVE_PROFILE_ID, null) @@ -276,6 +306,75 @@ class SwitchProfilesViewModelTest { assertEquals(1, restarts) } + @Test + fun `a switch that cannot be saved says so and does not restart`() { + val viewModel = viewModelThatCannotSave() + val work = createProfile("Work") + viewModel.refresh() + viewModel.requestSwitch(viewModel.profiles.value.single { it.id == work }) + + viewModel.switchProfile() + + assertEquals(work, viewModel.profileSwitchFailed.value?.id) + assertEquals(ProfileId.DEFAULT.value, lastActiveProfileId) + assertEquals(0, restarts) + } + + @Test + fun `another profile can be picked after a failed switch`() { + val viewModel = viewModelThatCannotSave() + createProfile("Work") + val personal = createProfile("Personal") + viewModel.refresh() + viewModel.requestSwitch(viewModel.profiles.value.single { it.name == "Work" }) + viewModel.switchProfile() + + viewModel.requestSwitch(viewModel.profiles.value.single { it.id == personal }) + + assertEquals(personal, viewModel.profileToSwitchTo.value?.id) + } + + @Test + fun `dismissing the failure message hides it`() { + val viewModel = viewModelThatCannotSave() + createProfile("Work") + viewModel.refresh() + viewModel.requestSwitch(viewModel.profiles.value.single { it.name == "Work" }) + viewModel.switchProfile() + + viewModel.dismissSwitchFailedMessage() + + assertNull(viewModel.profileSwitchFailed.value) + } + + @Test + fun `a restart that fails switches back and says so`() { + val work = createProfile("Work") + val viewModel = viewModelWithRestart { throw IllegalStateException("cannot start activities") } + viewModel.requestSwitch(viewModel.profiles.value.single { it.id == work }) + + viewModel.switchProfile() + + assertEquals(work, viewModel.profileSwitchFailed.value?.id) + assertEquals(ProfileId.DEFAULT.value, lastActiveProfileId, "the next start must not open Work unasked") + } + + @Test + fun `a restart that fails switches back even if a sync has started`() { + val work = createProfile("Work") + var restartFailed = false + val viewModel = + viewModelWithRestart(SafetyCheck { BlockReason.SYNC_IN_PROGRESS.takeIf { restartFailed } }) { + restartFailed = true + throw IllegalStateException("cannot start activities") + } + viewModel.requestSwitch(viewModel.profiles.value.single { it.id == work }) + + viewModel.switchProfile() + + assertEquals(ProfileId.DEFAULT.value, lastActiveProfileId) + } + @Test fun `dismissing the blocked message hides it`() { val viewModel = viewModel(SafetyCheck { BlockReason.BACKUP_IN_PROGRESS }) diff --git a/common/android/src/main/res/values/03-dialogs.xml b/common/android/src/main/res/values/03-dialogs.xml index dadd84326793..d9a30e63e053 100644 --- a/common/android/src/main/res/values/03-dialogs.xml +++ b/common/android/src/main/res/values/03-dialogs.xml @@ -308,4 +308,5 @@ also changes the interval of the card" AnkiDroid will restart to open this profile. Switch AnkiDroid is syncing or backing up. Try again when it finishes. + Couldn’t switch to “%1$s”. Try again. From 213a017aadbc2232a37ff883cd5229892fb1459b Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:23:26 +0530 Subject: [PATCH 4/7] fix(profiles): set up a profile before switching to it A profile whose folders couldn't be created only failed after the restart, and AnkiDroid then fell back to Default. Its folders and collection path are now created and checked before the switch is saved, and the switch is refused if that fails. Co-authored-by: David Allison <62114487+david-allison@users.noreply.github.com> Assisted-by: Claude Opus 5.5 --- .../ichi2/anki/multiprofile/ProfileManager.kt | 17 ++- .../anki/multiprofile/ProfileManagerTest.kt | 66 +++++++++++ .../SwitchProfilesIsolationReviewTest.kt | 108 ++++++++++++++++++ ...hProfilesStoragePreflightRegressionTest.kt | 51 +++++++++ 4 files changed, 241 insertions(+), 1 deletion(-) create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesIsolationReviewTest.kt create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesStoragePreflightRegressionTest.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileManager.kt b/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileManager.kt index e481ef6a3f88..8935aef948e6 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileManager.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/ProfileManager.kt @@ -14,6 +14,7 @@ import androidx.core.content.edit import com.ichi2.anki.common.crashreporting.CrashReportService import com.ichi2.anki.common.preferences.sharedPrefs import com.ichi2.anki.common.storage.AnkiDroidFolder +import com.ichi2.anki.common.storage.CollectionHelper import com.ichi2.anki.common.storage.CollectionHelper.PREF_COLLECTION_PATH import com.ichi2.anki.common.time.TimeManager import com.ichi2.anki.common.time.getTimestamp @@ -153,18 +154,32 @@ class ProfileManager private constructor( * Persists [newProfileId] as the active profile. * * @param newProfileId The [ProfileId] to activate on next launch. - * @return false if the switch could not be saved, in which case the current profile stays active + * @return false if the new profile could not be set up or the switch could not be saved, + * in which case the current profile stays active */ @VisibleForTesting context(_: ProfileSwitchContext) fun switchActiveProfile(newProfileId: ProfileId): Boolean { Timber.i("Switching profile to ID: $newProfileId") + try { + prepareProfile(newProfileId) + } catch (e: Exception) { + Timber.w(e, "Could not set up $newProfileId, staying on the current profile") + return false + } // commit, not apply: the app restarts straight after a switch, and a pending apply() would be lost val saved = profileRegistry.commitLastActiveProfileId(newProfileId) if (!saved) Timber.w("Could not save the switch to $newProfileId") return saved } + private fun prepareProfile(profileId: ProfileId) { + val wrapper = ProfileContextWrapper.create(profileContext, profileId, resolveProfileDirectory(profileId).file) + ensureProfileCollectionPath(wrapper) + if (profileId.isDefault()) return + CollectionHelper.initializeAnkiDroidDirectory(CollectionHelper.getCurrentAnkiDroidDirectory(wrapper)) + } + private fun loadProfileData(profileId: ProfileId) { val profileBaseDir = resolveProfileDirectory(profileId) diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/ProfileManagerTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/ProfileManagerTest.kt index 36656f74569a..cc1d0235c594 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/ProfileManagerTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/ProfileManagerTest.kt @@ -364,6 +364,69 @@ class ProfileManagerTest { assertEquals(0, registry.editors.sumOf { it.applies }) } + @Test + fun `switching sets up the new profile before saving it`() { + val manager = ProfileManager.create(context) + val work = manager.createNewProfile(ProfileName.fromTrustedSource("Work")) + + with(ProfileManager.ProfileSwitchContext) { manager.switchActiveProfile(work) } + + assertTrue("the profile folder must exist before the restart", File(appDataRoot, work.value).isDirectory) + val deckPath = profilePrefs(work).getString(PREF_COLLECTION_PATH, null) + assertEquals(File(context.getExternalFilesDir(null), work.value).absolutePath, deckPath) + assertTrue("the collection folder must exist before the restart", File(deckPath!!).isDirectory) + } + + @Test + fun `a profile that cannot be set up is not switched to`() { + val manager = ProfileManager.create(context) + val work = manager.createNewProfile(ProfileName.fromTrustedSource("Work")) + File(appDataRoot, work.value).writeText("a file where the profile folder belongs") + + val switched = with(ProfileManager.ProfileSwitchContext) { manager.switchActiveProfile(work) } + + assertFalse(switched) + assertEquals(ProfileId.DEFAULT.value, prefs.getString(PREF_LAST_ACTIVE_PROFILE_ID, null)) + } + + @Test + fun `switching keeps a collection folder the profile already chose`() { + val manager = ProfileManager.create(context) + val work = manager.createNewProfile(ProfileName.fromTrustedSource("Work")) + val chosen = File(context.getExternalFilesDir(null), "chosen-by-work").apply { mkdirs() } + writeProfileCollectionPath(work, chosen) + + with(ProfileManager.ProfileSwitchContext) { manager.switchActiveProfile(work) } + + assertEquals(chosen.absolutePath, profilePrefs(work).getString(PREF_COLLECTION_PATH, null)) + } + + @Test + fun `switching back to Default needs no setup`() { + val work = ProfileManager.create(context).createNewProfile(ProfileName.fromTrustedSource("Work")) + prefs.edit(commit = true) { putString(PREF_LAST_ACTIVE_PROFILE_ID, work.value) } + val manager = ProfileManager.create(context) + + val switched = with(ProfileManager.ProfileSwitchContext) { manager.switchActiveProfile(ProfileId.DEFAULT) } + + assertTrue(switched) + assertEquals(ProfileId.DEFAULT.value, prefs.getString(PREF_LAST_ACTIVE_PROFILE_ID, null)) + } + + @Test + @Config(minSdk = Build.VERSION_CODES.P) + fun `setting up the new profile leaves this process's WebView alone (API 28+)`() { + mockkStatic(WebView::class) + every { WebView.setDataDirectorySuffix(any()) } just runs + val manager = ProfileManager.create(context) + val work = manager.createNewProfile(ProfileName.fromTrustedSource("Work")) + + with(ProfileManager.ProfileSwitchContext) { manager.switchActiveProfile(work) } + + verify(exactly = 0) { WebView.setDataDirectorySuffix(any()) } + assertEquals(ProfileId.DEFAULT, manager.activeProfileContext.profileId) + } + @Test fun `a switch that cannot be saved keeps the current profile`() { val registry = DiskFullPreferences(context.getSharedPreferences(PROFILE_REGISTRY_FILENAME, Context.MODE_PRIVATE)) @@ -874,6 +937,9 @@ class ProfileManagerTest { } } + private fun profilePrefs(profileId: ProfileId) = + context.getSharedPreferences("profile_${profileId.value}_${context.packageName}_preferences", Context.MODE_PRIVATE) + private fun contextWithRegistry(registry: SharedPreferences) = object : ContextWrapper(context) { override fun getApplicationContext(): Context? = null diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesIsolationReviewTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesIsolationReviewTest.kt new file mode 100644 index 000000000000..4b264ee933ea --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesIsolationReviewTest.kt @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package com.ichi2.anki.preferences.profiles + +import android.content.Context +import android.content.ContextWrapper +import androidx.core.content.edit +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.ichi2.anki.AnkiDroidApp +import com.ichi2.anki.RobolectricTest +import com.ichi2.anki.common.preferences.sharedPrefs +import com.ichi2.anki.isLoggedIn +import com.ichi2.anki.multiprofile.ProfileId +import com.ichi2.anki.multiprofile.ProfileManager +import com.ichi2.anki.multiprofile.ProfileName +import com.ichi2.anki.multiprofile.ProfileSwitchGuard +import com.ichi2.anki.settings.Prefs +import kotlinx.coroutines.test.advanceUntilIdle +import org.junit.Test +import org.junit.runner.RunWith +import java.io.File +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertSame +import kotlin.test.assertTrue + +@RunWith(AndroidJUnit4::class) +class SwitchProfilesIsolationReviewTest : RobolectricTest() { + private val app get() = AnkiDroidApp.instance + + // Application.attachBaseContext receives the framework context before applicationContext is set. + private fun startupContext() = + object : ContextWrapper(app.baseContext) { + override fun getApplicationContext(): Context? = null + } + + private fun createProfile(manager: ProfileManager) = + manager.createNewProfile((ProfileName.validate("Work") as ProfileName.ValidationResult.Valid).name) + + @Test + fun `a healthy target profile does not expose Defaults sync account`() = + runTest { + val originalManager = requireNotNull(app.profileManager) + Prefs.hkey = "outgoing-default-account-key" + val target = createProfile(originalManager) + var accountVisibleAfterRestart: String? = "restart not reached" + try { + val model = + SwitchProfilesViewModel(originalManager, ProfileSwitchGuard(originalManager, emptyList())) { + app.profileManager = ProfileManager.createOrNull(startupContext()) + accountVisibleAfterRestart = Prefs.hkey + } + model.requestSwitch(model.profiles.value.single { it.id == target }) + model.switchProfile() + advanceUntilIdle() + + assertEquals(target, assertNotNull(app.profileManager).activeProfileContext.profileId) + assertNull(accountVisibleAfterRestart) + } finally { + app.profileManager = originalManager + app.sharedPrefs().edit(commit = true) { remove("hkey") } + ProfileManager.resetForTesting() + } + } + + @Test + fun `a target profile that cannot load is refused before the restart`() = + runTest { + val originalManager = requireNotNull(app.profileManager) + Prefs.hkey = "outgoing-default-account-key" + Prefs.username = "default@example.invalid" + assertTrue(isLoggedIn()) + val target = createProfile(originalManager) + val root = File(app.baseContext.filesDir.parentFile, target.value) + assertFalse(root.exists()) + root.writeText("not a directory") + var restarted = false + try { + val model = + SwitchProfilesViewModel(originalManager, ProfileSwitchGuard(originalManager, emptyList())) { + restarted = true + } + model.requestSwitch(model.profiles.value.single { it.id == target }) + model.switchProfile() + advanceUntilIdle() + + val registry = app.baseContext.getSharedPreferences(ProfileManager.PROFILE_REGISTRY_FILENAME, Context.MODE_PRIVATE) + assertEquals( + ProfileId.DEFAULT.value, + registry.getString(ProfileManager.PREF_LAST_ACTIVE_PROFILE_ID, null), + "Work could not be opened, so Default must stay selected", + ) + assertFalse(restarted, "a restart would fall back to Default under Work's name") + assertEquals(target, model.profileSwitchFailed.value?.id) + assertSame(originalManager, app.profileManager) + } finally { + app.profileManager = originalManager + root.delete() + app.sharedPrefs().edit(commit = true) { + remove("hkey") + remove("username") + } + ProfileManager.resetForTesting() + } + } +} diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesStoragePreflightRegressionTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesStoragePreflightRegressionTest.kt new file mode 100644 index 000000000000..a25efde5f55c --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesStoragePreflightRegressionTest.kt @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package com.ichi2.anki.preferences.profiles + +import android.content.Context +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.ichi2.anki.AnkiDroidApp +import com.ichi2.anki.RobolectricTest +import com.ichi2.anki.common.storage.CollectionHelper +import com.ichi2.anki.exception.StorageAccessException +import com.ichi2.anki.multiprofile.ProfileId +import com.ichi2.anki.multiprofile.ProfileManager +import com.ichi2.anki.multiprofile.ProfileName +import com.ichi2.anki.multiprofile.ProfileSwitchGuard +import kotlinx.coroutines.test.advanceUntilIdle +import org.junit.Test +import org.junit.runner.RunWith +import java.io.File +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse + +@RunWith(AndroidJUnit4::class) +class SwitchProfilesStoragePreflightRegressionTest : RobolectricTest() { + @Test + fun `a target with an unusable external collection directory is refused before restart`() = + runTest { + val app = AnkiDroidApp.instance + val manager = requireNotNull(app.profileManager) + val target = manager.createNewProfile((ProfileName.validate("Work") as ProfileName.ValidationResult.Valid).name) + val collectionDirectory = File(requireNotNull(app.getExternalFilesDir(null)), target.value) + assertFalse(collectionDirectory.exists()) + collectionDirectory.writeText("a file where the collection directory belongs") + var restarts = 0 + val model = SwitchProfilesViewModel(manager, ProfileSwitchGuard(manager, emptyList())) { restarts++ } + try { + assertFailsWith { CollectionHelper.initializeAnkiDroidDirectory(collectionDirectory) } + + model.requestSwitch(model.profiles.value.single { it.id == target }) + model.switchProfile() + advanceUntilIdle() + + val registry = app.baseContext.getSharedPreferences(ProfileManager.PROFILE_REGISTRY_FILENAME, Context.MODE_PRIVATE) + assertEquals(0, restarts, "The target's collection cannot be opened, so the running profile must remain usable") + assertEquals(ProfileId.DEFAULT.value, registry.getString(ProfileManager.PREF_LAST_ACTIVE_PROFILE_ID, null)) + assertEquals(target, model.profileSwitchFailed.value?.id) + } finally { + collectionDirectory.delete() + } + } +} From ab1bc644d97d5211abe670aa4d68b74dc9942fcf Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:23:26 +0530 Subject: [PATCH 5/7] fix(profiles): close the collection and restart in one step The restart closed the collection and then handed over to ProcessPhoenix as a separate step, so an operation queued in between could reopen it just before the process was killed. Both now happen in one step on the collection queue. Assisted-by: Claude Opus 5.5 --- .../com/ichi2/anki/multiprofile/AppRestart.kt | 9 ++-- .../ichi2/anki/multiprofile/AppRestartTest.kt | 48 +++++++++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/AppRestart.kt b/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/AppRestart.kt index 3618819e56f2..4175b36ceb07 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/AppRestart.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/AppRestart.kt @@ -24,7 +24,9 @@ import java.io.File * * Sync work is cancelled once the sync lock is held, since it carries the current * profile's AnkiWeb key. The work locks stay held until ProcessPhoenix kills this - * process, so nothing can start and reopen the collection in the meantime. + * process, so nothing can start and reopen the collection in the meantime. The + * collection is closed and handed over in one step on its queue, so an operation + * queued behind the restart cannot reopen it first. */ suspend fun safeRestartApp(context: Context) { Timber.i("Restarting the app") @@ -35,8 +37,9 @@ suspend fun safeRestartApp(context: Context) { } mediaSyncLock.holdUnlessThrows { backupLock.holdUnlessThrows { - CollectionManager.ensureClosed() - ProcessPhoenix.triggerRebirth(context, LauncherDestination.toIntent(context)) + CollectionManager.withClosedCollection { + ProcessPhoenix.triggerRebirth(context, LauncherDestination.toIntent(context)) + } } } } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/AppRestartTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/AppRestartTest.kt index 5f9388cd7501..3cd1bccccb23 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/AppRestartTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/AppRestartTest.kt @@ -15,10 +15,12 @@ import com.ichi2.anki.CollectionManager import com.ichi2.anki.IntentHandler import com.ichi2.anki.RobolectricTest import com.ichi2.anki.backupLock +import com.ichi2.anki.libanki.LibAnki import com.ichi2.anki.worker.UniqueWorkNames import com.ichi2.anki.worker.mediaSyncLock import com.ichi2.anki.worker.syncLock import com.jakewharton.processphoenix.PhoenixActivity +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex @@ -29,6 +31,7 @@ import org.junit.After import org.junit.Test import org.junit.runner.RunWith import org.robolectric.Shadows.shadowOf +import kotlin.coroutines.CoroutineContext import kotlin.test.assertEquals import kotlin.test.assertFailsWith import kotlin.test.assertFalse @@ -78,6 +81,37 @@ class AppRestartTest : RobolectricTest() { assertFalse(CollectionManager.isOpenUnsafe(), "the process is killed next, and a write in flight could corrupt it") } + @Test + fun `an operation queued during the restart cannot reopen the collection before the handover`() = + runTest { + CollectionManager.ensureOpen() + val queue = QueuedDispatcher() + val previousQueue = CollectionManager.setTestDispatcher(queue, useReentrantLock = false) + var openAtHandover: Boolean? = null + val observer = + object : ContextWrapper(context) { + override fun startActivity(intent: Intent) { + openAtHandover = collectionIsOpen() + super.startActivity(intent) + } + } + try { + val restart = launch { safeRestartApp(observer) } + advanceUntilIdle() + val lateOperation = launch { CollectionManager.withCol { } } + advanceUntilIdle() + + while (queue.tasks.isNotEmpty()) queue.tasks.removeFirst().run() + advanceUntilIdle() + restart.join() + lateOperation.join() + + assertEquals(false, openAtHandover, "ProcessPhoenix kills this process next, so an open collection could lose a write") + } finally { + CollectionManager.setTestDispatcher(previousQueue) + } + } + @Test fun `restart cancels sync work queued for the profile being left`() = runTest { @@ -190,6 +224,20 @@ class AppRestartTest : RobolectricTest() { fun `restart waits for a media sync that started after the checks passed`() = runTest { assertRestartWaitsFor(mediaSyncLock, "a media sync can start between the checks and the restart") } + @Suppress("DEPRECATION") + private fun collectionIsOpen() = LibAnki.collection?.dbClosed == false + + private class QueuedDispatcher : CoroutineDispatcher() { + val tasks = ArrayDeque() + + override fun dispatch( + context: CoroutineContext, + block: Runnable, + ) { + tasks.addLast(block) + } + } + @Test fun `only the ProcessPhoenix process is recognised as it`() { assertTrue(isPhoenixProcessName("com.ichi2.anki:phoenix")) From f1d8f63baf7da8a2bbf7a96f9552ab7bc4697132 Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:23:26 +0530 Subject: [PATCH 6/7] fix(profiles): clear the old profile's reminders when switching Reminder alarms and notifications only carry a reminder or deck ID, so after a switch they fired into the new profile or kept showing the old one's decks. The restart now unschedules the old profile's reminder alarms and clears its notifications, and schedules those alarms again if the restart fails. The new profile schedules its own reminders when it starts, as before. Co-authored-by: David Allison <62114487+david-allison@users.noreply.github.com> Assisted-by: Claude Opus 5.5 --- .../com/ichi2/anki/multiprofile/AppRestart.kt | 18 +- .../ReviewReminderAlarmManager.kt | 40 ++++ .../ichi2/anki/multiprofile/AppRestartTest.kt | 82 ++++++++ ...lesFailedRestartRemindersRegressionTest.kt | 63 ++++++ .../SwitchProfilesRestartReviewTest.kt | 192 ++++++++++++++++++ 5 files changed, 392 insertions(+), 3 deletions(-) create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesFailedRestartRemindersRegressionTest.kt create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesRestartReviewTest.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/AppRestart.kt b/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/AppRestart.kt index 4175b36ceb07..b858ef89d24a 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/AppRestart.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/multiprofile/AppRestart.kt @@ -5,11 +5,14 @@ package com.ichi2.anki.multiprofile import android.content.Context import androidx.annotation.VisibleForTesting +import androidx.core.app.NotificationManagerCompat import androidx.work.WorkManager import androidx.work.await import com.ichi2.anki.CollectionManager import com.ichi2.anki.backupLock import com.ichi2.anki.common.destinations.LauncherDestination +import com.ichi2.anki.reviewreminders.ReviewReminderAlarmManager +import com.ichi2.anki.reviewreminders.ReviewReminderId import com.ichi2.anki.toIntent import com.ichi2.anki.worker.UniqueWorkNames import com.ichi2.anki.worker.mediaSyncLock @@ -26,7 +29,8 @@ import java.io.File * profile's AnkiWeb key. The work locks stay held until ProcessPhoenix kills this * process, so nothing can start and reopen the collection in the meantime. The * collection is closed and handed over in one step on its queue, so an operation - * queued behind the restart cannot reopen it first. + * queued behind the restart cannot reopen it first. The notifications and reminder + * alarms of the profile being left are cleared, as the next profile can't act on them. */ suspend fun safeRestartApp(context: Context) { Timber.i("Restarting the app") @@ -37,8 +41,16 @@ suspend fun safeRestartApp(context: Context) { } mediaSyncLock.holdUnlessThrows { backupLock.holdUnlessThrows { - CollectionManager.withClosedCollection { - ProcessPhoenix.triggerRebirth(context, LauncherDestination.toIntent(context)) + var unscheduledReminders = emptySet() + try { + CollectionManager.withClosedCollection { + unscheduledReminders = ReviewReminderAlarmManager.unscheduleAllReviewReminderNotifications(context) + NotificationManagerCompat.from(context).cancelAll() + ProcessPhoenix.triggerRebirth(context, LauncherDestination.toIntent(context)) + } + } catch (e: Exception) { + ReviewReminderAlarmManager.rescheduleReviewReminderNotifications(context, unscheduledReminders) + throw e } } } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminderAlarmManager.kt b/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminderAlarmManager.kt index 2dc141553aed..74a8d5ffa03f 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminderAlarmManager.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminderAlarmManager.kt @@ -10,6 +10,7 @@ import androidx.core.app.PendingIntentCompat import com.ichi2.anki.common.android.AlarmManagement import com.ichi2.anki.common.time.TimeManager import com.ichi2.anki.services.NotificationService +import com.ichi2.anki.settings.Prefs import timber.log.Timber import java.util.Calendar @@ -202,6 +203,45 @@ object ReviewReminderAlarmManager { } } + fun unscheduleAllReviewReminderNotifications(context: Context): Set { + val actions = + listOf( + NotificationService.NotificationServiceAction.ScheduleRecurringNotifications, + NotificationService.NotificationServiceAction.SnoozeNotification, + ) + val unscheduledRecurring = mutableSetOf() + AlarmManagement.useAlarmManager(context) { alarmManager -> + for (id in 0 until Prefs.reviewReminderNextFreeId) { + for (action in actions) { + val intent = NotificationService.getIntent(context, ReviewReminderId(id), ReviewReminderScope.Global, action) + val pendingIntent = + PendingIntentCompat.getBroadcast(context, id, intent, PendingIntent.FLAG_NO_CREATE, false) ?: continue + alarmManager.cancel(pendingIntent) + pendingIntent.cancel() + if (action == NotificationService.NotificationServiceAction.ScheduleRecurringNotifications) { + unscheduledRecurring += ReviewReminderId(id) + } + } + } + } + Timber.i("${reminderLogPrefix()} Unscheduled all review reminder notifications") + return unscheduledRecurring + } + + suspend fun rescheduleReviewReminderNotifications( + context: Context, + ids: Set, + ) { + if (ids.isEmpty()) return + val reminders = + ReviewRemindersDatabase + .getAllReminders() + .getRemindersList() + .filter { it.id in ids } + reminders.forEach { scheduleReviewReminderNotification(context, it, attemptImmediateNotification = false) } + Timber.i("${reminderLogPrefix()} Rescheduled ${reminders.size} review reminders") + } + /** * Schedules notifications for all currently-enabled review reminders. Reads from the [ReviewRemindersDatabase]. * Also attempts a notification send just in case the most recent one was missed. Deduplication and aborting diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/AppRestartTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/AppRestartTest.kt index 3cd1bccccb23..d902f94a2bb4 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/AppRestartTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/AppRestartTest.kt @@ -3,19 +3,31 @@ package com.ichi2.anki.multiprofile +import android.app.AlarmManager import android.app.Application +import android.app.Notification +import android.app.NotificationManager import android.content.Context import android.content.ContextWrapper import android.content.Intent +import androidx.core.app.NotificationCompat +import androidx.core.app.PendingIntentCompat import androidx.core.content.IntentCompat import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.work.WorkInfo import com.ichi2.anki.CollectionManager import com.ichi2.anki.IntentHandler +import com.ichi2.anki.NotificationChannel +import com.ichi2.anki.R import com.ichi2.anki.RobolectricTest import com.ichi2.anki.backupLock import com.ichi2.anki.libanki.LibAnki +import com.ichi2.anki.reviewreminders.ReviewReminder +import com.ichi2.anki.reviewreminders.ReviewReminderAlarmManager +import com.ichi2.anki.reviewreminders.ReviewReminderScope +import com.ichi2.anki.reviewreminders.ReviewReminderTime +import com.ichi2.anki.services.NotificationService import com.ichi2.anki.worker.UniqueWorkNames import com.ichi2.anki.worker.mediaSyncLock import com.ichi2.anki.worker.syncLock @@ -112,6 +124,53 @@ class AppRestartTest : RobolectricTest() { } } + @Test + fun `restart clears the notifications of the profile being left`() = + runTest { + val notifications = context.getSystemService(NotificationManager::class.java) + notifications.notify(NotificationService.REVIEW_REMINDER_NOTIFICATION_TAG, 0, notification("Private French")) + notifications.notify(SYNC_RESULT_NOTIFICATION_ID, notification("Sync complete")) + + safeRestartApp(context) + + assertEquals(emptyList(), notifications.activeNotifications.map { it.notification.extras.getString(Notification.EXTRA_TITLE) }) + } + + @Test + fun `restart unschedules the recurring and snoozed reminders of the profile being left`() = + runTest { + val daily = ReviewReminder.createReviewReminder(time = ReviewReminderTime(20, 0)) + val deck = ReviewReminder.createReviewReminder(time = ReviewReminderTime(8, 30), scope = ReviewReminderScope.DeckSpecific(1L)) + ReviewReminderAlarmManager.scheduleReviewReminderNotification(context, daily, attemptImmediateNotification = false) + ReviewReminderAlarmManager.scheduleReviewReminderNotification(context, deck, attemptImmediateNotification = false) + ReviewReminderAlarmManager.scheduleSnoozedNotification(context, deck, snoozeIntervalInMinutes = 5) + assertEquals(3, reminderAlarms().size) + + safeRestartApp(context) + + assertEquals(emptyList(), reminderAlarms(), "they would fire into the next profile's reminders") + } + + @Test + fun `restart leaves alarms that are not reminders alone`() = + runTest { + val reminder = ReviewReminder.createReviewReminder(time = ReviewReminderTime(20, 0)) + ReviewReminderAlarmManager.scheduleReviewReminderNotification(context, reminder, attemptImmediateNotification = false) + val other = + PendingIntentCompat.getBroadcast( + context, + reminder.id.value, + Intent(context, NotificationService::class.java), + 0, + false, + )!! + context.getSystemService(AlarmManager::class.java).set(AlarmManager.RTC_WAKEUP, Long.MAX_VALUE, other) + + safeRestartApp(context) + + assertEquals(listOf(other), scheduledAlarms()) + } + @Test fun `restart cancels sync work queued for the profile being left`() = runTest { @@ -224,6 +283,25 @@ class AppRestartTest : RobolectricTest() { fun `restart waits for a media sync that started after the checks passed`() = runTest { assertRestartWaitsFor(mediaSyncLock, "a media sync can start between the checks and the restart") } + private fun notification(title: String) = + NotificationCompat + .Builder(context, NotificationChannel.GENERAL.id) + .setSmallIcon(R.drawable.ic_star_notify) + .setContentTitle(title) + .build() + + @Suppress("DEPRECATION") + private fun scheduledAlarms() = shadowOf(context.getSystemService(AlarmManager::class.java)).scheduledAlarms.map { it.operation } + + private fun reminderAlarms() = + scheduledAlarms().filter { + shadowOf(it).savedIntent.action in + listOf( + NotificationService.NotificationServiceAction.ScheduleRecurringNotifications.actionString, + NotificationService.NotificationServiceAction.SnoozeNotification.actionString, + ) + } + @Suppress("DEPRECATION") private fun collectionIsOpen() = LibAnki.collection?.dbClosed == false @@ -245,4 +323,8 @@ class AppRestartTest : RobolectricTest() { assertFalse(isPhoenixProcessName("com.ichi2.anki:acra")) assertFalse(isPhoenixProcessName(null)) } + + companion object { + private const val SYNC_RESULT_NOTIFICATION_ID = 1000 + } } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesFailedRestartRemindersRegressionTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesFailedRestartRemindersRegressionTest.kt new file mode 100644 index 000000000000..dac9e849d40e --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesFailedRestartRemindersRegressionTest.kt @@ -0,0 +1,63 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package com.ichi2.anki.preferences.profiles + +import android.app.AlarmManager +import android.content.Context +import android.content.ContextWrapper +import android.content.Intent +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.ichi2.anki.AnkiDroidApp +import com.ichi2.anki.RobolectricTest +import com.ichi2.anki.multiprofile.ProfileId +import com.ichi2.anki.multiprofile.ProfileManager +import com.ichi2.anki.multiprofile.ProfileName +import com.ichi2.anki.multiprofile.ProfileSwitchGuard +import com.ichi2.anki.multiprofile.safeRestartApp +import com.ichi2.anki.reviewreminders.ReviewReminder +import com.ichi2.anki.reviewreminders.ReviewReminderAlarmManager +import com.ichi2.anki.reviewreminders.ReviewReminderTime +import com.ichi2.anki.reviewreminders.ReviewRemindersDatabase +import com.ichi2.anki.services.NotificationService +import kotlinx.coroutines.test.advanceUntilIdle +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.Shadows.shadowOf +import kotlin.test.assertEquals + +@RunWith(AndroidJUnit4::class) +class SwitchProfilesFailedRestartRemindersRegressionTest : RobolectricTest() { + @Test + fun `a failed restart preserves recurring reminders for the restored profile`() = + runTest { + val app = AnkiDroidApp.instance + val reminder = ReviewReminder.createReviewReminder(time = ReviewReminderTime(20, 0)) + ReviewRemindersDatabase.insertReminder(reminder) + ReviewReminderAlarmManager.scheduleReviewReminderNotification(app, reminder, attemptImmediateNotification = false) + val alarms = shadowOf(app.getSystemService(AlarmManager::class.java)) + + @Suppress("DEPRECATION") + fun recurringAlarmCount() = + alarms.scheduledAlarms.count { + shadowOf(it.operation).savedIntent.action == + NotificationService.NotificationServiceAction.ScheduleRecurringNotifications.actionString + } + assertEquals(1, recurringAlarmCount()) + val manager = requireNotNull(app.profileManager) + val target = manager.createNewProfile((ProfileName.validate("Work") as ProfileName.ValidationResult.Valid).name) + val refusesActivities = + object : ContextWrapper(app) { + override fun startActivity(intent: Intent) = throw IllegalStateException("cannot start activities") + } + val model = SwitchProfilesViewModel(manager, ProfileSwitchGuard(manager, emptyList())) { safeRestartApp(refusesActivities) } + + model.requestSwitch(model.profiles.value.single { it.id == target }) + model.switchProfile() + advanceUntilIdle() + + val registry = app.baseContext.getSharedPreferences(ProfileManager.PROFILE_REGISTRY_FILENAME, Context.MODE_PRIVATE) + assertEquals(ProfileId.DEFAULT.value, registry.getString(ProfileManager.PREF_LAST_ACTIVE_PROFILE_ID, null)) + assertEquals(target, model.profileSwitchFailed.value?.id) + assertEquals(1, recurringAlarmCount(), "The app stayed in Default, so its reminder must still be scheduled") + } +} diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesRestartReviewTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesRestartReviewTest.kt new file mode 100644 index 000000000000..45fde2bdef9d --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesRestartReviewTest.kt @@ -0,0 +1,192 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package com.ichi2.anki.preferences.profiles + +import android.app.AlarmManager +import android.app.Notification +import android.app.NotificationManager +import android.content.Context +import android.content.ContextWrapper +import android.content.Intent +import androidx.core.content.edit +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.ichi2.anki.AnkiDroidApp +import com.ichi2.anki.CollectionManager +import com.ichi2.anki.IntentHandler +import com.ichi2.anki.RobolectricTest +import com.ichi2.anki.backupLock +import com.ichi2.anki.ioDispatcher +import com.ichi2.anki.libanki.LibAnki +import com.ichi2.anki.multiprofile.ProfileName +import com.ichi2.anki.multiprofile.ProfileSwitchGuard +import com.ichi2.anki.multiprofile.backupCheck +import com.ichi2.anki.multiprofile.mediaSyncCheck +import com.ichi2.anki.multiprofile.safeRestartApp +import com.ichi2.anki.multiprofile.syncCheck +import com.ichi2.anki.reviewreminders.ReviewReminder +import com.ichi2.anki.reviewreminders.ReviewReminderAlarmManager +import com.ichi2.anki.reviewreminders.ReviewReminderScope +import com.ichi2.anki.reviewreminders.ReviewReminderTime +import com.ichi2.anki.reviewreminders.ReviewRemindersDatabase +import com.ichi2.anki.services.NotificationService +import com.ichi2.anki.worker.mediaSyncLock +import com.ichi2.anki.worker.syncLock +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestCoroutineScheduler +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.setMain +import org.junit.After +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.Shadows.shadowOf +import kotlin.coroutines.CoroutineContext +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +@RunWith(AndroidJUnit4::class) +class SwitchProfilesRestartReviewTest : RobolectricTest() { + private val app get() = AnkiDroidApp.instance + + @After + fun releaseRestartLocks() { + listOf(syncLock, mediaSyncLock, backupLock).filter { it.isLocked }.forEach { it.unlock() } + } + + private fun model(restart: suspend () -> Unit = { safeRestartApp(app) }): SwitchProfilesViewModel { + val manager = requireNotNull(app.profileManager) + val target = manager.createNewProfile((ProfileName.validate("Work") as ProfileName.ValidationResult.Valid).name) + val model = + SwitchProfilesViewModel( + manager, + ProfileSwitchGuard(manager, listOf(syncCheck(), mediaSyncCheck(), backupCheck())), + restart, + ) + model.requestSwitch(model.profiles.value.single { it.id == target }) + return model + } + + @Test + fun `switch removes the old profile notification and its unscoped deck action`() = + runTest { + col.decks.rename(1L, "Private French") + addNoteUsingNoteTypeName("Basic", "Private question", "Private answer") + val reminder = + ReviewReminder.createReviewReminder( + time = ReviewReminderTime(20, 0), + scope = ReviewReminderScope.DeckSpecific(1L), + ) + ReviewRemindersDatabase.remindersSharedPrefs.edit(commit = true) { clear() } + ReviewRemindersDatabase.insertReminder(reminder) + NotificationService.handleReviewReminderNotification(app, reminder.id, reminder.scope, isRecurringNotification = false) + + val notifications = app.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager + val oldNotification = + notifications.activeNotifications.single { + it.tag == NotificationService.REVIEW_REMINDER_NOTIFICATION_TAG + } + assertTrue( + oldNotification.notification.extras + .getCharSequence(Notification.EXTRA_TITLE) + .toString() + .contains("Private French"), + ) + val action = shadowOf(oldNotification.notification.contentIntent).savedIntent + assertEquals(1L, action.getLongExtra(IntentHandler.EXTRA_DECK_ID, -1L)) + assertEquals(setOf(IntentHandler.EXTRA_DECK_ID), action.extras!!.keySet()) + + model().switchProfile() + advanceUntilIdle() + + assertTrue( + notifications.activeNotifications.none { it.tag == NotificationService.REVIEW_REMINDER_NOTIFICATION_TAG }, + "Default's notification, deck name, and deck-only click action survived the switch to Work", + ) + } + + @Test + fun `switch unschedules outgoing profile reminder alarms`() = + runTest { + val reminder = ReviewReminder.createReviewReminder(time = ReviewReminderTime(20, 0)) + ReviewReminderAlarmManager.scheduleReviewReminderNotification(app, reminder, attemptImmediateNotification = false) + val alarms = shadowOf(app.getSystemService(Context.ALARM_SERVICE) as AlarmManager) + + @Suppress("DEPRECATION") + fun reminderAlarmIsScheduled() = + alarms.scheduledAlarms.any { + shadowOf(it.operation).savedIntent.action == + NotificationService.NotificationServiceAction.ScheduleRecurringNotifications.actionString + } + assertTrue(reminderAlarmIsScheduled()) + + model().switchProfile() + advanceUntilIdle() + + assertFalse(reminderAlarmIsScheduled(), "An alarm containing only the outgoing profile's reminder id survived restart") + } + + @Test + fun `a queued collection operation cannot reopen the collection before process handover`() { + runBlocking { CollectionManager.ensureOpen() } + val reminder = ReviewReminder.createReviewReminder(time = ReviewReminderTime(20, 0)) + runBlocking { ReviewRemindersDatabase.insertReminder(reminder) } + val mainScheduler = TestCoroutineScheduler() + Dispatchers.setMain(StandardTestDispatcher(mainScheduler)) + ioDispatcher = UnconfinedTestDispatcher(mainScheduler) + val queue = QueuedDispatcher() + val previousQueue = CollectionManager.setTestDispatcher(queue, useReentrantLock = false) + var openAtHandover: Boolean? = null + val observer = + object : ContextWrapper(app) { + override fun startActivity(intent: Intent) { + openAtHandover = collectionIsOpen() + super.startActivity(intent) + } + } + try { + model { safeRestartApp(observer) }.switchProfile() + mainScheduler.runCurrent() + assertEquals(1, queue.tasks.size, "restart must be awaiting ensureClosed on the collection queue") + + val lateOperation = + CoroutineScope(Dispatchers.Unconfined).launch { + // A real alarm receiver's work can be queued behind the restart's close. + NotificationService.handleReviewReminderNotification(app, reminder.id, reminder.scope, isRecurringNotification = false) + } + assertEquals(2, queue.tasks.size) + queue.tasks.removeFirst().run() + assertFalse(collectionIsOpen()) + queue.tasks.removeFirst().run() + assertTrue(lateOperation.isCompleted) + + mainScheduler.runCurrent() + + assertNotNull(openAtHandover, "the real ProcessPhoenix handover must have run") + assertFalse(openAtHandover, "the collection reopened before handing off to a process that will kill this process") + } finally { + CollectionManager.setTestDispatcher(previousQueue) + } + } + + // Read without entering the intentionally paused collection queue. + @Suppress("DEPRECATION") + private fun collectionIsOpen() = LibAnki.collection?.dbClosed == false + + private class QueuedDispatcher : CoroutineDispatcher() { + val tasks = ArrayDeque() + + override fun dispatch( + context: CoroutineContext, + block: Runnable, + ) { + tasks.addLast(block) + } + } +} From 017b8f9cc0e90c4feb34b1ea2e95d28415c1a81e Mon Sep 17 00:00:00 2001 From: Ashish Yadav <48384865+criticalAY@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:23:26 +0530 Subject: [PATCH 7/7] fix(profiles): open a deck shortcut only in its own profile Deck shortcuts, widgets and reminder taps only store a deck ID, so after a switch a Default shortcut reviewed whichever deck had that ID in the new profile. Other profiles now tag them and their shortcut IDs, while Default's stay as they were, and AnkiDroid only opens a deck in the profile it came from. Otherwise it says which profile to switch to. Co-authored-by: David Allison <62114487+david-allison@users.noreply.github.com> Assisted-by: Claude Opus 5.5 --- .../main/java/com/ichi2/anki/AnkiDroidApp.kt | 4 + .../main/java/com/ichi2/anki/DeckPicker.kt | 4 +- .../main/java/com/ichi2/anki/IntentHandler.kt | 22 ++- .../com/ichi2/anki/ReviewDeckDestination.kt | 14 ++ .../multiprofile/ProfileDeckLaunchTest.kt | 163 ++++++++++++++++++ .../SwitchProfilesShortcutsReviewTest.kt | 132 ++++++++++++++ .../src/main/res/values/02-strings.xml | 1 + 7 files changed, 337 insertions(+), 3 deletions(-) create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/ProfileDeckLaunchTest.kt create mode 100644 AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesShortcutsReviewTest.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt b/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt index 1cc89cd20a15..cb04b7c5d39e 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt @@ -59,6 +59,7 @@ import com.ichi2.anki.logging.logActivityCreation import com.ichi2.anki.model.FieldFilters.NoSuggestFilter import com.ichi2.anki.multimedia.MultimediaArgsStorage import com.ichi2.anki.multiprofile.ProfileContextWrapper +import com.ichi2.anki.multiprofile.ProfileId import com.ichi2.anki.multiprofile.ProfileManager import com.ichi2.anki.multiprofile.isPhoenixProcess import com.ichi2.anki.navigation.initializeNavigator @@ -668,6 +669,9 @@ open class AnkiDroidApp : fun wrapForActiveProfile(base: Context): Context = (base.applicationContext as? AnkiDroidApp)?.profileManager?.wrap(base) ?: base + fun activeProfileId(context: Context): ProfileId = + (context.applicationContext as? AnkiDroidApp)?.profileManager?.activeProfileContext?.profileId ?: ProfileId.DEFAULT + /** HACK: Whether an exception report has been thrown - TODO: Rewrite an ACRA Listener to do this */ @VisibleForTesting var sentExceptionReportHack = false diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index d52ef721781f..5a843c62e9f1 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -2315,7 +2315,7 @@ open class DeckPicker : // This code should not be reachable with lower versions val shortcut = ShortcutInfoCompat - .Builder(this, shortcutData.deckId.toString()) + .Builder(this, deckShortcutId(shortcutData.deckId)) .setIntent( with(DeferredNavigation) { ReviewDeckDestination.ExternalLaunch(shortcutData.deckId).toIntent() }, ).setIcon(IconCompat.createWithResource(this, R.mipmap.ic_launcher)) @@ -2350,7 +2350,7 @@ open class DeckPicker : @NeedsTest("ensure collapsed decks are also deleted") private fun disableDeckAndChildrenShortcuts(deckTreeDids: List) { val errorMessage: CharSequence = getString(CommonString.deck_shortcut_doesnt_exist) - ShortcutUtils.disableShortcuts(this, deckTreeDids, errorMessage) + ShortcutUtils.disableShortcuts(this, deckTreeDids.map { deckShortcutId(it.toLong()) }, errorMessage) } fun renameDeckDialog(did: DeckId) { diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/IntentHandler.kt b/AnkiDroid/src/main/java/com/ichi2/anki/IntentHandler.kt index e80b734857f2..6054bb222f5b 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/IntentHandler.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/IntentHandler.kt @@ -30,6 +30,7 @@ import com.ichi2.anki.dialogs.DialogHandler.Companion.storeMessage import com.ichi2.anki.dialogs.DialogHandlerMessage import com.ichi2.anki.dialogs.requireDeckPickerOrShowError import com.ichi2.anki.libanki.DeckId +import com.ichi2.anki.multiprofile.ProfileId import com.ichi2.anki.settings.Prefs import com.ichi2.anki.shareddeck.SharedDecksDownloadFragment import com.ichi2.anki.sync.MeteredSyncPolicy @@ -196,6 +197,13 @@ class IntentHandler : AbstractIntentHandler() { ) { val deckId = intent.getLongExtra(EXTRA_DECK_ID, 0) Timber.i("Handling intent to review deck '%d'", deckId) + val deckProfile = intent.getStringExtra(EXTRA_PROFILE_ID)?.let(::ProfileId) ?: ProfileId.DEFAULT + if (deckProfile != AnkiDroidApp.activeProfileId(this)) { + Timber.w("Not reviewing deck '%d': it belongs to another profile", deckId) + showThemedToast(this, otherProfileDeckMessage(deckProfile), false) + launchDeckPickerIfNoOtherTasks(reloadIntent) + return + } val reviewIntent = if (Prefs.isNewStudyScreenEnabled) { @@ -216,6 +224,17 @@ class IntentHandler : AbstractIntentHandler() { finish() } + private fun otherProfileDeckMessage(profileId: ProfileId): String { + val profileName = + AnkiDroidApp.instance.profileManager + ?.getAllProfiles() + ?.get(profileId) + ?.displayName + ?.value + ?: return getString(CommonString.deck_shortcut_doesnt_exist) + return getString(CommonString.deck_in_other_profile, profileName) + } + private fun handleSyncIntent( reloadIntent: Intent, action: String?, @@ -383,6 +402,7 @@ class IntentHandler : AbstractIntentHandler() { companion object { private const val ACTION_DO_SYNC = "com.ichi2.anki.DO_SYNC" const val EXTRA_DECK_ID = "EXTRA_DECK_ID" + const val EXTRA_PROFILE_ID = "EXTRA_PROFILE_ID" private const val ACTION_COPY_DEBUG_INFO = "com.ichi2.anki.COPY_DEBUG_INFO" private const val EXTRA_CLIPBOARD_DATA = "clip_data" @@ -525,6 +545,6 @@ class IntentHandler : AbstractIntentHandler() { fun getReviewDeckIntent( context: Context, deckId: DeckId, - ): Intent = Intent(context, IntentHandler::class.java).putExtra(EXTRA_DECK_ID, deckId) + ): Intent = Intent(context, IntentHandler::class.java).putExtra(EXTRA_DECK_ID, deckId).putDeckProfile(context) } } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/ReviewDeckDestination.kt b/AnkiDroid/src/main/java/com/ichi2/anki/ReviewDeckDestination.kt index 8a6da6757301..fb251df73ea6 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/ReviewDeckDestination.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/ReviewDeckDestination.kt @@ -5,7 +5,9 @@ package com.ichi2.anki import android.content.Context import android.content.Intent import com.ichi2.anki.IntentHandler.Companion.EXTRA_DECK_ID +import com.ichi2.anki.IntentHandler.Companion.EXTRA_PROFILE_ID import com.ichi2.anki.common.destinations.ReviewDeckDestination +import com.ichi2.anki.libanki.DeckId /** Builds the [Intent] that opens the reviewer for this destination. */ fun ReviewDeckDestination.toIntent(context: Context): Intent = @@ -16,5 +18,17 @@ fun ReviewDeckDestination.toIntent(context: Context): Intent = // shortcuts require the intent to have an action setAction(Intent.ACTION_VIEW) putExtra(EXTRA_DECK_ID, deckId) + putDeckProfile(context) } } + +fun Intent.putDeckProfile(context: Context): Intent = + apply { + val profileId = AnkiDroidApp.activeProfileId(context) + if (!profileId.isDefault()) putExtra(EXTRA_PROFILE_ID, profileId.value) + } + +fun Context.deckShortcutId(deckId: DeckId): String { + val profileId = AnkiDroidApp.activeProfileId(this) + return if (profileId.isDefault()) deckId.toString() else "${profileId.value}_$deckId" +} diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/ProfileDeckLaunchTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/ProfileDeckLaunchTest.kt new file mode 100644 index 000000000000..0ca934a1d10e --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/multiprofile/ProfileDeckLaunchTest.kt @@ -0,0 +1,163 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// SPDX-FileCopyrightText: 2026 Ashish Yadav + +package com.ichi2.anki.multiprofile + +import android.content.Context +import android.content.ContextWrapper +import android.content.Intent +import androidx.core.content.edit +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.ichi2.anki.AnkiDroidApp +import com.ichi2.anki.CollectionManager +import com.ichi2.anki.CommonString +import com.ichi2.anki.IntentHandler +import com.ichi2.anki.Reviewer +import com.ichi2.anki.RobolectricTest +import com.ichi2.anki.common.destinations.ReviewDeckDestination +import com.ichi2.anki.deckShortcutId +import com.ichi2.anki.multiprofile.ProfileManager.Companion.PREF_LAST_ACTIVE_PROFILE_ID +import com.ichi2.anki.multiprofile.ProfileManager.Companion.PROFILE_REGISTRY_FILENAME +import com.ichi2.anki.toIntent +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.Robolectric +import org.robolectric.Shadows.shadowOf +import org.robolectric.shadows.ShadowToast +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +@RunWith(AndroidJUnit4::class) +class ProfileDeckLaunchTest : RobolectricTest() { + override fun getCollectionStorageMode() = CollectionStorageMode.ON_DISK + + private val app get() = AnkiDroidApp.instance + private lateinit var defaultManager: ProfileManager + + @Before + override fun setUp() { + super.setUp() + defaultManager = requireNotNull(app.profileManager) + } + + @After + override fun tearDown() { + runBlocking { CollectionManager.discardBackend() } + app.profileManager = defaultManager + super.tearDown() + } + + @Test + fun `a deck shortcut made in another profile opens its deck there`() { + openProfile(createProfile("Work")) + col.decks.rename(1L, "Work Spanish") + + val launched = launch(ReviewDeckDestination.ExternalLaunch(1L).toIntent(targetContext)) + + assertTrue(launched.any { it.component?.className == Reviewer::class.java.name }) + assertEquals("Work Spanish", col.decks.getLegacy(col.decks.selected())!!.name) + } + + @Test + fun `a deck shortcut made in another profile does not open in Default`() { + openProfile(createProfile("Work")) + val shortcut = ReviewDeckDestination.ExternalLaunch(1L).toIntent(targetContext) + openProfile(ProfileId.DEFAULT) + + val launched = launch(shortcut) + + assertFalse(launched.any { it.component?.className == Reviewer::class.java.name }, "Default's deck 1 is not Work's") + assertEquals(targetContext.getString(CommonString.deck_in_other_profile, "Work"), ShadowToast.getTextOfLatestToast()) + } + + @Test + fun `a reminder made in another profile does not open in Default`() { + openProfile(createProfile("Work")) + val reminderTap = IntentHandler.getReviewDeckIntent(targetContext, 1L) + openProfile(ProfileId.DEFAULT) + + val launched = launch(reminderTap) + + assertFalse(launched.any { it.component?.className == Reviewer::class.java.name }, "Default's deck 1 is not Work's") + } + + @Test + fun `a reminder made in another profile opens its deck there`() { + openProfile(createProfile("Work")) + col.decks.rename(1L, "Work Spanish") + + val launched = launch(IntentHandler.getReviewDeckIntent(targetContext, 1L)) + + assertTrue(launched.any { it.component?.className == Reviewer::class.java.name }) + assertEquals("Work Spanish", col.decks.getLegacy(col.decks.selected())!!.name) + } + + @Test + fun `a deck shortcut from a deleted profile says the deck is gone`() { + val shortcut = + ReviewDeckDestination + .ExternalLaunch(1L) + .toIntent(targetContext) + .putExtra(IntentHandler.EXTRA_PROFILE_ID, "p_deleted") + + val launched = launch(shortcut) + + assertFalse(launched.any { it.component?.className == Reviewer::class.java.name }) + assertEquals(targetContext.getString(CommonString.deck_shortcut_doesnt_exist), ShadowToast.getTextOfLatestToast()) + } + + @Test + fun `deck intents made in Default are unchanged`() { + val shortcut = ReviewDeckDestination.ExternalLaunch(1L).toIntent(targetContext) + val reminderTap = IntentHandler.getReviewDeckIntent(targetContext, 1L) + + assertEquals( + setOf(IntentHandler.EXTRA_DECK_ID), + shortcut.extras!!.keySet(), + "pinned shortcuts made before profiles must keep working", + ) + assertEquals(setOf(IntentHandler.EXTRA_DECK_ID), reminderTap.extras!!.keySet()) + } + + @Test + fun `deck shortcut ids in Default stay the deck id`() { + assertEquals("1", targetContext.deckShortcutId(1L), "pinned shortcuts made before profiles must keep their id") + } + + @Test + fun `deck shortcuts in another profile do not reuse Default's ids`() { + val work = createProfile("Work") + openProfile(work) + + assertEquals("${work.value}_1", targetContext.deckShortcutId(1L), "Default's pinned shortcut for deck 1 would be replaced") + } + + private fun createProfile(name: String) = + defaultManager.createNewProfile((ProfileName.validate(name) as ProfileName.ValidationResult.Valid).name) + + private fun openProfile(profileId: ProfileId) { + runBlocking { CollectionManager.discardBackend() } + app.baseContext.getSharedPreferences(PROFILE_REGISTRY_FILENAME, Context.MODE_PRIVATE).edit(commit = true) { + putString(PREF_LAST_ACTIVE_PROFILE_ID, profileId.value) + } + app.profileManager = + ProfileManager.create( + object : ContextWrapper(app.baseContext) { + override fun getApplicationContext(): Context? = null + }, + ) + } + + private fun launch(intent: Intent): List = + Robolectric.buildActivity(IntentHandler::class.java, intent).use { controller -> + controller.create() + buildList { + val activity = shadowOf(controller.get()) + while (true) add(activity.nextStartedActivity ?: break) + } + } +} diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesShortcutsReviewTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesShortcutsReviewTest.kt new file mode 100644 index 000000000000..b64fb55df43d --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/profiles/SwitchProfilesShortcutsReviewTest.kt @@ -0,0 +1,132 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package com.ichi2.anki.preferences.profiles + +import android.content.Context +import android.content.ContextWrapper +import android.content.Intent +import android.content.pm.ShortcutInfo +import android.content.pm.ShortcutManager +import android.os.Build +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.filters.SdkSuppress +import com.ichi2.anki.AnkiDroidApp +import com.ichi2.anki.CollectionManager +import com.ichi2.anki.IntentHandler +import com.ichi2.anki.Reviewer +import com.ichi2.anki.RobolectricTest +import com.ichi2.anki.backupLock +import com.ichi2.anki.common.destinations.ReviewDeckDestination +import com.ichi2.anki.multiprofile.ProfileId +import com.ichi2.anki.multiprofile.ProfileManager +import com.ichi2.anki.multiprofile.ProfileName +import com.ichi2.anki.multiprofile.ProfileSwitchGuard +import com.ichi2.anki.multiprofile.safeRestartApp +import com.ichi2.anki.toIntent +import com.ichi2.anki.worker.mediaSyncLock +import com.ichi2.anki.worker.syncLock +import kotlinx.coroutines.test.advanceUntilIdle +import org.junit.After +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.Robolectric +import org.robolectric.Shadows.shadowOf +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +@SdkSuppress(minSdkVersion = Build.VERSION_CODES.O) +@RunWith(AndroidJUnit4::class) +class SwitchProfilesShortcutsReviewTest : RobolectricTest() { + override fun getCollectionStorageMode() = CollectionStorageMode.ON_DISK + + @After + fun releaseRestartLocks() { + listOf(syncLock, mediaSyncLock, backupLock).filter { it.isLocked }.forEach { it.unlock() } + } + + @Test + fun `a shortcut opens the originating deck before switching`() { + col.decks.rename(1L, "Private French") + addBasicNote("French question", "French answer") + val shortcut = pinShortcut() + + assertTrue(launchPinnedShortcut(shortcut).any { it.component?.className == Reviewer::class.java.name }) + assertEquals("Private French", col.decks.getLegacy(col.decks.selected())!!.name) + } + + @Test + fun `a Default shortcut must not review Works deck with the same id`() = + runTest { + val app = AnkiDroidApp.instance + val originalManager = requireNotNull(app.profileManager) + col.decks.rename(1L, "Private French") + addBasicNote("French question", "French answer") + // This is the exact destination stored by DeckPicker.createIcon. + val shortcut = pinShortcut() + val target = originalManager.createNewProfile((ProfileName.validate("Work") as ProfileName.ValidationResult.Valid).name) + val model = + SwitchProfilesViewModel(originalManager, ProfileSwitchGuard(originalManager, emptyList())) { + safeRestartApp(app) + // Recreate the profile and backend as they would be after process restart. + CollectionManager.discardBackend() + releaseRestartLocks() + app.profileManager = + ProfileManager.create( + object : ContextWrapper(app.baseContext) { + override fun getApplicationContext(): Context? = null + }, + ) + } + try { + model.requestSwitch(model.profiles.value.single { it.id == target }) + model.switchProfile() + advanceUntilIdle() + assertEquals(target, app.profileManager!!.activeProfileContext.profileId) + assertEquals(0, col.noteCount(), "Work must have its own collection") + col.decks.rename(1L, "Work Spanish") + addBasicNote("Spanish question", "Spanish answer") + + val launched = launchPinnedShortcut(shortcut) + val wrongProfileReviewed = + app.profileManager!!.activeProfileContext.profileId != ProfileId.DEFAULT && + launched.any { it.component?.className == Reviewer::class.java.name } + assertFalse( + wrongProfileReviewed, + "A Private French shortcut opened ${col.decks.getLegacy(col.decks.selected())!!.name} in Work", + ) + } finally { + releaseRestartLocks() + CollectionManager.discardBackend() + app.profileManager = originalManager + } + } + + private fun pinShortcut(): String { + val manager = targetContext.getSystemService(ShortcutManager::class.java) + shadowOf(manager).setIsRequestPinShortcutSupported(true) + val shortcut = + ShortcutInfo + .Builder(targetContext, "1") + .setShortLabel("Private French") + .setIntent(ReviewDeckDestination.ExternalLaunch(1L).toIntent(targetContext)) + .build() + assertTrue(manager.requestPinShortcut(shortcut, null)) + return shortcut.id + } + + private fun launchPinnedShortcut(id: String): List { + val manager = targetContext.getSystemService(ShortcutManager::class.java) + val shortcut = manager.pinnedShortcuts.single { it.id == id } + return if (shortcut.isEnabled) launchShortcut(requireNotNull(shortcut.intent)) else emptyList() + } + + private fun launchShortcut(intent: Intent): List = + Robolectric.buildActivity(IntentHandler::class.java, intent).use { controller -> + controller.create() + buildList { + val activity = shadowOf(controller.get()) + while (true) add(activity.nextStartedActivity ?: break) + } + } +} diff --git a/common/android/src/main/res/values/02-strings.xml b/common/android/src/main/res/values/02-strings.xml index 4f48f3351e34..2e2af1cc4482 100644 --- a/common/android/src/main/res/values/02-strings.xml +++ b/common/android/src/main/res/values/02-strings.xml @@ -366,6 +366,7 @@ Deck deleted. Please remove the shortcut + Switch to the “%1$s” profile to study this deck Cards