smallrye / smallrye-config

SmallRye Config - A Java Configuration library
Apache License 2.0
156 stars 116 forks source link

Add support for property remapping directly in interface #1077

Open dmlloyd opened 8 months ago

dmlloyd commented 8 months ago

It is seemingly common to deprecate, remap, and/or relocate configuration properties. The configuration interface should have some way to do this. The exact mechanism would need to be discussed, and might end up relying on #767 or similar.

Something like this API sketch could work (note that the remapped configuration names are absolute, which might not be good):

public interface FooBarConfig {
    @WithRemapping(MyRemapper.class)
    String helloWorld();
    // ...
}
/*
 * The set of names that are remapped by this mapper.
 * Instead of causing an error when these properties are encountered,
 * a warning will be issued with the name(s) of the property or properties
 * that replace the remapped property.
 */
@RemapNames("foo.bar.hello-universe")
public final class MyRemapper implements ConfigRemapper {
    /**
     * {@return the remapped value for the corresponding property}
     * This method is only called if there is no user-supplied (non-default) value
     * for the remapped property.
     * The supplied context is the initial context for the configuration.
     * Remapped names must never have an entry in the default values configuration source.
     */
    @Override
    public ConfigValue remap(ConfigSourceInterceptorContext context) {
        ConfigValue mapped = context.proceed("foo.bar.hello-universe");
        return mapped == null ? null : mapped.withName("foo.bar.hello-world");
    }
}
magicprinc commented 7 months ago

A better out-of-box property name finder would also be very welcomed. I mean, something like Spring Boot has: https://docs.spring.io/spring-boot/docs/3.1.x/reference/htmlsingle/#features.external-config.typesafe-configuration-properties.relaxed-binding

radcortez commented 7 months ago

I mean, something like Spring Boot has: https://docs.spring.io/spring-boot/docs/3.1.x/reference/htmlsingle/#features.external-config.typesafe-configuration-properties.relaxed-binding

While I understand what relaxed binding tries to achieve, I believe it doesn't adjust to our way of handling the configuration properties.

Since our sources are layered, it would require multiple lookups on each source to find if a specific property is or is not available. Most lookups are misses, and I'm worried about adding these additional lookups. A properties-based Config source is fast, but other sources, like Vault, are slower in performing the lookups.

We also rely on the list of property names to populate dynamic structures like Map. If the list contains multiple candidates for the same key, we need to know where the key is coming from and define some ordering rules.

The user should also have a defined and unique representation of a configuration name. While I agree that a loosely binding name is convenient, it may also cause hard-to-spot issues. For instance, a method name fooBar and foo_bar are different in Java, but with the loosely binding rules fooBar and foo_bar for configuration, it will be the same.

dmlloyd commented 7 months ago

Using the layered strategy, it seems to me that the only "correct" way to do this is to have two layers do the remapping. Like this:

flowchart LR
    Any1("...")
    Any2("...")
    Any3("...")
    RemovalInterceptor("Removal interceptor")
    RemapInterceptor("Remapping interceptor")
    Sources("Sources")
    Defaults("Defaults")
    Any1-->RemovalInterceptor
    RemovalInterceptor-->Any2
    Any2-->Sources
    Sources-->Any3
    Any3-->RemapInterceptor
    RemapInterceptor-->Defaults

In this diagram, the job of the "Removal interceptor", which is near the front of the chain, is to remove properties which correspond to "old" or "compatibility" names, so that only the "new" name(s) appear. The job of the "Remapping interceptor", which is at the end of the chain just before defaults, is to respond to requests for the "new" name by restarting with the "old" name. This way, the lookup for the "old" name(s) only happens when the "new" name(s) are not present in any non-defaults source, and as far as the configuration frontend is concerned, the old name(s) will never be listed in a config source (though they can still be directly queried by name).

magicprinc commented 7 months ago

@radcortez I agree with you: my experience shows: the best property name is the property/field name from the class.

Advantages: 1) you can simply copy-paste it 2) "Find in files" works excellent 3) You can configure your annotations to get the name of the field and use it An example from my Spring integration:

@ConfigProperty // name = beanName.fieldName
int timeout4;

I actually have only one Interceptor: FallbackConfigSourceInterceptor with kebab-to-camel transformation! And this only because default ConfigMapping#namingStrategy == NamingStrategy.KEBAB_CASE

See more about it here https://github.com/smallrye/smallrye-config/issues/1091

dmlloyd commented 7 months ago

I'm currently experimenting with a solution that looks something like this:

@Remap("quarkus.native.enabled") // respond to this name/pattern...
@Masks("quarkus.package.type") // ...whenever this property is found
public static ConfigValue quarkusNativeEnabled(ConfigSourceInterceptorContext ctxt) {
    ConfigValue oldVal = ctxt.restart("quarkus.package.type");
    if (oldVal == null) {
        ctxt.proceed(name);
    }
    // works like a slightly-higher-priority default value
    return oldVal.withName("quarkus.native.enabled").withValue(
        Boolean.toString(oldVal.getValue().equals("native") || oldVal.getValue().equals("native-sources")));
}

@Remap("quarkus.native.sources-only") // respond to this name/pattern...
@Masks("quarkus.package.type") // ...whenever this property is found
public static ConfigValue quarkusNativeSourcesOnly(ConfigSourceInterceptorContext ctxt) {
    ConfigValue oldVal = ctxt.restart("quarkus.package.type");
    if (oldVal == null) {
        ctxt.proceed(name);
    }
    return oldVal.withName("quarkus.native.sources-only").withValue(
        Boolean.toString(oldVal.getValue().equals("native-sources")));
}

This only covers some cases though. In particular I am still trying to figure out a good way to handle (possibly patterned) properties which have simply relocated (for example quarkus.package.manifest.* to quarkus.package.jar.manifest.*). The challenge as mentioned above is that there is a need for a bidirectional mapping: the old property to the new property (for rewriting property name iterations) and the new property to the old property (to handle getting the value itself).