From 184bb7c23c1a76bb614824623cc6070ce0e201e7 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Thu, 1 Aug 2024 16:23:35 +0300 Subject: [PATCH] Polishing in ResponseBodyEmitterReturnValueHandler See gh-33194 --- .../annotation/ReactiveTypeHandler.java | 4 +- ...ResponseBodyEmitterReturnValueHandler.java | 77 +++++++++++-------- .../annotation/ReactiveTypeHandlerTests.java | 8 +- ...nseBodyEmitterReturnValueHandlerTests.java | 17 ++-- 4 files changed, 60 insertions(+), 46 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ReactiveTypeHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ReactiveTypeHandler.java index d2463f2caef..5e3e589b8b9 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ReactiveTypeHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ReactiveTypeHandler.java @@ -172,7 +172,7 @@ class ReactiveTypeHandler { new TextEmitterSubscriber(emitter, this.taskExecutor).connect(adapter, returnValue); return emitter; } - MediaType streamingResponseType = findConcreteStreamingMediaType(mediaTypes); + MediaType streamingResponseType = findConcreteJsonStreamMediaType(mediaTypes); if (streamingResponseType != null) { ResponseBodyEmitter emitter = getEmitter(streamingResponseType); new JsonEmitterSubscriber(emitter, this.taskExecutor).connect(adapter, returnValue); @@ -203,7 +203,7 @@ class ReactiveTypeHandler { */ @SuppressWarnings("deprecation") @Nullable - static MediaType findConcreteStreamingMediaType(Collection acceptedMediaTypes) { + static MediaType findConcreteJsonStreamMediaType(Collection acceptedMediaTypes) { for (MediaType acceptedType : acceptedMediaTypes) { if (WILDCARD_SUBTYPE_SUFFIXED_BY_NDJSON.includes(acceptedType)) { if (acceptedType.isConcrete()) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandler.java index fd23bf24d97..3dab9ed9036 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandler.java @@ -49,12 +49,25 @@ import org.springframework.web.method.support.HandlerMethodReturnValueHandler; import org.springframework.web.method.support.ModelAndViewContainer; /** - * Handler for return values of type {@link ResponseBodyEmitter} and subclasses - * such as {@link SseEmitter} including the same types wrapped with - * {@link ResponseEntity}. + * Handler for return values of type: + *
    + *
  • {@link ResponseBodyEmitter} including sub-class {@link SseEmitter} and others. + *
  • Reactive return types known to {@link ReactiveAdapterRegistry}. + *
  • Any of the above wrapped with {@link ResponseEntity}. + *
* - *

As of 5.0 also supports reactive return value types for any reactive - * library with registered adapters in {@link ReactiveAdapterRegistry}. + *

Single-value reactive types are adapted to {@link DeferredResult}. + * Multi-value reactive types are adapted to {@link ResponseBodyEmitter} or + * {@link SseEmitter} as follows: + *

    + *
  • SSE stream, if the element type is + * {@link org.springframework.http.codec.ServerSentEvent} or if negotiated by + * content type. + *
  • Text stream for a {@link org.reactivestreams.Publisher} of + * {@link CharSequence}. + *
  • A JSON stream if negotiated by content type to + * {@link MediaType#APPLICATION_NDJSON}. + *
* * @author Rossen Stoyanchev * @since 4.2 @@ -153,7 +166,7 @@ public class ResponseBodyEmitterReturnValueHandler implements HandlerMethodRetur else { emitter = this.reactiveHandler.handleValue(returnValue, returnType, mavContainer, webRequest); if (emitter == null) { - // Not streaming: write headers without committing response.. + // We're not streaming; write headers without committing response outputMessage.getHeaders().forEach((headerName, headerValues) -> { for (String headerValue : headerValues) { response.addHeader(headerName, headerValue); @@ -164,18 +177,17 @@ public class ResponseBodyEmitterReturnValueHandler implements HandlerMethodRetur } emitter.extendResponse(outputMessage); - // At this point we know we're streaming.. + // We are streaming ShallowEtagHeaderFilter.disableContentCaching(request); - // Wrap the response to ignore further header changes - // Headers will be flushed at the first write + // Ignore further header changes; response is committed after first event outputMessage = new StreamingServletServerHttpResponse(outputMessage); HttpMessageConvertingHandler handler; try { - DeferredResult deferredResult = new DeferredResult<>(emitter.getTimeout()); - WebAsyncUtils.getAsyncManager(webRequest).startDeferredResultProcessing(deferredResult, mavContainer); - handler = new HttpMessageConvertingHandler(outputMessage, deferredResult); + DeferredResult result = new DeferredResult<>(emitter.getTimeout()); + WebAsyncUtils.getAsyncManager(webRequest).startDeferredResultProcessing(result, mavContainer); + handler = new HttpMessageConvertingHandler(outputMessage, result); } catch (Throwable ex) { emitter.initializeWithError(ex); @@ -186,6 +198,26 @@ public class ResponseBodyEmitterReturnValueHandler implements HandlerMethodRetur } + /** + * Wrap to silently ignore header changes HttpMessageConverter's that would + * otherwise cause HttpHeaders to raise exceptions. + */ + private static class StreamingServletServerHttpResponse extends DelegatingServerHttpResponse { + + private final HttpHeaders mutableHeaders = new HttpHeaders(); + + public StreamingServletServerHttpResponse(ServerHttpResponse delegate) { + super(delegate); + this.mutableHeaders.putAll(delegate.getHeaders()); + } + + @Override + public HttpHeaders getHeaders() { + return this.mutableHeaders; + } + } + + /** * ResponseBodyEmitter.Handler that writes with HttpMessageConverter's. */ @@ -257,25 +289,4 @@ public class ResponseBodyEmitterReturnValueHandler implements HandlerMethodRetur } } - - /** - * Wrap to silently ignore header changes HttpMessageConverter's that would - * otherwise cause HttpHeaders to raise exceptions. - */ - private static class StreamingServletServerHttpResponse extends DelegatingServerHttpResponse { - - private final HttpHeaders mutableHeaders = new HttpHeaders(); - - public StreamingServletServerHttpResponse(ServerHttpResponse delegate) { - super(delegate); - this.mutableHeaders.putAll(delegate.getHeaders()); - } - - @Override - public HttpHeaders getHeaders() { - return this.mutableHeaders; - } - - } - } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ReactiveTypeHandlerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ReactiveTypeHandlerTests.java index 3c4fdf2d0a1..e59af479919 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ReactiveTypeHandlerTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ReactiveTypeHandlerTests.java @@ -130,7 +130,7 @@ class ReactiveTypeHandlerTests { MediaType.parseMediaType("application/*+x-ndjson"), MediaType.parseMediaType("application/vnd.myapp.v1+x-ndjson")); - assertThat(ReactiveTypeHandler.findConcreteStreamingMediaType(accept)) + assertThat(ReactiveTypeHandler.findConcreteJsonStreamMediaType(accept)) .isEqualTo(MediaType.APPLICATION_NDJSON); } @@ -142,7 +142,7 @@ class ReactiveTypeHandlerTests { MediaType.parseMediaType("application/*+x-ndjson"), MediaType.APPLICATION_NDJSON); - assertThat(ReactiveTypeHandler.findConcreteStreamingMediaType(accept)) + assertThat(ReactiveTypeHandler.findConcreteJsonStreamMediaType(accept)) .hasToString("application/vnd.myapp.v1+x-ndjson"); } @@ -154,7 +154,7 @@ class ReactiveTypeHandlerTests { MediaType.parseMediaType("application/*+x-ndjson"), MediaType.parseMediaType("application/vnd.myapp.v1+x-ndjson")); - assertThat(ReactiveTypeHandler.findConcreteStreamingMediaType(accept)) + assertThat(ReactiveTypeHandler.findConcreteJsonStreamMediaType(accept)) .isEqualTo(MediaType.APPLICATION_NDJSON); } @@ -167,7 +167,7 @@ class ReactiveTypeHandlerTests { MediaType.parseMediaType("application/*+x-ndjson"), MediaType.parseMediaType("application/vnd.myapp.v1+x-ndjson")); - assertThat(ReactiveTypeHandler.findConcreteStreamingMediaType(accept)) + assertThat(ReactiveTypeHandler.findConcreteJsonStreamMediaType(accept)) .isEqualTo(MediaType.APPLICATION_STREAM_JSON); } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandlerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandlerTests.java index 1f79b8efa63..1d7e1565e7c 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandlerTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandlerTests.java @@ -62,14 +62,14 @@ import static org.springframework.web.testfixture.method.ResolvableMethod.on; */ class ResponseBodyEmitterReturnValueHandlerTests { - private ResponseBodyEmitterReturnValueHandler handler = + private final ResponseBodyEmitterReturnValueHandler handler = new ResponseBodyEmitterReturnValueHandler(List.of(new MappingJackson2HttpMessageConverter())); - private MockHttpServletRequest request = new MockHttpServletRequest(); + private final MockHttpServletRequest request = new MockHttpServletRequest(); - private MockHttpServletResponse response = new MockHttpServletResponse(); + private final MockHttpServletResponse response = new MockHttpServletResponse(); - private NativeWebRequest webRequest = new ServletWebRequest(this.request, this.response); + private final NativeWebRequest webRequest = new ServletWebRequest(this.request, this.response); private final ModelAndViewContainer mavContainer = new ModelAndViewContainer(); @@ -93,12 +93,15 @@ class ResponseBodyEmitterReturnValueHandlerTests { assertThat(this.handler.supportsReturnType( on(TestController.class).resolveReturnType(ResponseEntity.class, ResponseBodyEmitter.class))).isTrue(); + ResolvableType stringFlux = forClassWithGenerics(Flux.class, String.class); + assertThat(this.handler.supportsReturnType( - on(TestController.class).resolveReturnType(Flux.class, String.class))).isTrue(); + on(TestController.class).resolveReturnType(stringFlux))).isTrue(); + + ResolvableType responseEntityStringFlux = forClassWithGenerics(ResponseEntity.class, stringFlux); assertThat(this.handler.supportsReturnType( - on(TestController.class).resolveReturnType(forClassWithGenerics(ResponseEntity.class, - forClassWithGenerics(Flux.class, String.class))))).isTrue(); + on(TestController.class).resolveReturnType(responseEntityStringFlux))).isTrue(); } @Test