Config property annotation approach

[tomas-langer]

Discussion about annotation.

Should we use a single annotation?
@ConfigProperty(value = "...", defaultValue = ".."
Should we use more:

@Value("a") 
@DefaultValue("b")
@Converter(MyConverter.class)

[Joofthan]

IMO a single annotation is better.
With multiple annotation the developer has to know which annotations can be combined. Mixing the annotation „DefaultValue“ from jax-rs with the one in config package could lead to errors.

With a single annotation all options are grouped together so that the developer does not have to remember the name of other combineable annotations. He can simply use the autocompletition of most IDEs.

I also would like to see useage like so @ConfigProperty("db.user").
I want to leave these options here for discussion:

1.@ConfigProperty(name="db.user", defaultValue="admin")
2.@ConfigProperty(value="db.user", defaultValue="admin")
3.@ConfigProperty(value="db.user", defaultContent="admin")
4.@ConfigProperty(value="db.user", defaultData="admin")
5.@ConfigProperty(value="db.user", fallback="admin")
6.@ConfigProperty(value="db.user", defaultFallback="admin")
7.@ConfigProperty(value="db.user", onMissing="admin")
8.@ConfigProperty(value="db.user", ifMissing="admin")
First one is the only one that does not allow @ConfigProperty("db.user") .

a. ConfigProperty("db.user")
b. Config("db.user")

[joakime]

Why do we have this setup like this?

@Config(value="db.user", defaultValue="admin")
private String dbUser;

This implies a two-way configuration, one that can serialize configs (read and write w/defaults).
If we are going to go this way (which I'm not advocating for), then we should also include a description="this is the sales database user" (like other config systems with default value in the annotation)

Lets keep this simple and just skip the whole default value and just have the user default it in code.

@Config("db.user")
private String dbUser = "admin";

[tomas-langer]

Default values in code are tricky, as you may not be able to access that value from the implementation.
Also this is unusable for primitive types, where you cannot guess, if @Config("server.port") int port = 0 is a default value in code, or unconfigured value that should throw an exception

[joakime]

The specific example of ...

@Config("server.port")
int port = 0;

... is actually a valid example, and used in many contexts exactly like that.
Having port 0 defined means you want the OS to bind the port.
As a default, this one makes sense if I were writing a Server to use for unit testing on a shared CI system.

[joakime]

IMO, If you have a field defined with a @Config annotation, and having it not be accessible is a bug.
Something that the Jakarta Config implementation either warns (and does nothing to that field) or fails (either the whole start, the component init/lifecycle, the deployable) when seen.
It would need to be addressed by the developer for it to function properly, especially so on Java 9+ (where reflection rules are tighter, and JPMS makes it even more strict).

[Emily-Jiang]

I am with @tomas-langer on the default value. The config is directly injected on the bean creation. It is an injection target so the default string assigned is totally ignored. I prefer to keep the original flavour as it is clear and easy to understand.

[joakime]

Here's a sampling of other config system annotations that seem to overlap with this discussion.

These both assume that the configuration is two-way, it can be read from multiple 1..n sources, and written / created / populated if it the config file doesn't exist, and also documentation and help systems can be generated from this information.

I'm intentionally not mentioning the exact project that uses these systems as I don't want to create a bias ("for" or "against") the concepts based on where/who they are used.
Both represent projects that are used by millions of users. (and I do mean users. not developers)

Example from config system that's been in use since 2005

    /**
     * @since <since-text>
     * @deprecated <deprecated-text>
     */
    @Parameter( name = "parameter",
                alias = "myAlias",
                property = "a.property",
                defaultValue = "an expression, possibly with ${variables}",
                readonly = <false|true>,
                required = <false|true> )

Example from config system that's been in use since 2011

@Config( moduleId=<String>, // name of configuration source file
         name=<String>, // name of property in config
         type=<Type>, // used to indicate conversion/validation type - eg: String in properties file to validated timestamp that is in the past
         category=<String> ) // for grouping similar config items together when writing config
@Comment(<String[]>) // comment seen in written config
@Name(<String>) // name seen in config - alt usage for @Config(name="") - used mainly for config that crosses modules/groups (eg: work.dir)
@RangeInt(min=<int>, max=<int>) // validate valid int range
@RangeDouble(min=<double>, max=<double>) // validate valid double range
@LangKey(<String>) // expose lang/translate key for description
@RequiresRestart // used to indicate that a restart of the
                 // component is needed if config changes at runtime.
@Ignore // used to skip entry when processing config

I don't want to see Jakarta Config 1.0 have these concepts, lets get the minimal viable solution out first. We can discuss these "nice to have" features for version 2.0

[m0mus]

I am not sure that validation annotations are in Jakarta Config scope. Can we optionally integrate with Jakarta Bean Validation for this purpose?

[struberg]

The problem I do see with having separated annotations all belonging together is that it might be rather hard to properly mix&match them.

And +1 for the idea to use beanvalidation if we want to introduce validations at some point!

[m0mus]

I like the idea of a single annotation too. It defines a scope and all possible options within that scope. It means that in your IDE you press a button to get all possible combination of parameters to choose from. IMO, it's more user friendly then using different annotations, assuming that some of them may come from different third-party packages.

[Emily-Jiang]

I prefer the annotation @ConfigProperty(name="myName", defaultValue="myValue") as it is clear and easy to understand. I am not a big fun of multiple annotations. The other thing is that if we put name=, it clearly means it is the name of the config.

[joakime]

Except that a String default value cannot convey the default properly in all use cases.

[joakime]

Using this technique you'll wind up with the same thing that Maven and Gradle had when they did this too (with annotations like this).
If you have a complex object to configure, you'll have to use multiple fields to handle it.
One field that is the entry point of config, and a lifecycle that uses that entrypoint to populate the actual field with the complex value.
You'll also wind up having to define a lifecycle for config now, and failures can happen later (config everything on object instance, then trigger lifecycle to normalize the config, then the object is ready for use).

[radcortez]

@ConfigProperty(value="", fallback="") provides a way to use the annotation shorthand value and fallback is a reasonable name to use for a fallback value. One of the issues that I have with defaultValue is that gives the impression that it sets a default for that particular configuration name when usually that is only valid for that injection point.

There are some cases for defaults that we should probably discuss and the result of that discussion may influence how we design the annotation. I'll add a few of them in a separate comment.

[radcortez]

Scenarios to consider for defaults:

• Consider that these examples do not have any setup configuration and need to fallback to defaults.
• Using the current annotation coming from MP Config.

1. Injected default provides a value to programmatic lookup?

@Inject
@ConfigProperty(name = "foo", defaultValue = "foo")
String foo;

ConfigProvider.getConfig().getValue("foo", String.class);

1.1 If yes how about when you get two injection points to the same key name?

@Inject
@ConfigProperty(name = "foo", defaultValue = "foo")
String foo;

@Inject
@ConfigProperty(name = "foo", defaultValue = "anotherFoo")
String anotherFoo;

ConfigProvider.getConfig().getValue("foo", String.class);

1.2 Same issue for expansion:

@Inject
@ConfigProperty(name = "foo", defaultValue = "foo")
String foo;

@Inject
@ConfigProperty(name = "foo", defaultValue = "anotherFoo")
String anotherFoo;

@Inject
@ConfigProperty(name = "bar", defaultValue = "${foo}")
String bar;

If we consider that the default is local to the injection point only, the rules are clear and remove a lot of the ambiguity. On the other hand, what I have observed is that users expect the default to be generally available when set at the injection point. Maybe one reason is how we perceive the meaning of a default. If the defaultValue ends up being local to the injection point then the annotation field name should better represent that behaviour, like fallback or ifMissing or some other name that doesnt include default.

If the defaultValue is indeed a source of general configuration contribution, then we need to define some rules on how to handle these cases. An obvious approach is to just fail the deployment if multiple injection points for the same configuration name have different defaults (which seems a resonable behaviour).

I also have some thoughts about complex objects but before going there we should probably discuss and decide the simple cases first and then adjust if possible.

[m0mus]

I think that the default value is always local to the injection point. It's fine to use different defaults for different injection points and it's a legit use case. If we want to introduce global defaults which are used across all injection points then it should be a part of the configuration meta-configuration or the schema. I don't think that we should go that far.

[radcortez]

I'm fine with that behaviour, but then we need to be very clear that these annotation defaults are only local to the injection point and do not work if you are doing a programmatic lookup. In this case, I believe that default is a misleading annotation configuration. We may end up with it anyway, but we may also try to think if there is a better way to describe this behaviour through the API.

[hantsy]

I noticed there is a Converter used with annotations, if possible to add a common ConversionService for all conversion cases, as a common fallbacks. eg. Config, JPA, JSF, etc. have their own converters, and make them also search the common ConversionService and adapt the common converters if not found in their parts.

[struberg]

Hi @hantsy we did discuss this originally if it makes sense to even start a Converter spec approach. But finally (10 years ago) we decided against this because Config only converts from String to a target type, but never the other way around.

[keilw]

I remember @jodastephen had all sorts of ideas for new JSRs (or later JEPs) like Converters (based on his JodaConvert) or JavaBeans 2.0 but none of that ever happened with the new "Lean" OpenJDK.