added null binding check for primitives for all conversion results; polishing
This commit is contained in:
parent
c5833b192e
commit
01cbfd4f6f
|
|
@ -36,6 +36,7 @@ import org.springframework.format.Formatter;
|
|||
import org.springframework.format.FormatterRegistry;
|
||||
import org.springframework.format.Parser;
|
||||
import org.springframework.format.Printer;
|
||||
import org.springframework.util.StringUtils;
|
||||
import org.springframework.util.StringValueResolver;
|
||||
|
||||
/**
|
||||
|
|
@ -124,10 +125,13 @@ public class FormattingConversionService extends GenericConversionService
|
|||
|
||||
@SuppressWarnings("unchecked")
|
||||
public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
|
||||
if (source == null) {
|
||||
return "";
|
||||
}
|
||||
if (!sourceType.isAssignableTo(this.printerObjectType)) {
|
||||
source = this.conversionService.convert(source, sourceType, this.printerObjectType);
|
||||
}
|
||||
return (source != null ? this.printer.print(source, LocaleContextHolder.getLocale()) : "");
|
||||
return this.printer.print(source, LocaleContextHolder.getLocale());
|
||||
}
|
||||
|
||||
private Class<?> resolvePrinterObjectType(Printer<?> printer) {
|
||||
|
|
@ -159,7 +163,7 @@ public class FormattingConversionService extends GenericConversionService
|
|||
|
||||
public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
|
||||
String text = (String) source;
|
||||
if (text == null || text.length() == 0) {
|
||||
if (!StringUtils.hasText(text)) {
|
||||
return null;
|
||||
}
|
||||
Object result;
|
||||
|
|
@ -169,6 +173,9 @@ public class FormattingConversionService extends GenericConversionService
|
|||
catch (ParseException ex) {
|
||||
throw new IllegalArgumentException("Unable to parse '" + text + "'", ex);
|
||||
}
|
||||
if (result == null) {
|
||||
throw new IllegalStateException("Parsers are not allowed to return null");
|
||||
}
|
||||
TypeDescriptor resultType = TypeDescriptor.valueOf(result.getClass());
|
||||
if (!resultType.isAssignableTo(targetType)) {
|
||||
result = this.conversionService.convert(result, resultType, targetType);
|
||||
|
|
|
|||
|
|
@ -39,9 +39,11 @@ import org.springframework.beans.factory.config.PropertyPlaceholderConfigurer;
|
|||
import org.springframework.beans.factory.support.RootBeanDefinition;
|
||||
import org.springframework.context.i18n.LocaleContextHolder;
|
||||
import org.springframework.context.support.GenericApplicationContext;
|
||||
import org.springframework.core.convert.ConversionFailedException;
|
||||
import org.springframework.core.convert.TypeDescriptor;
|
||||
import org.springframework.core.convert.converter.Converter;
|
||||
import org.springframework.core.convert.support.DefaultConversionService;
|
||||
import org.springframework.format.Formatter;
|
||||
import org.springframework.format.datetime.joda.DateTimeParser;
|
||||
import org.springframework.format.datetime.joda.JodaDateTimeFormatAnnotationFormatterFactory;
|
||||
import org.springframework.format.datetime.joda.ReadablePartialPrinter;
|
||||
|
|
@ -203,6 +205,24 @@ public class FormattingConversionServiceTests {
|
|||
assertNull(formattingService.convert("", TypeDescriptor.valueOf(String.class), TypeDescriptor.valueOf(Integer.class)));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testParseBlankString() throws ParseException {
|
||||
formattingService.addFormatterForFieldType(Number.class, new NumberFormatter());
|
||||
assertNull(formattingService.convert(" ", TypeDescriptor.valueOf(String.class), TypeDescriptor.valueOf(Integer.class)));
|
||||
}
|
||||
|
||||
@Test(expected=ConversionFailedException.class)
|
||||
public void testParseParserReturnsNull() throws ParseException {
|
||||
formattingService.addFormatterForFieldType(Integer.class, new NullReturningFormatter());
|
||||
assertNull(formattingService.convert("1", TypeDescriptor.valueOf(String.class), TypeDescriptor.valueOf(Integer.class)));
|
||||
}
|
||||
|
||||
@Test(expected=ConversionFailedException.class)
|
||||
public void testParseNullPrimitiveProperty() throws ParseException {
|
||||
formattingService.addFormatterForFieldType(Integer.class, new NumberFormatter());
|
||||
assertNull(formattingService.convert(null, TypeDescriptor.valueOf(String.class), TypeDescriptor.valueOf(int.class)));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testPrintNullDefault() throws ParseException {
|
||||
assertEquals(null, formattingService.convert(null, TypeDescriptor.valueOf(Integer.class), TypeDescriptor.valueOf(String.class)));
|
||||
|
|
@ -221,11 +241,9 @@ public class FormattingConversionServiceTests {
|
|||
|
||||
public static class Model {
|
||||
|
||||
@SuppressWarnings("unused")
|
||||
@org.springframework.format.annotation.DateTimeFormat(style="S-")
|
||||
public Date date;
|
||||
|
||||
@SuppressWarnings("unused")
|
||||
@org.springframework.format.annotation.DateTimeFormat(pattern="M-d-yy")
|
||||
public List<Date> dates;
|
||||
|
||||
|
|
@ -241,11 +259,9 @@ public class FormattingConversionServiceTests {
|
|||
|
||||
public static class ModelWithPlaceholders {
|
||||
|
||||
@SuppressWarnings("unused")
|
||||
@org.springframework.format.annotation.DateTimeFormat(style="${dateStyle}")
|
||||
public Date date;
|
||||
|
||||
@SuppressWarnings("unused")
|
||||
@org.springframework.format.annotation.DateTimeFormat(pattern="${datePattern}")
|
||||
public List<Date> dates;
|
||||
|
||||
|
|
@ -257,5 +273,17 @@ public class FormattingConversionServiceTests {
|
|||
this.dates = dates;
|
||||
}
|
||||
}
|
||||
|
||||
public static class NullReturningFormatter implements Formatter<Integer> {
|
||||
|
||||
public String print(Integer object, Locale locale) {
|
||||
return null;
|
||||
}
|
||||
|
||||
public Integer parse(String text, Locale locale) throws ParseException {
|
||||
return null;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -166,6 +166,9 @@ public class GenericConversionService implements ConversionService, ConverterReg
|
|||
if (sourceType == TypeDescriptor.NULL) {
|
||||
Assert.isTrue(source == null, "The value must be null if sourceType == TypeDescriptor.NULL");
|
||||
Object result = convertNullSource(sourceType, targetType);
|
||||
if (result == null) {
|
||||
assertNotPrimitiveTargetType(sourceType, targetType);
|
||||
}
|
||||
if (logger.isDebugEnabled()) {
|
||||
logger.debug("Converted to " + StylerUtils.style(result));
|
||||
}
|
||||
|
|
@ -178,15 +181,12 @@ public class GenericConversionService implements ConversionService, ConverterReg
|
|||
Assert.isTrue(source == null || sourceType.getObjectType().isInstance(source));
|
||||
GenericConverter converter = getConverter(sourceType, targetType);
|
||||
if (converter == null) {
|
||||
if (source == null || sourceType.isAssignableTo(targetType)) {
|
||||
logger.debug("No converter found - returning assignable source object as-is");
|
||||
return source;
|
||||
}
|
||||
else {
|
||||
throw new ConverterNotFoundException(sourceType, targetType);
|
||||
}
|
||||
return handleConverterNotFound(source, sourceType, targetType);
|
||||
}
|
||||
Object result = ConversionUtils.invokeConverter(converter, source, sourceType, targetType);
|
||||
if (result == null) {
|
||||
assertNotPrimitiveTargetType(sourceType, targetType);
|
||||
}
|
||||
if (logger.isDebugEnabled()) {
|
||||
logger.debug("Converted to " + StylerUtils.style(result));
|
||||
}
|
||||
|
|
@ -218,18 +218,12 @@ public class GenericConversionService implements ConversionService, ConverterReg
|
|||
/**
|
||||
* Template method to convert a null source.
|
||||
* <p>Default implementation returns <code>null</code>.
|
||||
* Throws a {@link ConversionFailedException} if the targetType is a primitive type,
|
||||
* as <code>null</code> cannot be assigned to a primitive type.
|
||||
* Subclasses may override to return custom null objects for specific target types.
|
||||
* @param sourceType the sourceType to convert from
|
||||
* @param targetType the targetType to convert to
|
||||
* @return the converted null object
|
||||
*/
|
||||
protected Object convertNullSource(TypeDescriptor sourceType, TypeDescriptor targetType) {
|
||||
if (targetType.isPrimitive()) {
|
||||
throw new ConversionFailedException(sourceType, targetType, null,
|
||||
new IllegalArgumentException("A null value cannot be assigned to a primitive type"));
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
|
|
@ -462,10 +456,10 @@ public class GenericConversionService implements ConversionService, ConverterReg
|
|||
}
|
||||
}
|
||||
|
||||
private void addInterfaceHierarchy(Class<?> ifc, LinkedList<Class<?>> classQueue) {
|
||||
classQueue.addFirst(ifc);
|
||||
for (Class<?> inheritedIfc : ifc.getInterfaces()) {
|
||||
addInterfaceHierarchy(inheritedIfc, classQueue);
|
||||
private void addInterfaceHierarchy(Class<?> interfaceType, LinkedList<Class<?>> classQueue) {
|
||||
classQueue.addFirst(interfaceType);
|
||||
for (Class<?> implementedInterface : interfaceType.getInterfaces()) {
|
||||
addInterfaceHierarchy(implementedInterface, classQueue);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -481,6 +475,26 @@ public class GenericConversionService implements ConversionService, ConverterReg
|
|||
return matchable.matchConverter(sourceFieldType, targetFieldType);
|
||||
}
|
||||
|
||||
private Object handleConverterNotFound(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
|
||||
if (source == null) {
|
||||
assertNotPrimitiveTargetType(sourceType, targetType);
|
||||
return source;
|
||||
} else if (sourceType.isAssignableTo(targetType)) {
|
||||
logger.debug("No converter found - returning assignable source object as-is");
|
||||
return source;
|
||||
}
|
||||
else {
|
||||
throw new ConverterNotFoundException(sourceType, targetType);
|
||||
}
|
||||
}
|
||||
|
||||
private void assertNotPrimitiveTargetType(TypeDescriptor sourceType, TypeDescriptor targetType) {
|
||||
if (targetType.isPrimitive()) {
|
||||
throw new ConversionFailedException(sourceType, targetType, null,
|
||||
new IllegalArgumentException("A null value cannot be assigned to a primitive type"));
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
private final class ConverterAdapter implements GenericConverter {
|
||||
|
||||
|
|
|
|||
|
|
@ -51,8 +51,7 @@ final class IdToEntityConverter implements ConditionalGenericConverter {
|
|||
|
||||
public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
|
||||
Method finder = getFinder(targetType.getType());
|
||||
return (finder != null &&
|
||||
this.conversionService.canConvert(sourceType, TypeDescriptor.valueOf(finder.getParameterTypes()[0])));
|
||||
return finder != null && this.conversionService.canConvert(sourceType, TypeDescriptor.valueOf(finder.getParameterTypes()[0]));
|
||||
}
|
||||
|
||||
public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
|
||||
|
|
@ -60,8 +59,7 @@ final class IdToEntityConverter implements ConditionalGenericConverter {
|
|||
return null;
|
||||
}
|
||||
Method finder = getFinder(targetType.getType());
|
||||
Object id = this.conversionService.convert(
|
||||
source, sourceType, TypeDescriptor.valueOf(finder.getParameterTypes()[0]));
|
||||
Object id = this.conversionService.convert(source, sourceType, TypeDescriptor.valueOf(finder.getParameterTypes()[0]));
|
||||
return ReflectionUtils.invokeMethod(finder, source, id);
|
||||
}
|
||||
|
||||
|
|
@ -69,8 +67,7 @@ final class IdToEntityConverter implements ConditionalGenericConverter {
|
|||
String finderMethod = "find" + getEntityName(entityClass);
|
||||
Method[] methods = entityClass.getDeclaredMethods();
|
||||
for (Method method : methods) {
|
||||
if (Modifier.isStatic(method.getModifiers()) && method.getParameterTypes().length == 1 &&
|
||||
method.getReturnType().equals(entityClass)) {
|
||||
if (Modifier.isStatic(method.getModifiers()) && method.getParameterTypes().length == 1 && method.getReturnType().equals(entityClass)) {
|
||||
if (method.getName().equals(finderMethod)) {
|
||||
return method;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -26,7 +26,6 @@ import static org.junit.Assert.fail;
|
|||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.Collections;
|
||||
import java.util.HashMap;
|
||||
import java.util.LinkedHashMap;
|
||||
import java.util.LinkedList;
|
||||
|
|
@ -62,6 +61,16 @@ public class GenericConversionServiceTests {
|
|||
assertEquals(null, conversionService.convert(null, Integer.class));
|
||||
}
|
||||
|
||||
@Test(expected=ConversionFailedException.class)
|
||||
public void convertNullSourcePrimitiveTarget() {
|
||||
assertEquals(null, conversionService.convert(null, int.class));
|
||||
}
|
||||
|
||||
@Test(expected=ConversionFailedException.class)
|
||||
public void convertNullSourcePrimitiveTargetTypeDescriptor() {
|
||||
assertEquals(null, conversionService.convert(null, TypeDescriptor.valueOf(String.class), TypeDescriptor.valueOf(int.class)));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void convertAssignableSource() {
|
||||
assertEquals(Boolean.FALSE, conversionService.convert(false, boolean.class));
|
||||
|
|
|
|||
Loading…
Reference in New Issue