Skip to content

Commit

Permalink
[core] Ensure onClose callbacks are invoked in reversed initializatio…
Browse files Browse the repository at this point in the history 
…n order
  • Loading branch information
@shchuko
shchuko committed Oct 11, 2024
1 parent 74f9198 commit 9401779
Show file tree
Hide file tree
Showing 3 changed files with 230 additions and 12 deletions.
30 changes: 28 additions & 2 deletions projects/core/koin-core/src/commonMain/kotlin/org/koin/core/instance/InstanceFactory.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

package org.koin.core.instance

import co.touchlab.stately.concurrency.AtomicBoolean
import co.touchlab.stately.concurrency.AtomicLong
import org.koin.core.definition.BeanDefinition
import org.koin.core.error.InstanceCreationException
import org.koin.core.parameter.ParametersHolder
Expand All @@ -28,8 +30,12 @@ import org.koin.mp.Lockable
/**
* Koin Instance Holder
* create/get/release an instance of given definition
*
* Implements [Comparable] to provide factories soring by instance creation order
*/
abstract class InstanceFactory<T>(val beanDefinition: BeanDefinition<T>) : Lockable() {
abstract class InstanceFactory<T>(val beanDefinition: BeanDefinition<T>) : Lockable(), Comparable<InstanceFactory<*>> {
private val _instanceCreationOrderSet = AtomicBoolean(false)
private var _instanceCreationOrderPosition = Long.MAX_VALUE

/**
* Retrieve an instance
Expand All @@ -47,17 +53,36 @@ abstract class InstanceFactory<T>(val beanDefinition: BeanDefinition<T>) : Locka
context.logger.debug("| (+) '$beanDefinition'")
try {
val parameters: ParametersHolder = context.parameters ?: emptyParametersHolder()
return beanDefinition.definition.invoke(
val instance = beanDefinition.definition.invoke(
context.scope,
parameters,
)
if (_instanceCreationOrderSet.compareAndSet(false, true)) {
_instanceCreationOrderPosition = _instanceCreationOrderCounter.incrementAndGet()

// just to be sure that counter provides normal values
check(_instanceCreationOrderPosition >= 0) {
"Unexpected negative instance creation order position"
}

// it's required to have `Long.MAX_VALUE` instantiations happened to make this check fail,
// which is not likely to happen
check(_instanceCreationOrderPosition < Long.MAX_VALUE) {
"Instance creation order position reached Long.MAX_VALUE"
}
}
return instance
} catch (e: Exception) {
val stack = KoinPlatformTools.getStackTrace(e)
context.logger.error("* Instance creation error : could not create instance for '$beanDefinition': $stack")
throw InstanceCreationException("Could not create instance for '$beanDefinition'", e)
}
}

override fun compareTo(other: InstanceFactory<*>): Int {
return _instanceCreationOrderPosition.compareTo(other._instanceCreationOrderPosition)
}

/**
* Is instance created
*/
Expand All @@ -72,5 +97,6 @@ abstract class InstanceFactory<T>(val beanDefinition: BeanDefinition<T>) : Locka

companion object {
const val ERROR_SEPARATOR = "\n\t"
private val _instanceCreationOrderCounter = AtomicLong(-1)
}
}
26 changes: 16 additions & 10 deletions projects/core/koin-core/src/commonMain/kotlin/org/koin/core/registry/InstanceRegistry.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ import org.koin.core.scope.Scope
import org.koin.core.scope.ScopeID
import org.koin.mp.KoinPlatformTools.safeHashMap
import kotlin.reflect.KClass
import kotlin.sequences.sortedDescending


@Suppress("UNCHECKED_CAST")
@OptIn(KoinInternalApi::class)
Expand All @@ -41,6 +43,7 @@ class InstanceRegistry(val _koin: Koin) {
val instances: Map<IndexKey, InstanceFactory<*>>
get() = _instances


private val eagerInstances = hashMapOf<Int, SingleInstanceFactory<*>>()

internal fun loadModules(modules: Set<Module>, allowOverride: Boolean) {
Expand Down Expand Up @@ -152,13 +155,18 @@ class InstanceRegistry(val _koin: Koin) {
}

internal fun dropScopeInstances(scope: Scope) {
_instances.values.filterIsInstance<ScopedInstanceFactory<*>>().forEach { factory -> factory.drop(scope) }
_instances.values
.asSequence()
.filterIsInstance<ScopedInstanceFactory<*>>()
.sortedDescending()
.forEach { factory -> factory.drop(scope) }
}

internal fun close() {
_instances.forEach { (_, factory) ->
factory.dropAll()
}
_instances.values
.asSequence()
.sortedDescending()
.forEach { factory -> factory.dropAll() }
_instances.clear()
}

Expand All @@ -181,12 +189,10 @@ class InstanceRegistry(val _koin: Koin) {
}

private fun unloadModule(module: Module) {
module.mappings.keys.forEach { mapping ->
if (_instances.containsKey(mapping)) {
_instances[mapping]?.dropAll()
_instances.remove(mapping)
}
}
module.mappings.keys
.mapNotNull { mapping -> _instances.remove(mapping) }
.sortedDescending()
.forEach { factory -> factory.dropAll() }
}

fun size(): Int {
Expand Down
186 changes: 186 additions & 0 deletions projects/core/koin-core/src/commonTest/kotlin/org/koin/dsl/CloseDefinitionTest.kt
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
package org.koin.dsl

import org.koin.Simple
import org.koin.core.annotation.KoinInternalApi
import org.koin.core.logger.Level
import org.koin.core.qualifier.named
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNotNull
import kotlin.test.assertTrue

class CloseDefinitionTest {
Expand Down Expand Up @@ -50,4 +54,186 @@ class CloseDefinitionTest {
koin.unloadModules(listOf(module))
assertTrue(!closed)
}

@OptIn(KoinInternalApi::class)
@Test
fun test_onClose_order() {
val closeLog = mutableListOf<String>()

val module = module {
single { Simple.ComponentC(get()) } onClose {
if (it != null) {
closeLog += "c"
}
}
single { Simple.ComponentA() } onClose {
if (it != null) {
closeLog += "a"
}
}
single { Simple.ComponentB(get()) } onClose {
if (it != null) {
closeLog += "b"
}
}
}

val koin = koinApplication {
printLogger(Level.DEBUG)
modules(
module,
)
}.koin

assertNotNull(koin.getOrNull<Simple.ComponentC>())
assertEquals(
expected = emptyList(),
actual = closeLog,
)

// check instances sorting meets instantiation order
assertEquals(
expected = listOf(Simple.ComponentA::class, Simple.ComponentB::class, Simple.ComponentC::class),
actual = koin.instanceRegistry.instances.values.sorted().map { it.beanDefinition.primaryType },
)

koin.close()
assertEquals(
expected = listOf("c", "b", "a"),
actual = closeLog,
)
}

@Test
fun test_onClose_order_unload() {
val closeLog = mutableListOf<String>()

val moduleFirst = module {
single { Simple.MySingle(42) } onClose {
if (it != null) {
closeLog += "single"
}
}
}

val moduleSecond = module {
single { Simple.ComponentC(get()) } onClose {
if (it != null) {
closeLog += "c"
}
}
single { Simple.ComponentA() } onClose {
if (it != null) {
closeLog += "a"
}
}
single { Simple.ComponentB(get()) } onClose {
if (it != null) {
closeLog += "b"
}
}
}

val koin = koinApplication {
printLogger(Level.DEBUG)
modules(
moduleFirst,
moduleSecond,
)
}.koin

assertNotNull(koin.getOrNull<Simple.MySingle>())
assertNotNull(koin.getOrNull<Simple.ComponentC>())
assertEquals(emptyList(), closeLog)

koin.unloadModules(listOf(moduleSecond))
assertEquals(
expected = listOf("c", "b", "a"),
actual = closeLog,
)

koin.loadModules(listOf(moduleSecond))
assertNotNull(koin.getOrNull<Simple.ComponentC>())

koin.unloadModules(listOf(moduleSecond))
assertEquals(
expected = listOf("c", "b", "a", "c", "b", "a"),
actual = closeLog,
)

koin.close()
assertEquals(
expected = listOf("c", "b", "a", "c", "b", "a", "single"),
actual = closeLog,
)
}


@Test
fun test_onClose_order_scoped() {
val closeLog = mutableListOf<String>()

val scopeId = "_test_scope_id_"
val scopeKey = named("_test_scope_")

val module = module {
scope(scopeKey) {
scoped { Simple.ComponentC(get()) } onClose {
if (it != null) {
closeLog += "c"
}
}
scoped { Simple.ComponentA() } onClose {
if (it != null) {
closeLog += "a"
}
}
scoped { Simple.ComponentB(get()) } onClose {
if (it != null) {
closeLog += "b"
}
}
}
}

val koin = koinApplication {
printLogger(Level.DEBUG)
modules(
module,
)
}.koin

assertEquals(
expected = emptyList(),
actual = closeLog,
)

koin.createScope(scopeId, scopeKey)
assertEquals(
expected = emptyList(),
actual = closeLog,
)

koin.getScope(scopeId).let {
assertNotNull(it.getOrNull<Simple.ComponentC>())
it.close()
}
assertEquals(
expected = listOf("c", "b", "a"),
actual = closeLog,
)

koin.createScope(scopeId, scopeKey)
assertNotNull(koin.getScope(scopeId).getOrNull<Simple.ComponentC>())
assertEquals(
expected = listOf("c", "b", "a"),
actual = closeLog,
)

koin.close()
assertEquals(
expected = listOf("c", "b", "a", "c", "b", "a"),
actual = closeLog,
)
}
}

0 comments on commit 9401779

Please sign in to comment.