Skip to content

Commit 0662dbf

Browse files
committed
String decoding for text only vs any MIME type
Follow-up to: 3d68c49 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
1 parent a287e67 commit 0662dbf

File tree

17 files changed

+75
-49
lines changed

17 files changed

+75
-49
lines changed

spring-core/src/main/java/org/springframework/core/codec/StringDecoder.java

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,21 +58,13 @@ public class StringDecoder extends AbstractDecoder<String> {
5858
private final boolean splitOnNewline;
5959

6060

61-
/**
62-
* Create a {@code StringDecoder} that decodes a bytes stream to a String stream
63-
* <p>By default, this decoder will split along new lines.
64-
*/
65-
public StringDecoder() {
66-
this(true);
67-
}
68-
6961
/**
7062
* Create a {@code StringDecoder} that decodes a bytes stream to a String stream
7163
* @param splitOnNewline whether this decoder should split the received data buffers
7264
* along newline characters
7365
*/
74-
public StringDecoder(boolean splitOnNewline) {
75-
super(new MimeType("text", "*", DEFAULT_CHARSET), MimeTypeUtils.ALL);
66+
private StringDecoder(boolean splitOnNewline, MimeType... mimeTypes) {
67+
super(mimeTypes);
7668
this.splitOnNewline = splitOnNewline;
7769
}
7870

@@ -136,4 +128,22 @@ private Charset getCharset(MimeType mimeType) {
136128
}
137129
}
138130

131+
132+
/**
133+
* Create a {@code StringDecoder} for {@code "text/plain"}.
134+
* @param splitOnNewline whether to split the byte stream into lines
135+
*/
136+
public static StringDecoder textPlainOnly(boolean splitOnNewline) {
137+
return new StringDecoder(splitOnNewline, new MimeType("text", "plain", DEFAULT_CHARSET));
138+
}
139+
140+
/**
141+
* Create a {@code StringDecoder} that supports all MIME types.
142+
* @param splitOnNewline whether to split the byte stream into lines
143+
*/
144+
public static StringDecoder allMimeTypes(boolean splitOnNewline) {
145+
return new StringDecoder(splitOnNewline,
146+
new MimeType("text", "plain", DEFAULT_CHARSET), MimeTypeUtils.ALL);
147+
}
148+
139149
}

spring-core/src/test/java/org/springframework/core/codec/StringDecoderTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
*/
3939
public class StringDecoderTests extends AbstractDataBufferAllocatingTestCase {
4040

41-
private StringDecoder decoder = new StringDecoder();
41+
private StringDecoder decoder = StringDecoder.allMimeTypes(true);
4242

4343

4444
@Test
@@ -57,7 +57,7 @@ public void canDecode() {
5757

5858
@Test
5959
public void decode() throws InterruptedException {
60-
this.decoder = new StringDecoder(false);
60+
this.decoder = StringDecoder.allMimeTypes(false);
6161
Flux<DataBuffer> source = Flux.just(stringBuffer("foo"), stringBuffer("bar"), stringBuffer("baz"));
6262
Flux<String> output = this.decoder.decode(source, ResolvableType.forClass(String.class),
6363
null, Collections.emptyMap());
@@ -111,7 +111,7 @@ public void decodeEmptyString() throws InterruptedException {
111111

112112
@Test
113113
public void decodeToMono() throws InterruptedException {
114-
this.decoder = new StringDecoder(false);
114+
this.decoder = StringDecoder.allMimeTypes(false);
115115
Flux<DataBuffer> source = Flux.just(stringBuffer("foo"), stringBuffer("bar"), stringBuffer("baz"));
116116
Mono<String> output = this.decoder.decodeToMono(source,
117117
ResolvableType.forClass(String.class), null, Collections.emptyMap());

spring-web/src/main/java/org/springframework/http/codec/ServerSentEventHttpMessageReader.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public class ServerSentEventHttpMessageReader implements HttpMessageReader<Objec
5555

5656
private static final DataBufferFactory bufferFactory = new DefaultDataBufferFactory();
5757

58-
private static final StringDecoder stringDecoder = new StringDecoder(false);
58+
private static final StringDecoder stringDecoder = StringDecoder.textPlainOnly(false);
5959

6060

6161
private final Decoder<?> decoder;
@@ -190,6 +190,9 @@ private Object decodeData(String data, ResolvableType dataType, Map<String, Obje
190190
public Mono<Object> readMono(ResolvableType elementType, ReactiveHttpInputMessage message,
191191
Map<String, Object> hints) {
192192

193+
// We're ahead of String + "*/*"
194+
// Let's see if we can aggregate the output (lest we time out)...
195+
193196
if (String.class.equals(elementType.getRawClass())) {
194197
Flux<DataBuffer> body = message.getBody();
195198
return stringDecoder.decodeToMono(body, elementType, null, null).cast(Object.class);

spring-web/src/main/java/org/springframework/http/codec/json/Jackson2CodecSupport.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ protected Jackson2CodecSupport(ObjectMapper mapper) {
7474
}
7575

7676

77+
protected boolean supportsMimeType(MimeType mimeType) {
78+
return mimeType == null ||
79+
JSON_MIME_TYPES.stream().anyMatch(m -> m.isCompatibleWith(mimeType));
80+
}
81+
7782
/**
7883
* Return the Jackson {@link JavaType} for the specified type and context class.
7984
* <p>The default implementation returns {@code typeFactory.constructType(type, contextClass)},

spring-web/src/main/java/org/springframework/http/codec/json/Jackson2JsonDecoder.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,12 @@ public Jackson2JsonDecoder(ObjectMapper mapper) {
6767
@Override
6868
public boolean canDecode(ResolvableType elementType, MimeType mimeType) {
6969
JavaType javaType = this.mapper.getTypeFactory().constructType(elementType.getType());
70-
return this.mapper.canDeserialize(javaType) &&
71-
(mimeType == null || JSON_MIME_TYPES.stream().anyMatch(m -> m.isCompatibleWith(mimeType)));
70+
// Skip String (CharSequenceDecoder + "*/*" comes after)
71+
return !CharSequence.class.isAssignableFrom(elementType.resolve(Object.class)) &&
72+
this.mapper.canDeserialize(javaType) && supportsMimeType(mimeType);
7273
}
7374

75+
7476
@Override
7577
public List<MimeType> getDecodableMimeTypes() {
7678
return JSON_MIME_TYPES;

spring-web/src/main/java/org/springframework/http/codec/json/Jackson2JsonEncoder.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,8 @@ public List<MimeType> getEncodableMimeTypes() {
104104

105105
@Override
106106
public boolean canEncode(ResolvableType elementType, MimeType mimeType) {
107-
return this.mapper.canSerialize(elementType.getRawClass()) &&
108-
(mimeType == null || JSON_MIME_TYPES.stream().anyMatch(m -> m.isCompatibleWith(mimeType)));
107+
Class<?> clazz = elementType.getRawClass();
108+
return this.mapper.canSerialize(clazz) && supportsMimeType(mimeType);
109109
}
110110

111111
@Override

spring-web/src/test/java/org/springframework/http/codec/json/Jackson2JsonDecoderTests.java

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import static java.util.Arrays.asList;
2323
import static java.util.Collections.*;
2424
import org.junit.Test;
25+
26+
import static org.springframework.core.ResolvableType.forClass;
2527
import static org.springframework.http.MediaType.*;
2628
import static org.springframework.http.codec.json.Jackson2JsonDecoder.*;
2729
import static org.springframework.http.codec.json.JacksonViewBean.*;
@@ -50,16 +52,18 @@ public class Jackson2JsonDecoderTests extends AbstractDataBufferAllocatingTestCa
5052
@Test
5153
public void canDecode() {
5254
Jackson2JsonDecoder decoder = new Jackson2JsonDecoder();
53-
ResolvableType type = ResolvableType.forClass(Pojo.class);
54-
assertTrue(decoder.canDecode(type, APPLICATION_JSON));
55-
assertTrue(decoder.canDecode(type, null));
56-
assertFalse(decoder.canDecode(type, APPLICATION_XML));
55+
56+
assertTrue(decoder.canDecode(forClass(Pojo.class), APPLICATION_JSON));
57+
assertTrue(decoder.canDecode(forClass(Pojo.class), null));
58+
59+
assertFalse(decoder.canDecode(forClass(String.class), null));
60+
assertFalse(decoder.canDecode(forClass(Pojo.class), APPLICATION_XML));
5761
}
5862

5963
@Test
6064
public void decodePojo() throws Exception {
6165
Flux<DataBuffer> source = Flux.just(stringBuffer("{\"foo\": \"foofoo\", \"bar\": \"barbar\"}"));
62-
ResolvableType elementType = ResolvableType.forClass(Pojo.class);
66+
ResolvableType elementType = forClass(Pojo.class);
6367
Flux<Object> flux = new Jackson2JsonDecoder().decode(source, elementType, null,
6468
emptyMap());
6569

@@ -71,7 +75,7 @@ public void decodePojo() throws Exception {
7175
@Test
7276
public void decodePojoWithError() throws Exception {
7377
Flux<DataBuffer> source = Flux.just(stringBuffer("{\"foo\":}"));
74-
ResolvableType elementType = ResolvableType.forClass(Pojo.class);
78+
ResolvableType elementType = forClass(Pojo.class);
7579
Flux<Object> flux = new Jackson2JsonDecoder().decode(source, elementType, null,
7680
emptyMap());
7781

@@ -98,7 +102,7 @@ public void decodeToFlux() throws Exception {
98102
Flux<DataBuffer> source = Flux.just(stringBuffer(
99103
"[{\"bar\":\"b1\",\"foo\":\"f1\"},{\"bar\":\"b2\",\"foo\":\"f2\"}]"));
100104

101-
ResolvableType elementType = ResolvableType.forClass(Pojo.class);
105+
ResolvableType elementType = forClass(Pojo.class);
102106
Flux<Object> flux = new Jackson2JsonDecoder().decode(source, elementType, null,
103107
emptyMap());
104108

@@ -112,7 +116,7 @@ public void decodeToFlux() throws Exception {
112116
public void fieldLevelJsonView() throws Exception {
113117
Flux<DataBuffer> source = Flux.just(
114118
stringBuffer("{\"withView1\" : \"with\", \"withView2\" : \"with\", \"withoutView\" : \"without\"}"));
115-
ResolvableType elementType = ResolvableType.forClass(JacksonViewBean.class);
119+
ResolvableType elementType = forClass(JacksonViewBean.class);
116120
Map<String, Object> hints = singletonMap(JSON_VIEW_HINT, MyJacksonView1.class);
117121
Flux<JacksonViewBean> flux = new Jackson2JsonDecoder()
118122
.decode(source, elementType, null, hints).cast(JacksonViewBean.class);
@@ -130,7 +134,7 @@ public void fieldLevelJsonView() throws Exception {
130134
public void classLevelJsonView() throws Exception {
131135
Flux<DataBuffer> source = Flux.just(stringBuffer(
132136
"{\"withView1\" : \"with\", \"withView2\" : \"with\", \"withoutView\" : \"without\"}"));
133-
ResolvableType elementType = ResolvableType.forClass(JacksonViewBean.class);
137+
ResolvableType elementType = forClass(JacksonViewBean.class);
134138
Map<String, Object> hints = singletonMap(JSON_VIEW_HINT, MyJacksonView3.class);
135139
Flux<JacksonViewBean> flux = new Jackson2JsonDecoder()
136140
.decode(source, elementType, null, hints).cast(JacksonViewBean.class);
@@ -147,7 +151,7 @@ public void classLevelJsonView() throws Exception {
147151
@Test
148152
public void decodeEmptyBodyToMono() throws Exception {
149153
Flux<DataBuffer> source = Flux.empty();
150-
ResolvableType elementType = ResolvableType.forClass(Pojo.class);
154+
ResolvableType elementType = forClass(Pojo.class);
151155
Mono<Object> mono = new Jackson2JsonDecoder().decodeToMono(source, elementType,
152156
null, emptyMap());
153157

spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ protected final void addDefaultHttpMessageReaders(List<ServerHttpMessageReader<?
331331
readers.add(new DecoderHttpMessageReader<>(new ByteArrayDecoder()));
332332
readers.add(new DecoderHttpMessageReader<>(new ByteBufferDecoder()));
333333
readers.add(new DecoderHttpMessageReader<>(new DataBufferDecoder()));
334-
readers.add(new DecoderHttpMessageReader<>(new StringDecoder()));
334+
readers.add(new DecoderHttpMessageReader<>(StringDecoder.allMimeTypes(true)));
335335
readers.add(new DecoderHttpMessageReader<>(new ResourceDecoder()));
336336
if (jaxb2Present) {
337337
readers.add(new DecoderHttpMessageReader<>(new Jaxb2XmlDecoder()));

spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultExchangeStrategiesBuilder.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,17 +75,17 @@ public void defaultConfiguration() {
7575
}
7676

7777
private void defaultReaders() {
78-
// SSE first (constrained to "text/event-stream")
79-
messageReader(new ServerSentEventHttpMessageReader(getSseDecoder()));
8078
messageReader(new DecoderHttpMessageReader<>(new ByteArrayDecoder()));
8179
messageReader(new DecoderHttpMessageReader<>(new ByteBufferDecoder()));
82-
messageReader(new DecoderHttpMessageReader<>(new StringDecoder(false)));
80+
messageReader(new DecoderHttpMessageReader<>(StringDecoder.textPlainOnly(false)));
8381
if (jaxb2Present) {
8482
messageReader(new DecoderHttpMessageReader<>(new Jaxb2XmlDecoder()));
8583
}
8684
if (jackson2Present) {
8785
messageReader(new DecoderHttpMessageReader<>(new Jackson2JsonDecoder()));
8886
}
87+
messageReader(new ServerSentEventHttpMessageReader(getSseDecoder()));
88+
messageReader(new DecoderHttpMessageReader<>(StringDecoder.allMimeTypes(false)));
8989
}
9090

9191
private Decoder<?> getSseDecoder() {

spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultHandlerStrategiesBuilder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ class DefaultHandlerStrategiesBuilder implements HandlerStrategies.Builder {
8484
public void defaultConfiguration() {
8585
messageReader(new DecoderHttpMessageReader<>(new ByteArrayDecoder()));
8686
messageReader(new DecoderHttpMessageReader<>(new ByteBufferDecoder()));
87-
messageReader(new DecoderHttpMessageReader<>(new StringDecoder()));
87+
messageReader(new DecoderHttpMessageReader<>(StringDecoder.allMimeTypes(true)));
8888
messageReader(new FormHttpMessageReader());
8989

9090
messageWriter(new EncoderHttpMessageWriter<>(new ByteArrayEncoder()));

0 commit comments

Comments
 (0)