Skip to content

Commit 54388a5

Browse files
mcarleiomcarleio
committed
fix(#189): do not require konvert.enforce-not-null for mapping to value class when source and target are nullable
1 parent 038fba3 commit 54388a5

File tree

5 files changed

+91
-64
lines changed

5 files changed

+91
-64
lines changed

‎CHANGELOG.md‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,19 @@ All notable changes to this project will be documented in this file.
44

55
## [Unreleased]
66

7+
### Bug fixes
8+
* do not require `konvert.enforce-not-null` for mapping to value class when source and target are nullable [#189](https://github.com/mcarleio/konvert/issues/189)
9+
710
## [4.3.1]
811

912
### Bug fixes
10-
* respect source and target class visibilities for extension function generation (#180, #181)
13+
* respect source and target class visibilities for extension function generation [#180](https://github.com/mcarleio/konvert/issues/180)
1114

1215
## [4.3.0]
1316

1417
### New features
1518

16-
* Two new type converters, which handle `value class` mappings, i.e., to map from/to e.g. String to/from value class with a String inside:
19+
* Two new type converters, which handle `value class` mappings, i.e., to map from/to e.g. String to/from value class with a String inside: [#178](https://github.com/mcarleio/konvert/issues/178)
1720
* `ValueClassToXConverter`
1821
* `XToValueClassConverter`
1922

@@ -161,7 +164,6 @@ Update to Kotlin 1.9.22 and KSP 1.0.16
161164
### Bug fixes:
162165
* ignore private and extension functions in `@Konverter` annotated interfaces [#30](https://github.com/mcarleio/konvert/issues/30)
163166

164-
165167
## [2.3.1]
166168

167169
### Publication fix

‎converter/src/main/kotlin/io/mcarle/konvert/converter/XToValueClassConverter.kt‎

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -26,29 +26,15 @@ class XToValueClassConverter : AbstractTypeConverter() {
2626
if (targetClassDeclaration.classKind != ClassKind.CLASS) return false
2727
if (Modifier.VALUE !in targetClassDeclaration.modifiers) return false
2828

29-
val sourceForTypeConverter = if (source.isNullable() && target.isNullable()) {
30-
source.makeNotNullable()
31-
} else {
32-
source
33-
}
34-
3529
return targetClassDeclaration
3630
.availableConstructors()
37-
.any { constructor ->
38-
val parameterType = constructor.parameters.first().type.resolve()
39-
TypeConverterRegistry.any {
40-
it.matches(
41-
source = sourceForTypeConverter,
42-
target = parameterType,
43-
)
44-
}
45-
}
31+
.any { constructor -> findMatchingTypeConverter(constructor, source, target) != null }
4632
}
4733

4834
override fun convert(fieldName: String, source: KSType, target: KSType): CodeBlock {
4935
val targetClassDeclaration = requireNotNull(target.classDeclaration())
5036

51-
val (constructor, typeConverter) = extractBestConstructor(targetClassDeclaration, source)
37+
val (constructor, typeConverter) = extractBestConstructor(source, target)
5238
val propertyType = constructor.parameters.first().type.resolve()
5339

5440
return if (source.isNullable() && target.isNullable()) {
@@ -81,24 +67,32 @@ class XToValueClassConverter : AbstractTypeConverter() {
8167
* extracts the best matching constructor based on the registered TypeConverters priorities
8268
*/
8369
private fun extractBestConstructor(
84-
targetClassDeclaration: KSClassDeclaration,
85-
source: KSType
70+
source: KSType,
71+
target: KSType
8672
): Pair<KSFunctionDeclaration, TypeConverter> {
87-
return targetClassDeclaration
73+
return requireNotNull(target.classDeclaration())
8874
.availableConstructors()
89-
.associateWith { constructor ->
90-
val parameterType = constructor.parameters.first().type.resolve()
91-
TypeConverterRegistry.firstOrNull {
92-
it.matches(
93-
source = source,
94-
target = parameterType,
95-
)
96-
}
97-
}
75+
.associateWith { constructor -> findMatchingTypeConverter(constructor, source, target) }
9876
.filterValueNotNull()
9977
.minBy { it.second.priority }
10078
}
10179

80+
private fun findMatchingTypeConverter(constructor: KSFunctionDeclaration, source: KSType, target: KSType): TypeConverter? {
81+
val sourceForTypeConverter = if (source.isNullable() && target.isNullable()) {
82+
source.makeNotNullable()
83+
} else {
84+
source
85+
}
86+
87+
val parameterType = constructor.parameters.first().type.resolve()
88+
return TypeConverterRegistry.firstOrNull {
89+
it.matches(
90+
source = sourceForTypeConverter,
91+
target = parameterType,
92+
)
93+
}
94+
}
95+
10296
private fun <K, V> Map<out K, V?>.filterValueNotNull(): List<Pair<K, V>> {
10397
return mapNotNull { (k, v) -> v?.let { k to it } }
10498
}

‎converter/src/test/kotlin/io/mcarle/konvert/converter/ValueClassToXConverterITest.kt‎

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,20 +25,42 @@ class ValueClassToXConverterITest : ConverterITest() {
2525

2626
@JvmStatic
2727
fun sourceAndTargets(): List<Arguments> = listOf(
28-
"SimpleValueClass",
29-
"ValueClassWithNullable",
30-
"ValueClassWithAdditionalProperties"
31-
).cartesianProductWithNullableCombinations(
32-
"String",
33-
"Int",
28+
// source, target, enforceNotNull
29+
Arguments.of("SimpleValueClass", "String", false),
30+
Arguments.of("SimpleValueClass", "String?", false),
31+
Arguments.of("SimpleValueClass?", "String", true),
32+
Arguments.of("SimpleValueClass?", "String?", false),
33+
Arguments.of("SimpleValueClass", "Int", false),
34+
Arguments.of("SimpleValueClass", "Int?", false),
35+
Arguments.of("SimpleValueClass?", "Int", true),
36+
Arguments.of("SimpleValueClass?", "Int?", false),
37+
38+
Arguments.of("ValueClassWithNullable", "String", true), // enforceNotNull because value class property is nullable
39+
Arguments.of("ValueClassWithNullable", "String?", false),
40+
Arguments.of("ValueClassWithNullable?", "String", true),
41+
Arguments.of("ValueClassWithNullable?", "String?", false),
42+
Arguments.of("ValueClassWithNullable", "Int", true), // enforceNotNull because value class property is nullable
43+
Arguments.of("ValueClassWithNullable", "Int?", false),
44+
Arguments.of("ValueClassWithNullable?", "Int", true),
45+
Arguments.of("ValueClassWithNullable?", "Int?", false),
46+
47+
Arguments.of("ValueClassWithAdditionalProperties", "String", false),
48+
Arguments.of("ValueClassWithAdditionalProperties", "String?", false),
49+
Arguments.of("ValueClassWithAdditionalProperties?", "String", true),
50+
Arguments.of("ValueClassWithAdditionalProperties?", "String?", false),
51+
Arguments.of("ValueClassWithAdditionalProperties", "Int", false),
52+
Arguments.of("ValueClassWithAdditionalProperties", "Int?", false),
53+
Arguments.of("ValueClassWithAdditionalProperties?", "Int", true),
54+
Arguments.of("ValueClassWithAdditionalProperties?", "Int?", false),
3455
)
3556

3657
}
3758

3859
@ParameterizedTest
3960
@MethodSource("sourceAndTargets")
40-
fun converterTest(sourceTypeName: String, targetTypeName: String) {
41-
enforceNotNull = true
61+
fun converterTest(sourceTypeName: String, targetTypeName: String, enforceNotNull: Boolean) {
62+
this.enforceNotNull = enforceNotNull
63+
4264
executeTest(
4365
sourceTypeName = sourceTypeName,
4466
targetTypeName = targetTypeName,

‎converter/src/test/kotlin/io/mcarle/konvert/converter/XToValueClassConverterITest.kt‎

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,20 +27,42 @@ class XToValueClassConverterITest : ConverterITest() {
2727

2828
@JvmStatic
2929
fun sourceAndTargets(): List<Arguments> = listOf(
30-
"String",
31-
"Int",
32-
).cartesianProductWithNullableCombinations(
33-
"SimpleValueClass",
34-
"ValueClassWithNullable",
35-
"ValueClassWithAdditionalProperties"
30+
// source, target, enforceNotNull
31+
Arguments.of("String", "SimpleValueClass", false),
32+
Arguments.of("String", "SimpleValueClass?", false),
33+
Arguments.of("String?", "SimpleValueClass", true),
34+
Arguments.of("String?", "SimpleValueClass?", false),
35+
Arguments.of("Int", "SimpleValueClass", false),
36+
Arguments.of("Int", "SimpleValueClass?", false),
37+
Arguments.of("Int?", "SimpleValueClass", true),
38+
Arguments.of("Int?", "SimpleValueClass?", false),
39+
40+
Arguments.of("String", "ValueClassWithNullable", false),
41+
Arguments.of("String", "ValueClassWithNullable?", false),
42+
Arguments.of("String?", "ValueClassWithNullable", false), // enforceNotNull not needed because value class property is nullable
43+
Arguments.of("String?", "ValueClassWithNullable?", false),
44+
Arguments.of("Int", "ValueClassWithNullable", false),
45+
Arguments.of("Int", "ValueClassWithNullable?", false),
46+
Arguments.of("Int?", "ValueClassWithNullable", false), // enforceNotNull not needed because value class property is nullable
47+
Arguments.of("Int?", "ValueClassWithNullable?", false),
48+
49+
Arguments.of("String", "ValueClassWithAdditionalProperties", false),
50+
Arguments.of("String", "ValueClassWithAdditionalProperties?", false),
51+
Arguments.of("String?", "ValueClassWithAdditionalProperties", true),
52+
Arguments.of("String?", "ValueClassWithAdditionalProperties?", false),
53+
Arguments.of("Int", "ValueClassWithAdditionalProperties", false),
54+
Arguments.of("Int", "ValueClassWithAdditionalProperties?", false),
55+
Arguments.of("Int?", "ValueClassWithAdditionalProperties", true),
56+
Arguments.of("Int?", "ValueClassWithAdditionalProperties?", false),
3657
)
3758

3859
}
3960

4061
@ParameterizedTest
4162
@MethodSource("sourceAndTargets")
42-
fun converterTest(sourceTypeName: String, targetTypeName: String) {
43-
enforceNotNull = true
63+
fun converterTest(sourceTypeName: String, targetTypeName: String, enforceNotNull: Boolean) {
64+
this.enforceNotNull = enforceNotNull
65+
4466
executeTest(
4567
sourceTypeName = sourceTypeName,
4668
targetTypeName = targetTypeName,
@@ -211,10 +233,10 @@ class XToValueClassConverterITest : ConverterITest() {
211233

212234

213235
val valueClassConstructor = (
214-
verificationData.targetKClass.members
215-
.first { it.name == targetName }
216-
.returnType.classifier as KClass<*>
217-
)
236+
verificationData.targetKClass.members
237+
.first { it.name == targetName }
238+
.returnType.classifier as KClass<*>
239+
)
218240
.primaryConstructor!!
219241

220242

‎converter/src/test/kotlin/io/mcarle/konvert/converter/extensions.kt‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package io.mcarle.konvert.converter
22

33
import io.mcarle.konvert.converter.api.TypeConverter
44
import org.junit.jupiter.params.provider.Arguments
5-
import org.paukov.combinatorics3.Generator
65

76
fun <T> List<T>.toConverterTestArguments(typeNameExtractor: (T) -> Pair<String?, String?>) = this.flatMap {
87
val (sourceTypeName, targetTypeName) = typeNameExtractor(it)
@@ -13,18 +12,6 @@ fun <T> List<T>.toConverterTestArguments(typeNameExtractor: (T) -> Pair<String?,
1312
)
1413
}
1514

16-
fun <T> List<T>.cartesianProductWithNullableCombinations(vararg other: T) = Generator.cartesianProduct(this, other.toList())
17-
.flatMap {
18-
val sourceTypeName = it.first()
19-
val targetTypeName = it.last()
20-
listOf(
21-
Arguments.arguments(sourceTypeName, targetTypeName),
22-
Arguments.arguments(sourceTypeName, "$targetTypeName?"),
23-
Arguments.arguments("$sourceTypeName?", "$targetTypeName"),
24-
Arguments.arguments("$sourceTypeName?", "$targetTypeName?")
25-
)
26-
}
27-
2815
fun <E : TypeConverter> List<E>.toConverterTestArgumentsWithType(typeNameExtractor: (E) -> Pair<String?, String?>) = this.flatMap {
2916
val (sourceTypeName, targetTypeName) = typeNameExtractor(it)
3017
listOf(

0 commit comments

Comments
 (0)