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 50afa02e54d..bc03395ab9f 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 @@ -44,6 +44,7 @@ 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); } 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 07202934a2c..1174c45bb8b 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,11 +22,9 @@ import java.util.List; import java.util.Map; import java.util.Set; -import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.TypeDescriptor; import org.springframework.core.convert.converter.ConverterRegistry; import org.springframework.core.convert.support.DefaultConversionService; -import org.springframework.core.convert.support.GenericConversionService; import org.springframework.core.convert.support.GenericConverter; import org.springframework.expression.EvaluationContext; import org.springframework.expression.EvaluationException; @@ -55,7 +53,7 @@ public class SpelMapper implements Mapper { SpelExpressionParserConfiguration.CreateObjectIfAttemptToReferenceNull | SpelExpressionParserConfiguration.GrowListsOnIndexBeyondSize); - private final Set mappings = new LinkedHashSet(); + private final Set mappings = new LinkedHashSet(); private boolean autoMappingEnabled = true; @@ -65,10 +63,6 @@ public class SpelMapper implements Mapper { this.autoMappingEnabled = autoMappingEnabled; } - public void setConversionService(ConversionService conversionService) { - this.conversionService.setParent(conversionService); - } - public MappingConfiguration addMapping(String fieldExpression) { return addMapping(fieldExpression, fieldExpression); } @@ -92,7 +86,7 @@ public class SpelMapper implements Mapper { throw new IllegalArgumentException("The mapping target '" + targetFieldExpression + "' is not a parseable property expression", e); } - Mapping mapping = new Mapping(sourceExp, targetExp); + SpelMapping mapping = new SpelMapping(sourceExp, targetExp); this.mappings.add(mapping); return mapping; } @@ -101,11 +95,11 @@ public class SpelMapper implements Mapper { EvaluationContext sourceContext = getMappingContext(source); EvaluationContext targetContext = getMappingContext(target); List failures = new LinkedList(); - for (Mapping mapping : this.mappings) { + for (SpelMapping mapping : this.mappings) { mapping.map(sourceContext, targetContext, failures); } - Set autoMappings = getAutoMappings(sourceContext, targetContext); - for (Mapping mapping : autoMappings) { + Set autoMappings = getAutoMappings(sourceContext, targetContext); + for (SpelMapping mapping : autoMappings) { mapping.map(sourceContext, targetContext, failures); } if (!failures.isEmpty()) { @@ -118,9 +112,9 @@ public class SpelMapper implements Mapper { return mappableTypeFactory.getMappableType(object).getEvaluationContext(object, this.conversionService); } - private Set getAutoMappings(EvaluationContext sourceContext, EvaluationContext targetContext) { + private Set getAutoMappings(EvaluationContext sourceContext, EvaluationContext targetContext) { if (this.autoMappingEnabled) { - Set autoMappings = new LinkedHashSet(); + Set autoMappings = new LinkedHashSet(); Set sourceFields = getMappableFields(sourceContext.getRootObject().getValue()); for (String field : sourceFields) { if (!explicitlyMapped(field)) { @@ -140,7 +134,7 @@ public class SpelMapper implements Mapper { } try { if (targetExpression.isWritable(targetContext)) { - autoMappings.add(new Mapping(sourceExpression, targetExpression)); + autoMappings.add(new SpelMapping(sourceExpression, targetExpression)); } } catch (EvaluationException e) { @@ -158,7 +152,7 @@ public class SpelMapper implements Mapper { } private boolean explicitlyMapped(String field) { - for (Mapping mapping : this.mappings) { + for (SpelMapping mapping : this.mappings) { if (mapping.getSourceExpressionString().startsWith(field)) { return true; } @@ -166,16 +160,11 @@ public class SpelMapper implements Mapper { return false; } - private class MappingConversionService extends GenericConversionService { - - public MappingConversionService() { - setParent(new DefaultConversionService()); - } + private static class MappingConversionService extends DefaultConversionService { @Override - protected GenericConverter getConverter(TypeDescriptor sourceType, TypeDescriptor targetType) { - GenericConverter converter = super.getConverter(sourceType, targetType); - return converter != null ? converter : new MapperConverter(new SpelMapper()); + protected GenericConverter getDefaultConverter(TypeDescriptor sourceType, TypeDescriptor targetType) { + return new MapperConverter(new SpelMapper()); } } diff --git a/org.springframework.context/src/main/java/org/springframework/mapping/support/SpelMapping.java b/org.springframework.context/src/main/java/org/springframework/mapping/support/SpelMapping.java new file mode 100644 index 00000000000..3d84b497ce4 --- /dev/null +++ b/org.springframework.context/src/main/java/org/springframework/mapping/support/SpelMapping.java @@ -0,0 +1,98 @@ +/* + * 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.mapping.support; + +import java.util.Collection; + +import org.springframework.core.convert.converter.Converter; +import org.springframework.core.convert.converter.ConverterFactory; +import org.springframework.core.convert.support.ConverterFactoryGenericConverter; +import org.springframework.core.convert.support.ConverterGenericConverter; +import org.springframework.core.convert.support.GenericConverter; +import org.springframework.expression.EvaluationContext; +import org.springframework.expression.Expression; +import org.springframework.mapping.MappingFailure; + +/** + * An individual mapping definition between two fields. + * @author Keith Donald + */ +class SpelMapping implements MappingConfiguration { + + private Expression source; + + private Expression target; + + private GenericConverter converter; + + public SpelMapping(Expression source, Expression target) { + this.source = source; + this.target = target; + } + + public String getSourceExpressionString() { + return this.source.getExpressionString(); + } + + public String getTargetExpressionString() { + return this.target.getExpressionString(); + } + + public MappingConfiguration setConverter(Converter converter) { + return setGenericConverter(new ConverterGenericConverter(converter)); + } + + public MappingConfiguration setConverterFactory(ConverterFactory converter) { + return setGenericConverter(new ConverterFactoryGenericConverter(converter)); + } + + public MappingConfiguration setGenericConverter(GenericConverter converter) { + this.converter = converter; + return this; + } + + public void map(EvaluationContext sourceContext, EvaluationContext targetContext, + Collection failures) { + try { + Object value = this.source.getValue(sourceContext); + if (this.converter != null) { + value = this.converter.convert(value, this.source.getValueTypeDescriptor(sourceContext), this.target + .getValueTypeDescriptor(targetContext)); + } + this.target.setValue(targetContext, value); + } catch (Exception e) { + failures.add(new MappingFailure(e)); + } + } + + public int hashCode() { + return getSourceExpressionString().hashCode() + getTargetExpressionString().hashCode(); + } + + public boolean equals(Object o) { + if (!(o instanceof SpelMapping)) { + return false; + } + SpelMapping m = (SpelMapping) o; + return getSourceExpressionString().equals(m.getSourceExpressionString()) + && getTargetExpressionString().equals(m.getTargetExpressionString()); + } + + public String toString() { + return "[Mapping<" + getSourceExpressionString() + " -> " + getTargetExpressionString() + ">]"; + } + +} \ No newline at end of file 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 696abd8ffdf..a3b4b42ce20 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 @@ -8,6 +8,7 @@ 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; @@ -122,7 +123,7 @@ public class SpelMapperTests { Person target = new Person(); - mapper.addMapping("nested.foo", "nested.foo"); + mapper.addMapping("nested.foo"); mapper.map(source, target); assertEquals("bar", target.nested.foo); @@ -242,6 +243,22 @@ public class SpelMapperTests { assertEquals(1, e.getMappingFailureCount()); } } + + @Test + @Ignore + public void mapCyclic() { + Person source = new Person(); + source.setName("Keith"); + source.setAge(3); + source.setFavoriteSport(Sport.FOOTBALL); + source.cyclic = source; + Person target = new Person(); + mapper.map(source, target); + assertEquals("Keith", target.getName()); + assertEquals(3, target.getAge()); + assertEquals(Sport.FOOTBALL, target.getFavoriteSport()); + assertEquals(source.cyclic, target.cyclic); + } public static class PersonDto { @@ -326,7 +343,7 @@ public class SpelMapperTests { private Nested nested; - // private Person cyclic; + private Person cyclic; private List favoriteSports; @@ -372,7 +389,6 @@ public class SpelMapperTests { this.nested = nested; } - /* public Person getCyclic() { return cyclic; } @@ -380,7 +396,6 @@ public class SpelMapperTests { public void setCyclic(Person cyclic) { this.cyclic = cyclic; } - */ public List getFavoriteSports() { return favoriteSports; 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 70c221837ba..0af53dd94bd 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,8 +16,6 @@ package org.springframework.core.convert.support; -import java.util.Collection; -import java.util.Map; /** * Default implementation of a conversion service. Will automatically register from string @@ -33,21 +31,6 @@ public class DefaultConversionService extends GenericConversionService { * Create a new default conversion service, installing the default converters. */ public DefaultConversionService() { - addGenericConverter(Object[].class, Object[].class, new ArrayToArrayConverter(this)); - addGenericConverter(Object[].class, Collection.class, new ArrayToCollectionConverter(this)); - addGenericConverter(Object[].class, Map.class, new ArrayToMapConverter(this)); - addGenericConverter(Object[].class, Object.class, new ArrayToObjectConverter(this)); - addGenericConverter(Collection.class, Collection.class, new CollectionToCollectionConverter(this)); - addGenericConverter(Collection.class, Object[].class, new CollectionToArrayConverter(this)); - addGenericConverter(Collection.class, Map.class, new CollectionToMapConverter(this)); - addGenericConverter(Collection.class, Object.class, new CollectionToObjectConverter(this)); - addGenericConverter(Map.class, Map.class, new MapToMapConverter(this)); - addGenericConverter(Map.class, Object[].class, new MapToArrayConverter(this)); - addGenericConverter(Map.class, Collection.class, new MapToCollectionConverter(this)); - addGenericConverter(Map.class, Object.class, new MapToObjectConverter(this)); - addGenericConverter(Object.class, Object[].class, new ObjectToArrayConverter(this)); - addGenericConverter(Object.class, Collection.class, new ObjectToCollectionConverter(this)); - addGenericConverter(Object.class, Map.class, new ObjectToMapConverter(this)); addConverter(new StringToBooleanConverter()); addConverter(new StringToCharacterConverter()); addConverter(new StringToLocaleConverter()); 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 4f5284e4576..3c842de818c 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 @@ -69,7 +69,6 @@ public class GenericConversionService implements ConversionService, ConverterReg * Generic converters for Collection types are registered. */ public GenericConversionService() { - // TODO should these only be registered in DefaultConversionService? addGenericConverter(Object[].class, Object[].class, new ArrayToArrayConverter(this)); addGenericConverter(Object[].class, Collection.class, new ArrayToCollectionConverter(this)); addGenericConverter(Object[].class, Map.class, new ArrayToMapConverter(this)); @@ -193,7 +192,6 @@ public class GenericConversionService implements ConversionService, ConverterReg * @param targetType the target type to convert to * @param converter the generic converter. */ - // TODO should this be public? protected void addGenericConverter(Class sourceType, Class targetType, GenericConverter converter) { getSourceMap(sourceType).put(targetType, converter); } @@ -219,7 +217,6 @@ public class GenericConversionService implements ConversionService, ConverterReg * Hook method to lookup the converter for a given sourceType/targetType pair. * First queries this ConversionService's converter map. * If no suitable Converter is found, and a {@link #setParent parent} is set, then queries the parent. - * If still no suitable Converter is found, returns a NO_OP Converter if the sourceType and targetType are assignable. * Returns null if this ConversionService simply cannot convert between sourceType and targetType. * Subclasses may override. * @param sourceType the source type to convert from @@ -233,11 +230,24 @@ public class GenericConversionService implements ConversionService, ConverterReg } else if (this.parent != null && this.parent.canConvert(sourceType, targetType)) { return this.parentConverterAdapter; } else { - if (sourceType.isAssignableTo(targetType)) { - return NO_OP_CONVERTER; - } else { - return null; - } + return getDefaultConverter(sourceType, targetType); + } + } + + /** + * Return the default converter if no converter is found for the given sourceType/targetType pair. + * Returns a NO_OP Converter if the sourceType is assignalbe to the targetType. + * Returns null otherwise, indicating no suitable converter could be found. + * Subclasses may override. + * @param sourceType the source type to convert from + * @param targetType the target type to convert to + * @return the default generic converter that will perform the conversion + */ + protected GenericConverter getDefaultConverter(TypeDescriptor sourceType, TypeDescriptor targetType) { + if (sourceType.isAssignableTo(targetType)) { + return NO_OP_CONVERTER; + } else { + return null; } }