method naming improvements; applyIndexObject call for array indexing

git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@3851 50f2f4bb-b051-0410-bef5-90022cba6387

org.springframework.core/src/main/java/org/springframework/core/convert/TypeDescriptor.java

@@ -309,19 +309,33 @@ public class TypeDescriptor {
 
 	// special case public operations
 
-	public TypeDescriptor(Class<?> componentType, MethodParameter methodParameter) {
+	/**
-		if (componentType == null) {
+	 * Constructs a new TypeDescriptor for a nested type declared within a method parameter, such as a collection type or map key or value type.
-			componentType = Object.class;
+	 */
+	public TypeDescriptor(Class<?> nestedType, MethodParameter methodParameter) {
+		if (nestedType == null) {
+			nestedType = Object.class;
 		}
-		this.type = componentType;
+		this.type = nestedType;
 		this.methodParameter = methodParameter;
 	}
 	
+	/**
+	 * Exposes the underlying MethodParameter providing context for this TypeDescriptor.
+	 * Used to support legacy code scenarios where callers are already using the MethodParameter API (BeanWrapper).
+	 * In general, favor use of the TypeDescriptor API over the MethodParameter API as it is independent of type context location.
+	 * May be null if no MethodParameter was provided when this TypeDescriptor was constructed.
+	 */
 	public MethodParameter getMethodParameter() {
 		return methodParameter;
 	}
 
-	public TypeDescriptor applyType(Object object) {
+	/**
+	 * Create a copy of this nested type descriptor and apply the specific type information from the indexed object.
+	 * Used to support collection and map indexing scenarios, where the indexer has a reference to the indexed type descriptor but needs to ensure its type actually represents the indexed object type.
+	 * This is necessary to support type conversion during index object binding operations.
+	 */
+	public TypeDescriptor applyIndexedObject(Object object) {
 		if (object == null) {
 			return this;
 		}
@@ -406,45 +420,45 @@ public class TypeDescriptor {
 		}
 	}
 
-	protected TypeDescriptor newComponentTypeDescriptor(Class<?> componentType, MethodParameter nested) {
+	protected TypeDescriptor newNestedTypeDescriptor(Class<?> nestedType, MethodParameter nested) {
-		return new TypeDescriptor(componentType, nested);
+		return new TypeDescriptor(nestedType, nested);
 	}
 	
 	// internal helpers
 
 	private TypeDescriptor resolveElementTypeDescriptor() {
 		if (isCollection()) {
-			return createComponentTypeDescriptor(resolveCollectionElementType());
+			return createNestedTypeDescriptor(resolveCollectionElementType());
 		}
 		else {
 			// TODO: GenericCollectionTypeResolver is not capable of applying nesting levels to array fields;
 			// this means generic info of nested lists or maps stored inside array method parameters or fields is not obtainable
-			return createComponentTypeDescriptor(getType().getComponentType());
+			return createNestedTypeDescriptor(getType().getComponentType());
 		}
 	}
 
 	private TypeDescriptor resolveMapKeyTypeDescriptor() {
-		return createComponentTypeDescriptor(resolveMapKeyType());
+		return createNestedTypeDescriptor(resolveMapKeyType());
 	}
 
 	private TypeDescriptor resolveMapValueTypeDescriptor() {
-		return createComponentTypeDescriptor(resolveMapValueType());
+		return createNestedTypeDescriptor(resolveMapValueType());
 	}
 
-	private TypeDescriptor createComponentTypeDescriptor(Class<?> componentType) {
+	private TypeDescriptor createNestedTypeDescriptor(Class<?> nestedType) {
-		if (componentType == null) {
+		if (nestedType == null) {
-			componentType = Object.class;
+			nestedType = Object.class;
 		}
 		if (this.methodParameter != null) {
 			MethodParameter nested = new MethodParameter(this.methodParameter);
 			nested.increaseNestingLevel();
-			return newComponentTypeDescriptor(componentType, nested);				
+			return newNestedTypeDescriptor(nestedType, nested);				
 		}
 		else if (this.field != null) {
-			return new TypeDescriptor(componentType, this.field, this.fieldNestingLevel + 1);
+			return new TypeDescriptor(nestedType, this.field, this.fieldNestingLevel + 1);
 		}
 		else {
-			return TypeDescriptor.valueOf(componentType);
+			return TypeDescriptor.valueOf(nestedType);
 		}
 	}
 
@@ -486,8 +500,8 @@ public class TypeDescriptor {
 
 	// internal constructors
 
-	private TypeDescriptor(Class<?> componentType, Field field, int nestingLevel) {
+	private TypeDescriptor(Class<?> nestedType, Field field, int nestingLevel) {
-		this.type = componentType;
+		this.type = nestedType;
 		this.field = field;
 		this.fieldNestingLevel = nestingLevel;
 	}

org.springframework.core/src/main/java/org/springframework/core/convert/support/PropertyTypeDescriptor.java

@@ -50,8 +50,8 @@ public class PropertyTypeDescriptor extends TypeDescriptor {
 		this.propertyDescriptor = propertyDescriptor;
 	}
 	
-	public PropertyTypeDescriptor(Class<?> componentType, MethodParameter methodParameter, PropertyDescriptor propertyDescriptor) {
+	public PropertyTypeDescriptor(Class<?> type, MethodParameter methodParameter, PropertyDescriptor propertyDescriptor) {
-		super(componentType, methodParameter);
+		super(type, methodParameter);
 		this.propertyDescriptor = propertyDescriptor;
 	}
 
@@ -102,8 +102,8 @@ public class PropertyTypeDescriptor extends TypeDescriptor {
 		return annMap.values().toArray(new Annotation[annMap.size()]);
 	}
 
-	public TypeDescriptor newComponentTypeDescriptor(Class<?> componentType, MethodParameter nested) {
+	public TypeDescriptor newNestedTypeDescriptor(Class<?> nestedType, MethodParameter nested) {
-		return new PropertyTypeDescriptor(componentType, nested,  this.propertyDescriptor);
+		return new PropertyTypeDescriptor(nestedType, nested, this.propertyDescriptor);
 	}
 
 }

org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java

@@ -93,7 +93,7 @@ public class Indexer extends SpelNodeImpl {
 			Object possiblyConvertedKey = index;
 			possiblyConvertedKey = state.convertValue(index, targetObjectTypeDescriptor.getMapKeyTypeDescriptor());
 			Object o = ((Map<?, ?>) targetObject).get(possiblyConvertedKey);
-			return new TypedValue(o, targetObjectTypeDescriptor.getMapValueTypeDescriptor().applyType(o));
+			return new TypedValue(o, targetObjectTypeDescriptor.getMapValueTypeDescriptor().applyIndexedObject(o));
 		}
 		
 		if (targetObject == null) {
@@ -104,7 +104,8 @@ public class Indexer extends SpelNodeImpl {
 		if ((targetObject instanceof Collection ) || targetObject.getClass().isArray() || targetObject instanceof String) {
 			int idx = (Integer)state.convertValue(index, TypeDescriptor.valueOf(Integer.class));		
 			if (targetObject.getClass().isArray()) {
-				return new TypedValue(accessArrayElement(targetObject, idx), targetObjectTypeDescriptor.getElementTypeDescriptor());
+				Object arrayElement = accessArrayElement(targetObject, idx);
+				return new TypedValue(arrayElement, targetObjectTypeDescriptor.getElementTypeDescriptor().applyIndexedObject(arrayElement));
 			} else if (targetObject instanceof Collection) {
 				Collection c = (Collection) targetObject;
 				if (idx >= c.size()) {
@@ -115,7 +116,7 @@ public class Indexer extends SpelNodeImpl {
 				int pos = 0;
 				for (Object o : c) {
 					if (pos == idx) {
-						return new TypedValue(o, targetObjectTypeDescriptor.getElementTypeDescriptor().applyType(o));
+						return new TypedValue(o, targetObjectTypeDescriptor.getElementTypeDescriptor().applyIndexedObject(o));
 					}
 					pos++;
 				}

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/Spr7839Tests.java

@@ -2,11 +2,13 @@ package org.springframework.web.servlet.mvc.annotation;
 
 import static org.junit.Assert.assertEquals;
 
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-import org.junit.Ignore;
+import org.junit.Before;
 import org.junit.Test;
+import org.springframework.core.convert.converter.Converter;
 import org.springframework.core.convert.support.ConversionServiceFactory;
 import org.springframework.core.convert.support.GenericConversionService;
 import org.springframework.mock.web.MockHttpServletRequest;
@@ -17,30 +19,78 @@ import org.springframework.web.bind.support.ConfigurableWebBindingInitializer;
 
 public class Spr7839Tests {
 
-	@Test
+	AnnotationMethodHandlerAdapter adapter = new AnnotationMethodHandlerAdapter();
-	@Ignore
+
-	public void test() throws Exception {
+	MockHttpServletRequest request = new MockHttpServletRequest();
-		AnnotationMethodHandlerAdapter adapter = new AnnotationMethodHandlerAdapter();
+
+	MockHttpServletResponse response = new MockHttpServletResponse();
+
+	Spr7839Controller controller = new Spr7839Controller();
+
+	@Before
+	public void setUp() {
 		ConfigurableWebBindingInitializer binder = new ConfigurableWebBindingInitializer();
 		GenericConversionService service = ConversionServiceFactory.createDefaultConversionService();
+		service.addConverter(new Converter<String, NestedBean>() {
+			public NestedBean convert(String source) {
+				return new NestedBean(source);
+			}
+		});
 		binder.setConversionService(service);
 		adapter.setWebBindingInitializer(binder);
-		Spr7839Controller controller = new Spr7839Controller();
+	}
-		MockHttpServletRequest request = new MockHttpServletRequest();
+
+	@Test
+	public void object() throws Exception {
 		request.setRequestURI("/nested");
-		request.setPathInfo("/nested");
+		request.addParameter("nested", "Nested");
-		request.addParameter("nested.map['apple'].foo", "bar");
+		adapter.handle(request, response, controller);
-		MockHttpServletResponse response = new MockHttpServletResponse();
+	}
+
+	@Test
+	public void list() throws Exception {
+		request.setRequestURI("/nested/list");
+		request.addParameter("nested.list", "Nested1,Nested2");
 		adapter.handle(request, response, controller);
 	}
 	
+	@Test
+	public void listElement() throws Exception {
+		request.setRequestURI("/nested/listElement");
+		request.addParameter("nested.list[0]", "Nested");
+		adapter.handle(request, response, controller);
+	}
+
+	@Test
+	public void map() throws Exception {
+		request.setRequestURI("/nested/map");		
+		request.addParameter("nested.map['apple'].foo", "bar");
+		adapter.handle(request, response, controller);
+	}
+
 	@Controller
 	public static class Spr7839Controller {
 
 		@RequestMapping("/nested")
 		public void handler(JavaBean bean) {
+			assertEquals("Nested", bean.nested.foo);
+		}
+
+		@RequestMapping("/nested/list")
+		public void handlerList(JavaBean bean) {
+			assertEquals("Nested2", bean.nested.list.get(1).foo);
+		}
+
+		@RequestMapping("/nested/map")
+		public void handlerMap(JavaBean bean) {
 			assertEquals("bar", bean.nested.map.get("apple").foo);
 		}
+
+		@RequestMapping("/nested/listElement")
+		public void handlerListElement(JavaBean bean) {
+			assertEquals("Nested", bean.nested.list.get(0).foo);
+		}
+
 	}
 	
 	public static class JavaBean {
@@ -64,8 +114,16 @@ public class Spr7839Tests {
 
 	    private List<NestedBean> list;
 		
-	    private Map<String, NestedBean> map;
+	    private Map<String, NestedBean> map = new HashMap<String, NestedBean>();
 
+	    public NestedBean() {
+	    	
+	    }
+	    
+	    public NestedBean(String foo) {
+	    	this.foo = foo;
+	    }
+	    
 		public String getFoo() {
 			return foo;
 		}