Improve ConfigProperty lookup

[Emily-Jiang]

Description

As a:

• Application user/user of the configuration itself
• API user (application developer)
• SPI user (container or runtime developer)
• Specification implementer

...I need to be able to:

use @Inject @ConfigProperty("property.name") instead of having to put a name attribute there
@Inject @ConfigProperty(name="property.name")

...which enables me to:
simply my lookup.

[Emily-Jiang]

This can be achieved by adding value attribute on ConfigProperty annotation, which will be doing what name attribute does. If someone happens use both attributes and the supplied the string is different, DeploymentException will be thrown.
For an instance,
@Inject @ConfigProperty(name="customer.name", value="Bob") String name;
The above should cause the exception DeploymentException to be thrown.

[radcortez]

The initial API was using value but it was later changed to name here: #99.

I can't find any reason for why it was changed. Do you remember?

[Emily-Jiang]

The initial API was using value but it was later changed to name here: #99.

I can't find any reason for why it was changed. Do you remember?

@radcortez thanks for digging further! Originally I proposed to have the value attribute. However, after introducing defaultValue attribute, it will be confusing to have something:
@Inject @ConfigProperty(value="customer.name", defaultValue="Bob") String cName;
At that time, I did not think about having both name and value. Hence, this issue is raised to add that.

With both name and value, end users can use value whendefultValue is absent.
@Inject @ConfigProperty("customer.name")
when defaultValue is present, they can continue using:
@Inject @ConfigProperty(name="customer.name", defaultValue="Bob") String cName;

Basically, the attribute name and value are identical. However, if someone uses both with a different content, an exception should be thrown.

[ljnelson]

I'd like to make sure I understand this absolutely clearly.

You are proposing that:

1. three elements will exist on the ConfigProperty annotation: name, value and defaultValue
2. All will be declared as not required, so that an annotation of, simply, @ConfigProperty is at least syntactically legal (if not semantically legal)
3. name and value will represent exactly the same thing even though they designate completely different concepts
4. All usages of this new ConfigProperty annotation must be scanned and validated to ensure that incompatible values are not specified together, and that at least one of either name or value is specified

Does that correctly represent what you are proposing?

This will almost certainly be immensely confusing to end users and consequently I object to it.

Perhaps a replacement annotation is called for instead (perhaps without default-value semantics as well, given how haphazardly #446 (and #531 and #532 and #533) was addressed, and how thorny default values are in general, and how difficult they are, therefore, to encode in the primary qualifier annotation, as many issues have borne out)?

Simpler, clearer replacement possibilities might include:

• ConfigKey("foo")
• ConfigKeyNamed("foo")
• Value("foo")
• ValueNamed("foo")
• Setting("foo")
• SettingNamed("foo")

[Emily-Jiang]

@ljnelson I really don't understand what you are objecting about. This issue neither fix or make the existing behaviours you listed above any worse though as I am not touching any defaultValue attribute. I have not seen any end users reporting an issue with defaultValue either. By the way, I don't see introducing new annotations any help.

[ljnelson]

I believe that using name and value to mean the same thing is confusing, since these words mean entirely different things.

See #671 for one issue about defaultValue.

[radcortez]

In SR Config we added additional annotations (they work only for the mapping classes), but maybe they can serve as an inspirations: @WithName, @WithDefault, @WithConverter, etc.

[NottyCode]

@Emily-Jiang I know this came from a discussion we had, but one downside of this is to make this work we would have to mark both value and name as optional. That would means someone could use the annotation with no attributes which would be wrong. However I have confirmed if there is an attribute of value then you can indeed do @ConfigProperty("my.config.prop") which leads to the simplification in the case of no default value.

[rdebusscher]

Is this 'simplification' worth the possible confusion about the meaning of all attributes?

[tomas-langer]

I think that a better solution would be to introduce a new annotation and deprecate the old one.
Such as @ConfigValue("my.config.prop") or @Value("my.config.prop")

This way there is no need to handle multiple optional annotation attributes - each annotation has a mandatory attribute.

I also think that default value should not be part of the annotation - we could do similar what JAX-RS is doing:

@Inject
public MyClass(@Value("my.config.prop") @DefaultValue("default") String value) {
...
}

[Emily-Jiang]

I don't see much value to introduce a brandnew annotation in order to achieve a fairly minimal improvement. This leads end users further learning new things. I would rather stay with ConfigProperty(name="something"). Anyway, it is just a few characters and IDE can help with it.

[m0mus]

I agree with @tomas-langer and @rdebusscher. Having 2 attributes meaning the same thing is confusing and it's not a simplification. I am voting for deprecation of @ConfigProperty with name and defaultValue attributes in favour of @ConfigValue and @DefaultValue. It's more clean and aligned with what JAX-RS folks are doing.

[NottyCode]

I can understand the perspective that says having name and value meaning the same thing would be confusing and would be more confusing than having people write name. However I think deprecating ConfigProperty at this point would be a mistake. ConfigProperty is the primary API for MP Config, it has persisted through multiple years of revision and throwing it away now would be very disruptive and I think it would alienate people who have adopted MicroProfile for little benefit.

If I saw @ConfigValue as an annotation I would expect that it provides a config value, not that it consumes one based just on the name. I would be very confused at using a @DefaultValue from Jakarta REST and if MP Config duplicated the name you'll have the development time headache that you'll likely see people selecting the wrong one when doing type auto-import in tools. Also if you have two annotations you have to get into the question of what happens if they are not used in combination, whereas at least for attributes on a single annotation there is conceptual grouping via the @ConfigProperty annotation.

[m0mus]

I share the concern of logical combination. @DefaultValue doesn't make sense if @ConfigValue is not present.

If a Java property is annotated with @ConfigProperty without either name or value specified, the actual configuration property name can be constructed basing on the Java property name using some rules defined by the spec.

Example:

@ConfigProperty
Integer foo; // Read from "foo" configuration property

@ConfigProperty
Integer fooBar; // Read from "foo.bar" configuration property

@ConfigProperty(defaultValue="5")
Integer fooBarBaz; // Read from "foo.bar.baz" configuration property with default value of 5