String decoding for text only vs any MIME type

Follow-up to:
3d68c496f1

StringDecoder can be created in text-only vs "*/*" mode which in turn
allows a more intuitive order of client side decoders, e.g. SSE does
not have to be ahead of StringDecoder.

The commit also explicitly disables String from the supported types in
Jackson2Decoder leaving it to the StringDecoder in "*/*" mode which
comes after. This does not change the current arrangement since the
the StringDecoder ahead having "*/*" picks up JSON content just the
same.

From a broader perspective this change allows any decoder to deal with
String if it wants to after examining the content type be it the SSE
or another, custom decoder. For Jackson there is very little value in
decoding to String which works only if the output contains a single
JSON string but will fail to parse anything else (JSON object/array)
while StringDecoder in "*/*" mode will not fail.

Issue: SPR-15374
This commit is contained in:
Rossen Stoyanchev 2017-03-23 16:23:34 -04:00
parent a287e67992
commit 0662dbf044
17 changed files with 75 additions and 49 deletions

View File

@ -58,21 +58,13 @@ public class StringDecoder extends AbstractDecoder<String> {
private final boolean splitOnNewline;
/**
* Create a {@code StringDecoder} that decodes a bytes stream to a String stream
* <p>By default, this decoder will split along new lines.
*/
public StringDecoder() {
this(true);
}
/**
* Create a {@code StringDecoder} that decodes a bytes stream to a String stream
* @param splitOnNewline whether this decoder should split the received data buffers
* along newline characters
*/
public StringDecoder(boolean splitOnNewline) {
super(new MimeType("text", "*", DEFAULT_CHARSET), MimeTypeUtils.ALL);
private StringDecoder(boolean splitOnNewline, MimeType... mimeTypes) {
super(mimeTypes);
this.splitOnNewline = splitOnNewline;
}
@ -136,4 +128,22 @@ public class StringDecoder extends AbstractDecoder<String> {
}
}
/**
* Create a {@code StringDecoder} for {@code "text/plain"}.
* @param splitOnNewline whether to split the byte stream into lines
*/
public static StringDecoder textPlainOnly(boolean splitOnNewline) {
return new StringDecoder(splitOnNewline, new MimeType("text", "plain", DEFAULT_CHARSET));
}
/**
* Create a {@code StringDecoder} that supports all MIME types.
* @param splitOnNewline whether to split the byte stream into lines
*/
public static StringDecoder allMimeTypes(boolean splitOnNewline) {
return new StringDecoder(splitOnNewline,
new MimeType("text", "plain", DEFAULT_CHARSET), MimeTypeUtils.ALL);
}
}

View File

@ -38,7 +38,7 @@ import static org.junit.Assert.assertTrue;
*/
public class StringDecoderTests extends AbstractDataBufferAllocatingTestCase {
private StringDecoder decoder = new StringDecoder();
private StringDecoder decoder = StringDecoder.allMimeTypes(true);
@Test
@ -57,7 +57,7 @@ public class StringDecoderTests extends AbstractDataBufferAllocatingTestCase {
@Test
public void decode() throws InterruptedException {
this.decoder = new StringDecoder(false);
this.decoder = StringDecoder.allMimeTypes(false);
Flux<DataBuffer> source = Flux.just(stringBuffer("foo"), stringBuffer("bar"), stringBuffer("baz"));
Flux<String> output = this.decoder.decode(source, ResolvableType.forClass(String.class),
null, Collections.emptyMap());
@ -111,7 +111,7 @@ public class StringDecoderTests extends AbstractDataBufferAllocatingTestCase {
@Test
public void decodeToMono() throws InterruptedException {
this.decoder = new StringDecoder(false);
this.decoder = StringDecoder.allMimeTypes(false);
Flux<DataBuffer> source = Flux.just(stringBuffer("foo"), stringBuffer("bar"), stringBuffer("baz"));
Mono<String> output = this.decoder.decodeToMono(source,
ResolvableType.forClass(String.class), null, Collections.emptyMap());

View File

@ -55,7 +55,7 @@ public class ServerSentEventHttpMessageReader implements HttpMessageReader<Objec
private static final DataBufferFactory bufferFactory = new DefaultDataBufferFactory();
private static final StringDecoder stringDecoder = new StringDecoder(false);
private static final StringDecoder stringDecoder = StringDecoder.textPlainOnly(false);
private final Decoder<?> decoder;
@ -190,6 +190,9 @@ public class ServerSentEventHttpMessageReader implements HttpMessageReader<Objec
public Mono<Object> readMono(ResolvableType elementType, ReactiveHttpInputMessage message,
Map<String, Object> hints) {
// We're ahead of String + "*/*"
// Let's see if we can aggregate the output (lest we time out)...
if (String.class.equals(elementType.getRawClass())) {
Flux<DataBuffer> body = message.getBody();
return stringDecoder.decodeToMono(body, elementType, null, null).cast(Object.class);

View File

@ -74,6 +74,11 @@ public abstract class Jackson2CodecSupport {
}
protected boolean supportsMimeType(MimeType mimeType) {
return mimeType == null ||
JSON_MIME_TYPES.stream().anyMatch(m -> m.isCompatibleWith(mimeType));
}
/**
* Return the Jackson {@link JavaType} for the specified type and context class.
* <p>The default implementation returns {@code typeFactory.constructType(type, contextClass)},

View File

@ -67,10 +67,12 @@ public class Jackson2JsonDecoder extends Jackson2CodecSupport implements HttpDec
@Override
public boolean canDecode(ResolvableType elementType, MimeType mimeType) {
JavaType javaType = this.mapper.getTypeFactory().constructType(elementType.getType());
return this.mapper.canDeserialize(javaType) &&
(mimeType == null || JSON_MIME_TYPES.stream().anyMatch(m -> m.isCompatibleWith(mimeType)));
// Skip String (CharSequenceDecoder + "*/*" comes after)
return !CharSequence.class.isAssignableFrom(elementType.resolve(Object.class)) &&
this.mapper.canDeserialize(javaType) && supportsMimeType(mimeType);
}
@Override
public List<MimeType> getDecodableMimeTypes() {
return JSON_MIME_TYPES;

View File

@ -104,8 +104,8 @@ public class Jackson2JsonEncoder extends Jackson2CodecSupport implements HttpEnc
@Override
public boolean canEncode(ResolvableType elementType, MimeType mimeType) {
return this.mapper.canSerialize(elementType.getRawClass()) &&
(mimeType == null || JSON_MIME_TYPES.stream().anyMatch(m -> m.isCompatibleWith(mimeType)));
Class<?> clazz = elementType.getRawClass();
return this.mapper.canSerialize(clazz) && supportsMimeType(mimeType);
}
@Override

View File

@ -22,6 +22,8 @@ import java.util.Map;
import static java.util.Arrays.asList;
import static java.util.Collections.*;
import org.junit.Test;
import static org.springframework.core.ResolvableType.forClass;
import static org.springframework.http.MediaType.*;
import static org.springframework.http.codec.json.Jackson2JsonDecoder.*;
import static org.springframework.http.codec.json.JacksonViewBean.*;
@ -50,16 +52,18 @@ public class Jackson2JsonDecoderTests extends AbstractDataBufferAllocatingTestCa
@Test
public void canDecode() {
Jackson2JsonDecoder decoder = new Jackson2JsonDecoder();
ResolvableType type = ResolvableType.forClass(Pojo.class);
assertTrue(decoder.canDecode(type, APPLICATION_JSON));
assertTrue(decoder.canDecode(type, null));
assertFalse(decoder.canDecode(type, APPLICATION_XML));
assertTrue(decoder.canDecode(forClass(Pojo.class), APPLICATION_JSON));
assertTrue(decoder.canDecode(forClass(Pojo.class), null));
assertFalse(decoder.canDecode(forClass(String.class), null));
assertFalse(decoder.canDecode(forClass(Pojo.class), APPLICATION_XML));
}
@Test
public void decodePojo() throws Exception {
Flux<DataBuffer> source = Flux.just(stringBuffer("{\"foo\": \"foofoo\", \"bar\": \"barbar\"}"));
ResolvableType elementType = ResolvableType.forClass(Pojo.class);
ResolvableType elementType = forClass(Pojo.class);
Flux<Object> flux = new Jackson2JsonDecoder().decode(source, elementType, null,
emptyMap());
@ -71,7 +75,7 @@ public class Jackson2JsonDecoderTests extends AbstractDataBufferAllocatingTestCa
@Test
public void decodePojoWithError() throws Exception {
Flux<DataBuffer> source = Flux.just(stringBuffer("{\"foo\":}"));
ResolvableType elementType = ResolvableType.forClass(Pojo.class);
ResolvableType elementType = forClass(Pojo.class);
Flux<Object> flux = new Jackson2JsonDecoder().decode(source, elementType, null,
emptyMap());
@ -98,7 +102,7 @@ public class Jackson2JsonDecoderTests extends AbstractDataBufferAllocatingTestCa
Flux<DataBuffer> source = Flux.just(stringBuffer(
"[{\"bar\":\"b1\",\"foo\":\"f1\"},{\"bar\":\"b2\",\"foo\":\"f2\"}]"));
ResolvableType elementType = ResolvableType.forClass(Pojo.class);
ResolvableType elementType = forClass(Pojo.class);
Flux<Object> flux = new Jackson2JsonDecoder().decode(source, elementType, null,
emptyMap());
@ -112,7 +116,7 @@ public class Jackson2JsonDecoderTests extends AbstractDataBufferAllocatingTestCa
public void fieldLevelJsonView() throws Exception {
Flux<DataBuffer> source = Flux.just(
stringBuffer("{\"withView1\" : \"with\", \"withView2\" : \"with\", \"withoutView\" : \"without\"}"));
ResolvableType elementType = ResolvableType.forClass(JacksonViewBean.class);
ResolvableType elementType = forClass(JacksonViewBean.class);
Map<String, Object> hints = singletonMap(JSON_VIEW_HINT, MyJacksonView1.class);
Flux<JacksonViewBean> flux = new Jackson2JsonDecoder()
.decode(source, elementType, null, hints).cast(JacksonViewBean.class);
@ -130,7 +134,7 @@ public class Jackson2JsonDecoderTests extends AbstractDataBufferAllocatingTestCa
public void classLevelJsonView() throws Exception {
Flux<DataBuffer> source = Flux.just(stringBuffer(
"{\"withView1\" : \"with\", \"withView2\" : \"with\", \"withoutView\" : \"without\"}"));
ResolvableType elementType = ResolvableType.forClass(JacksonViewBean.class);
ResolvableType elementType = forClass(JacksonViewBean.class);
Map<String, Object> hints = singletonMap(JSON_VIEW_HINT, MyJacksonView3.class);
Flux<JacksonViewBean> flux = new Jackson2JsonDecoder()
.decode(source, elementType, null, hints).cast(JacksonViewBean.class);
@ -147,7 +151,7 @@ public class Jackson2JsonDecoderTests extends AbstractDataBufferAllocatingTestCa
@Test
public void decodeEmptyBodyToMono() throws Exception {
Flux<DataBuffer> source = Flux.empty();
ResolvableType elementType = ResolvableType.forClass(Pojo.class);
ResolvableType elementType = forClass(Pojo.class);
Mono<Object> mono = new Jackson2JsonDecoder().decodeToMono(source, elementType,
null, emptyMap());

View File

@ -331,7 +331,7 @@ public class WebFluxConfigurationSupport implements ApplicationContextAware {
readers.add(new DecoderHttpMessageReader<>(new ByteArrayDecoder()));
readers.add(new DecoderHttpMessageReader<>(new ByteBufferDecoder()));
readers.add(new DecoderHttpMessageReader<>(new DataBufferDecoder()));
readers.add(new DecoderHttpMessageReader<>(new StringDecoder()));
readers.add(new DecoderHttpMessageReader<>(StringDecoder.allMimeTypes(true)));
readers.add(new DecoderHttpMessageReader<>(new ResourceDecoder()));
if (jaxb2Present) {
readers.add(new DecoderHttpMessageReader<>(new Jaxb2XmlDecoder()));

View File

@ -75,17 +75,17 @@ class DefaultExchangeStrategiesBuilder implements ExchangeStrategies.Builder {
}
private void defaultReaders() {
// SSE first (constrained to "text/event-stream")
messageReader(new ServerSentEventHttpMessageReader(getSseDecoder()));
messageReader(new DecoderHttpMessageReader<>(new ByteArrayDecoder()));
messageReader(new DecoderHttpMessageReader<>(new ByteBufferDecoder()));
messageReader(new DecoderHttpMessageReader<>(new StringDecoder(false)));
messageReader(new DecoderHttpMessageReader<>(StringDecoder.textPlainOnly(false)));
if (jaxb2Present) {
messageReader(new DecoderHttpMessageReader<>(new Jaxb2XmlDecoder()));
}
if (jackson2Present) {
messageReader(new DecoderHttpMessageReader<>(new Jackson2JsonDecoder()));
}
messageReader(new ServerSentEventHttpMessageReader(getSseDecoder()));
messageReader(new DecoderHttpMessageReader<>(StringDecoder.allMimeTypes(false)));
}
private Decoder<?> getSseDecoder() {

View File

@ -84,7 +84,7 @@ class DefaultHandlerStrategiesBuilder implements HandlerStrategies.Builder {
public void defaultConfiguration() {
messageReader(new DecoderHttpMessageReader<>(new ByteArrayDecoder()));
messageReader(new DecoderHttpMessageReader<>(new ByteBufferDecoder()));
messageReader(new DecoderHttpMessageReader<>(new StringDecoder()));
messageReader(new DecoderHttpMessageReader<>(StringDecoder.allMimeTypes(true)));
messageReader(new FormHttpMessageReader());
messageWriter(new EncoderHttpMessageWriter<>(new ByteArrayEncoder()));

View File

@ -114,10 +114,11 @@ public class RequestMappingHandlerAdapter implements HandlerAdapter, Application
public RequestMappingHandlerAdapter() {
// TODO: improve with better (shared) defaults
this.messageReaders.add(new DecoderHttpMessageReader<>(new ByteArrayDecoder()));
this.messageReaders.add(new DecoderHttpMessageReader<>(new ByteBufferDecoder()));
this.messageReaders.add(new DecoderHttpMessageReader<>(new DataBufferDecoder()));
this.messageReaders.add(new DecoderHttpMessageReader<>(new StringDecoder()));
this.messageReaders.add(new DecoderHttpMessageReader<>(StringDecoder.allMimeTypes(true)));
}

View File

@ -298,7 +298,7 @@ public class WebFluxConfigurationSupportTests {
@Override
protected void configureMessageReaders(List<ServerHttpMessageReader<?>> messageReaders) {
messageReaders.add(new DecoderHttpMessageReader<>(new StringDecoder()));
messageReaders.add(new DecoderHttpMessageReader<>(StringDecoder.textPlainOnly(true)));
}
@Override

View File

@ -67,7 +67,7 @@ public class BodyExtractorsTests {
public void createContext() {
final List<HttpMessageReader<?>> messageReaders = new ArrayList<>();
messageReaders.add(new DecoderHttpMessageReader<>(new ByteBufferDecoder()));
messageReaders.add(new DecoderHttpMessageReader<>(new StringDecoder()));
messageReaders.add(new DecoderHttpMessageReader<>(StringDecoder.allMimeTypes(true)));
messageReaders.add(new DecoderHttpMessageReader<>(new Jaxb2XmlDecoder()));
messageReaders.add(new DecoderHttpMessageReader<>(new Jackson2JsonDecoder()));
messageReaders.add(new FormHttpMessageReader());

View File

@ -123,7 +123,7 @@ public class DefaultClientResponseTests {
when(mockResponse.getBody()).thenReturn(body);
Set<HttpMessageReader<?>> messageReaders = Collections
.singleton(new DecoderHttpMessageReader<String>(new StringDecoder()));
.singleton(new DecoderHttpMessageReader<>(StringDecoder.allMimeTypes(true)));
when(mockExchangeStrategies.messageReaders()).thenReturn(messageReaders::stream);
Mono<String> resultMono = defaultClientResponse.body(toMono(String.class));
@ -144,7 +144,7 @@ public class DefaultClientResponseTests {
when(mockResponse.getBody()).thenReturn(body);
Set<HttpMessageReader<?>> messageReaders = Collections
.singleton(new DecoderHttpMessageReader<String>(new StringDecoder()));
.singleton(new DecoderHttpMessageReader<>(StringDecoder.allMimeTypes(true)));
when(mockExchangeStrategies.messageReaders()).thenReturn(messageReaders::stream);
Mono<String> resultMono = defaultClientResponse.bodyToMono(String.class);
@ -159,7 +159,7 @@ public class DefaultClientResponseTests {
when(mockResponse.getStatusCode()).thenReturn(HttpStatus.NOT_FOUND);
Set<HttpMessageReader<?>> messageReaders = Collections
.singleton(new DecoderHttpMessageReader<String>(new StringDecoder()));
.singleton(new DecoderHttpMessageReader<>(StringDecoder.allMimeTypes(true)));
when(mockExchangeStrategies.messageReaders()).thenReturn(messageReaders::stream);
Mono<String> resultMono = defaultClientResponse.bodyToMono(String.class);
@ -183,7 +183,7 @@ public class DefaultClientResponseTests {
when(mockResponse.getBody()).thenReturn(body);
Set<HttpMessageReader<?>> messageReaders = Collections
.singleton(new DecoderHttpMessageReader<String>(new StringDecoder()));
.singleton(new DecoderHttpMessageReader<>(StringDecoder.allMimeTypes(true)));
when(mockExchangeStrategies.messageReaders()).thenReturn(messageReaders::stream);
Flux<String> resultFlux = defaultClientResponse.bodyToFlux(String.class);
@ -199,7 +199,7 @@ public class DefaultClientResponseTests {
when(mockResponse.getStatusCode()).thenReturn(HttpStatus.INTERNAL_SERVER_ERROR);
Set<HttpMessageReader<?>> messageReaders = Collections
.singleton(new DecoderHttpMessageReader<String>(new StringDecoder()));
.singleton(new DecoderHttpMessageReader<>(StringDecoder.allMimeTypes(true)));
when(mockExchangeStrategies.messageReaders()).thenReturn(messageReaders::stream);
Flux<String> resultFlux = defaultClientResponse.bodyToFlux(String.class);

View File

@ -52,9 +52,10 @@ import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.UnsupportedMediaTypeStatusException;
import org.springframework.web.server.WebSession;
import static org.junit.Assert.*;
import static org.mockito.Mockito.*;
import static org.springframework.web.reactive.function.BodyExtractors.*;
import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.springframework.web.reactive.function.BodyExtractors.toMono;
/**
* @author Arjen Poutsma
@ -190,7 +191,7 @@ public class DefaultServerRequestTests {
when(mockRequest.getBody()).thenReturn(body);
Set<HttpMessageReader<?>> messageReaders = Collections
.singleton(new DecoderHttpMessageReader<String>(new StringDecoder()));
.singleton(new DecoderHttpMessageReader<>(StringDecoder.allMimeTypes(true)));
when(mockHandlerStrategies.messageReaders()).thenReturn(messageReaders::stream);
Mono<String> resultMono = defaultRequest.body(toMono(String.class));
@ -210,7 +211,7 @@ public class DefaultServerRequestTests {
when(mockRequest.getBody()).thenReturn(body);
Set<HttpMessageReader<?>> messageReaders = Collections
.singleton(new DecoderHttpMessageReader<String>(new StringDecoder()));
.singleton(new DecoderHttpMessageReader<>(StringDecoder.allMimeTypes(true)));
when(mockHandlerStrategies.messageReaders()).thenReturn(messageReaders::stream);
Mono<String> resultMono = defaultRequest.bodyToMono(String.class);
@ -230,7 +231,7 @@ public class DefaultServerRequestTests {
when(mockRequest.getBody()).thenReturn(body);
Set<HttpMessageReader<?>> messageReaders = Collections
.singleton(new DecoderHttpMessageReader<String>(new StringDecoder()));
.singleton(new DecoderHttpMessageReader<>(StringDecoder.allMimeTypes(true)));
when(mockHandlerStrategies.messageReaders()).thenReturn(messageReaders::stream);
Flux<String> resultFlux = defaultRequest.bodyToFlux(String.class);

View File

@ -74,7 +74,7 @@ public class HttpEntityArgumentResolverTests {
private HttpEntityArgumentResolver createResolver() {
List<ServerHttpMessageReader<?>> readers = new ArrayList<>();
readers.add(new DecoderHttpMessageReader<>(new StringDecoder()));
readers.add(new DecoderHttpMessageReader<>(StringDecoder.allMimeTypes(true)));
return new HttpEntityArgumentResolver(readers, new ReactiveAdapterRegistry());
}

View File

@ -67,7 +67,7 @@ public class RequestBodyArgumentResolverTests {
@Before
public void setup() {
List<ServerHttpMessageReader<?>> readers = new ArrayList<>();
readers.add(new DecoderHttpMessageReader<>(new StringDecoder()));
readers.add(new DecoderHttpMessageReader<>(StringDecoder.allMimeTypes(true)));
this.resolver = new RequestBodyArgumentResolver(readers, new ReactiveAdapterRegistry());
}