Koin: Failure to resolve lists (generics)

[westonal]

Describe the bug
If you register two lists and then resolve one inside a bean, the list contains elements of the wrong type. Probably not just limited to lists, but other generics.

To Reproduce

import org.junit.Test
import org.koin.KoinContext
import org.koin.dsl.module.applicationContext
import org.koin.standalone.StandAloneContext
import kotlin.reflect.KClass

class Type1
class Type2
class Type3(val list: List<Type1>)

class ListsTest {

    @Test
    fun `list resolution failure`() {

        val type1Element = Type1()
        StandAloneContext.startKoin(
            listOf(
                applicationContext {
                    bean { listOf(type1Element) }
                },
                applicationContext {
                    bean { listOf(Type2()) }
                },
                applicationContext {
                    bean {
                        Type3(get())
                    }
                }
            )
        )

        (StandAloneContext.koinContext as KoinContext).get<Type3>().list `should equal` listOf(type1Element)
    }
}

java.lang.AssertionError: 
Expected :[Type1@77b52d12]
Actual   :[Type2@2d554825]

And if you try to use the contents of the list, you will get a class cast error.

(StandAloneContext.koinContext as KoinContext).get<Type3>().list.forEach {  }
->
java.lang.ClassCastException: Type2 cannot be cast to Type1

Expected behavior
Type3 instance should have been resolved with the list of Type1 that was registered.

Koin project used and used version (please complete the following information):
koin-core version 0.9.3

[westonal]

Anyone facing this issue, this is a work around:

class Type1
class Type2
class Type3(val list: ListOne)

class ListOne(list: List<Type1>) : List<Type1> by list

class ListsTest {

    @Test
    fun `work around`() {

        val type1Element = Type1()
        StandAloneContext.startKoin(
            listOf(
                applicationContext {
                    bean { ListOne(listOf(type1Element)) }
                },
                applicationContext {
                    bean { listOf(Type2()) }
                },
                applicationContext {
                    bean {
                        Type3(get())
                    }
                }
            )
        )

        (StandAloneContext.koinContext as KoinContext).get<Type3>().list.toList() `should equal` listOf(type1Element)
        (StandAloneContext.koinContext as KoinContext).get<Type3>().list.forEach {  }
    }
}

[arnaudgiuliani]

Hello,

generics data are not kept in Koin definition metadata. Using directly the List type won't work.

Also annother to retrieve the right instance, is to name it:

module{
    single("ints") { ArrayList<Int>() }
    single("longs") { ArrayList<Long>() }
}

and then request it with get<ArrayList>(name = "ints")

[westonal]

The name work around in full.

class Type1
class Type2
class Type3(val list: List<Type1>)

class ListsTest {

    @Test
    fun `work around`() {

        val type1Element = Type1()
        StandAloneContext.startKoin(
                listOf(
                        applicationContext {
                            bean("List<Type1>") { listOf(type1Element) }
                        },
                        applicationContext {
                            bean { listOf(Type2()) }
                        },
                        applicationContext {
                            bean {
                                Type3(get("List<Type1>"))
                            }
                        }
                )
        )

        (StandAloneContext.koinContext as KoinContext).get<Type3>().list `should equal` listOf(type1Element)
    }
}

[westonal]

Just closing without comment? Leaking an unchecked cast to fall over outside Koin is nasty.

All we have is workarounds, this is still a unresolved bug.

[westonal]

Also why is it tagged as question? Its not a question but a detailed bug report.

[arnaudgiuliani]

As described above, current version does not keep generics metadata because we use Kotlin reified type KClass while capturing your definitions with functions.

We will give a try to find if there is a better way to keep the reified type class.

You have to name it, to differentiate your lists. I also suggest you write a list holder instead of using directly a list.

[westonal]

current version does not keep generics metadata

Yes I can see that's what it does now. But there are loads of solutions that are better than returning the wrong type.

For example, maybe dryRun should ensure no generic types were registered, or at least no repeats of the same generic.

I also suggest you write a list holder instead of using directly a list.

Yes that's the work around I suggested.

[westonal]

I also suggest you write a list holder instead of using directly a list.

I suspect an inline class would have the same issue... I need to try it.

inline class ListOne(list: List<Type1>) : List<Type1> by list

[arnaudgiuliani]

But there are loads of solutions that are better than returning the wrong type.

This is not the "wrong type", this is the reified type class: from ArrayList<Int> given type, we will retain the ArrayList class only. Appart if we can save the type "as it" and work with it, this is a limitation of working with such API.

[arnaudgiuliani]

We are working with KClass API which won't offer any additional data about generics.

[westonal]

This is not the "wrong type"

(StandAloneContext.koinContext as KoinContext).get<Type3>().list.forEach {  }
->
java.lang.ClassCastException: Type2 cannot be cast to Type1

This class cast exception is happing outside of Koin, because Koin returned a List<Type2> rather than a List<Type1>.

It may not be fixable, and you may have lost the type information for whatever reason and whatever else, but it's not correct type, so it is the wrong type.

Maybe Koin should just not accept generic registrations at all until it can cope. What do you think of the dry run suggestion?

I don't mind the work arounds, but I want to prevent/catch the issue early, as it's quite cryptic when this happens in my code at runtime!

[arnaudgiuliani]

At start, Koin prevent you from redefining the same definition. If you have 2 lists like below, it cannot be declared.

module {
            single { ArrayList<String>() }
            single { ArrayList<Int>() }
        }

[arnaudgiuliani]

Did you declared it in inner modules?

[westonal]

I don't know what an inner module is, but I gave you a full reproducible code example that I derived from my real world issue.

[arnaudgiuliani]

Yes, I understand what is the problem: https://github.com/InsertKoinIO/koin/issues/188#issuecomment-412334309

You're declaring 2 types of component:

• bean("List<Type1>") { listOf(type1Element) }
• bean { listOf(Type2()) }

even if those 2 have the same type, the first one has a name. Then you have no conflict with your definitions. If you have declared such things, Koin wouldn't let you do so:

• bean { listOf(type1Element) }
• bean { listOf(Type2()) }
  At start, you should have had an override exception.

[westonal]

You've linked the work around! That's not the issue.

Please go back to the very top #188. In the original failing test example, I do not declare a name and I do not get an override exception.

[arnaudgiuliani]

Ok, sorry.

I rewrite your initial test with Koin 1.0:

class Type1
class Type2
class Type3(val list: List<Type1>)


class GenericLists : KoinTest {

    @Test
    fun `list resolution failure`() {

        val type1Element = Type1()

        startKoin(
            listOf(
                module {
                    single { listOf(type1Element) }
                },
                module {
                    single { listOf(Type2()) }
                },
                module {
                    single {
                        Type3(get())
                    }
                }
            )
        )

        assertEquals(get<Type3>().list, listOf(type1Element))
    }
}

I got an BeanOverrideException at start. Then, it's protected against that case.

Then to fix it, the name workaround as you mentioned:

startKoin(
            listOf(
                module {
                    single("default") { listOf(type1Element) }
                },
                module {
                    single { listOf(Type2()) }
                },
                module {
                    single {
                        Type3(get("default"))
                    }
                }
            )
        )

I can add it to the documentation/quick refs to help people on such subject.

[arnaudgiuliani]

Section added to doc/website in beta-9