diff --git a/org.springframework.context/src/main/java/org/springframework/mapping/support/DefaultMapperTargetFactory.java b/org.springframework.context/src/main/java/org/springframework/mapping/support/DefaultMapperTargetFactory.java index dff7268f35a..d98a707d935 100644 --- a/org.springframework.context/src/main/java/org/springframework/mapping/support/DefaultMapperTargetFactory.java +++ b/org.springframework.context/src/main/java/org/springframework/mapping/support/DefaultMapperTargetFactory.java @@ -17,6 +17,7 @@ package org.springframework.mapping.support; import org.springframework.beans.BeanUtils; import org.springframework.core.convert.TypeDescriptor; +import org.springframework.util.ClassUtils; /** * Creates a mapping target by calling its default constructor. @@ -25,7 +26,11 @@ import org.springframework.core.convert.TypeDescriptor; */ class DefaultMapperTargetFactory implements MapperTargetFactory { - public Object createTarget(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { + public boolean supports(TypeDescriptor targetType) { + return ClassUtils.hasConstructor(targetType.getType(), null); + } + + public Object createTarget(TypeDescriptor targetType) { return BeanUtils.instantiate(targetType.getType()); } diff --git a/org.springframework.context/src/main/java/org/springframework/mapping/support/MapperConverter.java b/org.springframework.context/src/main/java/org/springframework/mapping/support/MapperConverter.java index bc03395ab9f..a5515b61c5c 100644 --- a/org.springframework.context/src/main/java/org/springframework/mapping/support/MapperConverter.java +++ b/org.springframework.context/src/main/java/org/springframework/mapping/support/MapperConverter.java @@ -15,6 +15,8 @@ */ package org.springframework.mapping.support; +import org.springframework.beans.BeanUtils; +import org.springframework.core.convert.ConversionFailedException; import org.springframework.core.convert.TypeDescriptor; import org.springframework.core.convert.support.GenericConverter; import org.springframework.mapping.Mapper; @@ -44,9 +46,33 @@ public class MapperConverter implements GenericConverter { if (source == null) { return null; } - // TODO - could detect cyclical reference here if had a mapping context? (source should not equal currently mapped object) - Object target = this.mappingTargetFactory.createTarget(source, sourceType, targetType); - return this.mapper.map(source, target); + if (SpelMappingContextHolder.contains(source)) { + return source; + } + if (sourceType.isAssignableTo(targetType) && isCopyByReference(sourceType, targetType)) { + return source; + } + return createAndMap(targetType, source, sourceType); } + private boolean isCopyByReference(TypeDescriptor sourceType, TypeDescriptor targetType) { + if (BeanUtils.isSimpleValueType(targetType.getType()) || Enum.class.isAssignableFrom(targetType.getType())) { + return true; + } else { + return false; + } + } + + private Object createAndMap(TypeDescriptor targetType, Object source, TypeDescriptor sourceType) { + if (this.mappingTargetFactory.supports(targetType)) { + Object target = this.mappingTargetFactory.createTarget(targetType); + return this.mapper.map(source, target); + } else { + IllegalStateException cause = new IllegalStateException("[" + + this.mappingTargetFactory.getClass().getName() + "] does not support target type [" + + targetType.getName() + "]"); + throw new ConversionFailedException(sourceType, targetType, source, cause); + } + + } } \ No newline at end of file diff --git a/org.springframework.context/src/main/java/org/springframework/mapping/support/MapperTargetFactory.java b/org.springframework.context/src/main/java/org/springframework/mapping/support/MapperTargetFactory.java index 35e7e7b0a2a..2618162861d 100644 --- a/org.springframework.context/src/main/java/org/springframework/mapping/support/MapperTargetFactory.java +++ b/org.springframework.context/src/main/java/org/springframework/mapping/support/MapperTargetFactory.java @@ -27,6 +27,13 @@ import org.springframework.mapping.Mapper; */ public interface MapperTargetFactory { + /** + * Does this factory support creating mapping targets of the specified type + * @param targetType the targe type + * @return true if so, false otherwise + */ + public boolean supports(TypeDescriptor targetType); + /** * Create the target object to be mapped to. * @param source the source object to map from @@ -34,5 +41,6 @@ public interface MapperTargetFactory { * @param targetType the target object type descriptor * @return the target */ - public Object createTarget(Object source, TypeDescriptor sourceType, TypeDescriptor targetType); + public Object createTarget(TypeDescriptor targetType); + } \ No newline at end of file diff --git a/org.springframework.context/src/main/java/org/springframework/mapping/support/SpelMapper.java b/org.springframework.context/src/main/java/org/springframework/mapping/support/SpelMapper.java index 1174c45bb8b..3d9c772803a 100644 --- a/org.springframework.context/src/main/java/org/springframework/mapping/support/SpelMapper.java +++ b/org.springframework.context/src/main/java/org/springframework/mapping/support/SpelMapper.java @@ -22,6 +22,8 @@ import java.util.List; import java.util.Map; import java.util.Set; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.core.convert.TypeDescriptor; import org.springframework.core.convert.converter.ConverterRegistry; import org.springframework.core.convert.support.DefaultConversionService; @@ -45,6 +47,8 @@ import org.springframework.mapping.MappingFailure; */ public class SpelMapper implements Mapper { + private static final Log logger = LogFactory.getLog(SpelMapper.class); + private static final MappableTypeFactory mappableTypeFactory = new MappableTypeFactory(); private static final SpelExpressionParser sourceExpressionParser = new SpelExpressionParser(); @@ -92,26 +96,39 @@ public class SpelMapper implements Mapper { } public Object map(Object source, Object target) { - EvaluationContext sourceContext = getMappingContext(source); - EvaluationContext targetContext = getMappingContext(target); - List failures = new LinkedList(); - for (SpelMapping mapping : this.mappings) { - mapping.map(sourceContext, targetContext, failures); + try { + SpelMappingContextHolder.push(source); + EvaluationContext sourceContext = getEvaluationContext(source); + EvaluationContext targetContext = getEvaluationContext(target); + List failures = new LinkedList(); + for (SpelMapping mapping : this.mappings) { + doMap(mapping, sourceContext, targetContext, failures); + } + Set autoMappings = getAutoMappings(sourceContext, targetContext); + for (SpelMapping mapping : autoMappings) { + doMap(mapping, sourceContext, targetContext, failures); + } + if (!failures.isEmpty()) { + throw new MappingException(failures); + } + return target; + } finally { + SpelMappingContextHolder.pop(); } - Set autoMappings = getAutoMappings(sourceContext, targetContext); - for (SpelMapping mapping : autoMappings) { - mapping.map(sourceContext, targetContext, failures); - } - if (!failures.isEmpty()) { - throw new MappingException(failures); - } - return target; } - private EvaluationContext getMappingContext(Object object) { + private EvaluationContext getEvaluationContext(Object object) { return mappableTypeFactory.getMappableType(object).getEvaluationContext(object, this.conversionService); } + private void doMap(SpelMapping mapping, EvaluationContext sourceContext, EvaluationContext targetContext, + List failures) { + if (logger.isDebugEnabled()) { + logger.debug(SpelMappingContextHolder.getLevel() + mapping); + } + mapping.map(sourceContext, targetContext, failures); + } + private Set getAutoMappings(EvaluationContext sourceContext, EvaluationContext targetContext) { if (this.autoMappingEnabled) { Set autoMappings = new LinkedHashSet(); diff --git a/org.springframework.context/src/main/java/org/springframework/mapping/support/SpelMappingContextHolder.java b/org.springframework.context/src/main/java/org/springframework/mapping/support/SpelMappingContextHolder.java new file mode 100644 index 00000000000..6c3556e25ce --- /dev/null +++ b/org.springframework.context/src/main/java/org/springframework/mapping/support/SpelMappingContextHolder.java @@ -0,0 +1,49 @@ +package org.springframework.mapping.support; + +import java.util.Stack; + +import org.springframework.core.NamedThreadLocal; + +class SpelMappingContextHolder { + + private static final ThreadLocal> mappingContextHolder = new NamedThreadLocal>( + "Mapping context"); + + public static void push(Object source) { + Stack context = getContext(); + if (context == null) { + context = new Stack(); + mappingContextHolder.set(context); + } + context.add(source); + } + + public static boolean contains(Object source) { + return getContext().contains(source); + } + + public static void pop() { + Stack context = getContext(); + if (context != null) { + context.pop(); + if (context.isEmpty()) { + mappingContextHolder.set(null); + } + } + } + + public static String getLevel() { + int size = getContext().size(); + StringBuilder builder = new StringBuilder(); + for (int i = 0; i < size; i++) { + builder.append("*"); + } + builder.append(" "); + return builder.toString(); + } + + private static Stack getContext() { + return mappingContextHolder.get(); + } + +} diff --git a/org.springframework.context/src/test/java/org/springframework/mapping/support/SpelMapperTests.java b/org.springframework.context/src/test/java/org/springframework/mapping/support/SpelMapperTests.java index 9d6d9d474be..25771aebbd4 100644 --- a/org.springframework.context/src/test/java/org/springframework/mapping/support/SpelMapperTests.java +++ b/org.springframework.context/src/test/java/org/springframework/mapping/support/SpelMapperTests.java @@ -2,13 +2,14 @@ package org.springframework.mapping.support; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import java.math.BigDecimal; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; -import org.junit.Ignore; import org.junit.Test; import org.springframework.core.convert.converter.Converter; import org.springframework.mapping.MappingException; @@ -237,9 +238,8 @@ public class SpelMapperTests { assertEquals(1, e.getMappingFailureCount()); } } - + @Test - @Ignore public void mapCyclic() { Person source = new Person(); source.setName("Keith"); @@ -254,6 +254,72 @@ public class SpelMapperTests { assertEquals(source.cyclic, target.cyclic); } + @Test + public void mapCyclicTypicalHibernateDomainModel() { + Order source = new Order(); + source.setNumber(1); + LineItem item = new LineItem(); + item.setAmount(new BigDecimal("30.00")); + item.setOrder(source); + source.setLineItem(item); + + Order target = new Order(); + + mapper.map(source, target); + assertEquals(1, target.getNumber()); + assertTrue(item != target.getLineItem()); + assertEquals(new BigDecimal("30.00"), target.getLineItem().getAmount()); + assertEquals(source, target.getLineItem().getOrder()); + } + + public static class Order { + + private int number; + + private LineItem lineItem; + + public int getNumber() { + return number; + } + + public void setNumber(int number) { + this.number = number; + } + + public LineItem getLineItem() { + return lineItem; + } + + public void setLineItem(LineItem lineItem) { + this.lineItem = lineItem; + } + + } + + public static class LineItem { + + private BigDecimal amount; + + private Order order; + + public BigDecimal getAmount() { + return amount; + } + + public void setAmount(BigDecimal amount) { + this.amount = amount; + } + + public Order getOrder() { + return order; + } + + public void setOrder(Order order) { + this.order = order; + } + + } + public static class PersonDto { private String fullName; diff --git a/org.springframework.context/src/test/resources/log4j.xml b/org.springframework.context/src/test/resources/log4j.xml index 767b96d6206..b09461e5a47 100644 --- a/org.springframework.context/src/test/resources/log4j.xml +++ b/org.springframework.context/src/test/resources/log4j.xml @@ -11,11 +11,7 @@ - - - - - + diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/converter/ConverterInfo.java b/org.springframework.core/src/main/java/org/springframework/core/convert/converter/ConverterInfo.java deleted file mode 100644 index ff258de0a66..00000000000 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/converter/ConverterInfo.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright 2002-2009 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.core.convert.converter; - -/** - * A meta interface a Converter may implement to describe what types he can convert between. - * - * Implementing this interface is required when registering converters that do not declare their - * parameterized types S and T with a {@link org.springframework.core.convert.ConversionService}. - * - * @author Keith Donald - * @since 3.0 - * @see Converter - */ -public interface ConverterInfo { - - /** - * The source type the converter converts from. - */ - Class getSourceType(); - - /** - * The target type the converter converts to. - */ - Class getTargetType(); - -} diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/DefaultConversionService.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/DefaultConversionService.java index 0af53dd94bd..dba0a1300a4 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/support/DefaultConversionService.java +++ b/org.springframework.core/src/main/java/org/springframework/core/convert/support/DefaultConversionService.java @@ -16,6 +16,7 @@ package org.springframework.core.convert.support; +import java.util.Locale; /** * Default implementation of a conversion service. Will automatically register from string @@ -31,15 +32,15 @@ public class DefaultConversionService extends GenericConversionService { * Create a new default conversion service, installing the default converters. */ public DefaultConversionService() { - addConverter(new StringToBooleanConverter()); - addConverter(new StringToCharacterConverter()); - addConverter(new StringToLocaleConverter()); - addConverter(new NumberToCharacterConverter()); - addConverter(new ObjectToStringConverter()); - addConverterFactory(new StringToNumberConverterFactory()); - addConverterFactory(new StringToEnumConverterFactory()); - addConverterFactory(new NumberToNumberConverterFactory()); - addConverterFactory(new CharacterToNumberFactory()); + addConverter(String.class, Boolean.class, new StringToBooleanConverter()); + addConverter(String.class, Character.class, new StringToCharacterConverter()); + addConverter(String.class, Locale.class, new StringToLocaleConverter()); + addConverter(Number.class, Character.class, new NumberToCharacterConverter()); + addConverter(Object.class, String.class, new ObjectToStringConverter()); + addConverterFactory(String.class, Number.class, new StringToNumberConverterFactory()); + addConverterFactory(String.class, Enum.class, new StringToEnumConverterFactory()); + addConverterFactory(Number.class, Number.class, new NumberToNumberConverterFactory()); + addConverterFactory(Character.class, Number.class, new CharacterToNumberFactory()); } } diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java index 3c842de818c..c292a5c1807 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java +++ b/org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java @@ -33,7 +33,6 @@ import org.springframework.core.convert.ConverterNotFoundException; import org.springframework.core.convert.TypeDescriptor; import org.springframework.core.convert.converter.Converter; import org.springframework.core.convert.converter.ConverterFactory; -import org.springframework.core.convert.converter.ConverterInfo; import org.springframework.core.convert.converter.ConverterRegistry; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -54,7 +53,7 @@ public class GenericConversionService implements ConversionService, ConverterReg } }; - private final Map> sourceTypeConverters = new HashMap>(); + private final Map> sourceTypeConverters = new HashMap>(36); private ConversionService parent; @@ -132,7 +131,7 @@ public class GenericConversionService implements ConversionService, ConverterReg } Class sourceType = typeInfo[0]; Class targetType = typeInfo[1]; - getSourceMap(sourceType).put(targetType, new ConverterAdapter(converter)); + addConverter(sourceType, targetType, converter); } public void addConverterFactory(ConverterFactory converterFactory) { @@ -143,7 +142,7 @@ public class GenericConversionService implements ConversionService, ConverterReg } Class sourceType = typeInfo[0]; Class targetType = typeInfo[1]; - getSourceMap(sourceType).put(targetType, new ConverterFactoryAdapter(converterFactory)); + addConverterFactory(sourceType, targetType, converterFactory); } public void removeConvertible(Class sourceType, Class targetType) { @@ -196,6 +195,28 @@ public class GenericConversionService implements ConversionService, ConverterReg getSourceMap(sourceType).put(targetType, converter); } + /** + * Registers a Converter with the sourceType and targetType to index on specified explicitly. + * This method performs better than {@link #addConverter(Converter)} because there parameterized types S and T don't have to be discovered. + * @param sourceType the source type to convert from + * @param targetType the target type to convert to + * @param converter the converter. + */ + protected void addConverter(Class sourceType, Class targetType, Converter converter) { + addGenericConverter(sourceType, targetType, new ConverterAdapter(converter)); + } + + /** + * Registers a ConverterFactory with the sourceType and targetType to index on specified explicitly. + * This method performs better than {@link #addConverter(ConverterFactory)} because there parameterized types S and T don't have to be discovered. + * @param sourceType the source type to convert from + * @param targetType the target type to convert to + * @param converter the converter.factory + */ + protected void addConverterFactory(Class sourceType, Class targetType, ConverterFactory converterFactory) { + addGenericConverter(sourceType, targetType, new ConverterFactoryAdapter(converterFactory)); + } + /** * Hook method to convert a null source. * Default implementation returns null. @@ -259,15 +280,7 @@ public class GenericConversionService implements ConversionService, ConverterReg } private Class[] getRequiredTypeInfo(Object converter, Class genericIfc) { - Class[] typeInfo = new Class[2]; - if (converter instanceof ConverterInfo) { - ConverterInfo info = (ConverterInfo) converter; - typeInfo[0] = info.getSourceType(); - typeInfo[1] = info.getTargetType(); - return typeInfo; - } else { - return GenericTypeResolver.resolveTypeArguments(converter.getClass(), genericIfc); - } + return GenericTypeResolver.resolveTypeArguments(converter.getClass(), genericIfc); } private GenericConverter findConverterByClassPair(Class sourceType, Class targetType) { diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConverter.java index 7ecd565524e..142b9939a40 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConverter.java +++ b/org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConverter.java @@ -37,7 +37,7 @@ public interface GenericConverter { /** * Convert the source to the targetType described by the TypeDescriptor. - * @param source the source object to convert (never null) + * @param source the source object to convert (may be null) * @param sourceType context about the source type to convert from * @param targetType context about the target type to convert to * @return the converted object diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/NumberToNumberConverterFactory.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/NumberToNumberConverterFactory.java index 9192d0d6b8b..273d36ab62a 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/support/NumberToNumberConverterFactory.java +++ b/org.springframework.core/src/main/java/org/springframework/core/convert/support/NumberToNumberConverterFactory.java @@ -47,11 +47,11 @@ final class NumberToNumberConverterFactory implements ConverterFactory implements Converter { private final Class targetType; - + public NumberToNumber(Class targetType) { this.targetType = targetType; } - + public T convert(Number source) { return NumberUtils.convertNumberToTargetClass(source, this.targetType); } diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/ObjectToStringConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/ObjectToStringConverter.java index b9fa55dcc6e..a81d881d929 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/support/ObjectToStringConverter.java +++ b/org.springframework.core/src/main/java/org/springframework/core/convert/support/ObjectToStringConverter.java @@ -28,6 +28,14 @@ import org.springframework.core.convert.converter.Converter; */ final class ObjectToStringConverter implements Converter { + public Class getSourceType() { + return Object.class; + } + + public Class getTargetType() { + return String.class; + } + public String convert(Object source) { return source.toString(); } diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/StringToBooleanConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/StringToBooleanConverter.java index ad28c7926c4..ef91679d18b 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/support/StringToBooleanConverter.java +++ b/org.springframework.core/src/main/java/org/springframework/core/convert/support/StringToBooleanConverter.java @@ -48,8 +48,7 @@ final class StringToBooleanConverter implements Converter { trueValues.add("1"); falseValues.add("0"); } - - + public Boolean convert(String source) { String value = (source != null ? source.trim() : null); if (!StringUtils.hasLength(value)) { diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/StringToCharacterConverter.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/StringToCharacterConverter.java index 9d1a875022d..7e3525eab7b 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/support/StringToCharacterConverter.java +++ b/org.springframework.core/src/main/java/org/springframework/core/convert/support/StringToCharacterConverter.java @@ -31,7 +31,9 @@ final class StringToCharacterConverter implements Converter { return null; } if (source.length() > 1) { - throw new IllegalArgumentException("Can only convert a [String] with length of 1 to a [Character]; string value '" + source + "' has length of " + source.length()); + throw new IllegalArgumentException( + "Can only convert a [String] with length of 1 to a [Character]; string value '" + source + + "' has length of " + source.length()); } return source.charAt(0); } diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/StringToEnumConverterFactory.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/StringToEnumConverterFactory.java index dec8e09153c..181ba2430cf 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/support/StringToEnumConverterFactory.java +++ b/org.springframework.core/src/main/java/org/springframework/core/convert/support/StringToEnumConverterFactory.java @@ -35,7 +35,7 @@ final class StringToEnumConverterFactory implements ConverterFactory implements Converter { private final Class enumType; - + public StringToEnum(Class enumType) { this.enumType = enumType; } diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/StringToNumberConverterFactory.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/StringToNumberConverterFactory.java index 5490ba264f6..9b0767cd4e0 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/support/StringToNumberConverterFactory.java +++ b/org.springframework.core/src/main/java/org/springframework/core/convert/support/StringToNumberConverterFactory.java @@ -47,11 +47,11 @@ final class StringToNumberConverterFactory implements ConverterFactory implements Converter { private final Class targetType; - + public StringToNumber(Class targetType) { this.targetType = targetType; } - + public T convert(String source) { if ("".equals(source)) { return null;