Add more subtle content negotiation in web layer

So that single Strings can be POSTed without JSON conversion.
There's still some work to do to support single POJOs in JSON, and
to reach parity with the WebFlux reactive type handlers, but it's
now closer to what we had before we moved the String conversion
out of the function layer.
This commit is contained in:
Dave Syer
2017-05-05 09:22:23 +01:00
parent e2c257b3e7
commit 69c22482d1
8 changed files with 124 additions and 42 deletions

View File

@@ -65,9 +65,9 @@ public class ReactorAutoConfiguration {
@ConditionalOnMissingClass("org.springframework.core.ReactiveAdapter")
protected static class FluxReturnValueConfiguration {
@Bean
public FluxReturnValueHandler fluxReturnValueHandler(
public FluxReturnValueHandler fluxReturnValueHandler(FunctionInspector inspector,
HttpMessageConverters converters) {
return new FluxReturnValueHandler(converters.getConverters());
return new FluxReturnValueHandler(inspector, converters.getConverters());
}
}

View File

@@ -16,7 +16,9 @@
package org.springframework.cloud.function.web.flux.request;
import java.nio.charset.Charset;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import javax.servlet.http.HttpServletRequest;
@@ -26,6 +28,8 @@ import com.fasterxml.jackson.databind.ObjectMapper;
import org.springframework.cloud.function.context.FunctionInspector;
import org.springframework.core.MethodParameter;
import org.springframework.core.Ordered;
import org.springframework.http.MediaType;
import org.springframework.util.StreamUtils;
import org.springframework.web.bind.support.WebDataBinderFactory;
import org.springframework.web.context.request.NativeWebRequest;
import org.springframework.web.method.support.HandlerMethodArgumentResolver;
@@ -67,13 +71,30 @@ public class FluxHandlerMethodArgumentResolver
if (type == null) {
type = Object.class;
}
List<Object> body = mapper.readValue(
webRequest.getNativeRequest(HttpServletRequest.class).getInputStream(),
mapper.getTypeFactory().constructCollectionLikeType(ArrayList.class,
type));
List<Object> body;
if (isPlainText(webRequest) && CharSequence.class.isAssignableFrom(type)) {
body = Arrays.asList(StreamUtils.copyToString(webRequest
.getNativeRequest(HttpServletRequest.class).getInputStream(),
Charset.forName("UTF-8")));
}
else {
body = mapper.readValue(
webRequest.getNativeRequest(HttpServletRequest.class)
.getInputStream(),
mapper.getTypeFactory().constructCollectionLikeType(ArrayList.class,
type));
}
return new FluxRequest<Object>(body);
}
private boolean isPlainText(NativeWebRequest webRequest) {
String value = webRequest.getHeader("Content-Type");
if (value!=null) {
return MediaType.valueOf(value).isCompatibleWith(MediaType.TEXT_PLAIN);
}
return false;
}
@Override
public boolean supportsParameter(MethodParameter parameter) {
return FluxRequest.class.isAssignableFrom(parameter.getParameterType());

View File

@@ -17,12 +17,15 @@
package org.springframework.cloud.function.web.flux.response;
import java.time.Duration;
import java.util.Arrays;
import java.util.List;
import javax.servlet.http.HttpServletResponse;
import org.reactivestreams.Publisher;
import org.springframework.cloud.function.context.FunctionInspector;
import org.springframework.cloud.function.web.flux.request.FluxHandlerMethodArgumentResolver;
import org.springframework.core.MethodParameter;
import org.springframework.core.ResolvableType;
import org.springframework.http.MediaType;
@@ -49,8 +52,12 @@ public class FluxReturnValueHandler implements AsyncHandlerMethodReturnValueHand
private long timeout = 1000L;
private static final MediaType EVENT_STREAM = MediaType.valueOf("text/event-stream");
public FluxReturnValueHandler(List<HttpMessageConverter<?>> messageConverters) {
delegate = new ResponseBodyEmitterReturnValueHandler(messageConverters);
private FunctionInspector inspector;
public FluxReturnValueHandler(FunctionInspector inspector,
List<HttpMessageConverter<?>> messageConverters) {
this.inspector = inspector;
this.delegate = new ResponseBodyEmitterReturnValueHandler(messageConverters);
}
/**
@@ -108,23 +115,52 @@ public class FluxReturnValueHandler implements AsyncHandlerMethodReturnValueHand
}
Publisher<?> flux = (Publisher<?>) adaptFrom;
Object handler = webRequest.getAttribute(
FluxHandlerMethodArgumentResolver.HANDLER,
NativeWebRequest.SCOPE_REQUEST);
Class<?> type = inspector.getOutputType(inspector.getName(handler));
MediaType mediaType = null;
if (webRequest.getHeader("Accept") != null) {
for (MediaType type : MediaType
.parseMediaTypes(webRequest.getHeader("Accept"))) {
if (!MediaType.ALL.equals(type)
&& MediaType.APPLICATION_JSON.isCompatibleWith(type)) {
mediaType = MediaType.APPLICATION_JSON;
break;
} else if (mediaType==null) {
mediaType = type;
}
}
if (isPlainText(webRequest) && CharSequence.class.isAssignableFrom(type)) {
mediaType = MediaType.TEXT_PLAIN;
} else {
mediaType = findMediaType(webRequest);
}
delegate.handleReturnValue(getEmitter(timeout, flux, mediaType), returnType,
mavContainer, webRequest);
}
private MediaType findMediaType(NativeWebRequest webRequest) {
List<MediaType> accepts = Arrays.asList(MediaType.ALL);
MediaType mediaType = null;
if (webRequest.getHeader("Accept") != null) {
accepts = MediaType.parseMediaTypes(webRequest.getHeader("Accept"));
for (MediaType accept : accepts) {
if (!MediaType.ALL.equals(accept)
&& MediaType.APPLICATION_JSON.isCompatibleWith(accept)) {
mediaType = MediaType.APPLICATION_JSON;
// Prefer JSON if that is acceptable
break;
}
else if (mediaType == null) {
mediaType = accept;
}
}
}
if (mediaType == null) {
mediaType = MediaType.APPLICATION_JSON;
}
return mediaType;
}
private boolean isPlainText(NativeWebRequest webRequest) {
String value = webRequest.getHeader("Content-Type");
if (value != null) {
return MediaType.valueOf(value).isCompatibleWith(MediaType.TEXT_PLAIN);
}
return false;
}
private ResponseBodyEmitter getEmitter(Long timeout, Publisher<?> flux,
MediaType mediaType) {
Publisher<?> exported = flux instanceof Mono ? Mono.from(flux)

View File

@@ -296,11 +296,6 @@ public class MvcRestApplicationTests {
return Mono.just(id).map(value -> "[" + value.trim().toUpperCase() + "]");
}
@PostMapping("/wrap")
public Flux<?> wrap(@RequestBody Flux<Integer> flux) {
return flux.log().map(value -> ".." + value + "..");
}
@GetMapping("/wrap/{id}")
public Mono<?> wrapGet(@PathVariable int id) {
return Mono.just(id).log().map(value -> ".." + value + "..");

View File

@@ -244,14 +244,29 @@ public class RestApplicationTests {
ResponseEntity<String> result = rest.exchange(RequestEntity
.post(new URI("/uppercase")).contentType(MediaType.APPLICATION_JSON)
.body("[\"foo\",\"bar\"]"), String.class);
assertThat(result.getBody()).isEqualTo("[\"[FOO]\",\"[BAR]\"]");
assertThat(result.getBody()).isEqualTo("[\"(FOO)\",\"(BAR)\"]");
}
@Test
public void uppercaseSingleValue() throws Exception {
ResponseEntity<String> result = rest.exchange(RequestEntity
.post(new URI("/uppercase")).contentType(MediaType.TEXT_PLAIN)
.body("foo"), String.class);
assertThat(result.getBody()).isEqualTo("(FOO)");
}
@Test
@Ignore("WebFlux would split the request body into lines: TODO make this work the same")
public void uppercasePlainText() throws Exception {
ResponseEntity<String> result = rest.exchange(RequestEntity
.post(new URI("/uppercase")).contentType(MediaType.TEXT_PLAIN)
.body("foo\nbar"), String.class);
assertThat(result.getBody()).isEqualTo("(FOO)(BAR)");
}
@Test
public void uppercaseFoos() throws Exception {
ResponseEntity<String> result = rest.exchange(RequestEntity
// TODO: does not require a content type header, but the plain MVC version
// does
.post(new URI("/upFoos")).contentType(MediaType.APPLICATION_JSON)
.body("[{\"value\":\"foo\"},{\"value\":\"bar\"}]"), String.class);
assertThat(result.getBody())
@@ -263,7 +278,7 @@ public class RestApplicationTests {
ResponseEntity<String> result = rest.exchange(RequestEntity
.post(new URI("/bareUppercase")).contentType(MediaType.APPLICATION_JSON)
.body("[\"foo\",\"bar\"]"), String.class);
assertThat(result.getBody()).isEqualTo("[\"[FOO]\",\"[BAR]\"]");
assertThat(result.getBody()).isEqualTo("[\"(FOO)\",\"(BAR)\"]");
}
@Test
@@ -271,7 +286,7 @@ public class RestApplicationTests {
ResponseEntity<String> result = rest.exchange(RequestEntity
.post(new URI("/transform")).contentType(MediaType.APPLICATION_JSON)
.body("[\"foo\",\"bar\"]"), String.class);
assertThat(result.getBody()).isEqualTo("[\"[FOO]\",\"[BAR]\"]");
assertThat(result.getBody()).isEqualTo("[\"(FOO)\",\"(BAR)\"]");
}
@Test
@@ -279,17 +294,17 @@ public class RestApplicationTests {
ResponseEntity<String> result = rest.exchange(RequestEntity
.post(new URI("/post/more")).contentType(MediaType.APPLICATION_JSON)
.body("[\"foo\",\"bar\"]"), String.class);
assertThat(result.getBody()).isEqualTo("[\"[FOO]\",\"[BAR]\"]");
assertThat(result.getBody()).isEqualTo("[\"(FOO)\",\"(BAR)\"]");
}
@Test
public void postMoreFoo() {
assertThat(rest.getForObject("/post/more/foo", String.class)).isEqualTo("[FOO]");
assertThat(rest.getForObject("/post/more/foo", String.class)).isEqualTo("(FOO)");
}
@Test
public void uppercaseGet() {
assertThat(rest.getForObject("/uppercase/foo", String.class)).isEqualTo("[FOO]");
assertThat(rest.getForObject("/uppercase/foo", String.class)).isEqualTo("(FOO)");
}
@Test
@@ -327,7 +342,7 @@ public class RestApplicationTests {
assertThat(rest.exchange(RequestEntity.post(new URI("/uppercase"))
.accept(EVENT_STREAM).contentType(MediaType.APPLICATION_JSON)
.body("[\"foo\",\"bar\"]"), String.class).getBody())
.isEqualTo(sse("[FOO]", "[BAR]"));
.isEqualTo(sse("(FOO)", "(BAR)"));
}
private String sse(String... values) {
@@ -343,12 +358,12 @@ public class RestApplicationTests {
@Bean({ "uppercase", "transform", "post/more" })
public Function<Flux<String>, Flux<String>> uppercase() {
return flux -> flux.log()
.map(value -> "[" + value.trim().toUpperCase() + "]");
.map(value -> "(" + value.trim().toUpperCase() + ")");
}
@Bean
public Function<String, String> bareUppercase() {
return value -> "[" + value.trim().toUpperCase() + "]";
return value -> "(" + value.trim().toUpperCase() + ")";
}
@Bean