Skip to content
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

Add support for enum default values in configuration processor #7562

Closed
qerub opened this issue Dec 3, 2016 · 11 comments
Closed

Add support for enum default values in configuration processor #7562

qerub opened this issue Dec 3, 2016 · 11 comments
Assignees
Labels
theme: config-data Issues related to the configuration theme type: enhancement A general enhancement
Milestone

Comments

@qerub
Copy link
Contributor

qerub commented Dec 3, 2016

It's quite common to use enums for configuration properties, but the configuration processor is currently unable to extract the default value for those (because of lack of support in JavaCompilerFieldValuesParser). This means that the generated configuration JSON specification is currently incomplete/incorrect wrt. to enum properties. Unless this functionality has been left out on purpose I can take a stab at adding it.

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

snicoll commented Dec 4, 2016

This is something that can be extracted from code and the IDE is supposed to do that for you. I am not keen to write such duplication in the metadata.

Why are you asking for this? Which IDE do you use?

@qerub
Copy link
Contributor Author

qerub commented Dec 4, 2016

I use IntelliJ IDEA but the wish for this came up outside the context of an IDE when trying to use the configuration meta-data to generate documentation describing all properties and their default values.

@qerub
Copy link
Contributor Author

qerub commented Dec 4, 2016

For example, I was hoping to generate tables with default configuration using jq:

cat **/spring-boot-sample-property-validation/**/spring-configuration-metadata.json | \
jq -r '.properties[] | [.name, .description, .type, .defaultValue | tostring] | @csv' | \
csvfix ascii_table -h "Name, Description, Type, Default value"
+-------------+--------------+-------------------+---------------+ 
|    Name     | Description  |       Type        | Default value |
+-------------+--------------+-------------------+---------------+
| sample.host | Sample host. | java.lang.String  | null          |
| sample.port | Sample port. | java.lang.Integer | 8080          |
+-------------+--------------+-------------------+---------------+

@wilkinsona
Copy link
Member

It sounds to me like there's some confusion here between listing all possible values for a property with an enum type (which an IDE can do automatically) and listing the default value for such a property.

In our own metadata I'd expect there to be a default value listed for security.headers.hsts but there is not:

{
    "name": "security.headers.hsts",
    "type": "org.springframework.boot.autoconfigure.security.SecurityProperties$Headers$HSTS",
    "description": "HTTP Strict Transport Security (HSTS) mode (none, domain, all).",
    "sourceType": "org.springframework.boot.autoconfigure.security.SecurityProperties$Headers"
},

This means that the IDE (I checked in IntelliJ) doesn't display a default value. It does, however, list the three possible values as completion options having extracted them from the enum.

@snicoll
Copy link
Member

snicoll commented Dec 5, 2016

Wow, for some reason my brain read "list of values" instead of default value. Thank you @wilkinsona for pointing that mistake.

Yes, we should try to improve this. If you want to give that a try, please note we use a canonical representation for as much content as we can (my-super-long-property). IDEs have harmonized to that, offering you the canonical representation of the enum value.

Clearly, it means the same work should be done when detecting MY_VALUE (and writing my-value in the metadata).

@snicoll snicoll added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 5, 2016
@qerub
Copy link
Contributor Author

qerub commented Dec 5, 2016

Clearly, it means the same work should be done when detecting MY_VALUE (and writing my-value in the metadata).

Is there a built-in function somewhere in Spring [Boot] I can use to do this conversion?

…or is it OK that I use the function ->kebab-case from my Clojure library https://github.com/qerub/camel-snake-kebab? 😅

@wilkinsona
Copy link
Member

wilkinsona commented Dec 5, 2016

@qerub ConfigurationMetadata.toDashedCase(String) should be able to do that for you.

@qerub
Copy link
Contributor Author

qerub commented Dec 5, 2016

@wilkinsona: Just checked toDashedCase and it doesn't treat NAMES_OF_CONSTANTS like we need (e.g. VERY_SLOWbecomes v-e-r-y-s-l-o-w). In this context of enum names, is it good enough to lowercase and replace _ with - or do we need handle non-idiomatic names as well?

@wilkinsona
Copy link
Member

Gah! Sorry, I should have remembered that. The behaviour is intentional (see #5330). I think lower case and replace should be good enough.

@snicoll
Copy link
Member

snicoll commented Jan 5, 2017

As discussed in #7593 there isn't a definitive way to know that the symbol represents an enum when we are looking for the default value. This could lead to false positive in the metadata so we recommend to use manual metadata for those. I'll update the documentation in #7890

@snicoll snicoll closed this as completed Jan 5, 2017
@snicoll snicoll added status: declined A suggestion or change that we don't feel we should currently apply and removed type: enhancement A general enhancement labels Jan 5, 2017
@snicoll snicoll removed their assignment Jan 5, 2017
@snicoll
Copy link
Member

snicoll commented Jan 5, 2017

Let's try to have another look still.

@snicoll snicoll reopened this Jan 5, 2017
@snicoll snicoll added priority: normal theme: config-data Issues related to the configuration theme type: enhancement A general enhancement and removed status: declined A suggestion or change that we don't feel we should currently apply labels Jan 12, 2017
@philwebb philwebb added this to the 2.x milestone Jan 25, 2019
@philwebb philwebb removed this from the 2.x milestone Aug 19, 2022
@philwebb philwebb added this to the 3.x milestone Aug 19, 2022
snicoll added a commit that referenced this issue Aug 1, 2024
This commit improves the configuration metadata annotation processor to
detect a default enum value. The algorithm is best-effort, similarly to
what it already does for well known prefixes (period, duration, etc).

Based on an expression and an identifier, the default value is inferred
if the expression matches the declaration of the property type.

See gh-7562
@snicoll snicoll closed this as completed in 7920e37 Aug 1, 2024
@snicoll snicoll modified the milestones: 3.x, 3.4.0-M2 Aug 1, 2024
@snicoll snicoll self-assigned this Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: config-data Issues related to the configuration theme type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants