Skip to content

Commit 79c5ef8

Browse files
committed
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
1 parent 0c80e5f commit 79c5ef8

File tree

7 files changed

+195
-10
lines changed

7 files changed

+195
-10
lines changed

spring-webflux/spring-webflux.gradle

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
description = "Spring WebFlux"
22

33
apply plugin: "kotlin"
4+
apply plugin: "kotlinx-serialization"
45

56
dependencies {
67
api(project(":spring-beans"))
@@ -45,6 +46,7 @@ dependencies {
4546
testImplementation('org.apache.httpcomponents.core5:httpcore5-reactive')
4647
testImplementation("com.squareup.okhttp3:mockwebserver")
4748
testImplementation("org.jetbrains.kotlin:kotlin-script-runtime")
49+
testImplementation("org.jetbrains.kotlinx:kotlinx-serialization-json")
4850
testRuntimeOnly("org.jetbrains.kotlin:kotlin-scripting-jsr223")
4951
testRuntimeOnly("org.jruby:jruby")
5052
testRuntimeOnly("org.python:jython-standalone")

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -139,8 +139,15 @@ protected Mono<Void> writeBody(@Nullable Object body, MethodParameter bodyParame
139139
}
140140
else {
141141
publisher = Mono.justOrEmpty(body);
142-
actualElementType = body != null ? ResolvableType.forInstance(body) : bodyType;
143-
elementType = (bodyType.toClass() == Object.class && body != null ? actualElementType : bodyType);
142+
ResolvableType bodyInstanceType = ResolvableType.forInstance(body);
143+
if (bodyType.toClass() == Object.class && body != null) {
144+
actualElementType = bodyInstanceType;
145+
elementType = bodyInstanceType;
146+
}
147+
else {
148+
actualElementType = (body == null || bodyInstanceType.hasUnresolvableGenerics()) ? bodyType : bodyInstanceType;
149+
elementType = bodyType;
150+
}
144151
}
145152

146153
if (elementType.resolve() == void.class || elementType.resolve() == Void.class) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public void requestMappingHandlerAdapter() {
112112
boolean condition = initializer.getValidator() instanceof LocalValidatorFactoryBean;
113113
assertThat(condition).isTrue();
114114
assertThat(initializer.getConversionService()).isSameAs(formatterRegistry.getValue());
115-
assertThat(codecsConfigurer.getValue().getReaders().size()).isEqualTo(14);
115+
assertThat(codecsConfigurer.getValue().getReaders()).hasSize(15);
116116
}
117117

118118
@Test

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ public void requestMappingHandlerAdapter() {
152152
assertThat(adapter).isNotNull();
153153

154154
List<HttpMessageReader<?>> readers = adapter.getMessageReaders();
155-
assertThat(readers.size()).isEqualTo(14);
155+
assertThat(readers).hasSize(15);
156156

157157
ResolvableType multiValueMapType = forClassWithGenerics(MultiValueMap.class, String.class, String.class);
158158

@@ -207,7 +207,7 @@ public void responseEntityResultHandler() {
207207
assertThat(handler.getOrder()).isEqualTo(0);
208208

209209
List<HttpMessageWriter<?>> writers = handler.getMessageWriters();
210-
assertThat(writers.size()).isEqualTo(13);
210+
assertThat(writers).hasSize(14);
211211

212212
assertHasMessageWriter(writers, forClass(byte[].class), APPLICATION_OCTET_STREAM);
213213
assertHasMessageWriter(writers, forClass(ByteBuffer.class), APPLICATION_OCTET_STREAM);
@@ -235,7 +235,7 @@ public void responseBodyResultHandler() {
235235
assertThat(handler.getOrder()).isEqualTo(100);
236236

237237
List<HttpMessageWriter<?>> writers = handler.getMessageWriters();
238-
assertThat(writers.size()).isEqualTo(13);
238+
assertThat(writers).hasSize(14);
239239

240240
assertHasMessageWriter(writers, forClass(byte[].class), APPLICATION_OCTET_STREAM);
241241
assertHasMessageWriter(writers, forClass(ByteBuffer.class), APPLICATION_OCTET_STREAM);

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2020 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -62,6 +62,7 @@
6262
* Unit tests for {@link AbstractMessageWriterResultHandler}.
6363
*
6464
* @author Rossen Stoyanchev
65+
* @author Sebastien Deleuze
6566
*/
6667
public class MessageWriterResultHandlerTests {
6768

@@ -180,6 +181,17 @@ public void jacksonTypeWithSubTypeOfListElement() throws Exception {
180181
assertResponseBody("[{\"id\":123,\"name\":\"foo\"},{\"id\":456,\"name\":\"bar\"}]");
181182
}
182183

184+
@Test
185+
public void jacksonTypeWithSubTypeAndObjectReturnValue() {
186+
MethodParameter returnType = on(TestController.class).resolveReturnType(Object.class);
187+
188+
SimpleBean body = new SimpleBean(123L, "foo");
189+
this.resultHandler.writeBody(body, returnType, this.exchange).block(Duration.ofSeconds(5));
190+
191+
assertThat(this.exchange.getResponse().getHeaders().getContentType()).isEqualTo(APPLICATION_JSON);
192+
assertResponseBody("{\"id\":123,\"name\":\"foo\"}");
193+
}
194+
183195

184196
private void assertResponseBody(String responseBody) {
185197
StepVerifier.create(this.exchange.getResponse().getBody())
@@ -287,6 +299,8 @@ void voidReturn() { }
287299
Identifiable identifiable() { return null; }
288300

289301
List<Identifiable> listIdentifiable() { return null; }
302+
303+
Object object() { return null; }
290304
}
291305

292306
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
*
4848
* @author Rossen Stoyanchev
4949
* @author Stephane Maldini
50+
* @author Sebastien Deleuze
5051
* @since 5.0
5152
*/
5253
class RequestMappingIntegrationTests extends AbstractRequestMappingIntegrationTests {
@@ -91,8 +92,8 @@ void forwardedHeaders(HttpServer httpServer) throws Exception {
9192
void stream(HttpServer httpServer) throws Exception {
9293
startServer(httpServer);
9394

94-
String[] expected = {"0", "1", "2", "3", "4"};
95-
assertThat(performGet("/stream", new HttpHeaders(), String[].class).getBody()).isEqualTo(expected);
95+
Integer[] expected = {0, 1, 2, 3, 4};
96+
assertThat(performGet("/stream", new HttpHeaders(), Integer[].class).getBody()).isEqualTo(expected);
9697
}
9798

9899

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
/*
2+
* Copyright 2002-2023 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.web.reactive.result.method.annotation
18+
19+
import kotlinx.serialization.Serializable
20+
import org.assertj.core.api.Assertions
21+
import org.junit.jupiter.api.Test
22+
import org.springframework.core.ResolvableType
23+
import org.springframework.core.io.buffer.DataBuffer
24+
import org.springframework.http.MediaType
25+
import org.springframework.http.ResponseEntity
26+
import org.springframework.http.codec.EncoderHttpMessageWriter
27+
import org.springframework.http.codec.HttpMessageWriter
28+
import org.springframework.http.codec.json.Jackson2JsonEncoder
29+
import org.springframework.http.codec.json.KotlinSerializationJsonEncoder
30+
import org.springframework.util.ObjectUtils
31+
import org.springframework.web.bind.annotation.GetMapping
32+
import org.springframework.web.bind.annotation.RestController
33+
import org.springframework.web.reactive.accept.RequestedContentTypeResolverBuilder
34+
import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest
35+
import org.springframework.web.testfixture.method.ResolvableMethod
36+
import org.springframework.web.testfixture.server.MockServerWebExchange
37+
import reactor.test.StepVerifier
38+
import java.nio.charset.StandardCharsets
39+
import java.time.Duration
40+
import java.util.*
41+
42+
/**
43+
* Kotlin unit tests for {@link AbstractMessageWriterResultHandler}.
44+
*
45+
* @author Sebastien Deleuze
46+
*/
47+
class KotlinMessageWriterResultHandlerTests {
48+
49+
private val resultHandler = initResultHandler()
50+
51+
private val exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/path"))
52+
53+
54+
private fun initResultHandler(vararg writers: HttpMessageWriter<*>): AbstractMessageWriterResultHandler {
55+
val writerList = if (ObjectUtils.isEmpty(writers)) {
56+
listOf(
57+
EncoderHttpMessageWriter(KotlinSerializationJsonEncoder()),
58+
EncoderHttpMessageWriter(Jackson2JsonEncoder())
59+
)
60+
} else {
61+
listOf(*writers)
62+
}
63+
val resolver = RequestedContentTypeResolverBuilder().build()
64+
return object : AbstractMessageWriterResultHandler(writerList, resolver) {}
65+
}
66+
67+
@Test
68+
fun nonSuspendWithoutResponseEntity() {
69+
val returnType = ResolvableMethod.on(SampleController::class.java)
70+
.resolveReturnType(List::class.java, Person::class.java)
71+
val body = listOf(Person(UserId(1), "John"))
72+
resultHandler.writeBody(body, returnType, exchange).block(Duration.ofSeconds(5))
73+
74+
Assertions.assertThat(exchange.response.headers.contentType).isEqualTo(MediaType.APPLICATION_JSON)
75+
assertResponseBody("[{\"userId\":1,\"name\":\"John\"}]")
76+
}
77+
78+
@Test
79+
fun nonSuspendWithResponseEntity() {
80+
val returnType = ResolvableMethod.on(SampleController::class.java)
81+
.returning(ResolvableType.forClassWithGenerics(ResponseEntity::class.java,
82+
ResolvableType.forClassWithGenerics(List::class.java, Person::class.java)))
83+
.build().returnType()
84+
val body = ResponseEntity.ok(listOf(Person(UserId(1), "John")))
85+
resultHandler.writeBody(body.body, returnType.nested(), returnType, exchange).block(Duration.ofSeconds(5))
86+
87+
Assertions.assertThat(exchange.response.headers.contentType).isEqualTo(MediaType.APPLICATION_JSON)
88+
assertResponseBody("[{\"userId\":1,\"name\":\"John\"}]")
89+
}
90+
91+
@Test
92+
fun suspendWithoutResponseEntity() {
93+
val returnType = ResolvableMethod.on(CoroutinesSampleController::class.java)
94+
.resolveReturnType(List::class.java, Person::class.java)
95+
val body = listOf(Person(UserId(1), "John"))
96+
resultHandler.writeBody(body, returnType, exchange).block(Duration.ofSeconds(5))
97+
98+
Assertions.assertThat(exchange.response.headers.contentType).isEqualTo(MediaType.APPLICATION_JSON)
99+
assertResponseBody("[{\"userId\":1,\"name\":\"John\"}]")
100+
}
101+
102+
@Test
103+
fun suspendWithResponseEntity() {
104+
val returnType = ResolvableMethod.on(CoroutinesSampleController::class.java)
105+
.returning(ResolvableType.forClassWithGenerics(ResponseEntity::class.java,
106+
ResolvableType.forClassWithGenerics(List::class.java, Person::class.java)))
107+
.build().returnType()
108+
val body = ResponseEntity.ok(listOf(Person(UserId(1), "John")))
109+
resultHandler.writeBody(body.body, returnType.nested(), returnType, exchange).block(Duration.ofSeconds(5))
110+
111+
Assertions.assertThat(exchange.response.headers.contentType).isEqualTo(MediaType.APPLICATION_JSON)
112+
assertResponseBody("[{\"userId\":1,\"name\":\"John\"}]")
113+
}
114+
115+
private fun assertResponseBody(responseBody: String) {
116+
StepVerifier.create(exchange.response.body)
117+
.consumeNextWith { buf: DataBuffer ->
118+
Assertions.assertThat(
119+
buf.toString(StandardCharsets.UTF_8)
120+
).isEqualTo(responseBody)
121+
}
122+
.expectComplete()
123+
.verify()
124+
}
125+
126+
@RestController
127+
class SampleController {
128+
129+
@GetMapping("/non-suspend-with-response-entity")
130+
fun withResponseEntity(): ResponseEntity<List<Person>> =
131+
TODO()
132+
133+
@GetMapping("/non-suspend-without-response-entity")
134+
fun withoutResponseEntity(): List<Person> =
135+
TODO()
136+
}
137+
138+
@RestController
139+
class CoroutinesSampleController {
140+
141+
@GetMapping("/suspend-with-response-entity")
142+
suspend fun suspendAndResponseEntity(): ResponseEntity<List<Person>> =
143+
TODO()
144+
145+
@GetMapping("/suspend-without-response-entity")
146+
suspend fun suspendWithoutResponseEntity(): List<Person> =
147+
TODO()
148+
149+
}
150+
151+
@Serializable
152+
data class Person(
153+
val userId: UserId,
154+
val name: String,
155+
)
156+
157+
@JvmInline
158+
@Serializable
159+
value class UserId(val id: Int)
160+
161+
}

0 commit comments

Comments
 (0)