diff --git a/mvrx-rxjava2/src/main/kotlin/com/airbnb/mvrx/BaseMvRxViewModel.kt b/mvrx-rxjava2/src/main/kotlin/com/airbnb/mvrx/BaseMvRxViewModel.kt index 48d0631b4..abe8039e0 100644 --- a/mvrx-rxjava2/src/main/kotlin/com/airbnb/mvrx/BaseMvRxViewModel.kt +++ b/mvrx-rxjava2/src/main/kotlin/com/airbnb/mvrx/BaseMvRxViewModel.kt @@ -26,8 +26,6 @@ import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach -import java.util.Collections -import java.util.concurrent.ConcurrentHashMap import kotlin.reflect.KProperty1 /** @@ -38,8 +36,6 @@ abstract class BaseMvRxViewModel( ) : MavericksViewModel(initialState) { private val tag by lazy { javaClass.simpleName } private val disposables = CompositeDisposable() - private val lastDeliveredStates = ConcurrentHashMap() - private val activeSubscriptions = Collections.newSetFromMap(ConcurrentHashMap()) /** * Define a [LifecycleOwner] to control subscriptions between [BaseMvRxViewModel]s. This only diff --git a/mvrx-rxjava2/src/test/kotlin/com/airbnb/mvrx/ViewModelSubscriberTest.kt b/mvrx-rxjava2/src/test/kotlin/com/airbnb/mvrx/ViewModelSubscriberTest.kt index 47fea4d63..43e7cc13c 100644 --- a/mvrx-rxjava2/src/test/kotlin/com/airbnb/mvrx/ViewModelSubscriberTest.kt +++ b/mvrx-rxjava2/src/test/kotlin/com/airbnb/mvrx/ViewModelSubscriberTest.kt @@ -12,7 +12,6 @@ import kotlinx.coroutines.cancel import kotlinx.coroutines.delay import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach -import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runBlockingTest import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -673,7 +672,7 @@ class ViewModelSubscriberTest : BaseTest() { } @Test - fun testSubscribeNotCalledOnStartIfNoUpdateOccurredInStop() { + fun testUniqueOnlySubscribeNotCalledOnStartIfNoUpdateOccurredInStop() { owner.lifecycle.currentState = Lifecycle.State.STARTED var callCount = 0 diff --git a/mvrx/src/main/kotlin/com/airbnb/mvrx/DeliveryMode.kt b/mvrx/src/main/kotlin/com/airbnb/mvrx/DeliveryMode.kt index 4680e2e75..1e4453593 100644 --- a/mvrx/src/main/kotlin/com/airbnb/mvrx/DeliveryMode.kt +++ b/mvrx/src/main/kotlin/com/airbnb/mvrx/DeliveryMode.kt @@ -7,6 +7,8 @@ import kotlin.reflect.KProperty1 * See: [RedeliverOnStart], [UniqueOnly]. */ sealed class DeliveryMode { + abstract val subscriptionId: String + internal fun appendPropertiesToId(vararg properties: KProperty1<*, *>): DeliveryMode { return when (this) { is RedeliverOnStart -> RedeliverOnStart @@ -21,11 +23,13 @@ sealed class DeliveryMode { * * Likewise, when a [MavericksView] resubscribes after a configuration change the most recent update will always be emitted. */ -object RedeliverOnStart : DeliveryMode() +object RedeliverOnStart : DeliveryMode() { + override val subscriptionId: String = RedeliverOnStart::javaClass.name +} /** * The subscription will receive the most recent state update when transitioning from locked to unlocked states (stopped -> started), - * only if the state has changed while locked. + * only if the state has changed while locked. This will include the initial state as a state update. * * Likewise, when a [MavericksView] resubscribes after a configuration change the most recent update will only be emitted * if the state has changed while locked. @@ -33,4 +37,4 @@ object RedeliverOnStart : DeliveryMode() * @param subscriptionId A uniqueIdentifier for this subscription. It is an error for two unique only subscriptions to * have the same id. */ -class UniqueOnly(val subscriptionId: String) : DeliveryMode() +class UniqueOnly(override val subscriptionId: String) : DeliveryMode() diff --git a/mvrx/src/main/kotlin/com/airbnb/mvrx/FlowExtensions.kt b/mvrx/src/main/kotlin/com/airbnb/mvrx/FlowExtensions.kt new file mode 100644 index 000000000..129a5a7ad --- /dev/null +++ b/mvrx/src/main/kotlin/com/airbnb/mvrx/FlowExtensions.kt @@ -0,0 +1,73 @@ +package com.airbnb.mvrx + +import androidx.lifecycle.DefaultLifecycleObserver +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.lifecycleScope +import kotlinx.coroutines.CoroutineStart +import kotlinx.coroutines.Job +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.collectLatest +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.dropWhile +import kotlinx.coroutines.flow.onCompletion +import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.launch +import kotlinx.coroutines.plus +import kotlinx.coroutines.yield +import java.util.concurrent.ConcurrentHashMap + +internal fun Flow.collectLatest( + lifecycleOwner: LifecycleOwner, + lastDeliveredStates: ConcurrentHashMap, + activeSubscriptions: MutableSet, + deliveryMode: DeliveryMode, + action: suspend (T) -> Unit +): Job { + val flow = when { + MavericksTestOverrides.FORCE_DISABLE_LIFECYCLE_AWARE_OBSERVER -> this + deliveryMode is UniqueOnly -> { + this.assertOneActiveSubscription(lifecycleOwner, activeSubscriptions, deliveryMode.subscriptionId) + .dropWhile { lastDeliveredStates.containsKey(deliveryMode.subscriptionId) && it == lastDeliveredStates[deliveryMode.subscriptionId] } + .flowWhenStarted(lifecycleOwner) + .distinctUntilChanged() + .onEach { lastDeliveredStates[deliveryMode.subscriptionId] = it } + } + else -> flowWhenStarted(lifecycleOwner) + } + + val scope = lifecycleOwner.lifecycleScope + Mavericks.viewModelConfigFactory.subscriptionCoroutineContextOverride + return scope.launch(start = CoroutineStart.UNDISPATCHED) { + // Use yield to ensure flow collect coroutine is dispatched rather than invoked immediately. + // This is necessary when Dispatchers.Main.immediate is used in scope. + // Coroutine is launched with start = CoroutineStart.UNDISPATCHED to perform dispatch only once. + yield() + flow.collectLatest(action) + } +} + +@Suppress("EXPERIMENTAL_API_USAGE") +internal fun Flow.assertOneActiveSubscription(lifecycleOwner: LifecycleOwner, activeSubscriptions: MutableSet, subscriptionId: String): Flow { + val observer = object : DefaultLifecycleObserver { + override fun onCreate(owner: LifecycleOwner) { + if (activeSubscriptions.contains(subscriptionId)) error(duplicateSubscriptionMessage(subscriptionId)) + activeSubscriptions += subscriptionId + } + + override fun onDestroy(owner: LifecycleOwner) { + activeSubscriptions.remove(subscriptionId) + } + } + + lifecycleOwner.lifecycle.addObserver(observer) + return onCompletion { + activeSubscriptions.remove(subscriptionId) + lifecycleOwner.lifecycle.removeObserver(observer) + } +} + +private fun duplicateSubscriptionMessage(subscriptionId: String) = """ + Subscribing with a duplicate subscription id: $subscriptionId. + If you have multiple uniqueOnly subscriptions in a MvRx view that listen to the same properties + you must use a custom subscription id. If you are using a custom MvRxView, make sure you are using the proper + lifecycle owner. See BaseMvRxFragment for an example. +""".trimIndent() diff --git a/mvrx/src/main/kotlin/com/airbnb/mvrx/MavericksLifecycleAwareFlow.kt b/mvrx/src/main/kotlin/com/airbnb/mvrx/MavericksLifecycleAwareFlow.kt index ea4a3d07c..42ff9bbbb 100644 --- a/mvrx/src/main/kotlin/com/airbnb/mvrx/MavericksLifecycleAwareFlow.kt +++ b/mvrx/src/main/kotlin/com/airbnb/mvrx/MavericksLifecycleAwareFlow.kt @@ -26,7 +26,7 @@ import kotlinx.coroutines.yield * It's possible because lifecycle state updated in the main thread * 2. Flow completes when either [this] flow completes or lifecycle is destroyed */ -fun Flow.flowWhenStarted(owner: LifecycleOwner): Flow = flow { +fun Flow.flowWhenStarted(owner: LifecycleOwner): Flow = flow { coroutineScope { val startedChannel = startedChannel(owner.lifecycle) val flowChannel = produce { collect { send(it) } } @@ -82,7 +82,7 @@ private fun startedChannel(owner: Lifecycle): Channel { return channel } -private inline fun SelectBuilder.onReceive( +private inline fun SelectBuilder.onReceive( channel: ReceiveChannel, crossinline onClosed: () -> Unit, noinline onReceive: suspend (value: T) -> Unit diff --git a/mvrx/src/main/kotlin/com/airbnb/mvrx/MavericksView.kt b/mvrx/src/main/kotlin/com/airbnb/mvrx/MavericksView.kt index 72adb169e..9f52e0a9b 100644 --- a/mvrx/src/main/kotlin/com/airbnb/mvrx/MavericksView.kt +++ b/mvrx/src/main/kotlin/com/airbnb/mvrx/MavericksView.kt @@ -8,6 +8,8 @@ import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.ViewModelStoreOwner +import kotlinx.coroutines.Job +import kotlinx.coroutines.flow.Flow import kotlin.reflect.KProperty1 // Set of [MavericksView identity hash codes that have a pending invalidate. @@ -42,11 +44,14 @@ interface MavericksView : LifecycleOwner { * Accessing mvrxViewId before calling super.onCreate() will cause a crash. */ val mvrxViewId: String + get() = mavericksViewInternalViewModel.mavericksViewId + + val mavericksViewInternalViewModel: MavericksViewInternalViewModel get() = when (this) { - is ViewModelStoreOwner -> ViewModelProvider(this).get(MavericksViewIdViewModel::class.java).mavericksViewId + is ViewModelStoreOwner -> ViewModelProvider(this).get(MavericksViewInternalViewModel::class.java) else -> error( - "If your MavericksView is not a ViewModelStoreOwner, you must implement mvrxViewId " + - "and return a string that is unique to this view and persistent across its entire lifecycle." + "If your MavericksView is not a ViewModelStoreOwner, you must implement mavericksViewInternalViewModel " + + "and return a MavericksViewInternalViewModel that is unique to this view and persistent across its entire lifecycle." ) } @@ -90,7 +95,7 @@ interface MavericksView : LifecycleOwner { * in this fragment with exact same properties (i.e. two subscribes, or two selectSubscribes with the same properties). */ fun uniqueOnly(customId: String? = null): UniqueOnly { - return UniqueOnly(listOfNotNull(mvrxViewId, customId).joinToString("_")) + return UniqueOnly(listOfNotNull(mvrxViewId, UniqueOnly::class.simpleName, customId).joinToString("_")) } /** @@ -104,7 +109,7 @@ interface MavericksView : LifecycleOwner { * Use [uniqueOnly] to automatically create a [UniqueOnly] mode with a unique id for this view. * * Default: [RedeliverOnStart]. - * @param action supports cooperative cancellation. The previous action will be cancelled if it as not completed before + * @param action supports cooperative cancellation. The previous action will be cancelled if it is not completed before * the next one is emitted. */ fun MavericksViewModel.onEach(deliveryMode: DeliveryMode = RedeliverOnStart, action: suspend (S) -> Unit) = @@ -122,7 +127,7 @@ interface MavericksView : LifecycleOwner { * Use [uniqueOnly] to automatically create a [UniqueOnly] mode with a unique id for this view. * * Default: [RedeliverOnStart]. - * @param action supports cooperative cancellation. The previous action will be cancelled if it as not completed before + * @param action supports cooperative cancellation. The previous action will be cancelled if it is not completed before * the next one is emitted. */ fun MavericksViewModel.onEach( @@ -142,7 +147,7 @@ interface MavericksView : LifecycleOwner { * Use [uniqueOnly] to automatically create a [UniqueOnly] mode with a unique id for this view. * * Default: [RedeliverOnStart] - * @param action supports cooperative cancellation. The previous action will be cancelled if it as not completed before + * @param action supports cooperative cancellation. The previous action will be cancelled if it is not completed before * the next one is emitted.. */ fun MavericksViewModel.onEach( @@ -163,7 +168,7 @@ interface MavericksView : LifecycleOwner { * Use [uniqueOnly] to automatically create a [UniqueOnly] mode with a unique id for this view. * * Default: [RedeliverOnStart]. - * @param action supports cooperative cancellation. The previous action will be cancelled if it as not completed before + * @param action supports cooperative cancellation. The previous action will be cancelled if it is not completed before * the next one is emitted. */ fun MavericksViewModel.onEach( @@ -185,7 +190,7 @@ interface MavericksView : LifecycleOwner { * Use [uniqueOnly] to automatically create a [UniqueOnly] mode with a unique id for this view. * * Default: [RedeliverOnStart]. - * @param action supports cooperative cancellation. The previous action will be cancelled if it as not completed before + * @param action supports cooperative cancellation. The previous action will be cancelled if it is not completed before * the next one is emitted. */ fun MavericksViewModel.onEach( @@ -208,7 +213,7 @@ interface MavericksView : LifecycleOwner { * Use [uniqueOnly] to automatically create a [UniqueOnly] mode with a unique id for this view. * * Default: [RedeliverOnStart]. - * @param action supports cooperative cancellation. The previous action will be cancelled if it as not completed before + * @param action supports cooperative cancellation. The previous action will be cancelled if it is not completed before * the next one is emitted. */ fun MavericksViewModel.onEach( @@ -232,7 +237,7 @@ interface MavericksView : LifecycleOwner { * Use [uniqueOnly] to automatically create a [UniqueOnly] mode with a unique id for this view. * * Default: [RedeliverOnStart]. - * @param action supports cooperative cancellation. The previous action will be cancelled if it as not completed before + * @param action supports cooperative cancellation. The previous action will be cancelled if it is not completed before * the next one is emitted. */ fun MavericksViewModel.onEach( @@ -257,7 +262,7 @@ interface MavericksView : LifecycleOwner { * Use [uniqueOnly] to automatically create a [UniqueOnly] mode with a unique id for this view. * * Default: [RedeliverOnStart]. - * @param action supports cooperative cancellation. The previous action will be cancelled if it as not completed before + * @param action supports cooperative cancellation. The previous action will be cancelled if it is not completed before * the next one is emitted. */ fun MavericksViewModel.onEach( @@ -295,4 +300,33 @@ interface MavericksView : LifecycleOwner { onFail: (suspend (Throwable) -> Unit)? = null, onSuccess: (suspend (T) -> Unit)? = null ) = _internalSF(subscriptionLifecycleOwner, asyncProp, deliveryMode, onFail, onSuccess) + + /** + * Subscribes to the given Flow within the coroutine scope of the `subscriptionLifecycleOwner`, starting the flow only when the lifecycle + * is started, and executing with the coroutine context of Mavericks' `subscriptionCoroutineContextOverride`. This can be utilized to create + * customized subscriptions, for example, to drop the first element in the flow before continuing. This is intended to be used with + * `viewmodel.stateflow`. + * + * @param deliveryMode If [UniqueOnly], when this [MavericksView] goes from a stopped to started lifecycle a value + * will only be emitted if the value has changed. This is useful for transient views that should only + * be shown once (toasts, poptarts), or logging. Most other views should use [RedeliverOnStart], as when a view is destroyed + * and recreated the previous state is necessary to recreate the view. + * + * Use [uniqueOnly] to automatically create a [UniqueOnly] mode with a unique id for this view. + * + * @param action supports cooperative cancellation. The previous action will be cancelled if it is not completed before + * the next one is emitted. + */ + fun Flow.collectLatest( + deliveryMode: DeliveryMode, + action: suspend (T) -> Unit + ): Job = mavericksViewInternalViewModel.let { + collectLatest( + subscriptionLifecycleOwner, + it.lastDeliveredStates, + it.activeSubscriptions, + deliveryMode, + action + ) + } } diff --git a/mvrx/src/main/kotlin/com/airbnb/mvrx/MavericksViewIdViewModel.kt b/mvrx/src/main/kotlin/com/airbnb/mvrx/MavericksViewIdViewModel.kt deleted file mode 100644 index 1595d11b4..000000000 --- a/mvrx/src/main/kotlin/com/airbnb/mvrx/MavericksViewIdViewModel.kt +++ /dev/null @@ -1,17 +0,0 @@ -package com.airbnb.mvrx - -import androidx.lifecycle.SavedStateHandle -import androidx.lifecycle.ViewModel -import java.util.UUID - -internal class MavericksViewIdViewModel(state: SavedStateHandle) : ViewModel() { - val mavericksViewId = state[PERSISTED_VIEW_ID_KEY] ?: generateUniqueId().also { id -> - state[PERSISTED_VIEW_ID_KEY] = id - } - - private fun generateUniqueId() = "MavericksView_" + UUID.randomUUID().toString() - - companion object { - private const val PERSISTED_VIEW_ID_KEY = "mavericks:persisted_view_id" - } -} diff --git a/mvrx/src/main/kotlin/com/airbnb/mvrx/MavericksViewInternalViewModel.kt b/mvrx/src/main/kotlin/com/airbnb/mvrx/MavericksViewInternalViewModel.kt new file mode 100644 index 000000000..b41b687fc --- /dev/null +++ b/mvrx/src/main/kotlin/com/airbnb/mvrx/MavericksViewInternalViewModel.kt @@ -0,0 +1,20 @@ +package com.airbnb.mvrx + +import androidx.lifecycle.SavedStateHandle +import androidx.lifecycle.ViewModel +import java.util.UUID +import java.util.concurrent.ConcurrentHashMap + +class MavericksViewInternalViewModel(state: SavedStateHandle) : ViewModel() { + internal val lastDeliveredStates: ConcurrentHashMap = ConcurrentHashMap() + internal val activeSubscriptions: MutableSet = mutableSetOf() + internal val mavericksViewId = state[PERSISTED_VIEW_ID_KEY] ?: generateUniqueId().also { id -> + state[PERSISTED_VIEW_ID_KEY] = id + } + + private fun generateUniqueId() = "MavericksView_" + UUID.randomUUID().toString() + + companion object { + private const val PERSISTED_VIEW_ID_KEY = "mavericks:persisted_view_id" + } +} diff --git a/mvrx/src/main/kotlin/com/airbnb/mvrx/MavericksViewModel.kt b/mvrx/src/main/kotlin/com/airbnb/mvrx/MavericksViewModel.kt index 321a03f44..30df5a512 100644 --- a/mvrx/src/main/kotlin/com/airbnb/mvrx/MavericksViewModel.kt +++ b/mvrx/src/main/kotlin/com/airbnb/mvrx/MavericksViewModel.kt @@ -1,9 +1,7 @@ package com.airbnb.mvrx import androidx.annotation.CallSuper -import androidx.lifecycle.DefaultLifecycleObserver import androidx.lifecycle.LifecycleOwner -import androidx.lifecycle.lifecycleScope import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineDispatcher @@ -16,10 +14,7 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.collectLatest -import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.dropWhile import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.flow.onCompletion import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch import kotlinx.coroutines.plus @@ -43,7 +38,8 @@ abstract class MavericksViewModel( ) { // Use the same factory for the life of the viewmodel, as it might change after this viewmodel is created (especially during tests) - private val configFactory = Mavericks.viewModelConfigFactory + @PublishedApi + internal val configFactory = Mavericks.viewModelConfigFactory @Suppress("LeakingThis") @InternalMavericksApi @@ -55,7 +51,7 @@ abstract class MavericksViewModel( val viewModelScope = config.coroutineScope private val stateStore = config.stateStore - private val lastDeliveredStates = ConcurrentHashMap() + private val lastDeliveredStates = ConcurrentHashMap() private val activeSubscriptions = Collections.newSetFromMap(ConcurrentHashMap()) private val tag by lazy { javaClass.simpleName } @@ -290,7 +286,7 @@ abstract class MavericksViewModel( /** * Subscribe to all state changes. * - * @param action supports cooperative cancellation. The previous action will be cancelled if it as not completed before + * @param action supports cooperative cancellation. The previous action will be cancelled if it is not completed before * the next one is emitted. */ protected fun onEach( @@ -300,7 +296,7 @@ abstract class MavericksViewModel( /** * Subscribe to state changes for a single property. * - * @param action supports cooperative cancellation. The previous action will be cancelled if it as not completed before + * @param action supports cooperative cancellation. The previous action will be cancelled if it is not completed before * the next one is emitted. */ protected fun onEach( @@ -311,7 +307,7 @@ abstract class MavericksViewModel( /** * Subscribe to state changes for two properties. * - * @param action supports cooperative cancellation. The previous action will be cancelled if it as not completed before + * @param action supports cooperative cancellation. The previous action will be cancelled if it is not completed before * the next one is emitted. */ protected fun onEach( @@ -323,7 +319,7 @@ abstract class MavericksViewModel( /** * Subscribe to state changes for three properties. * - * @param action supports cooperative cancellation. The previous action will be cancelled if it as not completed before + * @param action supports cooperative cancellation. The previous action will be cancelled if it is not completed before * the next one is emitted. */ protected fun onEach( @@ -336,7 +332,7 @@ abstract class MavericksViewModel( /** * Subscribe to state changes for four properties. * - * @param action supports cooperative cancellation. The previous action will be cancelled if it as not completed before + * @param action supports cooperative cancellation. The previous action will be cancelled if it is not completed before * the next one is emitted. */ protected fun onEach( @@ -350,7 +346,7 @@ abstract class MavericksViewModel( /** * Subscribe to state changes for five properties. * - * @param action supports cooperative cancellation. The previous action will be cancelled if it as not completed before + * @param action supports cooperative cancellation. The previous action will be cancelled if it is not completed before * the next one is emitted. */ protected fun onEach( @@ -365,7 +361,7 @@ abstract class MavericksViewModel( /** * Subscribe to state changes for six properties. * - * @param action supports cooperative cancellation. The previous action will be cancelled if it as not completed before + * @param action supports cooperative cancellation. The previous action will be cancelled if it is not completed before * the next one is emitted. */ protected fun onEach( @@ -381,7 +377,7 @@ abstract class MavericksViewModel( /** * Subscribe to state changes for seven properties. * - * @param action supports cooperative cancellation. The previous action will be cancelled if it as not completed before + * @param action supports cooperative cancellation. The previous action will be cancelled if it is not completed before * the next one is emitted. */ protected fun onEach( @@ -416,62 +412,19 @@ abstract class MavericksViewModel( deliveryMode: DeliveryMode, action: suspend (T) -> Unit ): Job { - val flow = if (lifecycleOwner == null || MavericksTestOverrides.FORCE_DISABLE_LIFECYCLE_AWARE_OBSERVER) { - this - } else if (deliveryMode is UniqueOnly) { - val lastDeliveredValue: T? = lastDeliveredValue(deliveryMode) - this - .assertOneActiveSubscription(lifecycleOwner, deliveryMode) - .dropWhile { it == lastDeliveredValue } - .flowWhenStarted(lifecycleOwner) - .distinctUntilChanged() - .onEach { lastDeliveredStates[deliveryMode.subscriptionId] = it } + return if (lifecycleOwner != null) { + collectLatest(lifecycleOwner, lastDeliveredStates, activeSubscriptions, deliveryMode, action) } else { - flowWhenStarted(lifecycleOwner) - } - - val scope = (lifecycleOwner?.lifecycleScope ?: viewModelScope) + configFactory.subscriptionCoroutineContextOverride - return scope.launch(start = CoroutineStart.UNDISPATCHED) { - // Use yield to ensure flow collect coroutine is dispatched rather than invoked immediately. - // This is necessary when Dispatchers.Main.immediate is used in scope. - // Coroutine is launched with start = CoroutineStart.UNDISPATCHED to perform dispatch only once. - yield() - flow.collectLatest(action) - } - } - - @Suppress("EXPERIMENTAL_API_USAGE") - private fun Flow.assertOneActiveSubscription(owner: LifecycleOwner, deliveryMode: UniqueOnly): Flow { - val observer = object : DefaultLifecycleObserver { - override fun onCreate(owner: LifecycleOwner) { - if (activeSubscriptions.contains(deliveryMode.subscriptionId)) error(duplicateSubscriptionMessage(deliveryMode)) - activeSubscriptions += deliveryMode.subscriptionId + (viewModelScope + configFactory.subscriptionCoroutineContextOverride).launch(start = CoroutineStart.UNDISPATCHED) { + // Use yield to ensure flow collect coroutine is dispatched rather than invoked immediately. + // This is necessary when Dispatchers.Main.immediate is used in scope. + // Coroutine is launched with start = CoroutineStart.UNDISPATCHED to perform dispatch only once. + yield() + collectLatest(action) } - - override fun onDestroy(owner: LifecycleOwner) { - activeSubscriptions.remove(deliveryMode.subscriptionId) - } - } - - owner.lifecycle.addObserver(observer) - return onCompletion { - activeSubscriptions.remove(deliveryMode.subscriptionId) - owner.lifecycle.removeObserver(observer) } } - private fun lastDeliveredValue(deliveryMode: UniqueOnly): T? { - @Suppress("UNCHECKED_CAST") - return lastDeliveredStates[deliveryMode.subscriptionId] as T? - } - - private fun duplicateSubscriptionMessage(deliveryMode: UniqueOnly) = """ - Subscribing with a duplicate subscription id: ${deliveryMode.subscriptionId}. - If you have multiple uniqueOnly subscriptions in a MvRx view that listen to the same properties - you must use a custom subscription id. If you are using a custom MvRxView, make sure you are using the proper - lifecycle owner. See BaseMvRxFragment for an example. - """.trimIndent() - private fun assertSubscribeToDifferentViewModel(viewModel: MavericksViewModel) { require(this != viewModel) { "This method is for subscribing to other view models. Please pass a different instance as the argument." diff --git a/mvrx/src/test/kotlin/com/airbnb/mvrx/FragmentSubscriberTest.kt b/mvrx/src/test/kotlin/com/airbnb/mvrx/FragmentSubscriberTest.kt index 9c5771734..1ded6f26e 100644 --- a/mvrx/src/test/kotlin/com/airbnb/mvrx/FragmentSubscriberTest.kt +++ b/mvrx/src/test/kotlin/com/airbnb/mvrx/FragmentSubscriberTest.kt @@ -7,6 +7,8 @@ import android.view.View import android.view.ViewGroup import android.widget.FrameLayout import androidx.fragment.app.Fragment +import kotlinx.coroutines.flow.drop +import kotlinx.coroutines.flow.map import org.junit.Assert.assertEquals import org.junit.Test @@ -22,6 +24,7 @@ open class ViewSubscriberFragment : Fragment(), MavericksView { var subscribeCallCount = 0 var subscribeUniqueOnlyCallCount = 0 + var subscribeCustomCallCount = 0 var selectSubscribeValue = -1 var selectSubscribeCallCount = 0 @@ -29,15 +32,22 @@ open class ViewSubscriberFragment : Fragment(), MavericksView { var selectSubscribeUniqueOnlyValue = -1 var selectSubscribeUniqueOnlyCallCount = 0 + var selectSubscribeCustomValue = -1 + var selectSubscribeCustomCallCount = 0 + var invalidateCallCount = 0 var viewCreatedSubscribeCallCount = 0 var viewCreatedUniqueOnlyCallCount = 0 + var viewCreatedCustomCallCount = 0 override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) viewModel.onEach { subscribeCallCount++ } viewModel.onEach(uniqueOnly("onCreate")) { subscribeUniqueOnlyCallCount++ } + viewModel.stateFlow.drop(1).collectLatest(UniqueOnly("onCreate_Custom")) { + subscribeCustomCallCount++ + } viewModel.onEach(ViewSubscriberState::foo) { selectSubscribeValue = it @@ -47,6 +57,13 @@ open class ViewSubscriberFragment : Fragment(), MavericksView { selectSubscribeUniqueOnlyValue = it selectSubscribeUniqueOnlyCallCount++ } + + viewModel.stateFlow.map { MavericksTuple1(ViewSubscriberState::foo.get(it)) }.drop(1).collectLatest( + UniqueOnly("onCreate_Custom").appendPropertiesToId(ViewSubscriberState::foo) + ) { (a) -> + selectSubscribeCustomValue = a + selectSubscribeCustomCallCount++ + } } override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? { @@ -58,6 +75,9 @@ open class ViewSubscriberFragment : Fragment(), MavericksView { super.onViewCreated(view, savedInstanceState) viewModel.onEach { viewCreatedSubscribeCallCount++ } viewModel.onEach(uniqueOnly("onCreateView")) { viewCreatedUniqueOnlyCallCount++ } + viewModel.stateFlow.drop(1).collectLatest(UniqueOnly("onCreateView_Custom")) { + viewCreatedCustomCallCount++ + } } fun setFoo(foo: Int) = viewModel.setFoo(foo) @@ -103,20 +123,33 @@ class FragmentSubscriberTest : BaseTest() { assertEquals(2, fragment.viewCreatedUniqueOnlyCallCount) } + @Test + fun testSubscribeCustom() { + val (_, fragment) = createFragmentInTestActivity() + assertEquals(0, fragment.subscribeCustomCallCount) + assertEquals(0, fragment.viewCreatedCustomCallCount) + + // No change in state does not trigger update. + fragment.setFoo(0) + assertEquals(0, fragment.subscribeCustomCallCount) + assertEquals(0, fragment.viewCreatedCustomCallCount) + + fragment.setFoo(1) + assertEquals(1, fragment.subscribeCustomCallCount) + assertEquals(1, fragment.viewCreatedCustomCallCount) + } + @Test fun testSelectSubscribe() { val (_, fragment) = createFragmentInTestActivity() assertEquals(0, fragment.selectSubscribeValue) - assertEquals(0, fragment.selectSubscribeUniqueOnlyValue) // No change in state does not trigger update. fragment.setFoo(0) assertEquals(0, fragment.selectSubscribeValue) - assertEquals(0, fragment.selectSubscribeUniqueOnlyValue) fragment.setFoo(1) assertEquals(1, fragment.selectSubscribeValue) - assertEquals(1, fragment.selectSubscribeUniqueOnlyValue) } @Test @@ -132,6 +165,20 @@ class FragmentSubscriberTest : BaseTest() { assertEquals(1, fragment.selectSubscribeUniqueOnlyValue) } + @Test + fun testSelectSubscribeCustom() { + val (_, fragment) = createFragmentInTestActivity() + assertEquals(-1, fragment.selectSubscribeCustomValue) + + // No change in state does not trigger update. + fragment.setFoo(0) + assertEquals(-1, fragment.selectSubscribeCustomValue) + + fragment.setFoo(1) + assertEquals(1, fragment.selectSubscribeCustomValue) + assertEquals(1, fragment.selectSubscribeCustomCallCount) + } + @Test fun invalidateCalledFromCreateToStart() { val (_, fragment) = createFragmentInTestActivity() @@ -167,6 +214,9 @@ class FragmentSubscriberTest : BaseTest() { assertEquals(0, fragment.selectSubscribeUniqueOnlyValue) assertEquals(1, fragment.selectSubscribeUniqueOnlyCallCount) + assertEquals(-1, fragment.selectSubscribeCustomValue) + assertEquals(0, fragment.selectSubscribeCustomCallCount) + controller.pause() controller.stop() @@ -176,6 +226,9 @@ class FragmentSubscriberTest : BaseTest() { assertEquals(0, fragment.selectSubscribeUniqueOnlyValue) assertEquals(1, fragment.selectSubscribeUniqueOnlyCallCount) + assertEquals(-1, fragment.selectSubscribeCustomValue) + assertEquals(0, fragment.selectSubscribeCustomCallCount) + controller.start() controller.resume() @@ -185,6 +238,9 @@ class FragmentSubscriberTest : BaseTest() { assertEquals(0, fragment.selectSubscribeUniqueOnlyValue) assertEquals(1, fragment.selectSubscribeUniqueOnlyCallCount) + + assertEquals(-1, fragment.selectSubscribeCustomValue) + assertEquals(0, fragment.selectSubscribeCustomCallCount) } @Test @@ -196,6 +252,9 @@ class FragmentSubscriberTest : BaseTest() { assertEquals(1, fragment.subscribeUniqueOnlyCallCount) assertEquals(1, fragment.viewCreatedUniqueOnlyCallCount) + assertEquals(0, fragment.subscribeCustomCallCount) + assertEquals(0, fragment.viewCreatedCustomCallCount) + controller.pause() controller.stop() @@ -206,6 +265,9 @@ class FragmentSubscriberTest : BaseTest() { assertEquals(1, fragment.subscribeUniqueOnlyCallCount) assertEquals(1, fragment.viewCreatedUniqueOnlyCallCount) + assertEquals(0, fragment.subscribeCustomCallCount) + assertEquals(0, fragment.viewCreatedCustomCallCount) + controller.start() controller.resume() @@ -214,6 +276,9 @@ class FragmentSubscriberTest : BaseTest() { assertEquals(2, fragment.subscribeUniqueOnlyCallCount) assertEquals(2, fragment.viewCreatedUniqueOnlyCallCount) + + assertEquals(1, fragment.subscribeCustomCallCount) + assertEquals(1, fragment.viewCreatedCustomCallCount) } @Test @@ -221,24 +286,29 @@ class FragmentSubscriberTest : BaseTest() { val (controller, fragment) = createFragmentInTestActivity() assertEquals(1, fragment.subscribeCallCount) assertEquals(1, fragment.subscribeUniqueOnlyCallCount) + assertEquals(0, fragment.subscribeCustomCallCount) assertEquals(1, fragment.viewCreatedSubscribeCallCount) assertEquals(1, fragment.viewCreatedUniqueOnlyCallCount) + assertEquals(0, fragment.viewCreatedCustomCallCount) controller.pause() controller.stop() assertEquals(1, fragment.subscribeCallCount) assertEquals(1, fragment.subscribeUniqueOnlyCallCount) + assertEquals(0, fragment.subscribeCustomCallCount) assertEquals(1, fragment.viewCreatedSubscribeCallCount) assertEquals(1, fragment.viewCreatedUniqueOnlyCallCount) + assertEquals(0, fragment.viewCreatedCustomCallCount) controller.start() controller.resume() assertEquals(2, fragment.subscribeCallCount) assertEquals(1, fragment.subscribeUniqueOnlyCallCount) + assertEquals(0, fragment.subscribeCustomCallCount) } @Test @@ -246,26 +316,32 @@ class FragmentSubscriberTest : BaseTest() { val (controller, fragment) = createFragmentInTestActivity() assertEquals(1, fragment.subscribeCallCount) assertEquals(1, fragment.subscribeUniqueOnlyCallCount) + assertEquals(0, fragment.subscribeCustomCallCount) assertEquals(1, fragment.viewCreatedSubscribeCallCount) assertEquals(1, fragment.viewCreatedUniqueOnlyCallCount) + assertEquals(0, fragment.viewCreatedCustomCallCount) val fragmentManager = controller.get().supportFragmentManager fragmentManager.beginTransaction().replace(CONTAINER_ID, Fragment(), "TAG").addToBackStack(null).commit() assertEquals(1, fragment.subscribeCallCount) assertEquals(1, fragment.subscribeUniqueOnlyCallCount) + assertEquals(0, fragment.subscribeCustomCallCount) assertEquals(1, fragment.viewCreatedSubscribeCallCount) assertEquals(1, fragment.viewCreatedUniqueOnlyCallCount) + assertEquals(0, fragment.viewCreatedCustomCallCount) fragmentManager.popBackStackImmediate() assertEquals(2, fragment.subscribeCallCount) assertEquals(1, fragment.subscribeUniqueOnlyCallCount) + assertEquals(0, fragment.subscribeCustomCallCount) assertEquals(2, fragment.viewCreatedSubscribeCallCount) assertEquals(1, fragment.viewCreatedUniqueOnlyCallCount) + assertEquals(0, fragment.viewCreatedCustomCallCount) } @Test @@ -273,9 +349,11 @@ class FragmentSubscriberTest : BaseTest() { val (controller, fragment) = createFragmentInTestActivity() assertEquals(1, fragment.subscribeCallCount) assertEquals(1, fragment.subscribeUniqueOnlyCallCount) + assertEquals(0, fragment.subscribeCustomCallCount) assertEquals(1, fragment.viewCreatedSubscribeCallCount) assertEquals(1, fragment.viewCreatedUniqueOnlyCallCount) + assertEquals(0, fragment.viewCreatedCustomCallCount) val fragmentManager = controller.get().supportFragmentManager fragmentManager.beginTransaction().replace(CONTAINER_ID, Fragment(), "TAG").addToBackStack(null).commit() @@ -284,17 +362,21 @@ class FragmentSubscriberTest : BaseTest() { fragment.setFoo(1) assertEquals(1, fragment.subscribeCallCount) assertEquals(1, fragment.subscribeUniqueOnlyCallCount) + assertEquals(0, fragment.subscribeCustomCallCount) assertEquals(1, fragment.viewCreatedSubscribeCallCount) assertEquals(1, fragment.viewCreatedUniqueOnlyCallCount) + assertEquals(0, fragment.viewCreatedCustomCallCount) fragmentManager.popBackStackImmediate() assertEquals(2, fragment.subscribeCallCount) assertEquals(2, fragment.subscribeUniqueOnlyCallCount) + assertEquals(1, fragment.subscribeCustomCallCount) assertEquals(2, fragment.viewCreatedSubscribeCallCount) assertEquals(2, fragment.viewCreatedUniqueOnlyCallCount) + assertEquals(0, fragment.viewCreatedCustomCallCount) } /** @@ -308,9 +390,11 @@ class FragmentSubscriberTest : BaseTest() { super.onStop() val subscribeCallCountBeforeFooChange = subscribeCallCount val subscribeUniqueOnlyCallCountBeforeFooChange = subscribeUniqueOnlyCallCount + val subscribeCustomCallCountBeforeFooChange = subscribeCustomCallCount this.setFoo(1) assertEquals(subscribeCallCountBeforeFooChange, subscribeCallCount) assertEquals(subscribeUniqueOnlyCallCountBeforeFooChange, subscribeUniqueOnlyCallCount) + assertEquals(subscribeCustomCallCountBeforeFooChange, subscribeCustomCallCount) } } @@ -323,6 +407,9 @@ class FragmentSubscriberTest : BaseTest() { assertEquals(0, fragment.selectSubscribeUniqueOnlyValue) assertEquals(1, fragment.selectSubscribeUniqueOnlyCallCount) + assertEquals(-1, fragment.selectSubscribeCustomValue) + assertEquals(0, fragment.selectSubscribeCustomCallCount) + // This will set foo to 1. See FragmentWithStateChangeDuringOrientationChange. controller.configurationChange(Configuration().apply { setToDefaults() @@ -338,6 +425,9 @@ class FragmentSubscriberTest : BaseTest() { // Even if unique is true, if the state changes we expect a value. assertEquals(1, recreatedFragment.selectSubscribeUniqueOnlyValue) assertEquals(1, recreatedFragment.selectSubscribeUniqueOnlyCallCount) + + assertEquals(-1, recreatedFragment.selectSubscribeCustomValue) + assertEquals(0, recreatedFragment.selectSubscribeCustomCallCount) } @Test @@ -349,6 +439,9 @@ class FragmentSubscriberTest : BaseTest() { assertEquals(0, fragment.selectSubscribeUniqueOnlyValue) assertEquals(1, fragment.selectSubscribeUniqueOnlyCallCount) + assertEquals(-1, fragment.selectSubscribeCustomValue) + assertEquals(0, fragment.selectSubscribeCustomCallCount) + controller.configurationChange(Configuration().apply { setToDefaults() this.orientation = Configuration.ORIENTATION_LANDSCAPE @@ -362,6 +455,9 @@ class FragmentSubscriberTest : BaseTest() { assertEquals(-1, recreatedFragment.selectSubscribeUniqueOnlyValue) assertEquals(0, recreatedFragment.selectSubscribeUniqueOnlyCallCount) + + assertEquals(-1, recreatedFragment.selectSubscribeCustomValue) + assertEquals(0, recreatedFragment.selectSubscribeCustomCallCount) } @Test @@ -369,9 +465,11 @@ class FragmentSubscriberTest : BaseTest() { val (controller, fragment) = createFragmentInTestActivity() assertEquals(1, fragment.subscribeCallCount) assertEquals(1, fragment.subscribeUniqueOnlyCallCount) + assertEquals(0, fragment.subscribeCustomCallCount) assertEquals(1, fragment.viewCreatedSubscribeCallCount) assertEquals(1, fragment.viewCreatedUniqueOnlyCallCount) + assertEquals(0, fragment.viewCreatedCustomCallCount) // This will set foo to 1. See FragmentWithStateChangeDuringOrientationChange. controller.configurationChange(Configuration().apply { @@ -384,9 +482,11 @@ class FragmentSubscriberTest : BaseTest() { // As the value changed, the unique only subscription will be called. assertEquals(1, recreatedFragment.subscribeCallCount) assertEquals(1, recreatedFragment.subscribeUniqueOnlyCallCount) + assertEquals(0, recreatedFragment.subscribeCustomCallCount) assertEquals(1, recreatedFragment.viewCreatedSubscribeCallCount) assertEquals(1, recreatedFragment.viewCreatedUniqueOnlyCallCount) + assertEquals(0, recreatedFragment.viewCreatedCustomCallCount) } @Test @@ -394,9 +494,11 @@ class FragmentSubscriberTest : BaseTest() { val (controller, fragment) = createFragmentInTestActivity() assertEquals(1, fragment.subscribeCallCount) assertEquals(1, fragment.subscribeUniqueOnlyCallCount) + assertEquals(0, fragment.subscribeCustomCallCount) assertEquals(1, fragment.viewCreatedSubscribeCallCount) assertEquals(1, fragment.viewCreatedSubscribeCallCount) + assertEquals(0, fragment.viewCreatedCustomCallCount) controller.configurationChange(Configuration().apply { setToDefaults() @@ -408,9 +510,11 @@ class FragmentSubscriberTest : BaseTest() { // As the value has not changed, the unique only subscription will not be called. assertEquals(1, recreatedFragment.subscribeCallCount) assertEquals(0, recreatedFragment.subscribeUniqueOnlyCallCount) + assertEquals(0, recreatedFragment.subscribeCustomCallCount) assertEquals(1, recreatedFragment.viewCreatedSubscribeCallCount) assertEquals(0, recreatedFragment.viewCreatedUniqueOnlyCallCount) + assertEquals(0, recreatedFragment.viewCreatedCustomCallCount) } @Test @@ -608,4 +712,24 @@ class FragmentSubscriberTest : BaseTest() { // as what it received previously. assertEquals(2, fragment.selectSubscribeUniqueOnlyCallCount) } + + @Test + fun testCustom() { + val (controller, fragment) = createFragmentInTestActivity() + fragment.setFoo(1) + assertEquals(1, fragment.selectSubscribeCustomCallCount + ) + + controller.pause() + controller.stop() + fragment.setFoo(2) + fragment.setFoo(1) + controller.start() + controller.resume() + + // In MvRx 1.0, this would have been 2. If the value for a uniqueOnly() subscription changed + // and changed back while stopped, it would redeliver the value even though it was the same + // as what it received previously. + assertEquals(1, fragment.selectSubscribeCustomCallCount) + } } diff --git a/view-binding-utils/src/main/java/com/airbnb/mvrx/viewbinding/FragmentViewBindingDelegate.kt b/view-binding-utils/src/main/java/com/airbnb/mvrx/viewbinding/FragmentViewBindingDelegate.kt index 6a3699668..56f90462e 100644 --- a/view-binding-utils/src/main/java/com/airbnb/mvrx/viewbinding/FragmentViewBindingDelegate.kt +++ b/view-binding-utils/src/main/java/com/airbnb/mvrx/viewbinding/FragmentViewBindingDelegate.kt @@ -7,7 +7,9 @@ import androidx.fragment.app.Fragment import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleObserver import androidx.lifecycle.OnLifecycleEvent +import androidx.lifecycle.lifecycleScope import androidx.viewbinding.ViewBinding +import kotlinx.coroutines.launch import kotlin.properties.ReadOnlyProperty import kotlin.reflect.KProperty @@ -21,16 +23,18 @@ class FragmentViewBindingDelegate( private val bindMethod = bindingClass.getMethod("bind", View::class.java) init { - fragment.viewLifecycleOwnerLiveData.observe(fragment) { viewLifecycleOwner -> - viewLifecycleOwner.lifecycle.addObserver(object : LifecycleObserver { - @OnLifecycleEvent(Lifecycle.Event.ON_DESTROY) - fun onDestroy() { - // Lifecycle listeners are called before onDestroyView in a Fragment. - // However, we want views to be able to use bindings in onDestroyView - // to do cleanup so we clear the reference one frame later. - clearBindingHandler.post { binding = null } - } - }) + fragment.lifecycleScope.launch { + fragment.viewLifecycleOwnerLiveData.observe(fragment) { viewLifecycleOwner -> + viewLifecycleOwner.lifecycle.addObserver(object : LifecycleObserver { + @OnLifecycleEvent(Lifecycle.Event.ON_DESTROY) + fun onDestroy() { + // Lifecycle listeners are called before onDestroyView in a Fragment. + // However, we want views to be able to use bindings in onDestroyView + // to do cleanup so we clear the reference one frame later. + clearBindingHandler.post { binding = null } + } + }) + } } }