From b204437cef0976f5af0e1c5290e77e266b306a51 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Sat, 2 Jul 2016 14:48:15 +0200 Subject: [PATCH] Polishing --- .../AbstractMethodMessageHandler.java | 56 ++++++---- .../htmlunit/HtmlUnitRequestBuilder.java | 17 +-- ...stractMessageConverterMethodProcessor.java | 104 +++++++++--------- 3 files changed, 96 insertions(+), 81 deletions(-) diff --git a/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/AbstractMethodMessageHandler.java b/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/AbstractMethodMessageHandler.java index 3160c7b7b6..d9a1f52c5c 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/AbstractMethodMessageHandler.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/AbstractMethodMessageHandler.java @@ -83,9 +83,17 @@ public abstract class AbstractMethodMessageHandler protected final Log logger = LogFactory.getLog(getClass()); - private final List customArgumentResolvers = new ArrayList(4); + private final List customArgumentResolvers = + new ArrayList(4); - private final List customReturnValueHandlers = new ArrayList(4); + private final List customReturnValueHandlers = + new ArrayList(4); + + private final HandlerMethodArgumentResolverComposite argumentResolvers = + new HandlerMethodArgumentResolverComposite(); + + private final HandlerMethodReturnValueHandlerComposite returnValueHandlers = + new HandlerMethodReturnValueHandlerComposite(); private final Map handlerMethods = new LinkedHashMap(); @@ -99,10 +107,6 @@ public abstract class AbstractMethodMessageHandler private Collection destinationPrefixes = new ArrayList(); - private HandlerMethodArgumentResolverComposite argumentResolvers = new HandlerMethodArgumentResolverComposite(); - - private HandlerMethodReturnValueHandlerComposite returnValueHandlers =new HandlerMethodReturnValueHandlerComposite(); - private ApplicationContext applicationContext; /** @@ -140,7 +144,6 @@ public abstract class AbstractMethodMessageHandler /** * Sets the list of custom {@code HandlerMethodArgumentResolver}s that will be used * after resolvers for supported argument type. - * @param customArgumentResolvers the list of resolvers; never {@code null}. */ public void setCustomArgumentResolvers(List customArgumentResolvers) { this.customArgumentResolvers.clear(); @@ -159,7 +162,6 @@ public abstract class AbstractMethodMessageHandler /** * Set the list of custom {@code HandlerMethodReturnValueHandler}s that will be used * after return value handlers for known types. - * @param customReturnValueHandlers the list of custom return value handlers, never {@code null}. */ public void setCustomReturnValueHandlers(List customReturnValueHandlers) { this.customReturnValueHandlers.clear(); @@ -168,13 +170,16 @@ public abstract class AbstractMethodMessageHandler } } + /** + * Return the complete list of argument resolvers. + */ public List getArgumentResolvers() { return this.argumentResolvers.getResolvers(); } /** - * Configure the complete list of supported argument types effectively overriding - * the ones configured by default. This is an advanced option. For most use cases + * Configure the complete list of supported argument types, effectively overriding + * the ones configured by default. This is an advanced option; for most use cases * it should be sufficient to use {@link #setCustomArgumentResolvers}. */ public void setArgumentResolvers(List argumentResolvers) { @@ -185,13 +190,16 @@ public abstract class AbstractMethodMessageHandler this.argumentResolvers.addResolvers(argumentResolvers); } + /** + * Return the complete list of return value handlers. + */ public List getReturnValueHandlers() { return this.returnValueHandlers.getReturnValueHandlers(); } /** - * Configure the complete list of supported return value types effectively overriding - * the ones configured by default. This is an advanced option. For most use cases + * Configure the complete list of supported return value types, effectively overriding + * the ones configured by default. This is an advanced option; for most use cases * it should be sufficient to use {@link #setCustomReturnValueHandlers}. */ public void setReturnValueHandlers(List returnValueHandlers) { @@ -202,13 +210,6 @@ public abstract class AbstractMethodMessageHandler this.returnValueHandlers.addHandlers(returnValueHandlers); } - /** - * Return a map with all handler methods and their mappings. - */ - public Map getHandlerMethods() { - return Collections.unmodifiableMap(this.handlerMethods); - } - public ApplicationContext getApplicationContext() { return this.applicationContext; } @@ -360,10 +361,19 @@ public abstract class AbstractMethodMessageHandler * (e.g. to support "global" {@code @MessageExceptionHandler}). * @since 4.2 */ - protected void registerExceptionHandlerAdvice(MessagingAdviceBean bean, AbstractExceptionHandlerMethodResolver resolver) { + protected void registerExceptionHandlerAdvice( + MessagingAdviceBean bean, AbstractExceptionHandlerMethodResolver resolver) { + this.exceptionHandlerAdviceCache.put(bean, resolver); } + /** + * Return a map with all handler methods and their mappings. + */ + public Map getHandlerMethods() { + return Collections.unmodifiableMap(this.handlerMethods); + } + @Override public void handleMessage(Message message) throws MessagingException { @@ -425,7 +435,7 @@ public abstract class AbstractMethodMessageHandler addMatchesToCollection(allMappings, message, matches); } if (matches.isEmpty()) { - handleNoMatch(handlerMethods.keySet(), lookupDestination, message); + handleNoMatch(this.handlerMethods.keySet(), lookupDestination, message); return; } Comparator comparator = new MatchComparator(getMappingComparator(message)); @@ -449,7 +459,6 @@ public abstract class AbstractMethodMessageHandler handleMatch(bestMatch.mapping, bestMatch.handlerMethod, lookupDestination, message); } - private void addMatchesToCollection(Collection mappingsToCheck, Message message, List matches) { for (T mapping : mappingsToCheck) { T match = getMatchingMapping(mapping, message); @@ -468,7 +477,6 @@ public abstract class AbstractMethodMessageHandler */ protected abstract T getMatchingMapping(T mapping, Message message); - protected void handleNoMatch(Set ts, String lookupDestination, Message message) { logger.debug("No matching message handler methods."); } @@ -481,7 +489,6 @@ public abstract class AbstractMethodMessageHandler */ protected abstract Comparator getMappingComparator(Message message); - protected void handleMatch(T mapping, HandlerMethod handlerMethod, String lookupDestination, Message message) { if (logger.isDebugEnabled()) { logger.debug("Invoking " + handlerMethod.getShortLogMessage()); @@ -579,6 +586,7 @@ public abstract class AbstractMethodMessageHandler protected abstract AbstractExceptionHandlerMethodResolver createExceptionHandlerMethodResolverFor( Class beanType); + @Override public String toString() { return getClass().getSimpleName() + "[prefixes=" + getDestinationPrefixes() + "]"; diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilder.java b/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilder.java index 32246149ff..4fdaa22a04 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilder.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilder.java @@ -427,8 +427,7 @@ final class HtmlUnitRequestBuilder implements RequestBuilder, Mergeable { private UriComponents uriComponents() { URL url = this.webRequest.getUrl(); - UriComponentsBuilder uriBldr = UriComponentsBuilder.fromUriString(url.toExternalForm()); - return uriBldr.build(); + return UriComponentsBuilder.fromUriString(url.toExternalForm()).build(); } @Override @@ -455,13 +454,14 @@ final class HtmlUnitRequestBuilder implements RequestBuilder, Mergeable { return this.webClient.getCookieManager(); } + /** - * An extension to {@link MockHttpServletRequest} that ensures that - * when a new {@link HttpSession} is created, it is added to the managed sessions. + * An extension to {@link MockHttpServletRequest} that ensures that when a + * new {@link HttpSession} is created, it is added to the managed sessions. */ private final class HtmlUnitMockHttpServletRequest extends MockHttpServletRequest { - private HtmlUnitMockHttpServletRequest(ServletContext servletContext, String method, String requestURI) { + public HtmlUnitMockHttpServletRequest(ServletContext servletContext, String method, String requestURI) { super(servletContext, method, requestURI); } @@ -490,16 +490,17 @@ final class HtmlUnitRequestBuilder implements RequestBuilder, Mergeable { } } + /** * An extension to {@link MockHttpSession} that ensures when - * {@link #invalidate()} is called that the {@link HttpSession} is - * removed from the managed sessions. + * {@link #invalidate()} is called that the {@link HttpSession} + * is removed from the managed sessions. */ private final class HtmlUnitMockHttpSession extends MockHttpSession { private final MockHttpServletRequest request; - private HtmlUnitMockHttpSession(MockHttpServletRequest request) { + public HtmlUnitMockHttpSession(MockHttpServletRequest request) { super(request.getServletContext()); this.request = request; } 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 5c0efcd799..c05101720e 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 @@ -62,12 +62,6 @@ import org.springframework.web.util.UrlPathHelper; public abstract class AbstractMessageConverterMethodProcessor extends AbstractMessageConverterMethodArgumentResolver implements HandlerMethodReturnValueHandler { - private static final MediaType MEDIA_TYPE_APPLICATION = new MediaType("application"); - - private static final UrlPathHelper RAW_URL_PATH_HELPER = new UrlPathHelper(); - - private static final UrlPathHelper DECODING_URL_PATH_HELPER = new UrlPathHelper(); - /* Extensions associated with the built-in message converters */ private static final Set WHITELISTED_EXTENSIONS = new HashSet(Arrays.asList( "txt", "text", "yml", "properties", "csv", @@ -77,11 +71,18 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe private static final Set WHITELISTED_MEDIA_BASE_TYPES = new HashSet( Arrays.asList("audio", "image", "video")); + private static final MediaType MEDIA_TYPE_APPLICATION = new MediaType("application"); + + private static final UrlPathHelper DECODING_URL_PATH_HELPER = new UrlPathHelper(); + + private static final UrlPathHelper RAW_URL_PATH_HELPER = new UrlPathHelper(); + static { RAW_URL_PATH_HELPER.setRemoveSemicolonContent(false); RAW_URL_PATH_HELPER.setUrlDecode(false); } + private final ContentNegotiationManager contentNegotiationManager; private final PathExtensionContentNegotiationStrategy pathStrategy; @@ -141,12 +142,12 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe * Writes the given return value to the given web request. Delegates to * {@link #writeWithMessageConverters(Object, MethodParameter, ServletServerHttpRequest, ServletServerHttpResponse)} */ - protected void writeWithMessageConverters(T returnValue, MethodParameter returnType, NativeWebRequest webRequest) + protected void writeWithMessageConverters(T value, MethodParameter returnType, NativeWebRequest webRequest) throws IOException, HttpMediaTypeNotAcceptableException, HttpMessageNotWritableException { ServletServerHttpRequest inputMessage = createInputMessage(webRequest); ServletServerHttpResponse outputMessage = createOutputMessage(webRequest); - writeWithMessageConverters(returnValue, returnType, inputMessage, outputMessage); + writeWithMessageConverters(value, returnType, inputMessage, outputMessage); } /** @@ -164,21 +165,27 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe ServletServerHttpRequest inputMessage, ServletServerHttpResponse outputMessage) throws IOException, HttpMediaTypeNotAcceptableException, HttpMessageNotWritableException { - Class clazz = getReturnValueType(value, returnType); - Type type = getGenericType(returnType); + Object outputValue; + Class valueType; + Type declaredType; - if (value != null && value instanceof CharSequence) { - clazz = String.class; - type = String.class; - value = (T) value.toString(); + if (value instanceof CharSequence) { + outputValue = value.toString(); + valueType = String.class; + declaredType = String.class; + } + else { + outputValue = value; + valueType = getReturnValueType(outputValue, returnType); + declaredType = getGenericType(returnType); } - HttpServletRequest servletRequest = inputMessage.getServletRequest(); - List requestedMediaTypes = getAcceptableMediaTypes(servletRequest); - List producibleMediaTypes = getProducibleMediaTypes(servletRequest, clazz, type); + HttpServletRequest request = inputMessage.getServletRequest(); + List requestedMediaTypes = getAcceptableMediaTypes(request); + List producibleMediaTypes = getProducibleMediaTypes(request, valueType, declaredType); - if (value != null && producibleMediaTypes.isEmpty()) { - throw new IllegalArgumentException("No converter found for return value of type: " + clazz); + if (outputValue != null && producibleMediaTypes.isEmpty()) { + throw new IllegalArgumentException("No converter found for return value of type: " + valueType); } Set compatibleMediaTypes = new LinkedHashSet(); @@ -190,7 +197,7 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe } } if (compatibleMediaTypes.isEmpty()) { - if (value != null) { + if (outputValue != null) { throw new HttpMediaTypeNotAcceptableException(producibleMediaTypes); } return; @@ -215,34 +222,33 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe selectedMediaType = selectedMediaType.removeQualityValue(); for (HttpMessageConverter messageConverter : this.messageConverters) { if (messageConverter instanceof GenericHttpMessageConverter) { - if (((GenericHttpMessageConverter) messageConverter).canWrite(type, - clazz, selectedMediaType)) { - value = (T) getAdvice().beforeBodyWrite(value, returnType, selectedMediaType, + if (((GenericHttpMessageConverter) messageConverter).canWrite( + declaredType, valueType, selectedMediaType)) { + outputValue = (T) getAdvice().beforeBodyWrite(outputValue, returnType, selectedMediaType, (Class>) messageConverter.getClass(), inputMessage, outputMessage); - if (value != null) { + if (outputValue != null) { addContentDispositionHeader(inputMessage, outputMessage); - ((GenericHttpMessageConverter) messageConverter).write(value, - type, selectedMediaType, outputMessage); + ((GenericHttpMessageConverter) messageConverter).write( + outputValue, declaredType, selectedMediaType, outputMessage); if (logger.isDebugEnabled()) { - logger.debug("Written [" + value + "] as \"" + - selectedMediaType + "\" using [" + messageConverter + "]"); + logger.debug("Written [" + outputValue + "] as \"" + selectedMediaType + + "\" using [" + messageConverter + "]"); } } return; } } - else if (messageConverter.canWrite(clazz, selectedMediaType)) { - value = (T) getAdvice().beforeBodyWrite(value, returnType, selectedMediaType, + else if (messageConverter.canWrite(valueType, selectedMediaType)) { + outputValue = (T) getAdvice().beforeBodyWrite(outputValue, returnType, selectedMediaType, (Class>) messageConverter.getClass(), inputMessage, outputMessage); - if (value != null) { + if (outputValue != null) { addContentDispositionHeader(inputMessage, outputMessage); - ((HttpMessageConverter) messageConverter).write(value, - selectedMediaType, outputMessage); + ((HttpMessageConverter) messageConverter).write(outputValue, selectedMediaType, outputMessage); if (logger.isDebugEnabled()) { - logger.debug("Written [" + value + "] as \"" + - selectedMediaType + "\" using [" + messageConverter + "]"); + logger.debug("Written [" + outputValue + "] as \"" + selectedMediaType + + "\" using [" + messageConverter + "]"); } } return; @@ -250,19 +256,19 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe } } - if (value != null) { + if (outputValue != null) { throw new HttpMediaTypeNotAcceptableException(this.allSupportedMediaTypes); } } /** - * Return the type of the value to be written to the response. Typically this - * is a simple check via getClass on the returnValue but if the returnValue is - * null, then the returnType needs to be examined possibly including generic - * type determination (e.g. {@code ResponseEntity}). + * Return the type of the value to be written to the response. Typically this is + * a simple check via getClass on the value but if the value is null, then the + * return type needs to be examined possibly including generic type determination + * (e.g. {@code ResponseEntity}). */ - protected Class getReturnValueType(Object returnValue, MethodParameter returnType) { - return (returnValue != null ? returnValue.getClass() : returnType.getParameterType()); + protected Class getReturnValueType(Object value, MethodParameter returnType) { + return (value != null ? value.getClass() : returnType.getParameterType()); } /** @@ -282,8 +288,8 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe * @see #getProducibleMediaTypes(HttpServletRequest, Class, Type) */ @SuppressWarnings({"unchecked", "unused"}) - protected List getProducibleMediaTypes(HttpServletRequest request, Class returnValueClass) { - return getProducibleMediaTypes(request, returnValueClass, null); + protected List getProducibleMediaTypes(HttpServletRequest request, Class valueClass) { + return getProducibleMediaTypes(request, valueClass, null); } /** @@ -296,7 +302,7 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe * @since 4.2 */ @SuppressWarnings("unchecked") - protected List getProducibleMediaTypes(HttpServletRequest request, Class returnValueClass, Type returnValueType) { + protected List getProducibleMediaTypes(HttpServletRequest request, Class valueClass, Type declaredType) { Set mediaTypes = (Set) request.getAttribute(HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE); if (!CollectionUtils.isEmpty(mediaTypes)) { return new ArrayList(mediaTypes); @@ -304,12 +310,12 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe else if (!this.allSupportedMediaTypes.isEmpty()) { List result = new ArrayList(); for (HttpMessageConverter converter : this.messageConverters) { - if (converter instanceof GenericHttpMessageConverter && returnValueType != null) { - if (((GenericHttpMessageConverter) converter).canWrite(returnValueType, returnValueClass, null)) { + if (converter instanceof GenericHttpMessageConverter && declaredType != null) { + if (((GenericHttpMessageConverter) converter).canWrite(declaredType, valueClass, null)) { result.addAll(converter.getSupportedMediaTypes()); } } - else if (converter.canWrite(returnValueClass, null)) { + else if (converter.canWrite(valueClass, null)) { result.addAll(converter.getSupportedMediaTypes()); } } @@ -412,7 +418,7 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe try { mediaTypes = this.pathStrategy.resolveMediaTypeKey(null, extension); } - catch (HttpMediaTypeNotAcceptableException e) { + catch (HttpMediaTypeNotAcceptableException ex) { // Ignore } if (CollectionUtils.isEmpty(mediaTypes)) { -- GitLab