From 0f8fbfef3ee6813d2d16b376f8b62114db7ed40f Mon Sep 17 00:00:00 2001 From: Shabinder Singh Date: Thu, 12 Dec 2024 11:06:24 +0530 Subject: [PATCH 1/5] typo fixes in Bug_report.md --- .github/ISSUE_TEMPLATE/Bug_report.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/Bug_report.md b/.github/ISSUE_TEMPLATE/Bug_report.md index 44f0c8471..7d711c609 100644 --- a/.github/ISSUE_TEMPLATE/Bug_report.md +++ b/.github/ISSUE_TEMPLATE/Bug_report.md @@ -21,4 +21,4 @@ A clear and concise description of what you expected to happen. [e.g]: `koin-core:3.4.3` **Snippet or Sample project to help reproduce** -Add a snippet or even a small sample project to hel reproduce your case. +Add a snippet or even a small sample project to help reproduce your case. From 3b6a84a52d4de203feafe6f982d0b51555565e89 Mon Sep 17 00:00:00 2001 From: Arnaud Giuliani Date: Mon, 6 Jan 2025 18:06:30 +0100 Subject: [PATCH 2/5] Update Feature_request.md add link to KFIP --- .github/ISSUE_TEMPLATE/Feature_request.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/ISSUE_TEMPLATE/Feature_request.md b/.github/ISSUE_TEMPLATE/Feature_request.md index 92face974..f30c5c477 100644 --- a/.github/ISSUE_TEMPLATE/Feature_request.md +++ b/.github/ISSUE_TEMPLATE/Feature_request.md @@ -4,6 +4,8 @@ about: Suggest an idea for this project --- +If your feature request is a new feature design, please follow [Koin KFIP](https://github.com/InsertKoinIO/KFIP?tab=readme-ov-file) request. + **Is your feature request related to a problem? Please describe.** A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] From 1294aaa0c227422483149deb951c47b5965645de Mon Sep 17 00:00:00 2001 From: Arnaud Giuliani Date: Wed, 8 Jan 2025 14:48:50 +0100 Subject: [PATCH 3/5] Bump 4.0.2-Beta1 --- examples/gradle/versions.gradle | 2 +- projects/gradle.properties | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/gradle/versions.gradle b/examples/gradle/versions.gradle index 561a7f967..231c72b46 100644 --- a/examples/gradle/versions.gradle +++ b/examples/gradle/versions.gradle @@ -2,7 +2,7 @@ ext { // Kotlin kotlin_version = '2.0.21' // Koin Versions - koin_version = '4.0.1-RC2' + koin_version = '4.0.2-Beta1' koin_android_version = koin_version koin_compose_version = koin_version diff --git a/projects/gradle.properties b/projects/gradle.properties index fdacfea77..a983ecb55 100644 --- a/projects/gradle.properties +++ b/projects/gradle.properties @@ -8,7 +8,7 @@ org.gradle.parallel=true kotlin.code.style=official #Koin -koinVersion=4.0.1 +koinVersion=4.0.2-Beta1 #Compose org.jetbrains.compose.experimental.jscanvas.enabled=true From 2c88d1154f3c0f4c598117754853a26cfc9f53f7 Mon Sep 17 00:00:00 2001 From: Arnaud Giuliani Date: Wed, 8 Jan 2025 14:48:58 +0100 Subject: [PATCH 4/5] Fix test app --- examples/androidx-samples/build.gradle | 2 +- .../koin/sample/sandbox/MainApplication.kt | 36 +++++++------------ 2 files changed, 14 insertions(+), 24 deletions(-) diff --git a/examples/androidx-samples/build.gradle b/examples/androidx-samples/build.gradle index 5fd9b4da7..956ce3a70 100644 --- a/examples/androidx-samples/build.gradle +++ b/examples/androidx-samples/build.gradle @@ -68,7 +68,7 @@ dependencies { implementation "io.insert-koin:koin-core-coroutines" implementation "io.insert-koin:koin-androidx-workmanager" implementation "io.insert-koin:koin-androidx-navigation" - implementation "io.insert-koin:koin-androidx-startup" +// implementation "io.insert-koin:koin-androidx-startup" testImplementation "io.insert-koin:koin-test-junit4" testImplementation "io.insert-koin:koin-android-test" } diff --git a/examples/androidx-samples/src/main/java/org/koin/sample/sandbox/MainApplication.kt b/examples/androidx-samples/src/main/java/org/koin/sample/sandbox/MainApplication.kt index 2583c2826..7e7c88931 100644 --- a/examples/androidx-samples/src/main/java/org/koin/sample/sandbox/MainApplication.kt +++ b/examples/androidx-samples/src/main/java/org/koin/sample/sandbox/MainApplication.kt @@ -11,7 +11,6 @@ import org.koin.android.ext.koin.androidFileProperties import org.koin.android.ext.koin.androidLogger import org.koin.androidx.fragment.koin.fragmentFactory import org.koin.androidx.workmanager.koin.workManagerFactory -import org.koin.androix.startup.KoinStartup import org.koin.core.context.GlobalContext.startKoin import org.koin.core.lazyModules import org.koin.core.logger.Level @@ -22,24 +21,15 @@ import org.koin.mp.KoinPlatform import org.koin.sample.sandbox.di.allModules -class MainApplication : Application(), KoinStartup { +class MainApplication : Application() { - override fun onKoinStartup() = koinConfiguration { - androidLogger(Level.DEBUG) - androidContext(this@MainApplication) - androidFileProperties() - fragmentFactory() - workManagerFactory() -// lazyModules(allModules, dispatcher = IO) - modules(allModules) - } - -// override fun onKoinStartup() : KoinAppDeclaration = { +// override fun onKoinStartup() = koinConfiguration { // androidLogger(Level.DEBUG) // androidContext(this@MainApplication) // androidFileProperties() // fragmentFactory() // workManagerFactory() +//// lazyModules(allModules, dispatcher = IO) // modules(allModules) // } @@ -68,20 +58,20 @@ class MainApplication : Application(), KoinStartup { startTime = System.currentTimeMillis() -// startKoin { -// androidLogger(Level.DEBUG) -// androidContext(this@MainApplication) -// androidFileProperties() -// fragmentFactory() -// workManagerFactory() -//// lazyModules(allModules, dispatcher = IO) -// modules(allModules) -// } + startKoin { + androidLogger(Level.DEBUG) + androidContext(this@MainApplication) + androidFileProperties() + fragmentFactory() + workManagerFactory() +// lazyModules(allModules, dispatcher = IO) + modules(allModules) + } //TODO Load/Unload Koin modules scenario cases cancelPendingWorkManager(this) - KoinPlatform.getKoin().waitAllStartJobs() +// KoinPlatform.getKoin().waitAllStartJobs() } } From 20a5ba923edaba74ca3a7afa02a02702b53267ea Mon Sep 17 00:00:00 2001 From: Arnaud Giuliani Date: Wed, 8 Jan 2025 14:41:02 +0100 Subject: [PATCH 5/5] Fix DeclaredScopedInstance solution to provide instance declared in a scope directly, and avoid leak the initial value --- .../core/error/MissingScopeValueException.kt | 22 ++++++++ .../core/instance/DeclaredScopedInstance.kt | 50 ------------------- .../core/instance/ScopedInstanceFactory.kt | 20 +++++--- .../koin/core/registry/InstanceRegistry.kt | 23 +++++---- .../kotlin/org/koin/core/scope/Scope.kt | 19 +++++-- .../org/koin/core/DeclareInstanceJVMTest.kt | 31 ++++++++++-- .../org/koin/core/DeclareInstanceTest.kt | 2 +- .../kotlin/org/koin/test/mock/DeclareMock.kt | 2 +- 8 files changed, 93 insertions(+), 76 deletions(-) create mode 100644 projects/core/koin-core/src/commonMain/kotlin/org/koin/core/error/MissingScopeValueException.kt delete mode 100644 projects/core/koin-core/src/commonMain/kotlin/org/koin/core/instance/DeclaredScopedInstance.kt rename projects/core/koin-core/src/{jvmTest => commonTest}/kotlin/org/koin/core/DeclareInstanceJVMTest.kt (58%) diff --git a/projects/core/koin-core/src/commonMain/kotlin/org/koin/core/error/MissingScopeValueException.kt b/projects/core/koin-core/src/commonMain/kotlin/org/koin/core/error/MissingScopeValueException.kt new file mode 100644 index 000000000..85ebafc2a --- /dev/null +++ b/projects/core/koin-core/src/commonMain/kotlin/org/koin/core/error/MissingScopeValueException.kt @@ -0,0 +1,22 @@ +/* + * Copyright 2017-Present the original author or authors. + * + * 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 org.koin.core.error + +/** + * Scoep value search for given ScopeId is not found + * @author Arnaud Giuliani + */ +class MissingScopeValueException(msg: String) : Exception(msg) diff --git a/projects/core/koin-core/src/commonMain/kotlin/org/koin/core/instance/DeclaredScopedInstance.kt b/projects/core/koin-core/src/commonMain/kotlin/org/koin/core/instance/DeclaredScopedInstance.kt deleted file mode 100644 index 11782fe4c..000000000 --- a/projects/core/koin-core/src/commonMain/kotlin/org/koin/core/instance/DeclaredScopedInstance.kt +++ /dev/null @@ -1,50 +0,0 @@ -/* - * Copyright 2017-Present the original author or authors. - * - * 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 org.koin.core.instance - -import org.koin.core.definition.BeanDefinition -import org.koin.core.scope.Scope -import org.koin.core.scope.ScopeID - -/** - * Declared Instance in scope - Value holder to get back the value for the given scope Id - * to avoid ScopedInstanceFactory where we need the bean definition to return a definition - * - * @author Arnaud Giuliani - */ -class DeclaredScopedInstance(beanDefinition: BeanDefinition, val scopeID : ScopeID) : - InstanceFactory(beanDefinition) { - - private var value : T? = null - - fun setValue(v : T){ - value = v - } - - override fun get(context: ResolutionContext): T { - return value ?: error("Scoped instance not found for ${context.scope.id} in $beanDefinition") - } - - override fun isCreated(context: ResolutionContext?): Boolean = value != null - - override fun drop(scope: Scope?) { - value = null - } - - override fun dropAll() { - value = null - } -} \ No newline at end of file diff --git a/projects/core/koin-core/src/commonMain/kotlin/org/koin/core/instance/ScopedInstanceFactory.kt b/projects/core/koin-core/src/commonMain/kotlin/org/koin/core/instance/ScopedInstanceFactory.kt index 9252c5061..f6fb9a7db 100644 --- a/projects/core/koin-core/src/commonMain/kotlin/org/koin/core/instance/ScopedInstanceFactory.kt +++ b/projects/core/koin-core/src/commonMain/kotlin/org/koin/core/instance/ScopedInstanceFactory.kt @@ -16,6 +16,7 @@ package org.koin.core.instance import org.koin.core.definition.BeanDefinition +import org.koin.core.error.MissingScopeValueException import org.koin.core.scope.Scope import org.koin.core.scope.ScopeID import org.koin.mp.KoinPlatformTools @@ -24,11 +25,18 @@ import org.koin.mp.KoinPlatformTools * Single instance holder * @author Arnaud Giuliani */ -class ScopedInstanceFactory(beanDefinition: BeanDefinition) : +class ScopedInstanceFactory(beanDefinition: BeanDefinition, val holdInstance : Boolean = true) : InstanceFactory(beanDefinition) { private var values = hashMapOf() + fun size() = values.size + + @PublishedApi + internal fun saveValue(id : ScopeID, value : T){ + values[id] = value + } + override fun isCreated(context: ResolutionContext?): Boolean = (values[context?.scope?.id] != null) override fun drop(scope: Scope?) { @@ -42,20 +50,20 @@ class ScopedInstanceFactory(beanDefinition: BeanDefinition) : return if (values[context.scope.id] == null) { super.create(context) } else { - values[context.scope.id] ?: error("Scoped instance not found for ${context.scope.id} in $beanDefinition") + values[context.scope.id] ?: throw MissingScopeValueException("Factory.create - Scoped instance not found for ${context.scope.id} in $beanDefinition") } } override fun get(context: ResolutionContext): T { if (context.scope.scopeQualifier != beanDefinition.scopeQualifier) { - error("Wrong Scope: trying to open instance for ${context.scope.id} in $beanDefinition") + error("Wrong Scope qualifier: trying to open instance for ${context.scope.id} in $beanDefinition") } KoinPlatformTools.synchronized(this) { - if (!isCreated(context)) { - values[context.scope.id] = create(context) + if (!isCreated(context) && holdInstance) { + values[context.scope.id] = super.create(context) } } - return values[context.scope.id] ?: error("Scoped instance not found for ${context.scope.id} in $beanDefinition") + return values[context.scope.id] ?: throw MissingScopeValueException("Factory.get -Scoped instance not found for ${context.scope.id} in $beanDefinition") } override fun dropAll() { diff --git a/projects/core/koin-core/src/commonMain/kotlin/org/koin/core/registry/InstanceRegistry.kt b/projects/core/koin-core/src/commonMain/kotlin/org/koin/core/registry/InstanceRegistry.kt index 11e1ecb16..f5476761c 100644 --- a/projects/core/koin-core/src/commonMain/kotlin/org/koin/core/registry/InstanceRegistry.kt +++ b/projects/core/koin-core/src/commonMain/kotlin/org/koin/core/registry/InstanceRegistry.kt @@ -17,11 +17,11 @@ package org.koin.core.registry import org.koin.core.Koin import org.koin.core.annotation.KoinInternalApi +import org.koin.core.definition.BeanDefinition import org.koin.core.definition.IndexKey import org.koin.core.definition.Kind import org.koin.core.definition._createDefinition import org.koin.core.definition.indexKey -import org.koin.core.instance.DeclaredScopedInstance import org.koin.core.instance.ResolutionContext import org.koin.core.instance.InstanceFactory import org.koin.core.instance.NoClass @@ -29,6 +29,7 @@ import org.koin.core.instance.ScopedInstanceFactory import org.koin.core.instance.SingleInstanceFactory import org.koin.core.module.Module import org.koin.core.module.overrideError +import org.koin.core.parameter.ParametersHolder import org.koin.core.qualifier.Qualifier import org.koin.core.scope.Scope import org.koin.core.scope.ScopeID @@ -114,25 +115,28 @@ class InstanceRegistry(val _koin: Koin) { @PublishedApi internal inline fun scopeDeclaredInstance( instance: T, + scopeQualifier: Qualifier, + scopeID: ScopeID, qualifier: Qualifier? = null, secondaryTypes: List> = emptyList(), allowOverride: Boolean = true, - scopeQualifier: Qualifier, - scopeID: ScopeID, + holdInstance : Boolean ) { - val def = _createDefinition(Kind.Scoped, qualifier, { instance }, secondaryTypes, scopeQualifier) - val indexKey = indexKey(def.primaryType, def.qualifier, def.scopeQualifier) - val existingFactory = instances[indexKey] as? DeclaredScopedInstance + val primaryType = T::class + val indexKey = indexKey(primaryType, qualifier, scopeQualifier) + val existingFactory = instances[indexKey] as? ScopedInstanceFactory if (existingFactory != null) { - existingFactory.setValue(instance) + existingFactory.saveValue(scopeID, instance) } else { - val factory = DeclaredScopedInstance(def,scopeID) + val definitionFunction : Scope.(ParametersHolder) -> T = if (!holdInstance) ( { error("Declared definition of type '$primaryType' shouldn't be executed") } ) else ({ instance }) + val def: BeanDefinition = _createDefinition(Kind.Scoped, qualifier, definitionFunction, secondaryTypes, scopeQualifier) + val factory = ScopedInstanceFactory(def, holdInstance = holdInstance) saveMapping(allowOverride, indexKey, factory) def.secondaryTypes.forEach { clazz -> val index = indexKey(clazz, def.qualifier, def.scopeQualifier) saveMapping(allowOverride, index, factory) } - factory.setValue(instance) + factory.saveValue(scopeID, instance) } } @@ -156,7 +160,6 @@ class InstanceRegistry(val _koin: Koin) { internal fun dropScopeInstances(scope: Scope) { _instances.values.filterIsInstance>().forEach { factory -> factory.drop(scope) } - _instances.values.removeAll { it is DeclaredScopedInstance<*> && it.scopeID == scope.id } } internal fun close() { diff --git a/projects/core/koin-core/src/commonMain/kotlin/org/koin/core/scope/Scope.kt b/projects/core/koin-core/src/commonMain/kotlin/org/koin/core/scope/Scope.kt index 15b07600b..977c0cd20 100644 --- a/projects/core/koin-core/src/commonMain/kotlin/org/koin/core/scope/Scope.kt +++ b/projects/core/koin-core/src/commonMain/kotlin/org/koin/core/scope/Scope.kt @@ -19,6 +19,7 @@ import org.koin.core.Koin import org.koin.core.annotation.KoinInternalApi import org.koin.core.error.ClosedScopeException import org.koin.core.error.MissingPropertyException +import org.koin.core.error.MissingScopeValueException import org.koin.core.error.NoDefinitionFoundException import org.koin.core.instance.ResolutionContext import org.koin.core.logger.Level @@ -61,7 +62,6 @@ class Scope( @KoinInternalApi private var parameterStack: ThreadLocal>? = null - private var _closed: Boolean = false val logger: Logger get() = _koin.logger @@ -182,6 +182,9 @@ class Scope( } catch (e: NoDefinitionFoundException) { _koin.logger.debug("* No instance found for type '${clazz.getFullName()}' on scope '${toString()}'") null + } catch (e: MissingScopeValueException) { + _koin.logger.debug("* No Scoped value found for type '${clazz.getFullName()}' on scope '${toString()}'") + null } } @@ -394,14 +397,16 @@ class Scope( qualifier: Qualifier? = null, secondaryTypes: List> = emptyList(), allowOverride: Boolean = true, + holdInstance : Boolean = false ) = KoinPlatformTools.synchronized(this) { _koin.instanceRegistry.scopeDeclaredInstance( instance, + scopeQualifier, + id, qualifier, secondaryTypes, allowOverride, - scopeQualifier, - id, + holdInstance = holdInstance ) } @@ -466,10 +471,16 @@ class Scope( */ fun close() = KoinPlatformTools.synchronized(this) { _koin.logger.debug("|- (-) Scope - id:'$id'") + _closed = true + _callbacks.forEach { it.onScopeClose(this) } _callbacks.clear() + sourceValue = null - _closed = true + + parameterStack?.get()?.clear() + parameterStack = null + _koin.scopeRegistry.deleteScope(this) } diff --git a/projects/core/koin-core/src/jvmTest/kotlin/org/koin/core/DeclareInstanceJVMTest.kt b/projects/core/koin-core/src/commonTest/kotlin/org/koin/core/DeclareInstanceJVMTest.kt similarity index 58% rename from projects/core/koin-core/src/jvmTest/kotlin/org/koin/core/DeclareInstanceJVMTest.kt rename to projects/core/koin-core/src/commonTest/kotlin/org/koin/core/DeclareInstanceJVMTest.kt index 0ae86b2a1..03ddf006d 100644 --- a/projects/core/koin-core/src/jvmTest/kotlin/org/koin/core/DeclareInstanceJVMTest.kt +++ b/projects/core/koin-core/src/commonTest/kotlin/org/koin/core/DeclareInstanceJVMTest.kt @@ -1,9 +1,13 @@ package org.koin.core -import org.junit.Test +import org.koin.core.annotation.KoinInternalApi +import org.koin.core.instance.ScopedInstanceFactory import org.koin.dsl.koinApplication import org.koin.dsl.module -import java.util.* +import org.koin.mp.KoinPlatformTools +import org.koin.mp.generateId +import kotlin.collections.first +import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotEquals @@ -19,15 +23,18 @@ class DeclareInstanceJVMTest { } } + @OptIn(KoinInternalApi::class) @Test fun `should resolve different scoped declared definitions`() { val koin: Koin = koinApplication { modules(module) }.koin + val factory = koin.instanceRegistry.instances.values.first() as ScopedInstanceFactory<*> + // Create two scopes - val scopeA = koin.createScope(UUID.randomUUID().toString(), MyScope()) - val scopeB = koin.createScope(UUID.randomUUID().toString(), MyScope()) + val scopeA = koin.createScope(KoinPlatformTools.generateId(), MyScope()) + val scopeB = koin.createScope(KoinPlatformTools.generateId(), MyScope()) // Declare scope data on each scope val value_A = "A" @@ -35,8 +42,24 @@ class DeclareInstanceJVMTest { val value_B = "B" scopeB.declare(MyScopedData(value_B)) + assertEquals(2, factory.size()) + // Get scope of each data assertNotEquals(scopeA.get().name, scopeB.get().name) + scopeA.close() + scopeB.close() + + assertEquals(0, factory.size()) + + // Create two scopes + val scopeC = koin.createScope(KoinPlatformTools.generateId(), MyScope()) + val value_C = "C" + scopeC.declare(MyScopedData(value_C)) + + assertEquals(1, factory.size()) + + scopeC.close() + assertEquals(0, factory.size()) } @Test diff --git a/projects/core/koin-core/src/commonTest/kotlin/org/koin/core/DeclareInstanceTest.kt b/projects/core/koin-core/src/commonTest/kotlin/org/koin/core/DeclareInstanceTest.kt index bee031b46..bb517ede2 100644 --- a/projects/core/koin-core/src/commonTest/kotlin/org/koin/core/DeclareInstanceTest.kt +++ b/projects/core/koin-core/src/commonTest/kotlin/org/koin/core/DeclareInstanceTest.kt @@ -260,7 +260,7 @@ class DeclareInstanceTest { val session1 = koin.createScope("session1", named("Session")) val session2 = koin.createScope("session2", named("Session")) - session1.declare(a, allowOverride = false) + session1.declare(a, allowOverride = false, holdInstance = true) session2.get() } diff --git a/projects/core/koin-test/src/commonMain/kotlin/org/koin/test/mock/DeclareMock.kt b/projects/core/koin-test/src/commonMain/kotlin/org/koin/test/mock/DeclareMock.kt index 49dd490f0..c701b211a 100644 --- a/projects/core/koin-test/src/commonMain/kotlin/org/koin/test/mock/DeclareMock.kt +++ b/projects/core/koin-test/src/commonMain/kotlin/org/koin/test/mock/DeclareMock.kt @@ -61,7 +61,7 @@ inline fun Scope.declareMock( stubbing: StubFunction = {}, ): T { val mock = MockProvider.makeMock() - declare(mock, qualifier, secondaryTypes + T::class, true) + declare(mock, qualifier, secondaryTypes + T::class, true, holdInstance = true) mock.apply(stubbing) return mock }