From 289f35da3a57bb5e491b30c7351072b4e801c519 Mon Sep 17 00:00:00 2001 From: Sebastien Deleuze Date: Thu, 25 Jun 2015 15:02:33 +0200 Subject: [PATCH] Call type aware canWrite() when using a GenericHttpMessageConverter This commit introduces the following changes: - In AbstractMessageConverterMethodProcessor, the type aware variant of canWrite() is now called when the converter implements GenericHttpMessageConverter. - The Javadoc has been updated in GenericHttpMessageConverter to make it clear that the type aware canRead() and canWrite() methods should perform the same checks than non type aware ones. - AbstractGenericHttpMessageConverter now implements default type aware canRead() and canWrite() methods than just call the non type aware variants. Due to this, if subclasses just override the non type aware variants, they still have the right behavior. Issue: SPR-13161 --- .../AbstractGenericHttpMessageConverter.java | 9 ++- .../GenericHttpMessageConverter.java | 6 ++ .../AbstractJackson2HttpMessageConverter.java | 2 +- .../json/GsonHttpMessageConverter.java | 7 +- ...stractMessageConverterMethodProcessor.java | 72 ++++++++++++++----- 5 files changed, 69 insertions(+), 27 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/converter/AbstractGenericHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/AbstractGenericHttpMessageConverter.java index ec0e8e1b81..1a0d4dd13d 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/AbstractGenericHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/AbstractGenericHttpMessageConverter.java @@ -58,8 +58,13 @@ public abstract class AbstractGenericHttpMessageConverter extends AbstractHtt } @Override - public boolean canWrite(Class contextClass, MediaType mediaType) { - return canWrite(null, contextClass, mediaType); + public boolean canRead(Type type, Class contextClass, MediaType mediaType) { + return canRead(contextClass, mediaType); + } + + @Override + public boolean canWrite(Type type, Class contextClass, MediaType mediaType) { + return canWrite(contextClass, mediaType); } /** diff --git a/spring-web/src/main/java/org/springframework/http/converter/GenericHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/GenericHttpMessageConverter.java index 56de4dac71..f3b001ca27 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/GenericHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/GenericHttpMessageConverter.java @@ -38,6 +38,9 @@ public interface GenericHttpMessageConverter extends HttpMessageConverter /** * Indicates whether the given type can be read by this converter. + * This method should perform the same checks than + * {@link HttpMessageConverter#canRead(Class, MediaType)} with additional ones + * related to the generic type. * @param type the type to test for readability * @param contextClass a context class for the target type, for example a class * in which the target type appears in a method signature (can be {@code null}) @@ -64,6 +67,9 @@ public interface GenericHttpMessageConverter extends HttpMessageConverter /** * Indicates whether the given class can be written by this converter. + * This method should perform the same checks than + * {@link HttpMessageConverter#canWrite(Class, MediaType)} with additional ones + * related to the generic type. * @param type the type to test for writability, can be {@code null} if not specified. * @param contextClass the class to test for writability * @param mediaType the media type to write, can be {@code null} if not specified. diff --git a/spring-web/src/main/java/org/springframework/http/converter/json/AbstractJackson2HttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/json/AbstractJackson2HttpMessageConverter.java index 0086dfd5a8..de9e2edfa8 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/json/AbstractJackson2HttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/json/AbstractJackson2HttpMessageConverter.java @@ -160,7 +160,7 @@ public abstract class AbstractJackson2HttpMessageConverter extends AbstractGener } @Override - public boolean canWrite(Type type, Class clazz, MediaType mediaType) { + public boolean canWrite(Class clazz, MediaType mediaType) { if (!jackson23Available || !logger.isWarnEnabled()) { return (this.objectMapper.canSerialize(clazz) && canWrite(mediaType)); } diff --git a/spring-web/src/main/java/org/springframework/http/converter/json/GsonHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/json/GsonHttpMessageConverter.java index e69c29ec52..1e795fb30f 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/json/GsonHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/json/GsonHttpMessageConverter.java @@ -120,12 +120,7 @@ public class GsonHttpMessageConverter extends AbstractGenericHttpMessageConverte } @Override - public boolean canRead(Type type, Class contextClass, MediaType mediaType) { - return canRead(mediaType); - } - - @Override - public boolean canWrite(Type type, Class clazz, MediaType mediaType) { + public boolean canWrite(Class clazz, MediaType mediaType) { return canWrite(mediaType); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java index 9ef5982259..697457f542 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java @@ -116,9 +116,10 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe throws IOException, HttpMediaTypeNotAcceptableException { Class returnValueClass = getReturnValueType(returnValue, returnType); + Type returnValueType = getGenericType(returnType); HttpServletRequest servletRequest = inputMessage.getServletRequest(); List requestedMediaTypes = getAcceptableMediaTypes(servletRequest); - List producibleMediaTypes = getProducibleMediaTypes(servletRequest, returnValueClass); + List producibleMediaTypes = getProducibleMediaTypes(servletRequest, returnValueClass, returnValueType); Assert.isTrue(returnValue == null || !producibleMediaTypes.isEmpty(), "No converter found for return value of type: " + returnValueClass); @@ -156,25 +157,30 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe if (selectedMediaType != null) { selectedMediaType = selectedMediaType.removeQualityValue(); for (HttpMessageConverter messageConverter : this.messageConverters) { - if (messageConverter.canWrite(returnValueClass, selectedMediaType)) { + if (messageConverter instanceof GenericHttpMessageConverter) { + if (((GenericHttpMessageConverter) messageConverter).canWrite(returnValueType, + returnValueClass, selectedMediaType)) { + returnValue = (T) getAdvice().beforeBodyWrite(returnValue, returnType, selectedMediaType, + (Class>) messageConverter.getClass(), + inputMessage, outputMessage); + if (returnValue != null) { + ((GenericHttpMessageConverter) messageConverter).write(returnValue, + returnValueType, selectedMediaType, outputMessage); + if (logger.isDebugEnabled()) { + logger.debug("Written [" + returnValue + "] as \"" + + selectedMediaType + "\" using [" + messageConverter + "]"); + } + } + return; + } + } + else if (messageConverter.canWrite(returnValueClass, selectedMediaType)) { returnValue = (T) getAdvice().beforeBodyWrite(returnValue, returnType, selectedMediaType, (Class>) messageConverter.getClass(), inputMessage, outputMessage); if (returnValue != null) { - if (messageConverter instanceof GenericHttpMessageConverter) { - Type type; - if (HttpEntity.class.isAssignableFrom(returnType.getParameterType())) { - returnType.increaseNestingLevel(); - type = returnType.getNestedGenericParameterType(); - } - else { - type = returnType.getGenericParameterType(); - } - ((GenericHttpMessageConverter) messageConverter).write(returnValue, type, selectedMediaType, outputMessage); - } - else { - ((HttpMessageConverter) messageConverter).write(returnValue, selectedMediaType, outputMessage); - } + ((HttpMessageConverter) messageConverter).write(returnValue, + selectedMediaType, outputMessage); if (logger.isDebugEnabled()) { logger.debug("Written [" + returnValue + "] as \"" + selectedMediaType + "\" using [" + messageConverter + "]"); @@ -200,6 +206,30 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe return (returnValue != null ? returnValue.getClass() : returnType.getParameterType()); } + /** + * Return the generic type of the {@code returnType} (or of the nested type if it is + * a {@link HttpEntity}). + */ + private Type getGenericType(MethodParameter returnType) { + Type type; + if (HttpEntity.class.isAssignableFrom(returnType.getParameterType())) { + returnType.increaseNestingLevel(); + type = returnType.getNestedGenericParameterType(); + } + else { + type = returnType.getGenericParameterType(); + } + return type; + } + + /** + * @see #getProducibleMediaTypes(HttpServletRequest, Class, Type) + */ + @SuppressWarnings("unchecked") + protected List getProducibleMediaTypes(HttpServletRequest request, Class returnValueClass) { + return getProducibleMediaTypes(request, returnValueClass, null); + } + /** * Returns the media types that can be produced: *
    @@ -207,9 +237,10 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe *
  • Media types of configured converters that can write the specific return value, or *
  • {@link MediaType#ALL} *
+ * @since 4.2 */ @SuppressWarnings("unchecked") - protected List getProducibleMediaTypes(HttpServletRequest request, Class returnValueClass) { + protected List getProducibleMediaTypes(HttpServletRequest request, Class returnValueClass, Type returnValueType) { Set mediaTypes = (Set) request.getAttribute(HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE); if (!CollectionUtils.isEmpty(mediaTypes)) { return new ArrayList(mediaTypes); @@ -217,7 +248,12 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe else if (!this.allSupportedMediaTypes.isEmpty()) { List result = new ArrayList(); for (HttpMessageConverter converter : this.messageConverters) { - if (converter.canWrite(returnValueClass, null)) { + if (converter instanceof GenericHttpMessageConverter && returnValueType != null) { + if (((GenericHttpMessageConverter) converter).canWrite(returnValueType, returnValueClass, null)) { + result.addAll(converter.getSupportedMediaTypes()); + } + } + else if (converter.canWrite(returnValueClass, null)) { result.addAll(converter.getSupportedMediaTypes()); } }