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

spring-web/src/main/java/org/springframework/http/converter/AbstractGenericHttpMessageConverter.java

@@ -58,8 +58,13 @@ public abstract class AbstractGenericHttpMessageConverter<T> 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);
 	}
 
 	/**

spring-web/src/main/java/org/springframework/http/converter/GenericHttpMessageConverter.java

@@ -38,6 +38,9 @@ public interface GenericHttpMessageConverter<T> extends HttpMessageConverter<T>
 
 	/**
 	 * 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<T> extends HttpMessageConverter<T>
 
 	/**
 	 * 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.

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));
 		}

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);
 	}
 

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<MediaType> requestedMediaTypes = getAcceptableMediaTypes(servletRequest);
-		List<MediaType> producibleMediaTypes = getProducibleMediaTypes(servletRequest, returnValueClass);
+		List<MediaType> 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<T>) messageConverter).canWrite(returnValueType,
+							returnValueClass, selectedMediaType)) {
+						returnValue = (T) getAdvice().beforeBodyWrite(returnValue, returnType, selectedMediaType,
+								(Class<? extends HttpMessageConverter<?>>) messageConverter.getClass(),
+								inputMessage, outputMessage);
+						if (returnValue != null) {
+							((GenericHttpMessageConverter<T>) 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<? extends HttpMessageConverter<?>>) 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<T>) messageConverter).write(returnValue, type, selectedMediaType, outputMessage);
-						}
-						else {
-							((HttpMessageConverter<T>) messageConverter).write(returnValue, selectedMediaType, outputMessage);
-						}
+						((HttpMessageConverter<T>) 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<MediaType> getProducibleMediaTypes(HttpServletRequest request, Class<?> returnValueClass) {
+		return getProducibleMediaTypes(request, returnValueClass, null);
+	}
+
 	/**
 	 * Returns the media types that can be produced:
 	 * <ul>
@@ -207,9 +237,10 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe
 	 * <li>Media types of configured converters that can write the specific return value, or
 	 * <li>{@link MediaType#ALL}
 	 * </ul>
+	 * @since 4.2
 	 */
 	@SuppressWarnings("unchecked")
-	protected List<MediaType> getProducibleMediaTypes(HttpServletRequest request, Class<?> returnValueClass) {
+	protected List<MediaType> getProducibleMediaTypes(HttpServletRequest request, Class<?> returnValueClass, Type returnValueType) {
 		Set<MediaType> mediaTypes = (Set<MediaType>) request.getAttribute(HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE);
 		if (!CollectionUtils.isEmpty(mediaTypes)) {
 			return new ArrayList<MediaType>(mediaTypes);
@@ -217,7 +248,12 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe
 		else if (!this.allSupportedMediaTypes.isEmpty()) {
 			List<MediaType> result = new ArrayList<MediaType>();
 			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());
 				}
 			}