spel mapping, polish

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);
 	}

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<Object, Object> {
 			SpelExpressionParserConfiguration.CreateObjectIfAttemptToReferenceNull
 					| SpelExpressionParserConfiguration.GrowListsOnIndexBeyondSize);
 
-	private final Set<Mapping> mappings = new LinkedHashSet<Mapping>();
+	private final Set<SpelMapping> mappings = new LinkedHashSet<SpelMapping>();
 
 	private boolean autoMappingEnabled = true;
 
@@ -65,10 +63,6 @@ public class SpelMapper implements Mapper<Object, Object> {
 		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<Object, Object> {
 			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<Object, Object> {
 		EvaluationContext sourceContext = getMappingContext(source);
 		EvaluationContext targetContext = getMappingContext(target);
 		List<MappingFailure> failures = new LinkedList<MappingFailure>();
-		for (Mapping mapping : this.mappings) {
+		for (SpelMapping mapping : this.mappings) {
 			mapping.map(sourceContext, targetContext, failures);
 		}
-		Set<Mapping> autoMappings = getAutoMappings(sourceContext, targetContext);
+		Set<SpelMapping> autoMappings = getAutoMappings(sourceContext, targetContext);
-		for (Mapping mapping : autoMappings) {
+		for (SpelMapping mapping : autoMappings) {
 			mapping.map(sourceContext, targetContext, failures);
 		}
 		if (!failures.isEmpty()) {
@@ -118,9 +112,9 @@ public class SpelMapper implements Mapper<Object, Object> {
 		return mappableTypeFactory.getMappableType(object).getEvaluationContext(object, this.conversionService);
 	}
 
-	private Set<Mapping> getAutoMappings(EvaluationContext sourceContext, EvaluationContext targetContext) {
+	private Set<SpelMapping> getAutoMappings(EvaluationContext sourceContext, EvaluationContext targetContext) {
 		if (this.autoMappingEnabled) {
-			Set<Mapping> autoMappings = new LinkedHashSet<Mapping>();
+			Set<SpelMapping> autoMappings = new LinkedHashSet<SpelMapping>();
 			Set<String> sourceFields = getMappableFields(sourceContext.getRootObject().getValue());
 			for (String field : sourceFields) {
 				if (!explicitlyMapped(field)) {
@@ -140,7 +134,7 @@ public class SpelMapper implements Mapper<Object, Object> {
 					}
 					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<Object, Object> {
 	}
 
 	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<Object, Object> {
 		return false;
 	}
 
-	private class MappingConversionService extends GenericConversionService {
+	private static class MappingConversionService extends DefaultConversionService {
-
-		public MappingConversionService() {
-			setParent(new DefaultConversionService());
-		}
 
 		@Override
-		protected GenericConverter getConverter(TypeDescriptor sourceType, TypeDescriptor targetType) {
+		protected GenericConverter getDefaultConverter(TypeDescriptor sourceType, TypeDescriptor targetType) {
-			GenericConverter converter = super.getConverter(sourceType, targetType);
+			return new MapperConverter(new SpelMapper());
-			return converter != null ? converter : new MapperConverter(new SpelMapper());
 		}
 
 	}

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<MappingFailure> 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() + ">]";
+	}
+
+}

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);
@@ -243,6 +244,22 @@ public class SpelMapperTests {
 		}
 	}
 	
+	@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 {
 
 		private String fullName;
@@ -326,7 +343,7 @@ public class SpelMapperTests {
 
 		private Nested nested;
 
-		// private Person cyclic;
+		private Person cyclic;
 
 		private List<Sport> 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<Sport> getFavoriteSports() {
 			return favoriteSports;

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 <i>from string</i>
@@ -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());

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 <code>null</code> 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 getDefaultConverter(sourceType, targetType);
-				return NO_OP_CONVERTER;
+		}
-			} else {
+	}
-				return null;
+
-			}
+	/**
+	 * 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 <code>null</code> 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;
 		}
 	}