Skip to content

Commit fede627

Browse files
luozejiaqunluozejiaqun
committed
keep MultibindingIterateKey & multibinding elements definition order across modules
1 parent 62535ab commit fede627

File tree

5 files changed

+214
-41
lines changed

5 files changed

+214
-41
lines changed

‎projects/core/koin-core/src/commonMain/kotlin/org/koin/core/instance/OrderedInstanceFactory.kt

+61
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/*
2+
* Copyright 2017-Present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.koin.core.instance
17+
18+
import org.koin.core.scope.Scope
19+
20+
/**
21+
* Instance holder with order
22+
* Keep the order of instance definitions across modules
23+
* @author luozejiaqun
24+
*/
25+
internal data class OrderedInstanceFactory<T>(
26+
private val instanceFactory: InstanceFactory<T>,
27+
private val order: Int,
28+
private val ascending: Boolean,
29+
) : InstanceFactory<T>(instanceFactory.beanDefinition), Comparable<OrderedInstanceFactory<*>> {
30+
31+
override fun isCreated(context: ResolutionContext?): Boolean =
32+
instanceFactory.isCreated(context)
33+
34+
override fun drop(scope: Scope?) {
35+
instanceFactory.drop(scope)
36+
}
37+
38+
override fun dropAll() {
39+
instanceFactory.dropAll()
40+
}
41+
42+
override fun create(context: ResolutionContext): T =
43+
instanceFactory.create(context)
44+
45+
override fun get(context: ResolutionContext): T =
46+
instanceFactory.get(context)
47+
48+
override fun compareTo(other: OrderedInstanceFactory<*>): Int {
49+
require(ascending == other.ascending) {
50+
"OrderedInstanceFactory can only be compared in same ascending order."
51+
}
52+
return if (ascending) {
53+
order.compareTo(other.order)
54+
} else {
55+
other.order.compareTo(order)
56+
}
57+
}
58+
}
59+
60+
internal fun InstanceFactory<*>.keepDefinitionOrderAcrossModules(ascending: Boolean) =
61+
OrderedInstanceFactory(this, 0, ascending)

‎projects/core/koin-core/src/commonMain/kotlin/org/koin/core/module/Multibinding.kt

+27-20
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import org.koin.core.definition.indexKey
2525
import org.koin.core.instance.InstanceFactory
2626
import org.koin.core.instance.ScopedInstanceFactory
2727
import org.koin.core.instance.SingleInstanceFactory
28+
import org.koin.core.instance.keepDefinitionOrderAcrossModules
2829
import org.koin.core.parameter.ParametersHolder
2930
import org.koin.core.parameter.emptyParametersHolder
3031
import org.koin.core.qualifier.Qualifier
@@ -59,15 +60,7 @@ class MapMultibindingKeyTypeException(msg: String) : Exception(msg)
5960
private class MultibindingIterateKey<T>(
6061
val elementKey: T,
6162
val multibindingQualifier: Qualifier,
62-
val definitionOrder: Int,
63-
) : Comparable<MultibindingIterateKey<T>> {
64-
override fun compareTo(other: MultibindingIterateKey<T>): Int =
65-
definitionOrder.compareTo(other.definitionOrder)
66-
67-
companion object {
68-
val order = AtomicInt(0)
69-
}
70-
}
63+
)
7164

7265
class MapMultibindingElementDefinition<K : Any, E : Any> @PublishedApi internal constructor(
7366
private val multibindingQualifier: Qualifier,
@@ -91,27 +84,35 @@ class MapMultibindingElementDefinition<K : Any, E : Any> @PublishedApi internal
9184

9285
private fun declareElement(key: K, definition: Definition<E>) {
9386
val elementQualifier = multibindingElementQualifier(multibindingQualifier, key)
94-
singleOrScopedInstance(elementQualifier, elementClass, definition)
87+
singleOrScopedInstance(elementQualifier, elementClass, definition) {
88+
it.keepDefinitionOrderAcrossModules(ascending = true)
89+
}
9590
}
9691

9792
private fun declareIterateKey(key: K) {
9893
val iterateKeyQualifier = multibindingIterateKeyQualifier(multibindingQualifier, key)
99-
val definitionOrder = MultibindingIterateKey.order.incrementAndGet()
10094
val oldInstanceFactory =
101-
singleOrScopedInstance(iterateKeyQualifier, MultibindingIterateKey::class) {
102-
MultibindingIterateKey(key, multibindingQualifier, definitionOrder)
103-
}
95+
singleOrScopedInstance(
96+
iterateKeyQualifier,
97+
MultibindingIterateKey::class,
98+
definition = {
99+
MultibindingIterateKey(key, multibindingQualifier)
100+
},
101+
instanceFactoryModifier = {
102+
it.keepDefinitionOrderAcrossModules(ascending = false)
103+
})
104104
checkMultibindingKeyCollision(oldInstanceFactory, key)
105105
}
106106

107107
/**
108108
* @return old definition
109109
*/
110110
@OptIn(KoinInternalApi::class)
111-
private fun <T> singleOrScopedInstance(
111+
private inline fun <T> singleOrScopedInstance(
112112
qualifier: Qualifier,
113113
instanceClass: KClass<*>,
114-
definition: Definition<T>
114+
noinline definition: Definition<T>,
115+
instanceFactoryModifier: (InstanceFactory<*>) -> InstanceFactory<*>
115116
): InstanceFactory<*>? {
116117
val instanceFactory = if (isRootScope) {
117118
SingleInstanceFactory(
@@ -133,7 +134,7 @@ class MapMultibindingElementDefinition<K : Any, E : Any> @PublishedApi internal
133134
Kind.Scoped,
134135
)
135136
)
136-
}
137+
}.let(instanceFactoryModifier)
137138
return indexPrimaryType(instanceFactory)
138139
}
139140

@@ -202,10 +203,16 @@ internal class MapMultibinding<K : Any, V>(
202203
val reversedKeys: LinkedHashSet<K>
203204
get() {
204205
val multibindingKeys = LinkedHashSet<K>()
206+
// MultibindingIterateKey is created by OrderedInstanceFactory(ascending = false)
207+
// so the list here is in reversed order
205208
scope.getAll<MultibindingIterateKey<*>>()
206-
.filter { it.multibindingQualifier == qualifier }
207-
.sortedByDescending { it.definitionOrder } // later element definition wins
208-
.mapNotNullTo(multibindingKeys) { it.elementKey as? K }
209+
.mapNotNullTo(multibindingKeys) {
210+
if (it.multibindingQualifier == qualifier) {
211+
it.elementKey as? K
212+
} else {
213+
null
214+
}
215+
}
209216
return multibindingKeys
210217
}
211218

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

+18-1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package org.koin.core.registry
1717

18+
import co.touchlab.stately.concurrency.AtomicInt
1819
import org.koin.core.Koin
1920
import org.koin.core.annotation.KoinInternalApi
2021
import org.koin.core.definition.IndexKey
@@ -24,6 +25,7 @@ import org.koin.core.definition.indexKey
2425
import org.koin.core.instance.ResolutionContext
2526
import org.koin.core.instance.InstanceFactory
2627
import org.koin.core.instance.NoClass
28+
import org.koin.core.instance.OrderedInstanceFactory
2729
import org.koin.core.instance.ScopedInstanceFactory
2830
import org.koin.core.instance.SingleInstanceFactory
2931
import org.koin.core.module.Module
@@ -37,6 +39,7 @@ import kotlin.reflect.KClass
3739
@Suppress("UNCHECKED_CAST")
3840
@OptIn(KoinInternalApi::class)
3941
class InstanceRegistry(val _koin: Koin) {
42+
private val instanceOrder = AtomicInt(0)
4043

4144
private val _instances = safeHashMap<IndexKey, InstanceFactory<*>>()
4245
val instances: Map<IndexKey, InstanceFactory<*>>
@@ -84,7 +87,11 @@ class InstanceRegistry(val _koin: Koin) {
8487
}
8588
}
8689
_koin.logger.debug("(+) index '$mapping' -> '${factory.beanDefinition}'")
87-
_instances[mapping] = factory
90+
_instances[mapping] = if (factory is OrderedInstanceFactory<*>) {
91+
factory.copy(order = instanceOrder.incrementAndGet())
92+
} else {
93+
factory
94+
}
8895
}
8996

9097
private fun createEagerInstances(instances: Collection<SingleInstanceFactory<*>>) {
@@ -170,9 +177,19 @@ class InstanceRegistry(val _koin: Koin) {
170177
(factory.beanDefinition.primaryType == clazz || factory.beanDefinition.secondaryTypes.contains(clazz))
171178
}
172179
.distinct()
180+
.sortedIfNecessary()
173181
.mapNotNull { it.get(instanceContext) as? T }
174182
}
175183

184+
private fun List<InstanceFactory<*>>.sortedIfNecessary(): List<InstanceFactory<*>> {
185+
return if (all { it is OrderedInstanceFactory<*> }) {
186+
this as List<OrderedInstanceFactory<*>>
187+
this.sorted()
188+
} else {
189+
this
190+
}
191+
}
192+
176193
internal fun unloadModules(modules: Set<Module>) {
177194
modules.forEach { unloadModule(it) }
178195
}

‎projects/core/koin-core/src/commonTest/kotlin/org/koin/core/MapMultibindingTest.kt

+58-5
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ class MapMultibindingTest {
163163
}
164164

165165
@Test
166-
fun `override map multibinding elements`() {
166+
fun `override map multibinding elements in same module`() {
167167
val app = koinApplication {
168168
modules(
169169
module {
@@ -179,17 +179,70 @@ class MapMultibindingTest {
179179
val rootMap: Map<String, Simple.ComponentInterface1> = koin.getMapMultibinding()
180180
assertEquals(1, rootMap.size)
181181
assertEquals(component2, rootMap[keyOfComponent1])
182-
koin.loadModules(
183-
listOf(
182+
assertTrue {
183+
rootMap.values.contains(component2)
184+
}
185+
}
186+
187+
@Test
188+
fun `override map multibinding elements across modules`() {
189+
val app = koinApplication {
190+
modules(
184191
module {
185192
declareMapMultibinding<String, Simple.ComponentInterface1> {
186193
intoMap(keyOfComponent1) { component1 }
187194
}
188-
}
195+
},
196+
module {
197+
declareMapMultibinding<String, Simple.ComponentInterface1> {
198+
intoMap(keyOfComponent1) { component2 }
199+
}
200+
},
189201
)
190-
)
202+
}
203+
204+
val koin = app.koin
205+
val rootMap: Map<String, Simple.ComponentInterface1> = koin.getMapMultibinding()
206+
assertEquals(1, rootMap.size)
207+
assertEquals(component2, rootMap[keyOfComponent1])
208+
assertTrue {
209+
rootMap.values.contains(component2)
210+
}
211+
}
212+
213+
@Test
214+
fun `override map multibinding elements across modules and scopes`() {
215+
val app = koinApplication {
216+
modules(
217+
module {
218+
declareMapMultibinding<String, Simple.ComponentInterface1> {
219+
intoMap(keyOfComponent1) { component1 }
220+
}
221+
},
222+
module {
223+
scope(scopeKey) {
224+
declareMapMultibinding<String, Simple.ComponentInterface1> {
225+
intoMap(keyOfComponent1) { component2 }
226+
}
227+
}
228+
},
229+
)
230+
}
231+
232+
val koin = app.koin
233+
val myScope = koin.createScope(scopeId, scopeKey)
234+
val rootMap: Map<String, Simple.ComponentInterface1> = koin.getMapMultibinding()
235+
val scopeMap: Map<String, Simple.ComponentInterface1> = myScope.getMapMultibinding()
191236
assertEquals(1, rootMap.size)
192237
assertEquals(component1, rootMap[keyOfComponent1])
238+
assertTrue {
239+
rootMap.values.contains(component1)
240+
}
241+
assertEquals(1, scopeMap.size)
242+
assertEquals(component2, scopeMap[keyOfComponent1])
243+
assertTrue {
244+
scopeMap.values.contains(component2)
245+
}
193246
}
194247

195248
@Test

0 commit comments

Comments
 (0)