SpringBoot Allow specifying unit type of configuration property when injected via constructor

Allow specifying unit type of configuration property when injected via constructor #20892

Comment From: snicoll

Before merging this we should double check that the metadata is generated with the correct unit.

Comment From: wilkinsona

The unit seems to be missing from the metadata. For example:

{
  "name": "test.size",
  "type": "org.springframework.util.unit.DataSize",
  "sourceType": "com.example.demo.ConstructorParameterWithConversionProperties",
  "defaultValue": "3"
}

The above is from a properties class based on the one that's added to the tests in this PR. The constructor argument is declared as @DefaultValue("3") @DataSizeUnit(DataUnit.MEGABYTES) DataSize size so I believe the default value should be 3MB.

Comment From: snicoll

I would have expected that indeed. We need to update this support to lookup the unit annotation on others places.

Comment From: snicoll

Looking at the metadata and how values are extracted, this applies using the code that's written for the field initialization something like, Duration someProperty = Duration.ofHours(40);. We can extract 40h from that. If we compare with the constructor based approach, we should provide the value as-is IMO as @DefaultValue is the text-based default value of the property. I think it should be @DefaultValue("3MB") @DataSizeUnit(DataUnit.MEGABYTES) rather than @DefaultValue("3") @DataSizeUnit(DataUnit.MEGABYTES).

Allowing the unit on the constructor parameter makes such construct possible so it'd be legitimate for users to expect the other variant to work. It means that the annotation processor has to look for those 3 additional annotations too and handle it only if the default value is an integer. I wonder if we want that extra complexity.

Comment From: snicoll

We've discussed this and concluded that having 3 in the metadata is acceptable considering that using 3 manually would lead to the expected result (3MB). To provide better metadata, the unit should ideally be specified again in @DefaultValue so that the generated default value in the metadata is a bit more explicit.