Consistently coerce booleans to enums

Rename `StringToEnumIgnoringCaseConverterFactory` to
`LenientStringToEnumConverterFactory` and extended it to support
binding of YAML style 'true'/'false' values to 'ON'/'OFF'.

Closes gh-17385
This commit is contained in:
Phillip Webb
2019-07-01 21:57:23 -07:00
parent 46b250549d
commit 07acc4af08
6 changed files with 86 additions and 27 deletions

View File

@@ -19,7 +19,6 @@ package org.springframework.boot.context.logging;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -93,7 +92,8 @@ public class LoggingApplicationListener implements GenericApplicationListener {
private static final ConfigurationPropertyName LOGGING_GROUP = ConfigurationPropertyName.of("logging.group");
private static final Bindable<Map<String, String>> STRING_STRING_MAP = Bindable.mapOf(String.class, String.class);
private static final Bindable<Map<String, LogLevel>> STRING_LOGLEVEL_MAP = Bindable.mapOf(String.class,
LogLevel.class);
private static final Bindable<Map<String, String[]>> STRING_STRINGS_MAP = Bindable.mapOf(String.class,
String[].class);
@@ -326,7 +326,7 @@ public class LoggingApplicationListener implements GenericApplicationListener {
Binder binder = Binder.get(environment);
Map<String, String[]> groups = getGroups();
binder.bind(LOGGING_GROUP, STRING_STRINGS_MAP.withExistingValue(groups));
Map<String, String> levels = binder.bind(LOGGING_LEVEL, STRING_STRING_MAP).orElseGet(Collections::emptyMap);
Map<String, LogLevel> levels = binder.bind(LOGGING_LEVEL, STRING_LOGLEVEL_MAP).orElseGet(Collections::emptyMap);
levels.forEach((name, level) -> {
String[] groupedNames = groups.get(name);
if (ObjectUtils.isEmpty(groupedNames)) {
@@ -344,30 +344,22 @@ public class LoggingApplicationListener implements GenericApplicationListener {
return groups;
}
private void setLogLevel(LoggingSystem system, String[] names, String level) {
private void setLogLevel(LoggingSystem system, String[] names, LogLevel level) {
for (String name : names) {
setLogLevel(system, name, level);
}
}
private void setLogLevel(LoggingSystem system, String name, String level) {
private void setLogLevel(LoggingSystem system, String name, LogLevel level) {
try {
name = name.equalsIgnoreCase(LoggingSystem.ROOT_LOGGER_NAME) ? null : name;
system.setLogLevel(name, coerceLogLevel(level));
system.setLogLevel(name, level);
}
catch (RuntimeException ex) {
this.logger.error("Cannot set level '" + level + "' for '" + name + "'");
}
}
private LogLevel coerceLogLevel(String level) {
String trimmedLevel = level.trim();
if ("false".equalsIgnoreCase(trimmedLevel)) {
return LogLevel.OFF;
}
return LogLevel.valueOf(trimmedLevel.toUpperCase(Locale.ENGLISH));
}
private void registerShutdownHookIfNecessary(Environment environment, LoggingSystem loggingSystem) {
boolean registerShutdownHook = environment.getProperty(REGISTER_SHUTDOWN_HOOK_PROPERTY, Boolean.class, false);
if (registerShutdownHook) {

View File

@@ -115,7 +115,7 @@ public class ApplicationConversionService extends FormattingConversionService {
registry.addConverter(new DurationToNumberConverter());
registry.addConverter(new StringToDataSizeConverter());
registry.addConverter(new NumberToDataSizeConverter());
registry.addConverterFactory(new StringToEnumIgnoringCaseConverterFactory());
registry.addConverterFactory(new LenientStringToEnumConverterFactory());
}
/**

View File

@@ -16,21 +16,41 @@
package org.springframework.boot.convert;
import java.util.Collections;
import java.util.EnumSet;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.springframework.core.convert.converter.Converter;
import org.springframework.core.convert.converter.ConverterFactory;
import org.springframework.util.Assert;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
/**
* Converts from a String to a {@link java.lang.Enum} by calling searching matching enum
* names (ignoring case).
* Converts from a String to a {@link java.lang.Enum} with lenient conversion rules.
* Specifically:
* <ul>
* <li>Uses a case insensitive search</li>
* <li>Does not consider {@code '_'}, {@code '$'} or other special characters</li>
* <li>Allows mapping of YAML style {@code "false"} and {@code "true"} to enums {@code ON}
* and {@code OFF}</li>
* </ul>
*
* @author Phillip Webb
*/
@SuppressWarnings({ "unchecked", "rawtypes" })
final class StringToEnumIgnoringCaseConverterFactory implements ConverterFactory<String, Enum> {
final class LenientStringToEnumConverterFactory implements ConverterFactory<String, Enum> {
private static Map<String, List<String>> ALIASES;
static {
MultiValueMap<String, String> aliases = new LinkedMultiValueMap<>();
aliases.add("true", "on");
aliases.add("false", "off");
ALIASES = Collections.unmodifiableMap(aliases);
}
@Override
public <T extends Enum> Converter<String, T> getConverter(Class<T> targetType) {
@@ -65,10 +85,19 @@ final class StringToEnumIgnoringCaseConverterFactory implements ConverterFactory
}
private T findEnum(String source) {
String name = getLettersAndDigits(source);
Map<String, T> candidates = new LinkedHashMap<String, T>();
for (T candidate : (Set<T>) EnumSet.allOf(this.enumType)) {
if (getLettersAndDigits(candidate.name()).equals(name)) {
return candidate;
candidates.put(getLettersAndDigits(candidate.name()), candidate);
}
String name = getLettersAndDigits(source);
T result = candidates.get(name);
if (result != null) {
return result;
}
for (String alias : ALIASES.getOrDefault(name, Collections.emptyList())) {
result = candidates.get(alias);
if (result != null) {
return result;
}
}
throw new IllegalArgumentException("No enum constant " + this.enumType.getCanonicalName() + "." + source);

View File

@@ -288,6 +288,14 @@ class SpringApplicationTests {
assertThat(application).hasFieldOrPropertyWithValue("bannerMode", Banner.Mode.OFF);
}
@Test
void bindsYamlStyleBannerModeToSpringApplication() {
SpringApplication application = new SpringApplication(ExampleConfig.class);
application.setWebApplicationType(WebApplicationType.NONE);
this.context = application.run("--spring.main.banner-mode=false");
assertThat(application).hasFieldOrPropertyWithValue("bannerMode", Banner.Mode.OFF);
}
@Test
void customId() {
SpringApplication application = new SpringApplication(ExampleConfig.class);

View File

@@ -44,6 +44,7 @@ import org.slf4j.impl.StaticLoggerBinder;
import org.springframework.boot.SpringApplication;
import org.springframework.boot.context.event.ApplicationFailedEvent;
import org.springframework.boot.context.event.ApplicationStartingEvent;
import org.springframework.boot.context.properties.bind.BindException;
import org.springframework.boot.context.properties.source.ConfigurationPropertySources;
import org.springframework.boot.logging.AbstractLoggingSystem;
import org.springframework.boot.logging.LogFile;
@@ -69,6 +70,7 @@ import org.springframework.core.env.MutablePropertySources;
import org.springframework.test.util.ReflectionTestUtils;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.not;
@@ -369,9 +371,8 @@ public class LoggingApplicationListenerTests {
public void parseLevelsFails() {
this.logger.setLevel(Level.INFO);
addPropertiesToEnvironment(this.context, "logging.level.org.springframework.boot=GARBAGE");
this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader());
this.logger.debug("testatdebug");
assertThat(this.outputCapture.toString()).doesNotContain("testatdebug").contains("Cannot set level 'GARBAGE'");
assertThatExceptionOfType(BindException.class).isThrownBy(
() -> this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()));
}
@Test

View File

@@ -26,11 +26,11 @@ import org.springframework.core.convert.ConversionService;
import static org.assertj.core.api.Assertions.assertThat;
/**
* Tests for {@link StringToEnumIgnoringCaseConverterFactory}.
* Tests for {@link LenientStringToEnumConverterFactory}.
*
* @author Phillip Webb
*/
class StringToEnumIgnoringCaseConverterFactoryTests {
class LenientStringToEnumConverterFactoryTests {
@ConversionServiceTest
void canConvertFromStringToEnumShouldReturnTrue(ConversionService conversionService) {
@@ -75,9 +75,26 @@ class StringToEnumIgnoringCaseConverterFactoryTests {
}
}
@ConversionServiceTest
void convertFromStringToEnumWhenYamlBooleanShouldConvertValue(ConversionService conversionService) {
assertThat(conversionService.convert("one", TestOnOffEnum.class)).isEqualTo(TestOnOffEnum.ONE);
assertThat(conversionService.convert("two", TestOnOffEnum.class)).isEqualTo(TestOnOffEnum.TWO);
assertThat(conversionService.convert("true", TestOnOffEnum.class)).isEqualTo(TestOnOffEnum.ON);
assertThat(conversionService.convert("false", TestOnOffEnum.class)).isEqualTo(TestOnOffEnum.OFF);
assertThat(conversionService.convert("TRUE", TestOnOffEnum.class)).isEqualTo(TestOnOffEnum.ON);
assertThat(conversionService.convert("FALSE", TestOnOffEnum.class)).isEqualTo(TestOnOffEnum.OFF);
assertThat(conversionService.convert("fA_lsE", TestOnOffEnum.class)).isEqualTo(TestOnOffEnum.OFF);
assertThat(conversionService.convert("one", TestTrueFalseEnum.class)).isEqualTo(TestTrueFalseEnum.ONE);
assertThat(conversionService.convert("two", TestTrueFalseEnum.class)).isEqualTo(TestTrueFalseEnum.TWO);
assertThat(conversionService.convert("true", TestTrueFalseEnum.class)).isEqualTo(TestTrueFalseEnum.TRUE);
assertThat(conversionService.convert("false", TestTrueFalseEnum.class)).isEqualTo(TestTrueFalseEnum.FALSE);
assertThat(conversionService.convert("TRUE", TestTrueFalseEnum.class)).isEqualTo(TestTrueFalseEnum.TRUE);
assertThat(conversionService.convert("FALSE", TestTrueFalseEnum.class)).isEqualTo(TestTrueFalseEnum.FALSE);
}
static Stream<? extends Arguments> conversionServices() {
return ConversionServiceArguments
.with((service) -> service.addConverterFactory(new StringToEnumIgnoringCaseConverterFactory()));
.with((service) -> service.addConverterFactory(new LenientStringToEnumConverterFactory()));
}
enum TestEnum {
@@ -86,6 +103,18 @@ class StringToEnumIgnoringCaseConverterFactoryTests {
}
enum TestOnOffEnum {
ONE, TWO, ON, OFF
}
enum TestTrueFalseEnum {
ONE, TWO, TRUE, FALSE, ON, OFF
}
enum LocaleSensitiveEnum {
ACCEPT_CASE_INSENSITIVE_PROPERTIES