Merge pull request #25507 from brenuart

* pr/25507: Polish "Include properties in source merge algorithm" Include properties in source merge algorithm Closes gh-25507
spring-projects · Mar 5, 2021 · 034e7d4 · 034e7d4
2 parents c62367e + 6ebc69d
commit 034e7d4
Show file tree

Hide file tree

Showing 3 changed files with 86 additions and 11 deletions.
diff --git a/...org/springframework/boot/configurationmetadata/SimpleConfigurationMetadataRepository.java b/...org/springframework/boot/configurationmetadata/SimpleConfigurationMetadataRepository.java
@@ -61,7 +61,7 @@ public void add(Collection<ConfigurationMetadataSource> sources) {
 			}
 			String sourceType = source.getType();
 			if (sourceType != null) {
-				putIfAbsent(group.getSources(), sourceType, source);
+				addOrMergeSource(group.getSources(), sourceType, source);
 			}
 		}
 	}
@@ -93,7 +93,7 @@ public void include(ConfigurationMetadataRepository repository) {
 				// Merge properties
 				group.getProperties().forEach((name, value) -> putIfAbsent(existingGroup.getProperties(), name, value));
 				// Merge sources
-				group.getSources().forEach((name, value) -> putIfAbsent(existingGroup.getSources(), name, value));
+				group.getSources().forEach((name, value) -> addOrMergeSource(existingGroup.getSources(), name, value));
 			}
 		}
 
@@ -111,6 +111,17 @@ private ConfigurationMetadataGroup getGroup(ConfigurationMetadataSource source)
 		return this.allGroups.get(source.getGroupId());
 	}
 
+	private void addOrMergeSource(Map<String, ConfigurationMetadataSource> sources, String name,
+			ConfigurationMetadataSource source) {
+		ConfigurationMetadataSource existingSource = sources.get(name);
+		if (existingSource == null) {
+			sources.put(name, source);
+		}
+		else {
+			source.getProperties().forEach((k, v) -> putIfAbsent(existingSource.getProperties(), k, v));
+		}
+	}
+
 	private <V> void putIfAbsent(Map<String, V> map, String key, V value) {
 		if (!map.containsKey(key)) {
 			map.put(key, value);

diff --git a/...framework/boot/configurationmetadata/ConfigurationMetadataRepositoryJsonBuilderTests.java b/...framework/boot/configurationmetadata/ConfigurationMetadataRepositoryJsonBuilderTests.java
@@ -18,6 +18,7 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.Arrays;
 import java.util.Map;
 
 import org.junit.jupiter.api.Test;
@@ -99,16 +100,56 @@ void severalRepositoriesIdenticalGroups() throws IOException {
 			try (InputStream foo2 = getInputStreamFor("foo2")) {
 				ConfigurationMetadataRepository repo = ConfigurationMetadataRepositoryJsonBuilder.create(foo, foo2)
 						.build();
-				assertThat(repo.getAllGroups()).hasSize(1);
+				Iterable<String> allKeys = Arrays.asList("spring.foo.name", "spring.foo.description",
+						"spring.foo.counter", "spring.foo.enabled", "spring.foo.type");
+				assertThat(repo.getAllProperties()).containsOnlyKeys(allKeys);
+				assertThat(repo.getAllGroups()).containsOnlyKeys("spring.foo");
 				ConfigurationMetadataGroup group = repo.getAllGroups().get("spring.foo");
-				contains(group.getSources(), "org.acme.Foo", "org.acme.Foo2", "org.springframework.boot.FooProperties");
-				assertThat(group.getSources()).hasSize(3);
-				contains(group.getProperties(), "spring.foo.name", "spring.foo.description", "spring.foo.counter",
-						"spring.foo.enabled", "spring.foo.type");
-				assertThat(group.getProperties()).hasSize(5);
-				contains(repo.getAllProperties(), "spring.foo.name", "spring.foo.description", "spring.foo.counter",
-						"spring.foo.enabled", "spring.foo.type");
-				assertThat(repo.getAllProperties()).hasSize(5);
+				assertThat(group.getProperties()).containsOnlyKeys(allKeys);
+				assertThat(group.getSources()).containsOnlyKeys("org.acme.Foo", "org.acme.Foo2",
+						"org.springframework.boot.FooProperties");
+				assertThat(group.getSources().get("org.acme.Foo").getProperties()).containsOnlyKeys("spring.foo.name",
+						"spring.foo.description");
+				assertThat(group.getSources().get("org.acme.Foo2").getProperties())
+						.containsOnlyKeys("spring.foo.enabled", "spring.foo.type");
+				assertThat(group.getSources().get("org.springframework.boot.FooProperties").getProperties())
+						.containsOnlyKeys("spring.foo.name", "spring.foo.counter");
+			}
+		}
+	}
+
+	@Test
+	void severalRepositoriesIdenticalGroupsWithSameType() throws IOException {
+		try (InputStream foo = getInputStreamFor("foo")) {
+			try (InputStream foo3 = getInputStreamFor("foo3")) {
+				ConfigurationMetadataRepository repo = ConfigurationMetadataRepositoryJsonBuilder.create(foo, foo3)
+						.build();
+				Iterable<String> allKeys = Arrays.asList("spring.foo.name", "spring.foo.description",
+						"spring.foo.counter", "spring.foo.enabled", "spring.foo.type");
+				assertThat(repo.getAllProperties()).containsOnlyKeys(allKeys);
+				assertThat(repo.getAllGroups()).containsOnlyKeys("spring.foo");
+				ConfigurationMetadataGroup group = repo.getAllGroups().get("spring.foo");
+				assertThat(group.getProperties()).containsOnlyKeys(allKeys);
+				assertThat(group.getSources()).containsOnlyKeys("org.acme.Foo",
+						"org.springframework.boot.FooProperties");
+				assertThat(group.getSources().get("org.acme.Foo").getProperties()).containsOnlyKeys("spring.foo.name",
+						"spring.foo.description", "spring.foo.enabled", "spring.foo.type");
+				assertThat(group.getSources().get("org.springframework.boot.FooProperties").getProperties())
+						.containsOnlyKeys("spring.foo.name", "spring.foo.counter");
+			}
+		}
+	}
+
+	@Test
+	void severalRepositoriesIdenticalGroupsWithSameTypeDoesNotOverrideSource() throws IOException {
+		try (InputStream foo = getInputStreamFor("foo")) {
+			try (InputStream foo3 = getInputStreamFor("foo3")) {
+				ConfigurationMetadataRepository repo = ConfigurationMetadataRepositoryJsonBuilder.create(foo, foo3)
+						.build();
+				ConfigurationMetadataGroup group = repo.getAllGroups().get("spring.foo");
+				ConfigurationMetadataSource fooSource = group.getSources().get("org.acme.Foo");
+				assertThat(fooSource.getSourceMethod()).isEqualTo("foo()");
+				assertThat(fooSource.getDescription()).isEqualTo("This is Foo.");
 			}
 		}
 	}

diff --git a/...-boot-configuration-metadata/src/test/resources/metadata/configuration-metadata-foo3.json b/...-boot-configuration-metadata/src/test/resources/metadata/configuration-metadata-foo3.json
@@ -0,0 +1,23 @@
+{
+  "groups": [
+    {
+      "name": "spring.foo",
+      "type": "org.acme.Foo",
+      "sourceType": "org.acme.config.FooApp",
+      "sourceMethod": "foo3()",
+      "description": "This is Foo3."
+    }
+  ],
+  "properties": [
+    {
+      "name": "spring.foo.enabled",
+      "type": "java.lang.Boolean",
+      "sourceType": "org.acme.Foo"
+    },
+    {
+      "name": "spring.foo.type",
+      "type": "java.lang.String",
+      "sourceType": "org.acme.Foo"
+    }
+  ]
+}