Check against duplicated entries for `ViewModelFactoryDsl`
* `ViewModelFactoryDsl` allows duplicated initializers, unnecessary keeping a reference to duplicates while the returned `InitializerViewModelFactory` ignores the duplicated entries. That can cause subtle bugs at runtime.
* To give immediate feedback, we now check against duplicated and throw an `IllegalArgumentException(message = "A `initializer` with the same `clazz` has already been added: $qualifiedName")`.
* In addition to those changes:
* All tests now are using `this: CreationExtras` in their `initializer` to access the extras.
* Different test values are using different keys.
RelNote: "`InitializerViewModelFactory` (including `viewModelFactory` builder function) will now throw an `IllegalArgumentException` if a `initializer` with the same `clazz: KClass<VM : ViewModel>` has already been added."
Fixes: b/326226169
Test: ViewModelInitializerTest
Change-Id: Ic3a36b87c170bd886e60fc264a352096be003c02
diff --git a/lifecycle/lifecycle-viewmodel/src/commonMain/kotlin/androidx/lifecycle/viewmodel/InitializerViewModelFactory.kt b/lifecycle/lifecycle-viewmodel/src/commonMain/kotlin/androidx/lifecycle/viewmodel/InitializerViewModelFactory.kt
index df164d3..a38409b 100644
--- a/lifecycle/lifecycle-viewmodel/src/commonMain/kotlin/androidx/lifecycle/viewmodel/InitializerViewModelFactory.kt
+++ b/lifecycle/lifecycle-viewmodel/src/commonMain/kotlin/androidx/lifecycle/viewmodel/InitializerViewModelFactory.kt
@@ -40,7 +40,7 @@
public class InitializerViewModelFactoryBuilder
public constructor() {
- private val initializers = mutableListOf<ViewModelInitializer<*>>()
+ private val initializers = mutableMapOf<KClass<*>, ViewModelInitializer<*>>()
/**
* Associates the specified [initializer] with the given [ViewModel] class.
@@ -53,7 +53,10 @@
clazz: KClass<T>,
initializer: CreationExtras.() -> T,
) {
- initializers += ViewModelInitializer(clazz, initializer)
+ require(clazz !in initializers) {
+ "A `initializer` with the same `clazz` has already been added: ${clazz.qualifiedName}."
+ }
+ initializers[clazz] = ViewModelInitializer(clazz, initializer)
}
/**
@@ -61,7 +64,7 @@
* builder.
*/
public fun build(): ViewModelProvider.Factory =
- ViewModelProviders.createInitializerFactory(initializers)
+ ViewModelProviders.createInitializerFactory(initializers.values)
}
/**
diff --git a/lifecycle/lifecycle-viewmodel/src/commonMain/kotlin/androidx/lifecycle/viewmodel/internal/ViewModelProviders.kt b/lifecycle/lifecycle-viewmodel/src/commonMain/kotlin/androidx/lifecycle/viewmodel/internal/ViewModelProviders.kt
index c419e3f..03de848 100644
--- a/lifecycle/lifecycle-viewmodel/src/commonMain/kotlin/androidx/lifecycle/viewmodel/internal/ViewModelProviders.kt
+++ b/lifecycle/lifecycle-viewmodel/src/commonMain/kotlin/androidx/lifecycle/viewmodel/internal/ViewModelProviders.kt
@@ -55,7 +55,7 @@
)
internal fun createInitializerFactory(
- initializers: List<ViewModelInitializer<*>>,
+ initializers: Collection<ViewModelInitializer<*>>,
): ViewModelProvider.Factory = InitializerViewModelFactory(*initializers.toTypedArray())
internal fun createInitializerFactory(
diff --git a/lifecycle/lifecycle-viewmodel/src/commonTest/kotlin/androidx/lifecycle/viewmodel/ViewModelInitializerTest.kt b/lifecycle/lifecycle-viewmodel/src/commonTest/kotlin/androidx/lifecycle/viewmodel/ViewModelInitializerTest.kt
index 7214a33..5132a22 100644
--- a/lifecycle/lifecycle-viewmodel/src/commonTest/kotlin/androidx/lifecycle/viewmodel/ViewModelInitializerTest.kt
+++ b/lifecycle/lifecycle-viewmodel/src/commonTest/kotlin/androidx/lifecycle/viewmodel/ViewModelInitializerTest.kt
@@ -19,19 +19,25 @@
import androidx.kruth.assertThat
import androidx.lifecycle.ViewModel
import kotlin.test.Test
+import kotlin.test.fail
class ViewModelInitializerTest {
+
@Test
- fun testInitializerFactory() {
- val key = object : CreationExtras.Key<String> {}
+ fun viewModelFactory_withUniqueInitializers_withCreationExtras_returnsViewModels() {
+ val key1 = object : CreationExtras.Key<String> {}
val value1 = "test_value1"
- val extras1 = MutableCreationExtras().apply { set(key, value1) }
+ val extras1 = MutableCreationExtras().apply { set(key1, value1) }
+
+ val key2 = object : CreationExtras.Key<String> {}
val value2 = "test_value2"
- val extras2 = MutableCreationExtras().apply { set(key, value2) }
+ val extras2 = MutableCreationExtras().apply { set(key2, value2) }
+
val factory = viewModelFactory {
- initializer { TestViewModel1(extras1[key]) }
- initializer { TestViewModel2(extras2[key]) }
+ initializer<TestViewModel1> { TestViewModel1(get(key1)) }
+ initializer<TestViewModel2> { TestViewModel2(get(key2)) }
}
+
val viewModel1: TestViewModel1 = factory.create(TestViewModel1::class, extras1)
val viewModel2: TestViewModel2 = factory.create(TestViewModel2::class, extras2)
assertThat(viewModel1.value).isEqualTo(value1)
@@ -39,13 +45,26 @@
}
@Test
- fun testInitializerFactoryNoInitializer() {
- val key = object : CreationExtras.Key<String> {}
- val value = "test_value"
- val extras = MutableCreationExtras().apply { set(key, value) }
+ fun viewModelFactory_withDuplicatedInitializers_throwsException() {
+ try {
+ viewModelFactory {
+ initializer<TestViewModel1> { TestViewModel1(null) }
+ initializer<TestViewModel1> { TestViewModel1(null) }
+ }
+ fail("Expected `IllegalArgumentException` but no exception has been throw.")
+ } catch (e: IllegalArgumentException) {
+ assertThat(e).hasMessageThat().isEqualTo(
+ "A `initializer` with the same `clazz` has already been added: " +
+ "${TestViewModel1::class.qualifiedName}."
+ )
+ }
+ }
+
+ @Test
+ fun viewModelFactory_noInitializers_throwsException() {
val factory = viewModelFactory { }
try {
- factory.create(TestViewModel1::class, extras)
+ factory.create(TestViewModel1::class, CreationExtras.Empty)
} catch (e: IllegalArgumentException) {
assertThat(e).hasMessageThat().isEqualTo(
"No initializer set for given class ${TestViewModel1::class.qualifiedName}"
