Fix DeclaredScopedInstance solution to provide instance declared in a…

… scope directly, and avoid leak the initial value

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)

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<T>(beanDefinition: BeanDefinition<T>) :
+class ScopedInstanceFactory<T>(beanDefinition: BeanDefinition<T>, val holdInstance : Boolean = true) :
     InstanceFactory<T>(beanDefinition) {
 
     private var values = hashMapOf<ScopeID, T>()
 
+    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<T>(beanDefinition: BeanDefinition<T>) :
         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() {

projects/core/koin-core/src/commonMain/kotlin/org/koin/core/registry/InstanceRegistry.kt

@@ -17,18 +17,19 @@ 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
 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 <reified T> scopeDeclaredInstance(
         instance: T,
+        scopeQualifier: Qualifier,
+        scopeID: ScopeID,
         qualifier: Qualifier? = null,
         secondaryTypes: List<KClass<*>> = 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<T>
+        val primaryType = T::class
+        val indexKey = indexKey(primaryType, qualifier, scopeQualifier)
+        val existingFactory = instances[indexKey] as? ScopedInstanceFactory<T>
         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<T> = _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<ScopedInstanceFactory<*>>().forEach { factory -> factory.drop(scope) }
-        _instances.values.removeAll { it is DeclaredScopedInstance<*> && it.scopeID == scope.id }
     }
 
     internal fun close() {

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<ArrayDeque<ParametersHolder>>? = 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<KClass<*>> = 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)
     }
 

projects/core/koin-core/src/jvmTest/kotlin/org/koin/core/DeclareInstanceJVMTest.kt → 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,24 +23,43 @@ 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<MyScope>(UUID.randomUUID().toString(), MyScope())
-        val scopeB = koin.createScope<MyScope>(UUID.randomUUID().toString(), MyScope())
+        val scopeA = koin.createScope<MyScope>(KoinPlatformTools.generateId(), MyScope())
+        val scopeB = koin.createScope<MyScope>(KoinPlatformTools.generateId(), MyScope())
 
         // Declare scope data on each scope
         val value_A = "A"
         scopeA.declare(MyScopedData(value_A))
         val value_B = "B"
         scopeB.declare(MyScopedData(value_B))
 
+        assertEquals(2, factory.size())
+
         // Get scope of each data
         assertNotEquals(scopeA.get<MyScopedData>().name, scopeB.get<MyScopedData>().name)
+        scopeA.close()
+        scopeB.close()
+
+        assertEquals(0, factory.size())
+
+        // Create two scopes
+        val scopeC = koin.createScope<MyScope>(KoinPlatformTools.generateId(), MyScope())
+        val value_C = "C"
+        scopeC.declare(MyScopedData(value_C))
+
+        assertEquals(1, factory.size())
+
+        scopeC.close()
+        assertEquals(0, factory.size())
     }
 
     @Test

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<Simple.ComponentA>()
     }

projects/core/koin-test/src/commonMain/kotlin/org/koin/test/mock/DeclareMock.kt

@@ -61,7 +61,7 @@ inline fun <reified T : Any> Scope.declareMock(
     stubbing: StubFunction<T> = {},
 ): T {
     val mock = MockProvider.makeMock<T>()
-    declare(mock, qualifier, secondaryTypes + T::class, true)
+    declare(mock, qualifier, secondaryTypes + T::class, true, holdInstance = true)
     mock.apply(stubbing)
     return mock
 }