Skip to content

Commit

Permalink
Ensure annotated service with Kotlin suspend function supports nullab…
Browse files Browse the repository at this point in the history 
…le parameters (#5638)

Motivation:
A regression was identified where an annotated service with a Kotlin suspend function does not work as expected when the parameter is nullable.

Modifications:
- Updated the function call to include null parameters when invoking `kFunction.callSuspendBy()`.

Result:
- The annotated service with a Kotlin suspend function works correctly even with a nullable parameter.
  • Loading branch information
@minwoox
minwoox committed Apr 26, 2024
1 parent 262bed7 commit 5c052ac
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 31 deletions.
1 change: 0 additions & 1 deletion core/src/test/java/com/linecorp/armeria/common/logging/JsonLogFormatterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ void formatRequest() {
final DefaultRequestLog log = (DefaultRequestLog) ctx.log();
log.endRequest();
final String requestLog = logFormatter.formatRequest(log);
System.err.println(requestLog);
assertThat(requestLog)
.matches("^\\{\"type\":\"request\",\"startTime\":\".+\",\"length\":\".+\"," +
"\"duration\":\".+\",\"scheme\":\".+\",\"name\":\".+\",\"headers\":\\{\".+\"}}$");
Expand Down
44 changes: 21 additions & 23 deletions kotlin/src/main/kotlin/com/linecorp/armeria/internal/common/kotlin/ArmeriaCoroutineUtil.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,35 +96,33 @@ private fun toArgMap(
kFunction: KFunction<*>,
target: Any,
args: Array<Any?>,
): Map<KParameter, Any> {
val argMap = HashMap<KParameter, Any>(args.size + 1)
): Map<KParameter, Any?> {
val argMap = HashMap<KParameter, Any?>(args.size + 1)
var index = 0
for (parameter in kFunction.parameters) {
when (parameter.kind) {
KParameter.Kind.INSTANCE -> argMap[parameter] = target
KParameter.Kind.VALUE -> {
if (!parameter.isOptional) {
args[index]?.let { arg ->
val classifier = parameter.type.classifier
if (classifier is KClass<*> && classifier.isValue) {
val methods =
ReflectionUtils.getAllMethods(
classifier.java,
Predicate {
it.isSynthetic && Modifier.isStatic(it.modifiers) &&
it.name == "box-impl"
},
)
checkState(
methods.size == 1,
"Unable to find a single box-impl synthetic static method in %s",
classifier.java.name,
if (!parameter.isOptional || args[index] != null) {
val classifier = parameter.type.classifier
if (classifier is KClass<*> && classifier.isValue) {
val methods =
ReflectionUtils.getAllMethods(
classifier.java,
Predicate {
it.isSynthetic && Modifier.isStatic(it.modifiers) &&
it.name == "box-impl"
},
)
// Convert value class to its boxed type.
argMap[parameter] = methods.first().invoke(null, arg)
} else {
argMap[parameter] = arg
}
checkState(
methods.size == 1,
"Unable to find a single box-impl synthetic static method in %s",
classifier.java.name,
)
// Convert value class to its boxed type.
argMap[parameter] = methods.first().invoke(null, args[index])
} else {
argMap[parameter] = args[index]
}
}
index++
Expand Down
68 changes: 61 additions & 7 deletions kotlin/src/test/kotlin/com/linecorp/armeria/server/kotlin/NullableTypeSupportTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,20 @@ class NullableTypeSupportTest {
@ParameterizedTest
@ValueSource(
strings = [
"/value-resolver/of-query-param",
"/value-resolver/of-request-converter",
"/value-resolver/of-bean-constructor",
"/value-resolver/of-bean-field",
"/value-resolver/of-bean-method",
"/of-query-param",
"/of-request-converter",
"/of-bean-constructor",
"/of-bean-field",
"/of-bean-method",
],
)
fun test_nullableParameters(testPath: String) {
testNullableParameters("/nullable-type/$testPath")
testNullableParameters("/nullable-type/value-resolver/$testPath")
testNullableParameters("/nullable-type/value-resolver/suspend/$testPath")

// Check for backward-compatibility
testNullableParameters("/nullable-annot/$testPath")
testNullableParameters("/nullable-annot/value-resolver/$testPath")
testNullableParameters("/nullable-annot/value-resolver/suspend/$testPath")
}

private fun testNullableParameters(testPath: String) {
Expand Down Expand Up @@ -89,21 +91,46 @@ class NullableTypeSupportTest {
@Param b: String?,
) = HttpResponse.of("a: $a, b: $b")

@Get("/suspend/of-query-param")
suspend fun suspendOfQueryParam(
@Param a: String,
@Param b: String?,
) = HttpResponse.of("a: $a, b: $b")

@Get("/of-request-converter")
@RequestConverter(FooBarRequestConverter::class)
fun ofRequestConverter(
foo: Foo,
bar: Bar?,
) = HttpResponse.of("a: ${foo.value}, b: ${bar?.value}")

@Get("/suspend/of-request-converter")
@RequestConverter(FooBarRequestConverter::class)
suspend fun suspendOfRequestConverter(
foo: Foo,
bar: Bar?,
) = HttpResponse.of("a: ${foo.value}, b: ${bar?.value}")

@Get("/of-bean-constructor")
fun ofBeanConstructor(baz: Baz) = HttpResponse.of("a: ${baz.a}, b: ${baz.b}")

@Get("/suspend/of-bean-constructor")
suspend fun suspendOfBeanConstructor(baz: Baz) =
HttpResponse.of("a: ${baz.a}, b: ${baz.b}")

@Get("/of-bean-field")
fun ofBeanField(qux: Qux) = HttpResponse.of("a: ${qux.a}, b: ${qux.b}")

@Get("/suspend/of-bean-field")
suspend fun suspendOfBeanField(qux: Qux) =
HttpResponse.of("a: ${qux.a}, b: ${qux.b}")

@Get("/of-bean-method")
fun ofBeanMethod(quux: Quux) = HttpResponse.of("a: ${quux.a}, b: ${quux.b}")

@Get("/suspend/of-bean-method")
suspend fun suspendOfBeanMethod(quux: Quux) =
HttpResponse.of("a: ${quux.a}, b: ${quux.b}")
},
)
sb.annotatedService(
Expand All @@ -117,21 +144,48 @@ class NullableTypeSupportTest {
b: String?,
) = HttpResponse.of("a: $a, b: $b")

@Get("/suspend/of-query-param")
suspend fun suspendOfQueryParam(
@Param a: String,
@Nullable
@Param
b: String?,
) = HttpResponse.of("a: $a, b: $b")

@Get("/of-request-converter")
@RequestConverter(FooBarRequestConverter::class)
fun ofRequestConverter(
foo: Foo,
@Nullable bar: Bar?,
) = HttpResponse.of("a: ${foo.value}, b: ${bar?.value}")

@Get("/suspend/of-request-converter")
@RequestConverter(FooBarRequestConverter::class)
suspend fun suspendOfRequestConverter(
foo: Foo,
@Nullable bar: Bar?,
) = HttpResponse.of("a: ${foo.value}, b: ${bar?.value}")

@Get("/of-bean-constructor")
fun ofBeanConstructor(baz: Baz0) = HttpResponse.of("a: ${baz.a}, b: ${baz.b}")

@Get("/suspend/of-bean-constructor")
suspend fun suspendOfBeanConstructor(baz: Baz0) =
HttpResponse.of("a: ${baz.a}, b: ${baz.b}")

@Get("/of-bean-field")
fun ofBeanField(qux: Qux0) = HttpResponse.of("a: ${qux.a}, b: ${qux.b}")

@Get("/suspend/of-bean-field")
suspend fun suspendOfBeanField(qux: Qux0) =
HttpResponse.of("a: ${qux.a}, b: ${qux.b}")

@Get("/of-bean-method")
fun ofBeanMethod(quux: Quux0) = HttpResponse.of("a: ${quux.a}, b: ${quux.b}")

@Get("/suspend/of-bean-method")
suspend fun suspendOfBeanMethod(quux: Quux0) =
HttpResponse.of("a: ${quux.a}, b: ${quux.b}")
},
)
}
Expand Down

0 comments on commit 5c052ac

Please sign in to comment.