Skip to content

Document that constructor binding does not support @DurationUnit and @DataSizeUnit #22565

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
jordigarcl opened this issue Jul 25, 2020 · 3 comments
Assignees
Labels
type: documentation A documentation update
Milestone

Comments

@jordigarcl
Copy link

According to this documentation, I should be able to use @DurationUnit to change the default unit when converting a duration. However, the example above keeps using MILLISECONDS as the duration unit.

Happening in the latest Spring version

  1. When not specifying a unit in application.yml:
myconfig:
  myDuration: 70
  1. And specifying DurationUnit to SECONDS in config properties class
@ConstructorBinding
@ConfigurationProperties("myconfig")
class MyProperties(
  @DurationUnit(ChronoUnit.SECONDS)
  val myDuration: Duration,
)
  1. Then duration is converted using default MILLISECONDS unit
myProperties.myDuration.toString() // -> "PT0.07S"
@jordigarcl
Copy link
Author

Also, I don't understand why the current documentation provides this example:

@ConfigurationProperties("app.system")
public class AppSystemProperties {

    @DurationUnit(ChronoUnit.SECONDS)
    private Duration sessionTimeout = Duration.ofSeconds(30);
}

Why is it programmatically setting Duration.ofSeconds(30) ? Isn't the whole purpose to use the properties value?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 25, 2020
@wilkinsona
Copy link
Member

Duration.ofSeconds(30) is the default value for the app.system.sessionTimeout property.

@wilkinsona wilkinsona changed the title @DurationUnit not working in Kotlin constructor @DurationUnit does not work with constructor binding Jul 28, 2020
@wilkinsona
Copy link
Member

This has been addressed in 2.4 where @DurationUnit can now be used on constructor parameters. I think we still need to do something in a maintenance branch, though. The problem isn't specific to Kotlin as the following Java does not work either:

@DurationUnit(ChronoUnit.SECONDS) 
private final Duration duration;

public MyProperties(Duration duration) {
	this.duration = duration;
}

We can't be sure that the duration constructor parameter will be stored in the duration field so there's no way of reliably discovering the duration unit. I suspect the best that we can do is to update the documentation to make it clear that @DurationUnit is only supported with JavaBean-based property binding.

@wilkinsona wilkinsona added the for: team-attention An issue we'd like other members of the team to review label Jul 28, 2020
@philwebb philwebb added type: documentation A documentation update and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Jul 29, 2020
@philwebb philwebb assigned philwebb and wilkinsona and unassigned philwebb Jul 29, 2020
@philwebb philwebb added this to the 2.2.x milestone Jul 29, 2020
@wilkinsona wilkinsona changed the title @DurationUnit does not work with constructor binding Document that constructor binding does not support @DurationUnit and @DataSizeUnit Jul 30, 2020
@wilkinsona wilkinsona modified the milestones: 2.2.x, 2.2.10 Jul 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation A documentation update
Projects
None yet
Development

No branches or pull requests

4 participants