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

Constructing a ConfigurationMetadataRepository from two distinct metadata file leads to incorrect results when the files contain properties that belong to the same group. Here is a scenario to illustrate the case.

Suppose the two following spring-configuration-metadata files. As you can see, they hold properties of different names but they all belong to the same group:

----------
meta1.json
----------
{
  "groups": [
    {
      "name": "server",
      "type": "org.springframework.boot.autoconfigure.web.ServerProperties",
      "sourceType": "org.springframework.boot.autoconfigure.web.ServerProperties"
    }
  ],
  "properties": [
    {
      "name": "server.port",
      "type": "java.lang.Integer",
      "description": "Server HTTP port.",
      "sourceType": "org.springframework.boot.autoconfigure.web.ServerProperties"
    }
  ]
}

----------
meta2.json
----------
{
  "groups": [
    {
      "name": "server",
      "type": "org.springframework.boot.autoconfigure.web.ServerProperties",
      "sourceType": "org.springframework.boot.autoconfigure.web.ServerProperties"
    }
  ],
  "properties": [
    {
      "sourceType": "org.springframework.boot.autoconfigure.web.ServerProperties",
      "name": "server.address",
      "description": "Network address to which the server should bind to.",
      "type": "java.net.InetAddress"
    }
  ]
}

Now I try to read these two files into the same ConfigurationMetadataRepository as follows:

ConfigurationMetadataRepositoryJsonBuilder metadataBuilder = ConfigurationMetadataRepositoryJsonBuilder.create();
metadataBuilder.withJsonResource(new FileInputStream("meta1.json"));
metadataBuilder.withJsonResource(new FileInputStream("meta2.json"));

ConfigurationMetadataRepository metadata = metadataBuilder.build();

As shown below, #getAllProperties() returns both properties as expected, and #getAllGroups() returns the only server group. Listing the properties of that group correctly returns to two properties as well. All this seems to work as expected.

metadata.getAllProperties()    --> server.port
                                   server.address

metadata.getAllGroups()        --> server

"server group".getProperties() --> server.port
                                   server.address

The "server" group contains a single "source" (as expected). However, listing the properties of that single source returns only the server.port address, not both as I would expect.

"server group".getSources()                  --> server
"server group".getSources().getProperties()  --> server.port

As far as I understand the model the following should apply: - group.properties should contain all properties of all the sources - the union of all source.properties should match group.properties

The last assumption is not verified in my test scenario.

According to me the issue is in the merge logic implemented in SimpleConfigurationMetadata#include.. The repository already contains a "server" group when the second metadata file is loaded and the include method tries to merge the content of the new group into the existing one. This is done by adding the properties of the new group into the existing one if they are missing. When done, the logic goes further by "merging" the sources. However, the merge logic for the sources seems incorrect: their properties should be merged as well.

Stated differently, adding the following method should solve the issue:

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));
  }
}

Comment From: snicoll

@brenuart thanks for the analysis. It looks sensible on surface to me. Could you please move that to a PR with an additional test that demonstrates the scenario you've described?

Comment From: snicoll

@brenuart thank you for the sample. I agree that the end result is a little bit surprising but I don't really understand how you would end up in such a situation.

A source is identified by a type. Let's say com.example.Acme. Now, if Acme has a number of properties, one metadata file should describe all its properties. In the example above, server may be contributed by multiple POJOs (although a bit unusual) but you wouldn't get properties from the same pojo in two different files. A type is processed locally and fully and can't get additional properties in a modular fashion (since it is tied to the type).

Considering the above, can you share a bit more about your use case concretely? In which situation do you have properties for the same type in several metadata files?

Comment From: brenuart

I agree with you - it is very unlikely that the same class is described in two distinct metadata files (which is somehow my scenario). Unless of course if two projects share the same package and class names - which is hopefully not our use case.

First of all I was a bit confused by the following comment set on ConfigurationMetadataRepositoryJsonBuilder#withJsonResource() that says:

Add the content of a {@link ConfigurationMetadataRepository} defined by the specified {@link InputStream} json document using the default charset. If this metadata repository holds items that were loaded previously, these are ignored.

So I expected the properties of the second file to be added (which is the case) and the group "discarded" since it is an exact duplicate of the existing one. However, this leads to an inconsistent situation as the test case illustrates.

We are actually looking for a way to "override" some metadata inherited from transient dependencies - mainly to provide a valid description when it is missing. This information is later used to generate documentation and additional tooling used at deployment time. The idea was therefore to add entries for these metadata in the file generated by the build the application. Since this file is loaded before the other, I expected them to "shadow" the originals loaded just after (as per the javadoc comment on withJsonResource. This is how I end up with duplicate metadata as illustrated by the test case.

I agree this may not be a "valid" usage of these tools. Another option is of course to use a separate custom metadata file of our own and merge its content at runtime with what is loaded from the SpringBoot standard metadata. However, re-using the original metadata has the additional benefit that our "patched" descriptions are available to other standard tools like Java IDE for instances.

Comment From: snicoll

We are actually looking for a way to "override" some metadata inherited from transient dependencies - mainly to provide a valid description when it is missing. This information is later used to generate documentation and additional tooling used at deployment time.

Thanks for the feedback. This is the piece that I was missing.

Closing in favor of PR #25507