root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 8 Packages Projects Releases Wiki Activity

Infer HTTP 404 from empty Optional/Publisher types 

This commit handles "empty" cases for `ResponseEntity` controller
handler return types when wrapped with a `java.util.Optional` in Spring
MVC or a single `Publisher` like `Mono`.

Given the following example for Spring MVC:

```
@GetMapping("/user")
public Optional<ResponseEntity<User>> fetchUser() {
	Optional<User> user = //...
	return user.map(ResponseEntity::ok);
}
```

If the resulting `Optional` is empty, Spring MVC will infer a
`ResponseEntity` with an empty body and a 404 HTTP response status.

The same reasoning is applied to Spring WebFlux with Publisher types:

```
@GetMapping("/user")
public Mono<ResponseEntity<User>> fetchUser() {
	Mono<User> user = //...
	return user.map(ResponseEntity::ok);
}
```

This feature is only valid for `HttpEntity` return types and does not
apply to `@ResponseBody` controller handlers.

Issue: SPR-13281
This commit is contained in:
Brian Clozel 2018-08-15 10:58:41 +02:00
parent 04141dee65
commit 7e91733502
6 changed files with 158 additions and 83 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityResultHandler.java
View File

@ -55,6 +55,8 @@ public class ResponseEntityResultHandler extends AbstractMessageWriterResultHand
private static final Set<HttpMethod> SAFE_METHODS = EnumSet.of(HttpMethod.GET, HttpMethod.HEAD);
private static final ResponseEntity<Object> notFound = new ResponseEntity<>(HttpStatus.NOT_FOUND);
/**
* Basic constructor with a default {@link ReactiveAdapterRegistry}.
@ -119,7 +121,8 @@ public class ResponseEntityResultHandler extends AbstractMessageWriterResultHand
if (adapter != null) {
Assert.isTrue(!adapter.isMultiValue(), "Only a single ResponseEntity supported");
returnValueMono = Mono.from(adapter.toPublisher(result.getReturnValue()));
returnValueMono = Mono.from(adapter.toPublisher(result.getReturnValue()))
.defaultIfEmpty(notFound);
bodyParameter = actualParameter.nested().nested();
}
else {

14
spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityResultHandlerTests.java
View File

@ -350,6 +350,20 @@ public class ResponseEntityResultHandlerTests {
assertResponseBodyIsEmpty(exchange);
}
@Test // SPR-13281
public void handleEmptyMonoShouldResultInNotFoundStatus() {
MockServerWebExchange exchange = MockServerWebExchange.from(get("/path"));
exchange.getAttributes().put(PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE, Collections.singleton(APPLICATION_JSON));
MethodParameter type = on(TestController.class).resolveReturnType(Mono.class, ResponseEntity.class);
HandlerResult result = new HandlerResult(new TestController(), Mono.empty(), type);
this.resultHandler.handleResult(exchange, result).block(Duration.ofSeconds(5));
assertEquals(HttpStatus.NOT_FOUND, exchange.getResponse().getStatusCode());
assertResponseBodyIsEmpty(exchange);
}
private void testHandle(Object returnValue, MethodParameter returnType) {

16
spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java
View File

@ -24,6 +24,7 @@ import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@ -33,6 +34,7 @@ import org.springframework.core.ResolvableType;
import org.springframework.http.HttpEntity;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.http.RequestEntity;
import org.springframework.http.ResponseEntity;
import org.springframework.http.converter.HttpMessageConverter;
@ -121,8 +123,9 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro
@Override
public boolean supportsReturnType(MethodParameter returnType) {
return (HttpEntity.class.isAssignableFrom(returnType.getParameterType()) &&
!RequestEntity.class.isAssignableFrom(returnType.getParameterType()));
Class<?> parameterType = returnType.nestedIfOptional().getNestedParameterType();
return (HttpEntity.class.isAssignableFrom(parameterType) &&
!RequestEntity.class.isAssignableFrom(parameterType));
}
@Override
@ -150,7 +153,7 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro
@Nullable
private Type getHttpEntityType(MethodParameter parameter) {
Assert.isAssignable(HttpEntity.class, parameter.getParameterType());
Assert.isAssignable(HttpEntity.class, parameter.getNestedParameterType());
Type parameterType = parameter.getGenericParameterType();
if (parameterType instanceof ParameterizedType) {
ParameterizedType type = (ParameterizedType) parameterType;
@ -169,6 +172,7 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro
}
@Override
@SuppressWarnings("unchecked")
public void handleReturnValue(@Nullable Object returnValue, MethodParameter returnType,
ModelAndViewContainer mavContainer, NativeWebRequest webRequest) throws Exception {
@ -180,6 +184,12 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro
ServletServerHttpRequest inputMessage = createInputMessage(webRequest);
ServletServerHttpResponse outputMessage = createOutputMessage(webRequest);
if (returnType.getParameterType() == Optional.class) {
Optional<HttpEntity<?>> optionalEntity = (Optional<HttpEntity<?>>) returnValue;
returnValue = optionalEntity.orElseGet(() -> new ResponseEntity<>(HttpStatus.NOT_FOUND));
returnType = returnType.nestedIfOptional();
}
Assert.isInstanceOf(HttpEntity.class, returnValue);
HttpEntity<?> responseEntity = (HttpEntity<?>) returnValue;

167
spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java
View File

@ -19,7 +19,6 @@ package org.springframework.web.servlet.mvc.method.annotation;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.lang.reflect.Method;
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.time.ZoneId;
@ -28,6 +27,7 @@ import java.time.temporal.ChronoUnit;
import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.Optional;
import org.hamcrest.Matchers;
import org.junit.Before;
@ -56,14 +56,17 @@ import org.springframework.web.HttpMediaTypeNotAcceptableException;
import org.springframework.web.HttpMediaTypeNotSupportedException;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.context.request.ServletWebRequest;
import org.springframework.web.method.ResolvableMethod;
import org.springframework.web.method.support.ModelAndViewContainer;
import static java.time.Instant.*;
import static java.time.format.DateTimeFormatter.*;
import static java.time.Instant.ofEpochMilli;
import static java.time.format.DateTimeFormatter.RFC_1123_DATE_TIME;
import static org.junit.Assert.*;
import static org.mockito.BDDMockito.*;
import static org.springframework.http.MediaType.*;
import static org.springframework.web.servlet.HandlerMapping.*;
import static org.springframework.http.MediaType.APPLICATION_OCTET_STREAM;
import static org.springframework.http.MediaType.TEXT_PLAIN;
import static org.springframework.web.method.ResolvableMethod.on;
import static org.springframework.web.servlet.HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE;
/**
* Test fixture for {@link HttpEntityMethodProcessor} delegating to a mock
@ -91,26 +94,6 @@ public class HttpEntityMethodProcessorMockTests {
private HttpMessageConverter<Object> resourceRegionMessageConverter;
private MethodParameter paramHttpEntity;
private MethodParameter paramRequestEntity;
private MethodParameter paramResponseEntity;
private MethodParameter paramInt;
private MethodParameter returnTypeResponseEntity;
private MethodParameter returnTypeResponseEntityProduces;
private MethodParameter returnTypeResponseEntityResource;
private MethodParameter returnTypeHttpEntity;
private MethodParameter returnTypeHttpEntitySubclass;
private MethodParameter returnTypeInt;
private ModelAndViewContainer mavContainer;
private MockHttpServletRequest servletRequest;
@ -119,6 +102,12 @@ public class HttpEntityMethodProcessorMockTests {
private ServletWebRequest webRequest;
private MethodParameter returnTypeResponseEntity =
on(TestHandler.class).resolveReturnType(ResponseEntity.class, String.class);
private MethodParameter returnTypeResponseEntityResource =
on(TestHandler.class).resolveReturnType(ResponseEntity.class, Resource.class);
@Before
@SuppressWarnings("unchecked")
@ -139,20 +128,6 @@ public class HttpEntityMethodProcessorMockTests {
processor = new HttpEntityMethodProcessor(Arrays.asList(
stringHttpMessageConverter, resourceMessageConverter, resourceRegionMessageConverter));
Method handle1 = getClass().getMethod("handle1", HttpEntity.class, ResponseEntity.class,
Integer.TYPE, RequestEntity.class);
paramHttpEntity = new MethodParameter(handle1, 0);
paramRequestEntity = new MethodParameter(handle1, 3);
paramResponseEntity = new MethodParameter(handle1, 1);
paramInt = new MethodParameter(handle1, 2);
returnTypeResponseEntity = new MethodParameter(handle1, -1);
returnTypeResponseEntityProduces = new MethodParameter(getClass().getMethod("handle4"), -1);
returnTypeHttpEntity = new MethodParameter(getClass().getMethod("handle2", HttpEntity.class), -1);
returnTypeHttpEntitySubclass = new MethodParameter(getClass().getMethod("handle2x", HttpEntity.class), -1);
returnTypeInt = new MethodParameter(getClass().getMethod("handle3"), -1);
returnTypeResponseEntityResource = new MethodParameter(getClass().getMethod("handle5"), -1);
mavContainer = new ModelAndViewContainer();
servletRequest = new MockHttpServletRequest("GET", "/foo");
servletResponse = new MockHttpServletResponse();
@ -162,25 +137,39 @@ public class HttpEntityMethodProcessorMockTests {
@Test
public void supportsParameter() {
assertTrue("HttpEntity parameter not supported", processor.supportsParameter(paramHttpEntity));
assertTrue("RequestEntity parameter not supported", processor.supportsParameter(paramRequestEntity));
assertFalse("ResponseEntity parameter supported", processor.supportsParameter(paramResponseEntity));
assertFalse("non-entity parameter supported", processor.supportsParameter(paramInt));
ResolvableMethod method = on(TestHandler.class).named("entityParams").build();
assertTrue("HttpEntity parameter not supported",
processor.supportsParameter(method.arg(HttpEntity.class, String.class)));
assertTrue("RequestEntity parameter not supported",
processor.supportsParameter(method.arg(RequestEntity.class, String.class)));
assertFalse("ResponseEntity parameter supported",
processor.supportsParameter(method.arg(ResponseEntity.class, String.class)));
assertFalse("non-entity parameter supported",
processor.supportsParameter(method.arg(Integer.class)));
}
@Test
public void supportsReturnType() {
assertTrue("ResponseEntity return type not supported", processor.supportsReturnType(returnTypeResponseEntity));
assertTrue("HttpEntity return type not supported", processor.supportsReturnType(returnTypeHttpEntity));
assertTrue("Custom HttpEntity subclass not supported", processor.supportsReturnType(returnTypeHttpEntitySubclass));
assertFalse("RequestEntity parameter supported",
processor.supportsReturnType(paramRequestEntity));
assertFalse("non-ResponseBody return type supported", processor.supportsReturnType(returnTypeInt));
assertTrue("ResponseEntity return type not supported",
processor.supportsReturnType(on(TestHandler.class).resolveReturnType(ResponseEntity.class, String.class)));
assertTrue("HttpEntity return type not supported",
processor.supportsReturnType(on(TestHandler.class).resolveReturnType(HttpEntity.class)));
assertTrue("Custom HttpEntity subclass not supported",
processor.supportsReturnType(on(TestHandler.class).resolveReturnType(CustomHttpEntity.class)));
assertTrue("Optional ResponseEntity not supported",
processor.supportsReturnType(on(TestHandler.class).resolveReturnType(Optional.class, ResponseEntity.class)));
assertFalse("RequestEntity return type supported",
processor.supportsReturnType(on(TestHandler.class)
.named("entityParams").build().arg(RequestEntity.class, String.class)));
assertFalse("non-ResponseBody return type supported",
processor.supportsReturnType(on(TestHandler.class).resolveReturnType(Integer.class)));
}
@Test
public void shouldResolveHttpEntityArgument() throws Exception {
String body = "Foo";
MethodParameter paramHttpEntity = on(TestHandler.class).named("entityParams")
.build().arg(HttpEntity.class, String.class);
MediaType contentType = TEXT_PLAIN;
servletRequest.addHeader("Content-Type", contentType.toString());
@ -199,6 +188,8 @@ public class HttpEntityMethodProcessorMockTests {
@Test
public void shouldResolveRequestEntityArgument() throws Exception {
String body = "Foo";
MethodParameter paramRequestEntity = on(TestHandler.class).named("entityParams")
.build().arg(RequestEntity.class, String.class);
MediaType contentType = TEXT_PLAIN;
servletRequest.addHeader("Content-Type", contentType.toString());
@ -225,6 +216,8 @@ public class HttpEntityMethodProcessorMockTests {
@Test
public void shouldFailResolvingWhenConverterCannotRead() throws Exception {
MethodParameter paramHttpEntity = on(TestHandler.class).named("entityParams")
.build().arg(HttpEntity.class, String.class);
MediaType contentType = TEXT_PLAIN;
servletRequest.setMethod("POST");
servletRequest.addHeader("Content-Type", contentType.toString());
@ -238,6 +231,8 @@ public class HttpEntityMethodProcessorMockTests {
@Test
public void shouldFailResolvingWhenContentTypeNotSupported() throws Exception {
MethodParameter paramHttpEntity = on(TestHandler.class).named("entityParams")
.build().arg(HttpEntity.class, String.class);
servletRequest.setMethod("POST");
servletRequest.setContent("some content".getBytes(StandardCharsets.UTF_8));
this.thrown.expect(HttpMediaTypeNotSupportedException.class);
@ -260,13 +255,14 @@ public class HttpEntityMethodProcessorMockTests {
@Test
public void shouldHandleReturnValueWithProducibleMediaType() throws Exception {
MethodParameter returnType = on(TestHandler.class).resolveReturnType(ResponseEntity.class, CharSequence.class);
String body = "Foo";
ResponseEntity<String> returnValue = new ResponseEntity<>(body, HttpStatus.OK);
servletRequest.addHeader("Accept", "text/*");
servletRequest.setAttribute(PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE, Collections.singleton(MediaType.TEXT_HTML));
given(stringHttpMessageConverter.canWrite(String.class, MediaType.TEXT_HTML)).willReturn(true);
processor.handleReturnValue(returnValue, returnTypeResponseEntityProduces, mavContainer, webRequest);
processor.handleReturnValue(returnValue, returnType, mavContainer, webRequest);
assertTrue(mavContainer.isRequestHandled());
verify(stringHttpMessageConverter).write(eq(body), eq(MediaType.TEXT_HTML), isA(HttpOutputMessage.class));
@ -311,6 +307,7 @@ public class HttpEntityMethodProcessorMockTests {
@Test
public void shouldFailHandlingWhenConverterCannotWrite() throws Exception {
MethodParameter returnType = on(TestHandler.class).resolveReturnType(ResponseEntity.class, CharSequence.class);
String body = "Foo";
ResponseEntity<String> returnValue = new ResponseEntity<>(body, HttpStatus.OK);
MediaType accepted = TEXT_PLAIN;
@ -322,7 +319,7 @@ public class HttpEntityMethodProcessorMockTests {
given(stringHttpMessageConverter.canWrite(String.class, accepted)).willReturn(false);
this.thrown.expect(HttpMediaTypeNotAcceptableException.class);
processor.handleReturnValue(returnValue, returnTypeResponseEntityProduces, mavContainer, webRequest);
processor.handleReturnValue(returnValue, returnType, mavContainer, webRequest);
}
@Test // SPR-9142
@ -361,6 +358,16 @@ public class HttpEntityMethodProcessorMockTests {
assertEquals("headerValue", outputMessage.getValue().getHeaders().get("header").get(0));
}
@Test // SPR-13281
public void shouldReturnNotFoundWhenEmptyOptional() throws Exception {
MethodParameter returnType = on(TestHandler.class).resolveReturnType(Optional.class, ResponseEntity.class);
Optional<ResponseEntity> returnValue = Optional.empty();
processor.handleReturnValue(returnValue, returnType, mavContainer, webRequest);
assertTrue(mavContainer.isRequestHandled());
assertEquals(HttpStatus.NOT_FOUND.value(), servletResponse.getStatus());
}
@Test
public void shouldHandleLastModifiedWithHttp304() throws Exception {
long currentTime = new Date().getTime();
@ -697,38 +704,46 @@ public class HttpEntityMethodProcessorMockTests {
}
}
private static class TestHandler {
@SuppressWarnings("unused")
public ResponseEntity<String> handle1(HttpEntity<String> httpEntity, ResponseEntity<String> entity,
int i, RequestEntity<String> requestEntity) {
@SuppressWarnings("unused")
public ResponseEntity<String> entityParams(HttpEntity<String> httpEntity, ResponseEntity<String> entity,
Integer i, RequestEntity<String> requestEntity) {
return entity;
}
return entity;
}
@SuppressWarnings("unused")
public HttpEntity<?> handle2(HttpEntity<?> entity) {
return entity;
}
@SuppressWarnings("unused")
public HttpEntity<?> httpEntity(HttpEntity<?> entity) {
return entity;
}
@SuppressWarnings("unused")
public CustomHttpEntity handle2x(HttpEntity<?> entity) {
return new CustomHttpEntity();
}
@SuppressWarnings("unused")
public CustomHttpEntity customHttpEntity(HttpEntity<?> entity) {
return new CustomHttpEntity();
}
@SuppressWarnings("unused")
public int handle3() {
return 42;
}
@SuppressWarnings("unused")
public Optional<ResponseEntity> handleOptionalEntity() {
return null;
}
@SuppressWarnings("unused")
@RequestMapping(produces = {"text/html", "application/xhtml+xml"})
public ResponseEntity<String> handle4() {
return null;
}
@SuppressWarnings("unused")
@RequestMapping(produces = {"text/html", "application/xhtml+xml"})
public ResponseEntity<CharSequence> produces() {
return null;
}
@SuppressWarnings("unused")
public ResponseEntity<Resource> resourceResponseEntity() {
return null;
}
@SuppressWarnings("unused")
public Integer integer() {
return 42;
}
@SuppressWarnings("unused")
public ResponseEntity<Resource> handle5() {
return null;
}
@SuppressWarnings("unused")

21
src/docs/asciidoc/web/webflux.adoc
View File

@ -1749,10 +1749,10 @@ controller. The following code snippet shows the usage:
[TIP]
====
Unlike the Servlet API "request paramater" concept that conflate query parameters, form
Unlike the Servlet API "request parameter" concept that conflate query parameters, form
data, and multiparts into one, in WebFlux each is accessed individually through the
`ServerWebExchange`. While `@RequestParam` binds to query parameters only, you can
use data binding to apply query paramerters, form data, and multiparts to a
use data binding to apply query parameters, form data, and multiparts to a
<<webflux-ann-modelattrib-method-args,command object>>.
====
@ -2275,6 +2275,23 @@ on a container object that specifies request headers and body. Below is an examp
}
----
As many return values, `ResponseEntity` can be wrapped by a reactive type such as `Mono`.
In case of a `Mono<ResponseEntity>` return type, the empty `Mono` case will be
automatically converted to a `ResponseEntity` with an empty body and an HTTP 404 status,
so you don't need to chain your `Mono` with an `switchIfEmpty` / `defaultIfEmpty` for simple
HTTP 404 responses.
Here's an example of this:
[source,java,indent=0]
[subs="verbatim,quotes"]
----
@GetMapping("/user")
public Mono<ResponseEntity<User>> fetchUser() {
Mono<User> user = //...
return user.map(ResponseEntity::ok);
}
----
[[webflux-ann-jackson]]
==== Jackson JSON

18
src/docs/asciidoc/web/webmvc.adoc
View File

@ -1816,7 +1816,7 @@ supported for all return values, see below for more details.
| The return value is converted through ``HttpMessageConverter``s and written to the
response. See <<mvc-ann-responsebody>>.
| `HttpEntity<B>`, `ResponseEntity<B>`
| `HttpEntity<B>`, `ResponseEntity<B>`, `Optional<HttpEntity<B>>`, `Optional<ResponseEntity<B>>`
| The return value specifies the full response including HTTP headers and body be converted
through ``HttpMessageConverter``s and written to the response.
See <<mvc-ann-responseentity>>.
@ -2660,6 +2660,22 @@ on a container object that specifies request headers and body. Below is an examp
}
----
In case of a `java.util.Optional<ResponseEntity>` return type, the `Optional.empty()`
case will be automatically converted to a `ResponseEntity` with an empty body
and an HTTP 404 status, so you don't need to chain your optional with
an `orElse` / `orElseGet` for simple HTTP 404 responses.
Here's an example of this:
[source,java,indent=0]
[subs="verbatim,quotes"]
----
@GetMapping("/user")
public Optional<ResponseEntity<User>> fetchUser() {
Optional<User> user = //...
return user.map(ResponseEntity::ok);
}
----
[[mvc-ann-jackson]]
==== Jackson JSON