Skip to content

Parameterized Configuration Metadata #11280

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
twicksell opened this issue Dec 7, 2017 · 5 comments
Closed

Parameterized Configuration Metadata #11280

twicksell opened this issue Dec 7, 2017 · 5 comments

Comments

@twicksell
Copy link

We've found configuration metadata to be incredibly helpful, not only for helping explore Spring Boot's properties, but as a way to document our internal property usage. However, we are seeing a recurring gap with configuration that has a dynamic element to its property name.

For example, lets look at DataSourceProperties. To support multiple datasources, the current setup requires the user to migrate from properties like spring.datasource.password=secret to foo.datasource.password=secret and bar.datasource.password=secret. However, at this point, we lose all tooling around this property metadata.

I'd like to propose an approach to improving this, at the very least to begin a discussion around how this could be done while retaining tooling support. Imagine refactoring the metadata for the above property to look like the following:

    {
      "name": "spring.datasource.${dataSourceName}.password",
      "type": "java.lang.String",
      "description": "Login password of the database.",
      "sourceType": "org.springframework.boot.autoconfigure.jdbc.DataSourceProperties",
      "parameters": [
             "dataSourceName" : {
                  "optional": true,
                  "defaultValue" : "defaultDataSource"
             }
       ] 
    }

In this model, we are declaring a section of the property is a named parameter with the name "dataSourceName". We specified that this is an optional parameter, so spring.datasource.password should be legal, and function exactly as it does today. However, spring.datasource.foo.password should apply to a datasource bean named foo. This metadata can now feed validation to provide early errors if the config is applied to a bean does not exist. The IDE's may now understand dynamic components of property keys, so that auto completion of properties for spring.datasource.foo can continue to function.

I'd appreciate any thoughts or feedback on this model. Does this seem like a plausible approach?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 7, 2017
@snicoll
Copy link
Member

snicoll commented Dec 7, 2017

Thanks for the proposal.

However, at this point, we lose all tooling around this property metadata.

This example in the documentation gives you full metadata support and a similar configuration experience so I must be missing something in the description of the problem.

I don't really understand the approach you are proposing either vs. map based access. You can effectively do this today with a Map<String,DataSourceProperties>. What you don't have is metadata for map values but the IDEs support that today (inspecting the class directly) and we also have #9945 to improve that use case further).

Regardless, there's definitely one thing I am not keen at all to pursue in your example is to have a dynamic part of the key used at the same level as fixed property names. To be clear, and to take your example, having a datasource that is named password would lead to spring.datasource.password to be both a value and a prefix.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Dec 7, 2017
@twicksell
Copy link
Author

Following that example exactly, the application will start, I will have two datasource beans with the properties applied correctly. But I do not get any support from the IDE around the property metadata, which is what I am specifically interested in.

image

This is from the latest version of STS. Am I just missing something? Should I have javadoc and autocompletion in this example? That is the end state I am looking for.

My idea is not around how I would interact with this in code, as you said injecting Map<String,DatasourceProperties> works perfectly. My idea is how would the actual configuration metadata need to change to signal to the IDE tooling that it is possible to have multiple datasources, each with a unique name/key.

Regarding overlap between datasource names and fields on the datasource, I agree that would be a problem. Some validation around collisions seems possible given we know all of the fields from the ConfigurationProperties object. But I did want to put the power into the hands of the library author (whoever defines DatasourceProperties vs the app owner who is registering multiple instances of them) so that they can design the property namespace that seems most suitable. You could amend my previous example to be something like spring.datasource.${dataSourceName}.props.password and eliminate conflicts.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Dec 7, 2017
@snicoll
Copy link
Member

snicoll commented Dec 7, 2017

Am I just missing something?

That or I am. To be sure, can you please share a sample that reproduces the problem that you're experiencing? It is definitely working for me.

My idea is how would the actual configuration metadata need to change to signal to the IDE tooling that it is possible to have multiple datasources, each with a unique name/key.

I think the sample will help as I don't understand the bits that would be missing at this point.

@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Dec 7, 2017
@spring-projects-issues
Copy link
Collaborator

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Dec 14, 2017
@spring-projects-issues
Copy link
Collaborator

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

@spring-projects-issues spring-projects-issues removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged labels Dec 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants