Commit dbee0cf9 authored by Scott Frederick's avatar Scott Frederick

Ignore empty values in config location properties

This commit updates config data property binding to ignore empty
elements in `spring.config.location` and `spring.config.import`
property values when a value is a comma-delimited string
representing a collection.

Fixes gh-26342
parent 65cb654a
/* /*
* Copyright 2012-2020 the original author or authors. * Copyright 2012-2021 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.
...@@ -16,7 +16,10 @@ ...@@ -16,7 +16,10 @@
package org.springframework.boot.context.config; package org.springframework.boot.context.config;
import java.util.Arrays;
import java.util.List; import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;
import org.springframework.boot.context.properties.bind.AbstractBindHandler; import org.springframework.boot.context.properties.bind.AbstractBindHandler;
import org.springframework.boot.context.properties.bind.BindContext; import org.springframework.boot.context.properties.bind.BindContext;
...@@ -30,6 +33,7 @@ import org.springframework.boot.origin.Origin; ...@@ -30,6 +33,7 @@ import org.springframework.boot.origin.Origin;
* objects. * objects.
* *
* @author Phillip Webb * @author Phillip Webb
* @author Scott Frederick
*/ */
class ConfigDataLocationBindHandler extends AbstractBindHandler { class ConfigDataLocationBindHandler extends AbstractBindHandler {
...@@ -40,19 +44,22 @@ class ConfigDataLocationBindHandler extends AbstractBindHandler { ...@@ -40,19 +44,22 @@ class ConfigDataLocationBindHandler extends AbstractBindHandler {
return withOrigin(context, (ConfigDataLocation) result); return withOrigin(context, (ConfigDataLocation) result);
} }
if (result instanceof List) { if (result instanceof List) {
List<Object> list = (List<Object>) result; List<Object> list = ((List<Object>) result).stream().filter(Objects::nonNull).collect(Collectors.toList());
for (int i = 0; i < list.size(); i++) { for (int i = 0; i < list.size(); i++) {
Object element = list.get(i); Object element = list.get(i);
if (element instanceof ConfigDataLocation) { if (element instanceof ConfigDataLocation) {
list.set(i, withOrigin(context, (ConfigDataLocation) element)); list.set(i, withOrigin(context, (ConfigDataLocation) element));
} }
} }
return list;
} }
if (result instanceof ConfigDataLocation[]) { if (result instanceof ConfigDataLocation[]) {
ConfigDataLocation[] locations = (ConfigDataLocation[]) result; ConfigDataLocation[] locations = Arrays.stream((ConfigDataLocation[]) result).filter(Objects::nonNull)
.toArray(ConfigDataLocation[]::new);
for (int i = 0; i < locations.length; i++) { for (int i = 0; i < locations.length; i++) {
locations[i] = withOrigin(context, locations[i]); locations[i] = withOrigin(context, locations[i]);
} }
return locations;
} }
return result; return result;
} }
......
...@@ -43,13 +43,14 @@ import static org.mockito.Mockito.mock; ...@@ -43,13 +43,14 @@ import static org.mockito.Mockito.mock;
* *
* @author Phillip Webb * @author Phillip Webb
* @author Madhura Bhave * @author Madhura Bhave
* @author Scott Frederick
*/ */
class ConfigDataEnvironmentContributorTests { class ConfigDataEnvironmentContributorTests {
private static final ConfigDataLocation TEST_LOCATION = ConfigDataLocation.of("test"); private static final ConfigDataLocation TEST_LOCATION = ConfigDataLocation.of("test");
private ConfigDataActivationContext activationContext = new ConfigDataActivationContext(CloudPlatform.KUBERNETES, private final ConfigDataActivationContext activationContext = new ConfigDataActivationContext(
null); CloudPlatform.KUBERNETES, null);
@Test @Test
void getKindReturnsKind() { void getKindReturnsKind() {
...@@ -125,6 +126,16 @@ class ConfigDataEnvironmentContributorTests { ...@@ -125,6 +126,16 @@ class ConfigDataEnvironmentContributorTests {
ConfigDataLocation.of("boot")); ConfigDataLocation.of("boot"));
} }
@Test
void getImportsIgnoresEmptyElements() {
MockPropertySource propertySource = new MockPropertySource();
propertySource.setProperty("spring.config.import", "spring,,boot,");
ConfigData configData = new ConfigData(Collections.singleton(propertySource));
ConfigDataEnvironmentContributor contributor = createBoundContributor(null, configData, 0);
assertThat(contributor.getImports()).containsExactly(ConfigDataLocation.of("spring"),
ConfigDataLocation.of("boot"));
}
@Test @Test
void hasUnprocessedImportsWhenNoImportsReturnsFalse() { void hasUnprocessedImportsWhenNoImportsReturnsFalse() {
ConfigData configData = new ConfigData(Collections.singleton(new MockPropertySource())); ConfigData configData = new ConfigData(Collections.singleton(new MockPropertySource()));
...@@ -205,9 +216,9 @@ class ConfigDataEnvironmentContributorTests { ...@@ -205,9 +216,9 @@ class ConfigDataEnvironmentContributorTests {
"classpath:application-profile.properties"); "classpath:application-profile.properties");
ConfigDataEnvironmentContributor classpathImports = createBoundContributor("classpath:/"); ConfigDataEnvironmentContributor classpathImports = createBoundContributor("classpath:/");
classpathImports = classpathImports.withChildren(ImportPhase.BEFORE_PROFILE_ACTIVATION, classpathImports = classpathImports.withChildren(ImportPhase.BEFORE_PROFILE_ACTIVATION,
Arrays.asList(classpathApplication)); Collections.singletonList(classpathApplication));
classpathImports = classpathImports.withChildren(ImportPhase.AFTER_PROFILE_ACTIVATION, classpathImports = classpathImports.withChildren(ImportPhase.AFTER_PROFILE_ACTIVATION,
Arrays.asList(classpathProfile)); Collections.singletonList(classpathProfile));
ConfigDataEnvironmentContributor root = createBoundContributor("root"); ConfigDataEnvironmentContributor root = createBoundContributor("root");
root = root.withChildren(ImportPhase.BEFORE_PROFILE_ACTIVATION, Arrays.asList(fileImports, classpathImports)); root = root.withChildren(ImportPhase.BEFORE_PROFILE_ACTIVATION, Arrays.asList(fileImports, classpathImports));
assertThat(asLocationsList(root.iterator())).containsExactly("file:application-profile.properties", assertThat(asLocationsList(root.iterator())).containsExactly("file:application-profile.properties",
......
/* /*
* Copyright 2012-2020 the original author or authors. * Copyright 2012-2021 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.
...@@ -30,6 +30,7 @@ import static org.assertj.core.api.Assertions.assertThat; ...@@ -30,6 +30,7 @@ import static org.assertj.core.api.Assertions.assertThat;
* Tests for {@link ConfigDataLocationBindHandler}. * Tests for {@link ConfigDataLocationBindHandler}.
* *
* @author Phillip Webb * @author Phillip Webb
* @author Scott Frederick
*/ */
class ConfigDataLocationBindHandlerTests { class ConfigDataLocationBindHandlerTests {
...@@ -54,6 +55,21 @@ class ConfigDataLocationBindHandlerTests { ...@@ -54,6 +55,21 @@ class ConfigDataLocationBindHandlerTests {
assertThat(bound[2].getOrigin()).hasToString(expectedLocation); assertThat(bound[2].getOrigin()).hasToString(expectedLocation);
} }
@Test
void bindToArrayFromCommaStringPropertyIgnoresEmptyElements() {
MapConfigurationPropertySource source = new MapConfigurationPropertySource();
source.put("locations", ",a,,b,c,");
Binder binder = new Binder(source);
ConfigDataLocation[] bound = binder.bind("locations", ARRAY, this.handler).get();
String expectedLocation = "\"locations\" from property source \"source\"";
assertThat(bound[0]).hasToString("a");
assertThat(bound[0].getOrigin()).hasToString(expectedLocation);
assertThat(bound[1]).hasToString("b");
assertThat(bound[1].getOrigin()).hasToString(expectedLocation);
assertThat(bound[2]).hasToString("c");
assertThat(bound[2].getOrigin()).hasToString(expectedLocation);
}
@Test @Test
void bindToArrayFromIndexedPropertiesSetsOrigin() { void bindToArrayFromIndexedPropertiesSetsOrigin() {
MapConfigurationPropertySource source = new MapConfigurationPropertySource(); MapConfigurationPropertySource source = new MapConfigurationPropertySource();
...@@ -85,6 +101,21 @@ class ConfigDataLocationBindHandlerTests { ...@@ -85,6 +101,21 @@ class ConfigDataLocationBindHandlerTests {
assertThat(bound.getLocation(2).getOrigin()).hasToString(expectedLocation); assertThat(bound.getLocation(2).getOrigin()).hasToString(expectedLocation);
} }
@Test
void bindToValueObjectFromCommaStringPropertyIgnoresEmptyElements() {
MapConfigurationPropertySource source = new MapConfigurationPropertySource();
source.put("test.locations", ",a,b,,c,");
Binder binder = new Binder(source);
ValueObject bound = binder.bind("test", VALUE_OBJECT, this.handler).get();
String expectedLocation = "\"test.locations\" from property source \"source\"";
assertThat(bound.getLocation(0)).hasToString("a");
assertThat(bound.getLocation(0).getOrigin()).hasToString(expectedLocation);
assertThat(bound.getLocation(1)).hasToString("b");
assertThat(bound.getLocation(1).getOrigin()).hasToString(expectedLocation);
assertThat(bound.getLocation(2)).hasToString("c");
assertThat(bound.getLocation(2).getOrigin()).hasToString(expectedLocation);
}
@Test @Test
void bindToValueObjectFromIndexedPropertiesSetsOrigin() { void bindToValueObjectFromIndexedPropertiesSetsOrigin() {
MapConfigurationPropertySource source = new MapConfigurationPropertySource(); MapConfigurationPropertySource source = new MapConfigurationPropertySource();
......
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