Remove HttpStatus from HttpMessageConversionException
HttpMessageConverter's are client and server and arguably shouldn't contain a server-side concept such a response status. The status field is recent, it was added to differentiate 400 vs 500 errors with Jackson 2.9+ but there is no need for it since the same distinction is reflected in raising an HttpMessageNotReadableException vs a general HttpMessageConversionException. Issue: SPR-15516
This commit is contained in:
@@ -17,9 +17,6 @@
|
||||
package org.springframework.http.converter;
|
||||
|
||||
import org.springframework.core.NestedRuntimeException;
|
||||
import org.springframework.http.HttpStatus;
|
||||
|
||||
import java.util.Optional;
|
||||
|
||||
/**
|
||||
* Thrown by {@link HttpMessageConverter} implementations when a conversion attempt fails.
|
||||
@@ -31,15 +28,12 @@ import java.util.Optional;
|
||||
@SuppressWarnings("serial")
|
||||
public class HttpMessageConversionException extends NestedRuntimeException {
|
||||
|
||||
private final HttpStatus errorStatus;
|
||||
|
||||
/**
|
||||
* Create a new HttpMessageConversionException.
|
||||
* @param msg the detail message
|
||||
*/
|
||||
public HttpMessageConversionException(String msg) {
|
||||
super(msg);
|
||||
this.errorStatus = null;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -49,25 +43,6 @@ public class HttpMessageConversionException extends NestedRuntimeException {
|
||||
*/
|
||||
public HttpMessageConversionException(String msg, Throwable cause) {
|
||||
super(msg, cause);
|
||||
this.errorStatus = null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a new HttpMessageConversionException.
|
||||
* @since 5.0
|
||||
* @param msg the detail message
|
||||
* @param cause the root cause (if any)
|
||||
* @param errorStatus the HTTP error status related to this exception
|
||||
*/
|
||||
public HttpMessageConversionException(String msg, Throwable cause, HttpStatus errorStatus) {
|
||||
super(msg, cause);
|
||||
this.errorStatus = errorStatus;
|
||||
}
|
||||
|
||||
/**
|
||||
* Return the HTTP error status related to this exception if any.
|
||||
*/
|
||||
public Optional<HttpStatus> getErrorStatus() {
|
||||
return Optional.ofNullable(errorStatus);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -16,8 +16,6 @@
|
||||
|
||||
package org.springframework.http.converter;
|
||||
|
||||
import org.springframework.http.HttpStatus;
|
||||
|
||||
/**
|
||||
* Thrown by {@link HttpMessageConverter} implementations when the
|
||||
* {@link HttpMessageConverter#read} method fails.
|
||||
@@ -45,15 +43,4 @@ public class HttpMessageNotReadableException extends HttpMessageConversionExcept
|
||||
super(msg, cause);
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a new HttpMessageNotReadableException.
|
||||
* @since 5.0
|
||||
* @param msg the detail message
|
||||
* @param cause the root cause (if any)
|
||||
* @param errorStatus the HTTP error status related to this exception
|
||||
*/
|
||||
public HttpMessageNotReadableException(String msg, Throwable cause, HttpStatus errorStatus) {
|
||||
super(msg, cause, errorStatus);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -41,9 +41,9 @@ import com.fasterxml.jackson.databind.type.TypeFactory;
|
||||
import org.springframework.core.GenericTypeResolver;
|
||||
import org.springframework.http.HttpInputMessage;
|
||||
import org.springframework.http.HttpOutputMessage;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.http.MediaType;
|
||||
import org.springframework.http.converter.AbstractGenericHttpMessageConverter;
|
||||
import org.springframework.http.converter.HttpMessageConversionException;
|
||||
import org.springframework.http.converter.HttpMessageConverter;
|
||||
import org.springframework.http.converter.HttpMessageNotReadableException;
|
||||
import org.springframework.http.converter.HttpMessageNotWritableException;
|
||||
@@ -230,11 +230,13 @@ public abstract class AbstractJackson2HttpMessageConverter extends AbstractGener
|
||||
return this.objectMapper.readValue(inputMessage.getBody(), javaType);
|
||||
}
|
||||
catch (InvalidDefinitionException ex) {
|
||||
throw new HttpMessageNotReadableException(
|
||||
"Could not map JSON to target object of " + javaType, ex, HttpStatus.INTERNAL_SERVER_ERROR);
|
||||
throw new HttpMessageConversionException("Type definition error: " + ex.getMessage(), ex);
|
||||
}
|
||||
catch (JsonProcessingException ex) {
|
||||
throw new HttpMessageNotReadableException("JSON parse error: " + ex.getMessage(), ex);
|
||||
}
|
||||
catch (IOException ex) {
|
||||
throw new HttpMessageNotReadableException("Could not read JSON document: " + ex.getMessage(), ex);
|
||||
throw new HttpMessageNotReadableException("I/O error while reading: " + ex.getMessage(), ex);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -34,14 +34,21 @@ import com.fasterxml.jackson.databind.ser.impl.SimpleFilterProvider;
|
||||
import org.junit.Test;
|
||||
|
||||
import org.springframework.core.ParameterizedTypeReference;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.http.MediaType;
|
||||
import org.springframework.http.MockHttpInputMessage;
|
||||
import org.springframework.http.MockHttpOutputMessage;
|
||||
import org.springframework.http.converter.HttpMessageConversionException;
|
||||
import org.springframework.http.converter.HttpMessageNotReadableException;
|
||||
|
||||
import static org.hamcrest.CoreMatchers.*;
|
||||
import static org.junit.Assert.*;
|
||||
import static org.hamcrest.CoreMatchers.containsString;
|
||||
import static org.hamcrest.CoreMatchers.endsWith;
|
||||
import static org.hamcrest.CoreMatchers.not;
|
||||
import static org.hamcrest.CoreMatchers.startsWith;
|
||||
import static org.junit.Assert.assertArrayEquals;
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertThat;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.junit.Assert.fail;
|
||||
|
||||
/**
|
||||
* Jackson 2.x converter tests.
|
||||
@@ -388,9 +395,8 @@ public class MappingJackson2HttpMessageConverterTests {
|
||||
try {
|
||||
converter.read(BeanWithNoDefaultConstructor.class, inputMessage);
|
||||
}
|
||||
catch (HttpMessageNotReadableException ex) {
|
||||
assertTrue(ex.getErrorStatus().isPresent());
|
||||
assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, ex.getErrorStatus().get());
|
||||
catch (HttpMessageConversionException ex) {
|
||||
assertTrue(ex.getMessage(), ex.getMessage().startsWith("Type definition error:"));
|
||||
return;
|
||||
}
|
||||
fail();
|
||||
|
||||
@@ -27,7 +27,6 @@ import org.apache.commons.logging.LogFactory;
|
||||
import org.springframework.beans.ConversionNotSupportedException;
|
||||
import org.springframework.beans.TypeMismatchException;
|
||||
import org.springframework.core.Ordered;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.http.MediaType;
|
||||
import org.springframework.http.converter.HttpMessageNotReadableException;
|
||||
import org.springframework.http.converter.HttpMessageNotWritableException;
|
||||
@@ -355,7 +354,7 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes
|
||||
if (logger.isWarnEnabled()) {
|
||||
logger.warn("Failed to read HTTP message: " + ex);
|
||||
}
|
||||
response.sendError(ex.getErrorStatus().orElse(HttpStatus.BAD_REQUEST).value());
|
||||
response.sendError(HttpServletResponse.SC_BAD_REQUEST);
|
||||
return new ModelAndView();
|
||||
}
|
||||
|
||||
|
||||
@@ -141,7 +141,14 @@ import org.springframework.web.servlet.mvc.support.RedirectAttributes;
|
||||
import org.springframework.web.servlet.support.RequestContextUtils;
|
||||
import org.springframework.web.servlet.view.InternalResourceViewResolver;
|
||||
|
||||
import static org.junit.Assert.*;
|
||||
import static org.junit.Assert.assertArrayEquals;
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertFalse;
|
||||
import static org.junit.Assert.assertNotNull;
|
||||
import static org.junit.Assert.assertNull;
|
||||
import static org.junit.Assert.assertSame;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.junit.Assert.fail;
|
||||
|
||||
/**
|
||||
* @author Rossen Stoyanchev
|
||||
|
||||
Reference in New Issue
Block a user