fix multi step type converters
This CL fixes a bug in TypeAdapterStore where we mixed up assignability
order when checking whether a type converter satisfies an output type,
causing it to either miss valid cases or returning false adapters.
For instance, it would miss an assignment from ArrayList to List but
accept assignment from List to ArrayList.
I've made two other changes:
* When looking for matches from candidates, we'll prioritize exact match
over assignable match. This is still not perfect because we don't visit
all candidates in queue before returning assignable match. In a
followup, we can change this to do a weighted BFS that visits all
possible nodes that are in the same level and add costs to assignable
matches.
* Instead of using isAssignableWithoutVariance, we now use isAssignable.
I'm not 100% sure on that but seems like the right method call though
I'm not sure if it will hit some weird KAPT case that is not covered in
tests. Still, it is better to use isAssignable until we find a failing
case (the test in the original CL which added withoutVariance passes
with this fix)
Bug: 175916061
Test: TypeAdapterStoreTest#multiStepTypeConverters
Change-Id: I7a750ada653ffa82dce477e2af3bbf5bf46e026d
diff --git a/room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt b/room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
index 68c4999..3d6ff1b 100644
--- a/room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
+++ b/room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
@@ -594,14 +594,28 @@
val excludes = arrayListOf<XType>()
val queue = LinkedList<TypeConverter>()
- fun exactMatch(candidates: List<TypeConverter>): TypeConverter? {
- return candidates.firstOrNull {
- outputs.any { output -> it.to.isAssignableFromWithoutVariance(output) }
+ fun List<TypeConverter>.findMatchingConverter(): TypeConverter? {
+ // We prioritize exact match over assignable. To do that, this variable keeps any
+ // assignable match and if we cannot find exactly same type match, we'll return the
+ // assignable match.
+ var assignableMatchFallback: TypeConverter? = null
+ this.forEach { converter ->
+ outputs.forEach { output ->
+ if (output.isSameType(converter.to)) {
+ return converter
+ } else if (assignableMatchFallback == null &&
+ output.isAssignableFrom(converter.to)
+ ) {
+ // if we don't find exact match, we'll return this.
+ assignableMatchFallback = converter
+ }
+ }
}
+ return assignableMatchFallback
}
inputs.forEach { input ->
val candidates = getAllTypeConverters(input, excludes)
- val match = exactMatch(candidates)
+ val match = candidates.findMatchingConverter()
if (match != null) {
return match
}
@@ -615,7 +629,7 @@
val prev = queue.pop()
val from = prev.to
val candidates = getAllTypeConverters(from, excludes)
- val match = exactMatch(candidates)
+ val match = candidates.findMatchingConverter()
if (match != null) {
return CompositeTypeConverter(prev, match)
}
diff --git a/room/compiler/src/test/kotlin/androidx/room/solver/TypeAdapterStoreTest.kt b/room/compiler/src/test/kotlin/androidx/room/solver/TypeAdapterStoreTest.kt
index 04d9d40e..e56425c 100644
--- a/room/compiler/src/test/kotlin/androidx/room/solver/TypeAdapterStoreTest.kt
+++ b/room/compiler/src/test/kotlin/androidx/room/solver/TypeAdapterStoreTest.kt
@@ -34,6 +34,7 @@
import androidx.room.ext.T
import androidx.room.parser.SQLTypeAffinity
import androidx.room.processor.Context
+import androidx.room.processor.CustomConverterProcessor
import androidx.room.processor.ProcessorErrors
import androidx.room.solver.binderprovider.DataSourceFactoryQueryResultBinderProvider
import androidx.room.solver.binderprovider.DataSourceQueryResultBinderProvider
@@ -45,9 +46,12 @@
import androidx.room.solver.shortcut.binderprovider.RxCallableDeleteOrUpdateMethodBinderProvider
import androidx.room.solver.shortcut.binderprovider.RxCallableInsertMethodBinderProvider
import androidx.room.solver.types.CompositeAdapter
+import androidx.room.solver.types.CompositeTypeConverter
+import androidx.room.solver.types.CustomTypeConverterWrapper
import androidx.room.solver.types.EnumColumnTypeAdapter
import androidx.room.solver.types.TypeConverter
import androidx.room.testing.context
+import com.google.common.truth.Truth.assertThat
import com.squareup.javapoet.TypeName
import org.hamcrest.CoreMatchers.`is`
import org.hamcrest.CoreMatchers.instanceOf
@@ -237,7 +241,7 @@
invocation.context,
dateTypeConverters(invocation.processingEnv)
)
- val tDate = invocation.processingEnv.requireType("java.util.Date")
+ val tDate = invocation.processingEnv.requireType("java.util.Date").makeNullable()
val adapter = store.findCursorValueReader(tDate, SQLTypeAffinity.INTEGER)
assertThat(adapter, notNullValue())
assertThat(adapter?.typeMirror(), `is`(tDate))
@@ -261,6 +265,99 @@
}
@Test
+ fun multiStepTypeConverters() {
+ val source = Source.kotlin(
+ "Foo.kt",
+ """
+ import androidx.room.*
+ interface Type1_Super
+ interface Type1 : Type1_Super
+ interface Type1_Sub : Type1
+ interface Type2_Super
+ interface Type2 : Type2_Super
+ interface Type2_Sub : Type2
+ interface JumpType_1
+ interface JumpType_2
+ interface JumpType_3
+ class MyConverters {
+ @TypeConverter
+ fun t1_jump1(inp : Type1): JumpType_1 = TODO()
+ @TypeConverter
+ fun jump1_t2_Sub(inp : JumpType_1): Type2_Sub = TODO()
+ @TypeConverter
+ fun jump1_t2(inp : JumpType_1): Type2 = TODO()
+ @TypeConverter
+ fun t1_super_jump2(inp : Type1_Super): JumpType_2 = TODO()
+ @TypeConverter
+ fun jump2_jump3(inp : JumpType_2): JumpType_3 = TODO()
+ @TypeConverter
+ fun jump2_Type2_Sub(inp : JumpType_3): Type2_Sub = TODO()
+ }
+ """.trimIndent()
+ )
+ runProcessorTest(sources = listOf(source)) { invocation ->
+ val convertersElm = invocation.processingEnv.requireTypeElement("MyConverters")
+ val converters = CustomConverterProcessor(invocation.context, convertersElm)
+ .process()
+ val store = TypeAdapterStore.create(
+ invocation.context,
+ converters.map(::CustomTypeConverterWrapper)
+ )
+
+ fun TypeConverter.toSignature(): String {
+ return when (this) {
+ is CompositeTypeConverter -> "${conv1.toSignature()} : ${conv2.toSignature()}"
+ else -> "${from.typeName} -> ${to.typeName}"
+ }
+ }
+
+ fun findConverter(from: String, to: String): String? {
+ val input = invocation.processingEnv.requireType(from)
+ val output = invocation.processingEnv.requireType(to)
+ return store.findTypeConverter(
+ input = input,
+ output = output
+ )?.also {
+ // validate that it makes sense to ensure test is correct
+ assertThat(output.isAssignableFrom(it.to)).isTrue()
+ assertThat(it.from.isAssignableFrom(input)).isTrue()
+ }?.toSignature()
+ }
+ assertThat(
+ findConverter("Type1", "Type2")
+ ).isEqualTo(
+ "Type1 -> JumpType_1 : JumpType_1 -> Type2"
+ )
+ assertThat(
+ findConverter("Type1", "Type2_Sub")
+ ).isEqualTo(
+ "Type1 -> JumpType_1 : JumpType_1 -> Type2_Sub"
+ )
+ assertThat(
+ findConverter("Type1_Super", "Type2_Super")
+ ).isEqualTo(
+ "Type1_Super -> JumpType_2 : JumpType_2 -> JumpType_3 : JumpType_3 -> Type2_Sub"
+ )
+ assertThat(
+ findConverter("Type1", "Type2_Sub")
+ ).isEqualTo(
+ "Type1 -> JumpType_1 : JumpType_1 -> Type2_Sub"
+ )
+ assertThat(
+ findConverter("Type1_Sub", "Type2_Sub")
+ ).isEqualTo(
+ "Type1 -> JumpType_1 : JumpType_1 -> Type2_Sub"
+ )
+ assertThat(
+ findConverter("Type2", "Type2_Sub")
+ ).isNull()
+ assertThat(
+ findConverter("Type2", "Type1")
+ ).isNull()
+ }
+ }
+
+ @Test
fun testIntList() {
runProcessorTest { invocation ->
val binders = createIntListToStringBinders(invocation)
diff --git a/room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/vo/JavaPojoConverter.kt b/room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/vo/JavaPojoConverter.kt
index 5144bf6..0d1d3b3 100644
--- a/room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/vo/JavaPojoConverter.kt
+++ b/room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/vo/JavaPojoConverter.kt
@@ -42,7 +42,7 @@
}
@TypeConverter
- fun toString(pojoList: List<JavaPojo>): String? {
+ fun toString(pojoList: List<JavaPojo>?): String {
return ""
}
}
\ No newline at end of file
