costin code review comments

This commit is contained in:
Keith Donald
2009-05-18 13:13:34 +00:00
parent 0cc3542aed
commit 8cbab5acb6
39 changed files with 242 additions and 255 deletions

View File

@@ -27,19 +27,19 @@ import org.springframework.util.Assert;
// TODO doesn't support more than depth of one (eg. Map<String,List<Foo>> or List<String>[])
/**
* A point where conversion needs to be performed. Provides additional context about the point such
* Context about a point where conversion needs to be performed. Provides context about the point such
* as field or method parameter information.
*
* @author Keith Donald
* @author Andy Clement
*/
public class ConversionPoint<T> {
public class ConversionContext<T> {
/**
* Constant value typeDescriptor for the type of a null value
* Constant value for the null object
*/
@SuppressWarnings("unchecked")
public final static ConversionPoint NULL = new ConversionPoint((Class<?>) null);
public final static ConversionContext NULL = new ConversionContext((Class<?>) null);
private MethodParameter methodParameter;
@@ -50,36 +50,35 @@ public class ConversionPoint<T> {
private Class<?> type;
/**
* Creates a new descriptor for the given type. Use this constructor when a bound value comes from a source such as
* Creates a new context for the given type. Use this constructor when a conversion point comes from a source such as
* a Map or collection, where no additional binding metadata is available.
* @param type the actual type
*/
public ConversionPoint(Class<?> type) {
public ConversionContext(Class<?> type) {
this.type = type;
}
/**
* Create a new descriptor for a method or constructor parameter. Use this constructor when a bound value originates
* Create a new context for a method or constructor parameter. Use this constructor when a conversion point originates
* from a method parameter, such as a setter method argument.
* @param methodParameter the MethodParameter to wrap
*/
public ConversionPoint(MethodParameter methodParameter) {
public ConversionContext(MethodParameter methodParameter) {
Assert.notNull(methodParameter, "MethodParameter must not be null");
this.methodParameter = methodParameter;
}
/**
* Create a new descriptor for a field. Use this constructor when a bound value originates from a field.
* Create a new context for a field. Use this constructor when a conversion point originates from a field.
* @param field the field to wrap
*/
public ConversionPoint(Field field) {
public ConversionContext(Field field) {
Assert.notNull(field, "Field must not be null");
this.field = field;
}
/**
* Determine the declared (non-generic) type of the wrapped parameter/field.
*
* @return the declared type (never <code>null</code>)
*/
public Class<?> getType() {
@@ -247,7 +246,7 @@ public class ConversionPoint<T> {
* @return true if this type is assignable to the target
*/
@SuppressWarnings("unchecked")
public boolean isAssignableTo(ConversionPoint targetType) {
public boolean isAssignableTo(ConversionContext targetType) {
return targetType.getType().isAssignableFrom(getType());
}
@@ -256,9 +255,9 @@ public class ConversionPoint<T> {
* @param type the class
* @return the type descriptor
*/
public static <T> ConversionPoint<T> valueOf(Class<T> type) {
public static <T> ConversionContext<T> valueOf(Class<T> type) {
// TODO needs a cache for common type descriptors
return new ConversionPoint<T>(type);
return new ConversionContext<T>(type);
}
/**
@@ -267,7 +266,7 @@ public class ConversionPoint<T> {
* @return the type descriptor
*/
@SuppressWarnings("unchecked")
public static ConversionPoint forObject(Object object) {
public static ConversionContext forObject(Object object) {
if (object == null) {
return NULL;
} else {

View File

@@ -19,7 +19,7 @@ package org.springframework.core.convert;
* A service interface for type conversion. This is the entry point into the convert system.
* <p>
* Call {@link #convert(Object, Class)} to perform a thread-safe type conversion using this system.<br>
* Call {@link #convert(Object, ConversionPoint)} to perform a conversion with additional context about the point
* Call {@link #convert(Object, ConversionContext)} to perform a conversion with additional context about the point
* where conversion needs to occur.
*
* @author Keith Donald
@@ -37,10 +37,10 @@ public interface TypeConverter {
/**
* Returns true if objects of sourceType can be converted to the type of the conversion point.
* @param source the source to convert from (may be null)
* @param point context about the point where conversion would occur
* @param context context about the point where conversion would occur
* @return true if a conversion can be performed, false if not
*/
boolean canConvert(Class<?> sourceType, ConversionPoint<?> point);
boolean canConvert(Class<?> sourceType, ConversionContext<?> context);
/**
* Convert the source to targetType.
@@ -54,10 +54,10 @@ public interface TypeConverter {
/**
* Convert the source to type T needed by the conversion point.
* @param source the source to convert from (may be null)
* @param point context about the point where conversion will occur
* @return the converted object, an instance of {@link ConversionPoint#getType()}</code>, or <code>null</code> if a null source was provided
* @param context context about the point where conversion will occur
* @return the converted object, an instance of {@link ConversionContext#getType()}</code>, or <code>null</code> if a null source was provided
* @throws ConvertException if an exception occurred
*/
<S, T> T convert(S source, ConversionPoint<T> point);
<S, T> T convert(S source, ConversionContext<T> context);
}

View File

@@ -24,21 +24,17 @@ public interface ConverterRegistry {
/**
* Add a converter to this registry.
*/
void addConverter(Converter<?, ?> converter);
void add(Converter<?, ?> converter);
/**
* Add a converter factory to this registry.
*/
void addConverterFactory(ConverterFactory<?, ?> converterFactory);
void add(ConverterFactory<?, ?> converterFactory);
/**
* Remove a converter from this registry.
* Remove the conversion logic from the sourceType to the targetType.
* @param sourceType the source type
* @param targetType the target type
*/
void removeConverter(Converter<?, ?> converter);
/**
* Remove a converter factory from this registry.
*/
void removeConverterFactory(ConverterFactory<?, ?> converterFactory);
void removeConverter(Class<?> sourceType, Class<?> targetType);
}

View File

@@ -16,7 +16,7 @@
package org.springframework.core.convert.support;
import org.springframework.core.convert.ConversionFailedException;
import org.springframework.core.convert.ConversionPoint;
import org.springframework.core.convert.ConversionContext;
/**
* Base class for converters that convert to and from collection types (arrays and java.util.Collection types)
@@ -28,18 +28,18 @@ abstract class AbstractCollectionConverter implements ConversionExecutor {
private ConversionExecutor elementConverter;
private ConversionPoint sourceCollectionType;
private ConversionContext sourceCollectionType;
private ConversionPoint targetCollectionType;
private ConversionContext targetCollectionType;
public AbstractCollectionConverter(ConversionPoint sourceCollectionType, ConversionPoint targetCollectionType, GenericTypeConverter conversionService) {
public AbstractCollectionConverter(ConversionContext sourceCollectionType, ConversionContext targetCollectionType, GenericTypeConverter conversionService) {
this.conversionService = conversionService;
this.sourceCollectionType = sourceCollectionType;
this.targetCollectionType = targetCollectionType;
Class<?> sourceElementType = sourceCollectionType.getElementType();
Class<?> targetElementType = targetCollectionType.getElementType();
if (sourceElementType != null && targetElementType != null) {
elementConverter = conversionService.getConversionExecutor(sourceElementType, ConversionPoint.valueOf(targetElementType));
elementConverter = conversionService.getConversionExecutor(sourceElementType, ConversionContext.valueOf(targetElementType));
} else {
elementConverter = NoOpConversionExecutor.INSTANCE;
}

View File

@@ -18,7 +18,7 @@ package org.springframework.core.convert.support;
import java.lang.reflect.Array;
import org.springframework.core.convert.TypeConverter;
import org.springframework.core.convert.ConversionPoint;
import org.springframework.core.convert.ConversionContext;
/**
* Special one-way converter that converts from a source array to a target array. Supports type conversion of the
@@ -29,7 +29,7 @@ import org.springframework.core.convert.ConversionPoint;
*/
class ArrayToArray extends AbstractCollectionConverter {
public ArrayToArray(ConversionPoint sourceArrayType, ConversionPoint targetArrayType, GenericTypeConverter conversionService) {
public ArrayToArray(ConversionContext sourceArrayType, ConversionContext targetArrayType, GenericTypeConverter conversionService) {
super(sourceArrayType, targetArrayType, conversionService);
}

View File

@@ -18,7 +18,7 @@ package org.springframework.core.convert.support;
import java.lang.reflect.Array;
import java.util.Collection;
import org.springframework.core.convert.ConversionPoint;
import org.springframework.core.convert.ConversionContext;
/**
* Special converter that converts from a source array to a target collection. Supports the selection of an
@@ -29,7 +29,7 @@ import org.springframework.core.convert.ConversionPoint;
*/
class ArrayToCollection extends AbstractCollectionConverter {
public ArrayToCollection(ConversionPoint sourceArrayType, ConversionPoint targetCollectionType,
public ArrayToCollection(ConversionContext sourceArrayType, ConversionContext targetCollectionType,
GenericTypeConverter conversionService) {
super(sourceArrayType, targetCollectionType, conversionService);
}

View File

@@ -19,7 +19,7 @@ import java.lang.reflect.Array;
import java.util.Collection;
import java.util.Iterator;
import org.springframework.core.convert.ConversionPoint;
import org.springframework.core.convert.ConversionContext;
/**
* Special converter that converts from target collection to a source array.
@@ -28,7 +28,7 @@ import org.springframework.core.convert.ConversionPoint;
*/
class CollectionToArray extends AbstractCollectionConverter {
public CollectionToArray(ConversionPoint sourceArrayType, ConversionPoint targetCollectionType,
public CollectionToArray(ConversionContext sourceArrayType, ConversionContext targetCollectionType,
GenericTypeConverter conversionService) {
super(sourceArrayType, targetCollectionType, conversionService);
}
@@ -52,7 +52,7 @@ class CollectionToArray extends AbstractCollectionConverter {
while (it.hasNext()) {
Object value = it.next();
if (value != null) {
elementConverter = getConversionService().getConversionExecutor(value.getClass(), ConversionPoint.valueOf(getTargetElementType()));
elementConverter = getConversionService().getConversionExecutor(value.getClass(), ConversionContext.valueOf(getTargetElementType()));
break;
}
}

View File

@@ -18,7 +18,7 @@ package org.springframework.core.convert.support;
import java.util.Collection;
import java.util.Iterator;
import org.springframework.core.convert.ConversionPoint;
import org.springframework.core.convert.ConversionContext;
/**
* A converter that can convert from one collection type to another.
@@ -27,7 +27,7 @@ import org.springframework.core.convert.ConversionPoint;
*/
class CollectionToCollection extends AbstractCollectionConverter {
public CollectionToCollection(ConversionPoint sourceCollectionType, ConversionPoint targetCollectionType,
public CollectionToCollection(ConversionContext sourceCollectionType, ConversionContext targetCollectionType,
GenericTypeConverter conversionService) {
super(sourceCollectionType, targetCollectionType, conversionService);
}
@@ -53,7 +53,7 @@ class CollectionToCollection extends AbstractCollectionConverter {
while (it.hasNext()) {
Object value = it.next();
if (value != null) {
elementConverter = getConversionService().getConversionExecutor(value.getClass(), ConversionPoint.valueOf(getTargetElementType()));
elementConverter = getConversionService().getConversionExecutor(value.getClass(), ConversionContext.valueOf(getTargetElementType()));
break;
}
}

View File

@@ -35,22 +35,22 @@ public class DefaultTypeConverter extends GenericTypeConverter {
* Add all default converters to the conversion service.
*/
protected void addDefaultConverters() {
addConverter(new StringToByte());
addConverter(new StringToBoolean());
addConverter(new StringToCharacter());
addConverter(new StringToShort());
addConverter(new StringToInteger());
addConverter(new StringToLong());
addConverter(new StringToFloat());
addConverter(new StringToDouble());
addConverter(new StringToBigInteger());
addConverter(new StringToBigDecimal());
addConverter(new StringToLocale());
addConverter(new NumberToCharacter());
addConverter(new ObjectToString());
addConverterFactory(new StringToEnumFactory());
addConverterFactory(new NumberToNumberFactory());
addConverterFactory(new CharacterToNumberFactory());
add(new StringToByte());
add(new StringToBoolean());
add(new StringToCharacter());
add(new StringToShort());
add(new StringToInteger());
add(new StringToLong());
add(new StringToFloat());
add(new StringToDouble());
add(new StringToBigInteger());
add(new StringToBigDecimal());
add(new StringToLocale());
add(new NumberToCharacter());
add(new ObjectToString());
add(new StringToEnumFactory());
add(new NumberToNumberFactory());
add(new CharacterToNumberFactory());
}
}

View File

@@ -26,7 +26,7 @@ import java.util.List;
import java.util.Map;
import org.springframework.core.GenericTypeResolver;
import org.springframework.core.convert.ConversionPoint;
import org.springframework.core.convert.ConversionContext;
import org.springframework.core.convert.ConverterNotFoundException;
import org.springframework.core.convert.TypeConverter;
import org.springframework.core.convert.converter.Converter;
@@ -71,11 +71,9 @@ public class GenericTypeConverter implements TypeConverter, ConverterRegistry {
this.parent = parent;
}
/**
* Register the Converter with this conversion service.
* @param converter the converter to register
*/
public void addConverter(Converter converter) {
// implementing ConverterRegistry
public void add(Converter converter) {
List typeInfo = getRequiredTypeInfo(converter);
Class sourceType = (Class) typeInfo.get(0);
Class targetType = (Class) typeInfo.get(1);
@@ -83,7 +81,7 @@ public class GenericTypeConverter implements TypeConverter, ConverterRegistry {
sourceMap.put(targetType, converter);
}
public void addConverterFactory(ConverterFactory<?, ?> converterFactory) {
public void add(ConverterFactory<?, ?> converterFactory) {
List typeInfo = getRequiredTypeInfo(converterFactory);
Class sourceType = (Class) typeInfo.get(0);
Class targetType = (Class) typeInfo.get(1);
@@ -91,15 +89,9 @@ public class GenericTypeConverter implements TypeConverter, ConverterRegistry {
sourceMap.put(targetType, converterFactory);
}
public void removeConverter(Converter<?, ?> converter) {
List typeInfo = getRequiredTypeInfo(converter);
Class sourceType = (Class) typeInfo.get(0);
Class targetType = (Class) typeInfo.get(1);
public void removeConverter(Class<?> sourceType, Class<?> targetType) {
Map sourceMap = getSourceMap(sourceType);
Converter existing = (Converter) sourceMap.get(targetType);
if (converter == existing) {
sourceMap.remove(targetType);
}
sourceMap.remove(targetType);
}
public void removeConverterFactory(ConverterFactory<?, ?> converter) {
@@ -113,19 +105,19 @@ public class GenericTypeConverter implements TypeConverter, ConverterRegistry {
}
}
// implementing ConversionService
// implementing TypeConverter
public boolean canConvert(Class<?> sourceType, Class<?> targetType) {
return canConvert(sourceType, ConversionPoint.valueOf(targetType));
return canConvert(sourceType, ConversionContext.valueOf(targetType));
}
public boolean canConvert(Class<?> sourceType, ConversionPoint<?> targetType) {
ConversionExecutor executor = getConversionExecutor(sourceType, targetType);
public boolean canConvert(Class<?> sourceType, ConversionContext<?> context) {
ConversionExecutor executor = getConversionExecutor(sourceType, context);
if (executor != null) {
return true;
} else {
if (parent != null) {
return parent.canConvert(sourceType, targetType);
return parent.canConvert(sourceType, context);
} else {
return false;
}
@@ -133,28 +125,28 @@ public class GenericTypeConverter implements TypeConverter, ConverterRegistry {
}
public <S, T> T convert(S source, Class<T> targetType) {
return convert(source, ConversionPoint.valueOf(targetType));
return convert(source, ConversionContext.valueOf(targetType));
}
public <S, T> T convert(S source, ConversionPoint<T> targetType) {
public <S, T> T convert(S source, ConversionContext<T> context) {
if (source == null) {
return null;
}
ConversionExecutor executor = getConversionExecutor(source.getClass(), targetType);
ConversionExecutor executor = getConversionExecutor(source.getClass(), context);
if (executor != null) {
return (T) executor.execute(source);
} else {
if (parent != null) {
return parent.convert(source, targetType);
return parent.convert(source, context);
} else {
throw new ConverterNotFoundException(source.getClass(), targetType.getType(),
throw new ConverterNotFoundException(source.getClass(), context.getType(),
"No converter found that can convert from sourceType [" + source.getClass().getName()
+ "] to targetType [" + targetType.getName() + "]");
+ "] to targetType [" + context.getName() + "]");
}
}
}
ConversionExecutor getConversionExecutor(Class sourceClass, ConversionPoint targetType)
ConversionExecutor getConversionExecutor(Class sourceClass, ConversionContext targetType)
throws ConverterNotFoundException {
Assert.notNull(sourceClass, "The sourceType to convert from is required");
Assert.notNull(targetType, "The targetType to convert to is required");
@@ -162,7 +154,7 @@ public class GenericTypeConverter implements TypeConverter, ConverterRegistry {
// TODO for Andy - is this correct way to handle the Null TypedValue?
return NoOpConversionExecutor.INSTANCE;
}
ConversionPoint sourceType = ConversionPoint.valueOf(sourceClass);
ConversionContext sourceType = ConversionContext.valueOf(sourceClass);
if (sourceType.isArray()) {
if (targetType.isArray()) {
return new ArrayToArray(sourceType, targetType, this);

View File

@@ -22,7 +22,7 @@ import java.util.SortedMap;
import java.util.TreeMap;
import org.springframework.core.convert.ConversionFailedException;
import org.springframework.core.convert.ConversionPoint;
import org.springframework.core.convert.ConversionContext;
/**
* Converts from one map to another map, with support for converting individual map elements based on generic type information.
@@ -31,9 +31,9 @@ import org.springframework.core.convert.ConversionPoint;
@SuppressWarnings("unchecked")
class MapToMap implements ConversionExecutor {
private ConversionPoint sourceType;
private ConversionContext sourceType;
private ConversionPoint targetType;
private ConversionContext targetType;
private GenericTypeConverter conversionService;
@@ -45,7 +45,7 @@ class MapToMap implements ConversionExecutor {
* @param targetType the target map type
* @param conversionService the conversion service
*/
public MapToMap(ConversionPoint sourceType, ConversionPoint targetType, GenericTypeConverter conversionService) {
public MapToMap(ConversionContext sourceType, ConversionContext targetType, GenericTypeConverter conversionService) {
this.sourceType = sourceType;
this.targetType = targetType;
this.conversionService = conversionService;
@@ -55,9 +55,9 @@ class MapToMap implements ConversionExecutor {
private EntryConverter createEntryConverter() {
if (sourceType.isMapEntryTypeKnown() && targetType.isMapEntryTypeKnown()) {
ConversionExecutor keyConverter = conversionService.getConversionExecutor(sourceType.getMapKeyType(),
ConversionPoint.valueOf(targetType.getMapKeyType()));
ConversionContext.valueOf(targetType.getMapKeyType()));
ConversionExecutor valueConverter = conversionService.getConversionExecutor(sourceType.getMapValueType(),
ConversionPoint.valueOf(targetType.getMapValueType()));
ConversionContext.valueOf(targetType.getMapValueType()));
return new EntryConverter(keyConverter, valueConverter);
} else {
return EntryConverter.NO_OP_INSTANCE;
@@ -94,11 +94,11 @@ class MapToMap implements ConversionExecutor {
Object key = entry.getKey();
Object value = entry.getValue();
if (keyConverter == null && key != null) {
keyConverter = conversionService.getConversionExecutor(key.getClass(), ConversionPoint
keyConverter = conversionService.getConversionExecutor(key.getClass(), ConversionContext
.valueOf(targetKeyType));
}
if (valueConverter == null && value != null) {
valueConverter = conversionService.getConversionExecutor(value.getClass(), ConversionPoint
valueConverter = conversionService.getConversionExecutor(value.getClass(), ConversionContext
.valueOf(targetValueType));
}
if (keyConverter != null && valueConverter != null) {

View File

@@ -16,7 +16,7 @@
package org.springframework.core.convert.support;
import org.springframework.core.convert.ConversionFailedException;
import org.springframework.core.convert.ConversionPoint;
import org.springframework.core.convert.ConversionContext;
import org.springframework.core.convert.converter.Converter;
import org.springframework.core.style.ToStringCreator;
@@ -27,13 +27,13 @@ import org.springframework.core.style.ToStringCreator;
@SuppressWarnings("unchecked")
class StaticConversionExecutor implements ConversionExecutor {
private final ConversionPoint sourceType;
private final ConversionContext sourceType;
private final ConversionPoint targetType;
private final ConversionContext targetType;
private final Converter converter;
public StaticConversionExecutor(ConversionPoint sourceType, ConversionPoint targetType, Converter converter) {
public StaticConversionExecutor(ConversionContext sourceType, ConversionContext targetType, Converter converter) {
this.sourceType = sourceType;
this.targetType = targetType;
this.converter = converter;