Skip to content

Incorrect ConfigurationMetadataRepository when loaded from json files containing properties of the same group #25507

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -117,4 +117,17 @@ private <V> void putIfAbsent(Map<String, V> map, String key, V value) {
}
}

/*
Uncomment this code to fix issue revealed by ConfigurationMetadataRepositoryJsonBuilderTests#severalRepositoriesIdenticalGroups3()

private void putIfAbsent(Map<String, ConfigurationMetadataSource> sources, String name,
ConfigurationMetadataSource source) {
ConfigurationMetadataSource existing = sources.get(name);
if (existing == null) {
sources.put(name, source);
} else {
source.getProperties().forEach((k, v) -> putIfAbsent(existing.getProperties(), k, v));
}
}
*/
}
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,102 @@ void severalRepositoriesIdenticalGroups() throws IOException {
}
}


/*
* A rewrite of severalRepositoriesIdenticalGroups() using "containsOnlyKeys" to show actual vs. expected when assert fails.
*/
@Test
void severalRepositoriesIdenticalGroups_rewritten() throws IOException {
try (InputStream foo = getInputStreamFor("foo")) {
try (InputStream foo2 = getInputStreamFor("foo2")) {
ConfigurationMetadataRepository repo = ConfigurationMetadataRepositoryJsonBuilder.create(foo, foo2)
.build();

// assert all properties are found
assertThat(repo.getAllProperties()).containsOnlyKeys(
"spring.foo.name",
"spring.foo.description",
"spring.foo.counter",
"spring.foo.enabled",
"spring.foo.type");

// we have a single group containing all properties
assertThat(repo.getAllGroups()).containsOnlyKeys("spring.foo");

ConfigurationMetadataGroup group = repo.getAllGroups().get("spring.foo");
assertThat(group.getProperties()).containsOnlyKeys(
"spring.foo.name",
"spring.foo.description",
"spring.foo.counter",
"spring.foo.enabled",
"spring.foo.type");

// the group contains 3 different sources
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");
}
}
}

/*
* "foo3" contains the same properties as "foo2" except they refer to a group that already exists in
* "foo1" (same NAME, same TYPE).
*
* This test shows that the union of properties collected from the sources is less than what the group actually
* contains (some properties are missing).
*/
@Test
void severalRepositoriesIdenticalGroups3() throws IOException {
try (InputStream foo = getInputStreamFor("foo")) {
try (InputStream foo3 = getInputStreamFor("foo3")) {
ConfigurationMetadataRepository repo = ConfigurationMetadataRepositoryJsonBuilder.create(foo, foo3)
.build();

assertThat(repo.getAllProperties()).containsOnlyKeys(
"spring.foo.name",
"spring.foo.description",
"spring.foo.counter",
"spring.foo.enabled",
"spring.foo.type");

assertThat(repo.getAllGroups()).containsOnlyKeys("spring.foo");

ConfigurationMetadataGroup group = repo.getAllGroups().get("spring.foo");
assertThat(group.getProperties()).containsOnlyKeys(
"spring.foo.name",
"spring.foo.description",
"spring.foo.counter",
"spring.foo.enabled",
"spring.foo.type");


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", // <-- missing although present in repo.getAllProperties()
"spring.foo.type"); // <-- missing although present in repo.getAllProperties()

assertThat(group.getSources().get("org.springframework.boot.FooProperties").getProperties()).containsOnlyKeys(
"spring.foo.name",
"spring.foo.counter");
}
}
}


@Test
void emptyGroups() throws IOException {
try (InputStream in = getInputStreamFor("empty-groups")) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"groups": [
{
"name": "spring.foo",
"type": "org.acme.Foo",
"sourceType": "org.acme.config.FooApp",
"sourceMethod": "foo2()",
"description": "This is Foo."
}
],
"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"
}
]
}