Calling scoped suspending provider should create singleton instance

[sdipendra]

Hi,

kotlin-inject currently doesn't allow scoping suspending provider method.

    @Provides
    @NetworkScope
    protected suspend fun api(service: Service): Api = service.connect()

Expectation: scoped suspending provider should create a single instance of the returned object. Observation: Adding scope to suspending provider method causes compilation to fail with error: "Suspension functions can be called only within coroutine body" & removing the scope passes compilation but creates new object on every invocation.

Below is the failing test for the same:

import kotlinx.coroutines.runBlocking
import me.tatarka.inject.annotations.Component
import me.tatarka.inject.annotations.Inject
import me.tatarka.inject.annotations.Provides
import me.tatarka.inject.annotations.Scope
import org.junit.jupiter.api.Test
import java.util.concurrent.atomic.AtomicInteger
import kotlin.test.assertEquals

@Scope
@Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION, AnnotationTarget.PROPERTY_GETTER)
annotation class NetworkScope

@Component
@NetworkScope
internal abstract class NetworkComponent(@get:Provides protected val apiInstanceCounter: AtomicInteger) {
    @NetworkScope
    abstract suspend fun api(): Api

    @Provides
    // Note: Uncommenting below gives error: Suspension functions can be called only within coroutine body
    // @NetworkScope
    protected suspend fun api(service: Service): Api = service.connect()

    protected val DummyNetworkService.bind: Service
        @Provides get() = this
}

interface Api

interface Service {
    suspend fun connect(): Api
}

@Inject
class DummyNetworkService(private val apiInstanceCounter: AtomicInteger) : Service {
    override suspend fun connect(): Api {
        apiInstanceCounter.incrementAndGet()

        return object : Api {}
    }
}

internal class KotlinInjectTest {
    @Test
    fun `calling scoped suspending provider should create singleton instance`() {
        val apiInstanceCounter = AtomicInteger(0)

        val networkComponent = NetworkComponent::class.create(apiInstanceCounter)

        runBlocking {
            val apiInstance0 = networkComponent.api()
            val apiInstance1 = networkComponent.api()

            assertEquals(apiInstance0, apiInstance1)
        }
    }
}

[evant]

As a work-around you can return a Deferred object

@Component
@NetworkScope
internal abstract class NetworkComponent(
    @get:Provides protected val apiInstanceCounter: AtomicInteger,
    @get:Provides val scope: CoroutineScope,
) {
    @NetworkScope
    abstract fun api(): Deferred<Api>

    @Provides
    @NetworkScope
    protected fun api(service: Service, scope: CoroutineScope): Deferred<Api> =
        scope.async(start = CoroutineStart.LAZY) { service.connect() }

    protected val DummyNetworkService.bind: Service
        @Provides get() = this
}

interface Api

interface Service {
    suspend fun connect(): Api
}

@Inject
class DummyNetworkService(private val apiInstanceCounter: AtomicInteger) : Service {
    override suspend fun connect(): Api {
        apiInstanceCounter.incrementAndGet()

        return object : Api {}
    }
}

internal class KotlinInjectTest {
    @Test
    fun `calling scoped suspending provider should create singleton instance`() {
        val apiInstanceCounter = AtomicInteger(0)

        runBlocking {
            val networkComponent = NetworkComponent::class.create(apiInstanceCounter, this)
            val apiInstance0 = networkComponent.api().await()
            val apiInstance1 = networkComponent.api().await()

            assertEquals(apiInstance0, apiInstance1)
        }
    }
}

[evant]

Marking as a bug because at the very least it should have a good error message. Will look into if this can be supported

[sdipendra]

Thanks for looking into it. It'll be great if this can be supported. My project currently uses Dagger(kotlin jvm) with Deferred as you have suggested. Using Deferred causes all the dependent objects to also return Deferred, which requires boiler plate code for launching the async, awaiting deferred value in deferred provider methods & passing the coroutineScope to the component. Even with Deferred "kotlin-inject" is better option than Dagger as it will only require Deferred at the point of "suspending provider" call & dependent object relationship can be expressed without Deferred by adding a suspend mapping from Deferred value to value using await().

    @Provides
    protected suspend fun api(deferredApi: Deferred<Api>): Api = deferredApi.await()

And then api can be returned/injected directly instead of Deferred:

    @NetworkScope
    abstract suspend fun api(): Api

Since it's technically possible with hand written code, it probably should be possible with code generation.

On seeing the location of the error, the problem seems to be in the init block of the get method of LazyMap:

    actual fun <T> get(key: String, init: () -> T): T

Maybe a suspending variant could help:

    actual suspend fun <T> get(key: String, init: suspend () -> T): T

[evant]

yeah, though it looks like kotlinx.coroutines is required to implement the suspend version, which isn't currently a dependency. Will have to figure out how to make that optional or if we can live with the extra dependency.

[evant]

note: looked into this more and you really need a coroutine-aware lock for this to work correctly, this means that either coroutines would have to be a hard-dependency or we'd need actually do different code gen based on a flag or something. both options feel kinda gross 😞

[sdipendra]

This is not blocking for my usecase, and is a good to have feature. The delta utility vs amount of effort required to implement/maintain it doesn't seems worth it. Providing an error message with the workaround steps should be good enough.