From e42846313bebe1372ff6dc97f5cd10102a2bdbb6 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Wed, 31 Jan 2024 17:57:16 -0500 Subject: [PATCH 01/10] Move contentComposed to the end --- .../kotlin/com/slack/circuit/star/home/HomeScreen.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/samples/star/src/commonMain/kotlin/com/slack/circuit/star/home/HomeScreen.kt b/samples/star/src/commonMain/kotlin/com/slack/circuit/star/home/HomeScreen.kt index d7bf382d0..e1c9d37c1 100644 --- a/samples/star/src/commonMain/kotlin/com/slack/circuit/star/home/HomeScreen.kt +++ b/samples/star/src/commonMain/kotlin/com/slack/circuit/star/home/HomeScreen.kt @@ -15,9 +15,11 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.saveable.rememberSaveableStateHolder import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color +import com.slack.circuit.backstack.rememberSaveableBackStack import com.slack.circuit.codegen.annotations.CircuitInject import com.slack.circuit.foundation.CircuitContent import com.slack.circuit.foundation.NavEvent @@ -70,6 +72,7 @@ fun HomePresenter(navigator: Navigator): HomeScreen.State { @Composable fun HomeContent(state: HomeScreen.State, modifier: Modifier = Modifier) { var contentComposed by rememberRetained { mutableStateOf(false) } + Scaffold( modifier = modifier.fillMaxWidth(), contentWindowInsets = WindowInsets(0, 0, 0, 0), @@ -82,13 +85,13 @@ fun HomeContent(state: HomeScreen.State, modifier: Modifier = Modifier) { } }, ) { paddingValues -> - contentComposed = true val screen = state.navItems[state.selectedIndex].screen CircuitContent( screen, modifier = Modifier.padding(paddingValues), onNavEvent = { event -> state.eventSink(ChildNav(event)) }, ) + contentComposed = true } Platform.ReportDrawnWhen { contentComposed } } From a8544ae134ddcd6d98862219affed51408f1f8cb Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Wed, 31 Jan 2024 18:26:52 -0500 Subject: [PATCH 02/10] WIP persist tab state This is just some tinkering to resolve #116 and after talking with @BryanStern. Wanna iterate on this more and see if we can make this simpler, and also possibly explore implementing an analogous `RetainedStateHolder`. Another thing this highlights is the fact that we always initially show a progress indicator even when restoring from a loaded state, wonder what we can do to improve that. --- .../circuit/foundation/CircuitContent.kt | 20 +---------- .../com/slack/circuit/foundation/NavEvent.kt | 25 ++++++++++++++ .../com/slack/circuit/runtime/Navigator.kt | 2 ++ .../com/slack/circuit/star/home/HomeScreen.kt | 33 +++++++++++++++---- 4 files changed, 54 insertions(+), 26 deletions(-) diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt index b5611cb9f..9306904b8 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt @@ -39,25 +39,7 @@ public fun CircuitContent( ) { val navigator = remember(onNavEvent) { - object : Navigator { - override fun goTo(screen: Screen) { - onNavEvent(NavEvent.GoTo(screen)) - } - - override fun resetRoot(newRoot: Screen): List { - onNavEvent(NavEvent.ResetRoot(newRoot)) - return emptyList() - } - - override fun pop(): Screen? { - onNavEvent(NavEvent.Pop) - return null - } - - override fun peek(): Screen { - return screen - } - } + Navigator.navEventNavigator(screen, onNavEvent) } CircuitContent(screen, navigator, modifier, circuit, unavailableContent) } diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavEvent.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavEvent.kt index bc373bd36..9f66eed2b 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavEvent.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavEvent.kt @@ -19,6 +19,31 @@ public fun Navigator.onNavEvent(event: NavEvent) { } } +public fun Navigator.Companion.navEventNavigator( + screen: Screen, + onNavEvent: (event: NavEvent) -> Unit, +): Navigator { + return object : Navigator { + override fun goTo(screen: Screen) { + onNavEvent(NavEvent.GoTo(screen)) + } + + override fun resetRoot(newRoot: Screen): List { + onNavEvent(NavEvent.ResetRoot(newRoot)) + return emptyList() + } + + override fun pop(): Screen? { + onNavEvent(NavEvent.Pop) + return null + } + + override fun peek(): Screen { + return screen + } + } +} + /** A sealed navigation interface intended to be used when making a navigation callback. */ public sealed interface NavEvent : CircuitUiEvent { /** Corresponds to [Navigator.pop]. */ diff --git a/circuit-runtime/src/commonMain/kotlin/com/slack/circuit/runtime/Navigator.kt b/circuit-runtime/src/commonMain/kotlin/com/slack/circuit/runtime/Navigator.kt index b4ab03287..c7aac015b 100644 --- a/circuit-runtime/src/commonMain/kotlin/com/slack/circuit/runtime/Navigator.kt +++ b/circuit-runtime/src/commonMain/kotlin/com/slack/circuit/runtime/Navigator.kt @@ -43,6 +43,8 @@ public interface Navigator { override fun resetRoot(newRoot: Screen): List = emptyList() } + + public companion object } /** Calls [Navigator.pop] until the given [predicate] is matched or it pops the root. */ diff --git a/samples/star/src/commonMain/kotlin/com/slack/circuit/star/home/HomeScreen.kt b/samples/star/src/commonMain/kotlin/com/slack/circuit/star/home/HomeScreen.kt index e1c9d37c1..1d2243120 100644 --- a/samples/star/src/commonMain/kotlin/com/slack/circuit/star/home/HomeScreen.kt +++ b/samples/star/src/commonMain/kotlin/com/slack/circuit/star/home/HomeScreen.kt @@ -19,11 +19,14 @@ import androidx.compose.runtime.saveable.rememberSaveableStateHolder import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color -import com.slack.circuit.backstack.rememberSaveableBackStack import com.slack.circuit.codegen.annotations.CircuitInject import com.slack.circuit.foundation.CircuitContent +import com.slack.circuit.foundation.LocalCircuit import com.slack.circuit.foundation.NavEvent +import com.slack.circuit.foundation.navEventNavigator import com.slack.circuit.foundation.onNavEvent +import com.slack.circuit.foundation.rememberPresenter +import com.slack.circuit.foundation.rememberUi import com.slack.circuit.retained.rememberRetained import com.slack.circuit.runtime.CircuitUiEvent import com.slack.circuit.runtime.CircuitUiState @@ -85,12 +88,28 @@ fun HomeContent(state: HomeScreen.State, modifier: Modifier = Modifier) { } }, ) { paddingValues -> - val screen = state.navItems[state.selectedIndex].screen - CircuitContent( - screen, - modifier = Modifier.padding(paddingValues), - onNavEvent = { event -> state.eventSink(ChildNav(event)) }, - ) + val saveableStateHolder = rememberSaveableStateHolder() + val currentScreen = state.navItems[state.selectedIndex].screen + saveableStateHolder.SaveableStateProvider(currentScreen) { + val circuit = requireNotNull(LocalCircuit.current) + val ui = rememberUi(currentScreen, factory = circuit::ui) + val presenter = + rememberPresenter( + currentScreen, + navigator = + Navigator.navEventNavigator(currentScreen) { event -> + state.eventSink(ChildNav(event)) + }, + factory = circuit::presenter, + ) + + CircuitContent( + screen = currentScreen, + modifier = Modifier.padding(paddingValues), + presenter = presenter!!, + ui = ui!!, + ) + } contentComposed = true } Platform.ReportDrawnWhen { contentComposed } From 13622e9df0a6132ec5c783b646dacfcc55ecfbf4 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Wed, 31 Jan 2024 18:27:33 -0500 Subject: [PATCH 03/10] Spotless --- .../kotlin/com/slack/circuit/foundation/CircuitContent.kt | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt index 9306904b8..241abf8b9 100644 --- a/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt +++ b/circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt @@ -37,10 +37,7 @@ public fun CircuitContent( unavailableContent: (@Composable (screen: Screen, modifier: Modifier) -> Unit) = circuit.onUnavailableContent, ) { - val navigator = - remember(onNavEvent) { - Navigator.navEventNavigator(screen, onNavEvent) - } + val navigator = remember(onNavEvent) { Navigator.navEventNavigator(screen, onNavEvent) } CircuitContent(screen, navigator, modifier, circuit, unavailableContent) } From dca28da121ad86b143cd9b7aa594427985903876 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Fri, 2 Feb 2024 16:33:11 -0500 Subject: [PATCH 04/10] Add RetainedStateHolder impls --- build.gradle.kts | 1 + .../android/RetainedStateHolderTest.kt | 318 ++++++++++++++++++ .../circuit/retained/AndroidContinuity.kt | 4 +- .../circuit/retained/RetainedStateHolder.kt | 111 ++++++ .../circuit/retained/RetainedStateRegistry.kt | 7 +- 5 files changed, 435 insertions(+), 6 deletions(-) create mode 100644 circuit-retained/src/androidInstrumentedTest/kotlin/com/slack/circuit/retained/android/RetainedStateHolderTest.kt create mode 100644 circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RetainedStateHolder.kt diff --git a/build.gradle.kts b/build.gradle.kts index a6ef925ec..3ede8b973 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -100,6 +100,7 @@ allprojects { "**/FilterList.kt", "**/Remove.kt", "**/Pets.kt", + "**/RetainedStateHolder*.kt", ) } } diff --git a/circuit-retained/src/androidInstrumentedTest/kotlin/com/slack/circuit/retained/android/RetainedStateHolderTest.kt b/circuit-retained/src/androidInstrumentedTest/kotlin/com/slack/circuit/retained/android/RetainedStateHolderTest.kt new file mode 100644 index 000000000..6e85e197b --- /dev/null +++ b/circuit-retained/src/androidInstrumentedTest/kotlin/com/slack/circuit/retained/android/RetainedStateHolderTest.kt @@ -0,0 +1,318 @@ +/* + * Copyright 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.slack.circuit.retained.android + +import android.os.Bundle +import androidx.activity.ComponentActivity +import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.MutableState +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.test.junit4.StateRestorationTester +import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat +import com.slack.circuit.retained.LocalRetainedStateRegistry +import com.slack.circuit.retained.RetainedStateHolder +import com.slack.circuit.retained.RetainedStateRegistry +import com.slack.circuit.retained.rememberRetained +import com.slack.circuit.retained.rememberRetainedStateHolder +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +// TODO adapt for retained more +@RunWith(AndroidJUnit4::class) +class RetainedStateHolderTest { + + @get:Rule val rule = createAndroidComposeRule() + + private val restorationTester = StateRestorationTester(rule) + + @Test + fun stateIsRestoredWhenGoBackToScreen1() { + var increment = 0 + var screen by mutableStateOf(Screens.Screen1) + var numberOnScreen1 = -1 + var restorableNumberOnScreen1 = -1 + restorationTester.setContent { + val holder = rememberRetainedStateHolder() + holder.RetainedStateProvider(screen) { + if (screen == Screens.Screen1) { + numberOnScreen1 = remember { increment++ } + restorableNumberOnScreen1 = rememberRetained { increment++ } + } else { + // screen 2 + remember { 100 } + } + } + } + + rule.runOnIdle { + assertThat(numberOnScreen1).isEqualTo(0) + assertThat(restorableNumberOnScreen1).isEqualTo(1) + screen = Screens.Screen2 + } + + // wait for the screen switch to apply + rule.runOnIdle { + numberOnScreen1 = -1 + restorableNumberOnScreen1 = -1 + // switch back to screen1 + screen = Screens.Screen1 + } + + rule.runOnIdle { + assertThat(numberOnScreen1).isEqualTo(2) + assertThat(restorableNumberOnScreen1).isEqualTo(1) + } + } + + @Test + fun simpleRestoreOnlyOneScreen() { + var increment = 0 + var number = -1 + var restorableNumber = -1 + restorationTester.setContent { + val holder = rememberRetainedStateHolder() + holder.RetainedStateProvider(Screens.Screen1) { + number = remember { increment++ } + restorableNumber = rememberRetained { increment++ } + } + } + + rule.runOnIdle { + assertThat(number).isEqualTo(0) + assertThat(restorableNumber).isEqualTo(1) + number = -1 + restorableNumber = -1 + } + + restorationTester.emulateSavedInstanceStateRestore() + + rule.runOnIdle { + assertThat(number).isEqualTo(2) + assertThat(restorableNumber).isEqualTo(1) + } + } + + @Test + fun switchToScreen2AndRestore() { + var increment = 0 + var screen by mutableStateOf(Screens.Screen1) + var numberOnScreen2 = -1 + var restorableNumberOnScreen2 = -1 + restorationTester.setContent { + val holder = rememberRetainedStateHolder() + holder.RetainedStateProvider(screen) { + if (screen == Screens.Screen2) { + numberOnScreen2 = remember { increment++ } + restorableNumberOnScreen2 = rememberRetained { increment++ } + } + } + } + + rule.runOnIdle { screen = Screens.Screen2 } + + // wait for the screen switch to apply + rule.runOnIdle { + assertThat(numberOnScreen2).isEqualTo(0) + assertThat(restorableNumberOnScreen2).isEqualTo(1) + numberOnScreen2 = -1 + restorableNumberOnScreen2 = -1 + } + + restorationTester.emulateSavedInstanceStateRestore() + + rule.runOnIdle { + assertThat(numberOnScreen2).isEqualTo(2) + assertThat(restorableNumberOnScreen2).isEqualTo(1) + } + } + + @Test + fun stateOfScreen1IsSavedAndRestoredWhileWeAreOnScreen2() { + var increment = 0 + var screen by mutableStateOf(Screens.Screen1) + var numberOnScreen1 = -1 + var restorableNumberOnScreen1 = -1 + restorationTester.setContent { + val holder = rememberRetainedStateHolder() + holder.RetainedStateProvider(screen) { + if (screen == Screens.Screen1) { + numberOnScreen1 = remember { increment++ } + restorableNumberOnScreen1 = rememberRetained { increment++ } + } else { + // screen 2 + remember { 100 } + } + } + } + + rule.runOnIdle { + assertThat(numberOnScreen1).isEqualTo(0) + assertThat(restorableNumberOnScreen1).isEqualTo(1) + screen = Screens.Screen2 + } + + // wait for the screen switch to apply + rule.runOnIdle { + numberOnScreen1 = -1 + restorableNumberOnScreen1 = -1 + } + + restorationTester.emulateSavedInstanceStateRestore() + + // switch back to screen1 + rule.runOnIdle { screen = Screens.Screen1 } + + rule.runOnIdle { + assertThat(numberOnScreen1).isEqualTo(2) + assertThat(restorableNumberOnScreen1).isEqualTo(1) + } + } + + @Test + fun weCanSkipSavingForCurrentScreen() { + var increment = 0 + var screen by mutableStateOf(Screens.Screen1) + var restorableStateHolder: RetainedStateHolder? = null + var restorableNumberOnScreen1 = -1 + restorationTester.setContent { + val holder = rememberRetainedStateHolder() + restorableStateHolder = holder + holder.RetainedStateProvider(screen) { + if (screen == Screens.Screen1) { + restorableNumberOnScreen1 = rememberRetained { increment++ } + } else { + // screen 2 + remember { 100 } + } + } + } + + rule.runOnIdle { + assertThat(restorableNumberOnScreen1).isEqualTo(0) + restorableNumberOnScreen1 = -1 + restorableStateHolder!!.removeState(Screens.Screen1) + screen = Screens.Screen2 + } + + rule.runOnIdle { + // switch back to screen1 + screen = Screens.Screen1 + } + + rule.runOnIdle { assertThat(restorableNumberOnScreen1).isEqualTo(1) } + } + + @Test + fun weCanRemoveAlreadySavedState() { + var increment = 0 + var screen by mutableStateOf(Screens.Screen1) + var restorableStateHolder: RetainedStateHolder? = null + var restorableNumberOnScreen1 = -1 + restorationTester.setContent { + val holder = rememberRetainedStateHolder() + restorableStateHolder = holder + holder.RetainedStateProvider(screen) { + if (screen == Screens.Screen1) { + restorableNumberOnScreen1 = rememberRetained { increment++ } + } else { + // screen 2 + remember { 100 } + } + } + } + + rule.runOnIdle { + assertThat(restorableNumberOnScreen1).isEqualTo(0) + restorableNumberOnScreen1 = -1 + screen = Screens.Screen2 + } + + rule.runOnIdle { + // switch back to screen1 + restorableStateHolder!!.removeState(Screens.Screen1) + screen = Screens.Screen1 + } + + rule.runOnIdle { assertThat(restorableNumberOnScreen1).isEqualTo(1) } + } + + @Test + fun restoringStateOfThePreviousPageAfterCreatingBundle() { + var showFirstPage by mutableStateOf(true) + var firstPageState: MutableState? = null + + rule.setContent { + val holder = rememberRetainedStateHolder() + holder.RetainedStateProvider(showFirstPage) { + if (showFirstPage) { + firstPageState = rememberRetained { mutableStateOf(0) } + } + } + } + + rule.runOnIdle { + assertThat(firstPageState!!.value).isEqualTo(0) + // change the value, so we can assert this change will be restored + firstPageState!!.value = 1 + firstPageState = null + showFirstPage = false + } + + rule.runOnIdle { + rule.activity.doFakeSave() + showFirstPage = true + } + + rule.runOnIdle { assertThat(firstPageState!!.value).isEqualTo(1) } + } + + @Test + fun saveNothingWhenNoRememberRetainedIsUsedInternally() { + var showFirstPage by mutableStateOf(true) + val registry = RetainedStateRegistry(emptyMap()) + + rule.setContent { + CompositionLocalProvider(LocalRetainedStateRegistry provides registry) { + val holder = rememberRetainedStateHolder() + holder.RetainedStateProvider(showFirstPage) {} + } + } + + rule.runOnIdle { showFirstPage = false } + + rule.runOnIdle { + val savedData = registry.saveAll() + assertThat(savedData).isEqualTo(emptyMap>()) + } + } + + class Activity : ComponentActivity() { + fun doFakeSave() { + onSaveInstanceState(Bundle()) + } + } +} + +enum class Screens { + Screen1, + Screen2, +} diff --git a/circuit-retained/src/androidMain/kotlin/com/slack/circuit/retained/AndroidContinuity.kt b/circuit-retained/src/androidMain/kotlin/com/slack/circuit/retained/AndroidContinuity.kt index d942617fb..8c50d39f3 100644 --- a/circuit-retained/src/androidMain/kotlin/com/slack/circuit/retained/AndroidContinuity.kt +++ b/circuit-retained/src/androidMain/kotlin/com/slack/circuit/retained/AndroidContinuity.kt @@ -27,9 +27,7 @@ internal class ContinuityViewModel : ViewModel(), RetainedStateRegistry { return delegate.registerValue(key, valueProvider) } - override fun saveAll() { - delegate.saveAll() - } + override fun saveAll() = delegate.saveAll() override fun saveValue(key: String) { delegate.saveValue(key) diff --git a/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RetainedStateHolder.kt b/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RetainedStateHolder.kt new file mode 100644 index 000000000..9ded193fb --- /dev/null +++ b/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RetainedStateHolder.kt @@ -0,0 +1,111 @@ +/* + * Copyright 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.slack.circuit.retained + +import androidx.compose.runtime.Composable +import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.ReusableContent +import androidx.compose.runtime.remember + +/** + * Allows to retain the state defined with [rememberRetained] for the subtree before disposing it to + * make it possible to compose it back next time with the restored state. It allows different + * navigation patterns to keep the ui state like scroll position for the currently not composed + * screens from the backstack. + * + * The content should be composed using [RetainedStateProvider] while providing a key representing + * this content. Next time [RetainedStateProvider] will be used with the same key its state will be + * restored. + */ +public interface RetainedStateHolder { + /** + * Put your content associated with a [key] inside the [content]. This will automatically retain + * all the states defined with [rememberRetained] before disposing the content and will restore + * the states when you compose with this key again. + * + * @param key to be used for saving and restoring the states for the subtree. Note that on Android + * you can only use types which can be stored inside the Bundle. + */ + @Composable public fun RetainedStateProvider(key: Any, content: @Composable () -> Unit) + + /** Removes the retained state associated with the passed [key]. */ + public fun removeState(key: Any) +} + +/** Creates and remembers the instance of [RetainedStateHolder]. */ +@Composable +public fun rememberRetainedStateHolder(): RetainedStateHolder = + rememberRetained { RetainedStateHolderImpl() } + .apply { parentRetainedStateRegistry = LocalRetainedStateRegistry.current } + +private class RetainedStateHolderImpl( + private val retainedStates: MutableMap>> = mutableMapOf() +) : RetainedStateHolder { + private val registryHolders = mutableMapOf() + var parentRetainedStateRegistry: RetainedStateRegistry? = null + + @Composable + override fun RetainedStateProvider(key: Any, content: @Composable () -> Unit) { + ReusableContent(key) { + val registryHolder = remember { RegistryHolder(key) } + CompositionLocalProvider( + LocalRetainedStateRegistry provides registryHolder.registry, + content = content, + ) + DisposableEffect(Unit) { + require(key !in registryHolders) { "Key $key was used multiple times " } + retainedStates -= key + registryHolders[key] = registryHolder + onDispose { + registryHolder.saveTo(retainedStates) + registryHolders -= key + } + } + } + } + + private fun saveAll(): MutableMap>>? { + val map = retainedStates.toMutableMap() + registryHolders.values.forEach { it.saveTo(map) } + return map.ifEmpty { null } + } + + override fun removeState(key: Any) { + val registryHolder = registryHolders[key] + if (registryHolder != null) { + registryHolder.shouldSave = false + } else { + retainedStates -= key + } + } + + inner class RegistryHolder(val key: Any) { + var shouldSave = true + val registry: RetainedStateRegistry = RetainedStateRegistry(retainedStates[key].orEmpty()) + + fun saveTo(map: MutableMap>>) { + if (shouldSave) { + val retainedData = registry.saveAll() + if (retainedData.isEmpty()) { + map -= key + } else { + map[key] = retainedData + } + } + } + } +} diff --git a/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RetainedStateRegistry.kt b/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RetainedStateRegistry.kt index 70e5651c7..4dd386437 100644 --- a/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RetainedStateRegistry.kt +++ b/circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RetainedStateRegistry.kt @@ -40,7 +40,7 @@ public interface RetainedStateRegistry { * Executes all the registered value providers and combines these values into a map. We have a * list of values for each key as it is allowed to have multiple providers for the same key. */ - public fun saveAll() + public fun saveAll(): Map> /** Executes the value providers registered with the given [key], and saves them for retrieval. */ public fun saveValue(key: String) @@ -108,7 +108,7 @@ internal class RetainedStateRegistryImpl(retained: MutableMap } } - override fun saveAll() { + override fun saveAll(): Map> { val values = valueProviders.mapValues { (_, list) -> // If we have multiple providers we should store null values as well to preserve @@ -132,6 +132,7 @@ internal class RetainedStateRegistryImpl(retained: MutableMap } // Clear the value providers now that we've stored the values valueProviders.clear() + return values } override fun saveValue(key: String) { @@ -152,7 +153,7 @@ internal object NoOpRetainedStateRegistry : RetainedStateRegistry { override fun registerValue(key: String, valueProvider: RetainedValueProvider): Entry = NoOpEntry - override fun saveAll() {} + override fun saveAll(): Map> = emptyMap() override fun saveValue(key: String) {} From 73d98bd0f4c29b7137a459d0a5492a373f6a6e65 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Fri, 2 Feb 2024 16:36:15 -0500 Subject: [PATCH 05/10] Add leak canary wrapper rule --- .../android/RetainedStateHolderTest.kt | 67 ++++++++++--------- 1 file changed, 36 insertions(+), 31 deletions(-) diff --git a/circuit-retained/src/androidInstrumentedTest/kotlin/com/slack/circuit/retained/android/RetainedStateHolderTest.kt b/circuit-retained/src/androidInstrumentedTest/kotlin/com/slack/circuit/retained/android/RetainedStateHolderTest.kt index 6e85e197b..09e05154d 100644 --- a/circuit-retained/src/androidInstrumentedTest/kotlin/com/slack/circuit/retained/android/RetainedStateHolderTest.kt +++ b/circuit-retained/src/androidInstrumentedTest/kotlin/com/slack/circuit/retained/android/RetainedStateHolderTest.kt @@ -25,24 +25,29 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.test.junit4.StateRestorationTester import androidx.compose.ui.test.junit4.createAndroidComposeRule -import androidx.test.ext.junit.runners.AndroidJUnit4 import com.google.common.truth.Truth.assertThat import com.slack.circuit.retained.LocalRetainedStateRegistry import com.slack.circuit.retained.RetainedStateHolder import com.slack.circuit.retained.RetainedStateRegistry import com.slack.circuit.retained.rememberRetained import com.slack.circuit.retained.rememberRetainedStateHolder +import leakcanary.DetectLeaksAfterTestSuccess.Companion.detectLeaksAfterTestSuccessWrapping import org.junit.Rule import org.junit.Test -import org.junit.runner.RunWith +import org.junit.rules.RuleChain // TODO adapt for retained more -@RunWith(AndroidJUnit4::class) class RetainedStateHolderTest { - @get:Rule val rule = createAndroidComposeRule() + private val composeTestRule = createAndroidComposeRule() - private val restorationTester = StateRestorationTester(rule) + @get:Rule + val rule = + RuleChain.emptyRuleChain().detectLeaksAfterTestSuccessWrapping(tag = "ActivitiesDestroyed") { + around(composeTestRule) + } + + private val restorationTester = StateRestorationTester(composeTestRule) @Test fun stateIsRestoredWhenGoBackToScreen1() { @@ -63,21 +68,21 @@ class RetainedStateHolderTest { } } - rule.runOnIdle { + composeTestRule.runOnIdle { assertThat(numberOnScreen1).isEqualTo(0) assertThat(restorableNumberOnScreen1).isEqualTo(1) screen = Screens.Screen2 } // wait for the screen switch to apply - rule.runOnIdle { + composeTestRule.runOnIdle { numberOnScreen1 = -1 restorableNumberOnScreen1 = -1 // switch back to screen1 screen = Screens.Screen1 } - rule.runOnIdle { + composeTestRule.runOnIdle { assertThat(numberOnScreen1).isEqualTo(2) assertThat(restorableNumberOnScreen1).isEqualTo(1) } @@ -96,7 +101,7 @@ class RetainedStateHolderTest { } } - rule.runOnIdle { + composeTestRule.runOnIdle { assertThat(number).isEqualTo(0) assertThat(restorableNumber).isEqualTo(1) number = -1 @@ -105,7 +110,7 @@ class RetainedStateHolderTest { restorationTester.emulateSavedInstanceStateRestore() - rule.runOnIdle { + composeTestRule.runOnIdle { assertThat(number).isEqualTo(2) assertThat(restorableNumber).isEqualTo(1) } @@ -127,10 +132,10 @@ class RetainedStateHolderTest { } } - rule.runOnIdle { screen = Screens.Screen2 } + composeTestRule.runOnIdle { screen = Screens.Screen2 } // wait for the screen switch to apply - rule.runOnIdle { + composeTestRule.runOnIdle { assertThat(numberOnScreen2).isEqualTo(0) assertThat(restorableNumberOnScreen2).isEqualTo(1) numberOnScreen2 = -1 @@ -139,7 +144,7 @@ class RetainedStateHolderTest { restorationTester.emulateSavedInstanceStateRestore() - rule.runOnIdle { + composeTestRule.runOnIdle { assertThat(numberOnScreen2).isEqualTo(2) assertThat(restorableNumberOnScreen2).isEqualTo(1) } @@ -164,14 +169,14 @@ class RetainedStateHolderTest { } } - rule.runOnIdle { + composeTestRule.runOnIdle { assertThat(numberOnScreen1).isEqualTo(0) assertThat(restorableNumberOnScreen1).isEqualTo(1) screen = Screens.Screen2 } // wait for the screen switch to apply - rule.runOnIdle { + composeTestRule.runOnIdle { numberOnScreen1 = -1 restorableNumberOnScreen1 = -1 } @@ -179,9 +184,9 @@ class RetainedStateHolderTest { restorationTester.emulateSavedInstanceStateRestore() // switch back to screen1 - rule.runOnIdle { screen = Screens.Screen1 } + composeTestRule.runOnIdle { screen = Screens.Screen1 } - rule.runOnIdle { + composeTestRule.runOnIdle { assertThat(numberOnScreen1).isEqualTo(2) assertThat(restorableNumberOnScreen1).isEqualTo(1) } @@ -206,19 +211,19 @@ class RetainedStateHolderTest { } } - rule.runOnIdle { + composeTestRule.runOnIdle { assertThat(restorableNumberOnScreen1).isEqualTo(0) restorableNumberOnScreen1 = -1 restorableStateHolder!!.removeState(Screens.Screen1) screen = Screens.Screen2 } - rule.runOnIdle { + composeTestRule.runOnIdle { // switch back to screen1 screen = Screens.Screen1 } - rule.runOnIdle { assertThat(restorableNumberOnScreen1).isEqualTo(1) } + composeTestRule.runOnIdle { assertThat(restorableNumberOnScreen1).isEqualTo(1) } } @Test @@ -240,19 +245,19 @@ class RetainedStateHolderTest { } } - rule.runOnIdle { + composeTestRule.runOnIdle { assertThat(restorableNumberOnScreen1).isEqualTo(0) restorableNumberOnScreen1 = -1 screen = Screens.Screen2 } - rule.runOnIdle { + composeTestRule.runOnIdle { // switch back to screen1 restorableStateHolder!!.removeState(Screens.Screen1) screen = Screens.Screen1 } - rule.runOnIdle { assertThat(restorableNumberOnScreen1).isEqualTo(1) } + composeTestRule.runOnIdle { assertThat(restorableNumberOnScreen1).isEqualTo(1) } } @Test @@ -260,7 +265,7 @@ class RetainedStateHolderTest { var showFirstPage by mutableStateOf(true) var firstPageState: MutableState? = null - rule.setContent { + composeTestRule.setContent { val holder = rememberRetainedStateHolder() holder.RetainedStateProvider(showFirstPage) { if (showFirstPage) { @@ -269,7 +274,7 @@ class RetainedStateHolderTest { } } - rule.runOnIdle { + composeTestRule.runOnIdle { assertThat(firstPageState!!.value).isEqualTo(0) // change the value, so we can assert this change will be restored firstPageState!!.value = 1 @@ -277,12 +282,12 @@ class RetainedStateHolderTest { showFirstPage = false } - rule.runOnIdle { - rule.activity.doFakeSave() + composeTestRule.runOnIdle { + composeTestRule.activity.doFakeSave() showFirstPage = true } - rule.runOnIdle { assertThat(firstPageState!!.value).isEqualTo(1) } + composeTestRule.runOnIdle { assertThat(firstPageState!!.value).isEqualTo(1) } } @Test @@ -290,16 +295,16 @@ class RetainedStateHolderTest { var showFirstPage by mutableStateOf(true) val registry = RetainedStateRegistry(emptyMap()) - rule.setContent { + composeTestRule.setContent { CompositionLocalProvider(LocalRetainedStateRegistry provides registry) { val holder = rememberRetainedStateHolder() holder.RetainedStateProvider(showFirstPage) {} } } - rule.runOnIdle { showFirstPage = false } + composeTestRule.runOnIdle { showFirstPage = false } - rule.runOnIdle { + composeTestRule.runOnIdle { val savedData = registry.saveAll() assertThat(savedData).isEqualTo(emptyMap>()) } From 21b0e1556512e952b8761204af351fe1f2e5020f Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Fri, 2 Feb 2024 16:49:56 -0500 Subject: [PATCH 06/10] Add RetainedStateRestorationTester --- build.gradle.kts | 1 + .../android/RetainedStateHolderTest.kt | 8 +- .../android/RetainedStateRestorationTester.kt | 134 ++++++++++++++++++ 3 files changed, 139 insertions(+), 4 deletions(-) create mode 100644 circuit-retained/src/androidInstrumentedTest/kotlin/com/slack/circuit/retained/android/RetainedStateRestorationTester.kt diff --git a/build.gradle.kts b/build.gradle.kts index 3ede8b973..1ac4f4887 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -101,6 +101,7 @@ allprojects { "**/Remove.kt", "**/Pets.kt", "**/RetainedStateHolder*.kt", + "**/RetainedStateRestorationTester.kt", ) } } diff --git a/circuit-retained/src/androidInstrumentedTest/kotlin/com/slack/circuit/retained/android/RetainedStateHolderTest.kt b/circuit-retained/src/androidInstrumentedTest/kotlin/com/slack/circuit/retained/android/RetainedStateHolderTest.kt index 09e05154d..7eb457815 100644 --- a/circuit-retained/src/androidInstrumentedTest/kotlin/com/slack/circuit/retained/android/RetainedStateHolderTest.kt +++ b/circuit-retained/src/androidInstrumentedTest/kotlin/com/slack/circuit/retained/android/RetainedStateHolderTest.kt @@ -47,7 +47,7 @@ class RetainedStateHolderTest { around(composeTestRule) } - private val restorationTester = StateRestorationTester(composeTestRule) + private val restorationTester = RetainedStateRestorationTester(composeTestRule) @Test fun stateIsRestoredWhenGoBackToScreen1() { @@ -108,7 +108,7 @@ class RetainedStateHolderTest { restorableNumber = -1 } - restorationTester.emulateSavedInstanceStateRestore() + restorationTester.emulateRetainedInstanceStateRestore() composeTestRule.runOnIdle { assertThat(number).isEqualTo(2) @@ -142,7 +142,7 @@ class RetainedStateHolderTest { restorableNumberOnScreen2 = -1 } - restorationTester.emulateSavedInstanceStateRestore() + restorationTester.emulateRetainedInstanceStateRestore() composeTestRule.runOnIdle { assertThat(numberOnScreen2).isEqualTo(2) @@ -181,7 +181,7 @@ class RetainedStateHolderTest { restorableNumberOnScreen1 = -1 } - restorationTester.emulateSavedInstanceStateRestore() + restorationTester.emulateRetainedInstanceStateRestore() // switch back to screen1 composeTestRule.runOnIdle { screen = Screens.Screen1 } diff --git a/circuit-retained/src/androidInstrumentedTest/kotlin/com/slack/circuit/retained/android/RetainedStateRestorationTester.kt b/circuit-retained/src/androidInstrumentedTest/kotlin/com/slack/circuit/retained/android/RetainedStateRestorationTester.kt new file mode 100644 index 000000000..203d0b04c --- /dev/null +++ b/circuit-retained/src/androidInstrumentedTest/kotlin/com/slack/circuit/retained/android/RetainedStateRestorationTester.kt @@ -0,0 +1,134 @@ +/* + * Copyright 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.slack.circuit.retained.android + +import androidx.compose.runtime.Composable +import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.test.junit4.ComposeContentTestRule +import com.slack.circuit.retained.LocalRetainedStateRegistry +import com.slack.circuit.retained.RetainedStateRegistry +import com.slack.circuit.retained.RetainedValueProvider +import com.slack.circuit.retained.rememberRetained + +/** + * Helps to test the retained state restoration for your Composable component. + * + * Instead of calling [ComposeContentTestRule.setContent] you need to use [setContent] on this + * object, then change your state so there is some change to be restored, then execute + * [emulateRetainedInstanceStateRestore] and assert your state is restored properly. + * + * Note that this tests only the restoration of the local state of the composable you passed to + * [setContent] and useful for testing [rememberRetained] integration. It is not testing the + * integration with any other life cycles or Activity callbacks. + */ +// TODO recreate for more realism? Need to save the content function to do that, or call it after +// TODO make this available in test utils? +class RetainedStateRestorationTester(private val composeTestRule: ComposeContentTestRule) { + + private var registry: RestorationRegistry? = null + + /** + * This functions is a direct replacement for [ComposeContentTestRule.setContent] if you are going + * to use [emulateRetainedInstanceStateRestore] in the test. + * + * @see ComposeContentTestRule.setContent + */ + fun setContent(composable: @Composable () -> Unit) { + composeTestRule.setContent { + InjectRestorationRegistry { registry -> + this.registry = registry + composable() + } + } + } + + /** + * Saves all the state stored via [rememberRetained], disposes current composition, and composes + * again the content passed to [setContent]. Allows to test how your component behaves when the + * state restoration is happening. Note that the state stored via regular state() or remember() + * will be lost. + */ + fun emulateRetainedInstanceStateRestore() { + val registry = checkNotNull(registry) { "setContent should be called first!" } + composeTestRule.runOnIdle { registry.saveStateAndDisposeChildren() } + composeTestRule.runOnIdle { registry.emitChildrenWithRestoredState() } + composeTestRule.runOnIdle { + // we just wait for the children to be emitted + } + } + + @Composable + private fun InjectRestorationRegistry(content: @Composable (RestorationRegistry) -> Unit) { + val original = + requireNotNull(LocalRetainedStateRegistry.current) { + "StateRestorationTester requires composeTestRule.setContent() to provide " + + "a RetainedStateRegistry implementation via LocalRetainedStateRegistry" + } + val restorationRegistry = remember { RestorationRegistry(original) } + CompositionLocalProvider(LocalRetainedStateRegistry provides restorationRegistry) { + if (restorationRegistry.shouldEmitChildren) { + content(restorationRegistry) + } + } + } + + private class RestorationRegistry(private val original: RetainedStateRegistry) : + RetainedStateRegistry { + + var shouldEmitChildren by mutableStateOf(true) + private set + + private var currentRegistry: RetainedStateRegistry = original + private var savedMap: Map> = emptyMap() + + fun saveStateAndDisposeChildren() { + savedMap = currentRegistry.saveAll() + shouldEmitChildren = false + } + + fun emitChildrenWithRestoredState() { + currentRegistry = RetainedStateRegistry(values = savedMap) + shouldEmitChildren = true + } + + override fun consumeValue(key: String): Any? { + return currentRegistry.consumeValue(key) + } + + override fun registerValue( + key: String, + valueProvider: RetainedValueProvider, + ): RetainedStateRegistry.Entry { + return currentRegistry.registerValue(key, valueProvider) + } + + override fun saveAll(): Map> { + return currentRegistry.saveAll() + } + + override fun saveValue(key: String) { + currentRegistry.saveValue(key) + } + + override fun forgetUnclaimedValues() { + currentRegistry.forgetUnclaimedValues() + } + } +} From 068c1a854c1fe37775314b73cc293cb25580bfad Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Fri, 2 Feb 2024 16:50:28 -0500 Subject: [PATCH 07/10] Spotless --- .../slack/circuit/retained/android/RetainedStateHolderTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/circuit-retained/src/androidInstrumentedTest/kotlin/com/slack/circuit/retained/android/RetainedStateHolderTest.kt b/circuit-retained/src/androidInstrumentedTest/kotlin/com/slack/circuit/retained/android/RetainedStateHolderTest.kt index 7eb457815..51410093e 100644 --- a/circuit-retained/src/androidInstrumentedTest/kotlin/com/slack/circuit/retained/android/RetainedStateHolderTest.kt +++ b/circuit-retained/src/androidInstrumentedTest/kotlin/com/slack/circuit/retained/android/RetainedStateHolderTest.kt @@ -23,7 +23,6 @@ import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue -import androidx.compose.ui.test.junit4.StateRestorationTester import androidx.compose.ui.test.junit4.createAndroidComposeRule import com.google.common.truth.Truth.assertThat import com.slack.circuit.retained.LocalRetainedStateRegistry From 70a151200ddd0a7ae3f3cc765ca3e2ecda994423 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Fri, 2 Feb 2024 16:51:53 -0500 Subject: [PATCH 08/10] Use in HomeScreen --- .../kotlin/com/slack/circuit/star/home/HomeScreen.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/samples/star/src/commonMain/kotlin/com/slack/circuit/star/home/HomeScreen.kt b/samples/star/src/commonMain/kotlin/com/slack/circuit/star/home/HomeScreen.kt index 1d2243120..1e7f7b95c 100644 --- a/samples/star/src/commonMain/kotlin/com/slack/circuit/star/home/HomeScreen.kt +++ b/samples/star/src/commonMain/kotlin/com/slack/circuit/star/home/HomeScreen.kt @@ -28,6 +28,7 @@ import com.slack.circuit.foundation.onNavEvent import com.slack.circuit.foundation.rememberPresenter import com.slack.circuit.foundation.rememberUi import com.slack.circuit.retained.rememberRetained +import com.slack.circuit.retained.rememberRetainedStateHolder import com.slack.circuit.runtime.CircuitUiEvent import com.slack.circuit.runtime.CircuitUiState import com.slack.circuit.runtime.Navigator @@ -88,9 +89,9 @@ fun HomeContent(state: HomeScreen.State, modifier: Modifier = Modifier) { } }, ) { paddingValues -> - val saveableStateHolder = rememberSaveableStateHolder() + val saveableStateHolder = rememberRetainedStateHolder() val currentScreen = state.navItems[state.selectedIndex].screen - saveableStateHolder.SaveableStateProvider(currentScreen) { + saveableStateHolder.RetainedStateProvider(currentScreen) { val circuit = requireNotNull(LocalCircuit.current) val ui = rememberUi(currentScreen, factory = circuit::ui) val presenter = From b2d0e16077431a6e240541bd9f67d652baa29550 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Fri, 2 Feb 2024 17:28:49 -0500 Subject: [PATCH 09/10] Switch back --- .../kotlin/com/slack/circuit/star/home/HomeScreen.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/samples/star/src/commonMain/kotlin/com/slack/circuit/star/home/HomeScreen.kt b/samples/star/src/commonMain/kotlin/com/slack/circuit/star/home/HomeScreen.kt index 1e7f7b95c..049c598e1 100644 --- a/samples/star/src/commonMain/kotlin/com/slack/circuit/star/home/HomeScreen.kt +++ b/samples/star/src/commonMain/kotlin/com/slack/circuit/star/home/HomeScreen.kt @@ -89,9 +89,9 @@ fun HomeContent(state: HomeScreen.State, modifier: Modifier = Modifier) { } }, ) { paddingValues -> - val saveableStateHolder = rememberRetainedStateHolder() + val saveableStateHolder = rememberSaveableStateHolder() val currentScreen = state.navItems[state.selectedIndex].screen - saveableStateHolder.RetainedStateProvider(currentScreen) { + saveableStateHolder.SaveableStateProvider(currentScreen) { val circuit = requireNotNull(LocalCircuit.current) val ui = rememberUi(currentScreen, factory = circuit::ui) val presenter = From 924ecab6e8face787c7f3ce15e9445cc738951f5 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Fri, 2 Feb 2024 17:28:55 -0500 Subject: [PATCH 10/10] Add to manifest --- circuit-retained/src/androidInstrumentedTest/AndroidManifest.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/circuit-retained/src/androidInstrumentedTest/AndroidManifest.xml b/circuit-retained/src/androidInstrumentedTest/AndroidManifest.xml index ada8acbc5..73df06725 100644 --- a/circuit-retained/src/androidInstrumentedTest/AndroidManifest.xml +++ b/circuit-retained/src/androidInstrumentedTest/AndroidManifest.xml @@ -2,5 +2,6 @@ + \ No newline at end of file