Skip to content

Commit 184bb7c

Browse files
rstoyanchevrstoyanchev
committed
Polishing in ResponseBodyEmitterReturnValueHandler
See gh-33194
1 parent f3ab390 commit 184bb7c

File tree

4 files changed

+60
-46
lines changed

4 files changed

+60
-46
lines changed

‎spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ReactiveTypeHandler.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ public ResponseBodyEmitter handleValue(Object returnValue, MethodParameter retur
172172
new TextEmitterSubscriber(emitter, this.taskExecutor).connect(adapter, returnValue);
173173
return emitter;
174174
}
175-
MediaType streamingResponseType = findConcreteStreamingMediaType(mediaTypes);
175+
MediaType streamingResponseType = findConcreteJsonStreamMediaType(mediaTypes);
176176
if (streamingResponseType != null) {
177177
ResponseBodyEmitter emitter = getEmitter(streamingResponseType);
178178
new JsonEmitterSubscriber(emitter, this.taskExecutor).connect(adapter, returnValue);
@@ -203,7 +203,7 @@ public ResponseBodyEmitter handleValue(Object returnValue, MethodParameter retur
203203
*/
204204
@SuppressWarnings("deprecation")
205205
@Nullable
206-
static MediaType findConcreteStreamingMediaType(Collection<MediaType> acceptedMediaTypes) {
206+
static MediaType findConcreteJsonStreamMediaType(Collection<MediaType> acceptedMediaTypes) {
207207
for (MediaType acceptedType : acceptedMediaTypes) {
208208
if (WILDCARD_SUBTYPE_SUFFIXED_BY_NDJSON.includes(acceptedType)) {
209209
if (acceptedType.isConcrete()) {

‎spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandler.java

+44-33
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,25 @@
4949
import org.springframework.web.method.support.ModelAndViewContainer;
5050

5151
/**
52-
* Handler for return values of type {@link ResponseBodyEmitter} and subclasses
53-
* such as {@link SseEmitter} including the same types wrapped with
54-
* {@link ResponseEntity}.
52+
* Handler for return values of type:
53+
* <ul>
54+
* <li>{@link ResponseBodyEmitter} including sub-class {@link SseEmitter} and others.
55+
* <li>Reactive return types known to {@link ReactiveAdapterRegistry}.
56+
* <li>Any of the above wrapped with {@link ResponseEntity}.
57+
* </ul>
5558
*
56-
* <p>As of 5.0 also supports reactive return value types for any reactive
57-
* library with registered adapters in {@link ReactiveAdapterRegistry}.
59+
* <p>Single-value reactive types are adapted to {@link DeferredResult}.
60+
* Multi-value reactive types are adapted to {@link ResponseBodyEmitter} or
61+
* {@link SseEmitter} as follows:
62+
* <ul>
63+
* <li>SSE stream, if the element type is
64+
* {@link org.springframework.http.codec.ServerSentEvent} or if negotiated by
65+
* content type.
66+
* <li>Text stream for a {@link org.reactivestreams.Publisher} of
67+
* {@link CharSequence}.
68+
* <li>A JSON stream if negotiated by content type to
69+
* {@link MediaType#APPLICATION_NDJSON}.
70+
* </ul>
5871
*
5972
* @author Rossen Stoyanchev
6073
* @since 4.2
@@ -153,7 +166,7 @@ public void handleReturnValue(@Nullable Object returnValue, MethodParameter retu
153166
else {
154167
emitter = this.reactiveHandler.handleValue(returnValue, returnType, mavContainer, webRequest);
155168
if (emitter == null) {
156-
// Not streaming: write headers without committing response..
169+
// We're not streaming; write headers without committing response
157170
outputMessage.getHeaders().forEach((headerName, headerValues) -> {
158171
for (String headerValue : headerValues) {
159172
response.addHeader(headerName, headerValue);
@@ -164,18 +177,17 @@ public void handleReturnValue(@Nullable Object returnValue, MethodParameter retu
164177
}
165178
emitter.extendResponse(outputMessage);
166179

167-
// At this point we know we're streaming..
180+
// We are streaming
168181
ShallowEtagHeaderFilter.disableContentCaching(request);
169182

170-
// Wrap the response to ignore further header changes
171-
// Headers will be flushed at the first write
183+
// Ignore further header changes; response is committed after first event
172184
outputMessage = new StreamingServletServerHttpResponse(outputMessage);
173185

174186
HttpMessageConvertingHandler handler;
175187
try {
176-
DeferredResult<?> deferredResult = new DeferredResult<>(emitter.getTimeout());
177-
WebAsyncUtils.getAsyncManager(webRequest).startDeferredResultProcessing(deferredResult, mavContainer);
178-
handler = new HttpMessageConvertingHandler(outputMessage, deferredResult);
188+
DeferredResult<?> result = new DeferredResult<>(emitter.getTimeout());
189+
WebAsyncUtils.getAsyncManager(webRequest).startDeferredResultProcessing(result, mavContainer);
190+
handler = new HttpMessageConvertingHandler(outputMessage, result);
179191
}
180192
catch (Throwable ex) {
181193
emitter.initializeWithError(ex);
@@ -186,6 +198,26 @@ public void handleReturnValue(@Nullable Object returnValue, MethodParameter retu
186198
}
187199

188200

201+
/**
202+
* Wrap to silently ignore header changes HttpMessageConverter's that would
203+
* otherwise cause HttpHeaders to raise exceptions.
204+
*/
205+
private static class StreamingServletServerHttpResponse extends DelegatingServerHttpResponse {
206+
207+
private final HttpHeaders mutableHeaders = new HttpHeaders();
208+
209+
public StreamingServletServerHttpResponse(ServerHttpResponse delegate) {
210+
super(delegate);
211+
this.mutableHeaders.putAll(delegate.getHeaders());
212+
}
213+
214+
@Override
215+
public HttpHeaders getHeaders() {
216+
return this.mutableHeaders;
217+
}
218+
}
219+
220+
189221
/**
190222
* ResponseBodyEmitter.Handler that writes with HttpMessageConverter's.
191223
*/
@@ -257,25 +289,4 @@ public void onCompletion(Runnable callback) {
257289
}
258290
}
259291

260-
261-
/**
262-
* Wrap to silently ignore header changes HttpMessageConverter's that would
263-
* otherwise cause HttpHeaders to raise exceptions.
264-
*/
265-
private static class StreamingServletServerHttpResponse extends DelegatingServerHttpResponse {
266-
267-
private final HttpHeaders mutableHeaders = new HttpHeaders();
268-
269-
public StreamingServletServerHttpResponse(ServerHttpResponse delegate) {
270-
super(delegate);
271-
this.mutableHeaders.putAll(delegate.getHeaders());
272-
}
273-
274-
@Override
275-
public HttpHeaders getHeaders() {
276-
return this.mutableHeaders;
277-
}
278-
279-
}
280-
281292
}

‎spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ReactiveTypeHandlerTests.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ void findsConcreteStreamingMediaType() {
130130
MediaType.parseMediaType("application/*+x-ndjson"),
131131
MediaType.parseMediaType("application/vnd.myapp.v1+x-ndjson"));
132132

133-
assertThat(ReactiveTypeHandler.findConcreteStreamingMediaType(accept))
133+
assertThat(ReactiveTypeHandler.findConcreteJsonStreamMediaType(accept))
134134
.isEqualTo(MediaType.APPLICATION_NDJSON);
135135
}
136136

@@ -142,7 +142,7 @@ void findsConcreteStreamingMediaType_vendorFirst() {
142142
MediaType.parseMediaType("application/*+x-ndjson"),
143143
MediaType.APPLICATION_NDJSON);
144144

145-
assertThat(ReactiveTypeHandler.findConcreteStreamingMediaType(accept))
145+
assertThat(ReactiveTypeHandler.findConcreteJsonStreamMediaType(accept))
146146
.hasToString("application/vnd.myapp.v1+x-ndjson");
147147
}
148148

@@ -154,7 +154,7 @@ void findsConcreteStreamingMediaType_plainNdJsonFirst() {
154154
MediaType.parseMediaType("application/*+x-ndjson"),
155155
MediaType.parseMediaType("application/vnd.myapp.v1+x-ndjson"));
156156

157-
assertThat(ReactiveTypeHandler.findConcreteStreamingMediaType(accept))
157+
assertThat(ReactiveTypeHandler.findConcreteJsonStreamMediaType(accept))
158158
.isEqualTo(MediaType.APPLICATION_NDJSON);
159159
}
160160

@@ -167,7 +167,7 @@ void findsConcreteStreamingMediaType_plainStreamingJsonFirst() {
167167
MediaType.parseMediaType("application/*+x-ndjson"),
168168
MediaType.parseMediaType("application/vnd.myapp.v1+x-ndjson"));
169169

170-
assertThat(ReactiveTypeHandler.findConcreteStreamingMediaType(accept))
170+
assertThat(ReactiveTypeHandler.findConcreteJsonStreamMediaType(accept))
171171
.isEqualTo(MediaType.APPLICATION_STREAM_JSON);
172172
}
173173

‎spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandlerTests.java

+10-7
Original file line numberDiff line numberDiff line change
@@ -62,14 +62,14 @@
6262
*/
6363
class ResponseBodyEmitterReturnValueHandlerTests {
6464

65-
private ResponseBodyEmitterReturnValueHandler handler =
65+
private final ResponseBodyEmitterReturnValueHandler handler =
6666
new ResponseBodyEmitterReturnValueHandler(List.of(new MappingJackson2HttpMessageConverter()));
6767

68-
private MockHttpServletRequest request = new MockHttpServletRequest();
68+
private final MockHttpServletRequest request = new MockHttpServletRequest();
6969

70-
private MockHttpServletResponse response = new MockHttpServletResponse();
70+
private final MockHttpServletResponse response = new MockHttpServletResponse();
7171

72-
private NativeWebRequest webRequest = new ServletWebRequest(this.request, this.response);
72+
private final NativeWebRequest webRequest = new ServletWebRequest(this.request, this.response);
7373

7474
private final ModelAndViewContainer mavContainer = new ModelAndViewContainer();
7575

@@ -93,12 +93,15 @@ void supportsReturnTypes() {
9393
assertThat(this.handler.supportsReturnType(
9494
on(TestController.class).resolveReturnType(ResponseEntity.class, ResponseBodyEmitter.class))).isTrue();
9595

96+
ResolvableType stringFlux = forClassWithGenerics(Flux.class, String.class);
97+
9698
assertThat(this.handler.supportsReturnType(
97-
on(TestController.class).resolveReturnType(Flux.class, String.class))).isTrue();
99+
on(TestController.class).resolveReturnType(stringFlux))).isTrue();
100+
101+
ResolvableType responseEntityStringFlux = forClassWithGenerics(ResponseEntity.class, stringFlux);
98102

99103
assertThat(this.handler.supportsReturnType(
100-
on(TestController.class).resolveReturnType(forClassWithGenerics(ResponseEntity.class,
101-
forClassWithGenerics(Flux.class, String.class))))).isTrue();
104+
on(TestController.class).resolveReturnType(responseEntityStringFlux))).isTrue();
102105
}
103106

104107
@Test

0 commit comments

Comments
 (0)