Skip to content

Commit

Permalink
Refine generic type management in AbstractMessageWriterResultHandler
Browse files Browse the repository at this point in the history
This commit updates AbstractMessageWriterResultHandler#writeBody in
order to use the declared bodyParameter instead of
ResolvableType.forInstance(body) when the former has unresolvable
generics.

Closes spring-projectsgh-30215
  • Loading branch information
sdeleuze committed Mar 30, 2023
1 parent 0c80e5f commit 79c5ef8
Show file tree
Hide file tree
Showing 7 changed files with 195 additions and 10 deletions.
2 changes: 2 additions & 0 deletions spring-webflux/spring-webflux.gradle
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
description = "Spring WebFlux"

apply plugin: "kotlin"
apply plugin: "kotlinx-serialization"

dependencies {
api(project(":spring-beans"))
Expand Down Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -139,8 +139,15 @@ protected Mono<Void> writeBody(@Nullable Object body, MethodParameter bodyParame
}
else {
publisher = Mono.justOrEmpty(body);
actualElementType = body != null ? ResolvableType.forInstance(body) : bodyType;
elementType = (bodyType.toClass() == Object.class && body != null ? actualElementType : bodyType);
ResolvableType bodyInstanceType = ResolvableType.forInstance(body);
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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public void requestMappingHandlerAdapter() {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public void requestMappingHandlerAdapter() {
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);

Expand Down Expand Up @@ -207,7 +207,7 @@ public void responseEntityResultHandler() {
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);
Expand Down Expand Up @@ -235,7 +235,7 @@ public void responseBodyResultHandler() {
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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -62,6 +62,7 @@
* Unit tests for {@link AbstractMessageWriterResultHandler}.
*
* @author Rossen Stoyanchev
* @author Sebastien Deleuze
*/
public class MessageWriterResultHandlerTests {

Expand Down Expand Up @@ -180,6 +181,17 @@ public void jacksonTypeWithSubTypeOfListElement() throws Exception {
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())
Expand Down Expand Up @@ -287,6 +299,8 @@ void voidReturn() { }
Identifiable identifiable() { return null; }

List<Identifiable> listIdentifiable() { return null; }

Object object() { return null; }
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
*
* @author Rossen Stoyanchev
* @author Stephane Maldini
* @author Sebastien Deleuze
* @since 5.0
*/
class RequestMappingIntegrationTests extends AbstractRequestMappingIntegrationTests {
Expand Down Expand Up @@ -91,8 +92,8 @@ void forwardedHeaders(HttpServer httpServer) throws Exception {
void stream(HttpServer httpServer) throws Exception {
startServer(httpServer);

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


Expand Down
Original file line number Diff line number Diff line change
@@ -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)

}

0 comments on commit 79c5ef8

Please sign in to comment.