Improve handling of environment variables in failure analysis

Prior to this change, the failure analysis for an invalid
configuration property value filtered out the configuration property
sources property source. This property source contains a "duplicate"
of all of the environment's other property sources but with
configuration property support (such as relaxed/fuzzy matching of
environment variables). This was done to prevent the reporting of
duplicates when a property was found in both the configuration
property sources property source and the "normal" property sources.
An unwanted side-effect of this was that fuzzy matching of
environment variables was lost so the origin of
com.example.some-property would be found in the environment variable
was COM_EXAMPLE_SOME_PROPERTY but would not be found if it was
COM_EXAMPLE_SOMEPROPERTY.

This commit addresses this side-effect by no longer filtering out
the configuration property sources property source. To then
prevent duplicates from being reported in the analysis, it instead
deduplicates things based on the origin of each property that's
found in the environment's property sources.

Fixes gh-43380

Author: wilkinsona

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/InvalidConfigurationPropertyValueFailureAnalyzer.java

@@ -16,16 +16,18 @@
 
 package org.springframework.boot.diagnostics.analyzer;
 
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.stream.Stream;
 
-import org.springframework.boot.context.properties.source.ConfigurationPropertySources;
 import org.springframework.boot.context.properties.source.InvalidConfigurationPropertyValueException;
 import org.springframework.boot.diagnostics.AbstractFailureAnalyzer;
 import org.springframework.boot.diagnostics.FailureAnalysis;
 import org.springframework.boot.diagnostics.FailureAnalyzer;
 import org.springframework.boot.origin.Origin;
 import org.springframework.boot.origin.OriginLookup;
+import org.springframework.boot.origin.PropertySourceOrigin;
 import org.springframework.core.env.ConfigurableEnvironment;
 import org.springframework.core.env.Environment;
 import org.springframework.core.env.PropertySource;
@@ -61,18 +63,23 @@ protected FailureAnalysis analyze(Throwable rootFailure, InvalidConfigurationPro
 	}
 
 	private List<Descriptor> getDescriptors(String propertyName) {
+		Set<Origin> seen = new HashSet<>();
 		return getPropertySources().filter((source) -> source.containsProperty(propertyName))
 			.map((source) -> Descriptor.get(source, propertyName))
+			.filter((descriptor) -> seen.add(getOrigin(descriptor)))
 			.toList();
 	}
 
-	private Stream<PropertySource<?>> getPropertySources() {
-		if (this.environment == null) {
-			return Stream.empty();
+	private Origin getOrigin(Descriptor descriptor) {
+		Origin origin = descriptor.origin;
+		if (origin instanceof PropertySourceOrigin propertySourceOrigin) {
+			origin = propertySourceOrigin.getOrigin();
 		}
-		return this.environment.getPropertySources()
-			.stream()
-			.filter((source) -> !ConfigurationPropertySources.isAttachedConfigurationPropertySource(source));
+		return origin;
+	}
+
+	private Stream<PropertySource<?>> getPropertySources() {
+		return (this.environment != null) ? this.environment.getPropertySources().stream() : Stream.empty();
 	}
 
 	private void appendDetails(StringBuilder message, InvalidConfigurationPropertyValueException cause,

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/InvalidConfigurationPropertyValueFailureAnalyzerTests.java

@@ -20,13 +20,16 @@
 
 import org.junit.jupiter.api.Test;
 
+import org.springframework.boot.context.properties.source.ConfigurationPropertySources;
 import org.springframework.boot.context.properties.source.InvalidConfigurationPropertyValueException;
 import org.springframework.boot.diagnostics.FailureAnalysis;
 import org.springframework.boot.origin.Origin;
 import org.springframework.boot.origin.OriginLookup;
 import org.springframework.core.env.EnumerablePropertySource;
 import org.springframework.core.env.MapPropertySource;
+import org.springframework.core.env.SystemEnvironmentPropertySource;
 import org.springframework.mock.env.MockEnvironment;
+import org.springframework.util.ObjectUtils;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -63,6 +66,26 @@ void analysisWithKnownProperty() {
 			.doesNotContain("Additionally, this property is also set");
 	}
 
+	@Test
+	void analysisWithKnownPropertyFromSystemEnvironment() {
+		MapPropertySource source = new SystemEnvironmentPropertySource("systemEnvironment",
+				Collections.singletonMap("COM_EXAMPLE_TESTPROPERTY", "invalid"));
+		this.environment.getPropertySources().addFirst(source);
+		ConfigurationPropertySources.attach(this.environment);
+		assertThat(this.environment.getProperty("com.example.test-property")).isEqualTo("invalid");
+		InvalidConfigurationPropertyValueException failure = new InvalidConfigurationPropertyValueException(
+				"com.example.test-property", "invalid", "This is not valid.");
+		FailureAnalysis analysis = performAnalysis(failure);
+		assertThat(analysis.getDescription()).contains("com.example.test-property")
+			.contains("invalid")
+			.contains("property source \"systemEnvironment\"");
+		assertThat(analysis.getCause()).isSameAs(failure);
+		assertThat(analysis.getAction()).contains("Review the value of the property with the provided reason.");
+		assertThat(analysis.getDescription()).contains("Validation failed for the following reason")
+			.contains("This is not valid.")
+			.doesNotContain("Additionally, this property is also set");
+	}
+
 	@Test
 	void analysisWithKnownPropertyAndNoReason() {
 		MapPropertySource source = new MapPropertySource("test", Collections.singletonMap("test.property", "invalid"));
@@ -84,6 +107,8 @@ void analysisWithKnownPropertyAndOtherCandidates() {
 		this.environment.getPropertySources().addFirst(OriginCapablePropertySource.get(source));
 		this.environment.getPropertySources().addLast(additional);
 		this.environment.getPropertySources().addLast(OriginCapablePropertySource.get(another));
+		this.environment.getPropertySources().addLast(OriginCapablePropertySource.get("another-again", another));
+		ConfigurationPropertySources.attach(this.environment);
 		InvalidConfigurationPropertyValueException failure = new InvalidConfigurationPropertyValueException(
 				"test.property", "invalid", "This is not valid.");
 		FailureAnalysis analysis = performAnalysis(failure);
@@ -92,7 +117,8 @@ void analysisWithKnownPropertyAndOtherCandidates() {
 		assertThat(analysis.getDescription())
 			.contains("Additionally, this property is also set in the following property sources:")
 			.contains("In 'additional' with the value 'valid'")
-			.contains("In 'another' with the value 'test' (originating from 'TestOrigin test.property')");
+			.contains("In 'another' with the value 'test' (originating from 'TestOrigin test.property')")
+			.doesNotContain("another-again");
 	}
 
 	@Test
@@ -122,7 +148,11 @@ static class OriginCapablePropertySource<T> extends EnumerablePropertySource<T>
 		private final EnumerablePropertySource<T> propertySource;
 
 		OriginCapablePropertySource(EnumerablePropertySource<T> propertySource) {
-			super(propertySource.getName(), propertySource.getSource());
+			this(propertySource.getName(), propertySource);
+		}
+
+		OriginCapablePropertySource(String name, EnumerablePropertySource<T> propertySource) {
+			super(name, propertySource.getSource());
 			this.propertySource = propertySource;
 		}
 
@@ -138,20 +168,53 @@ public String[] getPropertyNames() {
 
 		@Override
 		public Origin getOrigin(String name) {
-			return new Origin() {
-
-				@Override
-				public String toString() {
-					return "TestOrigin " + name;
-				}
-
-			};
+			return new TestOrigin(name, this.propertySource.getName());
 		}
 
 		static <T> OriginCapablePropertySource<T> get(EnumerablePropertySource<T> propertySource) {
 			return new OriginCapablePropertySource<>(propertySource);
 		}
 
+		static <T> OriginCapablePropertySource<T> get(String name, EnumerablePropertySource<T> propertySource) {
+			return new OriginCapablePropertySource<>(name, propertySource);
+		}
+
+		static final class TestOrigin implements Origin {
+
+			private final String name;
+
+			private final String sourceName;
+
+			private TestOrigin(String name, String sourceName) {
+				this.name = name;
+				this.sourceName = sourceName;
+			}
+
+			@Override
+			public boolean equals(Object obj) {
+				if (this == obj) {
+					return true;
+				}
+				if (obj == null || getClass() != obj.getClass()) {
+					return false;
+				}
+				TestOrigin other = (TestOrigin) obj;
+				return ObjectUtils.nullSafeEquals(this.name, other.name)
+						&& ObjectUtils.nullSafeEquals(this.sourceName, other.sourceName);
+			}
+
+			@Override
+			public int hashCode() {
+				return ObjectUtils.nullSafeHashCode(this.name);
+			}
+
+			@Override
+			public String toString() {
+				return "TestOrigin " + this.name;
+			}
+
+		}
+
 	}
 
 }