Provide configuration property metadata for binding to Map<String, RichType> and List<RichType>

[wilkinsona]

One example of such a Map is in Spring Cloud stream. As things stand, there's no metadata for any of the properties on BindingProperties. I think it would be interesting to explore the possibility of providing metadata for those properties, perhaps using a wildcard for the key in the map:

spring.cloud.stream.bindings.*.content-type
spring.cloud.stream.bindings.*.destination
spring.cloud.stream.bindings.*.group
…

For this to be useful, we'd need a corresponding enhancement in each of the IDE plugins.

[snicoll]

I've thought long and hard about this and I don't think we should do this (See also #9894). There are several reasons:

1. Scalar vs. Group: the only way for us to harvest nested object is by using @NestedConfigurationProperty (essentially, this is not a scalar value, please create a "sub-group" for all the properties defined by that type). If we support that for collections or maps now, how do we tell if the value is scalar or not?
2. Scope: if we find a way to fix this and we offer automatic harvesting of POJOs, where do we stop? The next step is to expect this to be somehow recursive (BindingProperties may hold another nested object).
3. Duplication: the use of the * above is what I ended up considering myself but that pojo can be used in multiple places, in multiples maps and/or in different objects. It means that the metadata is duplicated. If you don't want that, we need a third root objects (to harvest the pojo) and a way to reference to it somehow).

Let's see what the rest of the team thinks.

[philwebb]

For "1)" the @NestedConfigurationProperty could be the indicator that the value isn't a scalar. So @NestedConfigurationProperty Map<String, Foo> is a hint that Foo can be expanded with the * syntax, where as @NestedConfigurationProperty Map<String, Foo> cannot (and we expect to be able to convert a String to a Foo somehow.

For "2)" and "3)" I'd probably go with duplicate *. I don't see the duplication being all that much of a problem, as long as there isn't an infinite loop (e.g. Foo contains a Map<String,Foo>).

[philwebb]

FWIW, I think we shouldn't invest too much time thinking about this until after 2.0.

[snicoll]

and we expect to be able to convert a String to a Foo somehow.

That would break backward compatibility but I think having to add an annotation to ask the AP to "expand" is a nice way to solve the scope problem. Great idea! So 1) and 2) could be fixed by some signals that the AP should expand the metadata (and the binding should work as it does now, regardless of the annotation). That's pretty much what happens today for @NestedConfigurationProperty anyway.

As for 3, duplication is much easier but we need some feedback from IDE developers first IMO (ping @YannCebron @kdvolder and @AlexFalappa).

(By the way, I am kind of changing my mind because I realized recently there is no way to add hints for those types and implementing this would be a nice way to fix that).

[wilkinsona]

Since this issue was raised, the IDEs have plugged the gap to varying degrees. A new gap will appear if we add support for immutable configuration properties that use constructor injection. Without metadata, they'd have to plug that gap and look at the constructor rather than the property setters to figure out what properties to offer.

[rwinch]

@wilkinsona Perhaps I am missing something, but I haven't seen any improvements with this from a Spring Security standpoint. For example:

spring:
  security:
    oauth2:
      client:
        registration:
          github:
            |

I get no auto complete at | (pretend that is the cursor). This is something that I would really like for users.

Can you point me to how the IDE's have plugged the gap?

[snicoll]

I can't speak for Eclipse but IJ has support for this for ages on .properties. There isn't for yaml at the moment though. To be clear, we're now considering this with higher priority given the potential binding change.

[rwinch]

Thanks for the clarification @snicoll. I wasn't aware that .properties were supported by IntelliJ. Also glad to hear that this means this is a higher priority. I had misunderstood it to mean that the IDE already was taking care of the issue.

[kdvolder]

Should work in Eclipse / STS, in .yml as well as .properties. If it doesn't for your particular example please raise a bug with STS4.

[kdvolder]

@rwinch I couldn't get your specific example to work (presumably because I don't have the right stuff on the project's classpath). But here's a similar example, to show how it is (supposed to) work:

https://drive.google.com/open?id=1bfzAdMMV6Oi148yXxEvaKvAfxqTR4nM6

Do let us know by raising a STS bug if you find problems with your actual example.

[rwinch]

Thanks @kdvolder! I will have to give it a try in the latest STS.

PS: The dependency you need is org.springframework.boot:spring-boot-starter-oauth2-client

[mhalbritter]

This file is read by the spring-boot-properties-migrator too, to notify users of deprecations in the property names and on-the-fly rewriting. It would be great if this

a) would support for map-based types (i stumbled over spring.security.saml2.relyingparty.registration.*.identityprovider) and

b) would support wildcards to make renames simpler (e.g. management.metrics.* to management.*.metrics)

[wilkinsona]

The lack of this metadata allowed the mistake that @dreis2211 has fixed in this pull request to slip through. Our Asciidoctor extension only validates maps on a best-effort basis. The prefix, spring.security.saml2.relyingparty.registration, matched and there's no metadata to tell it what properties each entry in the map may have so it was unable to detect and report the error.

[buksvdl]

ALL our properties classes are nested this way. Without the extended functionality there is no reason to pull in the lib. The need stays, so we will go the extra mile and add the missing recursion.

[dgensert]

@philwebb thanks for the feedback. I did not mean to complain about the issue still being open but to rather state the point of view in light of the arguments from the discussion above, having recently experienced weird issues in this direction, particularly with the kotlin spring-boot integration. Maybe I could have chosen my words more elegantly. I fully understand that there are a lot of high priority issues.

In the end it would simply make a lot of sense to have the annotation processor generate properties for all fields involved in the chain of properties in order to provide developers with reasonable insights on how to set the right switches through IDE completions.

In my particular case I wrote a POJO for configuring spring security overloadings, more specifically for adding CORS Configurations per URI-Path (e.g. "/**", "/internal","/public", etc.) including various settings. Then I ended up getting properties not generated from the List which was extremely frustrating as security information should ideally be as-is generated and not manually added (generated from actual code) regardless of the code. Alongside, using Kotlin creates issues since datatypes are not properly translated for default values (e.g. java.lang.Boolean false is generated then the default initialization is true). Lastly, there was weird behavior with the data classes constructor initialization, where it would generate certain properties only if not all attributes where initialized with default values:

• val b: Boolean = true
• val list: List = listOf()

Would fail to generate info, whereas changing to "val list: List" would then yield the property b in the generated file.
Furthermore, the documentation and widely available tutorials on the topic do not yield a sufficient solution. In fact the overall impression from searching for a solution was that it is an antipattern to provide another @ConfigurationProperties Annotation in an inner Record/Data Class.

Wrapping it up: I hope this helps to understand why it makes sense to output generated data of properties containing potentially recurring POJOs for "type": "java.util.List/Map". I guess that it would be of great benefit to make this behavior selectable, e.g. as an additional option to the @NestedConfigurationProperty() or @ConfigurationProperties() annotation, indicating that POJOs in collections should be generated as well, thereby not changing current standard behavior in regards to the mentioned concerns.
It would even be an improvement to add the POJO class reference into the generated metadata, so that we can at least see what type this List references and then do a lookup ourselves.

I hope this feedback gives some more insight into the topic, again sorry if I came across a little too blunt earlier.
Cheers.

[nosan]

I've been working on this issue for a while and have come up with some changes: main...nosan:spring-boot:gh-9945

The proposed changes provide configuration property metadata support for binding to
Map<String, RichType> and List<RichType>. Consider the following example class:

@ConfigurationProperties(prefix = "config")
public class RichProperties<T> {

	private final CustomList customList = new CustomList();

	private final CustomMap customMap = new CustomMap();

	@NestedConfigurationProperty
	private final List rawList = new ArrayList<>();

	@NestedConfigurationProperty
	private final List<?> listOfWildcard = new ArrayList<>();

	@NestedConfigurationProperty
	private final List<List<Person>> listOfList = new ArrayList<>();

	@NestedConfigurationProperty
	private final List<Map<String, Person>> listOfMap = new ArrayList<>();

	@NestedConfigurationProperty
	private final List<Person> list = new ArrayList<>();

	@NestedConfigurationProperty
	private final List<T> listOfUnresolvedGeneric = new ArrayList<>();

	@NestedConfigurationProperty
	private final Map rawMap = new LinkedHashMap<>();

	@NestedConfigurationProperty
	private final Map<String, ?> mapOfWildcard = new LinkedHashMap<>();

	@NestedConfigurationProperty
	private final Map<String, List<Person>> mapOfList = new LinkedHashMap<>();

	@NestedConfigurationProperty
	private final Map<String, Map<String, Person>> mapOfMap = new LinkedHashMap<>();

	@NestedConfigurationProperty
	private final Map<String, Person> map = new LinkedHashMap<>();

	@NestedConfigurationProperty
	private final Map<String, T> mapOfUnresolvedGeneric = new LinkedHashMap<>();

	// get;set
	public static class Person {

		private String name;

		@NestedConfigurationProperty
		private final List<Address> addresses = new ArrayList<>();

		// get;set

	}

	public static class Address {

		private String street;

		// get;set

	}

	public static class CustomMap extends LinkedHashMap<String, Person> {

	}

	public static class CustomList extends LinkedHashSet<Person> {

	}

}

The generated JSON metadata will be:

{
  "groups": [
    {
      "name": "config",
      "type": "org.springframework.boot.configurationsample.specific.RichProperties",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties"
    },
    {
      "name": "config.custom-list",
      "type": "org.springframework.boot.configurationsample.specific.RichProperties$CustomList",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties",
      "sourceMethod": "getCustomList()"
    },
    {
      "name": "config.custom-list.[*].addresses",
      "type": "java.util.List",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Person",
      "sourceMethod": "getAddresses()"
    },
    {
      "name": "config.custom-map",
      "type": "org.springframework.boot.configurationsample.specific.RichProperties$CustomMap",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties",
      "sourceMethod": "getCustomMap()"
    },
    {
      "name": "config.custom-map.*.addresses",
      "type": "java.util.List",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Person",
      "sourceMethod": "getAddresses()"
    },
    {
      "name": "config.list",
      "type": "java.util.List",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties",
      "sourceMethod": "getList()"
    },
    {
      "name": "config.list-of-list",
      "type": "java.util.List",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties",
      "sourceMethod": "getListOfList()"
    },
    {
      "name": "config.list-of-list.[*].[*].addresses",
      "type": "java.util.List",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Person",
      "sourceMethod": "getAddresses()"
    },
    {
      "name": "config.list-of-map",
      "type": "java.util.List",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties",
      "sourceMethod": "getListOfMap()"
    },
    {
      "name": "config.list-of-map.[*].*.addresses",
      "type": "java.util.List",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Person",
      "sourceMethod": "getAddresses()"
    },
    {
      "name": "config.list-of-unresolved-generic",
      "type": "java.util.List",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties",
      "sourceMethod": "getListOfUnresolvedGeneric()"
    },
    {
      "name": "config.list-of-wildcard",
      "type": "java.util.List",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties",
      "sourceMethod": "getListOfWildcard()"
    },
    {
      "name": "config.list.[*].addresses",
      "type": "java.util.List",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Person",
      "sourceMethod": "getAddresses()"
    },
    {
      "name": "config.map",
      "type": "java.util.Map",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties",
      "sourceMethod": "getMap()"
    },
    {
      "name": "config.map-of-list",
      "type": "java.util.Map",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties",
      "sourceMethod": "getMapOfList()"
    },
    {
      "name": "config.map-of-list.*.[*].addresses",
      "type": "java.util.List",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Person",
      "sourceMethod": "getAddresses()"
    },
    {
      "name": "config.map-of-map",
      "type": "java.util.Map",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties",
      "sourceMethod": "getMapOfMap()"
    },
    {
      "name": "config.map-of-map.*.*.addresses",
      "type": "java.util.List",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Person",
      "sourceMethod": "getAddresses()"
    },
    {
      "name": "config.map-of-unresolved-generic",
      "type": "java.util.Map",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties",
      "sourceMethod": "getMapOfUnresolvedGeneric()"
    },
    {
      "name": "config.map-of-wildcard",
      "type": "java.util.Map",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties",
      "sourceMethod": "getMapOfWildcard()"
    },
    {
      "name": "config.map.*.addresses",
      "type": "java.util.List",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Person",
      "sourceMethod": "getAddresses()"
    },
    {
      "name": "config.raw-list",
      "type": "java.util.List",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties",
      "sourceMethod": "getRawList()"
    },
    {
      "name": "config.raw-map",
      "type": "java.util.Map",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties",
      "sourceMethod": "getRawMap()"
    }
  ],
  "properties": [
    {
      "name": "config.custom-list.[*].addresses.[*].street",
      "type": "java.lang.String",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Address"
    },
    {
      "name": "config.custom-list.[*].name",
      "type": "java.lang.String",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Person"
    },
    {
      "name": "config.custom-map.*.addresses.[*].street",
      "type": "java.lang.String",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Address"
    },
    {
      "name": "config.custom-map.*.name",
      "type": "java.lang.String",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Person"
    },
    {
      "name": "config.list-of-list.[*].[*].addresses.[*].street",
      "type": "java.lang.String",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Address"
    },
    {
      "name": "config.list-of-list.[*].[*].name",
      "type": "java.lang.String",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Person"
    },
    {
      "name": "config.list-of-map.[*].*.addresses.[*].street",
      "type": "java.lang.String",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Address"
    },
    {
      "name": "config.list-of-map.[*].*.name",
      "type": "java.lang.String",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Person"
    },
    {
      "name": "config.list.[*].addresses.[*].street",
      "type": "java.lang.String",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Address"
    },
    {
      "name": "config.list.[*].name",
      "type": "java.lang.String",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Person"
    },
    {
      "name": "config.map-of-list.*.[*].addresses.[*].street",
      "type": "java.lang.String",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Address"
    },
    {
      "name": "config.map-of-list.*.[*].name",
      "type": "java.lang.String",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Person"
    },
    {
      "name": "config.map-of-map.*.*.addresses.[*].street",
      "type": "java.lang.String",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Address"
    },
    {
      "name": "config.map-of-map.*.*.name",
      "type": "java.lang.String",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Person"
    },
    {
      "name": "config.map.*.addresses.[*].street",
      "type": "java.lang.String",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Address"
    },
    {
      "name": "config.map.*.name",
      "type": "java.lang.String",
      "sourceType": "org.springframework.boot.configurationsample.specific.RichProperties$Person"
    }
  ],
  "hints": [],
  "ignored": {
    "properties": []
  }
}

I've used the prefix * for Map types and the prefix [*] for Collection types. I
intentionally avoided using * for both to ensure the IDE can clearly distinguish between
Map and Collection types.

class Type {

	String name;

}

// map.*.name
Map<String, Type> map;

// map-of-map.*.*.name
Map<String, Map<String, Type>> mapOfMap;

// list.[*].name
List<Type> list;

// list-of-list.[*].[*].name
List<List<Type>> listOfList;

// list-of-map.[*].*.name
List<Map<String, Type>> listOfMap;

// map-of-list.*.[*].name
Map<String, List<Type>> mapOfList;