From a62f80d8bc67ce30de3ba1e05663c8062f42192e Mon Sep 17 00:00:00 2001 From: Ray Ryan Date: Mon, 5 Aug 2024 15:20:24 -0700 Subject: [PATCH] Return the `Job` from `WorkflowLayout.take` so it can be canceled. --- workflow-ui/core-android/api/core-android.api | 4 +- .../squareup/workflow1/ui/WorkflowLayout.kt | 10 +++- .../workflow1/ui/WorkflowLayoutTest.kt | 51 ++++++++++++++++++- 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/workflow-ui/core-android/api/core-android.api b/workflow-ui/core-android/api/core-android.api index 6e41bf3aa..dcd11bfbd 100644 --- a/workflow-ui/core-android/api/core-android.api +++ b/workflow-ui/core-android/api/core-android.api @@ -128,8 +128,8 @@ public final class com/squareup/workflow1/ui/WorkflowLayout : android/widget/Fra public synthetic fun (Landroid/content/Context;Landroid/util/AttributeSet;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public final fun show (Lcom/squareup/workflow1/ui/Screen;Lcom/squareup/workflow1/ui/ViewEnvironment;)V public static synthetic fun show$default (Lcom/squareup/workflow1/ui/WorkflowLayout;Lcom/squareup/workflow1/ui/Screen;Lcom/squareup/workflow1/ui/ViewEnvironment;ILjava/lang/Object;)V - public final fun take (Landroidx/lifecycle/Lifecycle;Lkotlinx/coroutines/flow/Flow;Landroidx/lifecycle/Lifecycle$State;Lkotlin/coroutines/CoroutineContext;)V - public static synthetic fun take$default (Lcom/squareup/workflow1/ui/WorkflowLayout;Landroidx/lifecycle/Lifecycle;Lkotlinx/coroutines/flow/Flow;Landroidx/lifecycle/Lifecycle$State;Lkotlin/coroutines/CoroutineContext;ILjava/lang/Object;)V + public final fun take (Landroidx/lifecycle/Lifecycle;Lkotlinx/coroutines/flow/Flow;Landroidx/lifecycle/Lifecycle$State;Lkotlin/coroutines/CoroutineContext;)Lkotlinx/coroutines/Job; + public static synthetic fun take$default (Lcom/squareup/workflow1/ui/WorkflowLayout;Landroidx/lifecycle/Lifecycle;Lkotlinx/coroutines/flow/Flow;Landroidx/lifecycle/Lifecycle$State;Lkotlin/coroutines/CoroutineContext;ILjava/lang/Object;)Lkotlinx/coroutines/Job; } public final class com/squareup/workflow1/ui/WorkflowViewStub : android/view/View { diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowLayout.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowLayout.kt index 250ba8298..4ba6bc3ef 100644 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowLayout.kt +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowLayout.kt @@ -17,6 +17,7 @@ import androidx.lifecycle.repeatOnLifecycle import com.squareup.workflow1.ui.androidx.OnBackPressedDispatcherOwnerKey import com.squareup.workflow1.ui.androidx.WorkflowAndroidXSupport.onBackPressedDispatcherOwnerOrNull import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Job import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.launch import kotlin.coroutines.CoroutineContext @@ -91,6 +92,11 @@ public class WorkflowLayout( * @param [collectionContext] additional [CoroutineContext] we want for the coroutine that is * launched to collect the renderings. This should not override the [CoroutineDispatcher][kotlinx.coroutines.CoroutineDispatcher] * but may include some other instrumentation elements. + * + * @return the [Job] started to collect [renderings], to give callers the option to + * [cancel][Job.cancel] collection -- e.g., before calling [take] again with a new + * [renderings] flow. In most cases the caller can ignore this, interacting with + * the [Job] is very unusual. */ @OptIn(ExperimentalStdlibApi::class) public fun take( @@ -98,12 +104,12 @@ public class WorkflowLayout( renderings: Flow, repeatOnLifecycle: State = STARTED, collectionContext: CoroutineContext = EmptyCoroutineContext - ) { + ): Job { // We remove the dispatcher as we want to use what is provided by the lifecycle.coroutineScope. val contextWithoutDispatcher = collectionContext.minusKey(CoroutineDispatcher.Key) val lifecycleDispatcher = lifecycle.coroutineScope.coroutineContext[CoroutineDispatcher.Key] // Just like https://medium.com/androiddevelopers/a-safer-way-to-collect-flows-from-android-uis-23080b1f8bda - lifecycle.coroutineScope.launch(contextWithoutDispatcher) { + return lifecycle.coroutineScope.launch(contextWithoutDispatcher) { lifecycle.repeatOnLifecycle(repeatOnLifecycle) { require(coroutineContext[CoroutineDispatcher.Key] == lifecycleDispatcher) { "Collection dispatch should happen on the lifecycle's dispatcher." diff --git a/workflow-ui/core-android/src/test/java/com/squareup/workflow1/ui/WorkflowLayoutTest.kt b/workflow-ui/core-android/src/test/java/com/squareup/workflow1/ui/WorkflowLayoutTest.kt index 1b6877af0..680207dfe 100644 --- a/workflow-ui/core-android/src/test/java/com/squareup/workflow1/ui/WorkflowLayoutTest.kt +++ b/workflow-ui/core-android/src/test/java/com/squareup/workflow1/ui/WorkflowLayoutTest.kt @@ -5,7 +5,10 @@ import android.os.Bundle import android.os.Parcelable import android.util.SparseArray import android.view.View +import androidx.activity.OnBackPressedDispatcher import androidx.activity.OnBackPressedDispatcherOwner +import androidx.activity.setViewTreeOnBackPressedDispatcherOwner +import androidx.core.view.get import androidx.lifecycle.Lifecycle import androidx.lifecycle.testing.TestLifecycleOwner import androidx.test.core.app.ApplicationProvider @@ -13,8 +16,10 @@ import com.google.common.truth.Truth.assertThat import com.squareup.workflow1.ui.androidx.OnBackPressedDispatcherOwnerKey import com.squareup.workflow1.ui.navigation.WrappedScreen import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.runTest import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @@ -28,7 +33,13 @@ import kotlin.coroutines.CoroutineContext internal class WorkflowLayoutTest { private val context: Context = ApplicationProvider.getApplicationContext() - private val workflowLayout = WorkflowLayout(context).apply { id = 42 } + private val workflowLayout = WorkflowLayout(context).apply { + id = 42 + setViewTreeOnBackPressedDispatcherOwner(object : OnBackPressedDispatcherOwner { + override fun getOnBackPressedDispatcher(): OnBackPressedDispatcher { error("yeah no") } + override val lifecycle: Lifecycle get() = error("nope") + }) + } @Test fun ignoresAlienViewState() { val weirdView = BundleSavingView(context) @@ -91,6 +102,44 @@ internal class WorkflowLayoutTest { // No crash then we safely removed the dispatcher. } + @Test fun takes() { + val lifecycleDispatcher = UnconfinedTestDispatcher() + val testLifecycle = TestLifecycleOwner( + initialState = Lifecycle.State.RESUMED, + coroutineDispatcher = lifecycleDispatcher + ) + val flow = MutableSharedFlow() + + runTest(lifecycleDispatcher) { + workflowLayout.take( + lifecycle = testLifecycle.lifecycle, + renderings = flow, + ) + assertThat(workflowLayout[0]).isInstanceOf(WorkflowViewStub::class.java) + flow.emit(WrappedScreen()) + assertThat(workflowLayout[0]).isNotInstanceOf(WorkflowViewStub::class.java) + } + } + + @Test fun canStopTaking() { + val lifecycleDispatcher = UnconfinedTestDispatcher() + val testLifecycle = TestLifecycleOwner( + initialState = Lifecycle.State.RESUMED, + coroutineDispatcher = lifecycleDispatcher + ) + val flow = MutableSharedFlow() + + runTest(lifecycleDispatcher) { + val job = workflowLayout.take( + lifecycle = testLifecycle.lifecycle, + renderings = flow, + ) + job.cancel() + flow.emit(WrappedScreen()) + assertThat(workflowLayout[0]).isInstanceOf(WorkflowViewStub::class.java) + } + } + private class BundleSavingView(context: Context) : View(context) { var saved = false