Skip to content

Commit f2506ca

Browse files
bclozelbclozel
committed
Revert "Infer HTTP 404 from empty Optional/Publisher types"
This reverts commit 7e91733.
1 parent 1b54d21 commit f2506ca

File tree

6 files changed

+86
-155
lines changed

6 files changed

+86
-155
lines changed

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,6 @@ public class ResponseEntityResultHandler extends AbstractMessageWriterResultHand
5555

5656
private static final Set<HttpMethod> SAFE_METHODS = EnumSet.of(HttpMethod.GET, HttpMethod.HEAD);
5757

58-
private static final ResponseEntity<Object> notFound = new ResponseEntity<>(HttpStatus.NOT_FOUND);
59-
6058

6159
/**
6260
* Basic constructor with a default {@link ReactiveAdapterRegistry}.
@@ -121,8 +119,7 @@ public Mono<Void> handleResult(ServerWebExchange exchange, HandlerResult result)
121119

122120
if (adapter != null) {
123121
Assert.isTrue(!adapter.isMultiValue(), "Only a single ResponseEntity supported");
124-
returnValueMono = Mono.from(adapter.toPublisher(result.getReturnValue()))
125-
.defaultIfEmpty(notFound);
122+
returnValueMono = Mono.from(adapter.toPublisher(result.getReturnValue()));
126123
bodyParameter = actualParameter.nested().nested();
127124
}
128125
else {

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

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -350,20 +350,6 @@ public void handleResponseEntityWithExistingResponseHeaders() throws Exception {
350350
assertResponseBodyIsEmpty(exchange);
351351
}
352352

353-
@Test // SPR-13281
354-
public void handleEmptyMonoShouldResultInNotFoundStatus() {
355-
MockServerWebExchange exchange = MockServerWebExchange.from(get("/path"));
356-
exchange.getAttributes().put(PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE, Collections.singleton(APPLICATION_JSON));
357-
358-
MethodParameter type = on(TestController.class).resolveReturnType(Mono.class, ResponseEntity.class);
359-
HandlerResult result = new HandlerResult(new TestController(), Mono.empty(), type);
360-
361-
this.resultHandler.handleResult(exchange, result).block(Duration.ofSeconds(5));
362-
363-
assertEquals(HttpStatus.NOT_FOUND, exchange.getResponse().getStatusCode());
364-
assertResponseBodyIsEmpty(exchange);
365-
}
366-
367353

368354

369355
private void testHandle(Object returnValue, MethodParameter returnType) {

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

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import java.util.EnumSet;
2525
import java.util.List;
2626
import java.util.Map;
27-
import java.util.Optional;
2827
import java.util.Set;
2928
import javax.servlet.http.HttpServletRequest;
3029
import javax.servlet.http.HttpServletResponse;
@@ -34,7 +33,6 @@
3433
import org.springframework.http.HttpEntity;
3534
import org.springframework.http.HttpHeaders;
3635
import org.springframework.http.HttpMethod;
37-
import org.springframework.http.HttpStatus;
3836
import org.springframework.http.RequestEntity;
3937
import org.springframework.http.ResponseEntity;
4038
import org.springframework.http.converter.HttpMessageConverter;
@@ -123,9 +121,8 @@ public boolean supportsParameter(MethodParameter parameter) {
123121

124122
@Override
125123
public boolean supportsReturnType(MethodParameter returnType) {
126-
Class<?> parameterType = returnType.nestedIfOptional().getNestedParameterType();
127-
return (HttpEntity.class.isAssignableFrom(parameterType) &&
128-
!RequestEntity.class.isAssignableFrom(parameterType));
124+
return (HttpEntity.class.isAssignableFrom(returnType.getParameterType()) &&
125+
!RequestEntity.class.isAssignableFrom(returnType.getParameterType()));
129126
}
130127

131128
@Override
@@ -153,7 +150,7 @@ public Object resolveArgument(MethodParameter parameter, @Nullable ModelAndViewC
153150

154151
@Nullable
155152
private Type getHttpEntityType(MethodParameter parameter) {
156-
Assert.isAssignable(HttpEntity.class, parameter.getNestedParameterType());
153+
Assert.isAssignable(HttpEntity.class, parameter.getParameterType());
157154
Type parameterType = parameter.getGenericParameterType();
158155
if (parameterType instanceof ParameterizedType) {
159156
ParameterizedType type = (ParameterizedType) parameterType;
@@ -172,7 +169,6 @@ else if (parameterType instanceof Class) {
172169
}
173170

174171
@Override
175-
@SuppressWarnings("unchecked")
176172
public void handleReturnValue(@Nullable Object returnValue, MethodParameter returnType,
177173
ModelAndViewContainer mavContainer, NativeWebRequest webRequest) throws Exception {
178174

@@ -184,12 +180,6 @@ public void handleReturnValue(@Nullable Object returnValue, MethodParameter retu
184180
ServletServerHttpRequest inputMessage = createInputMessage(webRequest);
185181
ServletServerHttpResponse outputMessage = createOutputMessage(webRequest);
186182

187-
if (returnType.getParameterType() == Optional.class) {
188-
Optional<HttpEntity<?>> optionalEntity = (Optional<HttpEntity<?>>) returnValue;
189-
returnValue = optionalEntity.orElseGet(() -> new ResponseEntity<>(HttpStatus.NOT_FOUND));
190-
returnType = returnType.nestedIfOptional();
191-
}
192-
193183
Assert.isInstanceOf(HttpEntity.class, returnValue);
194184
HttpEntity<?> responseEntity = (HttpEntity<?>) returnValue;
195185

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

Lines changed: 76 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.io.ByteArrayInputStream;
2020
import java.io.IOException;
2121
import java.io.InputStream;
22+
import java.lang.reflect.Method;
2223
import java.net.URI;
2324
import java.nio.charset.StandardCharsets;
2425
import java.time.ZoneId;
@@ -27,7 +28,6 @@
2728
import java.util.Arrays;
2829
import java.util.Collections;
2930
import java.util.Date;
30-
import java.util.Optional;
3131

3232
import org.hamcrest.Matchers;
3333
import org.junit.Before;
@@ -56,17 +56,14 @@
5656
import org.springframework.web.HttpMediaTypeNotSupportedException;
5757
import org.springframework.web.bind.annotation.RequestMapping;
5858
import org.springframework.web.context.request.ServletWebRequest;
59-
import org.springframework.web.method.ResolvableMethod;
6059
import org.springframework.web.method.support.ModelAndViewContainer;
6160

62-
import static java.time.Instant.ofEpochMilli;
63-
import static java.time.format.DateTimeFormatter.RFC_1123_DATE_TIME;
61+
import static java.time.Instant.*;
62+
import static java.time.format.DateTimeFormatter.*;
6463
import static org.junit.Assert.*;
6564
import static org.mockito.BDDMockito.*;
66-
import static org.springframework.http.MediaType.APPLICATION_OCTET_STREAM;
67-
import static org.springframework.http.MediaType.TEXT_PLAIN;
68-
import static org.springframework.web.method.ResolvableMethod.on;
69-
import static org.springframework.web.servlet.HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE;
65+
import static org.springframework.http.MediaType.*;
66+
import static org.springframework.web.servlet.HandlerMapping.*;
7067

7168
/**
7269
* Test fixture for {@link HttpEntityMethodProcessor} delegating to a mock
@@ -94,6 +91,26 @@ public class HttpEntityMethodProcessorMockTests {
9491

9592
private HttpMessageConverter<Object> resourceRegionMessageConverter;
9693

94+
private MethodParameter paramHttpEntity;
95+
96+
private MethodParameter paramRequestEntity;
97+
98+
private MethodParameter paramResponseEntity;
99+
100+
private MethodParameter paramInt;
101+
102+
private MethodParameter returnTypeResponseEntity;
103+
104+
private MethodParameter returnTypeResponseEntityProduces;
105+
106+
private MethodParameter returnTypeResponseEntityResource;
107+
108+
private MethodParameter returnTypeHttpEntity;
109+
110+
private MethodParameter returnTypeHttpEntitySubclass;
111+
112+
private MethodParameter returnTypeInt;
113+
97114
private ModelAndViewContainer mavContainer;
98115

99116
private MockHttpServletRequest servletRequest;
@@ -102,12 +119,6 @@ public class HttpEntityMethodProcessorMockTests {
102119

103120
private ServletWebRequest webRequest;
104121

105-
private MethodParameter returnTypeResponseEntity =
106-
on(TestHandler.class).resolveReturnType(ResponseEntity.class, String.class);
107-
108-
private MethodParameter returnTypeResponseEntityResource =
109-
on(TestHandler.class).resolveReturnType(ResponseEntity.class, Resource.class);
110-
111122

112123
@Before
113124
@SuppressWarnings("unchecked")
@@ -128,6 +139,20 @@ public void setup() throws Exception {
128139
processor = new HttpEntityMethodProcessor(Arrays.asList(
129140
stringHttpMessageConverter, resourceMessageConverter, resourceRegionMessageConverter));
130141

142+
Method handle1 = getClass().getMethod("handle1", HttpEntity.class, ResponseEntity.class,
143+
Integer.TYPE, RequestEntity.class);
144+
145+
paramHttpEntity = new MethodParameter(handle1, 0);
146+
paramRequestEntity = new MethodParameter(handle1, 3);
147+
paramResponseEntity = new MethodParameter(handle1, 1);
148+
paramInt = new MethodParameter(handle1, 2);
149+
returnTypeResponseEntity = new MethodParameter(handle1, -1);
150+
returnTypeResponseEntityProduces = new MethodParameter(getClass().getMethod("handle4"), -1);
151+
returnTypeHttpEntity = new MethodParameter(getClass().getMethod("handle2", HttpEntity.class), -1);
152+
returnTypeHttpEntitySubclass = new MethodParameter(getClass().getMethod("handle2x", HttpEntity.class), -1);
153+
returnTypeInt = new MethodParameter(getClass().getMethod("handle3"), -1);
154+
returnTypeResponseEntityResource = new MethodParameter(getClass().getMethod("handle5"), -1);
155+
131156
mavContainer = new ModelAndViewContainer();
132157
servletRequest = new MockHttpServletRequest("GET", "/foo");
133158
servletResponse = new MockHttpServletResponse();
@@ -137,39 +162,25 @@ public void setup() throws Exception {
137162

138163
@Test
139164
public void supportsParameter() {
140-
ResolvableMethod method = on(TestHandler.class).named("entityParams").build();
141-
assertTrue("HttpEntity parameter not supported",
142-
processor.supportsParameter(method.arg(HttpEntity.class, String.class)));
143-
assertTrue("RequestEntity parameter not supported",
144-
processor.supportsParameter(method.arg(RequestEntity.class, String.class)));
145-
assertFalse("ResponseEntity parameter supported",
146-
processor.supportsParameter(method.arg(ResponseEntity.class, String.class)));
147-
assertFalse("non-entity parameter supported",
148-
processor.supportsParameter(method.arg(Integer.class)));
165+
assertTrue("HttpEntity parameter not supported", processor.supportsParameter(paramHttpEntity));
166+
assertTrue("RequestEntity parameter not supported", processor.supportsParameter(paramRequestEntity));
167+
assertFalse("ResponseEntity parameter supported", processor.supportsParameter(paramResponseEntity));
168+
assertFalse("non-entity parameter supported", processor.supportsParameter(paramInt));
149169
}
150170

151171
@Test
152172
public void supportsReturnType() {
153-
assertTrue("ResponseEntity return type not supported",
154-
processor.supportsReturnType(on(TestHandler.class).resolveReturnType(ResponseEntity.class, String.class)));
155-
assertTrue("HttpEntity return type not supported",
156-
processor.supportsReturnType(on(TestHandler.class).resolveReturnType(HttpEntity.class)));
157-
assertTrue("Custom HttpEntity subclass not supported",
158-
processor.supportsReturnType(on(TestHandler.class).resolveReturnType(CustomHttpEntity.class)));
159-
assertTrue("Optional ResponseEntity not supported",
160-
processor.supportsReturnType(on(TestHandler.class).resolveReturnType(Optional.class, ResponseEntity.class)));
161-
assertFalse("RequestEntity return type supported",
162-
processor.supportsReturnType(on(TestHandler.class)
163-
.named("entityParams").build().arg(RequestEntity.class, String.class)));
164-
assertFalse("non-ResponseBody return type supported",
165-
processor.supportsReturnType(on(TestHandler.class).resolveReturnType(Integer.class)));
173+
assertTrue("ResponseEntity return type not supported", processor.supportsReturnType(returnTypeResponseEntity));
174+
assertTrue("HttpEntity return type not supported", processor.supportsReturnType(returnTypeHttpEntity));
175+
assertTrue("Custom HttpEntity subclass not supported", processor.supportsReturnType(returnTypeHttpEntitySubclass));
176+
assertFalse("RequestEntity parameter supported",
177+
processor.supportsReturnType(paramRequestEntity));
178+
assertFalse("non-ResponseBody return type supported", processor.supportsReturnType(returnTypeInt));
166179
}
167180

168181
@Test
169182
public void shouldResolveHttpEntityArgument() throws Exception {
170183
String body = "Foo";
171-
MethodParameter paramHttpEntity = on(TestHandler.class).named("entityParams")
172-
.build().arg(HttpEntity.class, String.class);
173184

174185
MediaType contentType = TEXT_PLAIN;
175186
servletRequest.addHeader("Content-Type", contentType.toString());
@@ -188,8 +199,6 @@ public void shouldResolveHttpEntityArgument() throws Exception {
188199
@Test
189200
public void shouldResolveRequestEntityArgument() throws Exception {
190201
String body = "Foo";
191-
MethodParameter paramRequestEntity = on(TestHandler.class).named("entityParams")
192-
.build().arg(RequestEntity.class, String.class);
193202

194203
MediaType contentType = TEXT_PLAIN;
195204
servletRequest.addHeader("Content-Type", contentType.toString());
@@ -216,8 +225,6 @@ public void shouldResolveRequestEntityArgument() throws Exception {
216225

217226
@Test
218227
public void shouldFailResolvingWhenConverterCannotRead() throws Exception {
219-
MethodParameter paramHttpEntity = on(TestHandler.class).named("entityParams")
220-
.build().arg(HttpEntity.class, String.class);
221228
MediaType contentType = TEXT_PLAIN;
222229
servletRequest.setMethod("POST");
223230
servletRequest.addHeader("Content-Type", contentType.toString());
@@ -231,8 +238,6 @@ public void shouldFailResolvingWhenConverterCannotRead() throws Exception {
231238

232239
@Test
233240
public void shouldFailResolvingWhenContentTypeNotSupported() throws Exception {
234-
MethodParameter paramHttpEntity = on(TestHandler.class).named("entityParams")
235-
.build().arg(HttpEntity.class, String.class);
236241
servletRequest.setMethod("POST");
237242
servletRequest.setContent("some content".getBytes(StandardCharsets.UTF_8));
238243
this.thrown.expect(HttpMediaTypeNotSupportedException.class);
@@ -255,14 +260,13 @@ public void shouldHandleReturnValue() throws Exception {
255260

256261
@Test
257262
public void shouldHandleReturnValueWithProducibleMediaType() throws Exception {
258-
MethodParameter returnType = on(TestHandler.class).resolveReturnType(ResponseEntity.class, CharSequence.class);
259263
String body = "Foo";
260264
ResponseEntity<String> returnValue = new ResponseEntity<>(body, HttpStatus.OK);
261265
servletRequest.addHeader("Accept", "text/*");
262266
servletRequest.setAttribute(PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE, Collections.singleton(MediaType.TEXT_HTML));
263267
given(stringHttpMessageConverter.canWrite(String.class, MediaType.TEXT_HTML)).willReturn(true);
264268

265-
processor.handleReturnValue(returnValue, returnType, mavContainer, webRequest);
269+
processor.handleReturnValue(returnValue, returnTypeResponseEntityProduces, mavContainer, webRequest);
266270

267271
assertTrue(mavContainer.isRequestHandled());
268272
verify(stringHttpMessageConverter).write(eq(body), eq(MediaType.TEXT_HTML), isA(HttpOutputMessage.class));
@@ -307,7 +311,6 @@ public void shouldFailHandlingWhenContentTypeNotSupported() throws Exception {
307311

308312
@Test
309313
public void shouldFailHandlingWhenConverterCannotWrite() throws Exception {
310-
MethodParameter returnType = on(TestHandler.class).resolveReturnType(ResponseEntity.class, CharSequence.class);
311314
String body = "Foo";
312315
ResponseEntity<String> returnValue = new ResponseEntity<>(body, HttpStatus.OK);
313316
MediaType accepted = TEXT_PLAIN;
@@ -319,7 +322,7 @@ public void shouldFailHandlingWhenConverterCannotWrite() throws Exception {
319322
given(stringHttpMessageConverter.canWrite(String.class, accepted)).willReturn(false);
320323

321324
this.thrown.expect(HttpMediaTypeNotAcceptableException.class);
322-
processor.handleReturnValue(returnValue, returnType, mavContainer, webRequest);
325+
processor.handleReturnValue(returnValue, returnTypeResponseEntityProduces, mavContainer, webRequest);
323326
}
324327

325328
@Test // SPR-9142
@@ -358,16 +361,6 @@ public void shouldHandleResponseHeaderAndBody() throws Exception {
358361
assertEquals("headerValue", outputMessage.getValue().getHeaders().get("header").get(0));
359362
}
360363

361-
@Test // SPR-13281
362-
public void shouldReturnNotFoundWhenEmptyOptional() throws Exception {
363-
MethodParameter returnType = on(TestHandler.class).resolveReturnType(Optional.class, ResponseEntity.class);
364-
Optional<ResponseEntity> returnValue = Optional.empty();
365-
366-
processor.handleReturnValue(returnValue, returnType, mavContainer, webRequest);
367-
assertTrue(mavContainer.isRequestHandled());
368-
assertEquals(HttpStatus.NOT_FOUND.value(), servletResponse.getStatus());
369-
}
370-
371364
@Test
372365
public void shouldHandleLastModifiedWithHttp304() throws Exception {
373366
long currentTime = new Date().getTime();
@@ -704,46 +697,38 @@ private void assertConditionalResponse(HttpStatus status, String body, String et
704697
}
705698
}
706699

707-
private static class TestHandler {
708-
709-
@SuppressWarnings("unused")
710-
public ResponseEntity<String> entityParams(HttpEntity<String> httpEntity, ResponseEntity<String> entity,
711-
Integer i, RequestEntity<String> requestEntity) {
712700

713-
return entity;
714-
}
715-
716-
@SuppressWarnings("unused")
717-
public HttpEntity<?> httpEntity(HttpEntity<?> entity) {
718-
return entity;
719-
}
701+
@SuppressWarnings("unused")
702+
public ResponseEntity<String> handle1(HttpEntity<String> httpEntity, ResponseEntity<String> entity,
703+
int i, RequestEntity<String> requestEntity) {
720704

721-
@SuppressWarnings("unused")
722-
public CustomHttpEntity customHttpEntity(HttpEntity<?> entity) {
723-
return new CustomHttpEntity();
724-
}
705+
return entity;
706+
}
725707

726-
@SuppressWarnings("unused")
727-
public Optional<ResponseEntity> handleOptionalEntity() {
728-
return null;
729-
}
708+
@SuppressWarnings("unused")
709+
public HttpEntity<?> handle2(HttpEntity<?> entity) {
710+
return entity;
711+
}
730712

731-
@SuppressWarnings("unused")
732-
@RequestMapping(produces = {"text/html", "application/xhtml+xml"})
733-
public ResponseEntity<CharSequence> produces() {
734-
return null;
735-
}
713+
@SuppressWarnings("unused")
714+
public CustomHttpEntity handle2x(HttpEntity<?> entity) {
715+
return new CustomHttpEntity();
716+
}
736717

737-
@SuppressWarnings("unused")
738-
public ResponseEntity<Resource> resourceResponseEntity() {
739-
return null;
740-
}
718+
@SuppressWarnings("unused")
719+
public int handle3() {
720+
return 42;
721+
}
741722

742-
@SuppressWarnings("unused")
743-
public Integer integer() {
744-
return 42;
745-
}
723+
@SuppressWarnings("unused")
724+
@RequestMapping(produces = {"text/html", "application/xhtml+xml"})
725+
public ResponseEntity<String> handle4() {
726+
return null;
727+
}
746728

729+
@SuppressWarnings("unused")
730+
public ResponseEntity<Resource> handle5() {
731+
return null;
747732
}
748733

749734
@SuppressWarnings("unused")

0 commit comments

Comments
 (0)