Skip to content
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

Add support for property remapping directly in interface #1077

Open
dmlloyd opened this issue Jan 3, 2024 · 5 comments
Open

Add support for property remapping directly in interface #1077

dmlloyd opened this issue Jan 3, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@dmlloyd
Copy link
Contributor

dmlloyd commented Jan 3, 2024

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");
    }
}
@dmlloyd dmlloyd added the enhancement New feature or request label Jan 3, 2024
@magicprinc
Copy link

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
Copy link
Member

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
Copy link
Contributor Author

dmlloyd commented Jan 16, 2024

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
Loading

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
Copy link

magicprinc commented Jan 16, 2024

@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 #1091

@dmlloyd
Copy link
Contributor Author

dmlloyd commented Jan 17, 2024

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants