diff --git a/app/build.gradle b/app/build.gradle index 39f1e49..393dc20 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -191,6 +191,7 @@ dependencies { implementation libs.androidx.activity.compose // Optional - Integration with ViewModels implementation libs.androidx.lifecycle.viewmodel.compose + implementation libs.androidx.lifecycle.viewmodel.navigation3 // Firebase implementation libs.firebase.analytics diff --git a/app/src/main/java/de/psdev/devdrawer/DevDrawerHost.kt b/app/src/main/java/de/psdev/devdrawer/DevDrawerHost.kt index 341cb82..2fa0695 100644 --- a/app/src/main/java/de/psdev/devdrawer/DevDrawerHost.kt +++ b/app/src/main/java/de/psdev/devdrawer/DevDrawerHost.kt @@ -72,7 +72,8 @@ fun DevDrawerHost( entry { key -> WidgetProfileEditor( profileId = key.id, - isNew = key.isNew + isNew = key.isNew, + onBack = { navigator.goBack() } ) } } diff --git a/app/src/main/java/de/psdev/devdrawer/NavigationState.kt b/app/src/main/java/de/psdev/devdrawer/NavigationState.kt index d9ea6d2..54cc4ce 100644 --- a/app/src/main/java/de/psdev/devdrawer/NavigationState.kt +++ b/app/src/main/java/de/psdev/devdrawer/NavigationState.kt @@ -9,6 +9,7 @@ import androidx.compose.runtime.saveable.rememberSerializable import androidx.compose.runtime.setValue import androidx.compose.runtime.snapshots.SnapshotStateList import androidx.compose.runtime.toMutableStateList +import androidx.lifecycle.viewmodel.navigation3.rememberViewModelStoreNavEntryDecorator import androidx.navigation3.runtime.NavBackStack import androidx.navigation3.runtime.NavEntry import androidx.navigation3.runtime.NavKey @@ -77,6 +78,9 @@ fun NavigationState.toEntries( val decoratedEntries = backStacks.mapValues { (_, stack) -> val decorators = listOf( rememberSaveableStateHolderNavEntryDecorator(), + // Each entry gets its own view models, cleared when it leaves the back stack; without this they were + // shared through the activity, so a second editor reused the first one's view model and arguments. + rememberViewModelStoreNavEntryDecorator(), ) rememberDecoratedNavEntries( backStack = stack, diff --git a/app/src/main/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditor.kt b/app/src/main/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditor.kt index d3c114f..3f3ea4e 100644 --- a/app/src/main/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditor.kt +++ b/app/src/main/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditor.kt @@ -33,6 +33,7 @@ import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf @@ -64,6 +65,7 @@ private const val COLLAPSED_APP_COUNT = 5 fun WidgetProfileEditor( profileId: String, isNew: Boolean = false, + onBack: () -> Unit = {}, modifier: Modifier = Modifier, viewModel: WidgetProfileEditorViewModel = hiltViewModel( creationCallback = { factory: WidgetProfileEditorViewModel.Factory -> @@ -79,7 +81,7 @@ fun WidgetProfileEditor( // Changes save as they are made; a name still being typed is saved when the editor closes. DisposableEffect(viewModel) { - onDispose { viewModel.onEditorClosed() } + onDispose { viewModel.onEditorHidden() } } Box(modifier = modifier.fillMaxSize()) { @@ -89,6 +91,7 @@ fun WidgetProfileEditor( onNameChange = viewModel::onNameChanged, onAddFilterClick = { currentDialog = WidgetProfileEditorDialogs.AddFilter }, onPackageFilterClick = { currentDialog = WidgetProfileEditorDialogs.PackageFilterInfo(it) }, + onMissing = onBack, onRemoveFilterClick = { filter -> viewModel.deleteFilter(filter) scope.launch { @@ -176,8 +179,13 @@ internal fun WidgetProfileEditor( onNameChange: (String) -> Unit = {}, onAddFilterClick: () -> Unit = {}, onPackageFilterClick: (PackageFilter) -> Unit = {}, - onRemoveFilterClick: (PackageFilter) -> Unit = {} + onRemoveFilterClick: (PackageFilter) -> Unit = {}, + onMissing: () -> Unit = {} ) { + if (viewState.isMissing) { + LaunchedEffect(Unit) { onMissing() } + return + } val widgetProfile = viewState.widgetProfile if (widgetProfile == null) { // Loading diff --git a/app/src/main/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditorViewModel.kt b/app/src/main/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditorViewModel.kt index f8e7813..96d64a2 100644 --- a/app/src/main/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditorViewModel.kt +++ b/app/src/main/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditorViewModel.kt @@ -16,6 +16,8 @@ import de.psdev.devdrawer.database.DevDrawerDatabase import de.psdev.devdrawer.database.FilterType import de.psdev.devdrawer.database.PackageFilter import de.psdev.devdrawer.profiles.IPackageFilterRepository +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.delay @@ -80,7 +82,8 @@ class WidgetProfileEditorViewModel @AssistedInject constructor( packageFilters = filters, matchingApps = matchingApps, filterAppCounts = filterAppCounts, - usedByWidgets = usedByWidgets + usedByWidgets = usedByWidgets, + isMissing = widgetProfile == null ) }.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5000), WidgetProfileEditorViewState.Empty) @@ -111,19 +114,26 @@ class WidgetProfileEditorViewModel @AssistedInject constructor( } } - /** - * Saves a name still waiting for typing to pause, and discards a new profile that was left untouched - * (no filters, no name typed) so backing out of "Create new profile" leaves nothing behind. - */ - fun onEditorClosed() { + /** Saves a name still waiting for typing to pause; called whenever the editor leaves the screen. */ + fun onEditorHidden() { val name = widgetNameState.value if (pendingNameSave?.isActive == true && name != null) { pendingNameSave?.cancel() saveName(name) } - if (isNew && name.isNullOrBlank()) { - write { - val profile = database.widgetProfileDao().findById(profileId) ?: return@write + } + + /** + * The editor was closed for good (its back stack entry is gone): discard a new profile that was left untouched + * (no filters, no name typed), so backing out of "Create new profile" leaves nothing behind. Not done when the + * editor is merely hidden, e.g. by switching tabs, since it comes back. + */ + override fun onCleared() { + if (!isNew || !widgetNameState.value.isNullOrBlank()) return + // viewModelScope is already cancelled here; the write must still happen, after any pending ones. + CoroutineScope(Dispatchers.IO).launch { + writeMutex.withLock { + val profile = database.widgetProfileDao().findById(profileId) ?: return@withLock if (database.packageFilterDao().findAllByProfile(profileId).isEmpty()) { database.widgetProfileDao().delete(profile) } diff --git a/app/src/main/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditorViewState.kt b/app/src/main/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditorViewState.kt index 257338a..1b8c9f6 100644 --- a/app/src/main/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditorViewState.kt +++ b/app/src/main/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditorViewState.kt @@ -15,7 +15,9 @@ data class WidgetProfileEditorViewState( /** How many installed apps each filter matches on its own, by filter id. */ val filterAppCounts: Map = emptyMap(), /** Names of the widgets showing this profile, which every edit affects. */ - val usedByWidgets: List = emptyList() + val usedByWidgets: List = emptyList(), + /** The profile was deleted (e.g. a new one discarded earlier); the editor leaves instead of loading forever. */ + val isMissing: Boolean = false ) { companion object { val Empty = WidgetProfileEditorViewState() diff --git a/app/src/test/java/de/psdev/devdrawer/NavigationViewModelScopeTest.kt b/app/src/test/java/de/psdev/devdrawer/NavigationViewModelScopeTest.kt new file mode 100644 index 0000000..0dca478 --- /dev/null +++ b/app/src/test/java/de/psdev/devdrawer/NavigationViewModelScopeTest.kt @@ -0,0 +1,92 @@ +package de.psdev.devdrawer + +import androidx.compose.material3.Text +import androidx.compose.runtime.remember +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.onNodeWithText +import androidx.lifecycle.ViewModel +import androidx.lifecycle.viewmodel.compose.viewModel +import androidx.navigation3.runtime.NavKey +import androidx.navigation3.runtime.entryProvider +import androidx.navigation3.ui.NavDisplay +import kotlinx.serialization.Serializable +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@Serializable +data class TestDetailRoute(val id: String) : NavKey + +class TestDetailViewModel(val id: String) : ViewModel() { + var cleared = false + override fun onCleared() { + cleared = true + } +} + +/** Each screen on the back stack must get its own view models, cleared when the screen is popped. */ +@RunWith(RobolectricTestRunner::class) +class NavigationViewModelScopeTest { + + @get:Rule + val composeTestRule = createComposeRule() + + private lateinit var navigator: Navigator + private val created = mutableListOf() + + private fun showApp() { + composeTestRule.setContent { + val state = rememberNavigationState(startRoute = WidgetListRoute, topLevelRoutes = setOf(WidgetListRoute, WidgetProfilesRoute)) + navigator = remember { Navigator(state) } + NavDisplay( + entries = state.toEntries( + entryProvider { + entry { Text("widgets") } + entry { Text("profiles") } + entry { key -> + val viewModel = viewModel { TestDetailViewModel(key.id).also { created += it } } + Text("detail ${viewModel.id}") + } + } + ), + onBack = { navigator.goBack() } + ) + } + } + + @Test + fun `given one detail screen was closed, when another opens, then it gets its own view model`() { + // Given + showApp() + composeTestRule.runOnIdle { navigator.navigate(TestDetailRoute("a")) } + composeTestRule.onNodeWithText("detail a").assertExists() + composeTestRule.runOnIdle { navigator.goBack() } + + // When + composeTestRule.runOnIdle { navigator.navigate(TestDetailRoute("b")) } + + // Then + composeTestRule.onNodeWithText("detail b").assertExists() + assertTrue(created.first { it.id == "a" }.cleared) + } + + @Test + fun `given a detail screen, when switching tabs and back, then its view model survives`() { + // Given + showApp() + composeTestRule.runOnIdle { navigator.navigate(WidgetProfilesRoute) } + composeTestRule.runOnIdle { navigator.navigate(TestDetailRoute("a")) } + composeTestRule.onNodeWithText("detail a").assertExists() + + // When + composeTestRule.runOnIdle { navigator.navigate(WidgetListRoute) } + composeTestRule.onNodeWithText("widgets").assertExists() + composeTestRule.runOnIdle { navigator.navigate(WidgetProfilesRoute) } + + // Then + composeTestRule.onNodeWithText("detail a").assertExists() + assertTrue(created.single().cleared.not()) + } +} diff --git a/app/src/test/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditorScreenTest.kt b/app/src/test/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditorScreenTest.kt index 159f639..c2676de 100644 --- a/app/src/test/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditorScreenTest.kt +++ b/app/src/test/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditorScreenTest.kt @@ -5,6 +5,7 @@ import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performTextReplacement import de.psdev.devdrawer.database.WidgetProfile import de.psdev.devdrawer.ui.theme.DevDrawerTheme +import org.junit.Assert.assertTrue import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -53,4 +54,20 @@ class WidgetProfileEditorScreenTest { // Then composeTestRule.onNodeWithText("Work apps").assertExists() } + + @Test + fun `given the profile no longer exists, when the editor shows, then it leaves instead of loading forever`() { + // Given + var left = false + + // When + composeTestRule.setContent { + DevDrawerTheme { + WidgetProfileEditor(viewState = WidgetProfileEditorViewState(isMissing = true), onMissing = { left = true }) + } + } + + // Then + composeTestRule.runOnIdle { assertTrue(left) } + } } diff --git a/app/src/test/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditorViewModelTest.kt b/app/src/test/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditorViewModelTest.kt index 8ff23e4..c9df6a9 100644 --- a/app/src/test/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditorViewModelTest.kt +++ b/app/src/test/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditorViewModelTest.kt @@ -1,5 +1,9 @@ package de.psdev.devdrawer.profiles.ui.editor +import androidx.lifecycle.ViewModelProvider +import androidx.lifecycle.ViewModelStore +import androidx.lifecycle.viewmodel.initializer +import androidx.lifecycle.viewmodel.viewModelFactory import androidx.room.Room import de.psdev.devdrawer.MainDispatcherRule import de.psdev.devdrawer.appwidget.PackageHashInfo @@ -10,15 +14,18 @@ import de.psdev.devdrawer.database.Widget import de.psdev.devdrawer.database.WidgetProfile import de.psdev.devdrawer.fakes.FakeAppsService import de.psdev.devdrawer.profiles.PackageFilterRepository +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.withContext +import kotlinx.coroutines.withTimeout import org.junit.After import org.junit.Assert.assertEquals -import org.junit.Assert.assertNull import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before @@ -62,8 +69,20 @@ class WidgetProfileEditorViewModelTest { database.close() } - private fun createViewModel(isNew: Boolean = false) = WidgetProfileEditorViewModel( - profileId = "p1", + /** A view model owned by a store, like a navigation entry's: clearing the store is the editor being closed. */ + private fun ownedViewModel(isNew: Boolean, profileId: String = "p1"): Pair { + val store = ViewModelStore() + val factory = viewModelFactory { initializer { createViewModel(isNew, profileId) } } + return ViewModelProvider.create(store, factory)[WidgetProfileEditorViewModel::class] to store + } + + /** Waits on real time for writes that run outside the test dispatcher (after the view model was cleared). */ + private suspend fun awaitDatabase(condition: suspend () -> Boolean) = withContext(Dispatchers.Default) { + withTimeout(5_000) { while (!condition()) delay(20) } + } + + private fun createViewModel(isNew: Boolean = false, profileId: String = "p1") = WidgetProfileEditorViewModel( + profileId = profileId, isNew = isNew, database = database, packageFilterRepository = PackageFilterRepository(RuntimeEnvironment.getApplication(), database), @@ -142,7 +161,7 @@ class WidgetProfileEditorViewModelTest { viewModel.onNameChanged("Work apps") // When - viewModel.onEditorClosed() + viewModel.onEditorHidden() // Then viewModel.state.first { it.widgetProfile?.name == "Work apps" } @@ -196,31 +215,50 @@ class WidgetProfileEditorViewModelTest { } @Test - fun `given a new profile left untouched, when the editor closes, then the profile is discarded`() = runTest { + fun `given a new profile left untouched, when the editor is only hidden by a tab switch, then the profile is kept`() = runTest { // Given database.packageFilterDao().delete(signatureFilter) - val viewModel = createViewModel(isNew = true) + val (viewModel, _) = ownedViewModel(isNew = true) backgroundScope.launch { viewModel.state.collect {} } viewModel.state.first { it.widgetProfile != null && it.packageFilters.isEmpty() } // When - viewModel.onEditorClosed() + viewModel.onEditorHidden() + withContext(Dispatchers.Default) { delay(300) } + + // Then + assertEquals("Mine", database.widgetProfileDao().findById("p1")?.name) + } + + @Test + fun `given a new profile left untouched, when the editor closes, then the profile is discarded`() = runTest { + // Given + database.packageFilterDao().delete(signatureFilter) + val (viewModel, store) = ownedViewModel(isNew = true) + val collector = backgroundScope.launch { viewModel.state.collect {} } + viewModel.state.first { it.widgetProfile != null && it.packageFilters.isEmpty() } + collector.cancel() + + // When + viewModel.onEditorHidden() + store.clear() // Then - viewModel.state.first { it.widgetProfile == null } - assertNull(database.widgetProfileDao().findById("p1")) + awaitDatabase { database.widgetProfileDao().findById("p1") == null } } @Test fun `given a new profile with a filter, when the editor closes, then the profile is kept`() = runTest { // Given - val viewModel = createViewModel(isNew = true) - backgroundScope.launch { viewModel.state.collect {} } + val (viewModel, store) = ownedViewModel(isNew = true) + val collector = backgroundScope.launch { viewModel.state.collect {} } viewModel.state.first { it.packageFilters.isNotEmpty() } + collector.cancel() // When - viewModel.onEditorClosed() - advanceUntilIdle() + viewModel.onEditorHidden() + store.clear() + withContext(Dispatchers.Default) { delay(300) } // Then assertEquals("Mine", database.widgetProfileDao().findById("p1")?.name) @@ -230,34 +268,48 @@ class WidgetProfileEditorViewModelTest { fun `given a new profile that was only renamed, when the editor closes, then the profile is kept`() = runTest { // Given database.packageFilterDao().delete(signatureFilter) - val viewModel = createViewModel(isNew = true) - backgroundScope.launch { viewModel.state.collect {} } + val (viewModel, store) = ownedViewModel(isNew = true) + val collector = backgroundScope.launch { viewModel.state.collect {} } viewModel.state.first { it.widgetProfile != null && it.packageFilters.isEmpty() } viewModel.onNameChanged("Clients") + collector.cancel() // When - viewModel.onEditorClosed() + viewModel.onEditorHidden() + store.clear() // Then - viewModel.state.first { it.widgetProfile?.name == "Clients" } + awaitDatabase { database.widgetProfileDao().findById("p1")?.name == "Clients" } } @Test fun `given an existing empty profile, when the editor closes, then it is kept`() = runTest { // Given database.packageFilterDao().delete(signatureFilter) - val viewModel = createViewModel(isNew = false) - backgroundScope.launch { viewModel.state.collect {} } + val (viewModel, store) = ownedViewModel(isNew = false) + val collector = backgroundScope.launch { viewModel.state.collect {} } viewModel.state.first { it.widgetProfile != null && it.packageFilters.isEmpty() } + collector.cancel() // When - viewModel.onEditorClosed() - advanceUntilIdle() + viewModel.onEditorHidden() + store.clear() + withContext(Dispatchers.Default) { delay(300) } // Then assertEquals("Mine", database.widgetProfileDao().findById("p1")?.name) } + @Test + fun `given a profile that no longer exists, when the editor loads, then it reports it as missing`() = runTest { + // Given + val viewModel = createViewModel(profileId = "gone") + backgroundScope.launch { viewModel.state.collect {} } + + // When / Then + viewModel.state.first { it.isMissing } + } + @Test fun `given widgets using the profile, when loaded, then the editor names them`() = runTest { // Given diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index e631634..a6cf4a7 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -46,6 +46,7 @@ androidx-hilt-lifecycle-viewmodel-compose = { module = "androidx.hilt:hilt-lifec androidx-hilt-work = { module = "androidx.hilt:hilt-work", version.ref = "hilt" } androidx-lifecycle-runtime-compose = { module = "androidx.lifecycle:lifecycle-runtime-compose", version.ref = "androidx-lifecycle" } androidx-lifecycle-viewmodel-compose = { module = "androidx.lifecycle:lifecycle-viewmodel-compose", version.ref = "androidx-lifecycle" } +androidx-lifecycle-viewmodel-navigation3 = { module = "androidx.lifecycle:lifecycle-viewmodel-navigation3", version.ref = "androidx-lifecycle" } androidx-material-icons-extended = { module = "androidx.compose.material:material-icons-extended" } androidx-material3 = { module = "androidx.compose.material3:material3" } androidx-preference-ktx = { module = "androidx.preference:preference-ktx", version.ref = "androidx-preference" }