From dfe24965869521dec37d6d1871f86599a255d7fb Mon Sep 17 00:00:00 2001 From: Patrick Steiger Date: Thu, 11 Sep 2025 12:05:04 -0400 Subject: [PATCH] Change ScopeProvider.coroutineScope throwing behavior when accessing it before scope is active, or after scope is inactive. --- libraries/rib-coroutines/build.gradle.kts | 5 +- .../com/uber/rib/core/RibCoroutineScopes.kt | 75 ++++++++++---- .../com/uber/rib/core/RibCoroutinesConfig.kt | 8 ++ .../uber/rib/core/RibCoroutineScopesTest.kt | 98 +++++++++++++++++++ 4 files changed, 165 insertions(+), 21 deletions(-) create mode 100644 libraries/rib-coroutines/src/test/kotlin/com/uber/rib/core/RibCoroutineScopesTest.kt diff --git a/libraries/rib-coroutines/build.gradle.kts b/libraries/rib-coroutines/build.gradle.kts index 0c176a0e..f2eb988f 100644 --- a/libraries/rib-coroutines/build.gradle.kts +++ b/libraries/rib-coroutines/build.gradle.kts @@ -13,22 +13,23 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - plugins { id("ribs.kotlin.library") alias(libs.plugins.maven.publish) } dependencies { - api(libs.autodispose.coroutines) api(libs.kotlinx.coroutines.android) api(libs.kotlinx.coroutines.rx2) compileOnly(libs.android.api) + implementation(libs.autodispose.lifecycle) + testImplementation(project(":libraries:rib-base")) testImplementation(project(":libraries:rib-test")) + testImplementation(project(":libraries:rib-coroutines-test")) testImplementation(testLibs.junit) testImplementation(testLibs.mockito) testImplementation(testLibs.mockito.kotlin) diff --git a/libraries/rib-coroutines/src/main/kotlin/com/uber/rib/core/RibCoroutineScopes.kt b/libraries/rib-coroutines/src/main/kotlin/com/uber/rib/core/RibCoroutineScopes.kt index 6d67979d..5506d50c 100755 --- a/libraries/rib-coroutines/src/main/kotlin/com/uber/rib/core/RibCoroutineScopes.kt +++ b/libraries/rib-coroutines/src/main/kotlin/com/uber/rib/core/RibCoroutineScopes.kt @@ -17,8 +17,10 @@ package com.uber.rib.core import android.app.Application import com.uber.autodispose.ScopeProvider -import com.uber.autodispose.coroutinesinterop.asCoroutineScope +import com.uber.autodispose.lifecycle.LifecycleEndedException import com.uber.rib.core.internal.CoroutinesFriendModuleApi +import io.reactivex.CompletableObserver +import io.reactivex.disposables.Disposable import java.util.WeakHashMap import kotlin.coroutines.CoroutineContext import kotlin.coroutines.EmptyCoroutineContext @@ -26,6 +28,7 @@ import kotlin.reflect.KProperty import kotlinx.coroutines.CoroutineName import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel import kotlinx.coroutines.job /** @@ -33,19 +36,29 @@ import kotlinx.coroutines.job * completed * * This scope is bound to - * [RibDispatchers.Main.immediate][kotlinx.coroutines.MainCoroutineDispatcher.immediate] + * [RibDispatchers.Main.immediate][kotlinx.coroutines.MainCoroutineDispatcher.immediate]. + * + * Calling this property outside of the lifecycle of the [ScopeProvider] will throw + * [OutsideScopeException][com.uber.autodispose.OutsideScopeException]. By setting + * [RibCoroutinesConfig.shouldCoroutineScopeFailSilentlyOnLifecycleEnded] to `true`, accessing this + * property after the [ScopeProvider] has completed will instead return a [CoroutineScope] that is + * immediately cancelled. */ @OptIn(CoroutinesFriendModuleApi::class) -public val ScopeProvider.coroutineScope: CoroutineScope by - LazyCoroutineScope { - val context: CoroutineContext = - SupervisorJob() + - RibDispatchers.Main.immediate + - CoroutineName("${this::class.simpleName}:coroutineScope") + - (RibCoroutinesConfig.exceptionHandler ?: EmptyCoroutineContext) - - asCoroutineScope(context) +public val ScopeProvider.coroutineScope: CoroutineScope by LazyCoroutineScope { + val context = createCoroutineContext() + try { + ScopeProviderCoroutineScope(this, context) + } catch (e: LifecycleEndedException) { + if (RibCoroutinesConfig.shouldCoroutineScopeFailSilentlyOnLifecycleEnded) { + CoroutineScope(context).also { + it.cancel("ScopeProvider is outside of scope. context = $context", e) + } + } else { + throw e + } } +} /** * [CoroutineScope] tied to this [Application]. This scope will not be cancelled, it lives for the @@ -55,17 +68,41 @@ public val ScopeProvider.coroutineScope: CoroutineScope by * [RibDispatchers.Main.immediate][kotlinx.coroutines.MainCoroutineDispatcher.immediate] */ @OptIn(CoroutinesFriendModuleApi::class) -public val Application.coroutineScope: CoroutineScope by - LazyCoroutineScope { - val context: CoroutineContext = - SupervisorJob() + - RibDispatchers.Main.immediate + - CoroutineName("${this::class.simpleName}:coroutineScope") + - (RibCoroutinesConfig.exceptionHandler ?: EmptyCoroutineContext) +public val Application.coroutineScope: CoroutineScope by LazyCoroutineScope { + CoroutineScope(createCoroutineContext()) +} - CoroutineScope(context) +private fun Any.createCoroutineContext() = + SupervisorJob() + + RibDispatchers.Main.immediate + + CoroutineName("${this::class.simpleName}:coroutineScope") + + (RibCoroutinesConfig.exceptionHandler ?: EmptyCoroutineContext) + +private class ScopeProviderCoroutineScope( + scopeProvider: ScopeProvider, + coroutineContext: CoroutineContext, +) : + ScopeProvider by scopeProvider, + CoroutineScope by CoroutineScope(coroutineContext), + CompletableObserver { + + init { + requestScope().subscribe(this) } + override fun onSubscribe(d: Disposable) { + coroutineContext.job.invokeOnCompletion { d.dispose() } + } + + override fun onComplete() { + cancel() + } + + override fun onError(e: Throwable) { + cancel("ScopeProvider completed with error", e) + } +} + @CoroutinesFriendModuleApi public class LazyCoroutineScope(private val initializer: This.() -> CoroutineScope) { public companion object { diff --git a/libraries/rib-coroutines/src/main/kotlin/com/uber/rib/core/RibCoroutinesConfig.kt b/libraries/rib-coroutines/src/main/kotlin/com/uber/rib/core/RibCoroutinesConfig.kt index 887bcf4c..0e438eb2 100644 --- a/libraries/rib-coroutines/src/main/kotlin/com/uber/rib/core/RibCoroutinesConfig.kt +++ b/libraries/rib-coroutines/src/main/kotlin/com/uber/rib/core/RibCoroutinesConfig.kt @@ -38,6 +38,14 @@ public object RibCoroutinesConfig { */ @JvmStatic public var exceptionHandler: CoroutineExceptionHandler? = null + /** + * When set, the `coroutineScope` extension property will fail silently (i.e. not throw) when + * accessed after the scope has completed. + * + * Defaults to `false`. + */ + @JvmStatic public var shouldCoroutineScopeFailSilentlyOnLifecycleEnded: Boolean = false + /** * Specify the [CoroutineDispatcher] to be used while binding a [com.uber.rib.Worker] via * [WorkerBinder] diff --git a/libraries/rib-coroutines/src/test/kotlin/com/uber/rib/core/RibCoroutineScopesTest.kt b/libraries/rib-coroutines/src/test/kotlin/com/uber/rib/core/RibCoroutineScopesTest.kt new file mode 100644 index 00000000..899fa432 --- /dev/null +++ b/libraries/rib-coroutines/src/test/kotlin/com/uber/rib/core/RibCoroutineScopesTest.kt @@ -0,0 +1,98 @@ +/* + * Copyright (C) 2024. Uber Technologies + * + * 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.uber.rib.core + +import com.google.common.truth.Truth.assertThat +import com.uber.autodispose.lifecycle.LifecycleEndedException +import com.uber.autodispose.lifecycle.LifecycleNotStartedException +import kotlin.contracts.ExperimentalContracts +import kotlin.contracts.InvocationKind +import kotlin.contracts.contract +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.Job +import kotlinx.coroutines.awaitCancellation +import kotlinx.coroutines.isActive +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertThrows +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.Parameterized +import org.mockito.kotlin.mock + +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(Parameterized::class) +class RibCoroutineScopesTest(private val failSilentlyOnLifecycleEnded: Boolean) { + @get:Rule val ribCoroutinesRule = RibCoroutinesRule() + private val interactor = TestInteractor() + + @Before + fun setUp() { + RibCoroutinesConfig.shouldCoroutineScopeFailSilentlyOnLifecycleEnded = + failSilentlyOnLifecycleEnded + } + + @Test + fun coroutineScope_whenCalledBeforeActive_throws() { + assertThrows(LifecycleNotStartedException::class.java) { interactor.coroutineScope } + } + + @Test + fun coroutineScope_whenCalledAfterInactive_throws() { + interactor.attachAndDetach {} + if (failSilentlyOnLifecycleEnded) { + assertThat(interactor.coroutineScope.isActive).isFalse() + } else { + assertThrows(LifecycleEndedException::class.java) { interactor.coroutineScope } + } + } + + @Test + fun coroutineScope_whenCalledWhileActive_cancelsWhenInactive() = runTest { + var launched = false + val job: Job + interactor.attachAndDetach { + job = + coroutineScope.launch { + launched = true + awaitCancellation() + } + runCurrent() + assertThat(launched).isTrue() + assertThat(job.isActive).isTrue() + } + assertThat(job.isCancelled).isTrue() + } + + companion object { + @JvmStatic + @Parameterized.Parameters(name = "failSilentlyOnLifecycleEnded = {0}") + fun data() = listOf(arrayOf(true), arrayOf(false)) + } +} + +private class TestInteractor : Interactor>() + +@OptIn(ExperimentalContracts::class) +private inline fun TestInteractor.attachAndDetach(block: TestInteractor.() -> Unit) { + contract { callsInPlace(block, InvocationKind.EXACTLY_ONCE) } + InteractorHelper.attach(this, Unit, mock(), null) + block() + InteractorHelper.detach(this) +}