Commit 79fc8838 authored by Phillip Webb's avatar Phillip Webb

Propagate Map conversion failures

Align `MapBinder` with `IndexedBinder` so that if a value is specified
any converter exception are propagated.

See gh-11493
parent 25609c06
/* /*
* Copyright 2012-2017 the original author or authors. * Copyright 2012-2018 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.
...@@ -27,7 +27,6 @@ import org.springframework.boot.context.properties.source.ConfigurationPropertyS ...@@ -27,7 +27,6 @@ import org.springframework.boot.context.properties.source.ConfigurationPropertyS
import org.springframework.boot.context.properties.source.IterableConfigurationPropertySource; import org.springframework.boot.context.properties.source.IterableConfigurationPropertySource;
import org.springframework.core.CollectionFactory; import org.springframework.core.CollectionFactory;
import org.springframework.core.ResolvableType; import org.springframework.core.ResolvableType;
import org.springframework.core.convert.ConverterNotFoundException;
import org.springframework.util.ClassUtils; import org.springframework.util.ClassUtils;
/** /**
...@@ -58,9 +57,12 @@ class MapBinder extends AggregateBinder<Map<Object, Object>> { ...@@ -58,9 +57,12 @@ class MapBinder extends AggregateBinder<Map<Object, Object>> {
Bindable<?> resolvedTarget = resolveTarget(target); Bindable<?> resolvedTarget = resolveTarget(target);
for (ConfigurationPropertySource source : getContext().getSources()) { for (ConfigurationPropertySource source : getContext().getSources()) {
if (!ConfigurationPropertyName.EMPTY.equals(name)) { if (!ConfigurationPropertyName.EMPTY.equals(name)) {
Object converted = convertIfFound(name, source, resolvedTarget); ConfigurationProperty property = source.getConfigurationProperty(name);
if (converted != null) { if (property != null) {
return converted; Object value = getContext().getPlaceholdersResolver()
.resolvePlaceholders(property.getValue());
return ResolvableTypeDescriptor.forType(target.getType())
.convert(getContext().getConversionService(), value);
} }
source = source.filter(name::isAncestorOf); source = source.filter(name::isAncestorOf);
} }
...@@ -77,20 +79,6 @@ class MapBinder extends AggregateBinder<Map<Object, Object>> { ...@@ -77,20 +79,6 @@ class MapBinder extends AggregateBinder<Map<Object, Object>> {
return target; return target;
} }
private Object convertIfFound(ConfigurationPropertyName name, ConfigurationPropertySource source, Bindable<?> target) {
ConfigurationProperty configurationProperty = source.getConfigurationProperty(name);
if (configurationProperty != null) {
try {
return ResolvableTypeDescriptor.forType(target.getType())
.convert(getContext().getConversionService(), configurationProperty.getValue());
}
catch (ConverterNotFoundException ex) {
}
}
return null;
}
@Override @Override
protected Map<Object, Object> merge(Map<Object, Object> existing, protected Map<Object, Object> merge(Map<Object, Object> existing,
Map<Object, Object> additional) { Map<Object, Object> additional) {
......
/* /*
* Copyright 2012-2017 the original author or authors. * Copyright 2012-2018 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.
...@@ -26,7 +26,9 @@ import java.util.Properties; ...@@ -26,7 +26,9 @@ import java.util.Properties;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import org.junit.Before; import org.junit.Before;
import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.mockito.Answers; import org.mockito.Answers;
import org.mockito.ArgumentCaptor; import org.mockito.ArgumentCaptor;
import org.mockito.InOrder; import org.mockito.InOrder;
...@@ -75,6 +77,9 @@ public class MapBinderTests { ...@@ -75,6 +77,9 @@ public class MapBinderTests {
private static final Bindable<Map<String, String[]>> STRING_ARRAY_MAP = Bindable private static final Bindable<Map<String, String[]>> STRING_ARRAY_MAP = Bindable
.mapOf(String.class, String[].class); .mapOf(String.class, String[].class);
@Rule
public ExpectedException thrown = ExpectedException.none();
private List<ConfigurationPropertySource> sources = new ArrayList<>(); private List<ConfigurationPropertySource> sources = new ArrayList<>();
private Binder binder; private Binder binder;
...@@ -532,7 +537,6 @@ public class MapBinderTests { ...@@ -532,7 +537,6 @@ public class MapBinderTests {
DefaultConversionService conversionService = new DefaultConversionService(); DefaultConversionService conversionService = new DefaultConversionService();
conversionService.addConverter(new MapConverter()); conversionService.addConverter(new MapConverter());
Binder binder = new Binder(this.sources, null, conversionService); Binder binder = new Binder(this.sources, null, conversionService);
MockConfigurationPropertySource source = new MockConfigurationPropertySource(); MockConfigurationPropertySource source = new MockConfigurationPropertySource();
source.put("foo", "a,b"); source.put("foo", "a,b");
this.sources.add(source); this.sources.add(source);
...@@ -546,8 +550,8 @@ public class MapBinderTests { ...@@ -546,8 +550,8 @@ public class MapBinderTests {
MockConfigurationPropertySource source = new MockConfigurationPropertySource(); MockConfigurationPropertySource source = new MockConfigurationPropertySource();
source.put("foo", "a,b"); source.put("foo", "a,b");
this.sources.add(source); this.sources.add(source);
BindResult<Map<String, String>> result = this.binder.bind("foo", STRING_STRING_MAP); this.thrown.expect(BindException.class);
assertThat(result.isBound()).isFalse(); this.binder.bind("foo", STRING_STRING_MAP);
} }
private <K, V> Bindable<Map<K, V>> getMapBindable(Class<K> keyGeneric, private <K, V> Bindable<Map<K, V>> getMapBindable(Class<K> keyGeneric,
...@@ -603,12 +607,14 @@ public class MapBinderTests { ...@@ -603,12 +607,14 @@ public class MapBinderTests {
} }
static class MapConverter implements Converter<String, Map<String, String>> { static class MapConverter implements Converter<String, Map<String, String>> {
@Override @Override
public Map<String, String> convert(String s) { public Map<String, String> convert(String s) {
Map<String, String> map = new HashMap<>(); Map<String, String> map = new HashMap<>();
StringUtils.commaDelimitedListToSet(s).forEach(k -> map.put(k, "")); StringUtils.commaDelimitedListToSet(s).forEach(k -> map.put(k, ""));
return map; return map;
} }
} }
} }
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