fixed rendering of select options for multi-list (SPR-6799)

org.springframework.context/src/main/java/org/springframework/validation/AbstractPropertyBindingResult.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2009 the original author or authors.
+ * Copyright 2002-2010 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.
@@ -145,14 +145,22 @@ public abstract class AbstractPropertyBindingResult extends AbstractBindingResul
 	 */
 	@Override
 	public PropertyEditor findEditor(String field, Class valueType) {
-		if (valueType == null) {
-			valueType = getFieldType(field);
+		Class valueTypeForLookup = valueType;
+		if (valueTypeForLookup == null) {
+			valueTypeForLookup = getFieldType(field);
 		}
-		PropertyEditor editor = super.findEditor(field, valueType);
+		PropertyEditor editor = super.findEditor(field, valueTypeForLookup);
 		if (editor == null && this.conversionService != null) {
-			TypeDescriptor td = (field != null ?
-					getPropertyAccessor().getPropertyTypeDescriptor(fixedField(field)) :
-					TypeDescriptor.valueOf(valueType));
+			TypeDescriptor td = null;
+			if (field != null) {
+				TypeDescriptor ptd = getPropertyAccessor().getPropertyTypeDescriptor(fixedField(field));
+				if (valueType == null || valueType.isAssignableFrom(ptd.getType())) {
+					td = ptd;
+				}
+			}
+			if (td == null) {
+				td = TypeDescriptor.valueOf(valueTypeForLookup);
+			}
 			if (this.conversionService.canConvert(TypeDescriptor.valueOf(String.class), td)) {
 				editor = new ConvertingPropertyEditorAdapter(this.conversionService, td);
 			}

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/tags/form/OptionWriter.java

@@ -16,8 +16,8 @@
 
 package org.springframework.web.servlet.tags.form;
 
+import java.beans.PropertyEditor;
 import java.util.Collection;
-import java.util.Iterator;
 import java.util.Map;
 import javax.servlet.jsp.JspException;
 
@@ -158,17 +158,16 @@ class OptionWriter {
 	 * @see #renderOption(TagWriter, Object, Object, Object)
 	 */
 	private void renderFromMap(final TagWriter tagWriter) throws JspException {
-		Map optionMap = (Map) this.optionSource;
-		for (Iterator iterator = optionMap.entrySet().iterator(); iterator.hasNext();) {
-			Map.Entry entry = (Map.Entry) iterator.next();
+		Map<?, ?> optionMap = (Map) this.optionSource;
+		for (Map.Entry entry : optionMap.entrySet()) {
 			Object mapKey = entry.getKey();
 			Object mapValue = entry.getValue();
 			BeanWrapper mapKeyWrapper = PropertyAccessorFactory.forBeanPropertyAccess(mapKey);
 			BeanWrapper mapValueWrapper = PropertyAccessorFactory.forBeanPropertyAccess(mapValue);
-			Object renderValue = (this.valueProperty != null ?
-					mapKeyWrapper.getPropertyValue(this.valueProperty) : mapKey.toString());
-			Object renderLabel = (this.labelProperty != null ?
-					mapValueWrapper.getPropertyValue(this.labelProperty) : mapValue.toString());
+			Object renderValue = (this.valueProperty != null ? mapKeyWrapper.getPropertyValue(this.valueProperty) :
+					mapKey.toString());
+			Object renderLabel = (this.labelProperty != null ? mapValueWrapper.getPropertyValue(this.labelProperty) :
+					mapValue.toString());
 			renderOption(tagWriter, mapKey, renderValue, renderLabel);
 		}
 	}
@@ -196,16 +195,15 @@ class OptionWriter {
 	 * {@link #labelProperty} property is used when rendering the label.
 	 */
 	private void doRenderFromCollection(Collection optionCollection, TagWriter tagWriter) throws JspException {
-		for (Iterator it = optionCollection.iterator(); it.hasNext();) {
-			Object item = it.next();
+		for (Object item : optionCollection) {
 			BeanWrapper wrapper = PropertyAccessorFactory.forBeanPropertyAccess(item);
 			Object value;
 			if (this.valueProperty != null) {
 				value = wrapper.getPropertyValue(this.valueProperty);
-			} 
+			}
 			else if (item instanceof Enum) {
 				value = ((Enum<?>) item).name();
-			} 
+			}
 			else {
 				value = item;
 			}
@@ -243,7 +241,8 @@ class OptionWriter {
 	 * HTML-escaped as required.
 	 */
 	private String getDisplayString(Object value) {
-		return ValueFormatter.getDisplayString(value, this.bindStatus.getEditor(), this.htmlEscape);
+		PropertyEditor editor = (value != null ? this.bindStatus.findEditor(value.getClass()) : null);
+		return ValueFormatter.getDisplayString(value, editor, this.htmlEscape);
 	}
 
 	/**

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/tags/form/SelectTag.java

@@ -205,7 +205,7 @@ public class SelectTag extends AbstractHtmlInputElementTag {
 		if (items != null) {
 			// Items specified, but might still be empty...
 			if (items != EMPTY) {
-				Object itemsObject = (items instanceof String ? evaluate("items", (String) items) : items);
+				Object itemsObject = evaluate("items", items);
 				if (itemsObject != null) {
 					String valueProperty = (getItemValue() != null ?
 							ObjectUtils.getDisplayString(evaluate("itemValue", getItemValue())) : null);

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/tags/form/SelectTagTests.java

@@ -19,6 +19,7 @@ package org.springframework.web.servlet.tags.form;
 import java.beans.PropertyEditor;
 import java.beans.PropertyEditorSupport;
 import java.io.StringReader;
+import java.text.ParseException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
@@ -28,7 +29,6 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.TreeMap;
-
 import javax.servlet.jsp.JspException;
 import javax.servlet.jsp.tagext.Tag;
 
@@ -40,6 +40,8 @@ import org.dom4j.io.SAXReader;
 
 import org.springframework.beans.TestBean;
 import org.springframework.beans.propertyeditors.CustomCollectionEditor;
+import org.springframework.format.Formatter;
+import org.springframework.format.support.FormattingConversionService;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.validation.BeanPropertyBindingResult;
 import org.springframework.validation.BindingResult;
@@ -59,7 +61,7 @@ public class SelectTagTests extends AbstractFormTagTests {
 
 	private SelectTag tag;
 
-	private TestBean bean;
+	private TestBeanWithRealCountry bean;
 
 
 	protected void onSetUp() {
@@ -454,9 +456,102 @@ public class SelectTagTests extends AbstractFormTagTests {
 
 		Element e = (Element) selectElement.selectSingleNode("option[@value = 'UK']");
 		assertEquals("UK node not selected", "selected", e.attribute("selected").getValue());
+		assertEquals("United Kingdom(UK)", e.getText());
+
+		e = (Element) selectElement.selectSingleNode("option[@value = 'AT']");
+		assertEquals("AT node not selected", "selected", e.attribute("selected").getValue());
+		assertEquals("Austria(AT)", e.getText());
+	}
+
+	public void testWithElementConverter() throws Exception {
+		this.bean.setRealCountry(Country.COUNTRY_UK);
+
+		BeanPropertyBindingResult errors = new BeanPropertyBindingResult(this.bean, COMMAND_NAME);
+		FormattingConversionService cs = new FormattingConversionService();
+		cs.addFormatterForFieldType(Country.class, new Formatter<Country>() {
+			public String print(Country object, Locale locale) {
+				return object.getName();
+			}
+			public Country parse(String text, Locale locale) throws ParseException {
+				return new Country(text, text);
+			}
+		});
+		errors.initConversion(cs);
+		exposeBindingResult(errors);
+
+		this.tag.setPath("realCountry");
+		this.tag.setItems("${countries}");
+		this.tag.setItemValue("isoCode");
+		int result = this.tag.doStartTag();
+		assertEquals(Tag.SKIP_BODY, result);
+
+		String output = getOutput();
+		output = "<doc>" + output + "</doc>";
+
+		SAXReader reader = new SAXReader();
+		Document document = reader.read(new StringReader(output));
+		Element rootElement = document.getRootElement();
+		assertEquals(1, rootElement.elements().size());
+
+		Element selectElement = rootElement.element("select");
+		assertEquals("select", selectElement.getName());
+		assertEquals("realCountry", selectElement.attribute("name").getValue());
+
+		List children = selectElement.elements();
+		assertEquals("Incorrect number of children", 4, children.size());
+
+		Element e = (Element) selectElement.selectSingleNode("option[@value = 'UK']");
+		assertEquals("UK node not selected", "selected", e.attribute("selected").getValue());
+		assertEquals("United Kingdom", e.getText());
+	}
+
+	public void testWithMultiListAndElementConverter() throws Exception {
+		List list = new ArrayList();
+		list.add(Country.COUNTRY_UK);
+		list.add(Country.COUNTRY_AT);
+		this.bean.setSomeList(list);
+
+		BeanPropertyBindingResult errors = new BeanPropertyBindingResult(this.bean, COMMAND_NAME);
+		FormattingConversionService cs = new FormattingConversionService();
+		cs.addFormatterForFieldType(Country.class, new Formatter<Country>() {
+			public String print(Country object, Locale locale) {
+				return object.getName();
+			}
+			public Country parse(String text, Locale locale) throws ParseException {
+				return new Country(text, text);
+			}
+		});
+		errors.initConversion(cs);
+		exposeBindingResult(errors);
+
+		this.tag.setPath("someList");
+		this.tag.setItems("${countries}");
+		this.tag.setItemValue("isoCode");
+		int result = this.tag.doStartTag();
+		assertEquals(Tag.SKIP_BODY, result);
+
+		String output = getOutput();
+		output = "<doc>" + output + "</doc>";
+
+		SAXReader reader = new SAXReader();
+		Document document = reader.read(new StringReader(output));
+		Element rootElement = document.getRootElement();
+		assertEquals(2, rootElement.elements().size());
+
+		Element selectElement = rootElement.element("select");
+		assertEquals("select", selectElement.getName());
+		assertEquals("someList", selectElement.attribute("name").getValue());
+
+		List children = selectElement.elements();
+		assertEquals("Incorrect number of children", 4, children.size());
+
+		Element e = (Element) selectElement.selectSingleNode("option[@value = 'UK']");
+		assertEquals("UK node not selected", "selected", e.attribute("selected").getValue());
+		assertEquals("United Kingdom", e.getText());
 
 		e = (Element) selectElement.selectSingleNode("option[@value = 'AT']");
 		assertEquals("AT node not selected", "selected", e.attribute("selected").getValue());
+		assertEquals("Austria", e.getText());
 	}
 
 	public void testWithMultiListAndCustomEditor() throws Exception {