Make ConfigDataLocation.of non-nullable

Update `ConfigDataLocation.of` to return an empty location rather than
null. Filtering of empty elements now happens in `ConfigDataProperties`
and `ConfigDataEnvironment` which allows us to simplify
`ConfigDataLocationBindHandler`.

Closes spring-projectsgh-47221

Co-authored-by: Phillip Webb <phil.webb@broadcom.com>

Author: mhalbritter

core/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataEnvironment.java

@@ -214,7 +214,9 @@ private ConfigDataLocation[] bindLocations(Binder binder, String propertyName, C
 	private void addInitialImportContributors(List<ConfigDataEnvironmentContributor> initialContributors,
 			ConfigDataLocation[] locations) {
 		for (int i = locations.length - 1; i >= 0; i--) {
-			initialContributors.add(createInitialImportContributor(locations[i]));
+			if (ConfigDataLocation.isNotEmpty(locations[i])) {
+				initialContributors.add(createInitialImportContributor(locations[i]));
+			}
 		}
 	}
 

core/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataLocation.java

@@ -38,6 +38,8 @@
  */
 public final class ConfigDataLocation implements OriginProvider {
 
+	private static final ConfigDataLocation EMPTY = new ConfigDataLocation(false, "", null);
+
 	/**
 	 * Prefix used to indicate that a {@link ConfigDataResource} is optional.
 	 */
@@ -89,10 +91,7 @@ public boolean hasPrefix(String prefix) {
 	 * @return the value with the prefix removed
 	 */
 	public String getNonPrefixedValue(String prefix) {
-		if (hasPrefix(prefix)) {
-			return this.value.substring(prefix.length());
-		}
-		return this.value;
+		return (!hasPrefix(prefix)) ? this.value : this.value.substring(prefix.length());
 	}
 
 	@Override
@@ -118,17 +117,26 @@ public ConfigDataLocation[] split() {
 	 * @since 2.4.7
 	 */
 	public ConfigDataLocation[] split(String delimiter) {
+		Assert.state(!this.value.isEmpty(), "Unable to split empty locations");
 		String[] values = StringUtils.delimitedListToStringArray(toString(), delimiter);
 		ConfigDataLocation[] result = new ConfigDataLocation[values.length];
 		for (int i = 0; i < values.length; i++) {
 			int index = i;
 			ConfigDataLocation configDataLocation = of(values[index]);
-			Assert.state(configDataLocation != null, () -> "Unable to parse '%s'".formatted(values[index]));
 			result[i] = configDataLocation.withOrigin(getOrigin());
 		}
 		return result;
 	}
 
+	/**
+	 * Create a new {@link ConfigDataLocation} with a specific {@link Origin}.
+	 * @param origin the origin to set
+	 * @return a new {@link ConfigDataLocation} instance.
+	 */
+	ConfigDataLocation withOrigin(@Nullable Origin origin) {
+		return new ConfigDataLocation(this.optional, this.value, origin);
+	}
+
 	@Override
 	public boolean equals(Object obj) {
 		if (this == obj) {
@@ -151,35 +159,19 @@ public String toString() {
 		return (!this.optional) ? this.value : OPTIONAL_PREFIX + this.value;
 	}
 
-	/**
-	 * Create a new {@link ConfigDataLocation} with a specific {@link Origin}.
-	 * @param origin the origin to set
-	 * @return a new {@link ConfigDataLocation} instance.
-	 */
-	ConfigDataLocation withOrigin(@Nullable Origin origin) {
-		return new ConfigDataLocation(this.optional, this.value, origin);
-	}
-
 	/**
 	 * Factory method to create a new {@link ConfigDataLocation} from a string.
 	 * @param location the location string
-	 * @return a {@link ConfigDataLocation} instance or {@code null} if no location was
-	 * provided
+	 * @return the {@link ConfigDataLocation} (which may be empty)
 	 */
-	public static @Nullable ConfigDataLocation of(@Nullable String location) {
+	public static ConfigDataLocation of(@Nullable String location) {
 		boolean optional = location != null && location.startsWith(OPTIONAL_PREFIX);
-		String value;
-		if (optional) {
-			Assert.state(location != null, "'location' can't be null here");
-			value = location.substring(OPTIONAL_PREFIX.length());
-		}
-		else {
-			value = location;
-		}
-		if (!StringUtils.hasText(value)) {
-			return null;
-		}
-		return new ConfigDataLocation(optional, value, null);
+		String value = (location != null && optional) ? location.substring(OPTIONAL_PREFIX.length()) : location;
+		return (StringUtils.hasText(value)) ? new ConfigDataLocation(optional, value, null) : EMPTY;
+	}
+
+	static boolean isNotEmpty(@Nullable ConfigDataLocation location) {
+		return (location != null) && !location.getValue().isEmpty();
 	}
 
 }

core/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataLocationBindHandler.java

@@ -19,13 +19,15 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
-import java.util.Objects;
 import java.util.stream.Collectors;
 
+import org.jspecify.annotations.Nullable;
+
 import org.springframework.boot.context.properties.bind.AbstractBindHandler;
 import org.springframework.boot.context.properties.bind.BindContext;
 import org.springframework.boot.context.properties.bind.BindHandler;
 import org.springframework.boot.context.properties.bind.Bindable;
+import org.springframework.boot.context.properties.source.ConfigurationProperty;
 import org.springframework.boot.context.properties.source.ConfigurationPropertyName;
 import org.springframework.boot.origin.Origin;
 
@@ -39,32 +41,35 @@
 class ConfigDataLocationBindHandler extends AbstractBindHandler {
 
 	@Override
-	public Object onSuccess(ConfigurationPropertyName name, Bindable<?> target, BindContext context, Object result) {
+	public @Nullable Object onSuccess(ConfigurationPropertyName name, Bindable<?> target, BindContext context,
+			Object result) {
+		OriginMapper originMapper = new OriginMapper(context.getConfigurationProperty());
 		if (result instanceof ConfigDataLocation location) {
-			return withOrigin(context, location);
+			return originMapper.map(location);
 		}
-		if (result instanceof List<?> list) {
-			return list.stream()
-				.filter(Objects::nonNull)
-				.map((element) -> (element instanceof ConfigDataLocation location) ? withOrigin(context, location)
-						: element)
-				.collect(Collectors.toCollection(ArrayList::new));
+		if (result instanceof List<?> locations) {
+			return locations.stream().map(originMapper::mapIfPossible).collect(Collectors.toCollection(ArrayList::new));
 		}
-		if (result instanceof ConfigDataLocation[] unfilteredLocations) {
-			return Arrays.stream(unfilteredLocations)
-				.filter(Objects::nonNull)
-				.map((element) -> withOrigin(context, element))
-				.toArray(ConfigDataLocation[]::new);
+		if (result instanceof ConfigDataLocation[] locations) {
+			return Arrays.stream(locations).map(originMapper::mapIfPossible).toArray(ConfigDataLocation[]::new);
 		}
 		return result;
 	}
 
-	private ConfigDataLocation withOrigin(BindContext context, ConfigDataLocation result) {
-		if (result.getOrigin() != null) {
-			return result;
+	private record OriginMapper(@Nullable ConfigurationProperty property) {
+
+		@Nullable Object mapIfPossible(@Nullable Object object) {
+			return (object instanceof ConfigDataLocation location) ? map(location) : object;
 		}
-		Origin origin = Origin.from(context.getConfigurationProperty());
-		return result.withOrigin(origin);
+
+		@Nullable ConfigDataLocation map(@Nullable ConfigDataLocation location) {
+			if (location == null) {
+				return null;
+			}
+			Origin origin = Origin.from(location);
+			return (origin != null) ? location : location.withOrigin(Origin.from(property()));
+		}
+
 	}
 
 }

core/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataProperties.java

@@ -53,7 +53,8 @@ class ConfigDataProperties {
 	 * @param activate the activate properties
 	 */
 	ConfigDataProperties(@Nullable @Name("import") List<ConfigDataLocation> imports, @Nullable Activate activate) {
-		this.imports = (imports != null) ? imports : Collections.emptyList();
+		this.imports = (imports != null) ? imports.stream().filter(ConfigDataLocation::isNotEmpty).toList()
+				: Collections.emptyList();
 		this.activate = activate;
 	}
 

core/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigDataLocationBindHandlerTests.java

@@ -56,18 +56,24 @@ void bindToArrayFromCommaStringPropertySetsOrigin() {
 	}
 
 	@Test
-	void bindToArrayFromCommaStringPropertyIgnoresEmptyElements() {
+	void bindToArrayFromCommaStringPropertyDoesNotFailOnEmptyElements() {
 		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]).hasToString("");
 		assertThat(bound[0].getOrigin()).hasToString(expectedLocation);
-		assertThat(bound[1]).hasToString("b");
+		assertThat(bound[1]).hasToString("a");
 		assertThat(bound[1].getOrigin()).hasToString(expectedLocation);
-		assertThat(bound[2]).hasToString("c");
+		assertThat(bound[2]).hasToString("");
 		assertThat(bound[2].getOrigin()).hasToString(expectedLocation);
+		assertThat(bound[3]).hasToString("b");
+		assertThat(bound[3].getOrigin()).hasToString(expectedLocation);
+		assertThat(bound[4]).hasToString("c");
+		assertThat(bound[4].getOrigin()).hasToString(expectedLocation);
+		assertThat(bound[5]).hasToString("");
+		assertThat(bound[5].getOrigin()).hasToString(expectedLocation);
 	}
 
 	@Test
@@ -102,18 +108,24 @@ void bindToValueObjectFromCommaStringPropertySetsOrigin() {
 	}
 
 	@Test
-	void bindToValueObjectFromCommaStringPropertyIgnoresEmptyElements() {
+	void bindToValueObjectFromCommaStringPropertyDoesNotFailOnEmptyElements() {
 		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)).hasToString("");
 		assertThat(bound.getLocation(0).getOrigin()).hasToString(expectedLocation);
-		assertThat(bound.getLocation(1)).hasToString("b");
+		assertThat(bound.getLocation(1)).hasToString("a");
 		assertThat(bound.getLocation(1).getOrigin()).hasToString(expectedLocation);
-		assertThat(bound.getLocation(2)).hasToString("c");
+		assertThat(bound.getLocation(2)).hasToString("b");
 		assertThat(bound.getLocation(2).getOrigin()).hasToString(expectedLocation);
+		assertThat(bound.getLocation(3)).hasToString("");
+		assertThat(bound.getLocation(3).getOrigin()).hasToString(expectedLocation);
+		assertThat(bound.getLocation(4)).hasToString("c");
+		assertThat(bound.getLocation(4).getOrigin()).hasToString(expectedLocation);
+		assertThat(bound.getLocation(5)).hasToString("");
+		assertThat(bound.getLocation(5).getOrigin()).hasToString(expectedLocation);
 	}
 
 	@Test

core/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigDataLocationTests.java

@@ -115,18 +115,21 @@ void withOriginSetsOrigin() {
 	}
 
 	@Test
-	void ofWhenNullValueReturnsNull() {
-		assertThat(ConfigDataLocation.of(null)).isNull();
+	void ofWhenNullValueReturnsEmptyLocation() {
+		ConfigDataLocation location = ConfigDataLocation.of(null);
+		assertThat(location.getValue().isEmpty()).isTrue();
 	}
 
 	@Test
-	void ofWhenEmptyValueReturnsNull() {
-		assertThat(ConfigDataLocation.of("")).isNull();
+	void ofWhenEmptyValueReturnsEmptyLocation() {
+		ConfigDataLocation location = ConfigDataLocation.of("");
+		assertThat(location.getValue().isEmpty()).isTrue();
 	}
 
 	@Test
-	void ofWhenEmptyOptionalValueReturnsNull() {
-		assertThat(ConfigDataLocation.of("optional:")).isNull();
+	void ofWhenEmptyOptionalValueReturnsEmptyLocation() {
+		ConfigDataLocation location = ConfigDataLocation.of("optional:");
+		assertThat(location.getValue().isEmpty()).isTrue();
 	}
 
 	@Test