Commit 6578239f authored by Stephane Nicoll's avatar Stephane Nicoll

Fix binding of Collection of enum

As there is no way to copy all the converters of a `ConversionService` to
another, `RelaxedConversionService` uses a fallback `ConversionService`
when the user-provided one failed.

That fallback is taking care of converting `String` to `Enum` in a case
insensitive way but it has no registered converter to convert a comma
separated String to a collection of something.

Ironically, our current test suite has plenty of cases where we map a
`String` to  a collection of enums and they all pass. This is because
the tests do not provide a custom `ConverterService` so we end up
immediately in the fallback scenario. Since no converter is able to
convert the String to a collection, the property editor support of the
binder takes care of that for us and try to convert each individual
value.

In a regular use case however, a `ConversionService` is provided and
fails to map the collection if the String value(s) don't have the exact
same case as the annotations they represent. Since the original
`ConversionService` has claimed it was able to convert a collection,
the raw `String` value is passed to the fallback converter and that one
fails to convert the raw String.

The fallback converter now registers the necessary converters to
convert collections. Additional tests have been added to test that in
a more explicit way.

Closes gh-4322
parent 04c87138
/* /*
* Copyright 2012-2014 the original author or authors. * Copyright 2012-2015 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
...@@ -24,6 +24,7 @@ import org.springframework.core.convert.ConversionService; ...@@ -24,6 +24,7 @@ import org.springframework.core.convert.ConversionService;
import org.springframework.core.convert.TypeDescriptor; import org.springframework.core.convert.TypeDescriptor;
import org.springframework.core.convert.converter.Converter; import org.springframework.core.convert.converter.Converter;
import org.springframework.core.convert.converter.ConverterFactory; import org.springframework.core.convert.converter.ConverterFactory;
import org.springframework.core.convert.support.DefaultConversionService;
import org.springframework.core.convert.support.GenericConversionService; import org.springframework.core.convert.support.GenericConversionService;
import org.springframework.util.Assert; import org.springframework.util.Assert;
...@@ -32,6 +33,7 @@ import org.springframework.util.Assert; ...@@ -32,6 +33,7 @@ import org.springframework.util.Assert;
* additional relaxed conversion. * additional relaxed conversion.
* *
* @author Phillip Webb * @author Phillip Webb
* @author Stephane Nicoll
* @since 1.1.0 * @since 1.1.0
*/ */
class RelaxedConversionService implements ConversionService { class RelaxedConversionService implements ConversionService {
...@@ -47,6 +49,7 @@ class RelaxedConversionService implements ConversionService { ...@@ -47,6 +49,7 @@ class RelaxedConversionService implements ConversionService {
RelaxedConversionService(ConversionService conversionService) { RelaxedConversionService(ConversionService conversionService) {
this.conversionService = conversionService; this.conversionService = conversionService;
this.additionalConverters = new GenericConversionService(); this.additionalConverters = new GenericConversionService();
DefaultConversionService.addCollectionConverters(this.additionalConverters);
this.additionalConverters this.additionalConverters
.addConverterFactory(new StringToEnumIgnoringCaseConverterFactory()); .addConverterFactory(new StringToEnumIgnoringCaseConverterFactory());
this.additionalConverters.addConverter(new StringToCharArrayConverter()); this.additionalConverters.addConverter(new StringToCharArrayConverter());
......
...@@ -53,6 +53,7 @@ import org.springframework.validation.DataBinder; ...@@ -53,6 +53,7 @@ import org.springframework.validation.DataBinder;
import org.springframework.validation.FieldError; import org.springframework.validation.FieldError;
import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean; import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
...@@ -671,6 +672,14 @@ public class RelaxedDataBinderTests { ...@@ -671,6 +672,14 @@ public class RelaxedDataBinderTests {
result = bind(target, "bingo: The_Other"); result = bind(target, "bingo: The_Other");
assertThat(result.getErrorCount(), equalTo(0)); assertThat(result.getErrorCount(), equalTo(0));
assertThat(target.getBingo(), equalTo(Bingo.THE_OTHER)); assertThat(target.getBingo(), equalTo(Bingo.THE_OTHER));
result = bind(target, "bingos: The_Other");
assertThat(result.getErrorCount(), equalTo(0));
assertThat(target.getBingos(), contains(Bingo.THE_OTHER));
result = bind(target, "bingos: The_Other, that");
assertThat(result.getErrorCount(), equalTo(0));
assertThat(target.getBingos(), contains(Bingo.THE_OTHER, Bingo.THAT));
} }
private BindingResult bind(Object target, String values) throws Exception { private BindingResult bind(Object target, String values) throws Exception {
...@@ -995,6 +1004,8 @@ public class RelaxedDataBinderTests { ...@@ -995,6 +1004,8 @@ public class RelaxedDataBinderTests {
private Bingo bingo; private Bingo bingo;
private List<Bingo> bingos;
public char[] getBar() { public char[] getBar() {
return this.bar; return this.bar;
} }
...@@ -1043,6 +1054,13 @@ public class RelaxedDataBinderTests { ...@@ -1043,6 +1054,13 @@ public class RelaxedDataBinderTests {
this.bingo = bingo; this.bingo = bingo;
} }
public List<Bingo> getBingos() {
return this.bingos;
}
public void setBingos(List<Bingo> bingos) {
this.bingos = bingos;
}
} }
enum Bingo { enum Bingo {
......
...@@ -16,6 +16,8 @@ ...@@ -16,6 +16,8 @@
package org.springframework.boot.context.properties; package org.springframework.boot.context.properties;
import java.util.List;
import javax.annotation.PostConstruct; import javax.annotation.PostConstruct;
import javax.validation.constraints.NotNull; import javax.validation.constraints.NotNull;
...@@ -43,6 +45,7 @@ import org.springframework.validation.Errors; ...@@ -43,6 +45,7 @@ import org.springframework.validation.Errors;
import org.springframework.validation.ValidationUtils; import org.springframework.validation.ValidationUtils;
import org.springframework.validation.Validator; import org.springframework.validation.Validator;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.startsWith; import static org.hamcrest.Matchers.startsWith;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
...@@ -175,6 +178,24 @@ public class ConfigurationPropertiesBindingPostProcessorTests { ...@@ -175,6 +178,24 @@ public class ConfigurationPropertiesBindingPostProcessorTests {
this.context.close(); this.context.close();
} }
@Test
public void testRelaxedPropertyWithSetOfEnum() {
doEnumSetTest("test.the-values:foo,bar", FooEnum.FOO, FooEnum.BAR);
doEnumSetTest("test.the-values:foo", FooEnum.FOO);
doEnumSetTest("TEST_THE_VALUES:FoO", FooEnum.FOO);
doEnumSetTest("test_the_values:BaR,FoO", FooEnum.BAR, FooEnum.FOO);
}
private void doEnumSetTest(String property, FooEnum... expected) {
this.context = new AnnotationConfigApplicationContext();
EnvironmentTestUtils.addEnvironment(this.context, property);
this.context.register(PropertyWithEnum.class);
this.context.refresh();
assertThat(this.context.getBean(PropertyWithEnum.class).getTheValues(),
contains(expected));
this.context.close();
}
@Test @Test
public void testValueBindingForDefaults() throws Exception { public void testValueBindingForDefaults() throws Exception {
this.context = new AnnotationConfigApplicationContext(); this.context = new AnnotationConfigApplicationContext();
...@@ -524,6 +545,8 @@ public class ConfigurationPropertiesBindingPostProcessorTests { ...@@ -524,6 +545,8 @@ public class ConfigurationPropertiesBindingPostProcessorTests {
private FooEnum theValue; private FooEnum theValue;
private List<FooEnum> theValues;
public void setTheValue(FooEnum value) { public void setTheValue(FooEnum value) {
this.theValue = value; this.theValue = value;
} }
...@@ -532,6 +555,14 @@ public class ConfigurationPropertiesBindingPostProcessorTests { ...@@ -532,6 +555,14 @@ public class ConfigurationPropertiesBindingPostProcessorTests {
return this.theValue; return this.theValue;
} }
public List<FooEnum> getTheValues() {
return this.theValues;
}
public void setTheValues(List<FooEnum> theValues) {
this.theValues = theValues;
}
} }
enum FooEnum { enum FooEnum {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment