Refine generic type management in AbstractMessageWriterResultHandler

This commit updates AbstractMessageWriterResultHandler#writeBody in
order to use the declared bodyParameter instead of
ResolvableType.forInstance(body) when the former has unresolvable
generics.

Closes gh-30215

spring-webflux/spring-webflux.gradle

@@ -1,6 +1,7 @@
 description = "Spring WebFlux"
 
 apply plugin: "kotlin"
+apply plugin: "kotlinx-serialization"
 
 dependencies {
 	api(project(":spring-beans"))
@@ -45,6 +46,7 @@ dependencies {
 	testImplementation('org.apache.httpcomponents.core5:httpcore5-reactive')
 	testImplementation("com.squareup.okhttp3:mockwebserver")
 	testImplementation("org.jetbrains.kotlin:kotlin-script-runtime")
+	testImplementation("org.jetbrains.kotlinx:kotlinx-serialization-json")
 	testRuntimeOnly("org.jetbrains.kotlin:kotlin-scripting-jsr223")
 	testRuntimeOnly("org.jruby:jruby")
 	testRuntimeOnly("org.python:jython-standalone")

spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageWriterResultHandler.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2021 the original author or authors.
+ * Copyright 2002-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -139,8 +139,15 @@ public abstract class AbstractMessageWriterResultHandler extends HandlerResultHa
 		}
 		else {
 			publisher = Mono.justOrEmpty(body);
-			actualElementType = body != null ? ResolvableType.forInstance(body) : bodyType;
+			ResolvableType bodyInstanceType = ResolvableType.forInstance(body);
-			elementType = (bodyType.toClass() == Object.class && body != null ? actualElementType : bodyType);
+			if (bodyType.toClass() == Object.class && body != null) {
+				actualElementType = bodyInstanceType;
+				elementType = bodyInstanceType;
+			}
+			else {
+				actualElementType = (body == null || bodyInstanceType.hasUnresolvableGenerics()) ? bodyType : bodyInstanceType;
+				elementType = bodyType;
+			}
 		}
 
 		if (elementType.resolve() == void.class || elementType.resolve() == Void.class) {

spring-webflux/src/test/java/org/springframework/web/reactive/config/DelegatingWebFluxConfigurationTests.java

@@ -112,7 +112,7 @@ public class DelegatingWebFluxConfigurationTests {
 		boolean condition = initializer.getValidator() instanceof LocalValidatorFactoryBean;
 		assertThat(condition).isTrue();
 		assertThat(initializer.getConversionService()).isSameAs(formatterRegistry.getValue());
-		assertThat(codecsConfigurer.getValue().getReaders().size()).isEqualTo(14);
+		assertThat(codecsConfigurer.getValue().getReaders()).hasSize(15);
 	}
 
 	@Test

spring-webflux/src/test/java/org/springframework/web/reactive/config/WebFluxConfigurationSupportTests.java

@@ -152,7 +152,7 @@ public class WebFluxConfigurationSupportTests {
 		assertThat(adapter).isNotNull();
 
 		List<HttpMessageReader<?>> readers = adapter.getMessageReaders();
-		assertThat(readers.size()).isEqualTo(14);
+		assertThat(readers).hasSize(15);
 
 		ResolvableType multiValueMapType = forClassWithGenerics(MultiValueMap.class, String.class, String.class);
 
@@ -207,7 +207,7 @@ public class WebFluxConfigurationSupportTests {
 		assertThat(handler.getOrder()).isEqualTo(0);
 
 		List<HttpMessageWriter<?>> writers = handler.getMessageWriters();
-		assertThat(writers.size()).isEqualTo(13);
+		assertThat(writers).hasSize(14);
 
 		assertHasMessageWriter(writers, forClass(byte[].class), APPLICATION_OCTET_STREAM);
 		assertHasMessageWriter(writers, forClass(ByteBuffer.class), APPLICATION_OCTET_STREAM);
@@ -235,7 +235,7 @@ public class WebFluxConfigurationSupportTests {
 		assertThat(handler.getOrder()).isEqualTo(100);
 
 		List<HttpMessageWriter<?>> writers = handler.getMessageWriters();
-		assertThat(writers.size()).isEqualTo(13);
+		assertThat(writers).hasSize(14);
 
 		assertHasMessageWriter(writers, forClass(byte[].class), APPLICATION_OCTET_STREAM);
 		assertHasMessageWriter(writers, forClass(ByteBuffer.class), APPLICATION_OCTET_STREAM);

spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/MessageWriterResultHandlerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -62,6 +62,7 @@ import static org.springframework.web.testfixture.method.ResolvableMethod.on;
  * Unit tests for {@link AbstractMessageWriterResultHandler}.
  *
  * @author Rossen Stoyanchev
+ * @author Sebastien Deleuze
  */
 public class MessageWriterResultHandlerTests {
 
@@ -180,6 +181,17 @@ public class MessageWriterResultHandlerTests {
 		assertResponseBody("[{\"id\":123,\"name\":\"foo\"},{\"id\":456,\"name\":\"bar\"}]");
 	}
 
+	@Test
+	public void jacksonTypeWithSubTypeAndObjectReturnValue() {
+		MethodParameter returnType = on(TestController.class).resolveReturnType(Object.class);
+
+		SimpleBean body = new SimpleBean(123L, "foo");
+		this.resultHandler.writeBody(body, returnType, this.exchange).block(Duration.ofSeconds(5));
+
+		assertThat(this.exchange.getResponse().getHeaders().getContentType()).isEqualTo(APPLICATION_JSON);
+		assertResponseBody("{\"id\":123,\"name\":\"foo\"}");
+	}
+
 
 	private void assertResponseBody(String responseBody) {
 		StepVerifier.create(this.exchange.getResponse().getBody())
@@ -287,6 +299,8 @@ public class MessageWriterResultHandlerTests {
 		Identifiable identifiable() { return null; }
 
 		List<Identifiable> listIdentifiable() { return null; }
+
+		Object object() { return null; }
 	}
 
 }

spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingIntegrationTests.java

@@ -47,6 +47,7 @@ import static org.assertj.core.api.Assertions.assertThat;
  *
  * @author Rossen Stoyanchev
  * @author Stephane Maldini
+ * @author Sebastien Deleuze
  * @since 5.0
  */
 class RequestMappingIntegrationTests extends AbstractRequestMappingIntegrationTests {
@@ -91,8 +92,8 @@ class RequestMappingIntegrationTests extends AbstractRequestMappingIntegrationTe
 	void stream(HttpServer httpServer) throws Exception {
 		startServer(httpServer);
 
-		String[] expected = {"0", "1", "2", "3", "4"};
+		Integer[] expected = {0, 1, 2, 3, 4};
-		assertThat(performGet("/stream", new HttpHeaders(), String[].class).getBody()).isEqualTo(expected);
+		assertThat(performGet("/stream", new HttpHeaders(), Integer[].class).getBody()).isEqualTo(expected);
 	}
 
 

spring-webflux/src/test/kotlin/org/springframework/web/reactive/result/method/annotation/KotlinMessageWriterResultHandlerTests.kt

@@ -0,0 +1,161 @@
+/*
+ * Copyright 2002-2023 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.web.reactive.result.method.annotation
+
+import kotlinx.serialization.Serializable
+import org.assertj.core.api.Assertions
+import org.junit.jupiter.api.Test
+import org.springframework.core.ResolvableType
+import org.springframework.core.io.buffer.DataBuffer
+import org.springframework.http.MediaType
+import org.springframework.http.ResponseEntity
+import org.springframework.http.codec.EncoderHttpMessageWriter
+import org.springframework.http.codec.HttpMessageWriter
+import org.springframework.http.codec.json.Jackson2JsonEncoder
+import org.springframework.http.codec.json.KotlinSerializationJsonEncoder
+import org.springframework.util.ObjectUtils
+import org.springframework.web.bind.annotation.GetMapping
+import org.springframework.web.bind.annotation.RestController
+import org.springframework.web.reactive.accept.RequestedContentTypeResolverBuilder
+import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest
+import org.springframework.web.testfixture.method.ResolvableMethod
+import org.springframework.web.testfixture.server.MockServerWebExchange
+import reactor.test.StepVerifier
+import java.nio.charset.StandardCharsets
+import java.time.Duration
+import java.util.*
+
+/**
+ * Kotlin unit tests for {@link AbstractMessageWriterResultHandler}.
+ *
+ * @author Sebastien Deleuze
+ */
+class KotlinMessageWriterResultHandlerTests {
+
+	private val resultHandler = initResultHandler()
+
+	private val exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/path"))
+
+
+	private fun initResultHandler(vararg writers: HttpMessageWriter<*>): AbstractMessageWriterResultHandler {
+		val writerList = if (ObjectUtils.isEmpty(writers)) {
+			listOf(
+				EncoderHttpMessageWriter(KotlinSerializationJsonEncoder()),
+				EncoderHttpMessageWriter(Jackson2JsonEncoder())
+			)
+		} else {
+			listOf(*writers)
+		}
+		val resolver = RequestedContentTypeResolverBuilder().build()
+		return object : AbstractMessageWriterResultHandler(writerList, resolver) {}
+	}
+
+	@Test
+	fun nonSuspendWithoutResponseEntity() {
+		val returnType = ResolvableMethod.on(SampleController::class.java)
+			.resolveReturnType(List::class.java, Person::class.java)
+		val body = listOf(Person(UserId(1), "John"))
+		resultHandler.writeBody(body, returnType, exchange).block(Duration.ofSeconds(5))
+
+		Assertions.assertThat(exchange.response.headers.contentType).isEqualTo(MediaType.APPLICATION_JSON)
+		assertResponseBody("[{\"userId\":1,\"name\":\"John\"}]")
+	}
+
+	@Test
+	fun nonSuspendWithResponseEntity() {
+		val returnType = ResolvableMethod.on(SampleController::class.java)
+			.returning(ResolvableType.forClassWithGenerics(ResponseEntity::class.java,
+				ResolvableType.forClassWithGenerics(List::class.java, Person::class.java)))
+			.build().returnType()
+		val body = ResponseEntity.ok(listOf(Person(UserId(1), "John")))
+		resultHandler.writeBody(body.body, returnType.nested(), returnType, exchange).block(Duration.ofSeconds(5))
+
+		Assertions.assertThat(exchange.response.headers.contentType).isEqualTo(MediaType.APPLICATION_JSON)
+		assertResponseBody("[{\"userId\":1,\"name\":\"John\"}]")
+	}
+
+	@Test
+	fun suspendWithoutResponseEntity() {
+		val returnType = ResolvableMethod.on(CoroutinesSampleController::class.java)
+			.resolveReturnType(List::class.java, Person::class.java)
+		val body = listOf(Person(UserId(1), "John"))
+		resultHandler.writeBody(body, returnType, exchange).block(Duration.ofSeconds(5))
+
+		Assertions.assertThat(exchange.response.headers.contentType).isEqualTo(MediaType.APPLICATION_JSON)
+		assertResponseBody("[{\"userId\":1,\"name\":\"John\"}]")
+	}
+
+	@Test
+	fun suspendWithResponseEntity() {
+		val returnType = ResolvableMethod.on(CoroutinesSampleController::class.java)
+			.returning(ResolvableType.forClassWithGenerics(ResponseEntity::class.java,
+				ResolvableType.forClassWithGenerics(List::class.java, Person::class.java)))
+			.build().returnType()
+		val body = ResponseEntity.ok(listOf(Person(UserId(1), "John")))
+		resultHandler.writeBody(body.body, returnType.nested(), returnType, exchange).block(Duration.ofSeconds(5))
+
+		Assertions.assertThat(exchange.response.headers.contentType).isEqualTo(MediaType.APPLICATION_JSON)
+		assertResponseBody("[{\"userId\":1,\"name\":\"John\"}]")
+	}
+
+	private fun assertResponseBody(responseBody: String) {
+		StepVerifier.create(exchange.response.body)
+			.consumeNextWith { buf: DataBuffer ->
+				Assertions.assertThat(
+					buf.toString(StandardCharsets.UTF_8)
+				).isEqualTo(responseBody)
+			}
+			.expectComplete()
+			.verify()
+	}
+
+	@RestController
+	class SampleController {
+
+		@GetMapping("/non-suspend-with-response-entity")
+		fun withResponseEntity(): ResponseEntity<List<Person>> =
+			TODO()
+
+		@GetMapping("/non-suspend-without-response-entity")
+		fun withoutResponseEntity(): List<Person> =
+			TODO()
+	}
+
+	@RestController
+	class CoroutinesSampleController {
+
+		@GetMapping("/suspend-with-response-entity")
+		suspend fun suspendAndResponseEntity(): ResponseEntity<List<Person>> =
+			TODO()
+
+		@GetMapping("/suspend-without-response-entity")
+		suspend fun suspendWithoutResponseEntity(): List<Person> =
+			TODO()
+
+	}
+
+	@Serializable
+	data class Person(
+		val userId: UserId,
+		val name: String,
+	)
+
+	@JvmInline
+	@Serializable
+	value class UserId(val id: Int)
+
+}