Spring Improve handling of ResponseEntity<?> in Spring MVC

We have separate HandlerMethodReturnValueHandlers for regular response body vs streaming type return values, and if you declare a controller method to return ResponseEntity<?>, with an intentionally opaque generic type (so you can either stream or not), we can't correctly handle it.

ReturnValueMethodParameter does consider the actual return value Class, and that helps with cases like SseEmitter vs some Object <T>, but doesn't help in cases like ResponseEntity<?> where you would also need generic type info.

We can consider improved support through a consolidated return value handler for the ResponseEntity wrapper that delegates to either the one for response body or the one for streaming after considering the actual body in the ResponseEntity.

Comment From: DeepDhamala

Hi! 👋

After doing some digging into the internals, I think I understand the issue like this and have an idea on what can be done—please correct me in case I’m overlooking something.

When the HandlerMethodReturnValueHandlerComposite method selectHandler(...) iterates through the available HandlerMethodReturnValueHandler implementations, the one we expect to handle a return type(actual return value) of ResponseEntity<AnyEmitter> is ResponseBodyEmitterReturnValueHandler. However, the supportsReturnType(...)method in ResponseBodyEmitterReturnValueHandler fails to match because it attempts to resolve the body type from the method signature .

Class<?> bodyType = ResponseEntity.class.isAssignableFrom(returnType.getParameterType()) ?
                ResolvableType.forMethodParameter(returnType).getGeneric().resolve() :
                returnType.getParameterType();

In this case, the method signature is declared as ResponseEntity<?>, as a result, the resolved bodyType is null, and the handler does not recognize it as a supported return type.

@Override
    public boolean supportsReturnType(MethodParameter returnType) {
        Class<?> bodyType = ResponseEntity.class.isAssignableFrom(returnType.getParameterType()) ?
                ResolvableType.forMethodParameter(returnType).getGeneric().resolve() :
                returnType.getParameterType();

        return (bodyType != null && (ResponseBodyEmitter.class.isAssignableFrom(bodyType) ||
                this.reactiveHandler.isReactiveType(bodyType)));
    }

In the HandlerMethodReturnValueHandlerComposite’s selectHandler(...)method, we already have access to the actual return object via the value parameter.

@Nullable
    private HandlerMethodReturnValueHandler selectHandler(@Nullable Object value, MethodParameter returnType) {
        boolean isAsyncValue = isAsyncReturnValue(value, returnType);
        for (HandlerMethodReturnValueHandler handler : this.returnValueHandlers) {
            if (isAsyncValue && !(handler instanceof AsyncHandlerMethodReturnValueHandler)) {
                continue;
            }
            if (handler.supportsReturnType(returnType)) {
                return handler;
            }
        }
        return null;
    }

Would it make sense to also consider the Object value (i.e. the actual return value) in order to determine the body type dynamically and delegate accordingly?

Comment From: rstoyanchev

@DeepDhamala yes one way or another the value needs to be considered, and typically is once a return value handler is selected. The problem is that in this case there are two potential handlers.

I've made a more targeted change to address the issue.

Comment From: DeepDhamala

Glad to see how it's implemented! The use of a wrapper ResponseEntityReturnValueHandler that delegates to ResponseBodyEmitterReturnValueHandler based on the runtime type of the body, or falls back to HttpEntityMethodProcessor.