From e49813f2c4c6bb645c0990b3bd0fc290fc7c9f8e Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 12 Dec 2016 22:49:40 +0100 Subject: [PATCH] Polishing --- .../org/springframework/core/Conventions.java | 2 +- .../support/CollectionToStringConverter.java | 12 ++- .../support/GenericConversionService.java | 30 +++---- .../support/ObjectToOptionalConverter.java | 8 +- .../jdbc/core/JdbcTemplate.java | 4 +- .../web/method/annotation/ModelFactory.java | 84 +++++++++---------- .../web/util/HierarchicalUriComponents.java | 4 +- .../jetty/JettyRequestUpgradeStrategy.java | 11 +-- 8 files changed, 80 insertions(+), 75 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/Conventions.java b/spring-core/src/main/java/org/springframework/core/Conventions.java index 340cd3b9b75..edd64ebf2a1 100644 --- a/spring-core/src/main/java/org/springframework/core/Conventions.java +++ b/spring-core/src/main/java/org/springframework/core/Conventions.java @@ -281,7 +281,7 @@ public abstract class Conventions { /** * Retrieves the {@code Class} of an element in the {@code Collection}. - * The exact element for which the {@code Class} is retreived will depend + * The exact element for which the {@code Class} is retrieved will depend * on the concrete {@code Collection} implementation. */ private static E peekAhead(Collection collection) { diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/CollectionToStringConverter.java b/spring-core/src/main/java/org/springframework/core/convert/support/CollectionToStringConverter.java index 7537c1c3e0a..1cd142fd618 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/CollectionToStringConverter.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/CollectionToStringConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -36,10 +36,12 @@ final class CollectionToStringConverter implements ConditionalGenericConverter { private final ConversionService conversionService; + public CollectionToStringConverter(ConversionService conversionService) { this.conversionService = conversionService; } + @Override public Set getConvertibleTypes() { return Collections.singleton(new ConvertiblePair(Collection.class, String.class)); @@ -47,7 +49,8 @@ final class CollectionToStringConverter implements ConditionalGenericConverter { @Override public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { - return ConversionUtils.canConvertElements(sourceType.getElementTypeDescriptor(), targetType, this.conversionService); + return ConversionUtils.canConvertElements( + sourceType.getElementTypeDescriptor(), targetType, this.conversionService); } @Override @@ -56,7 +59,7 @@ final class CollectionToStringConverter implements ConditionalGenericConverter { return null; } Collection sourceCollection = (Collection) source; - if (sourceCollection.size() == 0) { + if (sourceCollection.isEmpty()) { return ""; } StringBuilder sb = new StringBuilder(); @@ -65,7 +68,8 @@ final class CollectionToStringConverter implements ConditionalGenericConverter { if (i > 0) { sb.append(DELIMITER); } - Object targetElement = this.conversionService.convert(sourceElement, sourceType.elementTypeDescriptor(sourceElement), targetType); + Object targetElement = this.conversionService.convert( + sourceElement, sourceType.elementTypeDescriptor(sourceElement), targetType); sb.append(targetElement); i++; } diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java b/spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java index 39f4cd95b40..584600659bc 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java @@ -131,14 +131,14 @@ public class GenericConversionService implements ConfigurableConversionService { @Override public boolean canConvert(Class sourceType, Class targetType) { - Assert.notNull(targetType, "targetType to convert to cannot be null"); + Assert.notNull(targetType, "Target type to convert to cannot be null"); return canConvert((sourceType != null ? TypeDescriptor.valueOf(sourceType) : null), TypeDescriptor.valueOf(targetType)); } @Override public boolean canConvert(TypeDescriptor sourceType, TypeDescriptor targetType) { - Assert.notNull(targetType, "targetType to convert to cannot be null"); + Assert.notNull(targetType, "Target type to convert to cannot be null"); if (sourceType == null) { return true; } @@ -147,9 +147,9 @@ public class GenericConversionService implements ConfigurableConversionService { } /** - * Return whether conversion between the sourceType and targetType can be bypassed. + * Return whether conversion between the source type and the target type can be bypassed. *

More precisely, this method will return true if objects of sourceType can be - * converted to the targetType by returning the source object unchanged. + * converted to the target type by returning the source object unchanged. * @param sourceType context about the source type to convert from * (may be {@code null} if source is {@code null}) * @param targetType context about the target type to convert to (required) @@ -158,7 +158,7 @@ public class GenericConversionService implements ConfigurableConversionService { * @since 3.2 */ public boolean canBypassConvert(TypeDescriptor sourceType, TypeDescriptor targetType) { - Assert.notNull(targetType, "targetType to convert to cannot be null"); + Assert.notNull(targetType, "Target type to convert to cannot be null"); if (sourceType == null) { return true; } @@ -169,20 +169,20 @@ public class GenericConversionService implements ConfigurableConversionService { @Override @SuppressWarnings("unchecked") public T convert(Object source, Class targetType) { - Assert.notNull(targetType, "targetType to convert to cannot be null"); + Assert.notNull(targetType, "Target type to convert to cannot be null"); return (T) convert(source, TypeDescriptor.forObject(source), TypeDescriptor.valueOf(targetType)); } @Override public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { - Assert.notNull(targetType, "targetType to convert to cannot be null"); + Assert.notNull(targetType, "Target type to convert to cannot be null"); if (sourceType == null) { - Assert.isTrue(source == null, "source must be [null] if sourceType == [null]"); + Assert.isTrue(source == null, "Source must be [null] if source type == [null]"); return handleResult(null, targetType, convertNullSource(null, targetType)); } if (source != null && !sourceType.getObjectType().isInstance(source)) { - throw new IllegalArgumentException("source to convert from must be an instance of " + - sourceType + "; instead it was a " + source.getClass().getName()); + throw new IllegalArgumentException("Source to convert from must be an instance of [" + + sourceType + "]; instead it was a [" + source.getClass().getName() + "]"); } GenericConverter converter = getConverter(sourceType, targetType); if (converter != null) { @@ -194,9 +194,9 @@ public class GenericConversionService implements ConfigurableConversionService { /** * Convenience operation for converting a source object to the specified targetType, - * where the targetType is a descriptor that provides additional conversion context. + * where the target type is a descriptor that provides additional conversion context. * Simply delegates to {@link #convert(Object, TypeDescriptor, TypeDescriptor)} and - * encapsulates the construction of the sourceType descriptor using + * encapsulates the construction of the source type descriptor using * {@link TypeDescriptor#forObject(Object)}. * @param source the source object * @param targetType the target type @@ -223,8 +223,8 @@ public class GenericConversionService implements ConfigurableConversionService { * {@link java.util.Optional#empty()} instance if the target type is * {@code java.util.Optional}. Subclasses may override this to return * custom {@code null} objects for specific target types. - * @param sourceType the sourceType to convert from - * @param targetType the targetType to convert to + * @param sourceType the source type to convert from + * @param targetType the target type to convert to * @return the converted null object */ protected Object convertNullSource(TypeDescriptor sourceType, TypeDescriptor targetType) { @@ -268,7 +268,7 @@ public class GenericConversionService implements ConfigurableConversionService { /** * Return the default converter if no converter is found for the given sourceType/targetType pair. - *

Returns a NO_OP Converter if the sourceType is assignable to the targetType. + *

Returns a NO_OP Converter if the source type is assignable to the target type. * Returns {@code null} otherwise, indicating no suitable converter could be found. * @param sourceType the source type to convert from * @param targetType the target type to convert to diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/ObjectToOptionalConverter.java b/spring-core/src/main/java/org/springframework/core/convert/support/ObjectToOptionalConverter.java index 099cc66cf2d..0555adbfda1 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/ObjectToOptionalConverter.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/ObjectToOptionalConverter.java @@ -66,13 +66,13 @@ final class ObjectToOptionalConverter implements ConditionalGenericConverter { else if (source instanceof Optional) { return source; } - else if (targetType.getResolvableType() == null) { - return Optional.of(source); - } - else { + else if (targetType.getResolvableType() != null) { Object target = this.conversionService.convert(source, sourceType, new GenericTypeDescriptor(targetType)); return Optional.ofNullable(target); } + else { + return Optional.of(source); + } } diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/core/JdbcTemplate.java b/spring-jdbc/src/main/java/org/springframework/jdbc/core/JdbcTemplate.java index 08db91518a9..3b115c8c8fd 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/core/JdbcTemplate.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/core/JdbcTemplate.java @@ -129,8 +129,8 @@ public class JdbcTemplate extends JdbcAccessor implements JdbcOperations { private int queryTimeout = -1; /** - * If this variable is set to true then all results checking will be bypassed for any - * callable statement processing. This can be used to avoid a bug in some older Oracle + * If this variable is set to true, then all results checking will be bypassed for any + * callable statement processing. This can be used to avoid a bug in some older Oracle * JDBC drivers like 10.1.0.2. */ private boolean skipResultsProcessing = false; diff --git a/spring-web/src/main/java/org/springframework/web/method/annotation/ModelFactory.java b/spring-web/src/main/java/org/springframework/web/method/annotation/ModelFactory.java index 07fac01a493..836b237adb6 100644 --- a/spring-web/src/main/java/org/springframework/web/method/annotation/ModelFactory.java +++ b/spring-web/src/main/java/org/springframework/web/method/annotation/ModelFactory.java @@ -185,44 +185,6 @@ public final class ModelFactory { return result; } - /** - * Derives the model attribute name for a method parameter based on: - *

    - *
  1. The parameter {@code @ModelAttribute} annotation value - *
  2. The parameter type - *
- * @return the derived name; never {@code null} or an empty string - */ - public static String getNameForParameter(MethodParameter parameter) { - ModelAttribute ann = parameter.getParameterAnnotation(ModelAttribute.class); - String name = (ann != null ? ann.value() : null); - return StringUtils.hasText(name) ? name : Conventions.getVariableNameForParameter(parameter); - } - - /** - * Derive the model attribute name for the given return value using one of: - *
    - *
  1. The method {@code ModelAttribute} annotation value - *
  2. The declared return type if it is more specific than {@code Object} - *
  3. The actual return value type - *
- * @param returnValue the value returned from a method invocation - * @param returnType the return type of the method - * @return the model name, never {@code null} nor empty - */ - public static String getNameForReturnValue(Object returnValue, MethodParameter returnType) { - ModelAttribute ann = returnType.getMethodAnnotation(ModelAttribute.class); - if (ann != null && StringUtils.hasText(ann.value())) { - return ann.value(); - } - else { - Method method = returnType.getMethod(); - Class containingClass = returnType.getContainingClass(); - Class resolvedType = GenericTypeResolver.resolveReturnType(method, containingClass); - return Conventions.getVariableNameForReturnType(method, resolvedType, returnValue); - } - } - /** * Promote model attributes listed as {@code @SessionAttributes} to the session. * Add {@link BindingResult} attributes where necessary. @@ -250,10 +212,8 @@ public final class ModelFactory { List keyNames = new ArrayList<>(model.keySet()); for (String name : keyNames) { Object value = model.get(name); - if (isBindingCandidate(name, value)) { String bindingResultKey = BindingResult.MODEL_KEY_PREFIX + name; - if (!model.containsAttribute(bindingResultKey)) { WebDataBinder dataBinder = this.dataBinderFactory.createBinder(request, value, name); model.put(bindingResultKey, dataBinder.getBindingResult()); @@ -270,7 +230,7 @@ public final class ModelFactory { return false; } - Class attrType = (value != null) ? value.getClass() : null; + Class attrType = (value != null ? value.getClass() : null); if (this.sessionAttributesHandler.isHandlerSessionAttribute(attributeName, attrType)) { return true; } @@ -280,13 +240,53 @@ public final class ModelFactory { } + /** + * Derive the model attribute name for a method parameter based on: + *
    + *
  1. the parameter {@code @ModelAttribute} annotation value + *
  2. the parameter type + *
+ * @param parameter a descriptor for the method parameter + * @return the derived name (never {@code null} or empty String) + */ + public static String getNameForParameter(MethodParameter parameter) { + ModelAttribute ann = parameter.getParameterAnnotation(ModelAttribute.class); + String name = (ann != null ? ann.value() : null); + return (StringUtils.hasText(name) ? name : Conventions.getVariableNameForParameter(parameter)); + } + + /** + * Derive the model attribute name for the given return value based on: + *
    + *
  1. the method {@code ModelAttribute} annotation value + *
  2. the declared return type if it is more specific than {@code Object} + *
  3. the actual return value type + *
+ * @param returnValue the value returned from a method invocation + * @param returnType a descriptor for the return type of the method + * @return the derived name (never {@code null} or empty String) + */ + public static String getNameForReturnValue(Object returnValue, MethodParameter returnType) { + ModelAttribute ann = returnType.getMethodAnnotation(ModelAttribute.class); + if (ann != null && StringUtils.hasText(ann.value())) { + return ann.value(); + } + else { + Method method = returnType.getMethod(); + Class containingClass = returnType.getContainingClass(); + Class resolvedType = GenericTypeResolver.resolveReturnType(method, containingClass); + return Conventions.getVariableNameForReturnType(method, resolvedType, returnValue); + } + } + + private static class ModelMethod { private final InvocableHandlerMethod handlerMethod; private final Set dependencies = new HashSet<>(); - private ModelMethod(InvocableHandlerMethod handlerMethod) { + public ModelMethod(InvocableHandlerMethod handlerMethod) { this.handlerMethod = handlerMethod; for (MethodParameter parameter : handlerMethod.getMethodParameters()) { if (parameter.hasParameterAnnotation(ModelAttribute.class)) { diff --git a/spring-web/src/main/java/org/springframework/web/util/HierarchicalUriComponents.java b/spring-web/src/main/java/org/springframework/web/util/HierarchicalUriComponents.java index b212bdcf8bb..3c71e6276c5 100644 --- a/spring-web/src/main/java/org/springframework/web/util/HierarchicalUriComponents.java +++ b/spring-web/src/main/java/org/springframework/web/util/HierarchicalUriComponents.java @@ -195,7 +195,7 @@ final class HierarchicalUriComponents extends UriComponents { String hostTo = encodeUriComponent(this.host, charset, getHostType()); PathComponent pathTo = this.path.encode(charset); MultiValueMap paramsTo = encodeQueryParams(charset); - String fragmentTo = encodeUriComponent(this.getFragment(), charset, Type.FRAGMENT); + String fragmentTo = encodeUriComponent(getFragment(), charset, Type.FRAGMENT); return new HierarchicalUriComponents(schemeTo, userInfoTo, hostTo, this.port, pathTo, paramsTo, fragmentTo, true, false); } @@ -339,7 +339,7 @@ final class HierarchicalUriComponents extends UriComponents { String portTo = expandUriComponent(this.port, uriVariables); PathComponent pathTo = this.path.expand(uriVariables); MultiValueMap paramsTo = expandQueryParams(uriVariables); - String fragmentTo = expandUriComponent(this.getFragment(), uriVariables); + String fragmentTo = expandUriComponent(getFragment(), uriVariables); return new HierarchicalUriComponents(schemeTo, userInfoTo, hostTo, portTo, pathTo, paramsTo, fragmentTo, false, false); diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/server/jetty/JettyRequestUpgradeStrategy.java b/spring-websocket/src/main/java/org/springframework/web/socket/server/jetty/JettyRequestUpgradeStrategy.java index 937c5bdd7bf..bae6c82712b 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/server/jetty/JettyRequestUpgradeStrategy.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/server/jetty/JettyRequestUpgradeStrategy.java @@ -19,6 +19,7 @@ package org.springframework.web.socket.server.jetty; import java.io.IOException; import java.security.Principal; import java.util.ArrayList; +import java.util.LinkedList; import java.util.List; import java.util.Map; import javax.servlet.ServletContext; @@ -176,8 +177,8 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Serv Assert.isInstanceOf(ServletServerHttpResponse.class, response); HttpServletResponse servletResponse = ((ServletServerHttpResponse) response).getServletResponse(); - Assert.isTrue(this.factoryAdapter.getFactory() - .isUpgradeRequest(servletRequest, servletResponse), "Not a WebSocket handshake"); + Assert.isTrue(this.factoryAdapter.getFactory().isUpgradeRequest(servletRequest, servletResponse), + "Not a WebSocket handshake"); JettyWebSocketSession session = new JettyWebSocketSession(attributes, user); JettyWebSocketHandlerAdapter handlerAdapter = new JettyWebSocketHandlerAdapter(wsHandler, session); @@ -213,12 +214,12 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Serv this.handler = handler; this.selectedProtocol = protocol; if (CollectionUtils.isEmpty(extensions)) { - this.extensionConfigs = new ArrayList<>(); + this.extensionConfigs = new LinkedList<>(); } else { this.extensionConfigs = new ArrayList<>(); - for (WebSocketExtension e : extensions) { - this.extensionConfigs.add(new WebSocketToJettyExtensionConfigAdapter(e)); + for (WebSocketExtension extension : extensions) { + this.extensionConfigs.add(new WebSocketToJettyExtensionConfigAdapter(extension)); } } }