Skip to content

Commit cdee538

Browse files
jhoellerjhoeller
committed
fixed rendering of select options for multi-list (SPR-6799)
1 parent d9112d0 commit cdee538

File tree

4 files changed

+125
-23
lines changed

4 files changed

+125
-23
lines changed

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

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2009 the original author or authors.
2+
* Copyright 2002-2010 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -145,14 +145,22 @@ protected PropertyEditor getCustomEditor(String fixedField) {
145145
*/
146146
@Override
147147
public PropertyEditor findEditor(String field, Class valueType) {
148-
if (valueType == null) {
149-
valueType = getFieldType(field);
148+
Class valueTypeForLookup = valueType;
149+
if (valueTypeForLookup == null) {
150+
valueTypeForLookup = getFieldType(field);
150151
}
151-
PropertyEditor editor = super.findEditor(field, valueType);
152+
PropertyEditor editor = super.findEditor(field, valueTypeForLookup);
152153
if (editor == null && this.conversionService != null) {
153-
TypeDescriptor td = (field != null ?
154-
getPropertyAccessor().getPropertyTypeDescriptor(fixedField(field)) :
155-
TypeDescriptor.valueOf(valueType));
154+
TypeDescriptor td = null;
155+
if (field != null) {
156+
TypeDescriptor ptd = getPropertyAccessor().getPropertyTypeDescriptor(fixedField(field));
157+
if (valueType == null || valueType.isAssignableFrom(ptd.getType())) {
158+
td = ptd;
159+
}
160+
}
161+
if (td == null) {
162+
td = TypeDescriptor.valueOf(valueTypeForLookup);
163+
}
156164
if (this.conversionService.canConvert(TypeDescriptor.valueOf(String.class), td)) {
157165
editor = new ConvertingPropertyEditorAdapter(this.conversionService, td);
158166
}

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

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616

1717
package org.springframework.web.servlet.tags.form;
1818

19+
import java.beans.PropertyEditor;
1920
import java.util.Collection;
20-
import java.util.Iterator;
2121
import java.util.Map;
2222
import javax.servlet.jsp.JspException;
2323

@@ -158,17 +158,16 @@ private void renderFromArray(TagWriter tagWriter) throws JspException {
158158
* @see #renderOption(TagWriter, Object, Object, Object)
159159
*/
160160
private void renderFromMap(final TagWriter tagWriter) throws JspException {
161-
Map optionMap = (Map) this.optionSource;
162-
for (Iterator iterator = optionMap.entrySet().iterator(); iterator.hasNext();) {
163-
Map.Entry entry = (Map.Entry) iterator.next();
161+
Map<?, ?> optionMap = (Map) this.optionSource;
162+
for (Map.Entry entry : optionMap.entrySet()) {
164163
Object mapKey = entry.getKey();
165164
Object mapValue = entry.getValue();
166165
BeanWrapper mapKeyWrapper = PropertyAccessorFactory.forBeanPropertyAccess(mapKey);
167166
BeanWrapper mapValueWrapper = PropertyAccessorFactory.forBeanPropertyAccess(mapValue);
168-
Object renderValue = (this.valueProperty != null ?
169-
mapKeyWrapper.getPropertyValue(this.valueProperty) : mapKey.toString());
170-
Object renderLabel = (this.labelProperty != null ?
171-
mapValueWrapper.getPropertyValue(this.labelProperty) : mapValue.toString());
167+
Object renderValue = (this.valueProperty != null ? mapKeyWrapper.getPropertyValue(this.valueProperty) :
168+
mapKey.toString());
169+
Object renderLabel = (this.labelProperty != null ? mapValueWrapper.getPropertyValue(this.labelProperty) :
170+
mapValue.toString());
172171
renderOption(tagWriter, mapKey, renderValue, renderLabel);
173172
}
174173
}
@@ -196,16 +195,15 @@ private void renderFromEnum(final TagWriter tagWriter) throws JspException {
196195
* {@link #labelProperty} property is used when rendering the label.
197196
*/
198197
private void doRenderFromCollection(Collection optionCollection, TagWriter tagWriter) throws JspException {
199-
for (Iterator it = optionCollection.iterator(); it.hasNext();) {
200-
Object item = it.next();
198+
for (Object item : optionCollection) {
201199
BeanWrapper wrapper = PropertyAccessorFactory.forBeanPropertyAccess(item);
202200
Object value;
203201
if (this.valueProperty != null) {
204202
value = wrapper.getPropertyValue(this.valueProperty);
205-
}
203+
}
206204
else if (item instanceof Enum) {
207205
value = ((Enum<?>) item).name();
208-
}
206+
}
209207
else {
210208
value = item;
211209
}
@@ -243,7 +241,8 @@ private void renderOption(TagWriter tagWriter, Object item, Object value, Object
243241
* HTML-escaped as required.
244242
*/
245243
private String getDisplayString(Object value) {
246-
return ValueFormatter.getDisplayString(value, this.bindStatus.getEditor(), this.htmlEscape);
244+
PropertyEditor editor = (value != null ? this.bindStatus.findEditor(value.getClass()) : null);
245+
return ValueFormatter.getDisplayString(value, editor, this.htmlEscape);
247246
}
248247

249248
/**

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ protected int writeTagContent(TagWriter tagWriter) throws JspException {
205205
if (items != null) {
206206
// Items specified, but might still be empty...
207207
if (items != EMPTY) {
208-
Object itemsObject = (items instanceof String ? evaluate("items", (String) items) : items);
208+
Object itemsObject = evaluate("items", items);
209209
if (itemsObject != null) {
210210
String valueProperty = (getItemValue() != null ?
211211
ObjectUtils.getDisplayString(evaluate("itemValue", getItemValue())) : null);

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

Lines changed: 97 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.beans.PropertyEditor;
2020
import java.beans.PropertyEditorSupport;
2121
import java.io.StringReader;
22+
import java.text.ParseException;
2223
import java.util.ArrayList;
2324
import java.util.Collections;
2425
import java.util.Comparator;
@@ -28,7 +29,6 @@
2829
import java.util.Locale;
2930
import java.util.Map;
3031
import java.util.TreeMap;
31-
3232
import javax.servlet.jsp.JspException;
3333
import javax.servlet.jsp.tagext.Tag;
3434

@@ -40,6 +40,8 @@
4040

4141
import org.springframework.beans.TestBean;
4242
import org.springframework.beans.propertyeditors.CustomCollectionEditor;
43+
import org.springframework.format.Formatter;
44+
import org.springframework.format.support.FormattingConversionService;
4345
import org.springframework.mock.web.MockHttpServletRequest;
4446
import org.springframework.validation.BeanPropertyBindingResult;
4547
import org.springframework.validation.BindingResult;
@@ -59,7 +61,7 @@ public class SelectTagTests extends AbstractFormTagTests {
5961

6062
private SelectTag tag;
6163

62-
private TestBean bean;
64+
private TestBeanWithRealCountry bean;
6365

6466

6567
protected void onSetUp() {
@@ -454,9 +456,102 @@ public void testWithMultiList() throws Exception {
454456

455457
Element e = (Element) selectElement.selectSingleNode("option[@value = 'UK']");
456458
assertEquals("UK node not selected", "selected", e.attribute("selected").getValue());
459+
assertEquals("United Kingdom(UK)", e.getText());
460+
461+
e = (Element) selectElement.selectSingleNode("option[@value = 'AT']");
462+
assertEquals("AT node not selected", "selected", e.attribute("selected").getValue());
463+
assertEquals("Austria(AT)", e.getText());
464+
}
465+
466+
public void testWithElementConverter() throws Exception {
467+
this.bean.setRealCountry(Country.COUNTRY_UK);
468+
469+
BeanPropertyBindingResult errors = new BeanPropertyBindingResult(this.bean, COMMAND_NAME);
470+
FormattingConversionService cs = new FormattingConversionService();
471+
cs.addFormatterForFieldType(Country.class, new Formatter<Country>() {
472+
public String print(Country object, Locale locale) {
473+
return object.getName();
474+
}
475+
public Country parse(String text, Locale locale) throws ParseException {
476+
return new Country(text, text);
477+
}
478+
});
479+
errors.initConversion(cs);
480+
exposeBindingResult(errors);
481+
482+
this.tag.setPath("realCountry");
483+
this.tag.setItems("${countries}");
484+
this.tag.setItemValue("isoCode");
485+
int result = this.tag.doStartTag();
486+
assertEquals(Tag.SKIP_BODY, result);
487+
488+
String output = getOutput();
489+
output = "<doc>" + output + "</doc>";
490+
491+
SAXReader reader = new SAXReader();
492+
Document document = reader.read(new StringReader(output));
493+
Element rootElement = document.getRootElement();
494+
assertEquals(1, rootElement.elements().size());
495+
496+
Element selectElement = rootElement.element("select");
497+
assertEquals("select", selectElement.getName());
498+
assertEquals("realCountry", selectElement.attribute("name").getValue());
499+
500+
List children = selectElement.elements();
501+
assertEquals("Incorrect number of children", 4, children.size());
502+
503+
Element e = (Element) selectElement.selectSingleNode("option[@value = 'UK']");
504+
assertEquals("UK node not selected", "selected", e.attribute("selected").getValue());
505+
assertEquals("United Kingdom", e.getText());
506+
}
507+
508+
public void testWithMultiListAndElementConverter() throws Exception {
509+
List list = new ArrayList();
510+
list.add(Country.COUNTRY_UK);
511+
list.add(Country.COUNTRY_AT);
512+
this.bean.setSomeList(list);
513+
514+
BeanPropertyBindingResult errors = new BeanPropertyBindingResult(this.bean, COMMAND_NAME);
515+
FormattingConversionService cs = new FormattingConversionService();
516+
cs.addFormatterForFieldType(Country.class, new Formatter<Country>() {
517+
public String print(Country object, Locale locale) {
518+
return object.getName();
519+
}
520+
public Country parse(String text, Locale locale) throws ParseException {
521+
return new Country(text, text);
522+
}
523+
});
524+
errors.initConversion(cs);
525+
exposeBindingResult(errors);
526+
527+
this.tag.setPath("someList");
528+
this.tag.setItems("${countries}");
529+
this.tag.setItemValue("isoCode");
530+
int result = this.tag.doStartTag();
531+
assertEquals(Tag.SKIP_BODY, result);
532+
533+
String output = getOutput();
534+
output = "<doc>" + output + "</doc>";
535+
536+
SAXReader reader = new SAXReader();
537+
Document document = reader.read(new StringReader(output));
538+
Element rootElement = document.getRootElement();
539+
assertEquals(2, rootElement.elements().size());
540+
541+
Element selectElement = rootElement.element("select");
542+
assertEquals("select", selectElement.getName());
543+
assertEquals("someList", selectElement.attribute("name").getValue());
544+
545+
List children = selectElement.elements();
546+
assertEquals("Incorrect number of children", 4, children.size());
547+
548+
Element e = (Element) selectElement.selectSingleNode("option[@value = 'UK']");
549+
assertEquals("UK node not selected", "selected", e.attribute("selected").getValue());
550+
assertEquals("United Kingdom", e.getText());
457551

458552
e = (Element) selectElement.selectSingleNode("option[@value = 'AT']");
459553
assertEquals("AT node not selected", "selected", e.attribute("selected").getValue());
554+
assertEquals("Austria", e.getText());
460555
}
461556

462557
public void testWithMultiListAndCustomEditor() throws Exception {

0 commit comments

Comments
 (0)