koin icon indicating copy to clipboard operation
koin copied to clipboard

feat: Instances interface

Open monosoul opened this issue 1 year ago • 7 comments

The goal of this PR is to provide an abstraction that can be used to interact with both org.koin.core.scope.Scope and org.koin.core.Koin.

It is especially convenient when you'd like to have extension functions usable with both entities.

Tbh, not having this abstraction has been annoying me for a while, so figured I'll give it a shot :smile:

monosoul avatar Feb 03 '24 17:02 monosoul

Nice 🎉

hoc081098 avatar Feb 05 '24 11:02 hoc081098

Still need to evaluate. Can you provide tests & more details samples? 🙏

arnaudgiuliani avatar Feb 16 '24 08:02 arnaudgiuliani

more details samples

@arnaudgiuliani so in tests we usually don't use a global Koin context (in fact, we usually don't use it in main source set either, but that's another story :slightly_smiling_face: ), instead we build a "local" context with koinApplication to spin up some modules, and then get bean instances from that context to run tests.

What I'd like to have is some extension functions that I can use to access beans from the context both when I declare a bean, and in the tests.

E.g.:

fun Scope.getServiceTokenProvider(
    audience: String,
    config: ServiceTokenProviderConfig? = null,
) = get<ServiceTokenProvider> {
    parametersOf(audience, config)
}

fun someModule() = module {
   factory<ServiceTokenProvider> { parameters ->
       val audience = parameters.get<String>()
       val config = parameters.getOrNull<ServiceTokenProviderConfig>()
       ServiceTokenProvider(audience, config)
   }.onClose {
       it?.close()
   }
   single {
       SomeClassUsingTokenProvider(getServiceTokenProvider("someAudience"))
   }
}

class TestClass {
   val koin = koinApplication {
            modules(someModule(), ...)
   }.koin
   fun someTestFun() {
       koin.getServiceTokenProvider("someAudience") // <-- can't do that now
   }
}

So what I'd like to have is this Instances interface to be able to declare the extension function with it as a receiver instead of Scope, like that:

fun Instances.getServiceTokenProvider(
    audience: String,
    config: ServiceTokenProviderConfig? = null,
) = get<ServiceTokenProvider> {
    parametersOf(audience, config)
}

fun someModule() = module {
   factory<ServiceTokenProvider> { parameters ->
       val audience = parameters.get<String>()
       val config = parameters.getOrNull<ServiceTokenProviderConfig>()
       ServiceTokenProvider(audience, config)
   }.onClose {
       it?.close()
   }
   single {
       SomeClassUsingTokenProvider(getServiceTokenProvider("someAudience"))
   }
}

class TestClass {
   val koin = koinApplication {
            modules(someModule(), ...)
   }.koin
   
   fun someTestFun() {
       koin.getServiceTokenProvider("someAudience") // <-- this will work now
   }
}

monosoul avatar Feb 27 '24 15:02 monosoul

I guess it might be good to also make KoinComponent interface extend Instnaces interface, but then there will be a clash between the get and inject methods on the interface and the extension functions. It would probably be great if we can include that interface as a breaking change, then we can drop duplicate functions like that.

Although, maybe if Koin will implement Instances, it's not necessary for KoinComponent to extend it.

monosoul avatar Feb 27 '24 15:02 monosoul

This is wonderful. Hope to see this merged.

We have a project where we have loads of abstractions and Kotlin type abuse over KoinComponent, KoinScopeComponent, Koin and Scope for some custom scoping/injecting functionality. Basically injectable qualifiers which holds both a type-safe qualifier and the type to inject. Reduces a bunch of boilerplate for us. This would make those abstractions much easier to maintain (and more easily testable for us)

rmcmk avatar Feb 28 '24 15:02 rmcmk

I guess it might be good to also make KoinComponent interface extend Instnaces interface, but then there will be a clash between the get and inject methods on the interface and the extension functions. It would probably be great if we can include that interface as a breaking change, then we can drop duplicate functions like that.

Although, maybe if Koin will implement Instances, it's not necessary for KoinComponent to extend it.

You make a valid point. If Koin already implements Instances internally, there's no necessity for KoinComponent to extend it. Instead, we could simply delegate back to the Koin context within KoinComponent. Nevertheless, implementing this change would still constitute a breaking change. If any KoinComponent extensions are relied upon for overriding functionality specific to any KoinComponent, the behavior might not remain consistent.

rmcmk avatar Mar 01 '24 14:03 rmcmk

@arnaudgiuliani so what do you say? Does it make sense? I just want to make sure we're on the same page before I invest more time into writing tests etc.

monosoul avatar Mar 07 '24 11:03 monosoul

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Aug 10 '24 12:08 stale[bot]